EntityManager::getReference() should handle a PK which is also a FK

[le-yak]

EntityManager::getReference() should be able to handle a PK which is also a FK, just the way ::find() already does.

class Entity
{
  #[Id]
  #[ManyToOne(targetEntity: Related::class)]
  public Related $related
}

Current behaviour:

// case 1: by PK value
$entityManager->getReference(Entity::class, 1);
// Cannot assign int to property Entity::$related of type Related

// case 2: by reference
$related = $entityManager->getReference(Related::class, 1]);
$entityManager->getReference(Entity::class, $related);
// Object of class Related could not be converted to string

With this PR, the proxy factory would hydrate the proxy with a reference to the related entity if a relation is detected (addressing case 1 above)
Additionally EntityManager::getReference() would use the same approach as in find() and attempt to extract the underlying PK value when passed an entity (case 2).

In this first attempt, I just copy/pasted some code from find() to getReference(). These two have a lot in common, if this proposed change is of any interest I will improve it and deduplicate this code in a second version.

P.S. I believe this (somewhat related) issue #5640 may be closed.

[greg0ire]

It seems there are CI jobs failing. Please take a look at this guide for more on how to handle those.

[greg0ire]

if this proposed change is of any interest I will improve it and deduplicate this code in a second version.

I see no reason to reject your change, so I'd say please go ahead and deduplicate this.

[greg0ire]

If this also affects 2.x, please retarget.

[SenseException]

Does this need to be a draft? It looks like it's reviewable to me as a regular PR. Or is this still a WIP?

[le-yak]

Yes, I am still working on it. I duplicated some code for the proof of concept, this needs refactoring. In the meantime, I discovered that error cases also need to be addressed (tests marked as skipped with "work in progress").

Additionally, I discovered that updating the identifier of a proxy fails silently. This is pre-existing behaviour, but the change I propose introduces a whole class of scenarios (tests skipped with "use case needs to be confirmed") that I think should be addressed. I wanted to open a discussion about it, please let me know if I should open another issue instead.

Notice that all skipped tests pass if one replaces getReference() with find().

For information, I probably won't be able to work on it much before next week-end.

[le-yak]

Apologies for the delay, I've been quite busy lately. I should be able to submit a final proposal by the end of the weekend.

[le-yak]

I'm afraid I am unable to address either case:

1. attempting to access properties of a "broken" proxy (i.e. with an invalid, non-existing identifier) results in an obscure runtime error (Undefined array key "primary key column name") whereas a EntityNotFoundException would be in order. This only affects entities with relations in the identifier (i.e. those for which a proxy could not be created previously).
2. updating the identifier of a proxy fails silently. This already affected regular identifiers, and it now also affects identifier with relations.

I am not sure as how to move forward, please advise:

• is the incomplete fix acceptable as is? If so, should I just delete the skipped test cases?
• perhaps someone more capable should pick up from here?

[le-yak]

I figured out that Proxies behave slightly differently when "Lazy Ghost Objects" are not enabled. Through experimentation I reached the conclusion that only PHP < 8.1 appears to be displaying the legacy behaviour. This deprecation notice seems to confirm my hypothesis.

[le-yak]

I now realize I quite grossly underestimated the beast. I will not be able, nor willing, to dig deeper into this.