From 1ea0a059115c08101eb5f25ae8a7a80bdec36804 Mon Sep 17 00:00:00 2001 From: Qiang Zhao Date: Sun, 15 Jan 2023 19:29:45 +0800 Subject: [PATCH] [improve][broker] Follow up #19230 to tighten the validation scope (#19234) ### Motivation This PR is following up #19230 As @yuruguo mentioned https://github.com/apache/pulsar/pull/19230#issuecomment-1382865057, we can tighten the validation scope. I've checked the logic and found we have no way to create the partition topic with the `-partition-{index}` template. So we can righten the validation scope. I will keep working on the partition topic section and try to clarify the concept and logic. Plus, ensuring compatibility. ### Modifications - tighten the validation scope (cherry picked from commit 246c2701e5c43e02e9783c82d4d107d06b019951) --- .../broker/admin/v2/PersistentTopics.java | 5 ++++ .../PartitionKeywordCompatibilityTest.java | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java index 09d6e53974b87..f5b120f67a597 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java @@ -995,6 +995,11 @@ public void deletePartitionedTopic( @QueryParam("authoritative") @DefaultValue("false") boolean authoritative) { try { validateTopicName(tenant, namespace, encodedTopic); + if (topicName.isPartitioned()) { + // There's no way to create the partition topic with `-partition-{index}`, So we can reject it. + throw new RestException(Response.Status.PRECONDITION_FAILED, + "Partitioned Topic Name should not contain '-partition-'"); + } internalDeletePartitionedTopic(asyncResponse, authoritative, force); } catch (WebApplicationException wae) { asyncResponse.resume(wae); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/PartitionKeywordCompatibilityTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/PartitionKeywordCompatibilityTest.java index f815fef1c8772..afa5231c93911 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/PartitionKeywordCompatibilityTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/PartitionKeywordCompatibilityTest.java @@ -19,6 +19,7 @@ package org.apache.pulsar.broker.service.persistent; +import static org.testng.Assert.fail; import lombok.Cleanup; import org.apache.pulsar.broker.service.BrokerTestBase; import org.apache.pulsar.client.admin.PulsarAdminException; @@ -74,4 +75,30 @@ public void testAutoCreatePartitionTopicWithKeywordAndDeleteIt() Assert.assertFalse(topics.contains(topicName)); Assert.assertFalse(partitionedTopicList.contains(topicName)); } + + @Test + public void testDeletePartitionedTopicValidation() throws PulsarAdminException { + final String topicName = "persistent://public/default/testDeletePartitionedTopicValidation"; + final String partitionKeywordTopic = "persistent://public/default/testDelete-partition-edTopicValidation"; + final String partitionedTopic = "persistent://public/default/testDeletePartitionedTopicValidation-partition-0"; + try { + admin.topics().deletePartitionedTopic(topicName); + fail("expect not found!"); + } catch (PulsarAdminException.NotFoundException ex) { + //ok + } + try { + admin.topics().deletePartitionedTopic(partitionKeywordTopic); + fail("expect not found!"); + } catch (PulsarAdminException.NotFoundException ex) { + //ok + } + try { + admin.topics().deletePartitionedTopic(partitionedTopic); + fail("expect illegal argument"); + } catch (PulsarAdminException.PreconditionFailedException ex) { + Assert.assertTrue(ex.getMessage().contains("should not contain '-partition-'")); + // ok + } + } }