Skip to content

[Validator] Optimize serialized metadata and cleanup tests #61448

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.

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 1 commit into from
Aug 18, 2025

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Aug 18, 2025

Q A
Branch? 7.4
Bug fix? no
New feature? no
Deprecations? no
Issues -
License MIT

Our tests rely on direct access to @internal properties that come with notices like:

This property is public in order to reduce the size of the class' serialized representation. Do not access it. Use [...] instead

This should also apply to test cases.

Also, now that we use __serialize, we can also make these properties private. I'll propose it in a follow up PR.

(failures unrelated)

'constraintsByGroup' => $this->constraintsByGroup,
'traversalStrategy' => $this->traversalStrategy,
'autoMappingStrategy' => $this->autoMappingStrategy,
return array_filter(parent::__serialize() + [
Copy link
Member Author

@nicolas-grekas nicolas-grekas Aug 18, 2025

Choose a reason for hiding this comment

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

use array_filter to remove properties that are still at their default values.

$constraint->addImplicitGroupName($this->getDefaultGroup());
$member->addConstraint($constraint);
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of accessing constraintsByGroup directly

@@ -183,10 +183,14 @@ public function addConstraint(Constraint $constraint): static
return $this;
}

$this->constraints[] = $constraint;
if (!\in_array($constraint, $this->constraints, true)) {
Copy link
Member Author

@nicolas-grekas nicolas-grekas Aug 18, 2025

Choose a reason for hiding this comment

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

enables adding a constraint twice to bind it to new groups
used by ClassMetadata on L377

@@ -310,8 +310,6 @@ public function getTargets(): string|array

/**
* Optimizes the serialized value to minimize storage space.
*
* @internal
Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed anymore, __serialize is what the engine provides to hook on serialization, we cannot "privatize" it IMHO

@nicolas-grekas nicolas-grekas force-pushed the v-cleanup branch 2 times, most recently from 035d763 to bd5c311 Compare August 18, 2025 08:45
@nicolas-grekas nicolas-grekas merged commit 034386e into symfony:7.4 Aug 18, 2025
3 of 12 checks passed
@nicolas-grekas nicolas-grekas deleted the v-cleanup branch August 18, 2025 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants