From 2194c2f5e25830d12af99bf4a275b3e8a63cbefe Mon Sep 17 00:00:00 2001
From: Abhishek Kumar <abhi@apache.org>
Date: Mon, 9 Dec 2024 09:34:17 -0800
Subject: [PATCH] RANGER-5046: incoporating code review suggestions

---
 .../ozone/authorizer/RangerOzoneAuthorizer.java   |  6 +++---
 .../ranger/services/ozone/RangerServiceOzone.java | 14 +++++++-------
 .../ranger/services/ozone/client/OzoneClient.java | 15 ++++++++-------
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/plugin-ozone/src/main/java/org/apache/ranger/authorization/ozone/authorizer/RangerOzoneAuthorizer.java b/plugin-ozone/src/main/java/org/apache/ranger/authorization/ozone/authorizer/RangerOzoneAuthorizer.java
index 47a5c8551d..6454f1d3e6 100644
--- a/plugin-ozone/src/main/java/org/apache/ranger/authorization/ozone/authorizer/RangerOzoneAuthorizer.java
+++ b/plugin-ozone/src/main/java/org/apache/ranger/authorization/ozone/authorizer/RangerOzoneAuthorizer.java
@@ -107,7 +107,7 @@ public boolean checkAccess(IOzoneObj ozoneObject, RequestContext context) {
         RangerPerfTracer perf = null;
 
         if (RangerPerfTracer.isPerfTraceEnabled(PERF_OZONEAUTH_REQUEST_LOG)) {
-            perf = RangerPerfTracer.getPerfTracer(PERF_OZONEAUTH_REQUEST_LOG, "RangerOzoneAuthorizer.authorize(resource=" + resource + ")");
+            perf = RangerPerfTracer.getPerfTracer(PERF_OZONEAUTH_REQUEST_LOG, String.format("RangerOzoneAuthorizer.authorize(resource = %s)", resource));
         }
 
         Date   eventTime  = new Date();
@@ -115,7 +115,7 @@ public boolean checkAccess(IOzoneObj ozoneObject, RequestContext context) {
         if (accessType == null) {
             String message = String.format("Unsupported access type. operation = %s", operation);
             MiscUtil.logErrorMessageByInterval(LOG, message);
-            LOG.error(message + ", resource = " + resource);
+            LOG.error("{}, resource = {}", message, resource);
             return returnValue;
         }
         String action      = accessType;
@@ -150,7 +150,7 @@ public boolean checkAccess(IOzoneObj ozoneObject, RequestContext context) {
                 rangerResource.setValue(KEY_RESOURCE_KEY, ozoneObj.getKeyName());
             }
         } else {
-            LOG.error("Unsupported resource = " + resource);
+            LOG.error("Unsupported resource = {}", resource);
             String message = String.format("Unsupported resource type = %s for resource = %s, request = %s", ozoneObj.getResourceType(), resource, rangerRequest);
             MiscUtil.logErrorMessageByInterval(LOG, message);
             return returnValue;
diff --git a/plugin-ozone/src/main/java/org/apache/ranger/services/ozone/RangerServiceOzone.java b/plugin-ozone/src/main/java/org/apache/ranger/services/ozone/RangerServiceOzone.java
index 252d96f110..eb982a158c 100644
--- a/plugin-ozone/src/main/java/org/apache/ranger/services/ozone/RangerServiceOzone.java
+++ b/plugin-ozone/src/main/java/org/apache/ranger/services/ozone/RangerServiceOzone.java
@@ -64,20 +64,20 @@ public Map<String, Object> validateConfig() throws Exception {
         Map<String, Object> ret         = new HashMap<String, Object>();
         String              serviceName = getServiceName();
         if (LOG.isDebugEnabled()) {
-            LOG.debug("<== RangerServiceOzone.validateConfig Service: (" + serviceName + " )");
+            LOG.debug("<== RangerServiceOzone.validateConfig Service: ({})", serviceName);
         }
 
         if (configs != null) {
             try {
                 ret = OzoneResourceMgr.connectionTest(serviceName, configs);
             } catch (HadoopException e) {
-                LOG.error("<== RangerServiceOzone.validateConfig Error: " + e.getMessage(), e);
+                LOG.error("<== RangerServiceOzone.validateConfig Error: {}", e.getMessage(), e);
                 throw e;
             }
         }
 
         if (LOG.isDebugEnabled()) {
-            LOG.debug("<== RangerServiceOzone.validateConfig Response : (" + ret + " )");
+            LOG.debug("<== RangerServiceOzone.validateConfig Response : ({})", ret);
         }
 
         return ret;
@@ -90,18 +90,18 @@ public List<String> lookupResource(ResourceLookupContext context) throws Excepti
         String              serviceType = getServiceType();
         Map<String, String> configs     = getConfigs();
         if (LOG.isDebugEnabled()) {
-            LOG.debug("==> RangerServiceOzone.lookupResource Context: (" + context + ")");
+            LOG.debug("==> RangerServiceOzone.lookupResource Context: ({})", context);
         }
         if (context != null) {
             try {
                 ret = OzoneResourceMgr.getOzoneResources(serviceName, serviceType, configs, context);
             } catch (Exception e) {
-                LOG.error("<==RangerServiceOzone.lookupResource Error : " + e);
+                LOG.error("<==RangerServiceOzone.lookupResource Error : ", e);
                 throw e;
             }
         }
         if (LOG.isDebugEnabled()) {
-            LOG.debug("<== RangerServiceOzone.lookupResource Response: (" + ret + ")");
+            LOG.debug("<== RangerServiceOzone.lookupResource Response: ({})", ret);
         }
         return ret;
     }
@@ -141,7 +141,7 @@ public List<RangerPolicy> getDefaultRangerPolicies() throws Exception {
             }
         }
         if (LOG.isDebugEnabled()) {
-            LOG.debug("<== RangerServiceOzone.getDefaultRangerPolicies() : " + ret);
+            LOG.debug("<== RangerServiceOzone.getDefaultRangerPolicies() : {}", ret);
         }
         return ret;
     }
