Skip to content

#947 base event payload #976

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

Merged
bitwiseman merged 3 commits into from
Nov 17, 2020
Merged

#947 base event payload #976

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
929d9fb
#947 base event payload
seregamorph Nov 9, 2020
74496d3
Merge remote-tracking branch 'origin/master' into feature/base-event-…
seregamorph Nov 13, 2020
b51d655
#947 address review comments
seregamorph Nov 13, 2020
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
97 changes: 24 additions & 73 deletions src/main/java/org/kohsuke/github/GHEventPayload.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ public class GHEventPayload {
// Webhook payload object common properties: action, sender, repository, organization, installation
private String action;

@seregamorph seregamorph Nov 9, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as declared in #947 moved from subclasses. Field may be null depending on the event type

private GHUser sender;
GHRepository repository;
private GHRepository repository;
private GHOrganization organization;
GHAppInstallation installation;
private GHAppInstallation installation;

GHEventPayload() {
}

/**
* Most webhook payloads contain an action property that contains the specific activity that triggered the event.
* Gets the action for the triggered event. Most but not all webhook payloads contain an action property that
* contains the specific activity that triggered the event.
*
* @return event action
*/
Expand Down Expand Up @@ -112,11 +113,14 @@ void wrapUp(GitHub root) {
sender.wrapUp(root);
}
if (repository != null) {

@seregamorph seregamorph Nov 9, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aggregated from overriden wrapUp methods and a getter with side effect (see below) because of fields lift

repository.root = root;
repository.wrap(root);

@seregamorph seregamorph Nov 13, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: repository.root = root; replaced with repository.wrap(root);, a more complex. This way we can inherit this action in subclasses and remove repository.wrap(root); line from them.

}
if (organization != null) {
organization.wrapUp(root);
}
if (installation != null) {
installation.wrapUp(root);
}
}

// List of events that still need to be added:
Expand Down Expand Up @@ -192,8 +196,8 @@ void wrapUp(GitHub root) {
if (checkRun == null)
throw new IllegalStateException(
"Expected check_run payload, but got something else. Maybe we've got another type of event?");
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);

@seregamorph seregamorph Nov 13, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inherited

