Skip to content

Commit 8a906c4

Browse files
committed
Add externalId in CheckRun
1 parent 1028492 commit 8a906c4

File tree

8 files changed

+69
-82
lines changed

8 files changed

+69
-82
lines changed

github/src/main/java/eu/solven/cleanthat/anything/event/ICodePointerCleaner.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@
77
import eu.solven.cleanthat.code_provider.github.event.pojo.IExternalWebhookRelevancyResult;
88

99
/**
10-
* Holds the logic to clean a code poointer (e.g. local folder, or Git ref, ...)
10+
* Holds the logic to clean a code pointer (e.g. local folder, or Git ref, ...)
1111
*
1212
* @author Benoit Lacelle
1313
*/
14+
// TODO Related to IGithubRefCleaner
1415
public interface ICodePointerCleaner {
1516

1617
/**

github/src/main/java/eu/solven/cleanthat/code_provider/github/event/GithubWebhookHandler.java

+18-14
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,8 @@ public WebhookRelevancyResult filterWebhookEventTargetRelevantBranch(ICodeCleane
365365
throw new IllegalStateException("Should not happen");
366366
}
367367
if (optSha1.isPresent()) {
368-
createCheckRun(githubAuthAsInst, baseRepo, optSha1.get());
368+
String eventKey = githubEvent.getxGithubDelivery();
369+
createCheckRun(githubAuthAsInst, baseRepo, optSha1.get(), eventKey);
369370
}
370371
IGithubRefCleaner cleaner = cleanerFactory.makeCleaner(githubAuthAsInst);
371372
// BEWARE this branch may not exist: either it is a cleanthat branch yet to create. Or it may be deleted in the
@@ -379,16 +380,17 @@ public WebhookRelevancyResult filterWebhookEventTargetRelevantBranch(ICodeCleane
379380
return WebhookRelevancyResult.relevant(refToClean.get());
380381
}
381382

382-
public void createCheckRun(GithubAndToken githubAuthAsInst, GHRepository baseRepo, String sha1) {
383+
public void createCheckRun(GithubAndToken githubAuthAsInst, GHRepository baseRepo, String sha1, String eventKey) {
383384
if (GHPermissionType.WRITE == githubAuthAsInst.getPermissions().get(PERMISSION_CHECKS)) {
384385
// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#check_run
385386
// https://docs.github.com/en/rest/reference/checks#runs
386387
// https://docs.github.com/en/rest/reference/permissions-required-for-github-apps#permission-on-checks
387-
GHCheckRunBuilder checkRunBuilder = baseRepo.createCheckRun("CleanThat", sha1);
388+
GHCheckRunBuilder checkRunBuilder = baseRepo.createCheckRun("CleanThat", sha1).withExternalID(eventKey);
388389
try {
389-
// baseRepo.getCheckRuns("master").asList();
390-
GHCheckRun checkRun = checkRunBuilder.create();
391-
checkRun.update().withConclusion(Conclusion.SUCCESS).withStatus(Status.COMPLETED);
390+
GHCheckRun checkRun = checkRunBuilder.withStatus(Status.IN_PROGRESS).create();
391+
392+
// We complete right now, until we are able to complete this properly
393+
checkRun.update().withConclusion(Conclusion.SUCCESS).withStatus(Status.COMPLETED).create();
392394
} catch (IOException e) {
393395
// https://github.community/t/resource-not-accessible-when-trying-to-read-write-checkrun/193493
394396
if (LOGGER.isDebugEnabled()) {
@@ -512,14 +514,16 @@ public void doExecuteWebhookEvent(ICodeCleanerFactory cleanerFactory, IWebhookEv
512514
LOGGER.debug("The changes would have been committed directly in the head branch");
513515
}
514516

515-
// This is useful to investigate unexpected rateLimitHit
516-
{
517-
try {
518-
GHRateLimit rateLimit = github.getRateLimit();
519-
LOGGER.info("After process, rateLimit={} for installationId={}", rateLimit, installationId);
520-
} catch (IOException e) {
521-
LOGGER.warn("Issue with RateLimit", e);
522-
}
517+
logAfterCleaning(installationId, github);
518+
}
519+
520+
public void logAfterCleaning(long installationId, GitHub github) {
521+
try {
522+
// This is useful to investigate unexpected rateLimitHit
523+
GHRateLimit rateLimit = github.getRateLimit();
524+
LOGGER.info("After process, rateLimit={} for installationId={}", rateLimit, installationId);
525+
} catch (IOException e) {
526+
LOGGER.warn("Issue with RateLimit", e);
523527
}
524528
}
525529

github/src/main/java/eu/solven/cleanthat/code_provider/github/event/pojo/GithubWebhookEvent.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,14 @@ public static GithubWebhookEvent fromCleanThatEvent(IWebhookEvent githubAccepted
8484
throw new IllegalArgumentException("This does not hold a github event");
8585
}
8686

87-
return new GithubWebhookEvent(PepperMapHelper.getRequiredMap(body, "github", "body"));
87+
Map<String, ?> headers = PepperMapHelper.getRequiredMap(body, "github", "headers");
88+
String xGithubEvent = PepperMapHelper.getOptionalString(headers, "X-GitHub-Event").orElse("");
89+
String xGithubDelivery = PepperMapHelper.getOptionalString(headers, "X-GitHub-Delivery").orElse("");
90+
String xGithubSignature256 = PepperMapHelper.getOptionalString(headers, "X-GitHub-Signature-256").orElse("");
91+
92+
return new GithubWebhookEvent(xGithubEvent,
93+
xGithubDelivery,
94+
xGithubSignature256,
95+
PepperMapHelper.getRequiredMap(body, "github", "body"));
8896
}
8997
}

github/src/main/java/eu/solven/cleanthat/code_provider/github/refs/GithubPRCodeProvider.java

+1-41
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,12 @@
11
package eu.solven.cleanthat.code_provider.github.refs;
22

33
import java.io.IOException;
4-
import java.io.UncheckedIOException;
5-
import java.nio.file.Files;
6-
import java.nio.file.Path;
74
import java.util.Collection;
85
import java.util.List;
96
import java.util.Map;
107
import java.util.Optional;
118
import java.util.function.Consumer;
129

13-
import org.eclipse.jgit.api.Git;
14-
import org.eclipse.jgit.api.errors.GitAPIException;
1510
import org.kohsuke.github.GHFileNotFoundException;
1611
import org.kohsuke.github.GHPullRequest;
1712
import org.kohsuke.github.GHPullRequestFileDetail;
@@ -54,11 +49,6 @@ public void listFiles(Consumer<ICodeProviderFile> consumer) throws IOException {
5449
});
5550
}
5651

57-
// @Override
58-
// public boolean deprecatedFileIsRemoved(Object file) {
59-
// return "removed".equals(((GHPullRequestFileDetail) file).getStatus());
60-
// }
61-
6252
public static String loadContent(GHPullRequest pr, String filename) throws IOException {
6353
GHRepository repository = pr.getRepository();
6454
String sha1 = pr.getHead().getSha();
@@ -78,14 +68,7 @@ public String getTitle() {
7868
@Override
7969
public void persistChanges(Map<String, String> pathToMutatedContent,
8070
List<String> prComments,
81-
Collection<String> prLabels
82-
// ,
83-
// Optional<String> targetBranch
84-
) {
85-
// if (targetBranch.isPresent()) {
86-
// throw new UnsupportedOperationException("TODO");
87-
// }
88-
71+
Collection<String> prLabels) {
8972
String refName = pr.getHead().getRef();
9073
GHRepository repo = pr.getRepository();
9174

@@ -119,27 +102,4 @@ public Optional<String> loadContentForPath(String path) throws IOException {
119102
public String getRepoUri() {
120103
return pr.getRepository().getGitTransportUrl();
121104
}
122-
123-
// @Override
124-
public Git makeGitRepo() {
125-
Path tmpDir;
126-
try {
127-
tmpDir = Files.createTempDirectory("cleanthat-clone");
128-
} catch (IOException e) {
129-
throw new UncheckedIOException(e);
130-
}
131-
132-
try {
133-
GHRepository repo = pr.getRepository();
134-
return Git.cloneRepository()
135-
.setURI(repo.getGitTransportUrl())
136-
.setDirectory(tmpDir.toFile())
137-
.setBranch(pr.getHead().getRef())
138-
.setCloneAllBranches(false)
139-
.setCloneSubmodules(false)
140-
.call();
141-
} catch (GitAPIException e) {
142-
throw new IllegalArgumentException(e);
143-
}
144-
}
145105
}

github/src/main/java/eu/solven/cleanthat/code_provider/github/refs/GithubRefDiffCodeProvider.java

+2-15
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,10 @@
3434
*/
3535
public class GithubRefDiffCodeProvider extends AGithubCodeProvider
3636
implements IListOnlyModifiedFiles, ICodeProviderWriter {
37+
private static final Logger LOGGER = LoggerFactory.getLogger(GithubRefDiffCodeProvider.class);
3738

3839
private static final int LIMIT_COMMIT_IN_COMPARE = 250;
3940

40-
private static final Logger LOGGER = LoggerFactory.getLogger(GithubRefDiffCodeProvider.class);
41-
4241
final String token;
4342
final GHRepository baseRepository;
4443
final GHRef base;
@@ -82,11 +81,6 @@ public void listFiles(Consumer<ICodeProviderFile> consumer) throws IOException {
8281
});
8382
}
8483

