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

[resource collection] Interaction with PUT/DELETE/OPTIONS methods is likely wrong #744

Open
jameshadfield opened this issue Nov 1, 2023 · 2 comments

Comments

@jameshadfield
Copy link
Member

In #719 @tsibley states that "Interaction with PUT/DELETE/OPTIONS methods is likely wrong. Not an immediate problem because while we have endpoints for those methods on core/staging, we don't use them. But as-written the interaction/behaviour if they did run is incorrect, and it will be an issue for Groups or if we start using those methods for core/staging (which I think we should)."

@tsibley could you say more here? Are there tests you can write which would highlight the problem for me? I'd be happy to fix these issues.

@tsibley
Copy link
Member

tsibley commented Nov 2, 2023

The issue is that we haven't considered what it means to PUT and DELETE past versions (and how that affects OPTIONS). We can't support replacing past versions with PUT, so that would fail when tried, and while maybe we'd want to support deleting past versions with DELETE, we'd need to think thru the implications.

I could see a couple potential ways to address this.

One way to potentially address this is to extend the handlers (e.g. putDataset and deleteDataset) to throw a 405 Method Not Allowed if the resource in question is not the latest version. And something similar for optionsDataset, though that currently is solely based on authorization.

Alternatively, we could avoid registering handlers for those methods in the first place by using separate routes for versioned vs. unversioned paths. This might make more sense if part of a broader change to how versions are extracted by the handlers (something I've sketched out some unfinished thoughts on for myself).

@tsibley
Copy link
Member

tsibley commented Nov 2, 2023

Ah, and this is also a place where having a ResourceVersion class in the sources data model might be more helpful (compared to a version-aware Resource): to let us more easily express policies like "only owners can delete past versions". (Assuming we'd want to support deletion.)

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