Skip to content

Commit 62f138a

Browse files
authored
[#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.
1 parent 683ce0c commit 62f138a

File tree

7 files changed

+86
-40
lines changed

7 files changed

+86
-40
lines changed

authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/ChainedAuthorizationProperties.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public Map<String, String> fetchAuthPluginProperties(String pluginName) {
7777
Preconditions.checkArgument(
7878
properties.containsKey(CHAIN_PLUGINS_PROPERTIES_KEY)
7979
&& properties.get(CHAIN_PLUGINS_PROPERTIES_KEY) != null,
80-
String.format("%s is required", CHAIN_PLUGINS_PROPERTIES_KEY));
80+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, CHAIN_PLUGINS_PROPERTIES_KEY));
8181

8282
String[] pluginNames = properties.get(CHAIN_PLUGINS_PROPERTIES_KEY).split(PLUGINS_SPLITTER);
8383
Preconditions.checkArgument(
@@ -113,7 +113,7 @@ public Map<String, String> fetchAuthPluginProperties(String pluginName) {
113113
public void validate() {
114114
Preconditions.checkArgument(
115115
properties.containsKey(CHAIN_PLUGINS_PROPERTIES_KEY),
116-
String.format("%s is required", CHAIN_PLUGINS_PROPERTIES_KEY));
116+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, CHAIN_PLUGINS_PROPERTIES_KEY));
117117
List<String> pluginNames =
118118
Arrays.stream(properties.get(CHAIN_PLUGINS_PROPERTIES_KEY).split(PLUGINS_SPLITTER))
119119
.map(String::trim)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.gravitino.authorization.common;
21+
22+
public class ErrorMessages {
23+
public static final String MISSING_REQUIRED_ARGUMENT = "%s is required";
24+
public static final String PRIVILEGE_NOT_SUPPORTED =
25+
"The privilege %s is not supported for the securable object: %s";
26+
public static final String OWNER_PRIVILEGE_NOT_SUPPORTED =
27+
"The owner privilege is not supported for the securable " + "object: %s";
28+
public static final String INVALID_POLICY_NAME = "The policy name (%s) is invalid!";
29+
}

authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/RangerAuthorizationProperties.java

+14-10
Original file line numberDiff line numberDiff line change
@@ -82,30 +82,34 @@ public String getPropertiesPrefix() {
8282
public void validate() {
8383
Preconditions.checkArgument(
8484
properties.containsKey(RANGER_ADMIN_URL),
85-
String.format("%s is required", RANGER_ADMIN_URL));
85+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_ADMIN_URL));
8686
Preconditions.checkArgument(
8787
properties.containsKey(RANGER_SERVICE_TYPE),
88-
String.format("%s is required", RANGER_SERVICE_TYPE));
88+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_SERVICE_TYPE));
8989
Preconditions.checkArgument(
9090
properties.containsKey(RANGER_AUTH_TYPE),
91-
String.format("%s is required", RANGER_AUTH_TYPE));
91+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_AUTH_TYPE));
9292
Preconditions.checkArgument(
93-
properties.containsKey(RANGER_USERNAME), String.format("%s is required", RANGER_USERNAME));
93+
properties.containsKey(RANGER_USERNAME),
94+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_USERNAME));
9495
Preconditions.checkArgument(
95-
properties.containsKey(RANGER_PASSWORD), String.format("%s is required", RANGER_PASSWORD));
96+
properties.containsKey(RANGER_PASSWORD),
97+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_PASSWORD));
9698
Preconditions.checkArgument(
9799
properties.get(RANGER_ADMIN_URL) != null,
98-
String.format("%s is required", RANGER_ADMIN_URL));
100+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_ADMIN_URL));
99101
Preconditions.checkArgument(
100102
properties.get(RANGER_AUTH_TYPE) != null,
101-
String.format("%s is required", RANGER_AUTH_TYPE));
103+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_AUTH_TYPE));
102104
Preconditions.checkArgument(
103-
properties.get(RANGER_USERNAME) != null, String.format("%s is required", RANGER_USERNAME));
105+
properties.get(RANGER_USERNAME) != null,
106+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_USERNAME));
104107
Preconditions.checkArgument(
105-
properties.get(RANGER_PASSWORD) != null, String.format("%s is required", RANGER_PASSWORD));
108+
properties.get(RANGER_PASSWORD) != null,
109+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_PASSWORD));
106110

