Skip to content

Commit

Permalink
[Broker] Fix NPE of internalExpireMessagesByTimestamp (#14243)
Browse files Browse the repository at this point in the history
  • Loading branch information
nodece authored and congbobo184 committed Nov 11, 2022
1 parent 92c9cf9 commit 7d405c3
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3308,11 +3308,15 @@ private void internalExpireMessagesByTimestampForSinglePartition(String subName,
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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
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;
Expand Down Expand Up @@ -499,5 +500,24 @@ protected static ServiceConfiguration getDefaultConf() {
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);
}

0 comments on commit 7d405c3

Please sign in to comment.