8000 [Validator] deprecate member metadata accessors by Tobion · Pull Request #11703 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Validator] deprecate member metadata accessors #11703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 31, 2014
Merged

[Validator] deprecate member metadata accessors #11703

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04eb61b
[Validator] deprecate member metadata accessors in favor of existing …
Tobion Aug 19, 2014
14d3f97
[Validator] add getConstraints to MetadataInterface
Tobion Aug 20, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[Validator] deprecate member metadata accessors in favor of existing … 
…property metadata accessors

add changelog for deprecations

fix test
  • Loading branch information
@Tobion
Tobion committed Aug 19, 2014
commit 04eb61b80ed925d959e262d03d99060995f248b2
12 changes: 9 additions & 3 deletions src/Symfony/Component/Form/Extension/Validator/ValidatorTypeGuesser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
use Symfony\Component\Form\Guess\Guess;
use Symfony\Component\Form\Guess\TypeGuess;
use Symfony\Component\Form\Guess\ValueGuess;
use Symfony\Component\Validator\MetadataFactoryInterface;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\Mapping\ClassMetadataInterface;
use Symfony\Component\Validator\Mapping\GenericMetadata;
use Symfony\Component\Validator\MetadataFactoryInterface;

class ValidatorTypeGuesser implements FormTypeGuesserInterface
{
Expand Down Expand Up @@ -264,10 +266,14 @@ protected function guess($class, $property, \Closure $closure, $defaultValue = n
$guesses = array();
$classMetadata = $this->metadataFactory->getMetadataFor($class);

if ($classMetadata->hasMemberMetadatas($property)) {
$memberMetadatas = $classMetadata->getMemberMetadatas($property);
if ($classMetadata instanceof ClassMetadataInterface && $classMetadata->hasPropertyMetadata($property)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the metadata for a class should always be a ClassMetadataInterface so I don't think this check is necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's necessary because the interface only says it returns an MetadataInterface. Everything else is implementation detail we cannot trust.

$memberMetadatas = $classMetadata->getPropertyMetadata($property);

foreach ($memberMetadatas as $memberMetadata) {
if (!$memberMetadata instanceof GenericMetadata) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in Symfony 2.4 and lower, getConstraints is available on the class without it extending GenericMetadata (which does not exist). So you should check for \Symfony\Component\Validator\Mapping\ElementMetadata as well (otherwise you make the Form guesser of 2.6 incompatible with Validator 2.3 and 2.4 while composer says ~2.3)

continue;
}

$constraints = $memberMetadata->getConstraints();

foreach ($constraints as $constraint) {
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Form/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"symfony/property-access": "~2.3"
},
"require-dev": {
"symfony/validator": "~2.2",
"symfony/validator": "~2.3",
"symfony/http-foundation": "~2.2",
"symfony/http-kernel": "~2.4",
"symfony/security-csrf": "~2.4",
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/Validator/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ CHANGELOG
* [BC BREAK] `UserPasswordValidator` source message change
* [BC BREAK] added internal `ExecutionContextInterface::setConstraint()`
* added `ConstraintViolation::getConstraint()`
* deprecated `ClassMetadata::hasMemberMetadatas()`
* deprecated `ClassMetadata::getMemberMetadatas()`
* deprecated `ClassMetadata::addMemberMetadata()`

2.5.0
-----
Expand Down
48 changes: 31 additions & 17 deletions src/Symfony/Component/Validator/Mapping/ClassMetadata.php
View file
Open in desktop
67ED
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public function accept(ValidationVisitorInterface $visitor, $value, $group, $pro
$pathPrefix = empty($propertyPath) ? '' : $propertyPath.'.';

foreach ($this->getConstrainedProperties() as $property) {
foreach ($this->getMemberMetadatas($property) as $member) {
foreach ($this->getPropertyMetadata($property) as $member) {
$member->accept($visitor, $member->getPropertyValue($value), $group, $pathPrefix.$property, $propagatedGroup);
}
}
Expand Down Expand Up @@ -268,7 +268,7 @@ public function addPropertyConstraint($property, Constraint $constraint)
if (!isset($this->properties[$property])) {
$this->properties[$property] = new PropertyMetadata($this->getClassName(), $property);

$this->addMemberMetadata($this->properties[$property]);
$this->addPropertyMetadata($this->properties[$property]);
}

$constraint->addImplicitGroupName($this->getDefaultGroup());
Expand All @@ -294,7 +294,7 @@ public function addGetterConstraint($property, Constraint $constraint)
if (!isset($this->getters[$property])) {
$this->getters[$property] = new GetterMetadata($this->getClassName(), $property);

$this->addMemberMetadata($this->getters[$property]);
$this->addPropertyMetadata($this->getters[$property]);
}

$constraint->addImplicitGroupName($this->getDefaultGroup());
Expand All @@ -316,16 +316,18 @@ public function mergeConstraints(ClassMetadata $source)
}

foreach ($source->getConstrainedProperties() as $property) {
foreach ($source->getMemberMetadatas($property) as $member) {
foreach ($source->getPropertyMetadata($property) as $member) {
$member = clone $member;

foreach ($member->getConstraints() as $constraint) {
$constraint->addImplicitGroupName($this->getDefaultGroup());
if ($member instanceof GenericMetadata) {
foreach ($member->getConstraints() as $constraint) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webmozart should getConstraints be part of the MetadataInterface rather than being only in GenericMetadata ? Otherwise, we are technically forbidding to use any other implementation than GenericMetadata in 2.5+

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think that's a good change. @Tobion can you add this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In the future we would also need to replace typehints to ClassMetadata with ClassMetadataInterface. At this moment, also addConstraint, mergeConstraints etc would need to be part of the interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

$constraint->addImplicitGroupName($this->getDefaultGroup());
}
}

$this->addMemberMetadata($member);
$this->addPropertyMetadata($member);

if (!$member->isPrivate($this->name)) {
if ($member instanceof MemberMetadata && !$member->isPrivate($this->name)) {
$property = $member->getPropertyName();

if ($member instanceof PropertyMetadata && !isset($this->properties[$property])) {
Expand All @@ -342,12 +344,12 @@ public function mergeConstraints(ClassMetadata $source)
* Adds a member metadata.
*
* @param MemberMetadata $metadata
*
* @deprecated Deprecated since version 2.6, to be removed in 3.0.
*/
protected function addMemberMetadata(MemberMetadata $metadata)
{
$property = $metadata->getPropertyName();

$this->members[$property][] = $metadata;
$this->addPropertyMetadata($metadata);
}

/**
Expand All @@ -356,10 +358,12 @@ protected function addMemberMetadata(MemberMetadata $metadata)
* @param string $property The name of the property
*
* @return bool
*
* @deprecated Deprecated since version 2.6, to be removed in 3.0. Use {@link hasPropertyMetadata} instead.
*/
public function hasMemberMetadatas($property)
{
return array_key_exists($property, $this->members);
return $this->hasPropertyMetadata($property);
}

/**
Expand All @@ -368,14 +372,12 @@ public function hasMemberMetadatas($property)
* @param string $property The name of the property
*
* @return MemberMetadata[] An array of MemberMetadata
*
* @deprecated Deprecated since version 2.6, to be removed in 3.0. Use {@link getPropertyMetadata} instead.
*/
public function getMemberMetadatas($property)
{
if (!isset($this->members[$property])) {
return array();
}

return $this->members[$property];
return $this->getPropertyMetadata($property);
}

/**
Expand Down Expand Up @@ -505,4 +507,16 @@ public function getCascadingStrategy()
{
return CascadingStrategy::NONE;
}

/**
* Adds a property metadata.
*
* @param PropertyMetadataInterface $metadata
*/
private function addPropertyMetadata(PropertyMetadataInterface $metadata)
{
$property = $metadata->getPropertyName();

$this->members[$property][] = $metadata;
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/Validator/Tests/Fixtures/FakeClassMetadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

class FakeClassMetadata extends ClassMetadata
{
public function addPropertyMetadata($propertyName, $metadata)
public function addCustomPropertyMetadata($propertyName, $metadata)
{
if (!isset($this->members[$propertyName])) {
$this->members[$propertyName] = array();
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Component/Validator/Tests/Mapping/ClassMetadataTest.php
10000
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public function testMergeConstraintsMergesMemberConstraints()
))),
);

$members = $this->metadata->getMemberMetadatas('firstName');
$members = $this->metadata->getPropertyMetadata('firstName');

$this->assertCount(1, $members);
$this->assertEquals(self::PARENTCLASS, $members[0]->getClassName());
Expand All @@ -112,8 +112,8 @@ public function testMemberMetadatas()
{
$this->metadata->addPropertyConstraint('firstName', new ConstraintA());

$this->assertTrue($this->metadata->hasMemberMetadatas('firstName'));
$this->assertFalse($this->metadata->hasMemberMetadatas('non_existant_field'));
$this->assertTrue($this->metadata->hasPropertyMetadata('firstName'));
$this->assertFalse($this->metadata->hasPropertyMetadata('non_existant_field'));
}

public function testMergeConstraintsKeepsPrivateMembersSeparate()
Expand All @@ -138,7 +138,7 @@ public function testMergeConstraintsKeepsPrivateMembersSeparate()
))),
);

$members = $this->metadata->getMemberMetadatas('internal');
$members = $this->metadata->getPropertyMetadata('internal');

$this->assertCount(2, $members);
$this->assertEquals(self::PARENTCLASS, $members[0]->getClassName());
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Validator/Tests/Validator/Abstract2Dot5ApiTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ public function testPropertyMetadataMustImplementPropertyMetadataInterface()
// Legacy interface
$propertyMetadata = $this->getMock('Symfony\Component\Validator\MetadataInterface');
$metadata = new FakeClassMetadata(get_class($entity));
$metadata->addPropertyMetadata('firstName', $propertyMetadata);
$metadata->addCustomPropertyMetadata('firstName', $propertyMetadata);

$this->metadataFactory->addMetadata($metadata);

Expand Down
0