HDDS-14104. Refactor RequestContext creation (#9493)

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java

@@ -18,15 +18,14 @@
 package org.apache.hadoop.ozone.security.acl;
 
 import java.net.InetAddress;
-import org.apache.hadoop.ipc.ProtobufRpcEngine;
 import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType;
 import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
 import org.apache.hadoop.security.UserGroupInformation;
 
 /**
  * This class encapsulates information required for Ozone ACLs.
  * */
-public class RequestContext {
+public final class RequestContext {
   private final String host;
   private final InetAddress ip;
   private final UserGroupInformation clientUgi;
@@ -51,43 +50,22 @@ public class RequestContext {
    */
   private final String sessionPolicy;
 
-  @SuppressWarnings("parameternumber")
-  public RequestContext(String host, InetAddress ip,
-      UserGroupInformation clientUgi, String serviceId,
-      ACLIdentityType aclType, ACLType aclRights,
-      String ownerName) {
-    this(host, ip, clientUgi, serviceId, aclType, aclRights, ownerName,
-        false, null);
-  }
-
-  @SuppressWarnings("parameternumber")
-  public RequestContext(String host, InetAddress ip,
-      UserGroupInformation clientUgi, String serviceId,
-      ACLIdentityType aclType, ACLType aclRights,
-      String ownerName, boolean recursiveAccessCheck) {
-    this(host, ip, clientUgi, serviceId, aclType, aclRights, ownerName,
-        recursiveAccessCheck, null);
-  }
-
-  @SuppressWarnings("parameternumber")
-  public RequestContext(String host, InetAddress ip, UserGroupInformation clientUgi, String serviceId,
-      ACLIdentityType aclType, ACLType aclRights, String ownerName, boolean recursiveAccessCheck,
-      String sessionPolicy) {
-    this.host = host;
-    this.ip = ip;
-    this.clientUgi = clientUgi;
-    this.serviceId = serviceId;
-    this.aclType = aclType;
-    this.aclRights = aclRights;
-    this.ownerName = ownerName;
-    this.recursiveAccessCheck = recursiveAccessCheck;
-    this.sessionPolicy = sessionPolicy;
+  private RequestContext(Builder builder) {
+    this.host = builder.host;
+    this.ip = builder.ip;
+    this.clientUgi = builder.clientUgi;
+    this.serviceId = builder.serviceId;
+    this.aclType = builder.aclType;
+    this.aclRights = builder.aclRights;
+    this.ownerName = builder.ownerName;
+    this.recursiveAccessCheck = builder.recursiveAccessCheck;
+    this.sessionPolicy = builder.sessionPolicy;
   }
 
   /**
    * Builder class for @{@link RequestContext}.
    */
-  public static class Builder {
+  public static final class Builder {
     private String host;
     private InetAddress ip;
     private UserGroupInformation clientUgi;
@@ -104,6 +82,10 @@ public static class Builder {
     private boolean recursiveAccessCheck;
     private String sessionPolicy;
 
+    private Builder() {
+
+    }
+
     public Builder setHost(String bHost) {
       this.host = bHost;
       return this;
@@ -154,49 +136,14 @@ public Builder setSessionPolicy(String sessionPolicy) {
     }
 
     public RequestContext build() {
-      return new RequestContext(host, ip, clientUgi, serviceId, aclType,
-          aclRights, ownerName, recursiveAccessCheck, sessionPolicy);
+      return new RequestContext(this);
     }
   }
 
   public static Builder newBuilder() {
     return new Builder();
   }
 
-  public static RequestContext.Builder getBuilder(
-      UserGroupInformation ugi, InetAddress remoteAddress, String hostName,
-      ACLType aclType, String ownerName) {
-    return getBuilder(ugi, remoteAddress, hostName, aclType, ownerName,
-        false);
-  }
-
-  public static RequestContext.Builder getBuilder(
-      UserGroupInformation ugi, InetAddress remoteAddress, String hostName,
-      ACLType aclType, String ownerName, boolean recursiveAccessCheck) {
-    return getBuilder(ugi, remoteAddress, hostName, aclType, ownerName, recursiveAccessCheck, null);
-  }
-
-  public static RequestContext.Builder getBuilder(UserGroupInformation ugi, InetAddress remoteAddress, String hostName,
-      ACLType aclType, String ownerName, boolean recursiveAccessCheck, String sessionPolicy) {
-    return RequestContext.newBuilder()
-        .setClientUgi(ugi)
-        .setIp(remoteAddress)
-        .setHost(hostName)
-        .setAclType(ACLIdentityType.USER)
-        .setAclRights(aclType)
-        .setOwnerName(ownerName)
-        .setRecursiveAccessCheck(recursiveAccessCheck)
-        .setSessionPolicy(sessionPolicy);
-  }
-
-  public static RequestContext.Builder getBuilder(UserGroupInformation ugi,
-      ACLType aclType, String ownerName) {
-    return getBuilder(ugi,
-        ProtobufRpcEngine.Server.getRemoteIp(),
-        ProtobufRpcEngine.Server.getRemoteIp().getHostName(),
-        aclType, ownerName);
-  }
-
   public String getHost() {
     return host;
   }

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java

@@ -335,7 +335,7 @@ private void resetAclsAndValidateAccess(
     String group = (!testUgi.getGroups().isEmpty()) ?
         testUgi.getGroups().get(0) : "";
 
-    RequestContext.Builder builder = new RequestContext.Builder()
+    RequestContext.Builder builder = RequestContext.newBuilder()
         .setClientUgi(testUgi)
         .setAclType(accessType);
 

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestParentAcl.java

@@ -210,10 +210,11 @@ private void resetAcl(String vol, List<OzoneAcl> volAcls,
   private void testParentChild(OzoneObj child,
       ACLType parentAclType, ACLType childAclType) throws IOException {
 
-    RequestContext requestContext = new RequestContext.Builder()
+    RequestContext requestContext = RequestContext.newBuilder()
         .setClientUgi(testUgi1)
         .setAclType(USER)
-        .setAclRights(childAclType).build();
+        .setAclRights(childAclType)
+        .build();
 
     OzoneAcl childAcl = OzoneAcl.of(USER,
         testUgi1.getUserName(), ACCESS, childAclType);

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java

@@ -22,8 +22,6 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import java.io.IOException;
-import org.apache.hadoop.security.UserGroupInformation;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -32,103 +30,25 @@
 public class TestRequestContext {
 
   @Test
-  public void testRecursiveAccessFlag() throws IOException {
-    RequestContext context = getUserRequestContext("om",
-            IAccessAuthorizer.ACLType.CREATE, false, "volume1",
-            true);
-    assertTrue(context.isRecursiveAccessCheck(),
-        "Wrongly sets recursiveAccessCheck flag value");
+  void testRecursiveAccessFlag() {
+    RequestContext.Builder builder = RequestContext.newBuilder();
 
-    context = getUserRequestContext("om",
-            IAccessAuthorizer.ACLType.CREATE, false, "volume1",
-            false);
-    assertFalse(context.isRecursiveAccessCheck(),
-        "Wrongly sets recursiveAccessCheck flag value");
-
-    context = getUserRequestContext(
-            "user1", IAccessAuthorizer.ACLType.CREATE,
-            true, "volume1");
-    assertFalse(context.isRecursiveAccessCheck(),
-        "Wrongly sets recursiveAccessCheck flag value");
-
-    RequestContext.Builder builder = new RequestContext.Builder();
-
-    assertFalse(builder.build().isRecursiveAccessCheck(),
-        "Wrongly sets recursive flag value");
+    assertFalse(builder.build().isRecursiveAccessCheck(), "default value");
 
     builder.setRecursiveAccessCheck(true);
-    assertTrue(builder.build().isRecursiveAccessCheck(),
-        "Wrongly sets recursive flag value");
-
-    context = new RequestContext("host", null,
-            null, "serviceId",
-            IAccessAuthorizer.ACLIdentityType.GROUP,
-            IAccessAuthorizer.ACLType.CREATE, "owner");
-    assertFalse(context.isRecursiveAccessCheck(),
-        "Wrongly sets recursive flag value");
+    assertTrue(builder.build().isRecursiveAccessCheck());
 
-    context = new RequestContext("host", null,
-            null, "serviceId",
-            IAccessAuthorizer.ACLIdentityType.GROUP,
-            IAccessAuthorizer.ACLType.CREATE, "owner", false);
-    assertFalse(context.isRecursiveAccessCheck(),
-        "Wrongly sets recursive flag value");
-
-    context = new RequestContext("host", null,
-            null, "serviceId",
-            IAccessAuthorizer.ACLIdentityType.GROUP,
-            IAccessAuthorizer.ACLType.CREATE, "owner", true);
-    assertTrue(context.isRecursiveAccessCheck(),
-        "Wrongly sets recursive flag value");
+    builder.setRecursiveAccessCheck(false);
+    assertFalse(builder.build().isRecursiveAccessCheck());
   }
 
   @Test
-  public void testSessionPolicy() {
-    final RequestContext.Builder builder = new RequestContext.Builder();
-    RequestContext context = builder.build();
-    assertNull(context.getSessionPolicy(), "sessionPolicy should default to null");
+  void testSessionPolicy() {
+    RequestContext.Builder builder = RequestContext.newBuilder();
+    assertNull(builder.build().getSessionPolicy(), "default value");
 
     final String policy = "{\"Statement\":[]}";
-    context = new RequestContext.Builder()
-        .setSessionPolicy(policy)
-        .build();
-    assertEquals(policy, context.getSessionPolicy(), "sessionPolicy should be set via builder");
-
-    context = new RequestContext(
-        "host", null, null, "serviceId", IAccessAuthorizer.ACLIdentityType.GROUP,
-        IAccessAuthorizer.ACLType.CREATE, "owner", true, policy);
-    assertTrue(context.isRecursiveAccessCheck(), "recursiveAccessCheck should be true");
-    assertEquals(policy, context.getSessionPolicy(), "sessionPolicy should be set via constructor");
-
-    context = RequestContext.getBuilder(
-        UserGroupInformation.createRemoteUser("user1"), null, null,
-        IAccessAuthorizer.ACLType.CREATE, "volume1", true)
-        .setSessionPolicy(policy)
-        .build();
-    assertEquals(policy, context.getSessionPolicy(), "sessionPolicy should be set via getBuilder + builder");
-
-    context = RequestContext.getBuilder(
-        UserGroupInformation.createRemoteUser("user1"), null, null,
-        IAccessAuthorizer.ACLType.CREATE, "volume1", true, policy)
-        .build();
-    assertEquals(
-        policy, context.getSessionPolicy(),
-        "sessionPolicy should be set via getBuilder (all params) + builder");
-  }
-
-  private RequestContext getUserRequestContext(String username,
-      IAccessAuthorizer.ACLType type, boolean isOwner, String ownerName,
-      boolean recursiveAccessCheck) throws IOException {
-
-    return RequestContext.getBuilder(
-            UserGroupInformation.createRemoteUser(username), null, null,
-            type, ownerName, recursiveAccessCheck).build();
-  }
-
-  private RequestContext getUserRequestContext(String username,
-      IAccessAuthorizer.ACLType type, boolean isOwner, String ownerName) {
-    return RequestContext.getBuilder(
-            UserGroupInformation.createRemoteUser(username), null, null,
-            type, ownerName).build();
+    builder.setSessionPolicy(policy);
+    assertEquals(policy, builder.build().getSessionPolicy());
   }
 }

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java

@@ -245,9 +245,12 @@ public void testKeyOps() throws Exception {
 
   private RequestContext getUserRequestContext(String username,
       IAccessAuthorizer.ACLType type, boolean isOwner, String ownerName) {
-    return RequestContext.getBuilder(
-        UserGroupInformation.createRemoteUser(username), null, null,
-        type, ownerName).build();
+    return RequestContext.newBuilder()
+        .setClientUgi(UserGroupInformation.createRemoteUser(username))
+        .setAclType(IAccessAuthorizer.ACLIdentityType.USER)
+        .setAclRights(type)
+        .setOwnerName(ownerName)
+        .build();
   }
 
   private static String getTestVolumeName(int index) {