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

CachedResource: Uniform URI keys. #1180

Open
criske opened this issue Jun 10, 2021 · 5 comments
Open

CachedResource: Uniform URI keys. #1180

criske opened this issue Jun 10, 2021 · 5 comments

Comments

@criske
Copy link
Contributor

criske commented Jun 10, 2021

Having two URI's

/foo/bar?a=100&b=true
and
/foo/bar?b=true&a=100

Eventhough these URIs have query parameters in different order, they are pointing to the same Resource.
And this will lead to duplicate entries in JsonStorage - will be treated as different primary key.

Proposal: order URI query parameters by 'name' in alphabetical oder before storing CachedResource into JsonStorage.

Ex: /foo/bar?b=true&a=100 will be /foo/bar?a=100&b=true before storing.

@zoeself
Copy link
Collaborator

zoeself commented Jun 10, 2021

@criske thank you for reporting this. I'll assign someone to take care of it soon.

@criske
Copy link
Contributor Author

criske commented Jun 10, 2021

@amihaiemil now that I think about it, I think this should be done by JsonStorage implementation - both before storing and getting:

For example if we query for Json#getResource(/foo/bar?b=true&a=100), it should order internally and then query for resource..

Edit --
or in JsonResources, just to keep the logic in self-core. :D

@zoeself
Copy link
Collaborator

zoeself commented Jun 10, 2021

@amihaiemil I couldn't find any assignee for this task. This is either because there are no contributors with role DEV available or because the project does not have enough funds.

Please, make sure there is at least one available contributor with the required role and the project can afford to pay them.

@amihaiemil
Copy link
Member

@criske It's a valid issue, but as you said in Prod we will never face it. Github and GitLab URIs do not use query parameters at all (maybe just per_page). So I think it's virtually a useless fix: more complicated code for nothing : D

@zoeself
Copy link
Collaborator

zoeself commented Aug 27, 2022

@amihaiemil I couldn't find any assignee for this task. This is either because there are no contributors with role DEV available or because the project does not have enough funds.

Please, make sure there is at least one available contributor with the required role and the project can afford to pay them.

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

3 participants