Skip to content

Commit 1028492

Browse files
committed
Fix fetching single files from local clone
1 parent 11c7877 commit 1028492

File tree

9 files changed

+39
-21
lines changed

9 files changed

+39
-21
lines changed

code-cleaners/src/main/java/eu/solven/cleanthat/formatter/CodeProviderFormatter.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ protected AtomicLongMap<String> processFiles(ICodeProvider pr,
251251
Thread.currentThread().interrupt();
252252
throw new RuntimeException(e);
253253
} catch (ExecutionException e) {
254-
throw new RuntimeException("Arg", e);
254+
throw new RuntimeException("Issue while one of the asynchronous tasks", e);
255255
}
256256
}
257257

@@ -286,7 +286,9 @@ public String loadCodeOptMutated(ICodeProvider codeProvider,
286286
String code = optAlreadyMutated.orElseGet(() -> {
287287
try {
288288
Optional<String> optContent = codeProvider.loadContentForPath(filePath);
289-
return optContent.get();
289+
290+
return optContent
291+
.orElseThrow(() -> new IllegalStateException("Issue fiding code for path=" + filePath));
290292
} catch (IOException e) {
291293
throw new UncheckedIOException(e);
292294
}

github/src/main/java/eu/solven/cleanthat/code_provider/github/code_provider/AGithubSha1CodeProvider.java

+11-6
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,17 @@ public String deprecatedGetFilePath(Object file) {
130130

131131
@Override
132132
public Optional<String> loadContentForPath(String path) throws IOException {
133-
try {
134-
return Optional.of(loadContent(repo, path, getSha1()));
135-
} catch (GHFileNotFoundException e) {
136-
LOGGER.trace("We miss: {}", path, e);
137-
LOGGER.debug("We miss: {}", path);
138-
return Optional.empty();
133+
if (helper.localClone.get() != null) {
134+
// We have a local clone: load the file from it
135+
return helper.localClone.get().loadContentForPath(path);
136+
} else {
137+
try {
138+
return Optional.of(loadContent(repo, path, getSha1()));
139+
} catch (GHFileNotFoundException e) {
140+
LOGGER.trace("We miss: {}", path, e);
141+
LOGGER.debug("We miss: {}", path);
142+
return Optional.empty();
143+
}
139144
}
140145
}
141146

github/src/main/java/eu/solven/cleanthat/code_provider/github/code_provider/GithubSha1CodeProviderHelper.java

+5
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
public class GithubSha1CodeProviderHelper {
3333
private static final Logger LOGGER = LoggerFactory.getLogger(GithubSha1CodeProviderHelper.class);
3434

35+
// To be compared with the limit of 5000 calls per hour per installation
3536
private static final int MAX_FILE_BEFORE_CLONING = 512;
3637

3738
private static final boolean ZIP_ELSE_CLONE = true;
@@ -58,6 +59,10 @@ public void listFiles(Consumer<ICodeProviderFile> consumer) throws IOException {
5859
localClone.get().listFiles(consumer);
5960
}
6061

62+
/**
63+
*
64+
* @return true if we indeed clone locally. False if already cloned locally
65+
*/
6166
@SuppressWarnings("PMD.CloseResource")
6267
protected boolean ensureLocalClone() {
6368
// TODO Tests against multiple calls: the repo shall be cloned only once

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

+10
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,16 @@ public void doExecuteWebhookEvent(ICodeCleanerFactory cleanerFactory, IWebhookEv
511511
} else {
512512
LOGGER.debug("The changes would have been committed directly in the head branch");
513513
}
514+
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+
}
523+
}
514524
}
515525

516526
public Supplier<GHRef> prepareHeadSupplier(WebhookRelevancyResult relevancyResult,

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

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

33
import java.util.Objects;
4-
import java.util.concurrent.atomic.AtomicReference;
54

65
import org.kohsuke.github.GHRef;
76
import org.kohsuke.github.GHRepository;
87

98
import eu.solven.cleanthat.code_provider.github.code_provider.AGithubSha1CodeProvider;
109
import eu.solven.cleanthat.codeprovider.ICodeProvider;
11-
import eu.solven.cleanthat.jgit.JGitCodeProvider;
1210

1311
/**
1412
* An {@link ICodeProvider} for Github pull-requests
@@ -18,8 +16,6 @@
1816
public class GithubRefCodeProvider extends AGithubSha1CodeProvider {
1917
final GHRef ref;
2018

21-
final AtomicReference<JGitCodeProvider> localClone = new AtomicReference<>();
22-
2319
public GithubRefCodeProvider(String token, GHRepository repo, GHRef ref) {
2420
super(token, repo);
2521

java/src/main/java/eu/solven/cleanthat/java/mutators/RulesJavaMutator.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ public List<IClassTransformer> getTransformers() {
125125

126126
@Override
127127
public void registerCodeStyleFixer(IStyleEnforcer codeStyleFixer) {
128-
LOGGER.info("We register {} into {}", codeStyleFixer, this);
128+
// TODO This could be in INFO, but it is called once per file (unexpectedly)
129+
LOGGER.debug("We register {} into {}", codeStyleFixer, this);
129130
this.optCodeStyleFixer = Optional.of(codeStyleFixer);
130131
}
131132

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ public abstract class AWebhooksLambdaFunction extends ACleanThatXxxFunction {
9494
} else {
9595
// This would happen on Lambda direct invocation
9696
// But we always try to rely on events(SQS, DynamoDB, ...)
97+
// It may also happens in localhot invocation (e.g. ITCleanEventLocallyInDynamoDb)
9798
try {
9899
LOGGER.warn("TODO Add unit-test for: {}", objectMapper.writeValueAsString(input));
99100
} catch (JsonProcessingException ee) {
@@ -178,7 +179,7 @@ public IWebhookEvent wrapAsEvent(Map<String, ?> input) {
178179

179180
// SQS transfer the body 'as is'
180181
try {
181-
asMap = (Map<String, ?>) objectMapper.readValue(body, Map.class);
182+
asMap = objectMapper.readValue(body, Map.class);
182183
} catch (JsonProcessingException e) {
183184
LOGGER.warn("Issue while parsing: {}", body);
184185
LOGGER.warn("Issue while parsing body", e);

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

+5-3
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@
2121
import eu.solven.cleanthat.code_provider.github.refs.GithubRefCleaner;
2222
import eu.solven.cleanthat.lambda.AWebhooksLambdaFunction;
2323
import eu.solven.cleanthat.lambda.dynamodb.SaveToDynamoDb;
24-
import eu.solven.cleanthat.lambda.step1_checkconfiguration.CheckConfigWebhooksLambdaFunction;
24+
import eu.solven.cleanthat.lambda.step2_executeclean.ExecuteCleaningWebhooksLambdaFunction;
2525

2626
@RunWith(SpringRunner.class)
27-
@ContextConfiguration(classes = { CheckConfigWebhooksLambdaFunction.class })
27+
@ContextConfiguration(classes = { ExecuteCleaningWebhooksLambdaFunction.class })
2828
public class ITCleanEventLocallyInDynamoDb {
2929
GithubRefCleaner cleaner;
3030

@@ -38,7 +38,9 @@ public class ITCleanEventLocallyInDynamoDb {
3838
public void testInitWithDefaultConfiguration() throws IOException, JOSEException {
3939
AmazonDynamoDB dynamoDbClient = SaveToDynamoDb.makeDynamoDbClient();
4040

41-
String key = "random-01be8d8f-fde0-4895-8689-70288ace3819";
41+
// This is logged by: e.s.c.lambda.AWebhooksLambdaFunction|parseDynamoDbEvent
42+
// You can search logs for this key, in order to process given event locally
43+
String key = "random-6787961d-e2b6-4ec2-8df5-7a3db5722b82";
4244
GetItemResult item = dynamoDbClient.getItem(new GetItemRequest().withTableName("cleanthat_accepted_events")
4345
.withKey(Map.of("X-GitHub-Delivery", new AttributeValue().withS(key))));
4446

runnable/src/main/java/eu/solven/cleanthat/lambda/ACleanThatXxxFunction.java

-4
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,13 @@ public abstract class ACleanThatXxxFunction extends ACleanThatXxxApplication {
2424
try {
2525
return unsafeProcessOneEvent(input);
2626
} catch (RuntimeException e) {
27-
// if (input instanceof GithubWebhookEvent) {
2827
Map<String, ?> body = input.getBody();
2928

3029
try {
3130
LOGGER.warn("Issue with IWebhookEvent. body={}", new ObjectMapper().writeValueAsString(body));
3231
} catch (JsonProcessingException e1) {
3332
LOGGER.warn("Issue printing as json. body: {}", body);
3433
}
35-
// } else {
36-
// LOGGER.warn("Issue with {}", input.getClass());
37-
// }
3834

3935
RuntimeException wrapped = new RuntimeException(e);
4036
Sentry.captureException(wrapped, "Lambda");

0 commit comments

Comments
 (0)