Remove jackson members from data objects (#2209)

bitwiseman · web-flow · commit c7a02d9517b5 · 2026-03-24T01:33:48.000-07:00
* Remove jackson members from data objects

* Fix coverage and site
diff --git a/src/main/java/org/kohsuke/github/GHEventInfo.java b/src/main/java/org/kohsuke/github/GHEventInfo.java
@@ -1,6 +1,5 @@
 package org.kohsuke.github;
 
-import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
@@ -84,8 +83,7 @@ static GHEvent transformTypeToGHEvent(String type) {
     private long id;
     private GHOrganization org;
 
-    // we don't want to expose Jackson dependency to the user. This needs databinding
-    private ObjectNode payload;
+    private Object payload;
 
     // these are all shallow objects
     private GHEventRepository repo;
@@ -174,7 +172,9 @@ public GHOrganization getOrganization() throws IOException {
      *             if payload cannot be parsed
      */
     public <T extends GHEventPayload> T getPayload(Class<T> type) throws IOException {
-        T v = GitHubClient.getMappingObjectReader(root()).forType(type).readValue(payload);
+        T v = GitHubClient.getMappingObjectReader(root())
+                .forType(type)
+                .readValue(GitHubClient.getMappingObjectWriter().writeValueAsString(payload));
         v.lateBind();
         return v;
     }
diff --git a/src/main/java/org/kohsuke/github/GHRepositoryRule.java b/src/main/java/org/kohsuke/github/GHRepositoryRule.java
@@ -1,7 +1,7 @@
 package org.kohsuke.github;
 
-import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.JavaType;
+import com.fasterxml.jackson.databind.ObjectReader;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.kohsuke.github.internal.EnumUtils;
 
@@ -13,7 +13,9 @@
 /**
  * Represents a repository rule.
  */
-@SuppressFBWarnings(value = { "UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD", "UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD" },
+@SuppressFBWarnings(
+        value = { "UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD", "UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD",
+                "CT_CONSTRUCTOR_THROW" },
         justification = "JSON API")
 public class GHRepositoryRule extends GitHubInteractiveObject {
 
@@ -53,13 +55,7 @@ public static class BooleanParameter extends Parameter<Boolean> {
          *            the key
          */
         public BooleanParameter(String key) {
-            super(key);
-        }
-
-        @Override
-        TypeReference<Boolean> getType() {
-            return new TypeReference<Boolean>() {
-            };
+            super(key, Boolean.class);
         }
     }
     /**
@@ -115,30 +111,51 @@ public static class IntegerParameter extends Parameter<Integer> {
          *            the key
          */
         public IntegerParameter(String key) {
-            super(key);
-        }
-
-        @Override
-        TypeReference<Integer> getType() {
-            return new TypeReference<Integer>() {
-            };
+            super(key, Integer.class);
         }
     }
     /**
      * List parameter for a ruleset.
      *
-     * @param <T>
-     *            the type of the list
+     * @param <U>
+     *            the type of the items in the list
      */
-    public abstract static class ListParameter<T> extends Parameter<List<T>> {
+    public abstract static class ListParameter<U> extends Parameter<List<U>> {
+
+        private final Class<U> itemClass;
+
         /**
          * Instantiates a new list parameter.
          *
          * @param key
          *            the key
          */
         public ListParameter(String key) {
-            super(key);
+            super(key, null);
+            throw new GHException("This constructor should not have been public.");
+        }
+
+        /**
+         * Instantiates a new list parameter.
+         *
+         * @param key
+         *            the key
+         * @param itemClass
+         *            the class of items in the list parameter
+         */
+        ListParameter(String key, Class<U> itemClass) {
+            super(key, null);
+            this.itemClass = itemClass;
+        }
+
+        @Override
+        List<U> apply(String value, GitHub root) throws IOException {
+            if (value == null) {
+                return null;
+            }
+            ObjectReader objectReader = GitHubClient.getMappingObjectReader(root);
+            JavaType javaType = objectReader.getTypeFactory().constructParametricType(List.class, itemClass);
+            return objectReader.forType(javaType).readValue(value);
         }
     }
     /**
@@ -174,6 +191,7 @@ public static enum Operator {
      */
     public abstract static class Parameter<T> {
 
+        private final Class<T> clazz;
         private final String key;
 
         /**
@@ -183,14 +201,28 @@ public abstract static class Parameter<T> {
          *            the key
          */
         protected Parameter(String key) {
+            throw new GHException("This constructor should not have been protected.");
+        }
+
+        /**
+         * Instantiates a new parameter.
+         *
+         * @param key
+         *            the key
+         * @param clazz
+         *            the class the the parameter
+         */
+        Parameter(String key, Class<T> clazz) {
             this.key = key;
+            this.clazz = clazz;
         }
 
-        T apply(JsonNode jsonNode, GitHub root) throws IOException {
-            if (jsonNode == null) {
+        T apply(String value, GitHub root) throws IOException {
+            if (value == null) {
                 return null;
             }
-            return GitHubClient.getMappingObjectReader(root).forType(this.getType()).readValue(jsonNode);
+            ObjectReader objectReader = GitHubClient.getMappingObjectReader(root);
+            return objectReader.forType(clazz).readValue(value);
         }
 
         /**
@@ -201,11 +233,6 @@ T apply(JsonNode jsonNode, GitHub root) throws IOException {
         String getKey() {
             return this.key;
         }
-
-        /**
-         * Get the parameter type reference for type mapping.
-         */
-        abstract TypeReference<T> getType();
     }
 
     /**
@@ -216,12 +243,8 @@ public interface Parameters {
          * code_scanning_tools parameter
          */
         public static final ListParameter<CodeScanningTool> CODE_SCANNING_TOOLS = new ListParameter<CodeScanningTool>(
-                "code_scanning_tools") {
-            @Override
-            TypeReference<List<CodeScanningTool>> getType() {
-                return new TypeReference<List<CodeScanningTool>>() {
-                };
-            }
+                "code_scanning_tools",
+                CodeScanningTool.class) {
         };
         /**
          * dismiss_stale_reviews_on_push parameter
@@ -239,12 +262,7 @@ TypeReference<List<CodeScanningTool>> getType() {
         /**
          * operator parameter
          */
-        public static final Parameter<Operator> OPERATOR = new Parameter<Operator>("operator") {
-            @Override
-            TypeReference<Operator> getType() {
-                return new TypeReference<Operator>() {
-                };
-            }
+        public static final Parameter<Operator> OPERATOR = new Parameter<Operator>("operator", Operator.class) {
         };
         /**
          * regex parameter
@@ -259,12 +277,8 @@ TypeReference<Operator> getType() {
          * required_deployment_environments parameter
          */
         public static final ListParameter<String> REQUIRED_DEPLOYMENT_ENVIRONMENTS = new ListParameter<String>(
-                "required_deployment_environments") {
-            @Override
-            TypeReference<List<String>> getType() {
-                return new TypeReference<List<String>>() {
-                };
-            }
+                "required_deployment_environments",
+                String.class) {
         };
         /**
          * required_review_thread_resolution parameter
@@ -275,12 +289,8 @@ TypeReference<List<String>> getType() {
          * required_status_checks parameter
          */
         public static final ListParameter<StatusCheckConfiguration> REQUIRED_STATUS_CHECKS = new ListParameter<StatusCheckConfiguration>(
-                "required_status_checks") {
-            @Override
-            TypeReference<List<StatusCheckConfiguration>> getType() {
-                return new TypeReference<List<StatusCheckConfiguration>>() {
-                };
-            }
+                "required_status_checks",
+                StatusCheckConfiguration.class) {
         };
         /**
          * require_code_owner_review parameter
@@ -306,12 +316,8 @@ TypeReference<List<StatusCheckConfiguration>> getType() {
          * workflows parameter
          */
         public static final ListParameter<WorkflowFileReference> WORKFLOWS = new ListParameter<WorkflowFileReference>(
-                "workflows") {
-            @Override
-            TypeReference<List<WorkflowFileReference>> getType() {
-                return new TypeReference<List<WorkflowFileReference>>() {
-                };
-            }
+                "workflows",
+                WorkflowFileReference.class) {
         };
     }
 
@@ -409,13 +415,7 @@ public static class StringParameter extends Parameter<String> {
          *            the key
          */
         public StringParameter(String key) {
-            super(key);
-        }
-
-        @Override
-        TypeReference<String> getType() {
-            return new TypeReference<String>() {
-            };
+            super(key, String.class);
         }
     }
 
@@ -562,7 +562,7 @@ public String getSha() {
         }
     }
 
-    private Map<String, JsonNode> parameters;
+    private Map<String, Object> parameters;
 
     private long rulesetId;
 
@@ -593,11 +593,12 @@ public <T> Optional<T> getParameter(Parameter<T> parameter) throws IOException {
         if (this.parameters == null) {
             return Optional.empty();
         }
-        JsonNode jsonNode = this.parameters.get(parameter.getKey());
-        if (jsonNode == null) {
+        Object value = this.parameters.get(parameter.getKey());
+        if (value == null) {
             return Optional.empty();
         }
-        return Optional.ofNullable(parameter.apply(jsonNode, root()));
+        return Optional
+                .ofNullable(parameter.apply(GitHubClient.getMappingObjectWriter().writeValueAsString(value), root()));
     }
 
     /**
diff --git a/src/test/java/org/kohsuke/github/AotIntegrationTest.java b/src/test/java/org/kohsuke/github/AotIntegrationTest.java
@@ -80,6 +80,7 @@ public void testIfAllRequiredClassesAreRegisteredForAot() throws IOException {
 
     private Stream<String> readAotConfigToStreamOfClassNames(String reflectionConfig) throws IOException {
         byte[] reflectionConfigFileAsBytes = Files.readAllBytes(Path.of(reflectionConfig));
+        // Test methods are allowed to directly use whatever jackson is available.
         ArrayNode reflectConfigJsonArray = (ArrayNode) JsonMapper.builder()
                 .build()
                 .readTree(reflectionConfigFileAsBytes);
diff --git a/src/test/java/org/kohsuke/github/GHRepositoryRuleTest.java b/src/test/java/org/kohsuke/github/GHRepositoryRuleTest.java
@@ -16,11 +16,12 @@
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThrows;
 
 /**
  * Test class for GHRepositoryRule.
  */
-public class GHRepositoryRuleTest {
+public class GHRepositoryRuleTest extends AbstractGitHubWireMockTest {
 
     /**
      * Create default GHRepositoryRuleTest instance
@@ -70,15 +71,24 @@ public void testParameterReturnsNullOnNullArg() throws Exception {
 
     /**
      * Test to cover the constructor of the Parameters class.
+     *
+     * @throws Exception
+     *             if something goes wrong.
      */
     @Test
-    public void testParameters() {
-        assertThat(Parameters.REQUIRED_DEPLOYMENT_ENVIRONMENTS.getType(), is(notNullValue()));
-        assertThat(Parameters.REQUIRED_STATUS_CHECKS.getType(), is(notNullValue()));
-        assertThat(Parameters.OPERATOR.getType(), is(notNullValue()));
-        assertThat(Parameters.WORKFLOWS.getType(), is(notNullValue()));
-        assertThat(Parameters.CODE_SCANNING_TOOLS.getType(), is(notNullValue()));
-        assertThat(new StringParameter("any").getType(), is(notNullValue()));
+    public void testParameters() throws Exception {
+        assertThat(Parameters.REQUIRED_DEPLOYMENT_ENVIRONMENTS, is(notNullValue()));
+        assertThat(Parameters.REQUIRED_STATUS_CHECKS, is(notNullValue()));
+        assertThat(Parameters.OPERATOR, is(notNullValue()));
+        assertThat(Parameters.WORKFLOWS, is(notNullValue()));
+        assertThat(Parameters.CODE_SCANNING_TOOLS, is(notNullValue()));
+        assertThat(Parameters.CODE_SCANNING_TOOLS.apply("[]", this.gitHub), is(notNullValue()));
+        assertThat(new StringParameter("any"), is(notNullValue()));
+
+        assertThrows(GHException.class, () -> new GHRepositoryRule.ListParameter<Object>("") {
+        });
+        assertThrows(GHException.class, () -> new GHRepositoryRule.Parameter<Object>("") {
+        });
     }
 
     /**
