Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(275)

Issue 271210043: FS Adds support for non-root startpaths (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 5 months ago by Brett
Modified:
6 years, 11 months ago
Reviewers:
pjo, johntin0804, JohnL
CC:
connector-cr_google.com
Visibility:
Public.

Description

This change adds support for startpaths that are not at the root of a file share, a frequent customer request. The deeper startpaths supply an ACL that includes permissions inherited from it ancestors. This change revealed a flaw when testing ACLs on DFS paths which prompted two changes: * ACLs on verious nodes now use unique group names to make sure the correct ACL is used in the named resource. * I dispensed with the scetchy getFileOrRoot slight-of-hand when dealing with DFS and Share ACLs. The targets of DFS links now have a separate root in the mock filesystem.

Patch Set 1 #

Total comments: 15
Unified diffs Side-by-side diffs Delta from patch set Stats (+210 lines, -81 lines) Patch
M src/com/google/enterprise/adaptor/fs/FsAdaptor.java View 4 chunks +19 lines, -16 lines 10 comments Download
M src/com/google/enterprise/adaptor/fs/WindowsAclFileAttributeViews.java View 1 chunk +1 line, -1 line 2 comments Download
M test/com/google/enterprise/adaptor/fs/FsAdaptorTest.java View 10 chunks +184 lines, -33 lines 3 comments Download
M test/com/google/enterprise/adaptor/fs/MockFileDelegate.java View 2 chunks +6 lines, -19 lines 0 comments Download
M test/com/google/enterprise/adaptor/fs/MultiRootMockFileDelegate.java View 1 chunk +0 lines, -12 lines 0 comments Download

Messages

Total messages: 10
Brett
8 years, 5 months ago (2015-10-31 21:31:55 UTC) #1
pjo
Thank you https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode625 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:625: if (delegate.isDfsNamespace(file) || delegate.isDfsLink(file)) { been a ...
8 years, 4 months ago (2015-11-06 19:02:36 UTC) #2
Brett
https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode625 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:625: if (delegate.isDfsNamespace(file) || delegate.isDfsLink(file)) { On 2015/11/06 19:02:36, pjo ...
8 years, 4 months ago (2015-11-06 20:56:15 UTC) #3
myk
I don't find anything in this CL to object to, but I don't feel I ...
8 years, 4 months ago (2015-11-06 21:42:02 UTC) #4
pjo
Sorry for being slow with this CL. The material is not fresh in my mind. ...
8 years, 4 months ago (2015-11-06 23:44:16 UTC) #5
Brett
https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode727 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:727: } else if (dfsRoot != null && delegate.isDfsLink(dfsRoot)) { ...
8 years, 4 months ago (2015-11-07 00:13:49 UTC) #6
pjo
LGTM. Thank you https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode617 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:617: } indent 2 more spaces https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode625 ...
8 years, 4 months ago (2015-11-10 00:27:14 UTC) #7
Brett
Committed 13 November 2015 to Filesystem adaptor. https://github.com/googlegsa/filesystem/commit/1d06f8196a4408612eb2b1c4253b01dd20acbf92 https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java File src/com/google/enterprise/adaptor/fs/FsAdaptor.java (right): https://codereview.appspot.com/271210043/diff/1/src/com/google/enterprise/adaptor/fs/FsAdaptor.java#newcode617 src/com/google/enterprise/adaptor/fs/FsAdaptor.java:617: } ...
8 years, 4 months ago (2015-11-13 18:58:47 UTC) #8
johntin0804
6 years, 11 months ago (2017-04-28 04:32:13 UTC) #9
johntin0804
6 years, 11 months ago (2017-04-28 04:33:39 UTC) #10
Message was sent while issue was closed.

          
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b