Skip to content

Commit

Permalink
bug #58705 [Serializer] Revert Default groups (mtarld)
Browse files Browse the repository at this point in the history
This PR was merged into the 7.1 branch.

Discussion
----------

[Serializer] Revert Default groups

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #58576, Fix #57350
| License       | MIT

When introduced in #51514, the behavior of group selection was wrong and introduced BC breaks (see the two related issues).

This PR reverts this introduction so that they can be added properly in 7.3 (see #58656).

Commits
-------

ab3220f6cd5 [Serializer] Revert default groups
  • Loading branch information
fabpot committed Nov 9, 2024
2 parents 2154d9a + 6e7d9f9 commit 763e313
Show file tree
Hide file tree
Showing 9 changed files with 8 additions and 107 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ CHANGELOG

* Add arguments `$class`, `$format` and `$context` to `NameConverterInterface::normalize()` and `NameConverterInterface::denormalize()`
* Add `DateTimeNormalizer::CAST_KEY` context option
* Add `Default` and "class name" default groups
* Add `AbstractNormalizer::FILTER_BOOL` context option
* Add `CamelCaseToSnakeCaseNameConverter::REQUIRE_SNAKE_CASE_PROPERTIES` context option
* Deprecate `AbstractNormalizerContextBuilder::withDefaultContructorArguments(?array $defaultContructorArguments)`, use `withDefaultConstructorArguments(?array $defaultConstructorArguments)` instead (note the missing `s` character in Contructor word in deprecated method)
Expand Down
7 changes: 2 additions & 5 deletions NameConverter/MetadataAwareNameConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,13 @@ private function getCacheValueForAttributesMetadata(string $class, array $contex
}

$metadataGroups = $metadata->getGroups();

$contextGroups = (array) ($context[AbstractNormalizer::GROUPS] ?? []);
$contextGroupsHasBeenDefined = [] !== $contextGroups;
$contextGroups = array_merge($contextGroups, ['Default', (false !== $nsSep = strrpos($class, '\\')) ? substr($class, $nsSep + 1) : $class]);

if ($contextGroupsHasBeenDefined && !$metadataGroups) {
if ($contextGroups && !$metadataGroups) {
continue;
}

if ($metadataGroups && !array_intersect(array_merge($metadataGroups, ['*']), $contextGroups)) {
if ($metadataGroups && !array_intersect($metadataGroups, $contextGroups) && !\in_array('*', $contextGroups, true)) {
continue;
}

Expand Down
11 changes: 3 additions & 8 deletions Normalizer/AbstractNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -223,32 +223,27 @@ protected function getAllowedAttributes(string|object $classOrObject, array $con
return false;
}

$classMetadata = $this->classMetadataFactory->getMetadataFor($classOrObject);
$class = $classMetadata->getName();

$groups = $this->getGroups($context);
$groupsHasBeenDefined = [] !== $groups;
$groups = array_merge($groups, ['Default', (false !== $nsSep = strrpos($class, '\\')) ? substr($class, $nsSep + 1) : $class]);

$allowedAttributes = [];
$ignoreUsed = false;

foreach ($classMetadata->getAttributesMetadata() as $attributeMetadata) {
foreach ($this->classMetadataFactory->getMetadataFor($classOrObject)->getAttributesMetadata() as $attributeMetadata) {
if ($ignore = $attributeMetadata->isIgnored()) {
$ignoreUsed = true;
}

// If you update this check, update accordingly the one in Symfony\Component\PropertyInfo\Extractor\SerializerExtractor::getProperties()
if (
!$ignore
&& (!$groupsHasBeenDefined || array_intersect(array_merge($attributeMetadata->getGroups(), ['*']), $groups))
&& ([] === $groups || \in_array('*', $groups, true) || array_intersect($attributeMetadata->getGroups(), $groups))
&& $this->isAllowedAttribute($classOrObject, $name = $attributeMetadata->getName(), null, $context)
) {
$allowedAttributes[] = $attributesAsString ? $name : $attributeMetadata;
}
}

if (!$ignoreUsed && !$groupsHasBeenDefined && $allowExtraAttributes) {
if (!$ignoreUsed && [] === $groups && $allowExtraAttributes) {
// Backward Compatibility with the code using this method written before the introduction of @Ignore
return false;
}
Expand Down
24 changes: 0 additions & 24 deletions Tests/Fixtures/Attributes/GroupDummy.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ class GroupDummy extends GroupDummyParent implements GroupDummyInterface
protected $quux;
private $fooBar;
private $symfony;
#[Groups(['Default'])]
private $default;
#[Groups(['GroupDummy'])]
private $className;

#[Groups(['b'])]
public function setBar($bar)
Expand Down Expand Up @@ -84,24 +80,4 @@ public function setQuux($quux): void
{
$this->quux = $quux;
}

public function setDefault($default)
{
$this->default = $default;
}

public function getDefault()
{
return $this->default;
}

public function setClassName($className)
{
$this->className = $className;
}

public function getClassName()
{
return $this->className;
}
}
8 changes: 0 additions & 8 deletions Tests/Mapping/TestClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,6 @@ public static function createClassMetadata(string $namespace, bool $withParent =
$symfony->addGroup('name_converter');
}

$default = new AttributeMetadata('default');
$default->addGroup('Default');
$expected->addAttributeMetadata($default);

$className = new AttributeMetadata('className');
$className->addGroup('GroupDummy');
$expected->addAttributeMetadata($className);

// load reflection class so that the comparison passes
$expected->getReflectionClass();

Expand Down
56 changes: 3 additions & 53 deletions Tests/Normalizer/Features/GroupsTestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,9 @@ public function testGroupsNormalize()
$obj->setSymfony('symfony');
$obj->setKevin('kevin');
$obj->setCoopTilleuls('coopTilleuls');
$obj->setDefault('default');
$obj->setClassName('className');

$this->assertEquals([
'bar' => 'bar',
'default' => 'default',
'className' => 'className',
], $normalizer->normalize($obj, null, ['groups' => ['c']]));

$this->assertEquals([
Expand All @@ -52,54 +48,18 @@ public function testGroupsNormalize()
'bar' => 'bar',
'kevin' => 'kevin',
'coopTilleuls' => 'coopTilleuls',
'default' => 'default',
'className' => 'className',
], $normalizer->normalize($obj, null, ['groups' => ['a', 'c']]));

$this->assertEquals([
'default' => 'default',
'className' => 'className',
], $normalizer->normalize($obj, null, ['groups' => ['unknown']]));

$this->assertEquals([
'quux' => 'quux',
'symfony' => 'symfony',
'foo' => 'foo',
'fooBar' => 'fooBar',
'bar' => 'bar',
'kevin' => 'kevin',
'coopTilleuls' => 'coopTilleuls',
'default' => 'default',
'className' => 'className',
], $normalizer->normalize($obj));
}

public function testGroupsDenormalize()
{
$normalizer = $this->getDenormalizerForGroups();

$obj = new GroupDummy();
$obj->setDefault('default');
$obj->setClassName('className');

$data = [
'foo' => 'foo',
'bar' => 'bar',
'quux' => 'quux',
'default' => 'default',
'className' => 'className',
];

$denormalized = $normalizer->denormalize(
$data,
GroupDummy::class,
null,
['groups' => ['unknown']]
);
$this->assertEquals($obj, $denormalized);

$obj->setFoo('foo');

$data = ['foo' => 'foo', 'bar' => 'bar'];

$denormalized = $normalizer->denormalize(
$data,
GroupDummy::class,
Expand All @@ -117,11 +77,6 @@ public function testGroupsDenormalize()
['groups' => ['a', 'b']]
);
$this->assertEquals($obj, $denormalized);

$obj->setQuux('quux');

$denormalized = $normalizer->denormalize($data, GroupDummy::class);
$this->assertEquals($obj, $denormalized);
}

public function testNormalizeNoPropertyInGroup()
Expand All @@ -130,12 +85,7 @@ public function testNormalizeNoPropertyInGroup()

$obj = new GroupDummy();
$obj->setFoo('foo');
$obj->setDefault('default');
$obj->setClassName('className');

$this->assertEquals([
'default' => 'default',
'className' => 'className',
], $normalizer->normalize($obj, null, ['groups' => ['notExist']]));
$this->assertEquals([], $normalizer->normalize($obj, null, ['groups' => ['notExist']]));
}
}
2 changes: 0 additions & 2 deletions Tests/Normalizer/GetSetMethodNormalizerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,6 @@ public function testGroupsNormalizeWithNameConverter()
'bar' => null,
'foo_bar' => '@dunglas',
'symfony' => '@coopTilleuls',
'default' => null,
'class_name' => null,
],
$this->normalizer->normalize($obj, null, [GetSetMethodNormalizer::GROUPS => ['name_converter']])
);
Expand Down
2 changes: 0 additions & 2 deletions Tests/Normalizer/ObjectNormalizerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,6 @@ public function testGroupsNormalizeWithNameConverter()
'bar' => null,
'foo_bar' => '@dunglas',
'symfony' => '@coopTilleuls',
'default' => null,
'class_name' => null,
],
$this->normalizer->normalize($obj, null, [ObjectNormalizer::GROUPS => ['name_converter']])
);
Expand Down
4 changes: 0 additions & 4 deletions Tests/Normalizer/PropertyNormalizerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,6 @@ public function testNormalizeWithParentClass()
'fooBar' => null,
'symfony' => null,
'baz' => 'baz',
'default' => null,
'className' => null,
],
$this->normalizer->normalize($group, 'any')
);
Expand Down Expand Up @@ -321,8 +319,6 @@ public function testGroupsNormalizeWithNameConverter()
'bar' => null,
'foo_bar' => '@dunglas',
'symfony' => '@coopTilleuls',
'default' => null,
'class_name' => null,
],
$this->normalizer->normalize($obj, null, [PropertyNormalizer::GROUPS => ['name_converter']])
);
Expand Down

0 comments on commit 763e313

Please sign in to comment.