[Broker] Fix NPE in PersistentTopic.checkSubscriptionTypesEnable

[RobertIndie]

Motivation

The NPE may appear in PersistentTopic.checkSubscriptionTypesEnable. The topicPolicies.getSubscriptionTypesEnabled() may be null.

The problem occurred when upgrading from pulsar version 2.7 to 2.8. We added SubscriptionTypesEnable in 2.8: #9401. When we initialize a topic's topicPolicy in pulasr 2.7, and then get the topic's topicPolicy in version 2.8, the SubscriptionTypesEnable will be null. This leads to the problem.

Here are steps to reproduce:

• Start broker 2.7
• Create a topic and init the topic policy
• Upgrade broker to 2.8
• We will get the SubscriptionTypesEnable with the value of null

➜  pulsar-281 bin/pulsar-admin topics get-subscription-types-enabled  my-topic
null

• When we consume from that topic, the issue occurs

Caused by: java.lang.NullPointerException
	at org.apache.pulsar.broker.service.persistent.PersistentTopic.checkSubscriptionTypesEnable(PersistentTopic.java:3206) ~[io.streamnative-pulsar-broker-2.8.1.21.jar:2.8.1.21]
	at org.apache.pulsar.broker.service.persistent.PersistentTopic.subscribe(PersistentTopic.java:688) ~[io.streamnative-pulsar-broker-2.8.1.21.jar:2.8.1.21]
	at org.apache.pulsar.broker.service.ServerCnx.lambda$null$13(ServerCnx.java:1029) ~[io.streamnative-pulsar-broker-2.8.1.21.jar:2.8.1.21]
	at java.util.concurrent.CompletableFuture.uniComposeStage(CompletableFuture.java:1106) ~[?:?]
	... 31 more

Modifications

• Use CollectionUtils.isEmpty to determine if the list is empty to fix the problem of throwing NPE.

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

• Dependencies (does it add or upgrade a dependency): (no)
• The public API: (no)
• The schema: (no)
• The default values of configurations: (no)
• The wire protocol: (no)
• The rest endpoints: (no)
• The admin cli options: (no)
• Anything that affects deployment: (no)

Documentation

• no-need-doc

This is just a small bug fix.

[yuruguo]

topicPolicies.getSubscriptionTypesEnabled() default value is empty list, when can it become null?

pulsar/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/TopicPolicies.java

Lines 47 to 48 in 94736a4

	@Builder.Default
	private List<SubType> subscriptionTypesEnabled = new ArrayList<>();

In addition, it is recommended to combine the two judgments as below:

if (topicPolicies == null || CollectionUtils.isEmpty(topicPolicies.getSubscriptionTypesEnabled())) {
    return checkNsAndBrokerSubscriptionTypesEnable(topicName, subType);
} else {
    return topicPolicies.getSubscriptionTypesEnabled().contains(subType);
}

[gaozhangmin]

topicPolicies.getSubscriptionTypesEnabled() default value is empty list.
+1

[RobertIndie]

The problem occurred when upgrading from pulsar version 2.7 to 2.8. I have added the root cause of the problem and the reproduce steps to the PR description. Please check. @yuruguo @gaozhangmin

In addition, it is recommended to combine the two judgments as below:

if (topicPolicies == null || CollectionUtils.isEmpty(topicPolicies.getSubscriptionTypesEnabled())) {
    return checkNsAndBrokerSubscriptionTypesEnable(topicName, subType);
} else {
    return topicPolicies.getSubscriptionTypesEnabled().contains(subType);
}

@yuruguo I don't think we need to. The null check of topicPolicy has been done before.

[codelipenghui]

@RobertIndie Could you please add a test to avoid the regression?

[RobertIndie]

@RobertIndie Could you please add a test to avoid the regression?

The problem occurs between different versions of brokers and it seems difficult to add tests.