Fix identifiers behavior bugs

[soyuka]

Q	A
Bug fix?	yes
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#551
License	MIT
Doc PR	∅
#550 ref (sorry about multiple PR, github was buggy).

[soyuka]

I'm not fond of this, does anyone have a better idea in mind?

[teohhanhui]

Can you explain what's going on here?

What did you mean when you said:

This was working before #533

I don't see how that can be the case from the code before #533 -

core/src/Bridge/Symfony/Routing/IriConverter.php

Lines 72 to 90 in 037fece

	/**
	* {@inheritdoc}
	*/
	public function getIriFromItem($item, int $referenceType = UrlGeneratorInterface::ABS_PATH) : string
	{
	$resourceClass = $this->getObjectClass($item);
	$routeName = $this->getRouteName($resourceClass, false);
	$identifierValues = [];
	foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $propertyName) {
	$propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName);
	if ($propertyMetadata->isIdentifier()) {
	$identifierValues[] = $this->propertyAccessor->getValue($item, $propertyName);
	}
	}
	return $this->router->generate($routeName, ['id' => implode('-', $identifierValues)], $referenceType);
	}

We never checked for relations when getting identifier values.

[teohhanhui]

Anyway, for fixing this, why don't we just get the id from the entity if it's a relation? Like @Simperfit suggested...

[soyuka]

😕 I can't say why it was working before, anyway maybe it wasn't at least it's now tested.

That's exactly what I did here $identifiers[$propertyName] = $this->getIdentifiersFromItem($identifiers[$propertyName]);, or you had an easier solution in mind?

[teohhanhui]

Your current method allows for recursion of more than 1 level. I'm not sure how we're going to actually make that work... For the case of related entity used as an identifier, we can only allow non-composite identifier which is not a relation.

[soyuka]

I can try to remove recursion, I like recursion too much I think 😜.

For the case of related entity used as an identifier, we can only allow non-composite identifier which is not a relation.

Sorry I didn't understood. Isn't this enough? It lets the user choose if the identifier should be serialized or not, according to normalization groups.

---

Recursion removed, thx @teohhanhui it's better now.

[teohhanhui]

It lets the user choose if the identifier should be serialized or not, according to normalization groups.

It has nothing to do with serialization. We're talking about the URL here...

[soyuka]

Sorry, though you were talking about the second commit. I rewrote 3bf5323, should be better now.

[Simperfit]

👍

[soyuka]

ping @api-platform/core lgtm

[dunglas]

Prefer empty (for performance).

[soyuka]

I can squash my commits but I think they are best separated, let me know.

[dunglas]

Thanks @soyuka!