Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow doctrine/orm 3, doctrine/dbal 4 (WIP) #265

Closed
wants to merge 2 commits into from

Conversation

Chris53897
Copy link
Contributor

jackalope/jackalope-doctrine-dbal does not yet support dbal 4 (but they are working on it)

@Chris53897
Copy link
Contributor Author

Now an installable set is available.

There are multiple errors.
One is this, that change was intoduced in doctrine/orm 2.16.x (later removed) and reintoduced in 3.x.
If someone knows how to fix it, please let me know.

Doctrine\ORM\Exception\EntityIdentityCollisionException: While adding an entity of class Liip\Acme\Tests\App\Entity\User with an ID hash of "1" to the identity map,
  │ another object of class Liip\Acme\Tests\App\Entity\User was already present for the same ID. This exception
  │ is a safeguard against an internal inconsistency - IDs should uniquely map to
  │ entity object instances. This problem may occur if:
  │
  │ - you use application-provided IDs and reuse ID values;
  │ - database-provided IDs are reassigned after truncating the database without
  │ clearing the EntityManager;
  │ - you might have been using EntityManager#getReference() to create a reference
  │ for a nonexistent ID that was subsequently (by the RDBMS) assigned to another
  │ entity.
  │
  │ Otherwise, it might be an ORM-internal inconsistency, please report it.

@mbabker
Copy link
Contributor

mbabker commented Feb 19, 2024

One is this, that change was intoduced in doctrine/orm 2.16.x (later removed) and reintoduced in 3.x.
If someone knows how to fix it, please let me know.

Just looking at the test fixtures, I don't have a good solution here. But, all the setId() calls are probably what is causing the ORM's identity map exception. If the tests can be updated in a way that they don't rely on specific primary key values then that would be the best course of action.

@@ -29,15 +29,10 @@ doctrine:
connection: default
mappings:
LiipAcme:
type: php
type: attribute
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll need support for annotation mapping here as well, otherwise you'll need to skip tests on PHP 7.4 builds.

dir: "%kernel.project_dir%/Entity"
prefix: 'Liip\Acme\Tests\App\Entity'
is_bundle: false
LiipAcmeYml:
Copy link
Contributor

Choose a reason for hiding this comment

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

If the intent is to test with multiple mapping drivers, converting to XML instead of dropping the YAML mapping would be a good choice (you should be able to use the CLI tools when ORM 2.x is installed to help with this).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got the message, that the yamlDriver ist not available anymore.
Good point with the XMLDriver as replacement.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't remember why tests used annotations and YAML config, we can drop it and just assume that Doctrine read the configuration properly.

@Chris53897
Copy link
Contributor Author

@mbabker Thanks for the input.

My primary goal is to test my bundle via this bundle ;)

@@ -1,78 +1,80 @@
{
"name": "liip/test-fixtures-bundle",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please restore the original indentation or do this change in another PR.

@Chris53897
Copy link
Contributor Author

I will come back to this as soon, as i have some free time.

@Chris53897
Copy link
Contributor Author

I did split the PRs #269
and #268

After that is merged, maybe just simpify things and #272 and add support for doctrine/orm 3, doctrine/dbal 4 after that?

@alexislefebvre
Copy link
Collaborator

You can rebase it on 2.x.

Or apply these changes from the 3.x branch and target this branch when creating another PR, see:

@alexislefebvre
Copy link
Collaborator

alexislefebvre commented Mar 31, 2024

I think it has been partially fixed by this PR:

doctrine/dbal 4 is not supported yet.

@Chris53897
Copy link
Contributor Author

thanks @alexislefebvre
I did rebase against 2.x.
There are no useful changes in this PR anymore.
I will close this PR.

@Chris53897 Chris53897 closed this Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants