ChenSammi commented on a change in pull request #1716:
URL: https://github.com/apache/ozone/pull/1716#discussion_r547724814
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmOzoneAclMap.java
##########
@@ -100,28 +97,58 @@ private BitSet getAcl(OzoneAclType type, String user) {
// Add a new acl to the map
public void addAcl(OzoneAcl acl) throws OMException {
Objects.requireNonNull(acl, "Acl should not be null.");
+ OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name());
if (acl.getAclScope().equals(OzoneAcl.AclScope.DEFAULT)) {
- defaultAclList.add(OzoneAcl.toProtobuf(acl));
+ addDefaultAcl(acl);
return;
}
- OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name());
if (!getAccessAclMap(aclType).containsKey(acl.getName())) {
getAccessAclMap(aclType).put(acl.getName(), acl.getAclBitSet());
} else {
- // Check if we are adding new rights to existing acl.
- BitSet temp = (BitSet) acl.getAclBitSet().clone();
- BitSet curRights = (BitSet) getAccessAclMap(aclType).
- get(acl.getName()).clone();
- temp.or(curRights);
-
- if (temp.equals(curRights)) {
- // throw exception if acl is already added.
- throw new OMException("Acl " + acl + " already exist.",
- INVALID_REQUEST);
+ BitSet currBitSet = getAccessAclMap(aclType).get(acl.getName());
Review comment:
currBitSet -> curBitSet
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmOzoneAclMap.java
##########
@@ -100,28 +97,58 @@ private BitSet getAcl(OzoneAclType type, String user) {
// Add a new acl to the map
public void addAcl(OzoneAcl acl) throws OMException {
Objects.requireNonNull(acl, "Acl should not be null.");
+ OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name());
if (acl.getAclScope().equals(OzoneAcl.AclScope.DEFAULT)) {
- defaultAclList.add(OzoneAcl.toProtobuf(acl));
+ addDefaultAcl(acl);
return;
}
- OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name());
if (!getAccessAclMap(aclType).containsKey(acl.getName())) {
getAccessAclMap(aclType).put(acl.getName(), acl.getAclBitSet());
} else {
- // Check if we are adding new rights to existing acl.
- BitSet temp = (BitSet) acl.getAclBitSet().clone();
- BitSet curRights = (BitSet) getAccessAclMap(aclType).
- get(acl.getName()).clone();
- temp.or(curRights);
-
- if (temp.equals(curRights)) {
- // throw exception if acl is already added.
- throw new OMException("Acl " + acl + " already exist.",
- INVALID_REQUEST);
+ BitSet currBitSet = getAccessAclMap(aclType).get(acl.getName());
+ BitSet bitSet = checkAndGet(acl, currBitSet);
+ getAccessAclMap(aclType).replace(acl.getName(), bitSet);
+ }
+ }
+
+ private void addDefaultAcl(OzoneAcl acl) throws OMException {
+ OzoneAclInfo ozoneAclInfo = OzoneAcl.toProtobuf(acl);
+ if (defaultAclList.contains(ozoneAclInfo)) {
+ aclExistsError(acl);
+ } else {
+ for (int i = 0; i < defaultAclList.size(); i++) {
+ OzoneAclInfo old = defaultAclList.get(i);
+ if (old.getType() == ozoneAclInfo.getType() && old.getName().equals(
+ ozoneAclInfo.getName())) {
+ BitSet currBitSet = BitSet.valueOf(old.getRights().toByteArray());
+ BitSet bitSet = checkAndGet(acl, currBitSet);
+ ozoneAclInfo = OzoneAclInfo.newBuilder(ozoneAclInfo).setRights(
+ ByteString.copyFrom(bitSet.toByteArray())).build();
+ defaultAclList.remove(i);
+ defaultAclList.add(ozoneAclInfo);
+ return;
+ }
}
- getAccessAclMap(aclType).replace(acl.getName(), temp);
}
+ defaultAclList.add(ozoneAclInfo);
+ }
+
+ private void aclExistsError(OzoneAcl acl) throws OMException {
+ // throw exception if acl is already added.
+ throw new OMException("Acl " + acl + " already exist.", INVALID_REQUEST);
+ }
+
+ private BitSet checkAndGet(OzoneAcl acl, BitSet currBitSet)
+ throws OMException {
Review comment:
New line indent is 2. Here second line of a long line is 4.
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmOzoneAclMap.java
##########
@@ -100,28 +97,58 @@ private BitSet getAcl(OzoneAclType type, String user) {
// Add a new acl to the map
public void addAcl(OzoneAcl acl) throws OMException {
Objects.requireNonNull(acl, "Acl should not be null.");
+ OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name());
if (acl.getAclScope().equals(OzoneAcl.AclScope.DEFAULT)) {
- defaultAclList.add(OzoneAcl.toProtobuf(acl));
+ addDefaultAcl(acl);
return;
}
- OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name());
if (!getAccessAclMap(aclType).containsKey(acl.getName())) {
getAccessAclMap(aclType).put(acl.getName(), acl.getAclBitSet());
} else {
- // Check if we are adding new rights to existing acl.
- BitSet temp = (BitSet) acl.getAclBitSet().clone();
- BitSet curRights = (BitSet) getAccessAclMap(aclType).
- get(acl.getName()).clone();
- temp.or(curRights);
-
- if (temp.equals(curRights)) {
- // throw exception if acl is already added.
- throw new OMException("Acl " + acl + " already exist.",
- INVALID_REQUEST);
+ BitSet currBitSet = getAccessAclMap(aclType).get(acl.getName());
+ BitSet bitSet = checkAndGet(acl, currBitSet);
+ getAccessAclMap(aclType).replace(acl.getName(), bitSet);
+ }
+ }
+
+ private void addDefaultAcl(OzoneAcl acl) throws OMException {
+ OzoneAclInfo ozoneAclInfo = OzoneAcl.toProtobuf(acl);
+ if (defaultAclList.contains(ozoneAclInfo)) {
+ aclExistsError(acl);
+ } else {
+ for (int i = 0; i < defaultAclList.size(); i++) {
+ OzoneAclInfo old = defaultAclList.get(i);
+ if (old.getType() == ozoneAclInfo.getType() && old.getName().equals(
+ ozoneAclInfo.getName())) {
+ BitSet currBitSet = BitSet.valueOf(old.getRights().toByteArray());
+ BitSet bitSet = checkAndGet(acl, currBitSet);
+ ozoneAclInfo = OzoneAclInfo.newBuilder(ozoneAclInfo).setRights(
+ ByteString.copyFrom(bitSet.toByteArray())).build();
+ defaultAclList.remove(i);
+ defaultAclList.add(ozoneAclInfo);
+ return;
+ }
}
- getAccessAclMap(aclType).replace(acl.getName(), temp);
}
+ defaultAclList.add(ozoneAclInfo);
+ }
+
+ private void aclExistsError(OzoneAcl acl) throws OMException {
+ // throw exception if acl is already added.
+ throw new OMException("Acl " + acl + " already exist.", INVALID_REQUEST);
+ }
+
+ private BitSet checkAndGet(OzoneAcl acl, BitSet currBitSet)
Review comment:
currBitSet -> curBitSet
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmOzoneAclMap.java
##########
@@ -100,28 +97,58 @@ private BitSet getAcl(OzoneAclType type, String user) {
// Add a new acl to the map
public void addAcl(OzoneAcl acl) throws OMException {
Objects.requireNonNull(acl, "Acl should not be null.");
+ OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name());
if (acl.getAclScope().equals(OzoneAcl.AclScope.DEFAULT)) {
- defaultAclList.add(OzoneAcl.toProtobuf(acl));
+ addDefaultAcl(acl);
return;
}
- OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name());
if (!getAccessAclMap(aclType).containsKey(acl.getName())) {
getAccessAclMap(aclType).put(acl.getName(), acl.getAclBitSet());
} else {
- // Check if we are adding new rights to existing acl.
- BitSet temp = (BitSet) acl.getAclBitSet().clone();
- BitSet curRights = (BitSet) getAccessAclMap(aclType).
- get(acl.getName()).clone();
- temp.or(curRights);
-
- if (temp.equals(curRights)) {
- // throw exception if acl is already added.
- throw new OMException("Acl " + acl + " already exist.",
- INVALID_REQUEST);
+ BitSet currBitSet = getAccessAclMap(aclType).get(acl.getName());
+ BitSet bitSet = checkAndGet(acl, currBitSet);
+ getAccessAclMap(aclType).replace(acl.getName(), bitSet);
+ }
+ }
+
+ private void addDefaultAcl(OzoneAcl acl) throws OMException {
+ OzoneAclInfo ozoneAclInfo = OzoneAcl.toProtobuf(acl);
+ if (defaultAclList.contains(ozoneAclInfo)) {
+ aclExistsError(acl);
+ } else {
+ for (int i = 0; i < defaultAclList.size(); i++) {
+ OzoneAclInfo old = defaultAclList.get(i);
+ if (old.getType() == ozoneAclInfo.getType() && old.getName().equals(
+ ozoneAclInfo.getName())) {
+ BitSet currBitSet = BitSet.valueOf(old.getRights().toByteArray());
+ BitSet bitSet = checkAndGet(acl, currBitSet);
+ ozoneAclInfo = OzoneAclInfo.newBuilder(ozoneAclInfo).setRights(
+ ByteString.copyFrom(bitSet.toByteArray())).build();
+ defaultAclList.remove(i);
+ defaultAclList.add(ozoneAclInfo);
+ return;
+ }
}
- getAccessAclMap(aclType).replace(acl.getName(), temp);
}
+ defaultAclList.add(ozoneAclInfo);
+ }
+
+ private void aclExistsError(OzoneAcl acl) throws OMException {
Review comment:
Could you replace other same expection codes in this class using this
aclExistsError function?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