checkRun.wrap(repository);
} else {
checkRun.wrap(root);
Expand Down Expand Up @@ -226,8 +230,8 @@ void wrapUp(GitHub root) {
if (checkSuite == null)
throw new IllegalStateException(
"Expected check_suite payload, but got something else. Maybe we've got another type of event?");
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
checkSuite.wrap(repository);
} else {
checkSuite.wrap(root);
Expand Down Expand Up @@ -258,11 +262,10 @@ public List<GHRepository> getRepositories() {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
if (installation == null)
if (getInstallation() == null) {
throw new IllegalStateException(
"Expected check_suite payload, but got something else. Maybe we've got another type of event?");
else
installation.wrapUp(root);

@seregamorph seregamorph Nov 13, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inherited

}

if (repositories != null && !repositories.isEmpty()) {
try {
Expand Down Expand Up @@ -320,11 +323,10 @@ public List<GHRepository> getRepositoriesRemoved() {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
if (installation == null)
if (getInstallation() == null) {
throw new IllegalStateException(
"Expected check_suite payload, but got something else. Maybe we've got another type of event?");
else
installation.wrapUp(root);
}

List<GHRepository> repositories;
if ("added".equals(getAction()))
Expand Down Expand Up @@ -393,8 +395,8 @@ void wrapUp(GitHub root) {
if (pullRequest == null)
throw new IllegalStateException(
"Expected pull_request payload, but got something else. Maybe we've got another type of event?");
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
pullRequest.wrapUp(repository);
} else {
pullRequest.wrapUp(root);
Expand Down Expand Up @@ -441,8 +443,8 @@ void wrapUp(GitHub root) {

review.wrapUp(pullRequest);

GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
pullRequest.wrapUp(repository);
} else {
pullRequest.wrapUp(root);
Expand Down Expand Up @@ -489,8 +491,8 @@ void wrapUp(GitHub root) {

comment.wrapUp(pullRequest);

GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
pullRequest.wrapUp(repository);
} else {
pullRequest.wrapUp(root);
Expand Down Expand Up @@ -531,8 +533,8 @@ public void setIssue(GHIssue issue) {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
issue.wrap(repository);
} else {
issue.wrap(root);
Expand Down Expand Up @@ -593,8 +595,8 @@ public void setIssue(GHIssue issue) {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
issue.wrap(repository);
} else {
issue.wrap(root);
Expand Down Expand Up @@ -636,8 +638,8 @@ public void setComment(GHCommitComment comment) {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
comment.wrap(repository);
}
}
Expand Down Expand Up @@ -695,14 +697,6 @@ public String getMasterBranch() {
public String getDescription() {
return description;
}

@Override
void wrapUp(GitHub root) {

@seregamorph seregamorph Nov 13, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fully inherited - hence removed

super.wrapUp(root);
if (repository != null) {
repository.wrap(root);
}
}
}

/**
Expand Down Expand Up @@ -735,14 +729,6 @@ public String getRef() {
public String getRefType() {
return refType;
}

@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
if (repository != null) {
repository.wrap(root);
}
}
}

/**
Expand Down Expand Up @@ -777,8 +763,8 @@ public void setDeployment(GHDeployment deployment) {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
deployment.wrap(repository);
}
}
Expand Down Expand Up @@ -837,8 +823,8 @@ public void setDeployment(GHDeployment deployment) {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
deployment.wrap(repository);
deploymentStatus.wrap(repository);
}
Expand Down Expand Up @@ -878,9 +864,6 @@ public void setForkee(GHRepository forkee) {
void wrapUp(GitHub root) {
super.wrapUp(root);
forkee.wrap(root);
if (repository != null) {
repository.wrap(root);
}
}
}

Expand All @@ -891,12 +874,6 @@ void wrapUp(GitHub root) {
* event</a>
*/
public static class Ping extends GHEventPayload {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
if (repository != null)
repository.wrap(root);
}

}

Expand All @@ -908,13 +885,6 @@ void wrapUp(GitHub root) {
*/
public static class Public extends GHEventPayload {

@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
if (repository != null)
repository.wrap(root);
}

}

/**
Expand Down Expand Up @@ -1038,13 +1008,6 @@ public String getCompare() {
return compare;
}

@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
if (repository != null)
repository.wrap(root);
}

/**
* The type Pusher.
*/
Expand Down Expand Up @@ -1218,14 +1181,6 @@ public GHRelease getRelease() {
public void setRelease(GHRelease release) {
this.release = release;
}

@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
if (repository != null) {
repository.wrap(root);
}
}
}

/**
Expand All @@ -1236,11 +1191,7 @@ void wrapUp(GitHub root) {
* @see <a href="https://docs.github.com/en/rest/reference/repos">Repositories</a>
*/
public static class Repository extends GHEventPayload {
@Override
void wrapUp(GitHub root) {
super.wrapUp(root);
repository.wrap(root);
}

}

/**
Expand Down Expand Up @@ -1319,8 +1270,8 @@ void wrapUp(GitHub root) {
throw new IllegalStateException(
"Expected status payload, but got something else. Maybe we've got another type of event?");
}
GHRepository repository = getRepository();
if (repository != null) {
repository.wrap(root);
commit.wrapUp(repository);
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/test/java/org/kohsuke/github/GHEventPayloadTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,13 @@ public void release_published() throws Exception {
.parseEventPayload(payload.asReader(), GHEventPayload.Release.class);

assertThat(event.getAction(), is("published"));
assertThat(event.getSender().getLogin(), is("seregamorph"));
assertThat(event.getRepository().getName(), is("company-rest-api-framework"));
assertThat(event.getOrganization().getLogin(), is("company-group"));
assertThat(event.getInstallation(), nullValue());
assertThat(event.getRelease().getName(), is("4.2"));
assertThat(event.getRelease().getTagName(), is("rest-api-framework-4.2"));
assertThat(event.getRelease().getBody(), is("REST-269 - unique test executions (#86) Sergey Chernov"));
}

@Test
Expand Down