Skip to content

Commit bc25e88

Browse files
JingsongLiJingsongLi
committed
[hotfix] Refactor for REST and rollback
1 parent 744ea92 commit bc25e88

File tree

4 files changed

+54
-40
lines changed

4 files changed

+54
-40
lines changed

‎paimon-api/src/main/java/org/apache/paimon/rest/RESTApi.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.apache.paimon.annotation.VisibleForTesting;
2525
import org.apache.paimon.catalog.Identifier;
2626
import org.apache.paimon.function.FunctionChange;
27-
import org.apache.paimon.function.FunctionNameValidator;
2827
import org.apache.paimon.options.Options;
2928
import org.apache.paimon.partition.Partition;
3029
import org.apache.paimon.partition.PartitionStatistics;
@@ -96,6 +95,8 @@
9695

9796
import static java.util.Collections.emptyList;
9897
import static org.apache.paimon.options.CatalogOptions.WAREHOUSE;
98+
import static org.apache.paimon.rest.RESTFunctionValidator.checkFunctionName;
99+
import static org.apache.paimon.rest.RESTFunctionValidator.isValidFunctionName;
99100
import static org.apache.paimon.rest.RESTUtil.extractPrefixMap;
100101
import static org.apache.paimon.rest.auth.AuthProviderFactory.createAuthProvider;
101102

