[
https://issues.apache.org/jira/browse/HDFS-5673?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13852200#comment-13852200
]
Haohui Mai commented on HDFS-5673:
----------------------------------
Acl:
Acl.Builder can initialize the entries field along with the declaration.
The constructor of Acl does not need to copy the list.
AclTransformation:
It seems to me that you can pass in a wrapper for Acl to indicate that the Acl
list is well-formed. For example, you can have the following class:
{code}
class AclList {
boolean wellFormed;
List<AclEntry> acls;
}
{code}
and put Preconditions.check(AclList.wellFormed) in the functions of
AclTransformation. Of course you might set the variable {{wellFormed}}
incorrectly, but it is much easier to debug in the other places (and
potentially formally verifying the logic as well).
I'm not sure that why AclTransformation needs to be a functor. It makes sense
if you need to iterate through the ACL list for multiple times, otherwise
making them as a function will be simpler and more efficient.
I think the code is probably overly optimized. For example, in
filterAclEntriesByAclSpec(), I would probably write:
{code}
List<AclEntry> entries;
for (AclEntry existingEntry : existingAcl.getEntries()) {
if (aclSpec.indexOf(existingEntry) != -1) {
entries.add();
} else {
entries.delete();
}
}
sort(entries);
return new Acl.Builder().setEntries(entries).build();
{code}
Obviously it is turning an O ( n ) algorithm to an O(nlogn) algorithm, but n is
fairly small in this case (n < 32), and it is much simpler to read and to
reason about. The code is security sensitive therefore I would favor of trading
some efficiency to simplicity and readability.
> Implement logic for modification of ACLs.
> -----------------------------------------
>
> Key: HDFS-5673
> URL: https://issues.apache.org/jira/browse/HDFS-5673
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Affects Versions: HDFS ACLs (HDFS-4685)
> Reporter: Chris Nauroth
> Assignee: Chris Nauroth
> Attachments: HDFS-5673.1.patch
>
>
> This patch will include the core logic for modification of ACLs. This
> includes support for all user-facing APIs that modify ACLs. This will cover
> access ACLs, default ACLs, automatic mask calculations, automatic inference
> of unprovided default ACL entries, and validation to prevent creation of an
> invalid ACL.
--
This message was sent by Atlassian JIRA
(v6.1.4#6159)