You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by bo...@apache.org on 2022/11/11 03:52:17 UTC
[pulsar] branch branch-2.9 updated: [Broker] Fix NPE of internalExpireMessagesByTimestamp (#14243)
This is an automated email from the ASF dual-hosted git repository.
bogong pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.9 by this push:
new 7d405c3518a [Broker] Fix NPE of internalExpireMessagesByTimestamp (#14243)
7d405c3518a is described below
commit 7d405c3518a0c16e2e1ae95bee3bdabe29c951a8
Author: Zixuan Liu <no...@gmail.com>
AuthorDate: Mon Feb 14 11:22:05 2022 +0800
[Broker] Fix NPE of internalExpireMessagesByTimestamp (#14243)
This issue was triggered by #13880, it doesn't handle the NPE in [here](https://github.com/apache/pulsar/pull/13880/files#diff-66aeb65a64cbe7c541f013ae807c5bcbeab567bef77706c7ff2e0cbfe0d77eb1R3502), we can check the [diff code](https://github.com/apache/pulsar/pull/13880/files#diff-66aeb65a64cbe7c541f013ae807c5bcbeab567bef77706c7ff2e0cbfe0d77eb1L3489) in #13880.
(cherry picked from commit a3c8525c93edce681f3d99a0b8828624fb05919a)
---
.../broker/admin/impl/PersistentTopicsBase.java | 8 ++-
.../broker/admin/AdminApiSubscriptionTest.java | 72 ++++++++++++++++++++++
.../broker/auth/MockedPulsarServiceBaseTest.java | 20 ++++++
3 files changed, 98 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 d5952f8627e..0e591dee4ff 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
@@ -3308,11 +3308,15 @@ public class PersistentTopicsBase extends AdminResource {
if (subName.startsWith(topic.getReplicatorPrefix())) {
String remoteCluster = PersistentReplicator.getRemoteCluster(subName);
PersistentReplicator repl = (PersistentReplicator) topic.getPersistentReplicator(remoteCluster);
- checkNotNull(repl);
+ if (repl == null) {
+ throw new RestException(Status.NOT_FOUND, "Replicator not found");
+ }
issued = repl.expireMessages(expireTimeInSeconds);
} else {
PersistentSubscription sub = topic.getSubscription(subName);
- checkNotNull(sub);
+ if (sub == null) {
+ throw new RestException(Status.NOT_FOUND, "Subscription not found");
+ }
issued = sub.expireMessages(expireTimeInSeconds);
}
if (issued) {
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiSubscriptionTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiSubscriptionTest.java
new file mode 100644
index 00000000000..6f38ccd8b5c
--- /dev/null
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiSubscriptionTest.java
@@ -0,0 +1,72 @@
+/**
+ * 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.broker.admin;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.expectThrows;
+import javax.ws.rs.core.Response;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import org.apache.pulsar.client.api.MessageId;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+@Slf4j
+@Test(groups = "broker-admin")
+public class AdminApiSubscriptionTest extends MockedPulsarServiceBaseTest {
+ @BeforeMethod
+ @Override
+ public void setup() throws Exception {
+ super.internalSetup();
+ super.setupDefaultTenantAndNamespace();
+ }
+
+ @AfterMethod(alwaysRun = true)
+ @Override
+ public void cleanup() throws Exception {
+ super.internalCleanup();
+ }
+
+ @Test
+ public void testExpireNonExistTopic() throws Exception {
+ String topic = "test-expire-messages-topic";
+ String subscriptionName = "test-expire-messages-sub";
+ admin.topics().createSubscription(topic, subscriptionName, MessageId.latest);
+ assertEquals(expectThrows(PulsarAdminException.class,
+ () -> admin.topics().expireMessages(topic, subscriptionName, 1)).getStatusCode(),
+ Response.Status.CONFLICT.getStatusCode());
+ assertEquals(expectThrows(PulsarAdminException.class,
+ () -> admin.topics().expireMessagesForAllSubscriptions(topic, 1)).getStatusCode(),
+ Response.Status.CONFLICT.getStatusCode());
+ }
+
+ @Test
+ public void TestExpireNonExistTopicAndNonExistSub() {
+ String topic = "test-expire-messages-topic";
+ String subscriptionName = "test-expire-messages-sub";
+ assertEquals(expectThrows(PulsarAdminException.class,
+ () -> admin.topics().expireMessages(topic, subscriptionName, 1)).getStatusCode(),
+ Response.Status.NOT_FOUND.getStatusCode());
+ assertEquals(expectThrows(PulsarAdminException.class,
+ () -> admin.topics().expireMessagesForAllSubscriptions(topic, 1)).getStatusCode(),
+ Response.Status.NOT_FOUND.getStatusCode());
+ }
+}
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java
index c731d310d99..27ac9cb869d 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java
@@ -60,6 +60,7 @@ import org.apache.pulsar.client.api.ClientBuilder;
import org.apache.pulsar.client.api.PulsarClient;
import org.apache.pulsar.client.api.PulsarClientException;
import org.apache.pulsar.common.policies.data.ClusterData;
+import org.apache.pulsar.common.policies.data.TenantInfo;
import org.apache.pulsar.common.policies.data.TenantInfoImpl;
import org.apache.pulsar.metadata.api.MetadataStoreException;
import org.apache.pulsar.metadata.api.extended.MetadataStoreExtended;
@@ -499,5 +500,24 @@ public abstract class MockedPulsarServiceBaseTest extends TestRetrySupport {
return configuration;
}
+ protected void setupDefaultTenantAndNamespace() throws Exception {
+ final String tenant = "public";
+ final String namespace = tenant + "/default";
+
+ if (!admin.clusters().getClusters().contains(configClusterName)) {
+ admin.clusters().createCluster(configClusterName,
+ ClusterData.builder().serviceUrl(pulsar.getWebServiceAddress()).build());
+ }
+
+ if (!admin.tenants().getTenants().contains(tenant)) {
+ admin.tenants().createTenant(tenant, TenantInfo.builder().allowedClusters(
+ Sets.newHashSet(configClusterName)).build());
+ }
+
+ if (!admin.namespaces().getNamespaces(tenant).contains(namespace)) {
+ admin.namespaces().createNamespace(namespace);
+ }
+ }
+
private static final Logger log = LoggerFactory.getLogger(MockedPulsarServiceBaseTest.class);
}