Merge release 2.4.0 into 3.0.x #481
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows us to switch to failing on deprecation.
Configure PHPUnit to fail on doctrine deprecations
Bumps [doctrine/.github](https://github.com/doctrine/.github) from 8.0.0 to 11.0.0. - [Release notes](https://github.com/doctrine/.github/releases) - [Commits](doctrine/.github@8.0.0...11.0.0) --- updated-dependencies: - dependency-name: doctrine/.github dependency-version: 11.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
…ine/dot-github-11.0.0 Bump doctrine/.github from 8.0.0 to 11.0.0
Because the de-duplication of deprecation notices was disabled by default, not all actual deprecations were reported after #473.
Fix reporting of deprecations in tests
Merge 2.3.x up into 2.4.x
# Motivation There are a few issues about differences in behaviour when using the collection filtering API (the `Selectable` interface) against collections that are database-backed (in ORM, these are `PersistentCollections`) vs. memory-based collections using `ArrayCollection` from this package here. For example: * doctrine/orm#11160 * #170 * doctrine/orm#3591 * Maybe doctrine/orm#11021 Database-based matching can work on the raw field values only, as those values are persisted to the database and there is no PHP code involved when filtering at the database level. Memory-based matching currently tries a [series of access methods on the objects](https://www.doctrine-project.org/projects/doctrine-collections/en/2.3/index.html#selectable-methods:~:text=For%20collections%20that%20contain%20objects). The effects of this may be surprising. For example, with the ORM, it may be fine to filter entities based on the value of `private` or `protected` fields that have no getters. This works as long as a persistent collection is uninitialized. But as soon as it gets initialized, the `ArrayCollection` will require a getter method to be available. Another (more rare) example is a getter method that does some type of type conversion, like having a `string` field with values like `'y'|'n'` internally but returning a `bool` value from the getter; or, more generally, every type mismatch between the return value of the getter and the field value. Yet another example may be getters that cause side effects 🙈. # Proposed solution I discussed with @beberlei at the Doctrine hackathon that the primary use case for this library here was supporting the ORM/ODM use cases. This can be seen in places as `ClosureExpressionVisitor::getObjectFieldValue()` that take a `$field` parameter. So, although this library here has nothing to do with ORM/ODM mapping, I want to add a migration path here that moves the `doctrine/collection` behaviour closer to the implementation realities of ORM/ODM. This means to ultimately use direct (reflection-based) field access only. This is also what @Ocramius already suggested in [this comment](#149 (comment)). For reference, here is a list of discussions around which style of accessors, getters, issers, public access etc. should be used or not used – in the future, the answer would be "only direct state (raw property value) matters". * #276 * #263 * #149 * #135 * #134 * #95 * #62 # Migration path This feature is opt-in and will be activated by passing `accessRawFieldValues: true` to the `Criteria` constructor. The `Criteria` object is what is typically constructed by users in preparation for calling the `Selectable` API, so it seems to be a good fit. By opting in through this flag, memory-based comparisons and sorting will use direct field access only. Not activating the feature triggers a deprecation notice. In the next major version, direct field access will be the only (default) behaviour. The `$accessRawFieldValues` can be removed in the next major version (or, possibly, go through another round of deprecations in case when it is still passed, before being eventually removed). # Remaining edge case Given an inheritance hierarchy of classes where a multiple classes feature a `private` field of the same name, the downmost field will be picked. This may differ from Doctrine ORM behaviour when this field is not mapped at all in the ORM and another field (higher up the class hierarchy) is used as the mapped field instead. We cannot solve this without having access to ORM/ODM mapping metadata at hand, which is not possible from within an `ArrayCollection` that is typically created as [a newable type](https://testing.googleblog.com/2008/10/to-new-or-not-to-new.html). We rather plan to discourage or even prevent this kind of setup (entity class hierarchy with different classes having fields of the same name) at some point when loading and validating metadata.
Transition to using reflection-based, direct field access only
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release Notes for 2.4.0
Feature release (minor)
2.4.0
Deprecation
472: Transition to using reflection-based, direct field access only thanks to @mpdude
453: Utilizing Expression::visit method instead of match true thanks to @CloneException