This is an automated email from the ASF dual-hosted git repository. yubiao pushed a commit to branch branch-2.9 in repository https://gitbox.apache.org/repos/asf/pulsar.git
commit 4e2105ab69b5621e2553985a58364b17698733e5 Author: fengyubiao <[email protected]> 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 | 30 +++++++++++++++++++++- 2 files changed, 36 insertions(+), 3 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 e8ac4bb457d..a559ccb06d5 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 @@ -314,7 +314,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); @@ -1040,7 +1043,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, t.getMessage()); } 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 90ecff200f7..e03a0c1acb9 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 @@ -64,6 +64,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; @@ -860,7 +861,34 @@ public class ReplicatorTest extends ReplicatorTestBase { assertNull(producer); } - @Test(priority = 5, timeOut = 30000) + @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 ---"); final String topicName = BrokerTestUtil.newUniqueName("persistent://pulsar/ns/testReplicatorProducerName");
