Skip to content

Commit

Permalink
[fix] [admin] Make response code to 400 instead of 500 when delete to…
Browse files Browse the repository at this point in the history
…pic fails due to enabled geo-replication (apache#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
  • Loading branch information
poorbarcode committed Mar 22, 2023
1 parent f294be3 commit a903733
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
Expand Up @@ -1106,7 +1106,9 @@ public void deleteTopic(
ex = new RestException(Response.Status.PRECONDITION_FAILED,
t.getMessage());
}
if (isManagedLedgerNotFoundException(t)) {
if (t instanceof IllegalStateException){
ex = new RestException(422/* Unprocessable entity*/, t.getMessage());
} else if (isManagedLedgerNotFoundException(t)) {
ex = new RestException(Response.Status.NOT_FOUND,
getTopicNotFoundErrorMessage(topicName.toString()));
} else if (!isRedirectException(ex)) {
Expand Down
Expand Up @@ -70,6 +70,7 @@
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;
Expand Down Expand Up @@ -869,6 +870,33 @@ public void testReplicatorProducerClosing() throws Exception {
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 ---");
Expand Down

0 comments on commit a903733

Please sign in to comment.