exceptionfactory commented on code in PR #10134:
URL: https://github.com/apache/nifi/pull/10134#discussion_r2231062450


##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/StandardAuthorizableLookup.java:
##########
@@ -569,28 +569,8 @@ public Authorizable getAuthorizableFromResource(final 
String resource) {
             case DataTransfer:
             case ProvenanceData:
             case Operation:
-
-                // get the resource type
-                final String baseResource = 
StringUtils.substringAfter(resource, resourceType.getValue());
-                final ResourceType baseResourceType = 
ResourceType.fromRawValue(baseResource);
-
-                if (baseResourceType == null) {
-                    throw new ResourceNotFoundException("Unrecognized base 
resource: " + resource);
-                }
-
-                switch (resourceType) {
-                    case Policy:
-                        return new 
AccessPolicyAuthorizable(getAccessPolicy(baseResourceType, resource));
-                    case Data:
-                        return new 
DataAuthorizable(getAccessPolicy(baseResourceType, resource));
-                    case DataTransfer:
-                        return new 
DataTransferAuthorizable(getAccessPolicy(baseResourceType, resource));
-                    case ProvenanceData:
-                        return new 
ProvenanceDataAuthorizable(getAccessPolicy(baseResourceType, resource));
-                    case Operation:
-                        return new 
OperationAuthorizable(getAccessPolicy(baseResourceType, resource));
-                }
-            // fallthrough
+                final Authorizable authorizable = 
handleResourceTypeContainingOtherResourceType(resource, resourceType);
+                return authorizable;

Review Comment:
   It looks like this could be collapsed into a single line with a return.



##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/StandardAuthorizableLookup.java:
##########
@@ -628,6 +608,25 @@ public Authorizable getAuthorizableFromResource(final 
String resource) {
         }
     }
 
+    private Authorizable handleResourceTypeContainingOtherResourceType(final 
String resource, final ResourceType resourceType) {
+        // get the resource type
+        final String baseResource = StringUtils.substringAfter(resource, 
resourceType.getValue());
+        final ResourceType baseResourceType = 
ResourceType.fromRawValue(baseResource);
+
+        if (baseResourceType == null) {
+            throw new ResourceNotFoundException("Unrecognized base resource: " 
+ resource);
+        }
+
+        return switch (resourceType) {
+            case Policy -> new 
AccessPolicyAuthorizable(getAccessPolicy(baseResourceType, resource));
+            case Data -> new 
DataAuthorizable(getAccessPolicy(baseResourceType, resource));
+            case DataTransfer -> new 
DataTransferAuthorizable(getAccessPolicy(baseResourceType, resource));
+            case ProvenanceData -> new 
ProvenanceDataAuthorizable(getAccessPolicy(baseResourceType, resource));
+            case Operation -> new 
OperationAuthorizable(getAccessPolicy(baseResourceType, resource));
+            default -> null;

Review Comment:
   Did the previous fall through case return `null` as well? It seems like this 
should throw an exception.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to