Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Effective ACR discovery #244
Effective ACR discovery #244
Changes from 10 commits
51e0a64
98444d9
f234ea8
b3b02c2
5b7b0a0
9fcbb06
561d052
64b2efa
893dc36
54114f0
fc42c54
58f5882
c976ac5
aff3e23
458feef
59b6ee0
b0af854
eb03196
b964c53
d491ea0
1265368
12d5ef9
542ee62
ed04363
4610f80
42e999d
e2b4f81
6799dd9
0ebddb3
7bd0597
d3d5c7f
05bd3fc
8050dba
d744e61
6f97adf
94d0547
627ace9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is rev ldp contains in any spec? Any implementation? Why is this part of the evaluation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the WAC spec says
For ldp the way to find the container is by following the reverse of
ldp:contains
.As I understand you don't specify how to find the container. So I don't see any other way for ldp of finding the parent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right in that it is unspecified, and that can be a reasonable way for LDP clients to find the container of a resource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the client's point of view the only thing that matters should be the
Link: <....acl>; rel="acl">
relation in the header.The text about the effective ACL process (see my other comment) is aimed at the server I think.
But that means that the
Link: </.acl>, rel="acl">
pointing to the default only allows the client to edit the default ACL document, as the client cannot just create ACL resources by guessing them.So that means that: a server that allows the client to also edit acls for a resource must have 2
Link:
headers: one for the default and that could be created with a PUT, in order to override the default. I think @csarven agreed with that in the last meeting.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recall saying that. I probably said that (UN)LINK methods can be used or extensions can be introduced to do those things - as mentioned in the current WAC ED.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I thought we had come to the conclusion that having two acl links could solve the problem. But it is not in the meeting notes.
Does the spec explicitly state that one cannot have two link headers?
Anway, so if I understand you correctly, the right way to do things is for the acl link header to point to the non-existent acl for that resource, and if the client gets a 404 Page not found then it should follow the links down the hierarchy container by container to find the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a full description of what needs to be done in WAC with a default three levels deep in this commit. 893dc36
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superb, thank you Henry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the algorithm for determining the ACL of a resource in the spec is relevant to the client.
In the previous wiki version of the spec on which the current spec is based it was stated quite clearly that one should not try to guess.
So I think that text is only aimed at the server, to allow the server to determine which ACL to add to the header. Am I right @csarven ?
btw. I think that means the text "To determine the effective ACL..." should be: "For a server to determine..." the effective ACL of a resource...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is aimed at any anything that wants to determine the ACL of a resource, including servers, clients..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous version of the document that was the official one just a little over a month ago, stated that
Also the algorithm given for finding the container is very vague, because you are trying to specify WAC without assuming LDP, which would make finding the container a lot simpler.
So should we instead write up an example where the client has to try to guess its way back up the hierarchy to find the default ACL? I guess we can make our lives a bit simpler to ourselves if we also have a
Link: <.>; rev="http://www.w3.org/ns/ldp#contains">
on the resource, so that the client can find the default.Which do you prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous document did say that but we also had an issue on solid/specification#106 to change that. WAC ED's #effective-acl-resource addressed that issue.
As for WAC without LDP's containment.. that makes the spec more flexible and combined with other specs. Servers implementing LDP/Solid can already work with it.