diff --git a/plugin-ozone/src/main/java/org/apache/ranger/services/ozone/client/OzoneClient.java b/plugin-ozone/src/main/java/org/apache/ranger/services/ozone/client/OzoneClient.java
index ac497def6a..d5c4849370 100644
--- a/plugin-ozone/src/main/java/org/apache/ranger/services/ozone/client/OzoneClient.java
+++ b/plugin-ozone/src/main/java/org/apache/ranger/services/ozone/client/OzoneClient.java
@@ -80,7 +80,8 @@ public static Map<String, Object> connectionTest(String serviceName, Map<String,
                 generateResponseDataMap(true, successMsg, successMsg, null, null, responseData);
             } else {
                 String failureMsg = "Unable to retrieve any volumes using given parameters.";
-                generateResponseDataMap(false, failureMsg, failureMsg + ERR_MSG, null, null, responseData);
+                String errorMsg   = failureMsg + ERR_MSG;
+                generateResponseDataMap(false, failureMsg, errorMsg, null, null, responseData);
             }
         } finally {
             if (connectionObj != null) {
@@ -101,7 +102,7 @@ public void close() {
 
     public List<String> getVolumeList(String volumePrefix) {
         if (LOG.isDebugEnabled()) {
-            LOG.debug("==> OzoneClient getVolume volumePrefix : " + volumePrefix);
+            LOG.debug("==> OzoneClient getVolume volumePrefix : {}", volumePrefix);
         }
 
         List<String> ret = new ArrayList<>();
@@ -121,14 +122,14 @@ public List<String> getVolumeList(String volumePrefix) {
             }
         }
         if (LOG.isDebugEnabled()) {
-            LOG.debug("<== OzoneClient.getVolumeList(): " + ret);
+            LOG.debug("<== OzoneClient.getVolumeList(): {}", ret);
         }
         return ret;
     }
 
     public List<String> getBucketList(String bucketPrefix, List<String> finalVolumeList) {
         if (LOG.isDebugEnabled()) {
-            LOG.debug("==> OzoneClient getBucketList bucketPrefix : " + bucketPrefix);
+            LOG.debug("==> OzoneClient getBucketList bucketPrefix : {}", bucketPrefix);
         }
         List<String> ret = new ArrayList<>();
         try {
@@ -149,14 +150,14 @@ public List<String> getBucketList(String bucketPrefix, List<String> finalVolumeL
             }
         }
         if (LOG.isDebugEnabled()) {
-            LOG.debug("<== OzoneClient.getVolumeList(): " + ret);
+            LOG.debug("<== OzoneClient.getVolumeList(): {}", ret);
         }
         return ret;
     }
 
     public List<String> getKeyList(String keyPrefix, List<String> finalVolumeList, List<String> finalBucketList) {
         if (LOG.isDebugEnabled()) {
-            LOG.debug("==> OzoneClient getKeyList keyPrefix : " + keyPrefix);
+            LOG.debug("==> OzoneClient getKeyList keyPrefix : {}", keyPrefix);
         }
         List<String> ret = new ArrayList<>();
         try {
@@ -185,7 +186,7 @@ public List<String> getKeyList(String keyPrefix, List<String> finalVolumeList, L
             }
         }
         if (LOG.isDebugEnabled()) {
-            LOG.debug("<== OzoneClient.getVolumeList(): " + ret);
+            LOG.debug("<== OzoneClient.getVolumeList(): {}", ret);
         }
         return ret;
     }