85-
// @Override
86-
// public boolean deprecatedFileIsRemoved(Object file) {
87-
// return "removed".equals(((GHPullRequestFileDetail) file).getStatus());
88-
// }
89-
9084
@Override
9185
public String getHtmlUrl() {
9286
return diffSupplier.get().getHtmlUrl().toExternalForm();
@@ -100,14 +94,7 @@ public String getTitle() {
10094
@Override
10195
public void persistChanges(Map<String, String> pathToMutatedContent,
10296
List<String> prComments,
103-
Collection<String> prLabels
104-
// ,
105-
// Optional<String> targetBranch
106-
) {
107-
// if (targetBranch.isPresent()) {
108-
// throw new UnsupportedOperationException("TODO");
109-
// }
110-
97+
Collection<String> prLabels) {
11198
String refName = head.getRef();
11299
GHRepository repo = baseRepository;
113100

lambda/src/main/java/eu/solven/cleanthat/lambda/AWebhooksLambdaFunction.java

+35-8
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.util.Collection;
44
import java.util.Collections;
5+
import java.util.LinkedHashMap;
56
import java.util.List;
67
import java.util.Map;
78
import java.util.Optional;
@@ -126,8 +127,30 @@ public IWebhookEvent wrapAsEvent(Map<String, ?> input) {
126127
if (input.containsKey(KEY_BODY) && input.containsKey("headers")) {
127128
// see CheckWebhooksLambdaFunction.saveToDynamoDb(String, IWebhookEvent, AmazonDynamoDB)
128129
// event = SaveToDynamoDb.NONE;
129-
event = new CleanThatWebhookEvent((Map<String, ?>) input.get("headers"),
130-
(Map<String, ?>) input.get(KEY_BODY));
130+
131+
Map<String, Object> rootBody = PepperMapHelper.getRequiredMap(input, KEY_BODY);
132+
133+
if (rootBody.containsKey("github")) {
134+
Map<String, Object> github = PepperMapHelper.getRequiredMap(rootBody, "github");
135+
136+
Map<String, Object> githubHeaders = PepperMapHelper.getRequiredMap(github, "headers");
137+
138+
// Headers is typically empty as we fails fetching headers from API Gateway
139+
if (!githubHeaders.containsKey("X-GitHub-Delivery")) {
140+
githubHeaders = new LinkedHashMap<>(githubHeaders);
141+
142+
// TODO We should push this to the headers next to the actual github body, which may be deeper
143+
// We should also push it when the initial event is received
144+
String eventKey = PepperMapHelper.getRequiredString(input, "X-GitHub-Delivery");
145+
githubHeaders.put("X-GitHub-Delivery", eventKey);
146+
147+
// Install the updated headers
148+
github.put("headers", githubHeaders);
149+
}
150+
}
151+
152+
Map<String, Object> headers = PepperMapHelper.getRequiredMap(input, "headers");
153+
event = new CleanThatWebhookEvent(headers, rootBody);
131154
} else {
132155
event = new GithubWebhookEvent(input);
133156
}
@@ -144,12 +167,11 @@ public IWebhookEvent wrapAsEvent(Map<String, ?> input) {
144167

145168
String eventName = PepperMapHelper.getRequiredString(r, "eventName");
146169

147-
if (!"INSERT".equals(eventName) && !"MODIFY".equals(eventName)) {
148-
// https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_streams_Record.html
149-
// We are in a REMOVE event
150-
LOGGER.info("We discard eventName={}", eventName);
151-
asMap = Collections.emptyMap();
152-
} else {
170+
if (
171+
// INSERT event: this is something new process
172+
"INSERT".equals(eventName)
173+
// MODIFY event: this is typically an admin which modify an event manually
174+
|| "MODIFY".equals(eventName)) {
153175
Map<String, ?> dynamoDbMap = PepperMapHelper.getRequiredMap(r, "dynamodb", "NewImage");
154176

155177
// We receive from DynamoDb a json in a special format
@@ -164,6 +186,11 @@ public IWebhookEvent wrapAsEvent(Map<String, ?> input) {
164186

165187
LOGGER.info("Processing X-GitHub-Delivery={}",
166188
PepperMapHelper.getRequiredString(asMap, "X-GitHub-Delivery"));
189+
} else {
190+
// https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_streams_Record.html
191+
// We are in a REMOVE event
192+
LOGGER.info("We discard eventName={}", eventName);
193+
asMap = Collections.emptyMap();
167194
}
168195
return asMap;
169196
}

lambda/src/test/java/eu/solven/cleanthat/aws/dynamodb/ITCheckRepoLocallyInDynamoDb.java renamed to lambda/src/test/java/eu/solven/cleanthat/aws/dynamodb/ITCheckDynamoDbEventLocally.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
@RunWith(SpringRunner.class)
2727
@ContextConfiguration(classes = { ExecuteCleaningWebhooksLambdaFunction.class })
28-
public class ITCheckRepoLocallyInDynamoDb {
28+
public class ITCheckDynamoDbEventLocally {
2929
GithubRefCleaner cleaner;
3030

3131
@Autowired

lambda/src/test/java/eu/solven/cleanthat/aws/dynamodb/ITCleanEventLocallyInDynamoDb.java renamed to lambda/src/test/java/eu/solven/cleanthat/aws/dynamodb/ITCleanDynamoDbEventLocally.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
@RunWith(SpringRunner.class)
2727
@ContextConfiguration(classes = { ExecuteCleaningWebhooksLambdaFunction.class })
28-
public class ITCleanEventLocallyInDynamoDb {
28+
public class ITCleanDynamoDbEventLocally {
2929
GithubRefCleaner cleaner;
3030

3131
@Autowired

0 commit comments

Comments
 (0)