Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bertramakers opened this issue Dec 8, 2023 · 6 comments

Comments

@bertramakers
Copy link
Contributor

bertramakers commented Dec 8, 2023

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 bertramakers changed the title RuntimeException "<Custom collection class> must implement Tobyz\\JsonApiServer\\Resource\\Findable" RuntimeException <Custom collection class> must implement Tobyz\\JsonApiServer\\Resource\\Findable Dec 8, 2023
@bertramakers
Copy link
Contributor Author

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

@tobyzerner
Copy link
Owner

Hi @bertramakers, will look into this very soon!

@bertramakers
Copy link
Contributor Author

Thanks @tobyzerner! 🙏

@bertramakers
Copy link
Contributor Author

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: #72 (comment), 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
Copy link
Owner

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants