From 16afdd357b95f9321fee99294523da941612cc32 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Sat, 21 Mar 2026 12:37:50 -0700 Subject: [PATCH] Remove jackson members from data objects --- .../java/org/kohsuke/github/GHEventInfo.java | 8 +- .../org/kohsuke/github/GHRepositoryRule.java | 137 +++++++++--------- .../kohsuke/github/AotIntegrationTest.java | 1 + .../kohsuke/github/GHRepositoryRuleTest.java | 12 +- 4 files changed, 80 insertions(+), 78 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHEventInfo.java b/src/main/java/org/kohsuke/github/GHEventInfo.java index 7172583c51..b22f498bff 100644 --- 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 getPayload(Class 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 index d348153e85..ac6fbcb561 100644 --- 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 { * the key */ public BooleanParameter(String key) { - super(key); - } - - @Override - TypeReference getType() { - return new TypeReference() { - }; + super(key, Boolean.class); } } /** @@ -115,13 +111,7 @@ public static class IntegerParameter extends Parameter { * the key */ public IntegerParameter(String key) { - super(key); - } - - @Override - TypeReference getType() { - return new TypeReference() { - }; + super(key, Integer.class); } } /** @@ -130,7 +120,10 @@ TypeReference getType() { * @param * the type of the list */ - public abstract static class ListParameter extends Parameter> { + public abstract static class ListParameter extends Parameter> { + + private final Class itemClass; + /** * Instantiates a new list parameter. * @@ -138,7 +131,31 @@ public abstract static class ListParameter extends Parameter> { * the key */ public ListParameter(String key) { - super(key); + super(key, null); + throw new NoClassDefFoundError("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 itemClass) { + super(key, null); + this.itemClass = itemClass; + } + + @Override + List 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 { + private final Class clazz; private final String key; /** @@ -183,14 +201,28 @@ public abstract static class Parameter { * the key */ protected Parameter(String key) { + throw new NoClassDefFoundError("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 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 getType(); } /** @@ -216,12 +243,8 @@ public interface Parameters { * code_scanning_tools parameter */ public static final ListParameter CODE_SCANNING_TOOLS = new ListParameter( - "code_scanning_tools") { - @Override - TypeReference> getType() { - return new TypeReference>() { - }; - } + "code_scanning_tools", + CodeScanningTool.class) { }; /** * dismiss_stale_reviews_on_push parameter @@ -239,12 +262,7 @@ TypeReference> getType() { /** * operator parameter */ - public static final Parameter OPERATOR = new Parameter("operator") { - @Override - TypeReference getType() { - return new TypeReference() { - }; - } + public static final Parameter OPERATOR = new Parameter("operator", Operator.class) { }; /** * regex parameter @@ -259,12 +277,8 @@ TypeReference getType() { * required_deployment_environments parameter */ public static final ListParameter REQUIRED_DEPLOYMENT_ENVIRONMENTS = new ListParameter( - "required_deployment_environments") { - @Override - TypeReference> getType() { - return new TypeReference>() { - }; - } + "required_deployment_environments", + String.class) { }; /** * required_review_thread_resolution parameter @@ -275,12 +289,8 @@ TypeReference> getType() { * required_status_checks parameter */ public static final ListParameter REQUIRED_STATUS_CHECKS = new ListParameter( - "required_status_checks") { - @Override - TypeReference> getType() { - return new TypeReference>() { - }; - } + "required_status_checks", + StatusCheckConfiguration.class) { }; /** * require_code_owner_review parameter @@ -306,12 +316,8 @@ TypeReference> getType() { * workflows parameter */ public static final ListParameter WORKFLOWS = new ListParameter( - "workflows") { - @Override - TypeReference> getType() { - return new TypeReference>() { - }; - } + "workflows", + WorkflowFileReference.class) { }; } @@ -409,13 +415,7 @@ public static class StringParameter extends Parameter { * the key */ public StringParameter(String key) { - super(key); - } - - @Override - TypeReference getType() { - return new TypeReference() { - }; + super(key, String.class); } } @@ -562,7 +562,7 @@ public String getSha() { } } - private Map parameters; + private Map parameters; private long rulesetId; @@ -593,11 +593,12 @@ public Optional getParameter(Parameter 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 index f0215bb0e7..5273f8ac9a 100644 --- 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 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 index d5f88f3f5d..5eda530afc 100644 --- a/src/test/java/org/kohsuke/github/GHRepositoryRuleTest.java +++ b/src/test/java/org/kohsuke/github/GHRepositoryRuleTest.java @@ -73,12 +73,12 @@ public void testParameterReturnsNullOnNullArg() throws Exception { */ @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())); + 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(new StringParameter("any"), is(notNullValue())); } /**