You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by pe...@apache.org on 2022/04/04 12:33:14 UTC

[pulsar] 05/15: Provide an accurate error message when set ``autoTopicCreation `` (#14684)

This is an automated email from the ASF dual-hosted git repository.

penghui pushed a commit to branch branch-2.8
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit 365152f27c8fdb8d1ea8ef96265c89d228db55e6
Author: Qiang Zhao <74...@users.noreply.github.com>
AuthorDate: Thu Mar 17 19:47:50 2022 +0800

    Provide an accurate error message when set ``autoTopicCreation `` (#14684)
    
    (cherry picked from commit 50a7e50745e5c26200a4abd2e63e3750e34339fb)
---
 .../pulsar/broker/admin/impl/NamespacesBase.java   |  7 ++--
 .../common/policies/data/ValidateResult.java       | 40 ++++++++++++++++++++++
 .../data/impl/AutoTopicCreationOverrideImpl.java   | 18 +++++-----
 .../data/AutoTopicCreationOverrideTest.java        | 12 +++----
 4 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
index e91314d11d6..264e800b715 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
@@ -99,6 +99,7 @@ import org.apache.pulsar.common.policies.data.SchemaCompatibilityStrategy;
 import org.apache.pulsar.common.policies.data.SubscribeRate;
 import org.apache.pulsar.common.policies.data.SubscriptionAuthMode;
 import org.apache.pulsar.common.policies.data.TenantOperation;
+import org.apache.pulsar.common.policies.data.ValidateResult;
 import org.apache.pulsar.common.policies.data.impl.AutoTopicCreationOverrideImpl;
 import org.apache.pulsar.common.policies.data.impl.DispatchRateImpl;
 import org.apache.pulsar.common.util.FutureUtil;
@@ -834,9 +835,11 @@ public abstract class NamespacesBase extends AdminResource {
         validateNamespacePolicyOperation(namespaceName, PolicyName.AUTO_TOPIC_CREATION, PolicyOperation.WRITE);
         validatePoliciesReadOnlyAccess();
         if (autoTopicCreationOverride != null) {
-            if (!AutoTopicCreationOverrideImpl.isValidOverride(autoTopicCreationOverride)) {
+            ValidateResult validateResult = AutoTopicCreationOverrideImpl.validateOverride(autoTopicCreationOverride);
+            if (!validateResult.isSuccess()) {
                 throw new RestException(Status.PRECONDITION_FAILED,
-                        "Invalid configuration for autoTopicCreationOverride");
+                        "Invalid configuration for autoTopicCreationOverride. the detail is "
+                                + validateResult.getErrorInfo());
             }
             if (maxPartitions > 0 && autoTopicCreationOverride.getDefaultNumPartitions() > maxPartitions) {
                 throw new RestException(Status.NOT_ACCEPTABLE,
diff --git a/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ValidateResult.java b/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ValidateResult.java
new file mode 100644
index 00000000000..13821947cf2
--- /dev/null
+++ b/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ValidateResult.java
@@ -0,0 +1,40 @@
+/**
+ * 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.pulsar.common.policies.data;
+
+import lombok.Getter;
+
+@Getter
+public class ValidateResult {
+    private final boolean success;
+    private final String errorInfo;
+
+    private ValidateResult(boolean success, String errorInfo) {
+        this.success = success;
+        this.errorInfo = errorInfo;
+    }
+
+    public static ValidateResult fail(String errorInfo) {
+        return new ValidateResult(false, errorInfo);
+    }
+
+    public static ValidateResult success() {
+        return new ValidateResult(true, null);
+    }
+}
diff --git a/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/AutoTopicCreationOverrideImpl.java b/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/AutoTopicCreationOverrideImpl.java
index 1ce60d1c78b..ba6bc07780a 100644
--- a/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/AutoTopicCreationOverrideImpl.java
+++ b/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/impl/AutoTopicCreationOverrideImpl.java
@@ -23,6 +23,7 @@ import lombok.Data;
 import lombok.NoArgsConstructor;
 import org.apache.pulsar.common.policies.data.AutoTopicCreationOverride;
 import org.apache.pulsar.common.policies.data.TopicType;
+import org.apache.pulsar.common.policies.data.ValidateResult;
 
 /**
  * Override of autoTopicCreation settings on a namespace level.
@@ -35,28 +36,29 @@ public final class AutoTopicCreationOverrideImpl implements AutoTopicCreationOve
     private String topicType;
     private Integer defaultNumPartitions;
 
-    public static boolean isValidOverride(AutoTopicCreationOverride override) {
+    public static ValidateResult validateOverride(AutoTopicCreationOverride override) {
         if (override == null) {
-            return false;
+            return ValidateResult.fail("[AutoTopicCreationOverride] can not be null");
         }
         if (override.isAllowAutoTopicCreation()) {
             if (!TopicType.isValidTopicType(override.getTopicType())) {
-                return false;
+                return ValidateResult.fail(String.format("Unknown topic type [%s]", override.getTopicType()));
             }
             if (TopicType.PARTITIONED.toString().equals(override.getTopicType())) {
                 if (override.getDefaultNumPartitions() == null) {
-                    return false;
+                    return ValidateResult.fail("[defaultNumPartitions] cannot be null when the type is partitioned.");
                 }
-                if (!(override.getDefaultNumPartitions() > 0)) {
-                    return false;
+                if (override.getDefaultNumPartitions() <= 0) {
+                    return ValidateResult.fail("[defaultNumPartitions] cannot be less than 1 for partition type.");
                 }
             } else if (TopicType.NON_PARTITIONED.toString().equals(override.getTopicType())) {
                 if (override.getDefaultNumPartitions() != null) {
-                    return false;
+                    return ValidateResult.fail("[defaultNumPartitions] is not allowed to be"
+                            + " set when the type is non-partition.");
                 }
             }
         }
-        return true;
+        return ValidateResult.success();
     }
 
     public static AutoTopicCreationOverrideImplBuilder builder() {
diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/AutoTopicCreationOverrideTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/AutoTopicCreationOverrideTest.java
index 5092d433d0d..66769f0bfbc 100644
--- a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/AutoTopicCreationOverrideTest.java
+++ b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/AutoTopicCreationOverrideTest.java
@@ -32,7 +32,7 @@ public class AutoTopicCreationOverrideTest {
                 .allowAutoTopicCreation(true)
                 .topicType(TopicType.NON_PARTITIONED.toString())
                 .build();
-        assertTrue(AutoTopicCreationOverrideImpl.isValidOverride(override));
+        assertTrue(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
     }
 
     @Test
@@ -42,7 +42,7 @@ public class AutoTopicCreationOverrideTest {
                 .topicType(TopicType.PARTITIONED.toString())
                 .defaultNumPartitions(2)
                 .build();
-        assertTrue(AutoTopicCreationOverrideImpl.isValidOverride(override));
+        assertTrue(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
     }
 
     @Test
@@ -51,7 +51,7 @@ public class AutoTopicCreationOverrideTest {
                 .allowAutoTopicCreation(true)
                 .topicType("aaa")
                 .build();
-        assertFalse(AutoTopicCreationOverrideImpl.isValidOverride(override));
+        assertFalse(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
     }
 
     @Test
@@ -61,7 +61,7 @@ public class AutoTopicCreationOverrideTest {
                 .topicType(TopicType.PARTITIONED.toString())
                 .defaultNumPartitions(0)
                 .build();
-        assertFalse(AutoTopicCreationOverrideImpl.isValidOverride(override));
+        assertFalse(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
     }
 
     @Test
@@ -70,7 +70,7 @@ public class AutoTopicCreationOverrideTest {
                 .allowAutoTopicCreation(true)
                 .topicType(TopicType.PARTITIONED.toString())
                 .build();
-        assertFalse(AutoTopicCreationOverrideImpl.isValidOverride(override));
+        assertFalse(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
     }
 
     @Test
@@ -80,6 +80,6 @@ public class AutoTopicCreationOverrideTest {
                 .topicType(TopicType.NON_PARTITIONED.toString())
                 .defaultNumPartitions(2)
                 .build();
-        assertFalse(AutoTopicCreationOverrideImpl.isValidOverride(override));
+        assertFalse(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
     }
 }