The order of results is not guaranteed unless we do so, and the test can
fail in some cases:
There was 1 failure:
1) Doctrine\Tests\ORM\Functional\QueryTest::testToIterableWithMixedResultArbitraryJoinsScalars
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Doctrine 2'
+'lala 2'
/home/runner/work/orm/orm/tests/Tests/ORM/Functional/QueryTest.php:481
Cover additional enumType behaviors: nullable columns, database default
values with enum cases, usage in DQL/QueryBuilder/findBy, and XML
mapping syntax with enum-type attribute.
The enumType option on #[Column] was barely mentioned in the docs and
had no dedicated section. This adds a complete reference covering
single-value columns, collection types (json, simple_array), automatic
type inference, validation behavior, and platform compatibility.
I think #11289 did not completely fix problem for eager fetch.
Change in that PR checked if primary key of target class is composite but that does not matter when loading collection by foreign key.
It should check if foreign key on target class is composite.
Fix from that PR did not work for me because i had entity with regular autogenerated id (single column), but foreign key referenced entity with composite primary key, like SecondLevelWithoutCompositePrimaryKey in this PR.
Checking if foreign key is composite fixed the problem for me.
* Do not eagerly set metadata from ResolveTargetEntityListener
When using the `ResolveTargetEntityListener` to substitute `targetEntities` in association mappings, do not eagerly put the resolved (target) entity into the class metadata cache under the class name of the original entity.
#### Motivation
I have a library that wants to distribute a MappedSuperclass as the base for some functionality. It will be necessary that clients using the library will extend the MappedSuperclass to fill in some blanks, creating the first real `#[Entity]` instance of it.
This client-provided entity will be the primary means of working with the class. Thus, I was following the [note in the documentation](https://www.doctrine-project.org/projects/doctrine-orm/en/3.5/reference/inheritance-mapping.html#mapped-superclasses:~:text=It%20is%2C%20however) and using the `ResolveTargetEntityListener` to declare that whenever an association refers to that mapped superclass, the particular entity class shall be used instead.
> One-To-Many associations are not generally possible on a mapped superclass, since they require the "many" side to hold the foreign key.
> It is, however, possible to use the [ResolveTargetEntityListener](https://www.doctrine-project.org/projects/doctrine-orm/en/3.5/cookbook/resolve-target-entity-listener.html) to replace references to a mapped superclass with an entity class at runtime. As long as there is only one entity subclass inheriting from the mapped superclass and all references to the mapped superclass are resolved to that entity class at runtime, the mapped superclass can use One-To-Many associations and be named as the targetEntity on the owning sides.
#### Changes made
The `ResolveTargetEntityListener` primarily does what its name suggests: For newly loaded class metadata, it inspects all associations declared and replaces the `targetEntity` with new (resolved) values.
But additionally, when a loaded class is the target of such a resolution, it would also put the class metadata into the cache under the name of the original entity.
I think that extra step is wrong, and this PR removes it. It had the side effect that when other classes extending the MappedSuperclass were loaded _after_ the resolve target class has been seen for the first time, the metadata for those classes would not inherit from the mapped superclass anymore, but from the target entity class instead. In my real-life use case, this causes weird mapping errors down the road; as of ^3.0, it would throw a mapping exception asking to configure inheritance mapping. But note that there would be no inheritance between the two entity classes at all.
#### More background
The documentation [describes the use of `ResolveTargetEntityListener`](https://www.doctrine-project.org/projects/doctrine-orm/en/3.5/cookbook/resolve-target-entity-listener.html) with an interface that is resolved to an entity class. For an interface, adding the extra metadata does not make a difference, since it never interferes with actual entity or mapped superclasses.
The initial idea of adding a copy of the entity class metadata under the interface name came from commit
9c7f3f2747 in #385. The goal was to make it possible to also find entities by interface names, like so:
```
$em->find('Foo\BarBundle\Entity\PersonInterface', 1);
```
It then [turned out that this only worked when the resolution had already been applied](https://github.com/doctrine/orm/pull/385#issuecomment-6658893). So, the new `onClassMetadataNotFound` event was added and the resolution map would be checked in that case as well (#1181). The inital code stayed in place, possibly giving a small performance gain.
In my real-world use case and the test case I added in this PR, the associations are even self-referencing. That should not really be necessary for the problem to surface. I decided to keep it this way to show that the `targetEntity` need not be an interface after all, and that a MappedSuperclass can be used in the same way.
This PR fixes the `Criteria` matching API for `IN` and `NIN` conditions with values that are arrays, by making sure that type information for the matched field is passed to the DBAL level correctly.
Passing the right parameter type to DBAL is important to make sure parameter conversions are applied before matching at the database level.
Memory-based collections (`ArrayCollection`s or initialized collection fields) would perform matching on the objects in memory where no type conversion to the database representation is required, giving correct results.
But uninitialized collections that have their conditions evaluated at the database level need to convert parameter values to the database representation before performing the comparison.
One extra challenge is that the DBAL type system does currently not support array-valued parameters for custom types. Only a [limited list of types](https://www.doctrine-project.org/projects/doctrine-dbal/en/4.2/reference/data-retrieval-and-manipulation.html#list-of-parameters-conversion) is supported.
I discussed this with @morozov at the Doctrine Hackathon and came to the conclusion that it would be best to work around this limitation at the ORM level. Thus, this fix recognizes array-valued parameters and creates multiple placeholders (like `?, ?, ?`) for them, flattening out the arrays in the parameter list and repeating the type information for each one of them.
Previous stalled attempt to fix this was in #11897.
Fix JoinedSubclassPersister as BasicEntityPersister was already fixed in GH-10735.
The fix can be verified by modifying UnitOfWork to execute `BasicEntityPersister::executeInserts()` for multiple entities at once for the same entity class/persister instance - https://github.com/doctrine/orm/blob/2.20.3/src/UnitOfWork.php#L1186 - then reproducible on `Doctrine\Tests\ORM\Functional\Ticket\GH10531Test::testInserts` test.
As extending/modifying UnitOfWork in tests in not easily possible, I submit this fix for v2.x without a test.
When the persister is extended to do a multi update, the caching is not
wanted. The impact is minimal as the CPU/time overhead per query is
much bigger and the prepared statement is not cached anyway.
This command's purpose is to provide structured data, except for a call
to caution() that warns the user in case they do not have any mapped
entities or they have errors.
Symfony 7.3 is not available to all of our users, so we cannot switch to
native lazy objects, which require a PHP version higher than the lowest
PHP version we support.
In f256d996cc, I did a global move to
stderr for notifications, and went a bit overboard for
MappingDescribeCommand, which purpose is to output a description.
For some reason, it does not appear to work when nested inside a
code-block directive. Anyway, if you specify the language attribute, you
get markup identical to what you obtain when using code-block and
literalinclude, so this wrapping seems unneeded.
PostLoad listeners might initialize values for transient properties, so
the proxy should not skip initialization when using transient
properties.
Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>
The old proxy implementation of doctrine/common was triggered by public
methods rather than access to properties (making public properties
unsupported in entities), so tests could use public instance properties
to track the state of postLoad lifecycle callbacks without triggering
the proxy initialization when reading that state (which then changes the
state of triggering the postLoad callback).
As the new proxy implementation hooks into properties instead, the tests
now use a static method (ensuring it is reset properly before loading
the instance for which we care about the tracking) instead of an
instance property.
This fixes that using a `Criteria` with an `IN` or `NIN` expression on a to-many collection currently leads to an SQL error (#6173). The `ManyToMany` persister needs to know about the slightly different SQL syntax for `[NOT] IN ()`.
In the case of `[NOT] IN` expressions, the value will be an array, which also required me to change (I guess "fix") the parameter type handling. I have pulled the necessary code from the `BasicEntityPersister` and placed it as static helper methods in `PersisterHelper`.
This is somewhat inspired by #11516, which aims at fixing #11481: By re-using the parameter type handling code, it also fixes using backed enums in `EQ`, `IN` and `NIN` expressions within `Criteria` when `matching()` on one-to-many and many-to-many collections.
If scheduleForInsert was called in prePersist hook already, then persistNew need to check this case first, otherwise a ORMInvalidArgumentException will be thrown
This fixes a bug that arises when using Pagination and an entity relation is mapped with fetch-mode EAGER but setFetchMode LAZY (or anything that is not EAGER) has been used on the query. If the query use WITH condition, an exception is incorrectly raised (Associations with fetch-mode=EAGER may not be using WITH conditions).
The class LimitSubqueryOutputWalker clones the query, but not its parameters and hints, so the generated subquery does not know that fetch-mode has been overridden.
Fixes#11741
The bug related (#11694) and fixed mapping of sql column alias to field in entity (#11783) and
invalidate cache [cache/persisted/entity|cache/persisted/collection] when sql filter changes
* 2.20.x:
Introduce testNotListedValueInEnumArray
Fix documentation for JoinColumn nullable (#11798)
Ignore deprecations from doctrine/common
Fix fields of transient classes being considered duplicate with `reportFieldsWhereDeclared`
just one bracket (...) gives
Exception : [Doctrine\ORM\Query\QueryException] [Syntax Error] line 0, col xx: Error: Expected Doctrine\ORM\Query\Lexer::T_IDENTIFIER, got '('
When there are no conflicts between branches, we create pull requests
where the head branch is a branch on the origin repository. That branch
points to a commit that should already have coverage information
provided by the build that happens after merging a regular pull request.
The thing is, coverage information provided by builds that happen before
merging a pull request are associated with the commit of the head
repository. This means that when merging up 1.2 into 1.3, the build
produces coverage information that is the result of a merge between 1.2
and 1.3, and associates it with 1.2, although it is run on with a
codebase that is much closer to 1.3 (and is in fact supposed to become
1.3 after the merge).
This means that when we create a merge up PR from 1.2 to anything else,
the coverage information is going to be wrong until a PR targeting 1.2
gets merged.
I do not think we need coverage about conflictless merge up PRs more
than we need accurate numbers, so I propose we disable the upload for
those instead of, say, trying to associate them with the temporary merge
commit.
CachedPersisterContext::$selectJoinSql should be clear or regenerated when sqlFilter changed
The problem reproduce when in use fetch=EAGER and use additional sql filter on this property
I think it would be great to use literalinclude for big code snippets,
because our IDEs could warn us about issues, and it would be easily to
showcase our coding standard. Before we do that though, let us validate
that it renders as expected. I have picked a complex example where we
have a configuration block.
* Add a test covering the #11112 issue
* Add new OutputWalker and SqlFinalizer interfaces
* Add a SingleSelectSqlFinalizer that can take care of adding offset/limit as well as locking mode statements to a given SQL query.
Add a FinalizedSelectExecutor that executes given, finalized SQL statements.
* In SqlWalker, split SQL query generation into the two parts that shall happen before and after the finalization phase.
Move the part that generates "pre-finalization" SQL into a dedicated method. Use a side channel in SingleSelectSqlFinalizer to access the "finalization" logic and avoid duplication.
* Fix CS violations
* Skip the GH11112 test while applying refactorings
* Avoid a Psalm complaint due to invalid (?) docblock syntax
* Establish alternate code path - queries can obtain the sql executor through the finalizer, parser knows about output walkers yielding finalizers
* Remove a possibly premature comment
* Re-enable the #11112 test
* Fix CS
* Make RootTypeWalker inherit from SqlOutputWalker so it becomes finalizer-aware
* Update QueryCacheTest, since first/max results no longer need extra cache entries
* Fix ParserResultSerializationTest by forcing the parser to produce a ParserResult of the old kind (with the executor already constructed)
* Fix WhereInWalkerTest
* Update lib/Doctrine/ORM/Query/Exec/PreparedExecutorFinalizer.php
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* Fix tests
* Fix a Psalm complaint
* Fix a test
* Fix CS
* Make the NullSqlWalker an instance of SqlOutputWalker
* Avoid multiple cache entries caused by LimitSubqueryOutputWalker
* Fix Psalm complaints
* Fix static analysis complaints
* Remove experimental code that I committed accidentally
* Remove unnecessary baseline entry
* Make AddUnknownQueryComponentWalker subclass SqlOutputWalker
That way, we have no remaining classes in the codebase subclassing SqlWalker but not SqlOutputWalker
* Use more expressive exception classes
* Add a deprecation message
* Move SqlExecutor creation to ParserResult, to minimize public methods available on it
* Avoid keeping the SqlExecutor in the Query, since it must be generated just in time (e. g. in case Query parameters change)
* Address PHPStan complaints
* Fix tests
* Small refactorings
* Add an upgrade notice
* Small refactorings
* Update the Psalm baseline
* Add a missing namespace import
* Update Psalm baseline
* Fix CS
* Fix Psalm baseline
---------
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
It maybe happen that the SQL COMMIT statement is successful, but then
something goes wrong. In that kind of case, you do not want to attempt a
rollback.
This was implemented in UnitOfWork::commit(), but for some reason not in
the similar EntityManager methods.
We use this method only from within one of our own test cases, and I don't see how it would be useful to anybody else outside – it has to be called on the `Parser` instance which exists internally in the `Query` only.
Deprecating and removing it in 3.x allows for a slight simplification in the `Parser` there, since we do no longer need the field (it can be a local variable).
People that might have experimented with property hooks while still
using ORM < 2.20.0 need to know that they need to remove their
experiment or upgrade to a version that explicitly supports them.
* 2.19.x:
Make nullable parameters explicit in generated entities (#11625)
Update attributes-reference.rst
Bump doctrine/.github from 5.0.1 to 5.1.0 (#11616)
Move orphan metadata to where it belongs
PHPStan 1.12 (#11585)
When adding the same lifecycle event callback to two or more lifecycle events, the generator will create a stub for each event resulting in fatal 'Cannot redeclare' errors. That is, only if the callback name contains uppercase letters.
If the source entity for an inverse (non-owning) 1-1 relationship is
identified by an association then the identifying association may not
be set when an inverse one-to-one association is resolved. This means
that no data is available in the entity to resolve the needed column
value for the join query.
The original entity data can be retrieved from the unit of work and
is used as a fallback to populate the query condition.
Fixes#11108
single-inheritence entity parent as targetEntity.
When using the parent entity for a single-inheritence table as the
targetEntity for a property, the discriminator value should be all
of the values in the discriminator map.
OneToManyPersister::deleteEntityCollection has been amended to
reflect this.
The Query class (used for DQL queries) takes care of using the value and
type as is when a type was specified for a parameter instead of going
through the default processing of values.
The NativeQuery class was missing the equivalent check, making the
custom type work only if the default processing of values does not
convert the value to a different one.
Remove redundant condition to check if target class contains foreign
identifier in order to allow fetching a null for relations with
composite keys, when part of the key value is null.
* 2.19.x:
Fix OneToManyPersister::deleteEntityCollection missing discriminator column/value. (GH-11500)
Skip joined entity creation for empty relation (#10889)
ci: maintained and stable mariadb version (11.4 current lts) (#11490)
fix(docs): use string value in `addAttribute`
Replace assertion with exception (#11489)
Use ramsey/composer-install in PHPBench workflow
update EntityManager#transactional to EntityManager#wrapInTransaction
Fix cloning entities
Consider usage of setFetchMode when checking for simultaneous usage of fetch-mode EAGER and WITH condition.
This fixes a bug that arises when an entity relation is mapped with
fetch-mode EAGER but setFetchMode LAZY (or anything that is not EAGER)
has been used on the query. If the query use WITH condition, an
exception is incorrectly raised (Associations with fetch-mode=EAGER may
not be using WITH conditions).
Fixes#11128
Co-Authored-By: Albert Prat <albert.prat@interactiu.cat>
If the entity gets reloaded from database before the deletions are
executed UnitOfWork needs to be able to return the original instance in
REMOVED state.
Changed capitalized column names to lowercase for consistency. Other occurances of column names mentioned as lowercase several times at this same page.
See https://github.com/doctrine/collections/issues/368 for the same
issue in doctrine/collections which has been fixed there.
The issue happens when using ->contains(). Running psalm emits
> InvalidArgument - Argument 1 of Doctrine\ORM\PersistentCollection::contains
> expects
> TMaybeContained:fn-doctrine\common\collections\readablecollection::contains
> as mixed, but … provided.
Solution: we should either not define @template TMaybeContained or
re-define the complete psalm docblock from ReadableCollection.
Repairing the docblock necessitates an update to the psalm baseline:
one "known issue" is no longer an issue and thus removed.
* Fix loading SchemaTool::getSchemaFromMetadata() uniqueConstraint without a name
Fixes a type miss-match exception when reading a UniqueConstraint defined on an Entity which doesn't have a predefined name.
* Fix deprecation on DBAL 3
---------
Co-authored-by: Alexander M. Turek <me@derrabus.de>
`getAssociationMappedByTargetField()` returns `null` when called with
the owning side of an association.
This is undocumented and wrong because the phpdoc advertises a string as
a return type.
Instead, callers should ensure they are calling that method with an
inverse side.
Closes#11250
Although this method is guaranteed to return either null or something
that can be used as a fully qualified class name, it never actually
checks that the class actually exists. Adding such a check breaks
several tests, including some that expect a exceptions at some later
points in the execution.
After 2.17 (some?) EAGER fetched OneToMany associations stopped working, if they have multiple join columns. Loads for these associations will trigger a `MessingPositionalParameter` exception "Positional parameter at index 1 does not have a bound value".
This test case should reproduce this issue, so it can be fixed.
* Remove trailing newlines
* Recommend safer way to disable logging
Resetting the middlewares on the configuration object will only work if
the connection object hasn't been built from that configuration object
yet. Instead, people should find the logger bound to the logging
middleware and disable it.
* Add TokenType class
Co-authored-by: Alexander M. Turek <me@derrabus.de>
* Deprecated Lexer constants in favour of TokenType
* Replace all Lexer::T_ occurrences with TokenType::T_
* Add upgrade note
* Fixed import Lexer => TokenType
* Fixed deprecation phpdoc
* Replaced int value with matching constant of TokenType
* Update src/Query/Lexer.php
---------
Co-authored-by: Alexander M. Turek <me@derrabus.de>
After commit https://github.com/doctrine/orm/commit/4e8e3ef30b3d214640883aec5a17896afc006116 when `\Doctrine\ORM\Query\SqlWalker` generates dicsriminator column condition SQL (method `\Doctrine\ORM\Query\SqlWalker::generateDiscriminatorColumnConditionSQL`) it adds an empty string to the list of possible values if the inheritance hierarchy contains a non-root abstract class.
When the discriminator column is implemented with a custom type in PostgreSQL (equivalent of Enum) the query fails because the type cannot have a value of an empty string. It boils down to the fact that `\Doctrine\ORM\Mapping\ClassMetadataInfo::$subClasses` contains an abstract class and in its Metadata the value of `\Doctrine\ORM\Mapping\ClassMetadataInfo::$discriminatorValue` is `null`.
#### Previous behavior
In version 2.14.1 `\Doctrine\ORM\Mapping\ClassMetadataInfo::$subClasses` does not contain an abstract class.
Fixes#11199, fixes#11177, fixes#10846.
---------
Co-authored-by: Michael Skvortsov <michael.skvortsov@eleving.com>
Co-authored-by: Matthias Pigulla <mp@webfactory.de>
The "any" tags inside the definition for mapped superclasses and
embeddables duplicate what is already done for entities.
The other removed "any" tags are also redundant, as they duplicate
what's already done inside the grandparent "choice" tag.
Starting with version libxml 2.12, such redundant tags cause errors
about the content model not being "determinist".
Fixes#11117
When using `AttributeOverride` to override mapping information inherited from a parent class (a mapped superclass), make sure to keep information about where the field was originally declared.
This is important for `private` fields: Without the correct `declared` information, it will lead to errors when cached mapping information is loaded, reflection wakes up and looks for the private field in the wrong class.
#10927 reported that #10455 broke the way how the default `@SequenceGeneratorDefinition` is created and inherited by subclasses for ID columns using `@GeneratedValue(strategy="SEQUENCE")`.
First, I had to understand how `@SequenceGeneratorDefinition` has been handled before #10455 when entity inheritance comes into play:
* Entity and mapped superclasses inherit the ID generator type (as given by `@GeneratedValue`) from their parent classes
* `@SequenceGeneratorDefinition`, however, is not generally inherited
* ... instead, a default sequence generator definition is created for every class when no explicit configuration is given. In this case, sequence names are based on the current class' table name.
* Once a root entity has been identified, all subclasses inherit its sequence generator definition unchanged.
#### Why did #10455 break this?
When I implemented #10455, I was mislead by two tests `BasicInheritanceMappingTest::testGeneratedValueFromMappedSuperclass` and `BasicInheritanceMappingTest::testMultipleMappedSuperclasses`.
These tests check the sequence generator definition that is inherited by an entity class from a mapped superclass, either directly or through an additional (intermediate) mapped superclass.
The tests expect the sequence generator definition on the entity _to be the same_ as on the base mapped superclass.
The reason why the tests worked before was the quirky behaviour of the annotation and attribute drivers that #10455 was aiming at: The drivers did not report the `@SequenceGeneratorDefinition` on the base mapped superclass where it was actually defined. Instead, they reported this `@SequenceGeneratorDefinition` for the entity class only.
This means the inheritance rules stated above did not take effect, since the ID field with the sequence generator was virtually pushed down to the entity class.
In #10455, I did not realize that these failing tests had to do with the quirky and changed mapping driver behaviour. Instead, I tried to "fix" the inheritance rules by passing along the sequence generator definition unchanged once the ID column had been defined.
#### Consequences of the change suggested here
This PR reverts the changes made to `@SequenceGeneratorDefinition` inheritance behaviour that were done in #10455.
This means that with the new "report fields where declared" driver mode (which is active in our functional tests) we can not expect the sequence generator definition to be inherited from mapped superclasses. The two test cases from `BasicInheritanceMappingTest` are removed.
I will leave a notice in #10455 to indicate that the new driver mode also affects sequence generator definitions.
The `GH10927Test` test case validates the sequence names generated in a few cases. In fact, I wrote this test against the `2.15.x` branch to make sure we get results that are consistent with the previous behaviour.
This also means `@SequenceGeneratorDefinition` on mapped superclasses is pointless: The mapped superclass does not make use of the definition itself (it has no table), and the setting is never inherited to child classes.
Fixes#10927. There is another implementation with slightly different inheritance semantics in #11052, in case the fix is not good enough and we'd need to review the topic later on.
In order to resolve#10348, some changes were included in #10547 to improve the computed _delete_ order for entities.
One assumption was that foreign key references with `ON DELETE SET NULL` or `... CASCADE` need not need to be taken into consideration when planning the deletion order, since the RDBMS would unset or cascade-delete such associations by itself when necessary. Only associations that do _not_ use RDBMS-level cascade handling would be sequenced, to make sure the referring entity is deleted before the referred-to one.
This assumption is wrong for `ON DELETE CASCADE`. The following examples give reasons why we need to also consider such associations, and in addition, we need to be able to deal with cycles formed by them.
In the following diagrams, `odc` means `ON DELETE CASCADE`, and `ref` is a regular foreign key with no extra `ON DELETE` semantics.
```mermaid
graph LR;
C-->|ref| B;
B-->|odc| A;
```
In this example, C must be removed before B and A. If we ignore the B->A dependency in the delete order computation, the result may not to be correct. ACB is not a working solution.
```mermaid
graph LR;
A-->|odc| B;
B-->|odc| A;
C-->|ref| B;
```
This is the situation in #10912. We have to deal with a cycle in the graph. C must be removed before A as well as B. If we ignore the B->A dependency (e.g. because we set it to "optional" to get away with the cycle), we might end up with an incorrect order ACB.
```mermaid
graph LR;
A-->|odc| B;
B-->|odc| A;
A-->|ref| C;
C-->|ref| B;
```
This example has no possible remove order. But, if we treat `odc` edges as optional, A -> C -> B would wrongly be deemed suitable.
```mermaid
graph LR;
A-->|ref| B;
B-->|odc| C;
C-->|odc| B;
D-->|ref| C;
```
Here, we must first remove A and D in any order; then, B and C in any order. If we treat one of the `odc` edges as optional, we might find the invalid solutions ABDC or DCAB.
#### Solution implemented in this PR
First, build a graph with a node for every to-be-removed entity, and edges for `ON DELETE CASCADE` associations between those entities. Then, use [Tarjan's algorithm](https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm) to find strongly connected components (SCCs) in this graph. The significance of SCCs is that whenever we remove one of the entities in a SCC from the database (no matter which one), the DBMS will immediately remove _all_ the other entities of that group as well.
For every SCC, pick one (arbitrary) entity from the group to represent all entities of that group.
Then, build a second graph. Again we have nodes for all entities that are to be removed. This time, we insert edges for all regular (foreign key) associations and those with `ON DELETE CASCADE`. `ON DELETE SET NULL` can be left out. The edges are not added between the entities themselves, but between the entities representing the respective SCCs.
Also, for all non-trivial SCCs (those containing more than a single entity), add dependency edges to indicate that all entities of the SCC shall be processed _after_ the entity representing the group. This is to make sure we do not remove a SCC inadvertedly by removing one of its entities too early.
Run a topological sort on the second graph to get the actual delete order. Cycles in this second graph are a problem, there is no delete order.
Fixes#10912.
It will make fuzzy matchers more efficient, and configuration files more readable.
- lib/Doctrine/ORM becomes just src
- tests/Doctrine/ becomes just tests
Spotted while trying to merge https://github.com/doctrine/orm/pull/11076
(among other things) up into 3.0.x. On that branch, it is no longer
possible for an entity to extend another entity without specifying an
inheritance mapping type.
I think the goal of that inheritance was just to reuse the identifier
anyway, so let's just duplicate the identifier declaration instead.
This PR changes a detail in the commit order computation for depended-upon entities.
We have a parent-child relationship between two entity classes. The association is parent one-to-many children, with the child entities containing the (owning side) back-reference.
Cascade-persist is not used, so all entities have to be passed to `EntityManager::persist()`.
Before v2.16.0, two child entities C1 and C2 will be inserted in the same order in which they are passed to `persist()`, and that is regardless of whether the parent entity was passed to `persist()` before or after the child entities.
As of v2.16.0, passing the parent entity to `persist()` _after_ the child entities will lead to an insert order that is _reversed_ compared to the order of `persist()` calls.
This PR makes the order consistent in both cases, as it was before v2.16.0.
#### Cause
When the parent is passed to `persist()` after the children, commit order computation has to re-arrange the entities. The parent must be inserted first since it is referred to by the children.
The implementation of the topological sort from #10547 processed entities in reverse `persist()` order and unshifted finished nodes to an array to obtain the final result. That leads to dependencies (parent → before c1, parent → before c2) showing up in the result in the reverse order of which they were added.
This PR changes the topological sort to produce a result in the opposite order ("all edges pointing left"), which helps to avoid the duplicate array order reversal.
#### Discussion
* This PR _does not_ change semantics of the `persist()` so that entities would (under all ciscumstances) be inserted in the order of `persist()` calls.
* It fixes an unnecessary inconsistency between versions before 2.16.0 and after. In particular, it may be surprising that the insert order for the child entities depends on whether another referred-to entity (the parent) was added before or after them.
* _Both_ orders (c1 before or after c2) are technically and logically correct with regard to the agreement that `commit()` is free to arrange entities in a way that allows for efficient insertion into the database.
Fixes#11058.
Property names as returned by a cast to array are mangled, and that
mangling is not documented. Returning unprefixed produces the same
result, and is more likely to be supported by external tools relying on
the documented possible return values of __sleep.
For instance symfony/var-exporter does not support mangled names, which
leads to issues when caching query parsing results in Symfony
applications.
When unserializing from a cache entry in the previous format, the
sqlStatements need to be copied from the legacy property to the new
property before the reference is created.
The idea here is that instead of having a backward compatibility layer
in the next major branch, we can have a forward compatibility layer in
this branch.
When transforming these phpdoc types into native types, things break
down. They are correct according to the EBNF, but in practice, there are
so-called phase 2 optimizations that allow using ConditionalPrimary,
ConditionalFactor and ConditionalTerm instances in places where
ConditionalExpression is used.
doctrine/common has been split in several packages. A lot of what was
true about doctrine/common is true about doctrine/persistence today, so
let us simply reuse the existing paragraphs and mention persistence
instead of common.
This reduces our dependency to this shared library that now holds very
little code we use.
The class has not been copied verbatim:
- Unused parameters and methods have been removed.
- The class is final and internal.
- Coding standards have been enforced, including enabling strict_types,
which lead to casting a variable to string before feeding it to
explode().
- A bug found by static analysis has been addressed, where an INI
setting obtained with ini_get() was compared with true, which is never
returned by that function.
- Tests are improved to run on all PHP versions
What was optimal 10 years ago no longer is, and things might change in
the future. Using AUTO is still the best solution in most cases, and it
should be easy to make it mean something else when it is not.
* 2.16.x:
PHPStan 1.10.35, Psalm 5.15.0 (#10958)
docs: in text, refer to attributes when talking about metadata (#10956)
Fix bullet list layout (#10951)
docs[query-builder]: fix rendering of `Doctrine\DBAL\ParameterType::*` (#10945)
tests[ORMSetupTest]: testCacheNamespaceShouldBeGeneratedForApcu requires enabled apc (#10940)
docs: use modern named arguments syntax
Ignore "Unknown directive" error
Use a stable release
Remove output directory argument
tutorials[getting-started]: example fix bug id type definition
Verify UnitOfWork::HINT_DEFEREAGERLOAD exists and is true
This avoid situations where you might map a decimal to a float, when it
should really be mapped to a string. This is a big pitfall because in
that situation, the ORM will consider the field changed every time.
We have a lot of errors about "Unknown directive" that we should make
known when implementing guides for Doctrine, but cannot address by
modifying the docs.
The unknown directives are:
- configuration-block
- toc
- tocheader
- sectionauthor
* 2.16.x:
Use required classes for Lifecycle Callback examples (#10916)
Add space before backquote (#10918)
Add back throws annotation to getSingleScalarResult (#10907)
Fix link on known issues docs (#10904)
According to the RST docs,
> [inline markup] it must be separated from surrounding text by non-word
> characters. Use a backslash escaped space to work around that: thisis\ *one*\ word.
Because we were missing a space before backquotes here, the links were
not rendered. Escaping the space allow not to actually produce a space
in the output.
See https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#inline-markup
The changes from #10547, which landed in 2.16.0, cause problems for users calling `EntityManager::flush()` from within `postPersist` event listeners.
* When `UnitOfWork::commit()` is re-entered, the "inner" (reentrant) call will start working through all changesets. Eventually, it finishes with all insertions being performed and `UoW::$entityInsertions` being empty. After return, the entity insertion order, an array computed at the beginning of `UnitOfWork::executeInserts()`, still contains entities that now have been processed already. This leads to a strange-looking SQL error where the number of parameters used does not match the number of parameters bound. This has been reported as #10869.
* The fixes made to the commit order computation may lead to a different entity insertion order than previously. `postPersist` listener code may be affected by this when accessing generated IDs for other entities than the one the event has been dispatched for. This ID may not yet be available when the insertion order is different from the one that was used before 2.16. This has been mentioned in https://github.com/doctrine/orm/pull/10906#issuecomment-1682417987.
This PR suggests to address both issues by dispatching the `postPersist` event only after _all_ new entities have their rows inserted into the database. Likewise, dispatch `postRemove` only after _all_ deletions have been executed.
This solves the first issue because the sequence of insertions or deletions has been processed completely _before_ we start calling event listeners. This way, potential changes made by listeners will no longer be relevant.
Regarding the second issue, I think deferring `postPersist` a bit until _all_ entities have been inserted does not violate any promises given, hence is not a BC break. In 2.15, this event was raised after all insertions _for a particular class_ had been processed - so, it was never an "immediate" event for every single entity. #10547 moved the event handling to directly after every single insertion. Now, this PR moves it back a bit to after _all_ insertions.
Fix regression introduced in #10870
`$result = $this->execute(null, $hydrationMode);` in `getSingleResult` can still throw NoResultException exception.
#10880 reports a case where the changes from #10785 cause entity updates to be missed.
Upon closer inspection, this change seems to be causing it:
https://github.com/doctrine/orm/pull/10785/files#diff-55a900494fc8033ab498c53929716caf0aa39d6bdd7058e7d256787a24412ee4L2990-L3003
The code was changed to use `registerManaged()` instead, which basically does the same things, but (since #10785) also includes an additional check against duplicate entity instances.
But, one detail slipped through tests and reviews: `registerManaged()` also updates `\Doctrine\ORM\UnitOfWork::$originalEntityData`, which is used to compute entity changesets. An empty array `[]` was passed for $data here.
This will make the changeset computation assume that a partial object was loaded and effectively ignore all field updates here:
https://github.com/doctrine/orm/blob/a616914887ea160db4158d2c67752e99624f7c8a/lib/Doctrine/ORM/UnitOfWork.php#L762-L764
I think that, effectively, it is sufficient to call `registerManaged()` only in the two cases where a proxy was created.
Calling `registerManaged()` with `[]` as data for a proxy object is consistent with e. g. `\Doctrine\ORM\EntityManager::getReference()`.
In the case that a full entity has to be loaded, we need not call `registerManaged()` at all, since that will already happen inside `EntityManager::find()` (or, more specifically, `UnitOfWork::createEntity()` called inside it).
Note that the test case has to make some provisions so that we actually reach this case:
* Load an entity that uses `fetch="EAGER"` on a to-one association
* That association being against a class that uses inheritance (why's that?)
* 2.16.x:
Turn identity map collisions from exception to deprecation notice (#10878)
Add possibility to set reportFieldsWhereDeclared to true in ORMSetup (#10865)
Fix UnitOfWork->originalEntityData is missing not-modified collections after computeChangeSet (#9301)
Add an UPGRADE notice about the potential changes in commit order (#10866)
Update branch metadata (#10862)
In #10785, a check was added that prevents entity instances from getting into the identity map when another object for the same ID is already being tracked.
This caused regressions for users that work with application-provided IDs and expect this condition to fail with `UniqueConstraintViolationExceptions` when flushing to the database.
Thus, this PR turns the exception into a deprecation notice. Users can opt-in to the new behavior. In 3.0, the exception will be used.
Implements #10871.
* Fix original data incomplete after flush
* Apply suggestions from code review
Co-authored-by: Alexander M. Turek <me@derrabus.de>
---------
Co-authored-by: Alexander M. Turek <me@derrabus.de>
This excludes such associations from the commit order computation, since the foreign key constraint will be satisfied when inserting the row.
See https://github.com/doctrine/orm/pull/10735/ for more details about this edge case.
1. Inherit ClassMetadataInfo::requiresFetchAfterChange flag from root entity when process parent columns mapping (see ClassMetadataInfo::addInheritedFieldMapping(), it uses same condition as ClassMetadataInfo::mapField()) so JoinedSubclassPersister::assignDefaultVersionAndUpsertableValues() to be called in JoinedSubclassPersister::executeInserts().
2. Override JoinedSubclassPersister::fetchVersionAndNotUpsertableValues() to fetch all parent tables (see $this->getJoinSql() call) generated columns. So make protected BasicEntityPersister::identifierFlattener stateless service (use it flattenIdentifier() method) and BasicEntityPersister::extractIdentifierTypes() (to avoid copy-paste).
3. JoinedSubclassPersister::fetchVersionAndNotUpsertableValues() doesnt check empty $columnNames because it would be an error if ClassMetadataInfo::requiresFetchAfterChange is true while no generated columns in inheritance hierarchy.
4. Initialize JoinedInheritanceRoot not-nullable string properties with insertable=false attribute to avoid attempt to insert default null data which cause error:
PDOException: SQLSTATE[23502]: Not null violation: 7 ERROR: null value in column "rootwritablecontent" of relation "joined_inheritance_root" violates not-null constraint
DETAIL: Failing row contains (5, null, dbDefault, dbDefault, , nonUpdatable).
while $rootTableStmt->executeStatement() because JoinedSubclassPersister::getInsertColumnList() have no $insertData (prepared later) to decide is generated column provided by client code or not (so need to skip column)
1. Postgres gives error when insert root entity ($rootTableStmt->executeStatement()) in JoinedSubclassPersister::executeInserts():
PDOException: SQLSTATE[08P01]: <<Unknown error>>: 7 ERROR: bind message supplies 4 parameters, but prepared statement "" requires 6
so exclude notInsertable columns from JoinedSubclassPersister::getInsertColumnList() like it done in parent::prepareInsertData() which call BasicEntityPersister::prepareUpdateData(isInsert: true) where we have condition:
if ($isInsert && isset($fieldMapping['notInsertable']))
2. Try to get generated (notInsertable|notUpdatable) column value on flush() with JoinedSubclassPersister::executeInserts() also fails:
Unexpected empty result for database query.
because method it calls $this->assignDefaultVersionAndUpsertableValues() after insert root entity row, while generated columns in child-entity table, so move call just after insert child row
3. Use option['default'] = 'dbDefault' in functional test entities, to emulate generated value on insert, but declare as generated = 'ALWAYS' for tests purpose (correctness of JoinedSubclassPersister::fetchVersionAndNotUpsertableValues() sql-query)
4. Use JoinedInheritanceRoot::rootField to skip JoinedSubclassPersister::update() optimization for empty changeset in updatable:false columns tests
In 7fa3e6ec7c, a global search and replace
was used for http and https.
This broke the documentation examples in that as soon as you turn on XSD
validation, it will fail because the namespace in the XML file does not
match the ones defined in the XSD file, which do not exhibit the https.
Note that this is not a security concern, because these URIs are
not meant to be actually resolved, but to serve as a unique identifier
for the namespace in which we define our elements.
This change improves scheduling of extra updates in the `BasicEntityPersister`.
Extra updates can be avoided when
* the referred-to entity has already been inserted during the current insert batch/transaction
* we have a self-referencing entity with application-provided ID values (the `NONE` generator strategy).
As a corollary, with this change applications that provide their own IDs can define self-referencing associations as not NULLable.
I am considering this a bugfix since the ORM previously executed additional queries that were not strictly necessary, and that required users to work with NULLable columns where conceptually a non-NULLable column would be valid and more expressive.
One caveat, though:
In the absence of entity-level commit ordering (#10547), it is not guaranteed that entities with self-references (at the class level) will be inserted in a suitable order. The order depends on the sequence in which the entities were added with `persist()`.
Fixes#7877, closes#7882.
Co-authored-by: Sylvain Fabre <sylvain.fabre@assoconnect.com>
The way we have our docs, the sidebar is a separate document and as
such, needs a title. Let us prepend a dummy title until the guides-cli
provides a way to ignore that particular error.
* 2.15.x: (23 commits)
Fix cloning entities when using lazy-ghost proxies
Fixes recomputation of single entity change set when entity contains enum attributes. Due to the fact that originalEntityData contains enum objects and ReflectionEnumProperty::getValue() returns value of enum, comparison of values are always falsy, resulting to update columns value even though it has not changes.
Fix code style issues
Use absolute references
Avoid colon followed by double colon
Use correct syntax for references
Fix invalid reference syntax
Use rst syntax
Use internal link
Escape pipes
Introduce new workflow to test docs
Remove lone dash (#10812)
Treat id field proprites same as regular field
Move three "Ticket/"-style tests to the right namespace
Follow recommendation about multiline type
Fix unserialize() errors when running tests on PHP 8.3 (#10803)
Explain `EntityManager::getReference()` peculiarities (#10800)
Upgrade to Psalm 5.13
test: assert `postLoad` has data first
distinct() updates QueryBuilder state correctly
...
Due to the fact that originalEntityData contains enum objects and
ReflectionEnumProperty::getValue() returns value of enum,
comparison of values are always falsy, resulting to update
columns value even though it has not changes.
According to the Sphinx docs, when in reference/architecture.rst, a
reference to reference/inheritance-mapping would resolve to
reference/reference/inheritance-mapping.rst, because it is relative to
the current document
There are a few requests (#5742, #5368, #5109, #6776) that ask to change the order of operations in the UnitOfWork to perform "deletes before inserts", or where such a switch appears to solve a reported problem.
I don't want to say that this is not doable. But this PR at least adds two tricky examples where INSERTs need to be done before an UPDATE can refer to new database rows; and where the UPDATE needs to happen to release foreign key references to other entities before those can be DELETEd.
So, at least as long as all operations of a certain type are to be executed in blocks, this example allows no other order of operations than the current one.
While trying to understand #3037, I found that it may happen that we have more entries in `\Doctrine\ORM\UnitOfWork::$entityIdentifiers` than in `\Doctrine\ORM\UnitOfWork::$identityMap`.
The former is a mapping from `spl_object_id` values to ID hashes, the latter an array first of entity class names and then from ID hash to entity object instances.
(Basically, "ID hash" is a concatenation of all field values making up the `@Id` for a given entity.)
This means that at some point, we must have _different_ objects representing the same entity, or at least over time different objects are used for the same entity without the UoW properly updating its `::$entityIdentifiers` structure.
I don't think it makes sense to overwrite an entity in the identity map, since that means a currently `MANAGED` entity is replaced with something else.
If it makes sense at all to _replace_ an entity, that should happen through dedicated management methods to first detach the old entity before persisting, merging or otherwise adding the new one. This way we could make sure the internal structures remain consistent.
Apparently, there is consensus about multiline types between:
- PHPStan
- Psalm
- Slevomat Coding Standard
See https://github.com/slevomat/coding-standard/issues/1586#issuecomment-1610195706
Using parenthesis is less ambiguous, it makes it clear to the parser
where the type begins and where it ends.
The change has a positive impact on the Psalm baseline, showing
that psalm-return annotation was not really understood previously.
A lot of our tests mention it, but I do not think it is important to the
test. Maybe it was a way to have more efficient tests? Most times,
removing the hint does not affect the test outcome.
* Explain `EntityManager::getReference()` peculiarities
As one takeaway from https://github.com/doctrine/orm/issues/3037#issuecomment-1605657003 and #843, we should look into better explaining the `EntityManager::getReference()` method, it’s semantics, caveats and potential responsibilities placed on the user.
This PR tries to do that, so it fixes#10797.
* Update docs/en/reference/advanced-configuration.rst
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
---------
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
Previously calling distinct() when the QueryBuilder was in clean state would cause subsequent getDQL() calls to ignore the distinct queryPart
Fixes#10784
This PR tries to improve the situation/problem explained in #3037:
Under certain conditions – there may be multiple and not all are known/well-understood – we may get inconsistencies between the `\Doctrine\ORM\UnitOfWork::$entityIdentifiers` and `\Doctrine\ORM\UnitOfWork::$identityMap` arrays.
Since the `::$identityMap` is a plain array holding object references, objects contained in it cannot be garbage-collected.
`::$entityIdentifiers`, however, is indexed by `spl_object_id` values. When those objects are destructed and/or garbage-collected, the OID may be reused and reassigned to other objects later on.
When the OID re-assignment happens to be for another entity, the UoW may assume incorrect entity states and, for example, miss INSERT or UPDATE operations.
One cause for such inconsistencies is _replacing_ identity map entries with other object instances: This makes it possible that the old object becomes GC'd, while its OID is not cleaned up. Since that is not a use case we need to support (IMHO), #10785 is about adding a safeguard against it.
In this test shown here, the `merge()` operation is currently too eager in creating a proxy object for another referred-to entity. This proxy represents an entity already present in the identity map at that time, potentially leading to this problem later on.
Add tests for entity insertion and deletion that require the commit order calculation to happen on the entity level. This demonstrates the necessity for the changes in #10547.
This PR contains two tests with carefully constructed entity relationships, where we have a non-nullable `parent` foreign key relationships between entities stored in the same table.
Class diagram:
```mermaid
classDiagram
direction LR
class A
class B
A --> B : parent
B --|> A
```
Object graph:
```mermaid
graph LR;
b1 --> b2;
b2 --> a;
b3 --> b2;
```
#### Situation before #10547
The commit order is computed by looking at the associations at the _table_ (= _class_) level. Once the ordering of _tables_ has been found, entities being mapped to the same table will be processed in the order they were given to `persist()` or `remove()`.
That means only a particular ordering of `persist()` or `remove()` calls (see comment in the test) works:
For inserts, the order must be `$a, $b2, $b1, $b3` (or `... $b3, $b1`), for deletions `$b1, $b3, $b2, $a`.
#### Situation with entity-level commit order computation (as in #10547)
The ORM computes the commit order by considering associations at the _entity_ level. It will be able to find a working order by itself.
This refactoring does two things:
* We can avoid collecting the post insert IDs in a cumbersome array structure that will be returned by the EntityPersisters and processed by the UoW right after. Instead, use a more expressive API: Make the EntityPersisters tell the UoW about the IDs immediately.
* IDs will be available in inserted entities a tad sooner. That may help to resolve#10735, where we can use the IDs to skip extra updates.
When collection updates/join table cleanups do not happen through specialized Entity-/CollectionPersister methods but instead as "plain" updates, we may issue a lot more queries than expected.
In my case a custom doctrine type of Uuid object is converted to string by simply casting it, resulting in a hex DELETE FROM x WHERE id = ? query,
whilst it should've been converted along the way to it's binary representation. This leads to no deletions being made at all as you would expect making use of doctrine custom type's as an identifier.
This commit fixes usage of ramsey/uuid or symfony/uid as custom id types when making use of orphan removal.
This is the result of the contradiction between the phpdoc
(ClassMetadata), and the condition, which guarantees $metadata is not a
ClassMetadata. Relaxing the phpdoc leads to other phpstan issues, about
properties that exist in ClassMetadata but not in
PersistentClassMetadata. The right way to fix this would be to switch
from a deprecation to an exception, but that is not the path we have
taken, and all this will disappear in 3.0.x anyway, so let's not bother.
Add tests for entity insertion and deletion that require writes to different tables in an interleaved fashion, and that have to re-visit a particular table.
#### Background
In #10531, I've given an example where it is necessary to compute the commit order on the entity (instead of table) level.
Taking a closer look at the UoW to see how this could be achieved, I noticed that the current, table-level commit order manifests itself also in the API between the UoW and `EntityPersister`s.
#### Current situation
The UoW computes the commit order on the table level. All entity insertions for a particular table are passed through `EntityPersister::addInsert()` and finally written through `EntityPersister::executeInserts()`.
#### Suggested change
The test in this PR contains a carefully constructed set of four entities. Two of them are of the same class (are written to the same table), but require other entities to be processed first.
In order to be able to insert this set of entities, the ORM must be able to perform inserts for a given table repeatedly, interleaved with writing other entities to their respective tables.
This is part of the series of issues fixed by #10547. In particular, the changes from #10566 were relevant.
See #10348 for the bug description.
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
This is the third step to break https://github.com/doctrine/orm/pull/10547 into smaller PRs suitable for reviewing. It uses the new topological sort implementation from #10592 and the refactoring from #10651 to compute the UoW's commit order for entity insertions not on the entity class level, but for single entities and their actual dependencies instead.
#### Current situation
`UnitOfWork::getCommitOrder()` would compute the entity sequence on the class level with the following code:
https://github.com/doctrine/orm/blob/70477d81e96c0044ad6fd8c13c37b2270d082792/lib/Doctrine/ORM/UnitOfWork.php#L1310-L1325
#### Suggested change
* Instead of considering the classes of all entities that need to be inserted, updated or deleted, consider the new (inserted) entities only. We only need to find a sequence in situations where there are foreign key relationships between two _new_ entities.
* In the dependency graph, add edges for all to-one association target entities.
* Make edges "optional" when the association is nullable.
#### Test changes
I have not tried to fully understand the few changes necessary to fix the tests. My guess is that those are edge cases where the insert order changed and we need to consider this during clean-up.
Keep in mind that many of the functional tests we have assume that entities have IDs assigned in the order that they were added to the EntityManager. That does not change – so the order of entities is generally stable, equal to the previous implementation.
When computing the commit order for entity removals, we have to look out for `@ORM\JoinColumn(onDelete="SET NULL")` to find places where cyclic associations can be broken.
#### Background
The UoW computes a "commit order" to find the sequence in which tables shall be processed when inserting entities into the database or performing delete operations.
For the insert case, the ORM is able to schedule _extra updates_ that will be performed after all entities have been inserted. Associations which are configured as `@ORM\JoinColumn(nullable=true, ...)` can be left as `NULL` in the database when performing the initial `INSERT` statements, and will be updated once all new entities have been written to the database. This can be used to break cyclic associations between entity instances.
For removals, the ORM does not currently implement up-front `UPDATE` statements to `NULL` out associations before `DELETE` statements are executed. That means when associations form a cycle, users have to configure `@ORM\JoinColumn(onDelete="SET NULL", ...)` on one of the associations involved. This transfers responsibility to the DBMS to break the cycle at that place.
_But_, we still have to perform the delete statements in an order that makes this happen early enough. This may be a _different_ order than the one required for the insert case. We can find it _only_ by looking at the `onDelete` behaviour. We must ignore the `nullable` property, which is irrelevant, since we do not even try to `NULL` anything.
#### Example
Assume three entity classes `A`, `B`, `C`. There are unidirectional one-to-one associations `A -> B`, `B -> C`, `C -> A`. All those associations are `nullable= true`.
Three entities `$a`, `$b`, `$c` are created from these respective classes and associations are set up.
All operations `cascade` at the ORM level. So we can test what happens when we start the operations at the three individual entities, but in the end, they will always involve all three of them.
_Any_ insert order will work, so the improvements necessary to solve #10531 or #10532 are not needed here. Since all associations are between different tables, the current table-level computation is good enough.
For the removal case, only the `A -> B` association has `onDelete="SET NULL"`. So, the only possible execution order is `$b`, `$c`, `$a`. We have to find that regardless of where we start the cascade operation.
The DBMS will set the `A -> B` association on `$a` to `NULL` when we remove `$b`. We can then remove `$c` since it is no longer being referred to, then `$a`.
#### Related cases
These cases ask for the ORM to perform the extra update before the delete by itself, without DBMS-level support:
* #5665
* #10548
mappedBy is never defined on an inverse relationship.
Bi-directional self-referencing should IMO still result in 2 separate
associations, on 2 separate fields of the same class, like mentor or
mentee.
This is the first chunk to break #10547 into smaller PRs suitable for reviewing. It adds a new topological sort implementation.
#### Background
Topological sort is an algorithm that sorts the vertices of a directed acyclic graph (DAG) in a linear order such that for every directed edge from vertex A to vertex B, vertex A comes before vertex B in the ordering. This ordering is called a topological order.
Ultimately (beyond the scope of this PR), in the ORM we'll need this to find an order in which we can insert new entities into the database. When one entity needs to refer to another one by means of a foreign key, the referred-to entity must be inserted before the referring entity. Deleting entities is similar.
A topological sorting can be obtained by running a depth first search (DFS) on the graph. The order in which the DFS finishes on the vertices is a topological order. The DFS is possible iif there are no cycles in the graph. When there are cycles, the DFS will find them.
For more information about topological sorting, as well as a description of an DFS-based topological sorting algorithm, see https://en.wikipedia.org/wiki/Topological_sorting.
#### Current situation
There is a DFS-based topological sorting implemented in the `CommitOrderCalculator`. This implementation has two kinks:
1. It does not detect cycles
When there is a cycle in the DAG that cannot be resolved, we need to know about it. Ultimately, this means we will not be able to insert entities into the database in any order that allows all foreign keys constraints to be satisfied.
If you look at `CommitOrderCalculator`, you'll see that there is no code dealing with this situation.
2. It has an obscure concept of edge "weights"
To me, it is not clear how those are supposed to work. The weights are related to whether a foreign key is nullable or not, but can (could) be arbitrary integers. An edge will be ignored if it has a higher (lower) weight than another, already processed edge... 🤷🏻?
#### Suggested change
In fact, when inserting entities into the database, we have two kinds of foreign key relationships: Those that are `nullable`, and those that are not.
Non-nullable foreign keys are hard requirements: Referred-to entities must be inserted first, no matter what. These are "non-optional" edges in the dependency graph.
Nullable foreign keys can be set to `NULL` when first inserting an entity, and then coming back and updating the foreign key value after the referred-to (related) entity has been inserted into the database. This is already implemented in `\Doctrine\ORM\UnitOfWork::scheduleExtraUpdate`, at the expense of performing one extra `UPDATE` query after all the `INSERT`s have been processed. These edges are "optional".
When finding a cycle that consists of non-optional edges only, treat it as a failure. We won't be able to insert entities with a circular dependency when all foreign keys are non-NULLable.
When a cycle contains at least one optional edge, we can use it to break the cycle: Use backtracking to go back to the point before traversing the last _optional_ edge. This omits the edge from the topological sort order, but will cost one extra UPDATE later on.
To make the transition easier, the new implementation is provided as a separate class, which is marked as `@internal`.
#### Outlook
Backtracking to the last optional edge is the simplest solution for now. In general, it might be better to find _another_ (optional) edge that would also break the cycle, if that edge is also part of _other_ cycles.
Remember, every optional edge skipped costs an extra UPDATE query later on. The underlying problem is known as the "minimum feedback arc set" problem, and is not easily/efficiently solvable. Thus, for the time being, picking the nearest optional edge seems to be reasonable.
This PR will make the annotations and attribute mapping drivers report mapping configuration for the classes where it is declared, instead of omitting it and reporting it for subclasses only. This is necessary to be able to catch mis-configurations in `ClassMetadataFactory`.
Fixes#10417, closes#10450, closes#10454.
#### ⚠️ Summary for users getting `MappingExceptions` with the new mode
When you set the `$reportFieldsWhereDeclared` constructor parameters to `true` for the AnnotationDriver and/or AttributesDriver and get `MappingExceptions`, you may be doing one of the following:
* Using `private` fields with the same name in different classes of an entity inheritance hierarchy (see #10450)
* Redeclaring/overwriting mapped properties inherited from mapped superclasses and/or other entities (see #10454)
As explained in these two PRs, the ORM cannot (or at least, was not designed to) support such configurations. Unfortunately, due to the old – now deprecated – driver behaviour, the misconfigurations could not be detected, and due to previously missing tests, this in turn was not noticed.
#### Current situation
The annotations mapping driver has the following condition to skip properties that are reported by the PHP reflection API:
https://github.com/doctrine/orm/blob/69c7791ba256d947ddb1aafe5f2439ab31704937/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php#L345-L357
This code has been there basically unchanged since the initial 2.0 release. The same condition can be found in the attribute driver, probably it has been copied when attributes were added.
I _think_ what the driver tries to do here is to deal with the fact that Reflection will also report `public`/`protected` properties inherited from parent classes. This is supported by the observation (see #5744) that e. g. YAML and XML drivers do not contain this logic.
The conditions are not precise enough for edge cases. They lead to some fields and configuration values not even being reported by the driver.
Only since the fields would be "discovered" again when reflecting on subclasses, they eventually end up in class metadata structures for the subclasses. In one case of inherited ID generator mappings, the `ClassMetadataFactory` would also rely on this behaviour.
Two potential bugs that can result from this are demonstrated in #10450 and #10454.
#### Suggested solution
In order to find a more reliable way of separating properties that are merely reported again in subclasses from those that are actual re-declarations, use the information already available in `ClassMetadata`. In particular, `declared` tells us in which non-transient class a "field" was first seen.
Make the mapping driver skip only those properties for which we already know that they have been declared in parent classes, and skip them only when the observed declaring class matches the expectation.
For all other properties, report them to `ClassMetadataFactory` and let that deal with consistency checking/error handling.
#10450 and #10454 are merged into this PR to show that they pass now.
#### Soft deprecation strategy
To avoid throwing new/surprising exceptions (even for misconfigurations) during a minor version upgrade, the new driver mode is opt-in.
Users will have to set the `$reportFieldsWhereDeclared` constructor parameters to `true` for the `AnnotationDriver` and/or `AttributesDriver`. Unless they do so, a deprecation warning will be raised.
In 3.0, the "new" mode will become the default. The constructor parameter can be deprecated (as of ORM 3.1, probably) and is a no-op.
We need to follow up in other places (DoctrineBundle, ... – what else?) to make this driver parameter an easy-to-change configuration setting.
In the code examples of the use of the ``Query#toIterable()`` method, an index ``$i`` is used to control the size of the current batch, which ends when the condition ``($i % $batchSize) === 0`` becomes true.
Because this condition is preceded by ``++$i`` and ``$i`` is initialized to 1, the first batch is actually of size ``$batchSize - 1`` instead of ``$batchSize``.
To solve this, ``$i`` should be initialized to 0 instead of 1.
Currently, the AttributeDriver ignores any join column attribute specified on a many to many relationship.
Let's copy code from the AnnotationDriver to fix that.
Fixes#9902
Following up on #10652:
#### Current situation
The implementation of `\Doctrine\ORM\Mapping\ClassMetadataInfo::_validateAndCompleteOneToOneMapping` will consider a field with a one-to-one association to be the owning side also when it configures `@JoinColumn` settings.
#### Suggested change
For a one to one association, a field should be the inverse side when it uses the `mappedBy` attribute, and be the owning side otherwise. The `JoinColumn` may be configured on the owning side only.
This PR adds a deprecation notice when `@JoinColumn` is used on the side of a one-to-one association where `mappedBy` occurs.
In 3.0, this will throw a `MappingException`.
This makes no sense because it describes a one-to-one where each table
references the other one. I do not think this is deliberately supported,
and it probably will not be supported at all in 3.0.
This is the second chunk to break #10547 into smaller PRs suitable for reviewing. It prepares the `UnitOfWork` to work with a commit order computed on the entity level, as opposed to a class-based ordering as before.
#### Background
#10531 and #10532 show that it is not always possible to run `UnitOfWork::commit()` with a commit order given in terms of entity _classes_. Instead it is necessary to process entities in an order computed on the _object_ level. That may include
* a particular order for multiple entities of the _same_ class
* a particular order for entities of _different_ classes, possibly even going back and forth (entity `$a1` of class `A`, then `$b` of class `B`, then `$a2` of class `A` – revisiting that class).
This PR is about preparing the `UnitOfWork` so that its methods will be able to perform inserts and deletions on that level of granularity. Subsequent PRs will deal with implementing that particular order computation.
#### Suggested change
Change the private `executeInserts` and `executeDeletions` methods so that they do not take a `ClassMetadata` identifying the class of entities that shall be processed, but pass them the list of entities directly.
The lists of entities are built in two dedicated methods. That happens basically as previously, by iterating over `$this->entityInsertions` or `$this->entityDeletions` and filtering those by class.
#### Potential (BC breaking?) changes that need review scrutiny
* `\Doctrine\ORM\Persisters\Entity\EntityPersister::addInsert()` was previously called multiple times, before the insert would be performed by a call to `\Doctrine\ORM\Persisters\Entity\EntityPersister::executeInserts()`. With the changes here, this batching effectively no longer takes place. `executeInserts()` will always see one entity/insert only, and there may be multiple `executeInserts()` calls during a single `UoW::commit()` phase.
* The caching in `\Doctrine\ORM\Cache\Persister\Entity\AbstractEntityPersister` previously would cache entities from the last executed insert batch only. Now it will collect entities across multiple batches. I don't know if that poses a problem.
* Overhead in `\Doctrine\ORM\Persisters\Entity\BasicEntityPersister::executeInserts` is incurred multiple times; that may, however, only be about SQL statement preparation and might be salvageable.
* The `postPersist` event previously was not dispatched before _all_ instances of a particular entity class had been written to the database. Now, it will be dispatched immediately after every single entity that has been inserted.
* Add test case for https://github.com/doctrine/orm/issues/7717
* Do not hide null equality checks in `SqlValueVisitor::walkComparison`
* Annotate `GH7717Parent::$children` type
When an entity have a backed enum as identifier, `UnitOfWork` tries to
cast to string when generating the hash of the id.
This fix calls `->value` when identifier is a `BackedEnum`.
Fixes#10471Fixes#10334
DiscriminatorColumnMapping is just a specialization of the array shape
that is right of the pipe: it has the same fields, except fewer fields
are nullable. The union of that is the same thing as the array shape.
The default allocationSize for @SequenceGenerator has actually been set to 1 ever since 434325e (back in 2010!)
This was done to remove confusion, but the docs were never updated to reflect the change
This puts the remarks that apply to both JTI/STI in a common section at the beginning, and tries to explain a bit more in detail why it is that way.
It was sparked off by the discussion in #10538.
* 2.14.x:
Ignore the cache dir of PHPUnit 10 (#10546)
Make data providers static (#10544)
Bump dev tools (#10541)
Mark SqlWalker methods as not deprecated (#10540)
docs: consistency order for docblock in association mapping (#10534)
Correct use of PHP attribute
fix typo in faq.rst (#10526)
fix: use executeStatement in SchemaTool (#10516)
Write a test in a more specific way
Put up a warning sign that mapping may not be inherited from transient classes (#10392)
Avoid unnecessary information in query hints to improve query cache hit ratio
phpstan treats implementations of deprecated methods of an interface as being deprecated themselves by default.
However, SqlWalker does not intend to deprecate all those methods that are deprecated in TreeWalker, as they are
moved down. Marking them as not deprecated will avoid reporting usages of deprecated APIs when implementing
custom DQL functions for instance.
Working on converting these array shapes to DTO allowed me to find every
signature where they are supposed to be used.
The Psalm baseline gets worse because it considers accessing an array
key differently depending on whether it is defined vaguely, as
array<string, mixed>, or precisely, as array{my-key?: string}.
This _seems_ to work, but...
To my understanding, that is only because `ReflectionClass::getProperties` will report `protected` properties also for subclasses, including DocBlocks etc. The mapping drivers are unable to tell where a field comes from, so they pick up the mapping and treat it as originating from the inheriting class.
If that is indeed outside of what the ORM was designed for (sombody please confirm?), then we should explicitly discourage it.
Yes, I have seen examples of this in the wild.
Allow to-many associations to be used on mapped superclasses when the owning (inverse) side does not refer back to the mapped superclass, thanks to `ResolveTargetEntityListener`.
#### Current situation
The [documentation states](https://www.doctrine-project.org/projects/doctrine-orm/en/latest/reference/inheritance-mapping.html):
> No database table will be created for a mapped superclass itself
> [...] persistent relationships defined by a mapped superclass must be unidirectional (with an owning side only). This means that One-To-Many associations are not possible on a mapped superclass at all.
That's a though limitation.
~Obviously~ ~apparently~ Probably the limitation comes from the fact that in a to-many association the "many" side has to hold a foreign key. Since the mapped superclass does not have a database table (it's not an entity), no such backreference can be established.
Currently, to-many associations trigger an exception as soon as they are seen on a mapped superclass:
https://github.com/doctrine/orm/blob/d6c0031d44f04e04bbc0cd57a3ed7e05c7ea8b40/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php#L459-L461
#### `ResolveTargetEntityListener`
The `ResolveTargetEntityListener` can be used to substitute interface or class names in mapping configuration at runtime, during the metadata load phase.
When this gimmick is used to replace _all_ references to the mapped superclass with an entity class in time, it should be possible to have to-many associations on the inheriting entity classes.
#### Suggested solution
Instead of rejecting to-many associations on mapped superclasses right away, validate that at the end of the day (after the `loadClassMetadata` event has been processed) no association may target at a non-entity class. That includes mapped superclasses as well as transient classes.
#### Motivating example
Consider a library that comes with a `User` base class. This class is `abstract` and has to be subclassed/filled when the library is used.
By making this a mapped superclass, library users have the freedom to either have a simple user entity class or a user class hierarchy, but we do not impose any requirements on them. (NB we also don't want to have a root entity in the library, because that would have to declare the entire class hierarchy, including library users' classes.)
The actual user class to be used will be configured through the `ResolveTargetEntityListener`.
The library also includes a `SocialMediaAccount` entity. A `User` can have multiple of these accounts, and we want to be able to navigate the accounts from the user side.
To make the example even more fancy, there is a self-referencing association on the `User`: A `User` has been created by another user, and holds a collection of all other `User`s it created.
The test case contained in this PR contains this example and validates that all association mappings look just as if the final user class had been written as an entity directly, without the superclass.
#### Potential review talking points
- Am I missing other reasons why to-many is not feasible?
- We now reject association mappings with `targetEntity`s that are not entities; relevant BC break? (IMHO: no.)
#### Review tip
Review commit by commit, not all files at once. The last commit adds a lot of entity declarations that were previously missed in tests and now raised exceptions; that's a lot of clutter in the PR.
I've noticed that over time my query caches fill up with redundant queries, i. e. different cache entries for the DQL -> SQL translation that are exactly the same. For me, it's an issue because the cache entries fill up precious OPcache memory.
Further investigation revealed that the queries themselves do not differ, but only the query hints – that are part of the computed cache key – do.
In particular, only the value for the `WhereInWalker::HINT_PAGINATOR_ID_COUNT` query hint are different. Since `WhereInWalker` only needs to know _if_ there are matching IDs but not _how many_, we could avoid such cache misses by using just a boolean value as cache hint.
One could interpret the old description as if `Connection#transactional()` would not rollback the transaction. Also, the fact that the `EntityManager` gets closed in case of an exception was not mentioned.
* 2.14.x:
Remove calls to assertObjectHasAttribute() (#10502)
Remove calls to withConsecutive() (#10501)
Use recognized array key
Fix#9095 by re-applying #9096
Use linebreaks
"joinColumn" has no meaning for the static PHP driver. That's because it
performs no translation, unlike other drivers: we're using the
ClassMetadata API directly.
Before this change, changing the value of the "name" key did not break
the test suite. After this change, it does.
Make the `Paginator`-internal query (`... WHERE ... IN (id, id2,
id3...)`) cacheable in the query cache again.
When the Paginator creates the internal subquery that does the actual
result limiting, it has to take DBAL type conversions for the identifier
column of the paginated root entity into account (#7820, fixed in
#7821).
In order to perform this type conversion, we need to know the DBAL type
class for the root entity's `#[Id]`, and we have to figure it out based
on a given (arbitrary) DQL query. This requires DQL parsing and
inspecting the AST, so #7821 placed the conversion code in the
`WhereInWalker` where all the necessary information is available.
The problem is that type conversion has to happen every time the
paginator is run, but the query that results from running
`WhereInWalker` would be kept in the query cache. This was reported in
#7837 and fixed by #7865, by making this particular query expire every
time. The query must not be cached, since the necessary ID type
conversion happens as a side-effect of running the `WhereInWalker`.
The Paginator internal query that uses `WhereInWalker` has its DQL
re-parsed and transformed in every request.
This PR moves the code that determines the DBAL type out of
`WhereInWalker` into a dedicated SQL walker class, `RootTypeWalker`.
`RootTypeWalker` uses a ~hack~ clever trick to report the type back: It
sets the type as the resulting "SQL" string. The benefit is that
`RootTypeWalker` results can be cached in the query cache themselves.
Only the first time a given DQL query has to be paginated, we need to
run this walker to find out the root entity's ID type. After that, the
type will be returned from the query cache.
With the type information being provided, `Paginator` can take care of
the necessary conversions by itself. This happens every time the
Paginator is used.
The internal query that uses `WhereInWalker` can be cached again since
it no longer has side effects.
Since #10411 has been merged, no more need to specify all intermediate
abstract entity classes.
Thus, we can relax the schema validator check as requested in #9095. The
reasons given in #9142 no longer apply.
Fixes#9095
This spares us from referencing ClassMetadataInfo from other classes,
which is a good thing since it is deprecated. It also means merging up
is easier.
In #10431, some invalid inheritance declarations in our test base were fixed. However, the change missed to update XML, PHP and static PHP mapping configurations as well.
Apparently, this did not raise any flags because the misconfiguration only caused a deprecation notice in 2.15.x. Running the tests against 3.0 (where the misconfiguration will be an error) unveiled the mistake.
Inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one.
This is also pointed out in the opening comment for #8348:
> Entities are not allowed to extend from entities without an inheritence mapping relationship (Single Table or Joined Table inheritance). [...] While Doctrine so far allowed these things, they are fragile and will break on certain scenarios.
Throwing an exception in case of this misconfiguration is nothing we should do light-heartedly, given that it may surprise users in a bugfix or feature release. So, we should start with a deprecation notice and make this an exception in 3.0. The documentation is updated accordingly at #10429.
Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause.
In case you are affected by this, please understand that although things "previously worked" for you, you have been using the ORM outside of what it was designed to do. That may have worked in simple cases, but may also have caused invalid results (wrong or missing data after hydration?) that possibly went unnoticed in subtle cases.
This picks the test case from #9517 and rebases it onto 2.14.x.
The problem has been covered in #8415, so this PR closes#9517 and fixes#9516.
Co-authored-by: Robert D'Ercole <bobdercole@gmail.com>
This PR prevents the Paginator from causing OpCache "wasted memory" to increase _on every request_ when used with Symfony's `PhpFilesAdapter` as the cache implementation for the query cache.
Depending on configured thresholds, wasted memory this will either cause periodic opcache restarts or running out of memory and not being able to cache additional scripts ([Details](https://tideways.com/profiler/blog/fine-tune-your-opcache-configuration-to-avoid-caching-suprises)).
Fixes#9917, closes#10095.
There is a long story (#7820, #7821, #7837, #7865) behind how the Paginator can take care of DBAL type conversions when creating the pagination query. This conversion has to transform identifier values before they will be used as a query parameter, so it has to happen every time the Paginator is used.
For reasons, this conversion happens inside `WhereInWalker`. Tree walkers like this are used only during the DQL parsing/AST processing steps. Having a DQL query in the query cache short-cuts this step by fetching the parsing/processing result from the cache.
So, to make sure the conversion happens also with the query cache being enabled, this line
https://github.com/doctrine/orm/blob/1753d035005c1125c9fb4855c3fa629341e5734d/lib/Doctrine/ORM/Tools/Pagination/Paginator.php#L165
was added in #7837. It causes `\Doctrine\ORM\Query::parse()` to re-parse the query every time, but will also put the result into the query cache afterwards.
At this point, the setup described in #9917 – which, to my knowledge, is the default in Symfony + DoctrineBundle projects – will ultimately bring us to this code:
https://github.com/symfony/symfony/blob/4b3391725f2fc4a072e776974f00a992cbc70515/src/Symfony/Component/Cache/Adapter/PhpFilesAdapter.php#L248-L249
When writing a cache item with an already existing key, the driver has to make sure the opcache will honor the changed PHP file. This is what causes _wasted memory_ to increase.
Instead of using `\Doctrine\ORM\Query::expireQueryCache()`, which will force `\Doctrine\ORM\Query::parse()` to parse the query again before putting it into the cache, use `\Doctrine\ORM\Query::useQueryCache(false)`. The subtle difference is the latter will not place the processed query in the cache in the first place.
A test case is added to check that repeated use of the paginator does not call the cache to update existing keys. That should suffice to make sure we're not running into the issue, while at the same time not complicating tests by using the `PhpFilesAdapter` directly.
Note that in order to observe the described issue in tests, you will need to use the `PhpFilesDriver` and also make sure that OpCache is enabled and also activated for `php-cli` (which is running the unit tests).
This particular subquery generated/used by the Paginator is not put into the query cache. The DQL parsing/to-SQL conversion has to happen _every time_ the Paginator is used.
This, however, was already the case before this PR. In other words, this PR only changes that we do not store/update the cached result every time, but instead completely omit caching the query.
This should be replaced with a DTO in the next major.
To be able to have something usable, I had to move the validation of the
DTO (checking that it has a "class" attribute) to mapEmbedded, which
happens earlier than doLoadMetadata(). It is unclear to me why it was
not put here in the first place.
* Fixup GH8127 test case
This removes the unnecessary "middle2" class and puts back in the effect of the data provider.
Both changes were accidentally committed when I was working on #10411 and just meant as experiments during debugging.
* Fix CS
Looking at usages in the codebase, it is supposed to be at least
bool|string, but not string.
When dumping the value inside `getFieldMapping`, it is always a boolean.
This fixes two closely related bugs.
1. When inheriting a to-one association from a mapped superclass, update the `sourceEntity` class name to the current class only when the association is actually _declared_ in the mapped superclass.
2. Reject association types that are not allowed on mapped superclasses only when they are actually _declared_ in a mapped superclass, not when inherited from parent classes.
Currently, when a many-to-one association is inherited from a `MappedSuperclass`, mapping information will be updated so that the association has the current (inheriting) class as the source entity.
https://github.com/doctrine/orm/blob/2138cc93834cfae9cd3f86c991fa051a3129b693/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php#L384-L393
This was added in 7dc8ef1db9 for [DDC-671](https://github.com/doctrine/orm/issues/5181).
The reason for this is that a mapped superclass is not an entity itself and has no table.
So, in the database, associations can only be from the inheriting entities' tables towards the referred-to target. This is also the reason for the limitation that only to-one associations may be added in mapped superclasses, since for those the database foreign key can be placed on the table(s) of the inheriting entities (and there may be more than one child class).
Neither the decision to update the `sourceEntity` nor the validation check should be based on `$parent->isMappedSuperclass`.
This only works in the simple case where the class hierarchy is `Mapped Superclass → Entity`.
The check is wrong when we have an inheritance hierarchy set up and the class hierarchy is `Base Entity → Mapped Superclass → Child Entity`.
Bug 1: The association should keep the root entity as the source. After all, in a JTI, the root table will contain the foreign key, and we need to base joins on that table when traversing `FROM LeafClass l JOIN l.target`.
Bug 2: Do not reject the to-many association declared in the base class. It is ok to have the reverse (owning) side point back to the base entity, as it would be if there were no mapped superclasses at all. The mapped superclass does not declare, add or otherwise interfere with the to-many association at all.
Base the decision to change the `sourceEntity` on `$mapping['inherited']` being set. This field points to the topmost _parent entity_ class in the ancestry tree where the relationship mapping appears for the first time.
When it is not set, the current class is the first _entity_ class in the hierarchy with that association. Since we are inheriting the relation, it must have been added in a mapped superclass above, but was not yet present in the nearest parent entity class.
In that case, it may only be a to-one association and the source entity needs to be updated.
(See #10396 for a clarification of the semantics of `inherited`.)
Here is a simplified example of the class hierarchy.
See the two tests added for more details – one is for checking the correct usage of a to-one association against/with the base class in JTI. The other is to test that a to-many association on the base class is not rejected.
I am sure that there are other tests that (still) cover the update of `sourceEntity` is happening.
```php
/**
* @Entity
*/
class AssociationTarget
{
/**
* @Column(type="integer")
* @Id
* @GeneratedValue
*/
public $id;
}
/**
* @Entity
* @InheritanceType("JOINED")
* @DiscriminatorColumn(name="discriminator", type="string")
* @DiscriminatorMap({"1" = "BaseClass", "2" = "LeafClass"})
*/
class BaseClass
{
/**
* @Column(type="integer")
* @Id
* @GeneratedValue
*/
public $id;
/**
* @ManyToOne(targetEntity="AssociationTarget")
*/
public $target;
}
/**
* @MappedSuperclass
*/
class MediumSuperclass extends BaseClass
{
}
/**
* @Entity
*/
class LeafClass extends MediumSuperclass
{
}
```
When querying `FROM LeafClass l`, it should be possible to `JOIN l.target`. This currently leads to an SQL error because the SQL join will be made via `LeafClass.target_id` instead of `BaseClass.target_id`. `LeafClass` is considered the `sourceEntity` for the association – which is wrong–, and so the foreign key field is expected to be in the `LeafClass` table (using JTI here).
Fixes#5998, fixes#7825.
I have removed the abstract entity class, since it is not relevant for the issue and took the discussion off course. Also, the discriminator map now contains all classes.
Added the second variant of the bug, namely that a to-many association would wrongly be rejected in the same situation.
The ArgumentTypeCoercion one is hiding issues we could address.
Addressing them will require changes that should go in the next minor,
so I'm moving them to the baseline instead. That way, we cannot
introduce more issues of this type in this file.
* 2.14.x:
PHPStan 1.9.8, Psalm 5.4.0 (#10382)
fix typo for missing a comma (#10377)
Docs: Removing `type: 'integer'` from mappings (#10368)
Docs: Moving *attributes* mapping to first position (#10364)
Docs: Deleting duplicate mapping example (#10363)
Yet another micro-PR ;-) - as requested at https://github.com/doctrine/orm/pull/10364#issuecomment-1370155521
I also changed `$currentPrice` from `float` to `int`, since IMO it's better to store prices as `int` (=cents).
Question: Is there a reason why most typehints are `int|null`, instead of `?int`? Should I change them?
Following 1915dcd1e8, 0 is now used as a
default value, and Query::$firstResult is no longer nullable, but it
seems getFirstResult() was overlooked. The class is final, so this is no
breaking change.
* 2.14.x:
Shorter deprecation message (#10357)
Add Fully-Qualified class name in UnrecognizedField exception to ease debugging (#10342)
Include parameter types in hydration cache key generation (#10355)
It is important to have the same version of all dependencies in dev and
in the CI, otherwise it makes it hard to have the right static analysis
baseline for every environment.
Since doctrine/annotations 1.10, autoloading is used when everything
else has failed. Using registerFile() has been deprecated in favor of
that later on.
Previously, only a predefined set of automatic mappings was allowed, such as int, float, boolean, DateTime etc.
With this extension, it is possible to supply custom TypedFieldMapper implementation which takes as parameter the ReflectionProperty of a given field and decides the appropriate mapping.
A new configuration option was added to set and get the TypedFieldMapper.
The old logic was moved into a class DefaultTypedFieldMapper which is used by default when no mapper is supplied.
The selected TypedFieldMapper is passed into ClassMetadataInfo constructor. If empty, the DefaultTypedFieldMapper is used.
There is also ChainTypedFieldMapper class which allows chaining multiple TypedFieldMappers and apply them in a cascade (always if a field gets type assigned by the earlier mapper in the list, it will not be changed later).
This commit adds enumType option to DiscriminatorColumn as well as support for custom DBAL types which use PHP enums for PHP values.
Previously, the enumType option was completely missing, but also even using custom types that used PHP enums would end up in exception because ObjectHydrator would try to convert enums to string using (string) explicit conversion.
Apart from hydrators, ClassMetadataBuilder was extended to support specifying enumType.
Documentation was updated.
Previously, array of enums were incorrectly compared in UoW::computeChangeSet() resulting always in false positive, since the original data for comparison is fetched using ReflectionEnumProperty::getValue(), which returns the enum values, not enum objects.
This fix ensures comparing the individual enum array members' values.
Enum fields as ID have worked for some time, but referencing these fields from other entities in association mappings such as OneToOne, ManyToOne and ManyToMany resulted in fatal error as there was an attempt to convert enum value to string improperly.
Previously different hydrators would store the original data for enum
fields in different ways, the SimpleObjectHydrator would keep them as
strings while other hydrators would convert then to native php enums.
This would make the data in the internal UnitOfWork::$originalEntityData
array inconsistent which could've caused problems in the long run.
Now, all hydrators convert enum fields to native php enums ensuring the
original data is always consistent regardless of the hydrator used.
In 68bc00b6c6, while fixing coding style,
I introduced many, many use statements in that file. Using
half-qualified names sometimes, and unqualified names some other times
makes no sense.
If AST\ is used at least once, use it always.
It is not stable yet, but should be good enough, and this will help
taking care of #10118
Let us baseline all the new issues, just because they are new does not
mean they are more important than already-baselined errors. Besides, it
is more important to have higher standards for new code than to get an
increased baseline.
This exception is used in two places, one where $generatedMode is
clearly a string, and another where typing it as a string will cause a
type error, because $generatedMode is supposed to be an int there.
Inside the Query\AST namespace, many classes use public properties that
are supposed to be set from outside the class. Let us ignore
PropertyNotSetInConstructor for that entire namespace instead of doing
it on a case by case basis.
stderr is not a great name for something that is not meant to be
processed (piped into) a program, but to be read by humans.
Most commands should use stderr, and some of them should partially use
stdout, typically when dumping SQL requests.
The previous rework missed some details / left the documentation in an
inconsistent state. Searching for "annotation" case insensitively allows
to find discrepancies and forgotten things.
- migrate to attributes;
- add helpful phpdoc annotations;
- use typed properties;
- add type declarations.
Co-authored-by: Alexander M. Turek <me@derrabus.de>
The downside of this is that we will have to tweak the settings so that
no job is required anymore.
The upside is that builds should be faster, and less resource-intensive.
__sleep() does not list these properties as suposed to be serialized.
This means we have to assert that it is not in the many scenarios where
we resort to these properties. Introducing a private method allows us to
centralize the assertions.
* Add missing variable name in param phpdoc
* Use psalm-assert instead of psalm-param
The parameter can in fact be any string, but if an exception is not
thrown, then it was a valid value.
Deprecating this class means that users using SA tools have to
update their code specifying the ObjectManager implementation.
Instead of this, dedicated classes for each event should be
created before deprecating this class.
the description and semantics of the `--dump-sql` switch
indicate and sql dump. without that useless line
The following SQL statements will be executed:
the output can actually be used to generate a plain
dump.sql file that can be fed into sql command-
consuming programs.
resolves#8263 and #7186.
doctrine/dbal is the component responsible for generating the queries.
Let us make the test suite more robust by asserting that things work
from a functional point of view.
* 2.12.x:
PHPStan 1.8.0 (#9887)
Fix typo in AbstractQuery
ObjectHydrator: defer initialization of potentially empty collections
Migrate more usages of SchemaTool::createSchema()
preUpdate: Add restriction that changed field needs to be in computed changeset (#9871)
If ObjectHydrator faces an empty row to an uninitialized collection,
it initializes it, to prevent it from querying again (DDC-1526).
However, if that row is the first but not the only in the collection,
the next rows will be ignored, as the collection will be considered
"existing", and "existing" collections are only replaced if REFRESH hint
is present. To prevent it, we defer initialization to the end of the
hydration.
Fixes GH-9807
*FlushEventArgs classes should extend ManagerEventArgs from
doctrine/persistence to be able to use ManagerEventArgs for any
persistance implementation.
OnClearEventArgs should extend from OnClearEventArgs from
doctrine/persistence.
When I introduced OrmFunctionalTestCase::createSchemaForModels(), I made
several wrong assumptions:
- the call is always wrapped in a try / catch;
- that try / catch is always typed with "Exception".
Because of that, I missed, many, many occurrences of
SchemaTool::createSchema().
I recently noticed that contributors kept using the
SchemaTool::createSchema() and figured not everything had been
migrated.
Migrating some of them did not result in something far better until I
also introduced similar methods for
SchemaTool::getUpdateSchemaSql() and SchemaTool::getSchemaFromMetadata().
Hello, I would like to make a small change.
The need arose when using \Symfony\Bridge\Doctrine\IdGenerator\UuidGenerator in "symfony/doctrine-bridge" composite foreign keys
I'm sure these changes will not hurt performance
and allow other objects to be used as identifiers
* Remove comment about BC
I do not think we actually want to force our users to build an array
collection when they want to use setParameters().
* Make phpdoc more accurate
These tests were using the fact that some arguments of some methods of
the naming strategy interface are optional or nullable for now to avoid
providing some. In practice, these arguments are always provided, and
that should also be the case in tests.
When computing a foreign key column name, the referenced column name
may be null in the case of a self referencing entity with join columns
defined in the mapping.
* Fix inaccurate and imprecise phpdoc
* Remove extra argument
The method signature does not specify it, and it's always set to null.
It looks like this test and its data provider were copy/pasted from the
previous one.
* Specify what concrete class is passed
This is important as the signature of the concrete classes don't
necessarily match the one of the interface. Here, an extra argument that
is only defined in the classes is used.
Everything except the name key might be undefined when accessing to this public property, for instance in a LoadMetadataEvent
Co-authored-by: Pierre Bourdet <pbourdet@worldia.com>
ClassMetadataInfo used to be useful during entity generation, because it
allowed the entity not to exist. We no longer do entity generation, and
even if we did, the reflection methods have been moved to
ClassMetadataInfo as of 76e4f5a80b .
This documentation must be very old because this is no longer valid as
of e9e36dcf32 . The interface and abstract
file driver have since then been moved to doctrine/common, and the to
doctrine/persistence.
Although properties and methods are currently located in
ClassMetadataInfo, it is better to refer to ClassMetadata as the former
is deprecated in favor of the latter.
This was super annoying as UpdateCommand printed sqls prefixed with `*` so it was not possible to copy statements anymore without manually removing those asterisks.
This removes prefixing sqls and makes it consistent with Create and Drop commands.
This reverts commit 03f4468be2.
The inferring process seems fragile and MySQL-specific. The ORM might
not be the correct place to fix this issue (if it needs to be fixed at
all).
* Fix composer install in contributing readme
People that contribute know how to use composer.
* Fix static analysis for Persistence 2.5 (#9648)
Co-authored-by: Ruud Kamphuis <ruudk@users.noreply.github.com>
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
In setups where you have many parameters, or do not even realise you are
using an entity, that additional piece of context can be helpful. The
parameter name is not always available where the old exception was
called though.
* Add support for array of enums
This allows the use of 'array' and 'simple_array' in combination
with the enumType parameter.
* Reference is_array and array_map through a use statement nstead of global fallback
* Return the value of an array of enums correctly
* Add enum array mapping test
* Fix order of use parameters
* Fix return type docblock
* Apply phpcs feedback
* Fix static closure
* Add missing return type to static closure
* Add helper method for enum initialization to reduce code duplication
* Fix CS
* Replace mixed typehints with more specific ones
* Update docblock type hint to allow for array of string/int
* Fix types
* Fix types
Co-authored-by: Alexander M. Turek <me@derrabus.de>
* 2.11.x:
PHPStan 1.5.0 (#9607)
Remove Sphinx config
Use correct syntax for external links
Update XmlExporter.php - Type problem in php8.x (#9589)
Ignore deprecation from Persistence
Stands with Ukraine (#9567)
Use internal links when self-referencing
Link to docs for the stable version
I do not think this file is still useful, since AFAIK we are not using
Sphinx anymore. Besides, this is the only Doctrine project I could find
that still has that file. It was last updated 6 years ago.
Please see PHP interface SimpleXMLElement::addAttribute(string $name, string $value = null, string $namespace = null): void ....
The $value must be string or null.
This allows us to decouple further from doctrine/annotations, and to fix
some static analysis issues.
The assumption being made here is that the abstract class we are no
longer extending is not used in type declarations and instanceof checks.
* 2.11.x:
Baseline Psalm errors caused by DBAL 3.3.3 (#9577)
Make sure MemcachedAdapter is supported before tring to use it (#9574)
Fixing `:doc:` link (#9569)
Adding PHP attributes (#9555)
Remove reference to removed class
Wrong validation message is displayed when an incorrect bidirectional
bi-directional mapping is set up. When the owning side is configured
correctly and the target side is missing the back reference, the ORM
suggests adding inverseBy instead of mappedBy, with the field name
missing. This commit fixes this problem.
* 2.11.x:
Make creating test models more straightforward
Trigger the desired code path
Fix syntax typo in attributes reference (#9513)
Constructor-Argument "options" has the same type as the associated property. (#9501)
In https://github.com/doctrine/orm/pull/8962, I established that
swallowing exceptions while creating model was bad because it could hide
other helpful exceptions.
As it turns out however, swallowing exceptions is really the way to go
here, because of the performance implication of calling dropSchema(). It
is possible to catch a more precise exception as well, which should
preserve the benefits of not swallowing them.
It looks like I based my grep on the comment inside the catch and today,
I found many other occurrences of this pattern, without the easy to grep
comment.
I decided to fix them as well, but in a lazier way: one no longer has to
remember to call dropSchema in tearDown() now, it is automated.
In a88242ee6c, testDateSub() was modified
in order to satisfy different opinions to the question "What is now
minus one month", that has different answers for different pieces of
software on March 30th.
Today, we are January 30th, we need to do the same for testDateAdd().
Hopefully this is the last time we hear about this.
* 2.11.x:
Add support for PHP 8.1 enums in embedded classes (#9419)
Added class-string typehint on $targetEntity (#9415)
Allow DiscriminatorColumn with length=0 (#9410)
Move UnderscoreNamingStrategyTest to correct namespace (#9414)
* Corrected ORM version and added missing dependency
Noticed that the version wasn't updated, pointing to 2.11.0 instead of 2.10.2.
Also when following this tutotial ran into missing dependency for "doctrine/annotation" so added that too.
* Tutorial: Bump DBAL, YAML and Cache
Co-authored-by: Alexander M. Turek <me@derrabus.de>
* [GH-9380] Bugfix: Delegate ReflectionEnumProperty::getAttributes().
* [GH-9380] Add test for retrieving attributes via enum property.
* [GH-9380] Add test for retrieving attributes via enum property.
* [GH-9380] Call parent ReflectionProperty ctor for best behavior.
* Update tests/Doctrine/Tests/ORM/Functional/EnumTest.php
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* Generated/Virtual Columns: Insertable / Updateable
Defines whether a column is included in an SQL INSERT and/or UPDATE statement.
Throws an exception for UPDATE statements attempting to update this field/column.
Closes#5728
* Apply suggestions from code review
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* Add example for virtual column usage in attributes to docs.
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* Fix WhereInWalker description:
- change the verb "replace" with "append" to better describe the behaviour of this class
* Rephrase comment in WhereInWalker as suggested from reviewer
Co-authored-by: Alexander M. Turek <me@derrabus.de>
* Rephrase comment in WhereInWalker as suggested from reviewer
Co-authored-by: Alexander M. Turek <me@derrabus.de>
Co-authored-by: Alexander M. Turek <me@derrabus.de>
As of now, we cannot have specific config files for each DBAL version
and avoid repetition. We already have PHPStan performing checks with
DBAL 2, which could be considered enough.
This is enforced before writing to the property that holds FieldMapping
arrays.
As shown by the static analysis baselines reduction, this existence is
relied on throughout the codebase.
* Bump reusable workflows
* Fix union type on QueryExpressionVisitorTest::testWalkComparison() (#9294)
* Synchronize Psalm baseline (#9296)
* Fix return type (#9295)
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* Better explain limitations of DQL "DELETE" (#9281)
We think the current documentation does not stress these details enough, so that they are easily overlooked.
Co-authored-by: Malte Wunsch <mw@webfactory.de>
Co-authored-by: Malte Wunsch <mw@webfactory.de>
* Put actual value instead of index inside $originalEntityData. (#9244)
This fixes a bug with redundant UPDATE queries, that are executed when some entity uses foreign index of other entity as a primary key. This happens when after inserting related entities with $em->flush() call, you do the second $em->flush() without changing any data inside entities.
Fixes GH8217.
Co-authored-by: ivan <ivan.strygin@managinglife.com>
* Allow symfony/cache 6 (#9283)
* Fix XML export for `change-tracking-policy` (#9285)
* Whitelist composer plugins used by this repository (#9286)
Co-authored-by: Matthias Pigulla <mp@webfactory.de>
Co-authored-by: Malte Wunsch <mw@webfactory.de>
Co-authored-by: Ivan Strygin <feolius@gmail.com>
Co-authored-by: ivan <ivan.strygin@managinglife.com>
Co-authored-by: Fedir Zinchuk <getthesite@gmail.com>
This fixes a bug with redundant UPDATE queries, that are executed when some entity uses foreign index of other entity as a primary key. This happens when after inserting related entities with $em->flush() call, you do the second $em->flush() without changing any data inside entities.
Fixes GH8217.
Co-authored-by: ivan <ivan.strygin@managinglife.com>
We think the current documentation does not stress these details enough, so that they are easily overlooked.
Co-authored-by: Malte Wunsch <mw@webfactory.de>
Co-authored-by: Malte Wunsch <mw@webfactory.de>
* Docs: consistency for FQCN, spacing, etc (#9232)
* Docs: consistent spacing, consistent array-style, consistent FQCN, avoid double escaped slashes, avoid double quotes if not necessary
* Docs: use special note block instead of markdown-based style
* Docs: Quote FQCN in table with backticks to be compatible with all render engines
* Drop all mentions API doc - it is not available anymore
* Add missed FQCN for code snippets
* Revert "Fix SchemaValidator with abstract child class in discriminator map (#9096)" (#9262)
This reverts commit bbb68d0072.
* [docs] Fix wording for attributes=>parameters. (#9265)
Co-authored-by: Andrii Dembitskyi <andrew.dembitskiy@gmail.com>
Co-authored-by: olsavmic <molsavsky1@gmail.com>
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
* Improve compatibility with AbstractPlatform::getLocateExpression() in DBAL 4
* Improve compatibility with AbstractPlatform::getTrimExpression() in DBAL 4
* Improve compatibility with Connection::quote() in DBAL 4
* [GH-9240] Refactor Association/AttributeOverrides to use @NamedConstructorArguments with AnnotationDriver.
* [GH-9240] Add support for PHP 8.1 nested attributes.
Supported/new attributes are #[AttributeOverrides], #[AssociationOverrides], #[JoinTable] with nested joinColumns, inverseJoinColumns.
* [GH-9240] Add support for nesting Index, UniqueCosntraint into #[Table] on PHP 8.1
* Apply review comments by gregooire.
* Add documentation for new attributes.
* Add docs for new nested #[JoinTable] support of join columns
* Add docs for new nested #[Table] support of index, uniqueConstraints
* Rename "Required/Optional atttributes" to "Required/Optional parameters"
* Remove nesting for JoinTable#joinColumns and Table#indexes/uniqueConstraints again.
* Hosuekeeping: phpcs/psalm
* housekeeping
* Remove unused function imports.
* Docs: consistent spacing, consistent array-style, consistent FQCN, avoid double escaped slashes, avoid double quotes if not necessary
* Docs: use special note block instead of markdown-based style
* Docs: Quote FQCN in table with backticks to be compatible with all render engines
* Drop all mentions API doc - it is not available anymore
* Add missed FQCN for code snippets
* Fix Hidden fields triggering error when using getSingleScalarResult()
Fixes#4257
HIDDEN fields was causing the "unicity" check to fail (NonUniqueResultException), because we was counting raw data instead of gathered row data.
* Fix Coding Standards (7.4)
* Fix Coding Standards (7.4) #2
* Fix Coding Standards (7.4) - Fix whitespaces
* Fix Coding Standards (7.4) - Fix whitespaces in tests
* Fix Coding Standards (7.4) - Fix more things
* Refactor tests into separate methods
* Fix Coding Standards (7.4) - Equals sign not aligned with surrounding assignments
* [GH-7512] Bugfix: Load metadata on object-typed value in EntityPersisters
* [GH-7512] Refactor double check for object/entity and flatten code.
Co-authored-by: Joe Mizzi <themizzi@me.com>
Field mapping have different definitions
in property definition and method return.
As suggested in issue and to avoid further desynchronization,
a psalm type has been created.
Fixes#9193
* Deleting "Not needed for XML and YAML mapping" - this was stupid of me, since *all* annotations are obviously not needed in XML&YAML ;-)
* Shortening the @Column annotation, for consistency with the following event handlers
* Removing some blank lines from XML, for consistency with YAML
* Adding PHP Attributes
Testing with several platforms should not increase code coverage here,
since the DBAL is responsible for providing the concat expression for
each platform.
Moreover, whenever that concat expression changes for one of the tested
platforms, this test will break.
In doctrine/dbal 3.2, that is the case for SQLServer2012Platform, which
means this test no longer passes.
* Fixing more links
The first two I missed in https://github.com/doctrine/orm/pull/9151
The third is probably older.
Shouldn't the chapter name be displayed as link text by default?? Are you sure that everything is set up correctly with the parser?
* Update architecture.rst
* Update getting-started.rst
* Update events.rst
getTypeOfColumn() relies on getTypeOfField(), and does not suffer from
mismatching issues caused by quoting, because you cannot quote a field.
Since a field can be composite, that method returns an array, hence why we
need to select the first element.
As discussed in https://github.com/doctrine/orm/issues/9078 when entities utilize data mappings which are provided by the dbal lib it is expected behavior that users will explicitly define their dependency on the package.
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
Three references to `$isDevMode` were marked up with a single backtick, however two backticks are required in order for the variable name to be highlighted correctly (c.f. `ArrayCache`).
* Overview table for events
Better late than never - finally delivering what I announced at https://github.com/doctrine/orm/pull/8435#issuecomment-769940427 :-)
* Update events.rst
* Update events.rst
* Adding "Lifecycle Callback" column
* Update events.rst
Looking at twitter, the hashtag its hardly used. There was 1 question posted in the last year, and it went unanswered.
The `2` part has mostly been dropped everywhere, and orm is now just refered to doctrine orm instead of doctrine2
My previous attempts to disallow running a workflow when pushing a tag
failed, so let's ensure we can run said workflow. Maybe we will be able
to understand why it happened after it happens.
* 2.9.x:
Run PHP 8.1 CI with stable dependencies (#9058)
Duplicate testTwoIterateHydrations (#9048)
Add PHP 8.1 to CI (#9006)
Fix locking non-existing entity (#9053)
Signed-off-by: Alexander M. Turek <me@derrabus.de>
* 2.9.x:
Minor rewording (#8435)
Don't presume one-to-one lookup returned an entity (#9028)
Minor change about double The (#9038)
Remove duplicate comment (#9036)
Fix docblock types for some nullable properties (#9024)
Explicitly allow to use `Comparison` and `Composite` in JOIN conditions (#9022)
Fix some typehints in QueryBuilder
Bump PHPStan (#9014)
Add tests for advanced types in collection matching
Use types in collection persister
Signed-off-by: Alexander M. Turek <me@derrabus.de>
If `$this->em->find()` returns null, don't treat it like an object. Instead, just set the field to null and back out of the switch statement.
Fixes#9027
* 2.9.x:
Remove Proxy from EntityManagerInterface contract
Add extension point for the "embedded" XML node (#8992)
Fix return type at `EntityManagerInterface::get(Partial)Reference()` (#8922)
Fix class casing and avoid name collisions
Remove unused performance base test class
Drop unused test base classes
Fix mapped superclass missing in discriminator map
* 2.9.x:
Restore functional cache tests (#8981)
Fix English in `note`. (#8987)
Remove detach deprecation entry in UPGRADE.md (#8978)
Bump to PHPStan 0.12.98 and Psalm 4.10.0 (#8979)
Signed-off-by: Alexander M. Turek <me@derrabus.de>
* Added clarification of using change tracking policy on entities with embeddables
* Apply suggestions from code review
Co-Authored-By: carnage <carnage@users.noreply.github.com>
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
Getting the database name from a connection object results in a PDO
object being created, which might in turn result in an error message if
the database does not exist. For instance it does with PostgreSQL.
In some other situations, like when using sqlite, there is no database
name though, so we still have to fallback on the previous behavior.
JSON_ARRAY has been deprecated in favor of JSON for a while, we should
not encourage people to generate new entities with it, since it will
introduce technical debt for them.
Support for JSON is added instead.
This was probably done in order to get rid of exceptions about tables
already existing, but may and does swallow other exceptions as well, for
instance exceptions about sequences failing to be created on Oracle
because the identifier is too long. This makes it unnecessarily hard to
understand what is going on.
This strategy has been marked as TODO for more than 14 years. It should
be OK to remove some things related to it since they lead to an
exception being thrown.
Many of the variables in AbstractHydrator are not initialized in the
constructor, and should be documented as possibly null because of that.
Introducing accessors that perform null checks allows to to have to do
these null checks when using the accessors.
Making the member variables private would be a backwards-compatibility
break and could be considered for the next major version.
This makes Psalm's and PHPStan's baselines smaller, and should make
implementing new hydrators easier.
Fix psalm issues with type: `PossiblyNullArgument`, found after updating doctrine/dbal to v3. Override `null` passed as offset with `0` in calls to `Doctrine\DBAL\Platforms\AbstractPlatform::modifyLimitQuery`. Override `null` passed as lockMode with `LockMode::NONE` in calls to `Doctrine\DBAL\Platforms\AbstractPlatform::appendLockHint`.
Partialy fixes#8884
Making these jobs green will not result in a comprehensive result of the
features, but it is a good start, and having them should give a good
overview of what is left to do.
Namespacing is configured here, which means instead of just one layer,
we have a PSR cache wrapped in a doctrine cache with namespacing, itself
wrapped again with a PSR cache.
We want to get rid of it, because it is not really usable in context
when you use annotations from more than one namespace. This implies
importing classes for all annotations in use.
The session examples contained approaches that aren't favorable for an application
and needed an overhaul using either scalar values or an DTO instead of the whole
entity with detach() and merge().
Updated useResultCache() and setResultCacheLifetime() docs to clearly state $lifetime is measured in seconds;
Added description to useResultCache()'s params. Renamed $bool => $useCache.
When I tried the example code, I got this unique index too.
Plus my `ALTER TABLE` statement was a little different from the one in the docs:
> ALTER TABLE cart ADD CONSTRAINT FK_BA388B79395C3F3 FOREIGN KEY (customer_id) REFERENCES customer (id)
Should I change that too?
First, there are other drivers than the annotations-based one, and
second, one of them is base on attributes, which are basically
annotations native to PHP.
Closes#8785
Some relationship types are attached to properties that can be typed
since php 7.4.
To avoid repeating the property type in targetEntity, an autodetection
feature has been developed that allows building the mapping from that
type information and annotations.
Omitting the targetEntity stays deprecated for ManyToMany as there is no
auto-detection for that yet (although one could be built by analyzing
phpdoc annotations).
* fix: handle repeatable attributes
* Restore interface compatibility
A reader is supposed to only ever return objects. By introducing
RepeatableAttributeCollection, we fulfill the interface and improve
clarity.
* refactor: accurate type declarations and returns
* refactor: remove unused use
* Ignore AttributeReader in phpstan and psalm to pass on CI running PHP 7.4.
* test: isTransient
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
* Mark 2.8.x as unmaintained, and 2.9.x as current
* Fix ClassMetadataInfo template inference
* Fix metadata cache compatibility layer
* Bump doctrine/cache patch dependency to fix build with lowest deps
* Add generics to parameters
* Add note about performance and inheritance mapping (#8704)
Co-authored-by: Claudio Zizza <859964+SenseException@users.noreply.github.com>
* Adapt flush($argument) in documentation as it's deprecated. (#8728)
* [GH-8723] Remove use of nullability to automatically detect nullable status (#8732)
* [GH-8723] Remove use of nullability to automatically detect nullable status.
* [GH-8723] Make Column::$nullable default to false again, fix tests.
* Add automatic type detection for Embedded. (#8724)
* Add automatic type detection for Embedded.
* Inline statement.
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
Co-authored-by: Vincent Langlet <VincentLanglet@users.noreply.github.com>
Co-authored-by: Andreas Braun <git@alcaeus.org>
Co-authored-by: Fran Moreno <franmomu@gmail.com>
Co-authored-by: Juan Iglesias <juan.manuel.iglesias93@gmail.com>
Co-authored-by: Claudio Zizza <859964+SenseException@users.noreply.github.com>
Co-authored-by: Yup <warxcell@gmail.com>
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
When using inheritance, it is possible to map the same column to properties of
different child classes. This can result in the same column being selecting several
times with different aliases in one SQL query, and only one aliased field needs
to be hydrated per row.
We now check that such an aliased value is mapped to the class we are hydrated
before attempting to convert it as it might result in an error when using a custom
type that does not get the expected data to initialize php value.
Co-authored-by: Sergey Naumov <s.naumov@lamoda.ru>
It seems like IdentityMapTest cannot be run on its own when the second
level cache is enabled (with ENABLE_SECOND_LEVEL_CACHE=1).
It does work when running the whole test suite because
ExtraLazyCollectionTest disables part of that cache in its setUp()
method.
In this patch, we restore the class metadata as it was before running
setUp() and put the test in IdentityMapTest inside the group that is
excluded when running with ENABLE_SECOND_LEVEL_CACHE=1 on the CI.
* [GH-8327] Make EntityManagerProvider compatible with expected DoctrineBundle usage.
* phpcs
* [Gh-8327] Delegate to EntityManagerProvider::getDefaultManager in ConnectionFromManagerProvider
The code as is needlessly flushes after just one row is updated or
removed. It makes more sense to update after ``$batchSize`` elements are
updated or removed, just as it is in the insert case.
* #1168 Add support for array parameters on the SQLFilter
* DDC-1168 Add support for array parameters on the SQLFilter
* [GH-2624] Rework array support to use new getParameterList()
* [DDC-1952] Change at() mocking to using returnCallback()
* [DDC-1952] Make arrays of values explicit with new setParameterList
* Adjust tests to use country as list and locale as single value.
* [DDC-1952] Add tests for new exxeption conditions.
* Apply suggestions from code review
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
Co-authored-by: Manuel Nogales <nogales.manuel@gmail.com>
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
This includes:
- private methods
- return type declarations of final protected methods
- return type declarations of public and protected methods of final classes
Parameter type declarations are a more delicate matter and should
probably be handled separately to make it easier to catch issues during
code review.
Type declarations can be more trusted than simple phpdoc when it
comes to static analysis, having them means we can infer the phpdoc of
calling methods with confidence.
Note that it seems that some of the phpdoc I initially inferred these
declarations from were apparently wrong, in particular some mentioning
Doctrine\Dbal\Statement when was is really passed around is
Doctrine\Dbal\Driver\Statement.
* Reintroduce PHP 7.1 support
* phpcs
* Another object typehint
* More compatibility
* phpcs
* Reduce doctrine/inflector versions again since 1.4.4 is released.
* Housekeeping: phpcbf
* Simplify PHPUnit Polyfill abstraction.
* Missing assertDoesNotMatchRegularExpression
* phpcs
* Add 7.1 on Github actions, since dependencies now supported.
* Simplify code to work with renamed phpunit assertions and document when to remove.
* phpcs
* Downgrade target phpstan version to 7.1.0
* Run --prefer-lowest with PHP 7.1 not 7.3
* Rebase QueryHintReadOnly on 2.9.x
* Housekeeping: phpcs
* [GH-5202] Dont mark known entities as read only.
* [GH-5202] Not mark objects read only when proxy before.
* phpcs
* cli config
* [GH-8327] Deprecate EntityManager HelperSet for a provider abstraction.
* Housekeeping: phpcs
* [GH-8327] Refactor tests towards use of SingleManagerProvider instead of HelperSet.
* [GH-8327] Refactor tests towards use of SingleManagerProvider instead of HelperSet.
* [GH-8327] Refactor tests towards use of SingleManagerProvider instead of HelperSet.
* Housekeeping: cs
* Update tests/Doctrine/Tests/ORM/Tools/Console/ConsoleRunnerTest.php
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* [GH-8327] Change option from entity-manager to em for consistency with DoctrineBundle.
* Add final to new methods and classes
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* [GH-8327] Bugfix: phpstan detected stricter type checks needed.
* phpcs
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* Make Column::$type, Column::$nullable and JoinColumn::$nullable nullable by default
* Add tests for mapped typed properties (type and nullable)
* Fix Yaml driver tests and remove driver exceptions thrown too early
* Fix PHP driver tests
* Fix static PHP driver tests
* Fix XML driver tests
* Coding Standards
* Deprecate unused MappingException method
* Add manyToOne test and check nullable at the right place
* Coding Standards
* Bugfix: Temporarily change association join columns in CascadeRemoveOrderTest to circumvent new CommitOrderCalculator bug.
* phpcs
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
* Add possibility to use fields instead of column for unique constraint and indexes (#8345)
* Document changes in annotation reference
* phpcs
* Ensure exactly one of fields/columns is set for index/uniqueConstraint
* Adapt docs to fields/columns changes
* phpcs
* Implement fields in Attribute driver and fix mapping classes constructors.
* Coding Standard
* Apply suggestions from code review
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* phpcs
Co-authored-by: Jakub Caban <kuba.iluvatar@gmail.com>
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
This helps SA tools figure out that it is fine to call count on the
return value of that method.
As a side-effect, using $metadata->name is not really an option since it
is not part of the ClassMetadata interface.
There are many CS and SA-related changes in the ORM as we pursue better
code quality, and easier contributions. These often involve huge
changes, which can be hard to review and inevitably lead to some
regressions. I know some of those could have been avoided if we were
using stricter levels for PHPStan and Psalm.
By adding baselines, we ensure new code is at level 5 for both tools,
which should allow us to catch the most interesting issues.
Some executors may return integers, for instance executors that only
execute update or delete statements.
Also, in case an integer is not returned, what's actually returned is a
Doctrine\DBAL\Driver\ResultStatement, and not a Doctrine\DBAL\Driver\Statement
It makes tests more isolated from each other: another test relying on
some tables including some of the ones created here may fail creating
the tables it needs because they already exist.
* [GH-6394] Bugfix: IdentifierFlattener support for association non-object values
* [GH-6394] Bugfix: BasicEntityPersister::update used wrong identifiers for version assignment.
* Exclude MissingNativeTypeHint phpcs rule as 7.4 is not lowest version.
There seems to be at least 2 camps in the software world when it comes
to the question "What's today minus one month", today being at the end
of march.
While PHP and SQLite agree that that would be the 2nd of March, other
RDBMS than SQLite and humans will tell you that it's the last day of
February.
This patch ensures that we check one logic for SQLite, and the other
logic for other platforms.
Decorated text used to be wrapped too early in SymfonyStyle->block()
See https://github.com/symfony/symfony/pull/40348
The fix was not contributed to version 3, which means we have to rewrite
the test so that it passes for both the correct and the buggy version.
* [GH-8265] Prototype for Attribute Metadata Driver
* [GH-8265] Skip AttributeDriverTest on PHP 7.
* [GH-8265] Fill more test entities with Attribute declarations to pass tests.
* [GH-8265] More test entity attributes for passing AttributeDriverTest.
* [GH-8265] Final changes to get AttributeDriverTest passing with test entities.
* [GH-8265] automatically update cs for new code when possible.
* [GH-8265] exclude sniffs that break because of phpcs not knowing attributes.
* [GH-8265] Fix AttributeReader styles.
* [GH-8265] Fix AttributeReader styles.
* [GH-8265] Missing changes to AttributeDriver
* [GH-8265] Fix InverseJoinColumn attribute cs violations.
* [GH-8265] Fix AbstractMappingDriverTest::_loadDriver and other CS
* [GH-8265] Coding styles
* [GH-8265] Coding styles
* [GH-8265] Coding styles
* [GH-8265] Coding styles
* [GH-8265] Convert Cache, ChangeTrackingPolicy, Column to named annotations.
* [GH-8265] Convert all annotations to named constructor for attribute support.
* [GH-8265] Style after attribute changes.
* [GH-8265] more styles
* [GH-8265] Remove workaround code for attributes.
* More cs
* More cs
* More cs
* More cs
* Add Attribute Metadata driver reference.
* Housekeeping: phpcs
* More merge conflict resolutions
* phpcs
* fix broken merge
* Change NamedArgumentConstructorAnnotation interface to use NamedArgumentConstructor annotation instead.
* phpcs
* Housekeeping: cs
* Housekeeping: cs
* Update docs with review comments
* Improve attribute docs
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
* Rename AttributesDriver to AttributeDriver
Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
While doctrine still supports php7.2 the test cases need to run
under phpunit8 as well. However some assertion methods produce
deprecation warnings in the test output with phpunit >= 9.
This commit adds a thin forward compatiblity wrapper for the
new assertion method names so that they can be used with both
supported phpunit versions.
Some tests were still using deprecated assertion and mocking methods,
resulting in a long list of warnings in the phpunit output.
This commit resolves all the warnings:
* Fixes a couple of test names in `@depends` tags (presumably these
tests were renamed at some point for coding standards).
* Changes how mocks are configured when asserting the same method
is called multiple times with a sequence of arguments / sequence
of return values. The old `->at` expectation is deprecated because
it can be brittle and give unreliable results. Some of this
mocking could probably still be refactored further, but my focus
was on solving the deprecation with the existing setup.
* Removes one use of prophecy for mocking, in favour of using
phpunit mock objects. Prophecy now requires an extra composer
dependency and a trait which seems overkill given it was only
used in one place.
* Updates to the new names for assertFileExists and assertRegExp
(and their `not` versions) - these are functionally equivalent.
* Replaces the last few references to old PHPUnit_Framework_XXX
classes with their namespaced equivalent. These were mostly in
comments, or in native php `assert()` statements that were sanity
checking mocked object types. These asserts are probably redundant
(and are clearly not running in CI since the classes they referenced
no longer exist).
To avoid confusion, the `tmpdb_` test config values are now named
`privileged_db_` and better documented in the phpunit.xml.dist.
The TestUtil class has been refactored to more closely mirror the
structure and method / variable naming of the equivalent in
doctrine/dbal. This does not introduce any significant functional
changes. The only real difference is that the test output now prints
the selected database driver the first time it is referenced,
rather than repeating this through the test run.
These tests had not been running in CI so missed the previous
phpunit upgrade.
Note that assertions in decimal/floaty values in GH7941Test have
been changed to compare numerically with a reasonable level of
precision, instead of using regex. This is because the types
returned by the different drivers are inconsistent, and phpunit
now only allows regex comparisons on strings.
Builds using the github actions phpunit.xml files were not properly
recognising driver-specific configuration values, so were all
falling back to use the in-memory sqlite database instead of the
expected driver. This also meant a number of tests were skipped
as they rely on functionality not available in sqlite.
This commit addresses that by:
* REMOVING the automatic fallback to the sqlite memory database -
phpunit.xml must now always specify explicit parameters for the
desired connection.
* Displaying the active driver in the build output for visibility
and debugging.
* Changing the way TestUtil loads the database config in line
with the equivalent logic in doctrine/dbal, and to support the
way that the config is/was specified in the phpunit.xml files
for CI.
Note that this means a couple of the expected config variable names
have changed. Developers that are using customised phpunit.xml files
locally will need to update them to provide:
* Database config variables if they want to use the sqlite/memory
driver - see phpunit.xml.dist for details.
* `db_driver` instead of `db_type`
* `db_user` instead of `db_username`
* `db_dbname` instead of `db_name`
* And, if in use, the equivalent changes to the `tmpdb_` values
The other change is that now if you provide any value for
`db_driver` we will attempt to create that connection type and
that will throw if other details (username / password / etc as
required by the driver) are not provided. Previously providing
partial configuration would cause TestUtil to silently fall back
to the in-memory sqlite driver.
As discussed in #8527, when using optimistic locking with integer
version columns, Doctrine has always supported passing the lock
version as a string. For example when passing in a version
received in POST / GET.
Technically speaking this does not comply with the docs and phdoc
(which show the app explicitly casting to int before passing).
Nonetheless the maintainers decided it should continue to be valid
for now and reinstated the old soft-equals logic with #8531.
This modified test just avoids accidental changes in future.
[GH-7939] Detect column and association types from typed properties.
* Use typed properties for default metadata for #7939
* Coding Standards
* Remove $name from CmsUserTypes and adapt tests
* Factor out conditions required for typed property
* Factor out typed validation and completion methods
* Move Typed tests model to separate namespace
* Don't pass by reference, return array
* Document changes to default mapping for typed properties
* Better wording in annotation reference
* Add missing targetEntity assertion on typed association
* Try to comply with CS
* USe constants instead of strings
* Use one-line comments for single line content
* phpcs
* phpcs
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
When using optimistic lock with DateTimeInterface based version field a bug appears due to the use of the === operator for comparing the lock version and the entity version. This comparison always resolves to false because the === operator when comparing objects is only true when both sides are the exact same instance of the object.
To fix the issue I have decided to compare timestamps instead the DateTimeInterface based objects directly, calling getTimestamp() method and doing a strict comparison.
Modified OptimisticLockException to use DateTimeInterface instead of DateTime class.
Added test suite to cover case.
Fixes#8499
* [GH-8410] Fix memory leak in new toIterable and state bug.
The new AbstractQuery::toIterable() had a memory leak that
AbstractQuery::iterable() did not have. This leak is now fixed.
After fixing the leak, one test failed where the identity map in
ObjectHydrator triggered and lead to a notice. Introduced a new
AbstractHydrator::cleanupAfterRowIteration() that the ObjectHydrator
uses to cleanup the state.
* [GH-8413] Bugfix: Iterating with multiple, mixed results
When multiple entity results are part of a row, the result handling
must be different. In addition mixed results with scalars are broken
and now throw an exception as illegal operation.
* Housekeeping: phpcs
* [GH-8413] Add assertions for entity alias iteration.
* [GH-8387] Missing @deprecated on Query::iterate
* Ensure identifier type is set on deleteJoinTableRecords
* Housekeeping: phpcs
* Housekeeping: phpcs
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
* Introduce doctrine/deprecations, empty out VerifyDeprecations trait for now.
* Replace more usages of VerifyDeprecations (akwardkly for now)
* Update doctrine/deprecations to v0.2.0
* Remove ORM VerifyDeprecations trait and its use.
* Use doctrine/deprecatios VerifyDeprecations trait where useful
* Housekeeping: phpcs
* Fix reference link for toIterable/iterate deprecation
* Add support for INDEX BY an associated entity
This allows specifying an association in the INDEX BY clause of a query
which will index by the association's join column.
Related to #7661.
* Reintroduce IndexBy#simpleStateFieldPathExpression as deprecated property.
* Housekeeping: phpcs
* Housekeeping: phpcs
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
git-phpcs can publish comments to Github, that will be helpful.
phpcbf will still be helpful to ensure no one upgrades the coding
standard without also fixing issues that can be autofixed.
That release comes with a fix for a bug that affects us since we are
using return type declarations for wakeUp() in proxyfied classes in on
of our tests.
When an entity with change tracking policy "deferred explicit" gets
removed, then persisted again, it is not schedulded for a dirty check
synchronization. This is not the case for entities that are persisted
and are already in the managed state.
The support for passing \DateTimeImmutable instance as a query parameter has
been added to ORM in #1333 (the year 2015), a long time before immutable date
types (datetime_immutable etc) were introduced to DBAL in doctrine/dbal#2450
(2017).
Back then, it made sense to treat \DateTimeImmutable (or any
\DateTimeInterface) in the same way as \DateTime and infer parameter type as
datetime. However, when immutable date types were later added to DBAL, it
wasn't reflected anyhow in type inference in ORM and \DateTimeImmmutable
instances are still inferred as datetime DBAL type.
This PR fixes this IMO incorrect behaviour of
ParameterTypeInferer::inferType(): for a \DateTimeImmmutable parameter, it now
returns datetime_immutable DBAL type; for \DateTime or any other types
implementing \DateTimeInterface, it returns datetime DBAL type as it did
before.
This behaviour is in line with DateTimeImmutableType handling only
\DateTimeImmutable and DateTimeType handling any \DateTimeInterface.
Why? In most cases, it doesn't matter and datetime works for \DateTimeImmutable
parameters just fine. But it does matter if using custom implementation of
datetime_immutable type like UTCDateTimeImmutableType from
simpod/doctrine-utcdatetime. Then the broken type inference is revealed.
This is partially related to #6443, however, this PR isn't about custom DBAL
types but about correct type inference for build-in types.
* Fix ambiguous case where an entity is also a Traversable
* Address phpcs violations.
* Address phpcs violations.
* Address phpcs violations.
Co-authored-by: Laurent VOULLEMIER <laurent.voullemier@gmail.com>
* [GH-8229] Prevent AttributeOverride on fields from entities, only allowed for MappedSuperclass
* [GH-8229] Prevent AssociationOverride on fields from entities, only allowed for MappedSuperclass
* Revert "Fix SQL alias generation regression for simple inheritance (#8329)"
This reverts commit f4ebded63c.
* [GH-8229] Finalize checks for illegal attribute/assocation overrides.
* [GH-8229] Revert ccae8f7176 PR #8234
* [GH-8229] Update documentation to clarify only mapped superclass or trait works with overrides
* [GH-8229] Fix style violations introduced by revert
* [GH-8229] Fix style violations introduced by revert
* [GH-8229] Temporarily disable the exception until 2.8.
* Make phpcs happy
* Update doctrine/dbal to 2.12 for PHP 8 support.
* Change Query\Parser::match to Query\Parser::matchToken including DQL functions.
* Fix phpunit constraint to 9.4, adjust @group usage to workaround PHPUnit bug.
* Fix PHPUnit API related changes.
* Add PHP 8 support for EntityGenerator namespace detection.
* Use new assertEqualsWithDetla for QueryDqlFunctionTest with date comparisons
* Replace ReflectionParameter::getClass usage with non-deprecated ::getType instead.
* Revert "Change Query\Parser::match to Query\Parser::matchToken including DQL functions."
This reverts commit 279070491d50deaa4d41e17b28bb5a68f5a22796.
* More matchToken => match reverts
* Housekeeping: phpcs
* Housekeeping: phpcs
* Housekeeping: phpcs
* Housekeeping: phpcs
* Add PHP 8 testrunner, update composer.json and small fix in OrmFunctionalTestCase for new PHPUnit behvaior
* Update doctrine/coding-standard to 8.x
* Update rule names for doctrine/coding-standard v8.0
* Update to Psalm 4.
* Not failOnWarning anymore.
* Fix phpcs
* fix phpcs
* remove 7.2 for now until we can support in DBAL.
* Relax doctrine/dbal requirement and add 7.2 CI support again.
* Revert to whitelist
coverage requires PHPunit 9, and we don't have that yet.
* Upgrade to PHPUnit 8
This unlocks PCOV usage for coverage
* Upload coverage files to Codecov
* Move PHPUnit runs from Travis to Github Actions
This removes all artifacts used for TravisCI testing and replaces them
with the existing infrastructure for Github Actions from DBAL component.
In addition some test changes were needed and triggered larger Coding
Style cleanups in 3 test files.
* Remove composer.lock and improve naming in CI workflow.
* Update outdated doc parts
- The cache implementation moved from `Common` to `doctrine/cache`
- APCu is mor appropiate nowadays I guess
- AbstractQuery::useResultCache() is deprecated since 2.7
* Fix wrong argument
* Fix wrong arguments and remove useless line
* Use a classname that exists
Doctrine\ORM\Mapping\TableGenerator does not exist, only
Doctrine\ORM\Id\TableGenerator does.
* Upgrade doctrine/coding-standard
That library has a dependency on another library that requires composer
plugin API v1. Updating both libs allow to use Composer v2.
* Account for doctrine/reflection deprecation
There is parser bug for OrderByItem(), where a function is detected
before a SimpleArithmeticExpression while it should be the reverse:
simple arithmetic expressions can start with a function too, and thus
they should be matched first if present.
This fix enables the use of expressions in sorting and with window
functions.
This fixes problems with id columns defined in the parent class but renamed in the child class using an attribute override. Before this change always the child column name was used (which was not present in the parent class), now the correct column names are used for the parent table when creating inserts, joins and deletions for it.
Co-authored-by: Crossjoin <crossjoin@users.noreply.github.com>
I think this was a mistake when writing this documentation, and that the
original author meant to use attribute here, columns do not have
columns, they have attributes.
This is needed to preserve parent information and be able to find the
merge base with the head branch. The assumption here is that the common
ancestor can be found among the 10 parent commits.
* Fix type errors
(partially cherry picked from commit 17e7c2a42e)
* Fix QueryBuilder::getParameter() on parameter names with colons
Fixes#8106.
Co-authored-by: Michael Moravec <mail@majkl578.cz>
* Prevents incorrect table aliases to be generated
When a defined subclass has a case typo, the query builder will be lost
and will generate exotic table alias. This commit fixes the issue at the
root by prohibiting case typo in discriminator map.
See https://github.com/doctrine/orm/pull/8112 for the consequence of
such typo.
* Controls growing rate of the abstract test class
* Fixes incorrect test case
The Cube class must be autoloaded with the correct case, otherwise
composer autoloader will complain.
* Removes non architecture compliant code
See https://github.com/doctrine/orm/pull/8122/files#r423952247
* Ensures discriminator map is case sensitive
* Remove useless ternaries
If these expressions are truish inside the condition, they will still be
truish inside the if.
* Describe properties more accurately
These are not objects, they are strings holding class names for classes
that implement TreeWalker.
* Remove duplicate key
Comparison::IS and Comparison::EQ are the same. I chose to remove IS
because it does not seem to exist anymore on master
* Remove unwanted . before = operator
This worked, but makes no sense.
* Setup static analysis with Psalm
* Move PHPStan to Github actions
Change code examples for `cli-config.php` to use
`ConsoleRunner::createHelperSet` instead of constructing the helper set
array yourself.
Remove DBAL-specific documentation
Co-authored-by: Claudio Zizza <859964+SenseException@users.noreply.github.com>
A backwards-compatibility layer has been added to persistence to help
consumers move to the new namespacing. It is based on class aliases,
which means the type declaration changes should not be a BC-break: types
are the same.
See https://github.com/doctrine/persistence/pull/71
This means:
- using the new namespaces
- adding autoload calls for new types to types that may be extended and
use persistence types in type declarations of non-constructor methods,
so that signature compatibility is recognized by old versions of php.
More details on this at
https://dev.to/greg0ire/how-to-deprecate-a-type-in-php-48cf
I changed a relationship from eager to lazy loading which broker the behaviour of my application in regards to object removal. It was not clear for me that removing an object detaches it and subsequent calls like contains() in a OneToMany relationship with the object scheduled for removal will return false afterwards.
* [GH-7633] Bugfix: Partial queries were stored in 2LC.
There was a check in DefaultQueryCache that prevented partial queries,
because they are not supported. However the checked hint
Query::HINT_FORCE_PARTIAL_LOAD is optional, so cant be used to prevent
caching partial DQL queries.
Introduce a new hint that the SqlWalker sets on detecing a PARTIAL
query and throw an exception in the DefaultQueryCache if thats found.
* Housekeeping: CS
* [GH-7633] HINT_FORCE_PARTIAL_LOAD still needs to be checked.
* Housekeeping: Fix CS
* Add test case
* Treat parent embeddables as mapped superclasses
* [GH-8031] Bugfix: Get working again on nested embeddables in inherited embeddables.
* Housekeeping: CS
* Update note on limitations
* [GH-8031] Verify assocations still do not work with Embeddables.
* Housekeeping: CS
Co-authored-by: Benjamin Eberlei <kontakt@beberlei.de>
* Throw OptimisticLockException when connection::commit() returns false
* Update unit tests
* Fix doctrine persistence version to avoid deprecations changes
* Apply changes from 2.8.x
* Update from 2.8.x
Since `Doctrine\ORM\Version` is now deprecated it shall not be updated
on future releases.
This ensures that our CLI tool will present the correct version number.
Join table name doesnt depending on the platform.
Table name was "schema.table" instead of "schema__table".
(cherry picked from commit 4878cd3f4ef30ffc6047c18e0f7b16aafeabc3b4)
We broke our BC promises on the last patch release by changing how the
underscore naming strategy parses values with numbers.
This commit makes it possible to configure whether or not to make the
underscore naming strategy aware of numbers, keeping the old
configuration as default value.
We're keeping a BC layer in the hydrator, which prevents type conversion
in scalar results.
This makes bypasses such layer in order to always convert the identifier
types when limiting the result set during a pagination.
The main goal here is to keep the conversion DB->PHP inside of the
hydrator components.
The composer.lock is put under version control and it often happens to
be out of sync with the composer.json, which could lead to
hard-to-understand issues.
Using the --strict option here because we might as well aim for a
perfectly valid composer.json
This was regression from #7862 which tried to respect tracking config
when clearing collections, but this logic can happen in UOW only,
PersistentCollection::clear is triggered too early to know what
is (going to be) persisted.
Fixes#7862
In order to figure out the paginated query identifier type, we would
have to parse the DQL query into an AST+SQL anyway, so we'd have
to re-parse it manually: instead of doing that, we can force the
`WhereInWalker` to be reached at all times by forcing the
`$whereInQuery` to use no query cache.
While it is a sad performance regression, it is also not a
noticeable one, since we'll be performing an `O(1)` operation
around an I/O one (query execution, in this case).
Since `WhereInWalker` does not run, query parameters are not translated
from their in-memory type to the expected SQL type when the paginator
is run again with the same DQL string. This is an architectural
issue, since (for the sake of simplicity) we moved parameter
translation into the SQL walker, we didn't consider that SQL
walkers only act when no cache is in place. The translatio
needs to be moved into the paginator logic again.
If no orderings are given to PersistentCollection::matching(), the
orderBy annotation will be used if present. If the criteria contains
orderings, those will be used without merging them with the orderBy.
See #7836
Looking for a way to improve one of our bulk update treatment, I went back to this page then found elsewhere that setting logger to null was a really effective way to improve time and memory consumption. Might be a right place to state it ? Don't know if my edit style is ok
This logic was pre-existing, but I forgot about it while writing
#7820, therefore it was re-implemented inside this unit of
code. Now we just use the `PersisterHelper`, which does all
the nice and shiny identifier type discovery operations we need.
there be dragons, and this change does indeed rewrite the previous
approach by moving the responsibility of type conversion on a query
object from the `Paginator` to the `WhereInWalker`, which already
has access to class metadata for the root of the selection (and can
reliably detect the root of the selection too)
This patch introduces new internal API on the `ResultSetMapping` class, which is responsible
for finding the type of the single column identifier of a DQL query selection root.
When using a `Doctrine\ORM\Tools\Pagination\Paginator` to iterate over a query that has entities with a custom DBAL type used in the identifier, then `$id->__toString()` is used implicitly by PDO, instead of being converted by the `Doctrine\DBAL\Types` system.
In order to reproduce this, you must have identifiers implementing `#__toString()` (to allow the `UnitOfWork` to hash them) and other accessors that are used by the custom DBAL type during DB/PHP conversions. If `#__toString()` and the DBAL type conversions are asymmetric, then the paginator will fail to find records.
Tricky situation, but this very much affects `ramsey/uuid-doctrine` and anyone relying on the `uuid_binary`.
For some bizarre reason the underlying cache drivers are returning
unexpected values, which are leaking to the cache objects and causing
them to error.
This makes our cache regions much more strict about the types that are
fetched from the cache provider, ensuring that no invalid information is
ever sent to the hydrators.
The following mistakes occur occasionally:
```
Call to a member function resolveAssociationEntries() on boolean {"detail":"[object] (Symfony\\Component\\Debug\\Exception\\FatalThrowableError(code: 0): Call to a member function resolveAssociationEntries() on boolean at /www/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultQueryCache.php:140)"}
```
On cache miss the parameter `$entityEntry` sometimes will be false. This fixes issue #7266.
called when not specifying the type of a DQL parameter being bound via
`Doctrine\ORM\Query#setParameter()`:
```php
$query->setParameter('foo', $theValue, $theType);
```
A full parameter bind is required in order to gain back performance:
```php
$query->setParameter('foo', $theValue, $theType);
```
This is up for discussion with patch reviewers.
As previously reported by @flaushi in https://github.com/doctrine/doctrine2/pull/7471#discussion_r241949045, we discovered
that binding a parameter causes a `ClassMetadataFactory#getClassMetadata()` call, which in turn leads to large performance
regression when using any `object` type as parameter.
Following two snippets lead to an internal `ClassMetadataFactory#getClassMetadata()` call, which in turn leads to an
exception being thrown and garbage collected, plus multiple associated performance implications:
```php
$query->setParameter('foo', new DateTime());
$query->getResult();
```
```php
$query->setParameter('foo', new DateTime(), DateTimeType::NAME);
$query->getResult();
```
This is due to following portion of code:
https://github.com/doctrine/doctrine2/blob/434820973cadf2da2d66e7184be370084cc32ca8/lib/Doctrine/ORM/Query.php#L406-L409
Notice how `$value = $this->processParameterValue($value);` happens before attempting to infer the type for the parameter value.
That call leads to this segment being reached, which leads to the regression:
https://github.com/doctrine/doctrine2/blob/434820973cadf2da2d66e7184be370084cc32ca8/lib/Doctrine/ORM/AbstractQuery.php#L423-L433
Assuming the bound parameter type is provided, we can completely skip attempting to introspect the given object:
```php
$query->setParameter('foo', new DateTime(), DateTimeType::NAME);
$query->getResult();
```
Processing the parameter value is not needed in this case, so we can safely skip that logic for all known parameters.
In order to not introduce a BC break or change the `AbstractQuery#processParameterValue()` implementation, we could filter
out all parameters for which the type is given upfront, and later on merge them back in instead.
The test expectation to be set is for `UnitOfWork#getSingleIdentifierValue()` to never be called.
When calling `Query#getOneOrNullResult()` and there are more than one
objects in the result an `NonUniqueResultException` is thrown.
This information was missing in the documentation about the query result
formats.
This commit addresses #6793.
According to mapping drivers the discriminator values can always be
converted to strings so it's safe to assume that we can actually do a
strict comparison during hydration.
Hydrator was ignoring data from subclasses when using multiple
inheritance levels. With this patch it will now use the discriminator
values from all subclasses of the class being hydrated.
The `v2.5.x` series of the ORM allowed to have DELETE DQLs without using
an alias, even though it didn't follow the grammar rules of the parser.
We fixed that issue on `v2.6.0` however that was a BC-breaking change
and lots of people were relying on this faulty behaviour.
This workaround fixes the BC-break, without even trying to be elegant.
In `v2.7.0.` we should raise a deprecation notice to notify people that
we'll drop that "feature" in `v3.0`.
Now using PHP to calculate the expected date manipulation, keeping a day
as delta since PHP resets the hour when performing operations with
days/weeks/months/years.
February is a wonderful month, isn't it?
When memcached extension is loaded Doctrine\ORM\Tools\Setup.php calls to setMemcache method. The MemcachedCache class has the setMemcached method instead. Changed this call in Setup to setMemcached and $memcache to $memcached to keep the name like the extension
SchemaTool has custom logic for creating the primary key of a
joined-table inherited entity. This logic overlooked association maps
as a possible source for identity columns, resulting in a fatal error
when fetching the primary key list for child entities.
Removed any custom logic for generating primary keys for root entities
in joined-table inheritance, deferring to the common logic used for
other entities.
Also adjusted the child entity logic, scanning association maps for
identity columns, and including the column as appropriate. It also
ensures that the primary key columns are in the correct order.
This will make it easier to contribute. It will also make reviews safer
since you will be able to tell if a PR is reponsible for a broken build
or not. Catching errors early is still possible by having a look at
allowed failures.
This will provide the same look for all of the relations: one-to-many, many-to-one, one-to-one, many-to-many.
It helps during auto-completion of XML code when creating XML schema for an entity.
The order is as follows: field, target-entity, mapped-by, inversed-by, indexed-by, fetch, orphan-removal.
Right now, when generating entities, a column of type 'guid' will generate the following PHPDoc:
```
/**
* Get id
*
* @return guid
*/
public function getId() {...}
```
Since guid is not a valid PHP type, this throws a warning in PHPStorm, and I assume most IDEs.
Adding the type to the type alias list fixes the problem.
Since the UoW checks each item of a *-to-many association to ensure
it has the correct type, we should never say that we expect an instance
of `Doctrine\Common\Collections\Collection` or an `array`.
These constants are available since PHP 5.4 and since we're requiring
PHP 7.1 there's no reason to require them.
I've also simplified the `array_map()` call since it's useless to
define a closure that simply calls a function.
As per discussion in #6639, conflict with versions below 3.4
of the YAML component is superfluous, previous versions
would work just fine even with bb994b9e70.
#6217#6284 when hydrating an entity with a composite primary key that is both an `EAGER` and a `LAZY` association and also cached, the `DefaultQueryCache` tries to pass L2 cache implementation detail objects to the `UnitOfWork`
We are not hydrating some of the cached association data into entities due to keys missing in the cache association definition.
Since this is an extreme edge case that is just a mismatch between db and cache, a detailed explanation was provided in the fix snippet as well
Doctrine is affected by PHP bug #62577. simplexml_load_file is not
able to load files if libxml_disable_entity_loader(true) has been
called. simplexml_load_file fails with the message:
I/O warning : failed to load external entity "/my/mappings/my_entity.dcm.xml"
in /path-to/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php on line 711
This error occurs even if there are no external entities in the XML file.
Waiting for the PHP bug to be resolved is infeasible, because it is
unresolved since years. Therefore Doctrine needs to circumvent the bug
by replacing simplexml_load_file with simplexml_load_string while getting
the file contents itself. simplexml_load_string is not affected by the
PHP bug.
The current implementation of the exporters are not taking the entity
listeners into account. I have added test cases for most of the edge
cases I could think of and implemented the Exporter handling.
This PR originates from https://github.com/doctrine/doctrine2/pull/5864,
I was overwhelmed by the amount of conflicts I had to resolve so I
started anew on a clean master HEAD.
Squashed commits:
- Code review aftermath
- Add even more return type declarations
- Added `return null` to methods declared with nullable return types
- Removed unneeded docblocks when types are self-explanatory
That test was removed from "performance" group but we forgot to move it
to the correct namespace.
It was also not doing any assertion, which is quite bad.
More info:
- https://github.com/doctrine/doctrine2/issues/3333
There was a bug in the "INSTANCE OF" operator as described in
https://groups.google.com/forum/#!topic/doctrine-user/B8raq8CNMgg
"INSTANCE OF" was not taking into account subclasses.
It was merely translating the class to its discriminator.
This is not correct since the class can have subtypes and those
are, indeed, still instance of the superclass.
Also, classes may not have a discriminator (e.g. abstract classes).
This commit also provides useful tests to avoid regression.
Which is needed to test void methods that shouldn't raise any exception
on a certain condition. If the interpreter gets to the point where the
assertion count is incremented it means that no exceptions have been
thrown and our test is successful.
Important to note that some tests were slighly refactored to simplify
things a bit.
Hello,
thank you very much for your work, it's awesome!
I found a minor error in PHPDOC for method setParameter. Argument "type" can be a string or null but can be also an int if it comes from \PDO::PARAM_*. So I've changed typehint "string" into "mixed".
- Globally change 'Shipping' to 'Shipment' - shipment is a noun and thus a clearer name for an object.
- Add definition of unidirectional to the top of the document.
- Explain inversedBy and mappedBy the first time they are used.
- Clarify some ownership and bidirectionality text.
- Minor English and punctuation fixes.
- Globally change 'Shipping' to 'Shipment' - shipment is a noun and thus a
clearer name for an object.
- Move definition of unidirectional from example to the top of the document.
- Explain inversedBy and mappedBy the first time they are used.
- Clarify some ownership and bidirectionality text.
- Minor English and punctuation fixes.
- Globally change 'Shipping' to 'Shipment' - shipment is a noun and thus a
clearer name for an object.
- Move definition of unidirectional from example to the to of the document.
- Explain inversedBy and mappedBy the first time they are used.
- Clarify some ownership and bidirectionality text.
- Minor English and punctuation fixes.
Fixing case when lazy loading of entity in one-to-one relationship on the side without foreign key
did not return eht entity because of custom type not being detected and properly formatted before
applying to database query.
Closes https://github.com/doctrine/doctrine2/issues/5887
Apply quoting strategy to foreign key columns of one-to-one relation so
that correct select statements are generated. And use unquoted column
names in result mapping instead of quoted ones, as consumers (namely
IdentifierFlattener) expect unquoted column names.
There was a bug in the "INSTANCE OF" operator as described in
https://groups.google.com/forum/#!topic/doctrine-user/B8raq8CNMgg
"INSTANCE OF" was not taking into account subclasses.
It was merely translating the class to its discriminator.
This is not correct since the class can have subtypes and those
are, indeed, still instance of the superclass.
Also, classes may not have a discriminator (e.g. abstract classes).
This commit also provides useful tests to avoid regression.
With the current PHPUnit version and PHP 7.2/master, the test fails with:
1) Doctrine\Tests\ORM\Functional\Ticket\DDC2660Test::testIssueWithExtraColumn
array_flip(): Can only flip STRING and INTEGER values!
2) Doctrine\Tests\ORM\Functional\Ticket\DDC2660Test::testIssueWithoutExtraColumn
array_flip(): Can only flip STRING and INTEGER values!
Due to the group being NULL vs "" on older PHP versions.
I will also file a bug report or fix to PHPUnit and/or PHP itself, but it sounds
like the missing group was just a typo.
As requested here https://github.com/doctrine/doctrine2/issues/6247#issuecomment-274123780 I thought it should be interesting to note that.
The note could be longer, like "You should avoid to have multiple methods for the same event (e.g. multiple @PrePersist) since the execution order is not guaranteed".
Following up on https://github.com/doctrine/doctrine2/issues/2943 I started to clarify how it's supposed to be done.
Please check if this would be necessary (at line 511):
`$myFirstComment->setUser($user);`
...and add it (in case).
Mock_ParserResult_*::getParameterMappings() was returning null, which
was then passed to count() on Query.php:308, causing a "Parameter must
be an array or an object that implements Countable" error.
It fail now. In `XmlDriver::_parseOptions` we need somehow to maintain
a list of options, that are supposed to be boolean, and then call
`$this->evaluateBoolean()` on them.
The strategy for composite primary keys must be "NONE" although the id generator class is \Doctrine\ORM\Id\AssignedGenerator. Calling the strategy "ASSIGNED" is misleading.
The strategy for composite primary keys must be "NONE" although the id generator class is \Doctrine\ORM\Id\AssignedGenerator. Calling the strategy "ASSIGNED" is misleading.
When merging an entity with a to-many association, it will store the
original entity data using the object hash of the to-be-merged entity
instead of the managed entity. Since this to-be-merged entity is not
managed by Doctrine, it can disappear from the memory. A new object
can reuse the same memory location and thus have the same object hash.
When one tries to persist this object as new, Doctrine will refuse it
because it thinks that the entity is managed+dirty.
This patch is a very naive fix: it just disables storing the original
entity data in case of to-many associations. It may not be the ideal
or even a good solution at all, but it solves the problem of object
hash reuse.
The test case relies on the immediate reusing of memory locations by
PHP. The variable $user has twice the same object hash, though referring
a different object. Tested on PHP 5.6.17
Without the fix, the test fails on the last line with:
A managed+dirty entity Doctrine\Tests\Models\CMS\CmsUser@[...] can not
be scheduled for insertion.
It's quite handy when creating CUSTOM id generators that should also
rely on AUTO_INCREMENT stuff (one can just extend the `ClassMedatadaFactory`
and create a different instance of `ClassMetadata` that overrides that
method).
DebugStack starts queries array from index 1 rather than 0 so the last query was never printed.
Also array params caused an 'Array to string conversion' error
Added pointers on how to interpret the method names. This helped me to quicker grasp the concept of the Relations and there methods. Added a @var annotation to the `Collection` interface to make it clear that the `$groups` is a child of the `Collection` interface.
Background:
Test relied on an `A->B->C` association:
* `A#id` being `B`
* `B#id` being `C`
* `C#id` being an auto-generated identifier (post-insert)
This cannot work, because it breaks the UnitOfWork's identity map.
Specifically, no entries for `A` and `B` can exist in the identity map until `C` entries
are persisted (post-insert).
That means that the identifier generator for `A` and `B` should not be an "assigned"
generator, but should instead be a post-insert generator waiting for other entities
to be persisted.
We cannot fix this in ORM 2.x, but we'll need to invent something for 3.x in order to
fix that (directed graph, or caching the order of operations in the metadata graph).
The `ClassMetadataInfo` was always using the "current class" to
fetch the reflection of a property even when a field is declared
on the parent class (which causes `ReflectionProperty` to throw
an exception).
See this example on why the revert is needed: https://3v4l.org/8T34v
Code copied for reference:
```php
<?php
$a = 1;
switch ($a) {
case "1";
echo "FUCK YOU, STUPID LANGUAGE!";
break;
case 1;
echo __LINE__;
break;
}
```
rebased commits:
- Added test case for bi-directional OneToOne in YamlExporter
- Only inspect joinColumns for owning side in bi-directional OneToOne in YamlExporter
- Adding bi-directional test case without joinColumn to XmlExporter test
- Same testcase also applied to PhpExporter
- Fixing bi-directional issue in PhpExporter when inspecting joinColumns index
- Implemented @Ocramius suggestions
Add missing semicolon after `private $year` on line 12 of `Car` class example code to fix `Parse error: syntax error, unexpected 'public' (T_PUBLIC), expecting ',' or ';' in [...]/VehicleCatalogue/Model/Car.php on line 14`
Doctrine\Tests\ORM\Mapping\AnnotationDriverTest::testLoadMetadataForNonEntityThrowsException
Argument 1 passed to Doctrine\Common\Annotations\AnnotationReader::__construct() must be an instance of Doctrine\Common\Annotations\DocParser, instance of Doctrine\Common\Cache\ArrayCache given, called in .../tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php on line 19 and defined
See: https://revive.beccati.com/bamboo/browse/PHP-DOCTR-PHP55-646/test/case/11813971
Note: Even though the column definition generated for a discriminator column
would eventually default the type to string and length to 255, the docs specify
defaults that should be reflected explicitly in the discriminatorColumn mapping.
Since we have target entity resolver in doctrine this class check is not enought.
To gain interface resolution it is better to add interface check in addition to class_check here.
In PHP 5.x + XDebug < 2.4, the output would be "string:..."
In PHP 7.x + XDebug >= 2.4, the output would be "the/file/name.php:11:string:..."
This is an improvement in XDebug that is quite annoying for our purposes, but is actually welcome to most users anyway.
This commit simply fixes that incompatibility
the methods UnitOfWork#afterTransactionRolledBack() and UnitOfWork#afterTransactionComplete do almost the same, so it can be abstracted into another private method.
There can be unique indexes automatically created for join column.
If join column is also primary key we should keep only primary key on this column.
Oracle does not allow having both unique index and primary key on the same column, it is useless for mysql too.
(Previously it was done by DBAL, but now it allows duplicate indexes)
The documentation for the method ‘addJoinedEntityFromClassMetadata’ is
wrong. As we can see currently says you need to pass an object and that
is wrong. The $relation variable is passed to ‘addJoinedEntityResult’
which is using it as a ‘string’.
The introduction of the second parameter in EntityRepository#createQueryBuilder generates a runtime notice if you have a sub-class of EntityRepository that has a second parameter in the createQueryBuilder method
When trying to setup MTI with doctrine, this was always the sentence where I stopped reading as it seems useless if you are forced to specify list of children when you don't even know them. Today I accidentaly read further and found out that this is not necessary.
Actually isEmpty() is always loading the collection in LazyCriteriaCollection.
A lazy version should use the existing functionality of count() to check if there are no elements if the collection is not initialized.
It seems that existing code was outdated and lacked ``use`` statement in order to work correctly.
We also added instructions on how to configure the new type, since it may not be straightforward for newcomers.
This use the brand new supported addon mariadb (not yet officially announced).
This is unfortunately a bit verbose, but I don't think there is any
alternative because we cannot install the addon when testing against mysql
otherwise it would overwrite mysql install.
expr()->countDistinct allows to create COUNT(DISTINCT) expression with mulltiple fields but parser requires only one field.
\Doctrine\ORM\Query\Parser::AggregateExpression
some body, please, fix this problem
The $metadata of the main entity is not always the metadata you need here, for example when you do join A with B and then B with C. For the second join it was using the metadata from A.
The `$lexer` variable is not used, the method `parser::getLexer()` is just a dumb getter and do nothing, so in my opinion, the call to `parser::getLexer()` is useless in this context.
Can you confirm?
Currently null can be passed and is set as default, however if you do this you get an exception. This allows null to be passed and set.
There is an if statement later on to see if $this->_hydrationCacheProfile is null so it seems logical you can set it to be null.
If I understand [2.5 release info](https://github.com/doctrine/doctrine2/releases/tag/v2.5.0) correctly, 2.4 will have last bugfix, then security fixes. I suppose then 2.3 and lower are in EOL, thus not needed to check badge for. Also they don't have any branch.
about: Have you encountered an issue during upgrade? 💣
---
<!--
Before reporting a BC break, please consult the upgrading document to make sure it's not an expected change: https://github.com/doctrine/orm/blob/2.9.x/UPGRADE.md
-->
### BC Break Report
<!-- Fill in the relevant information below to help triage your issue. -->
| Q | A
|------------ | ------
| BC Break | yes
| Version | x.y.z
#### Summary
<!-- Provide a summary describing the problem you are experiencing. -->
#### Previous behavior
<!-- What was the previous (working) behavior? -->
#### Current behavior
<!-- What is the current (broken) behavior? -->
#### How to reproduce
<!--
Provide steps to reproduce the BC break.
If possible, also add a code snippet with relevant configuration, entity mappings, DQL etc.
Adding a failing Unit or Functional Test would help us a lot - you can submit it in a Pull Request separately, referencing this bug report.
As you can see, we have a method "setBlockEntity" which ties a potential strategy to an object of type AbstractBlock. This type will simply define the basic behaviour of our blocks and could potentially look something like this:
..code-block::php
<?php
@@ -152,16 +152,16 @@ As you can see, we have a method "setBlockEntity" which ties a potential strateg
/**
* This var contains the classname of the strategy
* that is used for this blockitem. (This string (!) value will be persisted by Doctrine 2)
* that is used for this blockitem. (This string (!) value will be persisted by Doctrine ORM)
*
* This is a doctrine field, so make sure that you use an @column annotation or setup your
* yaml or xml files correctly
* This is a doctrine field, so make sure that you use a
#[Column] attribute or setup your yaml or xml files correctly
* @var string
*/
protected$strategyClassName;
/**
* This var contains an instance of $this->blockStrategy. Will not be persisted by Doctrine 2.
* This var contains an instance of $this->blockStrategy. Will not be persisted by Doctrine ORM.
*
* @var BlockStrategyInterface
*/
@@ -177,7 +177,7 @@ As you can see, we have a method "setBlockEntity" which ties a potential strateg
publicfunctiongetStrategyClassName(){
return$this->strategyClassName;
}
/**
* Returns the instantiated strategy
*
@@ -186,7 +186,7 @@ As you can see, we have a method "setBlockEntity" which ties a potential strateg
publicfunctiongetStrategyInstance(){
return$this->strategyInstance;
}
/**
* Sets the strategy this block / panel should work as. Make sure that you've used
* this method before persisting the block!
@@ -199,7 +199,7 @@ As you can see, we have a method "setBlockEntity" which ties a potential strateg
$strategy->setBlockEntity($this);
}
Now, the important point is that $strategyClassName is a Doctrine 2
Now, the important point is that $strategyClassName is a Doctrine ORM
field, i.e. Doctrine will persist this value. This is only the
class name of your strategy and not an instance!
@@ -213,28 +213,29 @@ This might look like this:
..code-block::php
<?php
use\Doctrine\ORM,
\Doctrine\Common;
useDoctrine\Common\EventSubscriber;
useDoctrine\ORM\Event\LifecycleEventArgs;
useDoctrine\ORM\Events;
/**
* The BlockStrategyEventListener will initialize a strategy after the
* MySQL example: full_name char(41) GENERATED ALWAYS AS (concat(firstname,' ',lastname)),
*/
protected$fullname;
.._annref_column_result:
@ColumnResult
@@ -207,7 +245,7 @@ Optional attributes:
~~~~~~~~~~~~~~~~~~~~~
The Change Tracking Policy annotation allows to specify how the
Doctrine 2 UnitOfWork should detect changes in properties of
Doctrine ORM UnitOfWork should detect changes in properties of
entities during flush. By default each entity is checked according
to a deferred implicit strategy, which means upon flush UnitOfWork
compares all the properties of an entity to a previously stored
@@ -231,16 +269,43 @@ Example:
*/
classUser{}
.._annref_customidgenerator:
@CustomIdGenerator
~~~~~~~~~~~~~~~~~~~~~
This annotations allows you to specify a user-provided class to generate identifiers. This annotation only works when both :ref:`@Id <annref_id>` and :ref:`@GeneratedValue(strategy="CUSTOM") <annref_generatedvalue>` are specified.
Required attributes:
-**class**: name of the class which should extend Doctrine\ORM\Id\AbstractIdGenerator
When we don't explicitly specify a column name via the ``name`` option, Doctrine
assumes the field name is also the column name. This means that:
assumes the field name is also the column name. So in this example:
* the ``id`` property will map to the column ``id`` using the type ``integer``;
* the ``text`` property will map to the column ``text`` with the default mapping type ``string``;
* the ``postedAt`` property will map to the ``posted_at`` column with the ``datetime`` type.
The Column annotation has some more attributes. Here is a complete
list:
Here is a complete list of ``Column``s attributes (all optional):
-``type``: (optional, defaults to 'string') The mapping type to
use for the column.
-``name``: (optional, defaults to field name) The name of the
column in the database.
-``length``: (optional, default 255) The length of the column in
the database. (Applies only if a string-valued column is used).
-``unique``: (optional, default FALSE) Whether the column is a
unique key.
-``nullable``: (optional, default FALSE) Whether the database
column is nullable.
-``precision``: (optional, default 0) The precision for a decimal
(exact numeric) column (applies only for decimal column),
- ``type`` (default: 'string'): The mapping type to use for the column.
-``name`` (default: name of property): The name of the column in the database.
-``length`` (default: 255): The length of the column in the database.
Applies only if a string-valued column is used.
-``unique`` (default: ``false``): Whether the column is a unique key.
-``nullable`` (default: ``false``): Whether the column is nullable.
-``insertable`` (default: ``true``): Whether the column should be inserted.
-``updatable`` (default: ``true``): Whether the column should be updated.
-``generated`` (default: ``null``): Whether the generated strategy should be ``'NEVER'``, ``'INSERT'`` and ``ALWAYS``.
-``enumType`` (requires PHP 8.1 and ``doctrine/orm`` 2.11): The PHP enum class name to convert the database value into. See :ref:`reference-enum-mapping`.
-``precision`` (default: 0): The precision for a decimal (exact numeric) column
(applies only for decimal column),
which is the maximum number of digits that are stored for the values.
-``scale``: (optional, default 0) The scale for a decimal (exact
-``scale`` (default: 0): The scale for a decimal (exact
numeric) column (applies only for decimal column), which represents
the number of digits to the right of the decimal point and must
not be greater than *precision*.
-``columnDefinition``: (optional) Allows to define a custom
not be greater than ``precision``.
-``columnDefinition``: Allows to define a custom
DDL snippet that is used to create the column. Warning: This normally
confuses the SchemaTool to always detect the column as changed.
-``options``: (optional) Key-value pairs of options that get passed
confuses the :doc:`SchemaTool <tools>` to always detect the column as changed.
-``options``: Key-value pairs of options that get passed
to the underlying database platform when generating DDL statements.
Specifying default values
~~~~~~~~~~~~~~~~~~~~~~~~~
While it is possible to specify default values for properties in your
PHP class, Doctrine also allows you to specify default values for
database columns using the ``default`` key in the ``options`` array of
Inside the ``Setup`` methods several assumptions are made:
..note::
If you want to use yml mapping you should add yaml dependency to your `composer.json`:
- If `$isDevMode` is true caching is done in memory with the ``ArrayCache``. Proxy objects are recreated on every request.
- If `$isDevMode` is false, check for Caches in the order APC, Xcache, Memcache (127.0.0.1:11211), Redis (127.0.0.1:6379) unless `$cache` is passed as fourth argument.
- If `$isDevMode` is false, set then proxy classes have to be explicitly created through the command line.
::
"symfony/yaml": "*"
Inside the ``ORMSetup`` methods several assumptions are made:
- If ``$isDevMode`` is true caching is done in memory with the ``ArrayAdapter``. Proxy objects are recreated on every request.
- If ``$isDevMode`` is false, check for Caches in the order APCu, Redis (127.0.0.1:6379), Memcache (127.0.0.1:11211) unless `$cache` is passed as fourth argument.
- If ``$isDevMode`` is false, set then proxy classes have to be explicitly created through the command line.
- If third argument `$proxyDir` is not set, use the systems temporary directory.
If you want to configure Doctrine in more detail, take a look at the :doc:`Advanced
DQL stands for Doctrine Query Language and is an Object
Query Language derivate that is very similar to the Hibernate
Query Language derivative that is very similar to the Hibernate
Query Language (HQL) or the Java Persistence Query Language (JPQL).
In essence, DQL provides powerful querying capabilities over your
@@ -34,9 +34,9 @@ object model.
DQL SELECT statements are a very powerful way of retrieving parts
of your domain model that are not accessible via associations.
Additionally they allow to retrieve entities and their associations
Additionally they allow you to retrieve entities and their associations
in one single SQL select statement which can make a huge difference
in performance in contrast to using several queries.
in performance compared to using several queries.
DQL UPDATE and DELETE statements offer a way to execute bulk
changes on the entities of your domain model. This is often
@@ -49,10 +49,6 @@ SELECT queries
DQL SELECT clause
~~~~~~~~~~~~~~~~~
The select clause of a DQL query specifies what appears in the
query result. The composition of all the expressions in the select
clause also influences the nature of the query result.
Here is an example that selects all users with an age > 20:
..code-block::php
@@ -83,14 +79,58 @@ Lets examine the query:
The result of this query would be a list of User objects where all
users are older than 20.
The SELECT clause allows to specify both class identification
variables that signal the hydration of a complete entity class or
just fields of the entity using the syntax ``u.name``. Combinations
of both are also allowed and it is possible to wrap both fields and
identification values into aggregation and DQL functions. Numerical
fields can be part of computations using mathematical operations.
See the sub-section on `Functions, Operators, Aggregates`_ for
more information.
Result format
~~~~~~~~~~~~~
The composition of the expressions in the SELECT clause also
influences the nature of the query result. There are three
cases:
**All objects**
..code-block::sql
SELECTu,p,nFROMUsersu...
In this case, the result will be an array of User objects because of
the FROM clause, with children ``p`` and ``n`` hydrated because of
their inclusion in the SELECT clause.
**All scalars**
..code-block::sql
SELECTu.name,u.addressFROMUsersu...
In this case, the result will be an array of arrays. In the example
above, each element of the result array would be an array of the
scalar name and address values.
You can select scalars from any entity in the query.
**Mixed**
..code-block::sql
SELECTu,p.quantityFROMUsersu...
Here, the result will again be an array of arrays, with each element
being an array made up of a User object and the scalar value
``p.quantity``.
Multiple FROM clauses are allowed, which would cause the result
array elements to cycle through the classes included in the
multiple FROM clauses.
..note::
You cannot select other entities unless you also select the
root of the selection (which is the first entity in FROM).
For example, ``SELECT p,n FROM Users u...`` would be wrong because
``u`` is not part of the SELECT
Doctrine throws an exception if you violate this constraint.
Joins
~~~~~
@@ -140,10 +180,10 @@ not need to lazy load the association with another query.
Doctrine allows you to walk all the associations between
all the objects in your domain model. Objects that were not already
loaded from the database are replaced with lazyload proxy
instances. Non-loaded Collections are also replaced by lazy-load
loaded from the database are replaced with lazy-loading proxy
instances. Non-loaded Collections are also replaced by lazy-loading
instances that fetch all the contained objects upon first access.
However relying on the lazy-load mechanism leads to many small
However relying on the lazy-loading mechanism leads to many small
queries executed against the database, which can significantly
affect the performance of your application. **Fetch Joins** are the
solution to hydrate most or all of the entities that you need in a
@@ -210,7 +250,7 @@ Retrieve the Username and Name of a CmsUser:
$users=$query->getResult();// array of CmsUser username and name values
echo$users[0]['username'];
Retrieve a ForumUser and his single associated entity:
Retrieve a ForumUser and its single associated entity:
..code-block::php
@@ -219,7 +259,7 @@ Retrieve a ForumUser and his single associated entity:
$users=$query->getResult();// array of ForumUser objects with the avatar association loaded
echoget_class($users[0]->getAvatar());
Retrieve a CmsUser and fetch join all the phonenumbers he has:
Retrieve a CmsUser and fetch join all the phonenumbers it has:
..code-block::php
@@ -279,11 +319,11 @@ With Nested Conditions in WHERE Clause:
<?php
$query=$em->createQuery('SELECT u FROM ForumUser u WHERE (u.username = :name OR u.username = :name2) AND u.id = :id');
$query->setParameters(array(
$query->setParameters([
'name'=>'Bob',
'name2'=>'Alice',
'id'=>321,
));
]);
$users=$query->getResult();// array of ForumUser objects
With COUNT DISTINCT:
@@ -319,7 +359,8 @@ article-ids:
$query=$em->createQuery('SELECT u.id, a.id as article_id FROM CmsUser u LEFT JOIN u.articles a');
$results=$query->getResult();// array of user ids and every article_id for each user
Restricting a JOIN clause by additional conditions:
Restricting a JOIN clause by additional conditions specified by
WITH:
..code-block::php
@@ -417,40 +458,71 @@ Get all users that have no phonenumber
Get all instances of a specific type, for use with inheritance
hierarchies:
..versionadded:: 2.1
..code-block::php
<?php
$query=$em->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF Doctrine\Tests\Models\Company\CompanyEmployee');
$query=$em->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u INSTANCE OF ?1');
$query=$em->createQuery('SELECT u FROM Doctrine\Tests\Models\Company\CompanyPerson u WHERE u NOT INSTANCE OF ?1');
Get all users visible on a given website that have chosen certain gender:
..versionadded:: 2.2
..code-block::php
<?php
$query=$em->createQuery('SELECT u FROM User u WHERE u.gender IN (SELECT IDENTITY(agl.gender) FROM Site s JOIN s.activeGenderList agl WHERE s.id = ?1)');
..versionadded:: 2.4
Starting with 2.4, the IDENTITY() DQL function also works for composite primary keys:
The IDENTITY() DQL function also works for composite primary keys
..code-block::php
<?php
$query=$em->createQuery("SELECT IDENTITY(c.location, 'latitude') AS latitude, IDENTITY(c.location, 'longitude') AS longitude FROM Checkpoint c WHERE c.user = ?1");
Joins between entities without associations were not possible until version
2.4, where you can generate an arbitrary join with the following syntax:
Joins between entities without associations are available,
where you can generate an arbitrary join with the following syntax:
..code-block::php
<?php
$query=$em->createQuery('SELECT u FROM User u JOIN Blacklist b WITH u.email = b.email');
$query=$em->createQuery('SELECT u FROM User u JOIN Banlist b WITH u.email = b.email');
With an arbitrary join the result differs from the joins using a mapped property.
The result of an arbitrary join is an one dimensional array with a mix of the entity from the ``SELECT``
and the joined entity fitting to the filtering of the query. In case of the example with ``User``
and ``Banlist``, it can look like this:
- User
- Banlist
- Banlist
- User
- Banlist
- User
- Banlist
- Banlist
- Banlist
In this form of join, the ``Banlist`` entities found by the filtering in the ``WITH`` part are not fetched by an accessor
method on ``User``, but are already part of the result. In case the accessor method for Banlists is invoked on a User instance,
it loads all the related ``Banlist`` objects corresponding to this ``User``. This change of behaviour needs to be considered
when the DQL is switched to an arbitrary join.
..note::
The differences between WHERE, WITH and HAVING clauses may be
confusing.
- WHERE is applied to the results of an entire query
- WITH is applied to a join as an additional condition. For
arbitrary joins (SELECT f, b FROM Foo f, Bar b WITH f.id = b.id)
the WITH is required, even if it is 1 = 1
- HAVING is applied to the results of a query after
aggregation (GROUP BY)
Partial Object Syntax
^^^^^^^^^^^^^^^^^^^^^
@@ -481,14 +553,12 @@ You use the partial syntax when joining as well:
"NEW" Operator Syntax
^^^^^^^^^^^^^^^^^^^^^
..versionadded:: 2.4
Using the ``NEW`` operator you can construct Data Transfer Objects (DTOs) directly from DQL queries.
- When using ``SELECT NEW`` you don't need to specify a mapped entity.
- You can specify any PHP class, it's only require that the constructor of this class matches the ``NEW`` statement.
- You can specify any PHP class, it only requires that the constructor of this class matches the ``NEW`` statement.
- This approach involves determining exactly which columns you really need,
and instantiating data-transfer object that containing a constructor with those arguments.
and instantiating a data-transfer object that contains a constructor with those arguments.
If you want to select data-transfer objects you should create a class:
@@ -558,6 +628,13 @@ then phonenumber-id:
...
'nameUpper'=>string'JWAGE'(length=5)
You can also index by a to-one association, which will use the id of
the associated entity (the join column) as the key in the result set:
Doctrine 2.2 features a filter system that allows the developer to add SQL to
Doctrine ORM features a filter system that allows the developer to add SQL to
the conditional clauses of queries, regardless the place where the SQL is
generated (e.g. from a DQL query, or by loading associated entities).
@@ -30,21 +28,22 @@ table alias of the SQL table of the entity.
In the case of joined or single table inheritance, you always get passed the ClassMetadata of the
inheritance root. This is necessary to avoid edge cases that would break the SQL when applying the filters.
Parameters for the query should be set on the filter object by
``SQLFilter#setParameter()``. Only parameters set via this function can be used
in filters. The ``SQLFilter#getParameter()`` function takes care of the
proper quoting of parameters.
For the filter to correctly function, the following rules must be followed. Failure to do so will lead to unexpected results from the query cache.
1. Parameters for the query should be set on the filter object by ``SQLFilter#setParameter()`` before the filter is used by the ORM ( i.e. do not set parameters inside ``SQLFilter#addFilterConstraint()`` function ).
2. The filter must be deterministic. Don't change the values based on external inputs.
The ``SQLFilter#getParameter()`` function takes care of the proper quoting of parameters.
We try to make using Doctrine2 a very pleasant experience.
We try to make using Doctrine ORM a very pleasant experience.
Therefore we think it is very important to be honest about the
current limitations to our users. Much like every other piece of
software Doctrine2 is not perfect and far from feature complete.
software the ORM is not perfect and far from feature complete.
This section should give you an overview of current limitations of
Doctrine 2 as well as critical known issues that you should know
Doctrine ORM as well as critical known issues that you should know
about.
Current Limitations
@@ -63,7 +63,7 @@ Where the ``attribute_name`` column contains the key and
``$attributes``.
The feature request for persistence of primitive value arrays
`is described in the DDC-298 ticket <http://www.doctrine-project.org/jira/browse/DDC-298>`_.
`is described in the DDC-298 ticket <https://github.com/doctrine/orm/issues/3743>`_.
Cascade Merge with Bi-directional Associations
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -71,8 +71,8 @@ Cascade Merge with Bi-directional Associations
There are two bugs now that concern the use of cascade merge in combination with bi-directional associations.
Make sure to study the behavior of cascade merge if you are using it:
-`DDC-875 <http://www.doctrine-project.org/jira/browse/DDC-875>`_ Merge can sometimes add the same entity twice into a collection
-`DDC-763 <http://www.doctrine-project.org/jira/browse/DDC-763>`_ Cascade merge on associated entities can insert too many rows through "Persistence by Reachability"
-`DDC-875 <https://github.com/doctrine/orm/issues/5398>`_ Merge can sometimes add the same entity twice into a collection
-`DDC-763 <https://github.com/doctrine/orm/issues/5277>`_ Cascade merge on associated entities can insert too many rows through "Persistence by Reachability"
Custom Persisters
~~~~~~~~~~~~~~~~~
@@ -83,10 +83,8 @@ Currently there is no way to overwrite the persister implementation
for a given entity, however there are several use-cases that can
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Alexander M. Turek