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

Specify the semantics of DELETE #41

Closed
acoburn opened this issue Sep 19, 2019 · 13 comments
Closed

Specify the semantics of DELETE #41

acoburn opened this issue Sep 19, 2019 · 13 comments

Comments

@acoburn
Copy link
Member

acoburn commented Sep 19, 2019

The LDP specification makes DELETE optional. For a Solid server implementing DELETE, what, if any, constraints are on delete?

Specifically, can a client DELETE a non-empty container? If so, is that request recursive? Does the presence of contained resources affect the value of the Allow response header? If a DELETE operation is attempted on a non-empty container (and the request is rejected), is there a recommended ldp:constrainedBy relation that a server should use to inform a client why the request failed?

@dmitrizagidulin
Copy link
Member

dmitrizagidulin commented Sep 24, 2019

Propose we rename this issue to: "Specify the semantics of DELETE".

In addition to the questions you raised in the description, I would also add:

Related issues:

Other important questions:

@acoburn acoburn changed the title What constraints exist on the use of DELETE Specify the semantics of DELETE Sep 24, 2019
@dmitrizagidulin
Copy link
Member

Oh, uh.. one other question :) is DELETE supposed to return 200 OK or 204 No Content?

@RubenVerborgh
Copy link
Contributor

Both 200/204 are fine; I don’t think we need to specify.

@pmcb55
Copy link

pmcb55 commented Sep 25, 2019

@RubenVerborgh Sure, but would a non-normative note be useful to state that success return codes (i.e. generally, not just for DELETE) are the range 200-299 ('cos I think your typical client-side developer might only explicitly check for a 200)? In other words, would W3C specs normally state basic principals of HTTP like this, even non-normatively...?

@RubenVerborgh
Copy link
Contributor

Yes; but we should avoid repeating the HTTP spec too much.

@kjetilk
Copy link
Member

kjetilk commented Sep 25, 2019

It'd be an or in the test suite, though. Which is fine, but a bit more work :-)

@csarven
Copy link
Member

csarven commented Dec 4, 2019

Specifically, can a client DELETE a non-empty container?

Yes, if and only if they have Write access to the container and all contained resources.

If so, is that request recursive?

Yes, following above.

Does the presence of contained resources affect the value of the Allow response header?

Allow should be a reflection of the access modes in junction with available methods on the container resource at the time of the request. The Allow value may be affected by the permissions on contained resources eg. if server wants to check access modes on each contained resource recursively and can better guarantee the methods that'd be allowed. New resources may come to exist or disappear in the container, along with changes to authorization policies in the meantime, hence the fundamental requirement is on the container.

If a DELETE operation is attempted on a non-empty container (and the request is rejected), is there a recommended ldp:constrainedBy relation that a server should use to inform a client why the request failed?

Yes, pending resolution of #44

@csarven
Copy link
Member

csarven commented Dec 4, 2019

  • solid-spec/195 - Which permissions are required for deleting a resource?

Covered in https://github.com/solid/solid-spec/issues/195#issuecomment-559619352 , https://github.com/solid/solid-spec/issues/195#issuecomment-559799154 , https://github.com/solid/solid-spec/issues/195#issuecomment-559810431 , #41 (comment) .

Summary/Proposal: Both Write on the resource and the container is required, factoring in updates to containment triples as well as recursive deletion behaviour.

  • solid-spec/196 - Which permissions are required for deleting an ACL document?

Covered in https://github.com/solid/solid-spec/issues/196#issuecomment-560377845 .

Summary/Proposal: Write on resource or associated ACL is the only requirement to delete ACL.

  • solid-spec/173 - proposal: deleting a container is a noop
  • solid-spec/190 - Recursive deletion of members on deletion of container

Covered in https://github.com/solid/solid-spec/issues/195#issuecomment-559810431 , #41 (comment) .

  • solid-spec/157 - Deleting folders when invisible files are left

Covered in solid/solid-spec#157 (comment) .

Summary/Proposal: There is no spooninvisible files. Resources that are not part of containment and are not dependent on a resource's lifecycle are not applicable.

@kjetilk
Copy link
Member

kjetilk commented Dec 5, 2019

  • solid-spec/195 - Which permissions are required for deleting a resource?

Covered in solid/solid-spec#195 (comment) , solid/solid-spec#195 (comment) , solid/solid-spec#195 (comment) , #41 (comment) .

Summary/Proposal: Both Write on the resource and the container is required, factoring in updates to containment triples as well as recursive deletion behaviour.

👍 to that, but I would like to have a decision on #126 , so that we can found the decision on a more general principle.

  • solid-spec/196 - Which permissions are required for deleting an ACL document?

Covered in solid/solid-spec#196 (comment) .

Summary/Proposal: Write on resource or associated ACL is the only requirement to delete ACL.

This seems trickier... I would suggest that write on resource does not suffice, since if you just delete the ACL, it changes the permissions on the resource, since it would then use an inherited ACL.

Thus, I would suggest that deleting the resource, which also deletes the ACL requires just acl:Write, but deleting the ACL directly requires write on the ACL, i.e. acl:Control.

  • solid-spec/173 - proposal: deleting a container is a noop
  • solid-spec/190 - Recursive deletion of members on deletion of container

Covered in solid/solid-spec#195 (comment) , #41 (comment) .

I think we haven't really settled this yet, one thing is to delete resources, another thing is to delete containers. I.e. rm * vs rm -r *.

I think I'll bring solid-spec/190 over to the specification repo, and continue the discussion there.

But for now, this is my proposal: We constrain ourselves to define rm *, this constraint should be relaxed later.

  • solid-spec/157 - Deleting folders when invisible files are left

Covered in solid/solid-spec#157 (comment) .

Summary/Proposal: There is no spooninvisible files. Resources that are not part of containment and are not dependent on a resource's lifecycle are not applicable.

👍 Good formulation!

@kjetilk
Copy link
Member

kjetilk commented Jan 30, 2020

I'm returning to the test suite, and I would find it very useful if we could find rough consensus on this.

Apart from the things where there clearly is consensus above, I'd like to propose two things:

  1. that we do no recurse in the FPWD, we do rm *, and we then let Recursive deletion of members on deletion of container #132 remain open for consideration later.
  2. Deleting the resource, which also deletes the ACL requires just acl:Write, but deleting the ACL directly requires write on the ACL, i.e. acl:Control, since it potentially changes the permissions on the resource.

Anyone opposed?

@csarven
Copy link
Member

csarven commented Jan 30, 2020

No objection to either of the points.

Related to point 2, delete ACL requiring Control is aligned with rough consensus on Control being required to update ACL ( #42 ).

@kjetilk
Copy link
Member

kjetilk commented Feb 4, 2020

Come to think of it, the operation of DELETE container/ doesn't have a UNIX equivalent, AFAICTO, because we haven't got globbing, and rm container/* wouldn't remove the container, and rm -r container/ would recurse.

So, what we're really looking at now is rm container/* ; rmdir container/, i.e. remove the resources, then remove the container, but fail if the container has other containers. Right?

@csarven
Copy link
Member

csarven commented Feb 4, 2020

Right but I think we can be more specific. I think it boils down to:

If the operation is atomic, it would mean that a container can be deleted if and only if all of its containments and associated resources can be deleted. If not, non-containers and associated resources can be deleted independently. The latter probably requires a response indicating what didn't get deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

7 participants