Skip to content
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

Relationship data generator is invoked more than once if data is empty #252

Open
lindyhopchris opened this issue Feb 3, 2021 · 16 comments

Comments

@lindyhopchris
Copy link
Collaborator

I've been getting this error:

Exception: Cannot traverse an already closed generator
src/Parser/RelationshipData/RelationshipDataIsCollection.php:132

It only occurs when relationship data is empty. The cause of the bug is these lines here:
https://github.com/neomerx/json-api/blob/master/src/Parser/RelationshipData/RelationshipDataIsCollection.php#L130-L131

As it does not assign an empty array to $this->parsedResources before iterating over the resource, the parsedResources property will remain null. Therefore when it is called a second time, there is no cache to use and the closed generator is used.

I'll submit a PR with a test reproducing the problem and a fix.

@lindyhopchris
Copy link
Collaborator Author

@neomerx see you haven't been active on Github since March last year. Hope everything is ok.

Hope you can tag the fix as 4.0.2. If you need help maintaining the package going forward, do let me know.

mahagr pushed a commit to mahagr/json-api that referenced this issue Feb 19, 2021
@mahagr
Copy link

mahagr commented Feb 19, 2021

Thanks! I also started my own fork as I need to add the missing pieces to the API, such as pagination and filtering.

@lindyhopchris
Copy link
Collaborator Author

@mahagr looks like I'm going to have to run a fork too, as I'm not sure that PR is going to get merged. My fork is here:
https://github.com/laravel-json-api/neomerx-json-api

@PabloKowalczyk
Copy link
Contributor

Hey @neomerx, do you need some help maintain this repo?

@mahagr
Copy link

mahagr commented Jan 31, 2022

I am wondering the same. Otherwise, it would make sense to have an official fork instead of everyone doing their own.

@gfemorris
Copy link

Hey it seems this project is not maintained anymore. Did you agree which fork should be the "official" one?

@PabloKowalczyk
Copy link
Contributor

I can create and maintain something like json-api-php/json-api.

@gfemorris
Copy link

@lindyhopchris created a fork and already has v5.0.0 in it maybe he wants to or you can adapt his changes?

@lindyhopchris
Copy link
Collaborator Author

Yeah I'm happy to maintain my fork because it's a critical part of Laravel JSON:API. I'm not up for doing any re-writing, more just keeping it up-to-date for PHP versions and with the spec if that does change (though it's taking them ages to get to 1.1 any way, so doubt we'll be talking about 1.2 any time soon).

@PabloKowalczyk
Copy link
Contributor

Making it (fork) framework/library agnostic should be better for long term maintenance.

@lindyhopchris
Copy link
Collaborator Author

Yeah to be clear, even though it's in the laravel-json-api organisation, it'll always be left as framework agnostic. That's because I wrap it in another package to tie it into the Laravel JSON:API implementation - i.e. there's a laravel-json-api/encoder-neomerx package that does that wrapping.

So my fork will always remain framework agnostic.

@PabloKowalczyk
Copy link
Contributor

But name can be confusing for newcomers :)

@lindyhopchris
Copy link
Collaborator Author

Yeah can see that. FWIW I'm planning on keeping my fork for the time being. If there comes a point where there's another fork that is fully supported and maintained, then I can switch to that in the future.

@mahagr
Copy link

mahagr commented Apr 4, 2022

Alright, I'm going to look into switching to laravel fork myself when I get back to the API.

@gfemorris
Copy link

Thank @lindyhopchris .. i will switch to your fork next time i'll do any upgrades.. thanks for your work.

@PabloKowalczyk
Copy link
Contributor

Hello guys, it's been a while but I started my fork of this library - https://github.com/jsonapiphp/jsonapi. It started with v1.0.0 which is the same code as v4.0.1 of this lib, so you can change them easily.
My fork will concentrate on performance and jsonapi specification compatible.

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

Successfully merging a pull request may close this issue.

4 participants