From 57f5f4c4242ae7262753da33d533f84ed5358a7a Mon Sep 17 00:00:00 2001
From: Martin Lowe <martin.lowe@eclipse-foundation.org>
Date: Tue, 13 Jun 2023 16:09:21 -0400
Subject: [PATCH] Fix nullable values for incoming Github commit data
Missing commit data caused cascading failures in the request that should
have resulted in just the specific commit being marked as invalid.
---
.../git/eca/api/models/GithubCommit.java | 32 +++++++++----------
.../eca/resource/GithubAdjacentResource.java | 29 +++++++++++++----
2 files changed, 39 insertions(+), 22 deletions(-)
diff --git a/src/main/java/org/eclipsefoundation/git/eca/api/models/GithubCommit.java b/src/main/java/org/eclipsefoundation/git/eca/api/models/GithubCommit.java
index 3cb2ea3e..631c78cf 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/api/models/GithubCommit.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/api/models/GithubCommit.java
@@ -28,9 +28,9 @@ public abstract class GithubCommit {
public abstract CommitData getCommit();
- public abstract CommitUser getCommitter();
+ public abstract GithubCommitUser getCommitter();
- public abstract CommitUser getAuthor();
+ public abstract GithubCommitUser getAuthor();
public abstract List<ParentCommit> getParents();
@@ -45,9 +45,9 @@ public abstract class GithubCommit {
public abstract Builder setCommit(CommitData commit);
- public abstract Builder setCommitter(CommitUser committer);
+ public abstract Builder setCommitter(GithubCommitUser committer);
- public abstract Builder setAuthor(CommitUser author);
+ public abstract Builder setAuthor(GithubCommitUser author);
public abstract Builder setParents(List<ParentCommit> parents);
@@ -57,9 +57,9 @@ public abstract class GithubCommit {
@AutoValue
@JsonDeserialize(builder = AutoValue_GithubCommit_CommitData.Builder.class)
public abstract static class CommitData {
- public abstract BriefCommitUser getAuthor();
+ public abstract GitCommitUser getAuthor();
- public abstract BriefCommitUser getCommitter();
+ public abstract GitCommitUser getCommitter();
public static Builder builder() {
return new AutoValue_GithubCommit_CommitData.Builder();
@@ -68,23 +68,23 @@ public abstract class GithubCommit {
@AutoValue.Builder
@JsonPOJOBuilder(withPrefix = "set")
public abstract static class Builder {
- public abstract Builder setAuthor(BriefCommitUser author);
+ public abstract Builder setAuthor(GitCommitUser author);
- public abstract Builder setCommitter(BriefCommitUser committer);
+ public abstract Builder setCommitter(GitCommitUser committer);
public abstract CommitData build();
}
}
@AutoValue
- @JsonDeserialize(builder = AutoValue_GithubCommit_BriefCommitUser.Builder.class)
- public abstract static class BriefCommitUser {
+ @JsonDeserialize(builder = AutoValue_GithubCommit_GitCommitUser.Builder.class)
+ public abstract static class GitCommitUser {
public abstract String getName();
public abstract String getEmail();
public static Builder builder() {
- return new AutoValue_GithubCommit_BriefCommitUser.Builder();
+ return new AutoValue_GithubCommit_GitCommitUser.Builder();
}
@AutoValue.Builder
@@ -94,17 +94,17 @@ public abstract class GithubCommit {
public abstract Builder setEmail(String email);
- public abstract BriefCommitUser build();
+ public abstract GitCommitUser build();
}
}
@AutoValue
- @JsonDeserialize(builder = AutoValue_GithubCommit_CommitUser.Builder.class)
- public abstract static class CommitUser {
+ @JsonDeserialize(builder = AutoValue_GithubCommit_GithubCommitUser.Builder.class)
+ public abstract static class GithubCommitUser {
public abstract String getLogin();
public static Builder builder() {
- return new AutoValue_GithubCommit_CommitUser.Builder();
+ return new AutoValue_GithubCommit_GithubCommitUser.Builder();
}
@AutoValue.Builder
@@ -112,7 +112,7 @@ public abstract class GithubCommit {
public abstract static class Builder {
public abstract Builder setLogin(String login);
- public abstract CommitUser build();
+ public abstract GithubCommitUser build();
}
}
diff --git a/src/main/java/org/eclipsefoundation/git/eca/resource/GithubAdjacentResource.java b/src/main/java/org/eclipsefoundation/git/eca/resource/GithubAdjacentResource.java
index 5fc19e28..f1fc5496 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/resource/GithubAdjacentResource.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/resource/GithubAdjacentResource.java
@@ -16,6 +16,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
+import java.util.function.Supplier;
import java.util.stream.Collectors;
import javax.inject.Inject;
@@ -100,6 +101,7 @@ public abstract class GithubAdjacentResource {
* @param repositoryUrl the URL of the repository that contains the commits to validate
* @return the populated validation request for the Github request information
*/
+ @SuppressWarnings("null")
ValidationRequest generateRequest(String installationId, String repositoryFullName, int pullRequestNumber, String repositoryUrl) {
checkRequestParameters(installationId, repositoryFullName, pullRequestNumber);
// get the commits that will be validated, don't cache as changes can come in too fast for it to be useful
@@ -121,15 +123,15 @@ public abstract class GithubAdjacentResource {
.setHash(c.getSha())
.setAuthor(GitUser
.builder()
- .setMail(c.getCommit().getAuthor().getEmail())
- .setName(c.getCommit().getAuthor().getName())
- .setExternalId(c.getAuthor().getLogin())
+ .setMail(getNullableString(() -> c.getCommit().getAuthor().getEmail()))
+ .setName(getNullableString(() -> c.getCommit().getAuthor().getName()))
+ .setExternalId(getNullableString(() -> c.getAuthor().getLogin()))
.build())
.setCommitter(GitUser
.builder()
- .setMail(c.getCommit().getCommitter().getEmail())
- .setName(c.getCommit().getCommitter().getName())
- .setExternalId(c.getCommitter().getLogin())
+ .setMail(getNullableString(() -> c.getCommit().getCommitter().getEmail()))
+ .setName(getNullableString(() -> c.getCommit().getCommitter().getName()))
+ .setExternalId(getNullableString(() -> c.getCommitter().getLogin()))
.build())
.setParents(c.getParents().stream().map(ParentCommit::getSha).collect(Collectors.toList()))
.build())
@@ -272,6 +274,21 @@ public abstract class GithubAdjacentResource {
.build());
}
+ /**
+ * Wraps a nullable value fetch to handle errors and will return null if the value can't be retrieved.
+ *
+ * @param supplier the method with potentially nullable values
+ * @return the value if it can be found, or null
+ */
+ private String getNullableString(Supplier<String> supplier) {
+ try {
+ return supplier.get();
+ } catch (NullPointerException e) {
+ // suppress, as we don't care at this point if its null
+ }
+ return null;
+ }
+
/**
* Validates required fields for processing requests.
*
--
GitLab