[5.x] Missing meta
colq2 opened this issue · 8 comments
Hey @lindyhopchris, thanks for the v5! Referencing #632 here, we found an issuer during the Upgrade.
Unfortunately, we are missing the meta of resources. Already diving into the code, I discovered that in the neomerx package the methods were renamed from getPrimaryMeta
and getInclusionMeta
to getResourceMeta
.
We could determine this because the function getPrimaryMeta
and getInclusionMeta
are not called.
Diving into the code, we could make it work by editing CloudCreativity\LaravelJsonApi\Schema
public function getResourceMeta($resource)
{
if(method_exists($this->provider, 'getPrimaryMeta')){
return $this->provider->getPrimaryMeta($resource);
}
if(method_exists($this->provider, 'getInclusionMeta')){
return $this->provider->getInclusionMeta($resource);
}
return null;
}
I would like to create a PR, but I don't know how to solve this properly.
One open question: Should we callgetPrimaryMeta
and getInclusionMeta
?
Second: How to test this?
The SchemaInterface
also defines getIdentifierMeta
. If I got it right the getIdentifierMeta
is for the meta which is on the same level as type
and id
but not in a resource in the included data. But getResourceMeta
is the opposite.
On the other side, getPrimaryMeta
was present in the primary requested resource and getInclusionMeta
was present in the included data. Am I right here?
Do you maybe know what could cause this issue?
Hi! Thanks for raising this. I'll have a look at this as a priority next time I'm working on Open Source things. Unfortunately this week I'm quite busy, so it's likely to be at the weekend.
No worries. Thank you for maintaining this!
Hey @lindyhopchris have you by any chance already had the time to look into this? Please let me know if I can help in any way 👋
Sorry, no I haven't had time to look at this.
I'm not sure off the top of my head how to solve this one, because for backwards-compatibility we need to distinguish between when the resource is being serialized in the data
member or in the included
member. I need to dig into the code to find out how we determine that, plus whether it's possible to determine that within the new getResourceMeta
.
Sorry, this one isn't straight forward which is why I need to find a chunk of time to dig into it.
I can totally understand that! Just let me know if I can do anything else.
@colq2 so I've looked into this.
There is no way when generating meta for a resource to determine whether the resource is in the data
or included
member of the JSON:API document. This means there is no backwards compatible way to support that distinction between meta on a resource.
Neomerx must have dropped support for it. Which I actually agree with - it never made any sense to me as to why the meta should vary dependning on that distinction, as meta about the resource should not be dependent on location within the document.
So there's no way for me to support how you've previously been doing it.
Seeing as there's no distinction, I'd suggest detecting the presence of a single method, getResourceMeta(object $resource)
on the schema provided that Schema
delegates to. I can't see any other way forward, but thought I'd check with you in case you had any thoughts?
I consider the lack of support for resource meta a bug, so this definitely does need fixing.
@lindyhopchris thank you for looking into this!
I also agree in dropping it. Looked into our resources and we almost used everywhere.
public function getInclusionMeta($resource)
{
return $this->getPrimaryMeta($resource);
}
I don't fully understand your suggestion. Did you see my suggestion in the first comment?
Is there anything against checking if the old methods?
Maybe it is a good idea to fully drop the old methods and put them into the upgrade guide.