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

Fix seedlist to not return duplicate json keys #5025

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

nickva
Copy link
Contributor

@nickva nickva commented Apr 2, 2024

Previously we used lists:ukeymerge which only works if the entries in the list are kept in a sorted order but they weren't. Since we now have maps, use those instead of proplists which would avoid the sorting issue.

While at it, make sure to test the _up and _uuids "misc" handler endpoints and modernize the seedlist test suite as well.

Fix: #5009

Copy link
Contributor

@jaydoane jaydoane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great improvement!

My only quibble is that some of the updated code is apparently not covered by tests, as shown in the following screen shots.
image
image

@nickva
Copy link
Contributor Author

nickva commented Apr 3, 2024

I'll add a few tests for the success path. Would have to mock the internal replicator we don't have a multi-node test framework.

Previously, we used `lists:ukeymerge` which only works if the entries in the
list are kept in a sorted order, but they weren't. Since we now have maps
available in Erlang, use those instead of proplists which would avoid the
sorting issue.

While at it, make sure to test the `_up` and `_uuids` "misc" handler endpoints
and modernize the seedlist test suite as well.
@nickva
Copy link
Contributor Author

nickva commented Apr 3, 2024

Coverage looks a bit better:

Screenshot 2024-04-03 at 7 05 18 PM

@nickva nickva merged commit 8bbf776 into main Apr 4, 2024
14 checks passed
@nickva nickva deleted the fix-seedlist-up-endpoint branch April 4, 2024 02:34
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 this pull request may close these issues.

Response of /_up is Not valid Json on one Node
3 participants