[pulsar] branch branch-2.10 updated: [fix] [admin] Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication (#19879)

[yubiao]

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

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


The following commit(s) were added to refs/heads/branch-2.10 by this push:
     new db6a59b0c3a [fix] [admin] Make response code to 400 instead of 500 
when delete topic fails due to enabled geo-replication (#19879)
db6a59b0c3a is described below

commit db6a59b0c3a8f978c6e159a48da37544fb9da3a6
Author: fengyubiao <yubiao.f...@streamnative.io>
AuthorDate: Wed Mar 22 16:49:13 2023 +0800

    [fix] [admin] Make response code to 400 instead of 500 when delete topic 
fails due to enabled geo-replication (#19879)
    
    Motivation: As expected, If geo-replication is enabled, a topic cannot be 
deleted. However deleting that topic returns a 500, and no further info.
    Modifications: Make response code to 400 instead of 500 when delete topic 
fails due to enabled geo-replication
    (cherry picked from commit a9037334a399af905fae94d2aefa5db339cbd5b1)
---
 .../broker/admin/impl/PersistentTopicsBase.java    |  9 +++++--
 .../pulsar/broker/service/ReplicatorTest.java      | 28 ++++++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git 
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
 
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
index 51525303ac0..a426aa83ff3 100644
--- 
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
+++ 
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
@@ -338,7 +338,10 @@ public class PersistentTopicsBase extends AdminResource {
         try {
             pulsar().getBrokerService().deleteTopic(topicName.toString(), 
true, deleteSchema).get();
         } catch (Exception e) {
-            if (isManagedLedgerNotFoundException(e)) {
+            Throwable t = FutureUtil.unwrapCompletionException(e);
+            if (t instanceof IllegalStateException){
+                throw new RestException(422/* Unprocessable entity*/, 
t.getMessage());
+            } else if (isManagedLedgerNotFoundException(e)) {
                 log.info("[{}] Topic was already not existing {}", 
clientAppId(), topicName, e);
             } else {
                 log.error("[{}] Failed to delete topic forcefully {}", 
clientAppId(), topicName, e);
@@ -1104,7 +1107,9 @@ public class PersistentTopicsBase extends AdminResource {
         } catch (Exception e) {
             Throwable t = e.getCause();
             log.error("[{}] Failed to delete topic {}", clientAppId(), 
topicName, t);
-            if (t instanceof TopicBusyException) {
+            if (t instanceof IllegalStateException){
+                throw new RestException(422/* Unprocessable entity*/, 
t.getMessage());
+            } else if (t instanceof TopicBusyException) {
                 throw new RestException(Status.PRECONDITION_FAILED, "Topic has 
active producers/subscriptions");
             } else if (isManagedLedgerNotFoundException(e)) {
                 throw new RestException(Status.NOT_FOUND, "Topic not found");
diff --git 
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
 
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
index 2468ace10f8..d4d7c9d5e13 100644
--- 
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
+++ 
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
@@ -70,6 +70,7 @@ import 
org.apache.pulsar.broker.service.BrokerServiceException.NamingException;
 import org.apache.pulsar.broker.service.persistent.PersistentReplicator;
 import org.apache.pulsar.broker.service.persistent.PersistentTopic;
 import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
 import org.apache.pulsar.client.api.Consumer;
 import org.apache.pulsar.client.api.Message;
 import org.apache.pulsar.client.api.MessageId;
@@ -766,6 +767,33 @@ public class ReplicatorTest extends ReplicatorTestBase {
         assertNull(producer);
     }
 
+    @Test
+    public void testDeleteTopicFailure() throws Exception {
+        final String topicName = 
BrokerTestUtil.newUniqueName("persistent://pulsar/ns/tp_" + UUID.randomUUID());
+        admin1.topics().createNonPartitionedTopic(topicName);
+        try {
+            admin1.topics().delete(topicName);
+            fail("Delete topic should fail if enabled replicator");
+        } catch (Exception ex) {
+            assertTrue(ex instanceof PulsarAdminException);
+            assertEquals(((PulsarAdminException) ex).getStatusCode(), 422/* 
Unprocessable entity*/);
+        }
+    }
+
+    @Test
+    public void testDeletePartitionedTopicFailure() throws Exception {
+        final String topicName = 
BrokerTestUtil.newUniqueName("persistent://pulsar/ns/tp_" + UUID.randomUUID());
+        admin1.topics().createPartitionedTopic(topicName, 2);
+        admin1.topics().createSubscription(topicName, "sub1", 
MessageId.earliest);
+        try {
+            admin1.topics().deletePartitionedTopic(topicName);
+            fail("Delete topic should fail if enabled replicator");
+        } catch (Exception ex) {
+            assertTrue(ex instanceof PulsarAdminException);
+            assertEquals(((PulsarAdminException) ex).getStatusCode(), 422/* 
Unprocessable entity*/);
+        }
+    }
+
     @Test(priority = 4, timeOut = 30000)
     public void testReplicatorProducerName() throws Exception {
         log.info("--- Starting ReplicatorTest::testReplicatorProducerName 
---");