Add more tests

internal/venice-common/src/main/java/com/linkedin/venice/acl/handler/AbstractStoreAclHandler.java

@@ -32,7 +32,7 @@
  * Store-level access control handler, which is being used by both Router and Server.
  */
 @ChannelHandler.Sharable
-public abstract class AbstractStoreAclHandler extends SimpleChannelInboundHandler<HttpRequest> {
+public abstract class AbstractStoreAclHandler<REQUEST_TYPE> extends SimpleChannelInboundHandler<HttpRequest> {
   private static final Logger LOGGER = LogManager.getLogger(AbstractStoreAclHandler.class);
 
   private final IdentityParser identityParser;
@@ -73,19 +73,20 @@ public void channelRead0(ChannelHandlerContext ctx, HttpRequest req) throws SSLP
 
     // Parse resource type and store name
     String[] requestParts = URI.create(uri).getPath().split("/");
+    REQUEST_TYPE requestType = validateRequest(requestParts);
 
-    if (isInvalidRequest(requestParts)) {
+    if (requestType == null) {
       errorHandler.accept(HttpResponseStatus.BAD_REQUEST, "Invalid request uri: " + uri);
       return;
     }
 
-    if (!needsAclValidation(requestParts)) {
+    if (!needsAclValidation(requestType)) {
       ReferenceCountUtil.retain(req);
       ctx.fireChannelRead(req);
       return;
     }
 
-    String storeName = extractStoreName(requestParts);
+    String storeName = extractStoreName(requestType, requestParts);
     X509Certificate clientCert = extractClientCert(ctx);
 
     try {
@@ -113,11 +114,17 @@ protected boolean isAccessAlreadyApproved(ChannelHandlerContext ctx) {
     return false;
   }
 
-  protected abstract boolean needsAclValidation(String[] requestParts);
+  protected abstract boolean needsAclValidation(REQUEST_TYPE requestType);
 
-  protected abstract String extractStoreName(String[] requestParts);
+  protected abstract String extractStoreName(REQUEST_TYPE requestType, String[] requestParts);
 
-  protected abstract boolean isInvalidRequest(String[] requestParts);
+  /**
+   * Validate the request and return the request type. If the request is invalid, return {@code null}
+   *
+   * @param requestParts the parts of the request URI
+   * @return the request type; null if the request is invalid
+   */
+  protected abstract REQUEST_TYPE validateRequest(String[] requestParts);
 
   @VisibleForTesting
   protected boolean hasAccess(

internal/venice-common/src/test/java/com/linkedin/venice/acl/handler/AbstractStoreAclHandlerTest.java

@@ -320,12 +320,16 @@ public boolean matches(FullHttpResponse argument) {
     }
   }
 
+  private enum TestRequestType {
+    GOOD_URI, NO_ACL
+  }
+
   /**
    * Assume a service with the following endpoints:
    * /goodUri/{storeName}/random
    * /noAcl
    */
-  private static class MockStoreAclHandler extends AbstractStoreAclHandler {
+  private static class MockStoreAclHandler extends AbstractStoreAclHandler<TestRequestType> {
     public MockStoreAclHandler(
         IdentityParser identityParser,
         DynamicAccessController accessController,
@@ -334,18 +338,24 @@ public MockStoreAclHandler(
     }
 
     @Override
-    protected boolean needsAclValidation(String[] requestParts) {
-      return !requestParts[1].equals("noAcl");
+    protected boolean needsAclValidation(TestRequestType requestType) {
+      return requestType != TestRequestType.NO_ACL;
     }
 
     @Override
-    protected String extractStoreName(String[] requestParts) {
+    protected String extractStoreName(TestRequestType requestType, String[] requestParts) {
       return requestParts[2];
     }
 
     @Override
-    protected boolean isInvalidRequest(String[] requestParts) {
-      return requestParts[1].equals("badUri");
+    protected TestRequestType validateRequest(String[] requestParts) {
+      if (requestParts[1].equals("noAcl")) {
+        return TestRequestType.NO_ACL;
+      } else if (requestParts[1].equals("goodUri")) {
+        return TestRequestType.GOOD_URI;
+      } else {
+        return null;
+      }
     }
   }
 }

services/venice-router/src/main/java/com/linkedin/venice/router/acl/RouterStoreAclHandler.java

@@ -26,7 +26,7 @@
  * Store-level access control handler, which is being used by both Router and Server.
  */
 @ChannelHandler.Sharable
-public class RouterStoreAclHandler extends AbstractStoreAclHandler {
+public class RouterStoreAclHandler extends AbstractStoreAclHandler<RouterResourceType> {
   private final HelixReadOnlyStoreConfigRepository storeConfigRepository;
   private final VeniceRouterConfig config;
 
@@ -43,9 +43,7 @@ public RouterStoreAclHandler(
   }
 
   @Override
-  protected boolean needsAclValidation(String[] requestParts) {
-    String requestType = requestParts[1].toLowerCase();
-    RouterResourceType resourceType = RouterResourceType.getTypeResourceType(requestType);
+  protected boolean needsAclValidation(RouterResourceType resourceType) {
     switch (resourceType) {
       case TYPE_LEADER_CONTROLLER:
       case TYPE_LEADER_CONTROLLER_LEGACY:
@@ -62,41 +60,42 @@ protected boolean needsAclValidation(String[] requestParts) {
       case TYPE_ADMIN: // Access control for Admin operations are handled in AdminOperationsHandler
       case TYPE_CURRENT_VERSION:
       case TYPE_RESOURCE_STATE:
-        return false;
+      case TYPE_BLOB_DISCOVERY:
       case TYPE_REQUEST_TOPIC:
+        return false;
       case TYPE_STORAGE:
       case TYPE_COMPUTE:
-      case TYPE_BLOB_DISCOVERY:
         return true;
       case TYPE_INVALID:
       default:
-        throw new VeniceUnsupportedOperationException(requestType);
+        throw new VeniceUnsupportedOperationException(resourceType.name());
     }
   }
 
   /**
    * Extract the store name from the incoming resource name.
    */
-  protected String extractStoreName(String[] requestParts) {
+  @Override
+  protected String extractStoreName(RouterResourceType resourceType, String[] requestParts) {
+    // In Routers, all requests that go through ACL checks have the 2nd part as the store name
     return requestParts[2];
   }
 
   @Override
-  protected boolean isInvalidRequest(String[] requestParts) {
+  protected RouterResourceType validateRequest(String[] requestParts) {
     int partsLength = requestParts.length;
-    boolean invalidRequest = false;
 
     if (partsLength < 3) {
       // In routers, all requests have at least the request type and store name
-      invalidRequest = true;
+      return null;
     } else { // throw exception to retain current behavior for invalid query actions
       String requestType = requestParts[1].toLowerCase();
-      if (RouterResourceType.getTypeResourceType(requestType) == TYPE_INVALID) {
-        invalidRequest = true;
+      RouterResourceType resourceType = RouterResourceType.getTypeResourceType(requestType);
+      if (resourceType == TYPE_INVALID) {
+        return null;
       }
+      return resourceType;
     }
-
-    return invalidRequest;
   }
 
   @Override