Skip to content

Remove jackson members from data objects #2209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bitwiseman wants to merge 1 commit into
base: main
Choose a base branch
from
+80 −78
Open

Remove jackson members from data objects #2209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/main/java/org/kohsuke/github/GHEventInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
137 changes: 69 additions & 68 deletions src/main/java/org/kohsuke/github/GHRepositoryRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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 {

Expand Down Expand Up @@ -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);
}
}
/**
Expand Down Expand Up @@ -115,13 +111,7 @@ 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);
}
}
/**
Expand All @@ -130,15 +120,42 @@ TypeReference<Integer> getType() {
* @param <T>
* the type of 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 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<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);
}
}
/**
Expand Down Expand Up @@ -174,6 +191,7 @@ public static enum Operator {
*/
public abstract static class Parameter<T> {

private final Class<T> clazz;
private final String key;

/**
Expand All @@ -183,14 +201,28 @@ public abstract static class Parameter<T> {
* 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<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);
}

/**
Expand All @@ -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();
}

/**
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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) {
};
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -562,7 +562,7 @@ public String getSha() {
}
}

private Map<String, JsonNode> parameters;
private Map<String, Object> parameters;

private long rulesetId;

Expand Down Expand Up @@ -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()));
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/test/java/org/kohsuke/github/AotIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/org/kohsuke/github/GHRepositoryRuleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}

/**
Expand Down
Loading