This is an automated email from the ASF dual-hosted git repository.
jmclean pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 62f138a62d [#6530] improvement(authorization): Reduce repeated strings
in Ranger classes (#6685)
62f138a62d is described below
commit 62f138a62d2888d0c2f8dd6f7cd63eb677f2c728
Author: Lord of Abyss <[email protected]>
AuthorDate: Thu Mar 13 15:07:29 2025 +0800
[#6530] improvement(authorization): Reduce repeated strings in Ranger
classes (#6685)
### What changes were proposed in this pull request?
Reduce repeated strings in Ranger classes, abstract the error message
and make them static.
### Why are the changes needed?
Fix: #6530
### Does this PR introduce _any_ user-facing change?
no.
### How was this patch tested?
local test.
---
.../common/ChainedAuthorizationProperties.java | 4 +--
.../authorization/common/ErrorMessages.java | 29 +++++++++++++++++
.../common/RangerAuthorizationProperties.java | 24 +++++++++------
.../authorization/ranger/RangerAuthorization.java | 5 ++-
.../ranger/RangerAuthorizationHDFSPlugin.java | 24 +++++++++------
.../ranger/RangerAuthorizationHadoopSQLPlugin.java | 36 +++++++++++++---------
.../ranger/RangerAuthorizationPlugin.java | 4 +--
7 files changed, 86 insertions(+), 40 deletions(-)
diff --git
a/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/ChainedAuthorizationProperties.java
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/ChainedAuthorizationProperties.java
index 7e5aea0ca2..410f068f3c 100644
---
a/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/ChainedAuthorizationProperties.java
+++
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/ChainedAuthorizationProperties.java
@@ -77,7 +77,7 @@ public class ChainedAuthorizationProperties extends
AuthorizationProperties {
Preconditions.checkArgument(
properties.containsKey(CHAIN_PLUGINS_PROPERTIES_KEY)
&& properties.get(CHAIN_PLUGINS_PROPERTIES_KEY) != null,
- String.format("%s is required", CHAIN_PLUGINS_PROPERTIES_KEY));
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
CHAIN_PLUGINS_PROPERTIES_KEY));
String[] pluginNames =
properties.get(CHAIN_PLUGINS_PROPERTIES_KEY).split(PLUGINS_SPLITTER);
Preconditions.checkArgument(
@@ -113,7 +113,7 @@ public class ChainedAuthorizationProperties extends
AuthorizationProperties {
public void validate() {
Preconditions.checkArgument(
properties.containsKey(CHAIN_PLUGINS_PROPERTIES_KEY),
- String.format("%s is required", CHAIN_PLUGINS_PROPERTIES_KEY));
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
CHAIN_PLUGINS_PROPERTIES_KEY));
List<String> pluginNames =
Arrays.stream(properties.get(CHAIN_PLUGINS_PROPERTIES_KEY).split(PLUGINS_SPLITTER))
.map(String::trim)
diff --git
a/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/ErrorMessages.java
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/ErrorMessages.java
new file mode 100644
index 0000000000..51014fe313
--- /dev/null
+++
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/ErrorMessages.java
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.authorization.common;
+
+public class ErrorMessages {
+ public static final String MISSING_REQUIRED_ARGUMENT = "%s is required";
+ public static final String PRIVILEGE_NOT_SUPPORTED =
+ "The privilege %s is not supported for the securable object: %s";
+ public static final String OWNER_PRIVILEGE_NOT_SUPPORTED =
+ "The owner privilege is not supported for the securable " + "object: %s";
+ public static final String INVALID_POLICY_NAME = "The policy name (%s) is
invalid!";
+}
diff --git
a/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/RangerAuthorizationProperties.java
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/RangerAuthorizationProperties.java
index 71cf686efa..3b7b9c0cc5 100644
---
a/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/RangerAuthorizationProperties.java
+++
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/RangerAuthorizationProperties.java
@@ -82,30 +82,34 @@ public class RangerAuthorizationProperties extends
AuthorizationProperties {
public void validate() {
Preconditions.checkArgument(
properties.containsKey(RANGER_ADMIN_URL),
- String.format("%s is required", RANGER_ADMIN_URL));
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_ADMIN_URL));
Preconditions.checkArgument(
properties.containsKey(RANGER_SERVICE_TYPE),
- String.format("%s is required", RANGER_SERVICE_TYPE));
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_SERVICE_TYPE));
Preconditions.checkArgument(
properties.containsKey(RANGER_AUTH_TYPE),
- String.format("%s is required", RANGER_AUTH_TYPE));
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_AUTH_TYPE));
Preconditions.checkArgument(
- properties.containsKey(RANGER_USERNAME), String.format("%s is
required", RANGER_USERNAME));
+ properties.containsKey(RANGER_USERNAME),
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_USERNAME));
Preconditions.checkArgument(
- properties.containsKey(RANGER_PASSWORD), String.format("%s is
required", RANGER_PASSWORD));
+ properties.containsKey(RANGER_PASSWORD),
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_PASSWORD));
Preconditions.checkArgument(
properties.get(RANGER_ADMIN_URL) != null,
- String.format("%s is required", RANGER_ADMIN_URL));
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_ADMIN_URL));
Preconditions.checkArgument(
properties.get(RANGER_AUTH_TYPE) != null,
- String.format("%s is required", RANGER_AUTH_TYPE));
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_AUTH_TYPE));
Preconditions.checkArgument(
- properties.get(RANGER_USERNAME) != null, String.format("%s is
required", RANGER_USERNAME));
+ properties.get(RANGER_USERNAME) != null,
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_USERNAME));
Preconditions.checkArgument(
- properties.get(RANGER_PASSWORD) != null, String.format("%s is
required", RANGER_PASSWORD));
+ properties.get(RANGER_PASSWORD) != null,
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_PASSWORD));
Preconditions.checkArgument(
properties.get(RANGER_SERVICE_NAME) != null,
- String.format("%s is required", RANGER_SERVICE_NAME));
+ String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT,
RANGER_SERVICE_NAME));
}
}
diff --git
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorization.java
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorization.java
index b179b94c02..7a4c18926c 100644
---
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorization.java
+++
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorization.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.authorization.ranger;
import com.google.common.base.Preconditions;
import java.util.Map;
+import org.apache.gravitino.authorization.common.ErrorMessages;
import org.apache.gravitino.authorization.common.RangerAuthorizationProperties;
import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
import org.apache.gravitino.connector.authorization.BaseAuthorization;
@@ -36,7 +37,9 @@ public class RangerAuthorization extends
BaseAuthorization<RangerAuthorization>
String metalake, String catalogProvider, Map<String, String> properties)
{
Preconditions.checkArgument(
properties.containsKey(RangerAuthorizationProperties.RANGER_SERVICE_TYPE),
- String.format("%s is required",
RangerAuthorizationProperties.RANGER_SERVICE_TYPE));
+ String.format(
+ ErrorMessages.MISSING_REQUIRED_ARGUMENT,
+ RangerAuthorizationProperties.RANGER_SERVICE_TYPE));
String serviceType =
properties.get(RangerAuthorizationProperties.RANGER_SERVICE_TYPE).toUpperCase();
switch (serviceType) {
diff --git
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHDFSPlugin.java
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHDFSPlugin.java
index e5c8434970..9f2fb01eec 100644
---
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHDFSPlugin.java
+++
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHDFSPlugin.java
@@ -49,6 +49,7 @@ import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.MetadataObjectChange;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.authorization.SecurableObject;
+import org.apache.gravitino.authorization.common.ErrorMessages;
import org.apache.gravitino.authorization.common.PathBasedMetadataObject;
import org.apache.gravitino.authorization.common.PathBasedSecurableObject;
import org.apache.gravitino.authorization.common.RangerAuthorizationProperties;
@@ -475,8 +476,9 @@ public class RangerAuthorizationHDFSPlugin extends
RangerAuthorizationPlugin {
break;
default:
throw new AuthorizationPluginException(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
break;
case CREATE_SCHEMA:
@@ -505,8 +507,9 @@ public class RangerAuthorizationHDFSPlugin extends
RangerAuthorizationPlugin {
break;
default:
throw new AuthorizationPluginException(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
break;
case SELECT_TABLE:
@@ -543,14 +546,16 @@ public class RangerAuthorizationHDFSPlugin extends
RangerAuthorizationPlugin {
break;
default:
throw new AuthorizationPluginException(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
break;
default:
throw new AuthorizationPluginException(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
});
@@ -584,8 +589,7 @@ public class RangerAuthorizationHDFSPlugin extends
RangerAuthorizationPlugin {
break;
default:
throw new AuthorizationPluginException(
- "The owner privilege is not supported for the securable object:
%s",
- gravitinoMetadataObject.type());
+ ErrorMessages.OWNER_PRIVILEGE_NOT_SUPPORTED,
gravitinoMetadataObject.type());
}
return rangerSecurableObjects;
diff --git
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHadoopSQLPlugin.java
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHadoopSQLPlugin.java
index bef0562f19..d519dd2e52 100644
---
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHadoopSQLPlugin.java
+++
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHadoopSQLPlugin.java
@@ -45,6 +45,7 @@ import
org.apache.gravitino.authorization.MetadataObjectChange;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.SecurableObjects;
+import org.apache.gravitino.authorization.common.ErrorMessages;
import org.apache.gravitino.authorization.common.RangerAuthorizationProperties;
import
org.apache.gravitino.authorization.ranger.RangerPrivileges.RangerHadoopSQLPrivilege;
import
org.apache.gravitino.authorization.ranger.reference.RangerDefines.PolicyResource;
@@ -229,7 +230,7 @@ public class RangerAuthorizationHadoopSQLPlugin extends
RangerAuthorizationPlugi
AuthorizationSecurableObject.DOT_SPLITTER.splitToList(policyName));
Preconditions.checkArgument(
policyNames.size() >= oldAuthzMetaObject.names().size(),
- String.format("The policy name (%s) is invalid!",
policyName));
+ String.format(ErrorMessages.INVALID_POLICY_NAME,
policyName));
if
(policyNames.get(index).equals(RangerHelper.RESOURCE_ALL)) {
// Doesn't need to rename the policy `*`
return;
@@ -527,8 +528,7 @@ public class RangerAuthorizationHadoopSQLPlugin extends
RangerAuthorizationPlugi
break;
default:
throw new AuthorizationPluginException(
- "The owner privilege is not supported for the securable object:
%s",
- gravitinoMetadataObject.type());
+ ErrorMessages.OWNER_PRIVILEGE_NOT_SUPPORTED,
gravitinoMetadataObject.type());
}
return rangerSecurableObjects;
@@ -574,8 +574,9 @@ public class RangerAuthorizationHadoopSQLPlugin extends
RangerAuthorizationPlugi
break;
default:
throw new AuthorizationPluginException(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
break;
case CREATE_SCHEMA:
@@ -592,8 +593,9 @@ public class RangerAuthorizationHadoopSQLPlugin extends
RangerAuthorizationPlugi
break;
default:
throw new AuthorizationPluginException(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
break;
case USE_SCHEMA:
@@ -619,8 +621,9 @@ public class RangerAuthorizationHadoopSQLPlugin extends
RangerAuthorizationPlugi
break;
default:
throw new AuthorizationPluginException(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
break;
case CREATE_TABLE:
@@ -672,8 +675,9 @@ public class RangerAuthorizationHadoopSQLPlugin extends
RangerAuthorizationPlugi
case TABLE:
if (gravitinoPrivilege.name() ==
Privilege.Name.CREATE_TABLE) {
throw new AuthorizationPluginException(
- "The privilege %s is not supported for the
securable object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
} else {
translateMetadataObject(securableObject).stream()
.forEach(
@@ -700,14 +704,16 @@ public class RangerAuthorizationHadoopSQLPlugin extends
RangerAuthorizationPlugi
break;
default:
LOG.warn(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
break;
default:
LOG.warn(
- "The privilege %s is not supported for the securable
object: %s",
- gravitinoPrivilege.name(), securableObject.type());
+ ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
+ gravitinoPrivilege.name(),
+ securableObject.type());
}
});
diff --git
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
index b7189ba6a6..e1a8c21c4b 100644
---
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
+++
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
@@ -44,6 +44,7 @@ import org.apache.gravitino.authorization.Role;
import org.apache.gravitino.authorization.RoleChange;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.User;
+import org.apache.gravitino.authorization.common.ErrorMessages;
import org.apache.gravitino.authorization.common.RangerAuthorizationProperties;
import org.apache.gravitino.authorization.ranger.reference.VXGroup;
import org.apache.gravitino.authorization.ranger.reference.VXGroupList;
@@ -574,8 +575,7 @@ public abstract class RangerAuthorizationPlugin
break;
default:
throw new AuthorizationPluginException(
- "The owner privilege is not supported for the securable object:
%s",
- metadataObject.type());
+ ErrorMessages.OWNER_PRIVILEGE_NOT_SUPPORTED,
metadataObject.type());
}
return Boolean.TRUE;
