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

API Post Versions Errors If No Results Are Found #448

Open
DonovanDMC opened this issue Nov 17, 2022 · 2 comments
Open

API Post Versions Errors If No Results Are Found #448

DonovanDMC opened this issue Nov 17, 2022 · 2 comments

Comments

@DonovanDMC
Copy link
Member

search[locked_tags] (a2934002-5e29-425c-8d99-f7eaaa40dfb6)
search[locked_tags_added] (4dca4a68-8d4f-4ee9-b612-e2743990f554)
search[locked_tags_removed] (10868778-a478-43bc-b11e-a6dd10b26e3d)
search[tags] (dc481d60-a763-4631-aa9a-5e488f730b9d)
search[tags_added] (c5cf2457-03f9-4720-9951-a8aee4ff6633)
search[tags_removed] (bec2ca09-575f-4acb-aec7-5f161799e512)

This extends a bit beyond the title, an error will be returned if the tag has not been used in the specified search area (tags/locked tags). It works fine if the tag has previously been present, but no longer is. A program I have to check tags is getting caught up on this due to implications happening and silently disappearing.

@Earlopain
Copy link
Collaborator

Earlopain commented Nov 27, 2022

Looked into this again and finally learned something new. The issue is with the active_model_serializers dependency.

It's used to customize the serialization on posts, but as a side-effect also messes with all other serialization happening. When there are no results and it can't determine the root from the class, it throws. Post changes seem to just return an empty array instead of a typed collection like when using ActiveModel, presumably because of elasticsearch. This is not the ony place this happens, the tag preview on the uploader has the same issue.

The gem is also responsible for the object response type when there are no results. {"tags": []} instead of the expected []. I looked into the configuration of this gem, but haven't found a way to fix this. The last time this came up in their issue tracker it was made to raise instead of emiting {"":[]}. But no solution to the underlying issue as far as I can tell.

The fix that comes to mind is to just remove the gem and use normal Rails functionality to reproduce the desired behaviour. This would be somewhat of an API breaking change, since empty responses will also return an array instead of the object. But I would classify that as a bug, and fixing that makes consuming the API more predictable and less error-prone. What do you think @zwagoth? I don't believe it would break existing consumers. The object response is the exception and it should just continue to work with the array codepath, which every application must already handle. I'm having trouble coming up with a way to parse it that breaks if this behavior is changed.

Btw, the posts endpoint goes around the issue by manually specifying the root key, which is why posts always come back wrapped an object instead of a plain array, like all other endpoints.

@Earlopain Earlopain changed the title Post Versions Errors If Tag Has Not Been On Post Post Versions Errors If No Results Are Found Nov 27, 2022
@Earlopain Earlopain changed the title Post Versions Errors If No Results Are Found API Post Versions Errors If No Results Are Found Nov 27, 2022
@Earlopain
Copy link
Collaborator

I will try to get rid of the dependency causing this issue. That would also close #359. We are fine with changing the response format in these cases. Posts will continue to always be returned as an object, changing that would be a pretty major breaking change.

The only reason for using this dependency was performance reasons from what Kira told me. The default Rails serialization needs to have performance on par with the gem, and I hope Rails improved in that regard. If the performance is still bad the gem will have to stay, and I'm still not sure on how to work around the issue without always returning an object.

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