From 34cab7d043cc2dacbd9436e9b75a0279d353d0e0 Mon Sep 17 00:00:00 2001 From: ran Date: Fri, 30 Jun 2023 12:46:01 +0800 Subject: [PATCH] [improvement] Remove expensive useless String.format() in canConsumeAsync (#1893) (cherry picked from commit fa63a4ab278012ae83319ba2690e201013f2245b) This PR is used to cherry-pick the commit https://github.com/datastax/starlight-for-kafka/commit/fa63a4ab278012ae83319ba2690e201013f2245b. ### Motivation The String.format() is an expensive operation in the method canConsumeAsync. ### Modifications Remove the String.format() operation. Co-authored-by: Enrico Olivelli (cherry picked from commit 45bd29c4cfe4fe9e8fc0fd8ccc4105ccde07ef1c) --- .../security/auth/SimpleAclAuthorizer.java | 43 ++++++++----------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/security/auth/SimpleAclAuthorizer.java b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/security/auth/SimpleAclAuthorizer.java index 6562b3596e..f09ff281af 100644 --- a/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/security/auth/SimpleAclAuthorizer.java +++ b/kafka-impl/src/main/java/io/streamnative/pulsar/handlers/kop/security/auth/SimpleAclAuthorizer.java @@ -13,9 +13,6 @@ */ package io.streamnative.pulsar.handlers.kop.security.auth; - -import static com.google.common.base.Preconditions.checkArgument; - import io.streamnative.pulsar.handlers.kop.security.KafkaPrincipal; import java.util.concurrent.CompletableFuture; import lombok.extern.slf4j.Slf4j; @@ -70,9 +67,7 @@ private CompletableFuture authorizeTenantPermission(KafkaPrincipal prin @Override public CompletableFuture canAccessTenantAsync(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.TENANT, - String.format("Expected resource type is TENANT, but have [%s]", resource.getResourceType())); - + checkResourceType(resource, ResourceType.TENANT); CompletableFuture canAccessFuture = new CompletableFuture<>(); authorizeTenantPermission(principal, resource).whenComplete((hasPermission, ex) -> { if (ex != null) { @@ -92,9 +87,7 @@ public CompletableFuture canAccessTenantAsync(KafkaPrincipal principal, @Override public CompletableFuture canCreateTopicAsync(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.TOPIC, - String.format("Expected resource type is TOPIC, but have [%s]", resource.getResourceType())); - + checkResourceType(resource, ResourceType.TOPIC); TopicName topicName = TopicName.get(resource.getName()); return authorizationService.allowNamespaceOperationAsync( topicName.getNamespaceObject(), @@ -105,9 +98,7 @@ public CompletableFuture canCreateTopicAsync(KafkaPrincipal principal, @Override public CompletableFuture canDeleteTopicAsync(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.TOPIC, - String.format("Expected resource type is TOPIC, but have [%s]", resource.getResourceType())); - + checkResourceType(resource, ResourceType.TOPIC); TopicName topicName = TopicName.get(resource.getName()); return authorizationService.allowNamespaceOperationAsync( topicName.getNamespaceObject(), @@ -118,9 +109,7 @@ public CompletableFuture canDeleteTopicAsync(KafkaPrincipal principal, @Override public CompletableFuture canAlterTopicAsync(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.TOPIC, - String.format("Expected resource type is TOPIC, but have [%s]", resource.getResourceType())); - + checkResourceType(resource, ResourceType.TOPIC); TopicName topicName = TopicName.get(resource.getName()); return authorizationService.allowTopicPolicyOperationAsync( topicName, PolicyName.PARTITION, PolicyOperation.WRITE, @@ -129,9 +118,7 @@ public CompletableFuture canAlterTopicAsync(KafkaPrincipal principal, R @Override public CompletableFuture canManageTenantAsync(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.TOPIC, - String.format("Expected resource type is TOPIC, but have [%s]", resource.getResourceType())); - + checkResourceType(resource, ResourceType.TOPIC); TopicName topicName = TopicName.get(resource.getName()); return authorizationService.allowTopicOperationAsync( topicName, TopicOperation.LOOKUP, principal.getName(), principal.getAuthenticationData()); @@ -139,16 +126,14 @@ public CompletableFuture canManageTenantAsync(KafkaPrincipal principal, @Override public CompletableFuture canLookupAsync(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.TOPIC, - String.format("Expected resource type is TOPIC, but have [%s]", resource.getResourceType())); + checkResourceType(resource, ResourceType.TOPIC); TopicName topicName = TopicName.get(resource.getName()); return authorizationService.canLookupAsync(topicName, principal.getName(), principal.getAuthenticationData()); } @Override public CompletableFuture canGetTopicList(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.NAMESPACE, - String.format("Expected resource type is NAMESPACE, but have [%s]", resource.getResourceType())); + checkResourceType(resource, ResourceType.NAMESPACE); return authorizationService.allowNamespaceOperationAsync( NamespaceName.get(resource.getName()), NamespaceOperation.GET_TOPICS, @@ -158,19 +143,25 @@ public CompletableFuture canGetTopicList(KafkaPrincipal principal, Reso @Override public CompletableFuture canProduceAsync(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.TOPIC, - String.format("Expected resource type is TOPIC, but have [%s]", resource.getResourceType())); + checkResourceType(resource, ResourceType.TOPIC); TopicName topicName = TopicName.get(resource.getName()); return authorizationService.canProduceAsync(topicName, principal.getName(), principal.getAuthenticationData()); } @Override public CompletableFuture canConsumeAsync(KafkaPrincipal principal, Resource resource) { - checkArgument(resource.getResourceType() == ResourceType.TOPIC, - String.format("Expected resource type is TOPIC, but have [%s]", resource.getResourceType())); + checkResourceType(resource, ResourceType.TOPIC); TopicName topicName = TopicName.get(resource.getName()); return authorizationService.canConsumeAsync( topicName, principal.getName(), principal.getAuthenticationData(), ""); } + private void checkResourceType(Resource actual, ResourceType expected) { + if (actual.getResourceType() != expected) { + throw new IllegalArgumentException( + String.format("Expected resource type is [%s], but have [%s]", + expected, actual.getResourceType())); + } + } + } \ No newline at end of file