107111
Preconditions.checkArgument(
108112
properties.get(RANGER_SERVICE_NAME) != null,
109-
String.format("%s is required", RANGER_SERVICE_NAME));
113+
String.format(ErrorMessages.MISSING_REQUIRED_ARGUMENT, RANGER_SERVICE_NAME));
110114
}
111115
}

authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorization.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.google.common.base.Preconditions;
2222
import java.util.Map;
23+
import org.apache.gravitino.authorization.common.ErrorMessages;
2324
import org.apache.gravitino.authorization.common.RangerAuthorizationProperties;
2425
import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
2526
import org.apache.gravitino.connector.authorization.BaseAuthorization;
@@ -36,7 +37,9 @@ public AuthorizationPlugin newPlugin(
3637
String metalake, String catalogProvider, Map<String, String> properties) {
3738
Preconditions.checkArgument(
3839
properties.containsKey(RangerAuthorizationProperties.RANGER_SERVICE_TYPE),
39-
String.format("%s is required", RangerAuthorizationProperties.RANGER_SERVICE_TYPE));
40+
String.format(
41+
ErrorMessages.MISSING_REQUIRED_ARGUMENT,
42+
RangerAuthorizationProperties.RANGER_SERVICE_TYPE));
4043
String serviceType =
4144
properties.get(RangerAuthorizationProperties.RANGER_SERVICE_TYPE).toUpperCase();
4245
switch (serviceType) {

authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHDFSPlugin.java

+14-10
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.apache.gravitino.authorization.MetadataObjectChange;
5050
import org.apache.gravitino.authorization.Privilege;
5151
import org.apache.gravitino.authorization.SecurableObject;
52+
import org.apache.gravitino.authorization.common.ErrorMessages;
5253
import org.apache.gravitino.authorization.common.PathBasedMetadataObject;
5354
import org.apache.gravitino.authorization.common.PathBasedSecurableObject;
5455
import org.apache.gravitino.authorization.common.RangerAuthorizationProperties;
@@ -475,8 +476,9 @@ public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject sec
475476
break;
476477
default:
477478
throw new AuthorizationPluginException(
478-
"The privilege %s is not supported for the securable object: %s",
479-
gravitinoPrivilege.name(), securableObject.type());
479+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
480+
gravitinoPrivilege.name(),
481+
securableObject.type());
480482
}
481483
break;
482484
case CREATE_SCHEMA:
@@ -505,8 +507,9 @@ public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject sec
505507
break;
506508
default:
507509
throw new AuthorizationPluginException(
508-
"The privilege %s is not supported for the securable object: %s",
509-
gravitinoPrivilege.name(), securableObject.type());
510+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
511+
gravitinoPrivilege.name(),
512+
securableObject.type());
510513
}
511514
break;
512515
case SELECT_TABLE:
@@ -543,14 +546,16 @@ public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject sec
543546
break;
544547
default:
545548
throw new AuthorizationPluginException(
546-
"The privilege %s is not supported for the securable object: %s",
547-
gravitinoPrivilege.name(), securableObject.type());
549+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
550+
gravitinoPrivilege.name(),
551+
securableObject.type());
548552
}
549553
break;
550554
default:
551555
throw new AuthorizationPluginException(
552-
"The privilege %s is not supported for the securable object: %s",
553-
gravitinoPrivilege.name(), securableObject.type());
556+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
557+
gravitinoPrivilege.name(),
558+
securableObject.type());
554559
}
555560
});
556561

@@ -584,8 +589,7 @@ public List<AuthorizationSecurableObject> translateOwner(MetadataObject gravitin
584589
break;
585590
default:
586591
throw new AuthorizationPluginException(
587-
"The owner privilege is not supported for the securable object: %s",
588-
gravitinoMetadataObject.type());
592+
ErrorMessages.OWNER_PRIVILEGE_NOT_SUPPORTED, gravitinoMetadataObject.type());
589593
}
590594

591595
return rangerSecurableObjects;

authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHadoopSQLPlugin.java

