From b2796f39785dde2178f1882638aea2feae70211c Mon Sep 17 00:00:00 2001
From: feynmanlin <feynmanlin@tencent.com>
Date: Fri, 5 Mar 2021 07:43:55 +0800
Subject: [PATCH] Fix flaky-test: AuthorizationTest.simple (#9740)

Fixes #9709

### Motivation
```
Caused by: java.lang.NullPointerException
	at java.util.TreeMap.getEntry(TreeMap.java:347)
```
From the stack trace, it is caused by the key read by `TreeMap` being null. Therefore, the role must be null.
```
//PulsarAuthorizationProvider.java:404
Map<String, Set<AuthAction>> namespaceRoles = policies.get().auth_policies.namespace_auth;
Set<AuthAction> namespaceActions = namespaceRoles.get(role);
```

According to the code, the role is read through `httpRequest.getAttribute` in `PulsarWebResource#clientAppId`, so it is normal to be null.

Although the role is null, why do unit tests pass occasionally?
I added logs and found that the implementation class of Map is not always TreeMap, but occasionally HashMap.
When it is a HashMap, no error will be reported if the key is null. This problem is caused by Jackson's serialization.

I checked AuthPolicies, all Maps are initialized as TreeMap, but we have never used any features of TreeMap.
In order to avoid serialization problems elsewhere, I changed TreeMap to HashMap.
Another way is to add the annotation @JsonDeserialize(as = TreeMap.class), and then make the judgment that the key is null.
Why was TreeMap used here? May need @merlimat  to help explain.
---
 .../pulsar/broker/admin/impl/PersistentTopicsBase.java      | 6 +++---
 .../apache/pulsar/common/policies/data/AuthPolicies.java    | 6 +++---
 2 files changed, 6 insertions(+), 6 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 f80acc3544ebb..ac82b11f61448 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
@@ -31,11 +31,11 @@
 import io.netty.buffer.ByteBuf;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.TreeMap;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutionException;
@@ -198,7 +198,7 @@ protected Map<String, Set<AuthAction>> internalGetPermissionsOnTopic() {
             Policies policies = namespaceResources().get(path(POLICIES, namespaceName.toString()))
                     .orElseThrow(() -> new RestException(Status.NOT_FOUND, "Namespace does not exist"));
 
-            Map<String, Set<AuthAction>> permissions = Maps.newTreeMap();
+            Map<String, Set<AuthAction>> permissions = Maps.newHashMap();
             AuthPolicies auth = policies.auth_policies;
 
             // First add namespace level permissions
@@ -323,7 +323,7 @@ private void grantPermissions(String topicUri, String role, Set<AuthAction> acti
         try {
             namespaceResources().set(path(POLICIES, namespaceName.toString()), (policies) -> {
                 if (!policies.auth_policies.destination_auth.containsKey(topicUri)) {
-                    policies.auth_policies.destination_auth.put(topicUri, new TreeMap<String, Set<AuthAction>>());
+                    policies.auth_policies.destination_auth.put(topicUri, new HashMap<>());
                 }
 
                 policies.auth_policies.destination_auth.get(topicUri).put(role, actions);
diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/AuthPolicies.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/AuthPolicies.java
index 23902003fcc29..880409666bf64 100644
--- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/AuthPolicies.java
+++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/AuthPolicies.java
@@ -35,9 +35,9 @@ public class AuthPolicies {
     public final Map<String, Set<String>> subscription_auth_roles;
 
     public AuthPolicies() {
-        namespace_auth = Maps.newTreeMap();
-        destination_auth = Maps.newTreeMap();
-        subscription_auth_roles = Maps.newTreeMap();
+        namespace_auth = Maps.newHashMap();
+        destination_auth = Maps.newHashMap();
+        subscription_auth_roles = Maps.newHashMap();
     }
 
     @Override