Skip to content

[Validator] Fix caching of constraints derived from non-serializable parents #20793

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

Closed
wants to merge 3 commits into from

Conversation

uwej711
Copy link
Contributor

@uwej711 uwej711 commented Dec 6, 2016

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #12302
License MIT
Doc PR -

This change allows to still cache constraints even when an uncachable constraint (i.e. Callback)
is added to a parent class at runtime.

This is achived by caching only the constraints that are loaded for the class in question only
and merging parent and interface constraints after reading from cache.

This change allows to still cache constraints even when an uncachable constraint (i.e. Callback)
is added to a parent class at runtime.

This is achived by caching only the constraints that are loaded for the class in question only
and merging parent and interface constraints after reading from cache.
@dunglas
Copy link
Member

dunglas commented Dec 7, 2016

The Travis error looks related to your patch.

@uwej711
Copy link
Contributor Author

uwej711 commented Dec 7, 2016

Had only PHP 5.5 to test locally ...

@uwej711
Copy link
Contributor Author

uwej711 commented Dec 7, 2016

Oh dear, to early in the morning to spot the same two lines further down ...

return $this->loadedClasses[$class] = $metadata;
}

private function mergeConstraints(ClassMetadata $metadata)
Copy link
Member

Choose a reason for hiding this comment

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

I think I would use a name like mergeConstraintFromParentClasses().

@xabbuh
Copy link
Member

xabbuh commented Dec 31, 2016

@uwej711 The change itself looks good to me. However, this will only solve the issue mentioned in #12302 (comment), right?

@stof
Copy link
Member

stof commented Jan 12, 2017

looks good to me. 👍

@nicolas-grekas
Copy link
Member

👍

@nicolas-grekas nicolas-grekas changed the title [Validator] Fix issue 12302 - cache constraints [Validator] Fix caching of constraints derived from non-serializable parents Jan 12, 2017
@nicolas-grekas
Copy link
Member

Thank you @uwej711.

nicolas-grekas added a commit that referenced this pull request Jan 12, 2017
…rializable parents (uwej711)

This PR was squashed before being merged into the 2.7 branch (closes #20793).

Discussion
----------

[Validator] Fix caching of constraints derived from non-serializable parents

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #12302
| License       | MIT
| Doc PR        | -

This change allows to still cache constraints even when an uncachable constraint (i.e. Callback)
is added to a parent class at runtime.

This is achived by caching only the constraints that are loaded for the class in question only
and merging parent and interface constraints after reading from cache.

Commits
-------

9dd6b0c [Validator] Fix caching of constraints derived from non-serializable parents
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.

6 participants