+21-15
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.apache.gravitino.authorization.Privilege;
4646
import org.apache.gravitino.authorization.SecurableObject;
4747
import org.apache.gravitino.authorization.SecurableObjects;
48+
import org.apache.gravitino.authorization.common.ErrorMessages;
4849
import org.apache.gravitino.authorization.common.RangerAuthorizationProperties;
4950
import org.apache.gravitino.authorization.ranger.RangerPrivileges.RangerHadoopSQLPrivilege;
5051
import org.apache.gravitino.authorization.ranger.reference.RangerDefines.PolicyResource;
@@ -229,7 +230,7 @@ protected void updatePolicyByMetadataObject(
229230
AuthorizationSecurableObject.DOT_SPLITTER.splitToList(policyName));
230231
Preconditions.checkArgument(
231232
policyNames.size() >= oldAuthzMetaObject.names().size(),
232-
String.format("The policy name (%s) is invalid!", policyName));
233+
String.format(ErrorMessages.INVALID_POLICY_NAME, policyName));
233234
if (policyNames.get(index).equals(RangerHelper.RESOURCE_ALL)) {
234235
// Doesn't need to rename the policy `*`
235236
return;
@@ -527,8 +528,7 @@ public List<AuthorizationSecurableObject> translateOwner(MetadataObject gravitin
527528
break;
528529
default:
529530
throw new AuthorizationPluginException(
530-
"The owner privilege is not supported for the securable object: %s",
531-
gravitinoMetadataObject.type());
531+
ErrorMessages.OWNER_PRIVILEGE_NOT_SUPPORTED, gravitinoMetadataObject.type());
532532
}
533533

534534
return rangerSecurableObjects;
@@ -574,8 +574,9 @@ public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject sec
574574
break;
575575
default:
576576
throw new AuthorizationPluginException(
577-
"The privilege %s is not supported for the securable object: %s",
578-
gravitinoPrivilege.name(), securableObject.type());
577+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
578+
gravitinoPrivilege.name(),
579+
securableObject.type());
579580
}
580581
break;
581582
case CREATE_SCHEMA:
@@ -592,8 +593,9 @@ public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject sec
592593
break;
593594
default:
594595
throw new AuthorizationPluginException(
595-
"The privilege %s is not supported for the securable object: %s",
596-
gravitinoPrivilege.name(), securableObject.type());
596+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
597+
gravitinoPrivilege.name(),
598+
securableObject.type());
597599
}
598600
break;
599601
case USE_SCHEMA:
@@ -619,8 +621,9 @@ public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject sec
619621
break;
620622
default:
621623
throw new AuthorizationPluginException(
622-
"The privilege %s is not supported for the securable object: %s",
623-
gravitinoPrivilege.name(), securableObject.type());
624+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
625+
gravitinoPrivilege.name(),
626+
securableObject.type());
624627
}
625628
break;
626629
case CREATE_TABLE:
@@ -672,8 +675,9 @@ public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject sec
672675
case TABLE:
673676
if (gravitinoPrivilege.name() == Privilege.Name.CREATE_TABLE) {
674677
throw new AuthorizationPluginException(
675-
"The privilege %s is not supported for the securable object: %s",
676-
gravitinoPrivilege.name(), securableObject.type());
678+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
679+
gravitinoPrivilege.name(),
680+
securableObject.type());
677681
} else {
678682
translateMetadataObject(securableObject).stream()
679683
.forEach(
@@ -700,14 +704,16 @@ public List<AuthorizationSecurableObject> translatePrivilege(SecurableObject sec
700704
break;
701705
default:
702706
LOG.warn(
703-
"The privilege %s is not supported for the securable object: %s",
704-
gravitinoPrivilege.name(), securableObject.type());
707+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
708+
gravitinoPrivilege.name(),
709+
securableObject.type());
705710
}
706711
break;
707712
default:
708713
LOG.warn(
709-
"The privilege %s is not supported for the securable object: %s",
710-
gravitinoPrivilege.name(), securableObject.type());
714+
ErrorMessages.PRIVILEGE_NOT_SUPPORTED,
715+
gravitinoPrivilege.name(),
716+
securableObject.type());
711717
}
712718
});
713719

authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.apache.gravitino.authorization.RoleChange;
4545
import org.apache.gravitino.authorization.SecurableObject;
4646
import org.apache.gravitino.authorization.User;
47+
import org.apache.gravitino.authorization.common.ErrorMessages;
4748
import org.apache.gravitino.authorization.common.RangerAuthorizationProperties;
4849
import org.apache.gravitino.authorization.ranger.reference.VXGroup;
4950
import org.apache.gravitino.authorization.ranger.reference.VXGroupList;
@@ -574,8 +575,7 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n
574575
break;
575576
default:
576577
throw new AuthorizationPluginException(
577-
"The owner privilege is not supported for the securable object: %s",
578-
metadataObject.type());
578+
ErrorMessages.OWNER_PRIVILEGE_NOT_SUPPORTED, metadataObject.type());
579579
}
580580

581581
return Boolean.TRUE;

0 commit comments

Comments
 (0)