tobyzerner / json-api-server

A JSON:API server implementation in PHP.
https://tobyzerner.github.io/json-api-server/
MIT License
63 stars 21 forks source link

RuntimeException `<Custom collection class> must implement Tobyz\\JsonApiServer\\Resource\\Findable` #93

Closed bertramakers closed 7 months ago

bertramakers commented 7 months ago

Hi @tobyzerner .

I ran into an issue today that apparently this syntax doesn't work anymore for defining polymorphic relationships:

ToOne::make('example')
  ->type([
    Model1::class => 'model1',
    Model2::class => 'model2',
  ])

When I define it like that and create a new resource with this relationship, the related resource is always of the type model1 even though I explicitly put model2 in the request.

I read here that polymorphic relationships should now be defined using a custom CollectionInterface implementation: https://tobyzerner.github.io/json-api-server/relationships.html#polymorphic-relationships

However when I follow the documentation to define the collection and I send a request to create a new resource with the polymorphic relationship, I get the following error:

RuntimeException "<Custom collection class> must implement Tobyz\\JsonApiServer\\Resource\\Findable"

Note that I have registered the collection, and that I have defined it on the ToOne relationship using ->collection() instead of ->type().

This is quite an urgent issue for me because I had already updated and merged all the code that was broken with the recent updates, but I overlooked this one and now I can't seem to fix it because I'm not sure how to implement Findable on my collection. It just gets passed $id and $context, but no info on the type that is supposed to be loaded. And I cannot deduce that from the $id alone because our app uses incremental ids for every resource type, so they're not unique sadly.

Additionally, I'm not sure why the collection needs to implement Findable? All of the types I return in resources() on my collection also are registered resources, so their respective resource classes can be used to find the model?

bertramakers commented 7 months ago

Also a small error in the documentation, the code example uses implements Collection while it should be implements CollectionInterface

tobyzerner commented 7 months ago

Hi @bertramakers, will look into this very soon!

bertramakers commented 7 months ago

Thanks @tobyzerner! 🙏

bertramakers commented 7 months ago

Hi @tobyzerner , I was able to work around the issue for now by defining the polymorphic relationship like this:

ToOne::make('example')
  ->type(['model1', 'model2'])

And then following your advice from solution 2 in this comment: https://github.com/tobyzerner/json-api-server/issues/72#issuecomment-1832901218, to override the resource() method on my Resource classes to return null if the given $model is not of the right type.

I suspect that is why my original relationship wasn't working anymore. I thought it actually saved the wrong resource type to the DB, but it was actually broken when returning the data from the DB because the new code doesn't look at the array keys anymore and requested a type from the first resource type it encountered in the list, and the base Resource::resource() implementation always returns its type.

So this is now not urgent anymore for me, but I'll leave this open since the "official" approach to polymorphic relationships seems to be broken or not documented completely.

tobyzerner commented 7 months ago

Thanks for all the information @bertramakers, and sorry for the trouble. This should be fixed now - the resource specified in the linkage data will be used to find the model, rather than the relationship's collection. Let me know how you go - and if you have any ideas about how to document this better.

I've just released 1.0.0-beta.3 which includes the fix, as well as dropping the Interface suffix from interfaces (the docs were a little bit ahead of the release).

bertramakers commented 7 months ago

Thanks for the update @tobyzerner ! Just updated to beta 3, and the docs with breaking changes were super clear. 👍