From 19db04c430d57c09b65be3be459fa9af91fb9b6f Mon Sep 17 00:00:00 2001
From: guqing <38999863+guqing@users.noreply.github.com>
Date: Thu, 2 Jun 2022 10:52:10 +0800
Subject: [PATCH] refactor: policy rule constructor to avoid null values
 (#2131)

* refactor: policy rule constructor to avoid null values

* fix: unit test case
---
 .../identity/authorization/PolicyRule.java    | 35 +++++----
 .../authorization/PolicyRuleTest.java         | 72 +++++++++++++++++++
 2 files changed, 89 insertions(+), 18 deletions(-)
 create mode 100644 src/test/java/run/halo/app/identity/authorization/PolicyRuleTest.java

diff --git a/src/main/java/run/halo/app/identity/authorization/PolicyRule.java b/src/main/java/run/halo/app/identity/authorization/PolicyRule.java
index f18f4d138..e39717652 100644
--- a/src/main/java/run/halo/app/identity/authorization/PolicyRule.java
+++ b/src/main/java/run/halo/app/identity/authorization/PolicyRule.java
@@ -1,8 +1,6 @@
 package run.halo.app.identity.authorization;
 
-import lombok.AllArgsConstructor;
 import lombok.Data;
-import lombok.NoArgsConstructor;
 
 /**
  * PolicyRule holds information that describes a policy rule, but does not contain information
@@ -12,8 +10,6 @@ import lombok.NoArgsConstructor;
  * @since 2.0.0
  */
 @Data
-@AllArgsConstructor
-@NoArgsConstructor
 public class PolicyRule {
 
     /**
@@ -53,6 +49,22 @@ public class PolicyRule {
      */
     String[] verbs;
 
+    public PolicyRule(String[] apiGroups, String[] resources, String[] resourceNames,
+        String[] nonResourceURLs, String[] verbs) {
+        this.apiGroups = nullElseEmpty(apiGroups);
+        this.resources = nullElseEmpty(resources);
+        this.resourceNames = nullElseEmpty(resourceNames);
+        this.nonResourceURLs = nullElseEmpty(nonResourceURLs);
+        this.verbs = nullElseEmpty(verbs);
+    }
+
+    String[] nullElseEmpty(String... items) {
+        if (items == null) {
+            return new String[] {};
+        }
+        return items;
+    }
+
     public static class Builder {
         String[] apiGroups;
         String[] resources;
@@ -85,21 +97,8 @@ public class PolicyRule {
             return this;
         }
 
-        String[] nullElseEmpty(String... items) {
-            if (items == null) {
-                return new String[] {};
-            }
-            return items;
-        }
-
         public PolicyRule build() {
-            return new PolicyRule(
-                nullElseEmpty(apiGroups),
-                nullElseEmpty(resources),
-                nullElseEmpty(resourceNames),
-                nullElseEmpty(nonResourceURLs),
-                nullElseEmpty(verbs)
-            );
+            return new PolicyRule(apiGroups, resources, resourceNames, nonResourceURLs, verbs);
         }
     }
 }
diff --git a/src/test/java/run/halo/app/identity/authorization/PolicyRuleTest.java b/src/test/java/run/halo/app/identity/authorization/PolicyRuleTest.java
new file mode 100644
index 000000000..41c25bbc6
--- /dev/null
+++ b/src/test/java/run/halo/app/identity/authorization/PolicyRuleTest.java
@@ -0,0 +1,72 @@
+package run.halo.app.identity.authorization;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import run.halo.app.infra.utils.JsonUtils;
+
+/**
+ * Tests for {@link PolicyRule}.
+ *
+ * @author guqing
+ * @since 2.0.0
+ */
+class PolicyRuleTest {
+    private ObjectMapper objectMapper;
+
+    @BeforeEach
+    void setUp() {
+        objectMapper = JsonUtils.DEFAULT_JSON_MAPPER;
+    }
+
+    @Test
+    public void constructPolicyRule() throws JsonProcessingException {
+        PolicyRule policyRule = new PolicyRule(null, null, null, null, null);
+        assertThat(policyRule).isNotNull();
+        JsonNode policyRuleJson = objectMapper.valueToTree(policyRule);
+        assertThat(policyRuleJson).isEqualTo(objectMapper.readTree("""
+            {"apiGroups":[],"resources":[],"resourceNames":[],"nonResourceURLs":[],"verbs":[]}
+            """));
+
+        PolicyRule policyByBuilder = new PolicyRule.Builder().build();
+        JsonNode policyByBuilderJson = objectMapper.valueToTree(policyByBuilder);
+        assertThat(policyByBuilderJson).isEqualTo(objectMapper.readTree("""
+            {"apiGroups":[],"resources":[],"resourceNames":[],"nonResourceURLs":[],"verbs":[]}
+             """));
+
+        PolicyRule policyNonNull = new PolicyRule.Builder()
+            .apiGroups("group")
+            .resources("resource-1", "resource-2")
+            .resourceNames("resourceName")
+            .nonResourceURLs("non resource url")
+            .verbs("verbs")
+            .build();
+
+        JsonNode expected = objectMapper.readTree("""
+            {
+                "apiGroups": [
+                    "group"
+                ],
+                "resources": [
+                    "resource-1",
+                    "resource-2"
+                ],
+                "resourceNames": [
+                    "resourceName"
+                ],
+                "nonResourceURLs": [
+                    "non resource url"
+                ],
+                "verbs": [
+                    "verbs"
+                ]
+            }
+            """);
+        JsonNode policyNonNullJson = objectMapper.valueToTree(policyNonNull);
+        assertThat(policyNonNullJson).isEqualTo(expected);
+    }
+}
\ No newline at end of file