Problem/Motivation
In \Drupal\jsonapi\Normalizer\RelationshipItemNormalizer::getUuid
The logic in this method is obviously backwards:
public function getUuid($data) {
if (isset($data['uuid'])) {
return NULL;
}
$uuid = $data['uuid'];
// The value may be a nested array like $uuid[0]['value'].
if (is_array($uuid) && isset($uuid[0]['value'])) {
$uuid = $uuid[0]['value'];
}
return $uuid;
}
If the uuid is set the function returns NULL
Proposed resolution
The isset() call should be changed to empty() or it should be changed to !isset()
Remaining tasks
patch and test
User interface changes
n/a
API changes
n/a
Data model changes
n/a
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2925377-11.patch | 1.23 KB | wim leers |
Comments
Comment #2
ioana apetri commentedComment #3
pwolanin commentedstill needs tests
Comment #4
e0ipsoAfter looking at it, I don't think this code is being used anywhere. How did you stumble upon this bug? Was it executed in your use case?
I don't think we can remove this code anyways because of BC reasons, but if it's not used we should fix it with your patch and mark it
@deprecated.Comment #5
wim leersIt definitely looks like dead code.
Comment #6
e0ipsoYeah, this is probably coming from the HAL normalizer's initial copy.
Comment #7
wim leersYep, you're right:
\Drupal\hal\Normalizer\EntityReferenceItemNormalizer::getUuid():Interestingly, that code was last modified on Feb 3, 2015. But JSON API's code is from after that time. Since it's different, it looks like JSON API did use it at some point.
So definitely dead code today.
Given that it's indeed obviously broken, shouldn't we just delete it? How does fixing unused, broken code make sense? If we keep BC, we wouldn't be able to change it, but which code could use it since it's broken? Interesting dilemma 😀
Comment #8
wim leersPatch for #7.
Comment #10
wim leersAha, apparently there is a thing called
UuidReferenceInterface… Never heard of that before! :OCreated #2930243: Remove unused "entity resolver" functionality from Serialization to fix that weirdness.
Comment #11
wim leersComment #12
wim leersIgnore #11's interdiff, it's the wrong one. The patch is correct though.
Fortunately, he difference between #8 and #11 is so tiny, that it's easy to spot/review even without an interdiff :)
Comment #14
e0ipsoNice little improvement. People will cease to be puzzled by this :-)