@@ -853,17 +854,16 @@ public List<String> listFunctions(String databaseName) {
853854
* @throws ForbiddenException if the user lacks permission to access the function
854855
*/
855856
public GetFunctionResponse getFunction(Identifier identifier) {
856-
if (FunctionNameValidator.isValidName(identifier.getObjectName())) {
857-
return client.get(
858-
resourcePaths.function(
859-
identifier.getDatabaseName(), identifier.getObjectName()),
860-
GetFunctionResponse.class,
861-
restAuthFunction);
857+
if (!isValidFunctionName(identifier.getObjectName())) {
858+
throw new NoSuchResourceException(
859+
ErrorResponse.RESOURCE_TYPE_FUNCTION,
860+
identifier.getObjectName(),
861+
"Invalid function name: " + identifier.getObjectName());
862862
}
863-
throw new NoSuchResourceException(
864-
ErrorResponse.RESOURCE_TYPE_FUNCTION,
865-
identifier.getObjectName(),
866-
"Invalid function name: " + identifier.getObjectName());
863+
return client.get(
864+
resourcePaths.function(identifier.getDatabaseName(), identifier.getObjectName()),
865+
GetFunctionResponse.class,
866+
restAuthFunction);
867867
}
868868

869869
/**
@@ -877,7 +877,7 @@ public GetFunctionResponse getFunction(Identifier identifier) {
877877
*/
878878
public void createFunction(
879879
Identifier identifier, org.apache.paimon.function.Function function) {
880-
FunctionNameValidator.check(identifier.getObjectName());
880+
checkFunctionName(identifier.getObjectName());
881881
client.post(
882882
resourcePaths.functions(identifier.getDatabaseName()),
883883
new CreateFunctionRequest(function),
@@ -893,7 +893,7 @@ public void createFunction(
893893
* this function
894894
*/
895895
public void dropFunction(Identifier identifier) {
896-
FunctionNameValidator.check(identifier.getObjectName());
896+
checkFunctionName(identifier.getObjectName());
897897
client.delete(
898898
resourcePaths.function(identifier.getDatabaseName(), identifier.getObjectName()),
899899
restAuthFunction);
@@ -908,7 +908,7 @@ public void dropFunction(Identifier identifier) {
908908
* @throws ForbiddenException if the user lacks permission to modify the function
909909
*/
910910
public void alterFunction(Identifier identifier, List<FunctionChange> changes) {
911-
FunctionNameValidator.check(identifier.getObjectName());
911+
checkFunctionName(identifier.getObjectName());
912912
client.post(
913913
resourcePaths.function(identifier.getDatabaseName(), identifier.getObjectName()),
914914
new AlterFunctionRequest(changes),

paimon-api/src/main/java/org/apache/paimon/function/FunctionNameValidator.java renamed to paimon-api/src/main/java/org/apache/paimon/rest/RESTFunctionValidator.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,23 @@
1616
* limitations under the License.
1717
*/
1818

19-
package org.apache.paimon.function;
19+
package org.apache.paimon.rest;
2020

2121
import java.util.regex.Pattern;
2222

2323
/** Validator for function name. */
24-
public class FunctionNameValidator {
24+
public class RESTFunctionValidator {
25+
2526
private static final Pattern FUNCTION_NAME_PATTERN =
2627
Pattern.compile("^(?=.*[A-Za-z])[A-Za-z0-9._-]+$");
2728

28-
public static boolean isValidName(String name) {
29+
public static boolean isValidFunctionName(String name) {
2930
return org.apache.paimon.utils.StringUtils.isNotEmpty(name)
3031
&& FUNCTION_NAME_PATTERN.matcher(name).matches();
3132
}
3233

33-
public static void check(String name) {
34-
boolean isValid = isValidName(name);
34+
public static void checkFunctionName(String name) {
35+
boolean isValid = isValidFunctionName(name);
3536
if (!isValid) {
3637
throw new IllegalArgumentException("Invalid function name: " + name);
3738
}

‎paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -503,14 +503,13 @@ public void rollbackTo(long snapshotId) {
503503
SnapshotManager snapshotManager = snapshotManager();
504504
try {
505505
snapshotManager.rollback(Instant.snapshot(snapshotId));
506-
return;
507-
} catch (UnsupportedOperationException ignore) {
506+
} catch (UnsupportedOperationException e) {
507+
checkArgument(
508+
snapshotManager.snapshotExists(snapshotId),
509+
"Rollback snapshot '%s' doesn't exist.",
510+
snapshotId);
511+
rollbackHelper().updateLatestAndCleanLargerThan(snapshotManager.snapshot(snapshotId));
508512
}
509-
checkArgument(
510-
snapshotManager.snapshotExists(snapshotId),
511-
"Rollback snapshot '%s' doesn't exist.",
512-
snapshotId);
513-
rollbackHelper().updateLatestAndCleanLargerThan(snapshotManager.snapshot(snapshotId));
514513
}
515514

516515
public Snapshot findSnapshot(long fromSnapshotId) throws SnapshotNotExistException {

‎paimon-core/src/test/java/org/apache/paimon/rest/RESTCatalogTest.java‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.apache.paimon.function.Function;
3232
import org.apache.paimon.function.FunctionChange;
3333
import org.apache.paimon.function.FunctionDefinition;
34-
import org.apache.paimon.function.FunctionNameValidator;
3534
import org.apache.paimon.options.Options;
3635
import org.apache.paimon.partition.Partition;
3736
import org.apache.paimon.partition.PartitionStatistics;
@@ -1707,19 +1706,34 @@ void testAlterFunction() throws Exception {
17071706

17081707
@Test
17091708
public void testValidateFunctionName() throws Exception {
1710-
assertDoesNotThrow(() -> FunctionNameValidator.check("a"));
1711-
assertDoesNotThrow(() -> FunctionNameValidator.check("a1_"));
1712-
assertDoesNotThrow(() -> FunctionNameValidator.check("a-b_c"));
1713-
assertDoesNotThrow(() -> FunctionNameValidator.check("a-b_c.1"));
1714-
1715-
assertThrows(IllegalArgumentException.class, () -> FunctionNameValidator.check("a\\/b"));
1716-
assertThrows(IllegalArgumentException.class, () -> FunctionNameValidator.check("a$?b"));
1717-
assertThrows(IllegalArgumentException.class, () -> FunctionNameValidator.check("a@b"));
1718-
assertThrows(IllegalArgumentException.class, () -> FunctionNameValidator.check("a*b"));
1719-
assertThrows(IllegalArgumentException.class, () -> FunctionNameValidator.check("123"));
1720-
assertThrows(IllegalArgumentException.class, () -> FunctionNameValidator.check("_-"));
1721-
assertThrows(IllegalArgumentException.class, () -> FunctionNameValidator.check(""));
1722-
assertThrows(IllegalArgumentException.class, () -> FunctionNameValidator.check(null));
1709+
assertDoesNotThrow(() -> RESTFunctionValidator.checkFunctionName("a"));
1710+
assertDoesNotThrow(() -> RESTFunctionValidator.checkFunctionName("a1_"));
1711+
assertDoesNotThrow(() -> RESTFunctionValidator.checkFunctionName("a-b_c"));
1712+
assertDoesNotThrow(() -> RESTFunctionValidator.checkFunctionName("a-b_c.1"));
1713+
1714+
assertThrows(
1715+
IllegalArgumentException.class,
1716+
() -> RESTFunctionValidator.checkFunctionName("a\\/b"));
1717+
assertThrows(
1718+
IllegalArgumentException.class,
1719+
() -> RESTFunctionValidator.checkFunctionName("a$?b"));
1720+
assertThrows(
1721+
IllegalArgumentException.class,
1722+
() -> RESTFunctionValidator.checkFunctionName("a@b"));
1723+
assertThrows(
1724+
IllegalArgumentException.class,
1725+
() -> RESTFunctionValidator.checkFunctionName("a*b"));
1726+
assertThrows(
1727+
IllegalArgumentException.class,
1728+
() -> RESTFunctionValidator.checkFunctionName("123"));
1729+
assertThrows(
1730+
IllegalArgumentException.class,
1731+
() -> RESTFunctionValidator.checkFunctionName("_-"));
1732+
assertThrows(
1733+
IllegalArgumentException.class, () -> RESTFunctionValidator.checkFunctionName(""));
1734+
assertThrows(
1735+
IllegalArgumentException.class,
1736+
() -> RESTFunctionValidator.checkFunctionName(null));
17231737
}
17241738

17251739
@Test

0 commit comments

Comments
 (0)