Skip to content

Conversation

@kdambekalns
Copy link
Member

@kdambekalns kdambekalns commented Oct 21, 2025

Hopefully fixes #2602 and allows to use PHP attributes to configure Doctrine ORM, not only annotations like before.

Upgrade instructions

No change should be needed.

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@kdambekalns
Copy link
Member Author

This does not yet work – whether this "minimally invasive" approach is feasible is yet to be proven…

@kdambekalns kdambekalns force-pushed the bugfix/support-orm-attributes branch 2 times, most recently from b4e3c87 to 07c4739 Compare October 21, 2025 16:48
@kdambekalns
Copy link
Member Author

@mficzel Using the reflection service in our own reader implementation works.

If our assumptions are correct, new tests should pass, too.

@mficzel
Copy link
Member

mficzel commented Oct 22, 2025

@kdambekalns something is still fishy ...

Doctrine\ORM\Mapping\MappingException: No identifier/primary key specified for Entity "Neos\Flow\Tests\Functional\Persistence\FixturesPHP8\AnnotatedIdEntity" sub class of "Neos\Flow\Tests\Functional\Persistence\FixturesPHP8\AnnotatedIdEntity_Original". Every Entity must have an identifier/primary key.

/var/www/html/Packages/Libraries/doctrine/orm/src/Mapping/MappingException.php:44
/var/www/html/Packages/Libraries/doctrine/orm/src/Mapping/ClassMetadataInfo.php:1194
/var/www/html/Packages/Libraries/doctrine/orm/src/Mapping/ClassMetadataFactory.php:284
/var/www/html/Packages/Libraries/doctrine/orm/src/Mapping/ClassMetadataFactory.php:264
/var/www/html/Packages/Libraries/doctrine/persistence/src/Persistence/Mapping/AbstractClassMetadataFactory.php:343
/var/www/html/Packages/Libraries/doctrine/persistence/src/Persistence/Mapping/AbstractClassMetadataFactory.php:207
/var/www/html/Packages/Libraries/doctrine/persistence/src/Persistence/Mapping/AbstractClassMetadataFactory.php:96
/var/www/html/Data/Temporary/Testing/Cache/Code/Flow_Object_Classes/Neos_Flow_Persistence_Doctrine_PersistenceManager.php:317
/var/www/html/Packages/Framework/Neos.Flow/Tests/FunctionalTestCase.php:159

@mficzel
Copy link
Member

mficzel commented Oct 22, 2025

I wonder wether we also have to adjust \Neos\Flow\Persistence\Doctrine\Mapping\ClassMetadata->_initializeReflection

@kdambekalns
Copy link
Member Author

Making @Flow\Entity an attribute in "real code" within the Flow classes allows a clean compile run. And #[ORM\Id] and #[Flow\Identity] do work, too. So, is only testing affected?

@kdambekalns
Copy link
Member Author

Doh… I removed all the adjusted entities except AnnotatedIdEntity to limit the scope. Turns out this is broken:

#[Flow\Entity]
class AnnotatedIdEntity
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\SequenceGenerator(sequenceName: 'annotatedidentity_seq')]
    protected string $id;

But this does work:

#[Flow\Entity]
class AnnotatedIdEntity
{
    /** @var string */
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\SequenceGenerator(sequenceName: 'annotatedidentity_seq')]
    protected string $id;

The missing @var breaks it! On to digging deeper…

@mficzel
Copy link
Member

mficzel commented Oct 22, 2025

We should think about targeting Neos 8.4 + PHP 8.2 as i noticed that in Neos 8.3 with PHP 8.0 we:

  • Cannot declare nested attributes in PHP as those are a PHP 8.1 feature
  • Even with PHP 8.2 + Neos 8.3 Nested Attributes are destroyed by the proxy class builder. I hope this is better in 8.4

@mficzel
Copy link
Member

mficzel commented Oct 22, 2025

Checked Neos 8.4 and the proxy builder still destroys nested attributes. Created #3511 for that.

@kdambekalns
Copy link
Member Author

kdambekalns commented Oct 23, 2025

I am confused. Locally this PR passes on PHP 8.3 if I add this:
var-annotations.patch

How does this relate to broken attribute rendering?

  • In the sense it will never work on PHP 8.0 and 8.1, and the solution is to go for Flow 8.4, as that requires PHP 8.2? Fine with me!
  • But that will not automagically fix the "missing type issue", or is that something that has changed in Flow 8.4 already?

@mficzel
Copy link
Member

mficzel commented Oct 23, 2025

I tried rebasing this pr to 8.4 and together with the changes from #3512 i could successfully define a repository and entity via attributes.

However the properties in the entity were not detected unless i added the @var annotation. So this problem persists even in 8.4.

However it looks like it might really be the last real problem as the proxy classes were successfully created otherwise and flow did not complain about anything.

@kdambekalns
Copy link
Member Author

However it looks like it might really be the last real problem as the proxy classes were successfully created otherwise and flow did not complain about anything.

That's great news already!

@kdambekalns kdambekalns changed the base branch from 8.3 to 8.4 October 30, 2025 20:52
@github-actions github-actions bot added 8.4 and removed 8.3 labels Oct 30, 2025
kdambekalns and others added 6 commits October 30, 2025 22:08
# Conflicts:
#	Neos.Flow/Tests/Functional/Persistence/Aspect/PersistenceMagicAspectTest.php
The Doctrine reader exclusively used annotations, our reflection service
also takes attributes into account.

This should allow to use both for configuring ORM.
This is here to demonstrate that the tests can succeed and are a 1:1 copy of the classic fixtures.
mficzel and others added 3 commits October 30, 2025 22:17
- ORM annotations > ORM attributes
- Flow annotation > Flow attributes
- @param and @return annotations to parameter and return types

!!! Nested attributes are not supported in php 8.0 so they stay annotations for now !!!
Without this change, any class property is ignored, if it has no `@var`
tag. As soon as a property lacks that tag, the early return in method
`evaluateClassPropertyAnnotationsForSchema` kicks in.

When using type declarations, this is nonsense, though. This change
thus makes sure the type is fetched from the native type declaration
if no `@var` tag is found. If a `@var` tag exists, it takes precedence.
This allows to do things like this to declare typed collections:

```php
/**
 * @var Collection<AnnotatedIdentitiesEntity>
 */
#[ORM\ManyToMany(indexBy: 'author')]
protected Collection $annotatedIdentitiesEntities;
```
@kdambekalns kdambekalns force-pushed the bugfix/support-orm-attributes branch from 02747ea to 6b049a0 Compare October 30, 2025 22:07
The ``json_array`` type is removed in Doctrine DBAL 3.0.
This allows complex return types like `ArrayCollection|Collection` or
`static`.
@kdambekalns
Copy link
Member Author

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants