Skip to content

Commit 51f3f1c

Browse files
committed
Avoid instantiating installation clients when not desirable
- if all events are raw events and we don't inject a GitHub client, we don't need an installation client - if the event is installation.deleted, we don't have access to the installation anymore so we shouldn't instantiate an installation client Fixes #806
1 parent 39ca136 commit 51f3f1c

File tree

5 files changed

+71
-23
lines changed

5 files changed

+71
-23
lines changed

deployment/src/main/java/io/quarkiverse/githubapp/deployment/DispatchingConfiguration.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.quarkiverse.githubapp.deployment;
22

33
import static io.quarkiverse.githubapp.deployment.GitHubAppDotNames.DYNAMIC_GRAPHQL_CLIENT;
4+
import static io.quarkiverse.githubapp.deployment.GitHubAppDotNames.GITHUB;
45
import static io.quarkiverse.githubapp.deployment.GitHubAppDotNames.GITHUB_EVENT;
56

67
import java.util.HashSet;
@@ -55,6 +56,22 @@ public void addEventDispatchingMethod(EventDispatchingMethod eventDispatchingMet
5556
.add(eventDispatchingMethod);
5657
}
5758

59+
public boolean requiresGitHubClient() {
60+
for (EventDispatchingConfiguration eventDispatchingConfiguration : eventConfigurations.values()) {
61+
// if any event is a GitHub API GHEventPayload (i.e. not a @RawEvent)
62+
if (eventDispatchingConfiguration.getPayloadType() != null) {
63+
return true;
64+
}
65+
}
66+
for (EventDispatchingMethod eventDispatchingMethod : methods.values().stream().flatMap(Set::stream).toList()) {
67+
if (eventDispatchingMethod.requiresGitHubClient()) {
68+
return true;
69+
}
70+
}
71+
72+
return false;
73+
}
74+
5875
public boolean requiresGraphQLClient() {
5976
for (EventDispatchingMethod eventDispatchingMethod : methods.values().stream().flatMap(Set::stream).toList()) {
6077
if (eventDispatchingMethod.requiresGraphQLClient()) {
@@ -247,6 +264,10 @@ public boolean requiresGraphQLClient() {
247264
return method.parameterTypes().stream().map(Type::name).anyMatch(DYNAMIC_GRAPHQL_CLIENT::equals);
248265
}
249266

267+
public boolean requiresGitHubClient() {
268+
return method.parameterTypes().stream().map(Type::name).anyMatch(GITHUB::equals);
269+
}
270+
250271
@Override
251272
public int compareTo(EventDispatchingMethod other) {
252273
int classNameCompareTo = method.declaringClass().name().compareTo(other.method.declaringClass().name());

deployment/src/main/java/io/quarkiverse/githubapp/deployment/GitHubAppProcessor.java

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,9 @@ private static void generateDispatcher(ClassOutput beanClassOutput,
608608

609609
ResultHandle gitHubEventRh = dispatchMethodCreator.getMethodParam(0);
610610

611+
ResultHandle supportsInstallationRh = dispatchMethodCreator.invokeInterfaceMethod(
612+
MethodDescriptor.ofMethod(GitHubEvent.class, "supportsInstallation", boolean.class),
613+
gitHubEventRh);
611614
ResultHandle installationIdRh = dispatchMethodCreator.invokeInterfaceMethod(
612615
MethodDescriptor.ofMethod(GitHubEvent.class, "getInstallationId", Long.class),
613616
gitHubEventRh);
@@ -623,45 +626,55 @@ private static void generateDispatcher(ClassOutput beanClassOutput,
623626

624627
TryBlock tryBlock = dispatchMethodCreator.tryBlock();
625628

626-
// if the installation id is defined, we can push the installation client
629+
// if the event supports installation, we can push the installation client
627630
// if not, we have to use the very limited application client
628631
AssignableResultHandle gitHubRh = tryBlock.createVariable(GitHub.class);
629632
AssignableResultHandle gitHubGraphQLClientRh = tryBlock.createVariable(DynamicGraphQLClient.class);
630-
BranchResult testInstallationId = tryBlock.ifNotNull(installationIdRh);
631-
BytecodeCreator installationIdSet = testInstallationId.trueBranch();
632-
installationIdSet.assign(gitHubRh, installationIdSet.invokeVirtualMethod(
633-
MethodDescriptor.ofMethod(GitHubService.class, "getInstallationClient", GitHub.class, long.class),
634-
installationIdSet.readInstanceField(
635-
FieldDescriptor.of(dispatcherClassCreator.getClassName(), GITHUB_SERVICE_FIELD, GitHubService.class),
636-
installationIdSet.getThis()),
637-
installationIdRh));
633+
BranchResult testSupportsInstallation = tryBlock.ifTrue(supportsInstallationRh);
634+
BytecodeCreator supportsInstallationTrue = testSupportsInstallation.trueBranch();
635+
if (dispatchingConfiguration.requiresGitHubClient()) {
636+
supportsInstallationTrue.assign(gitHubRh, supportsInstallationTrue.invokeVirtualMethod(
637+
MethodDescriptor.ofMethod(GitHubService.class, "getInstallationClient", GitHub.class, long.class),
638+
supportsInstallationTrue.readInstanceField(
639+
FieldDescriptor.of(dispatcherClassCreator.getClassName(), GITHUB_SERVICE_FIELD,
640+
GitHubService.class),
641+
supportsInstallationTrue.getThis()),
642+
installationIdRh));
643+
} else {
644+
supportsInstallationTrue.assign(gitHubRh, supportsInstallationTrue.loadNull());
645+
}
638646
if (dispatchingConfiguration.requiresGraphQLClient()) {
639-
installationIdSet.assign(gitHubGraphQLClientRh, installationIdSet.invokeVirtualMethod(
647+
supportsInstallationTrue.assign(gitHubGraphQLClientRh, supportsInstallationTrue.invokeVirtualMethod(
640648
MethodDescriptor.ofMethod(GitHubService.class, "getInstallationGraphQLClient", DynamicGraphQLClient.class,
641649
long.class),
642-
installationIdSet.readInstanceField(
650+
supportsInstallationTrue.readInstanceField(
643651
FieldDescriptor.of(dispatcherClassCreator.getClassName(), GITHUB_SERVICE_FIELD,
644652
GitHubService.class),
645-
installationIdSet.getThis()),
653+
supportsInstallationTrue.getThis()),
646654
installationIdRh));
647655
} else {
648-
installationIdSet.assign(gitHubGraphQLClientRh, installationIdSet.loadNull());
656+
supportsInstallationTrue.assign(gitHubGraphQLClientRh, supportsInstallationTrue.loadNull());
657+
}
658+
BytecodeCreator supportsInstallationFalse = testSupportsInstallation.falseBranch();
659+
if (dispatchingConfiguration.requiresGitHubClient()) {
660+
supportsInstallationFalse.assign(gitHubRh, supportsInstallationFalse.invokeVirtualMethod(
661+
MethodDescriptor.ofMethod(GitHubService.class, "getTokenOrApplicationClient", GitHub.class),
662+
supportsInstallationFalse.readInstanceField(
663+
FieldDescriptor.of(dispatcherClassCreator.getClassName(), GITHUB_SERVICE_FIELD,
664+
GitHubService.class),
665+
supportsInstallationFalse.getThis())));
666+
} else {
667+
supportsInstallationFalse.assign(gitHubRh, supportsInstallationFalse.loadNull());
649668
}
650-
BytecodeCreator installationIdNull = testInstallationId.falseBranch();
651-
installationIdNull.assign(gitHubRh, installationIdNull.invokeVirtualMethod(
652-
MethodDescriptor.ofMethod(GitHubService.class, "getTokenOrApplicationClient", GitHub.class),
653-
installationIdNull.readInstanceField(
654-
FieldDescriptor.of(dispatcherClassCreator.getClassName(), GITHUB_SERVICE_FIELD, GitHubService.class),
655-
installationIdNull.getThis())));
656669
if (dispatchingConfiguration.requiresGraphQLClient()) {
657-
installationIdNull.assign(gitHubGraphQLClientRh, installationIdNull.invokeVirtualMethod(
670+
supportsInstallationFalse.assign(gitHubGraphQLClientRh, supportsInstallationFalse.invokeVirtualMethod(
658671
MethodDescriptor.ofMethod(GitHubService.class, "getTokenGraphQLClientOrNull", DynamicGraphQLClient.class),
659-
installationIdNull.readInstanceField(
672+
supportsInstallationFalse.readInstanceField(
660673
FieldDescriptor.of(dispatcherClassCreator.getClassName(), GITHUB_SERVICE_FIELD,
661674
GitHubService.class),
662-
installationIdNull.getThis())));
675+
supportsInstallationFalse.getThis())));
663676
} else {
664-
installationIdNull.assign(gitHubGraphQLClientRh, installationIdNull.loadNull());
677+
supportsInstallationFalse.assign(gitHubGraphQLClientRh, supportsInstallationFalse.loadNull());
665678
}
666679

667680
for (EventDispatchingConfiguration eventDispatchingConfiguration : dispatchingConfiguration.getEventConfigurations()

runtime/src/main/java/io/quarkiverse/githubapp/GitHubEvent.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ public interface GitHubEvent {
1010

1111
Long getInstallationId();
1212

13+
boolean supportsInstallation();
14+
1315
Optional<String> getAppName();
1416

1517
String getDeliveryId();

runtime/src/main/java/io/quarkiverse/githubapp/runtime/SimpleGitHubEvent.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
*/
1717
public class SimpleGitHubEvent implements GitHubEvent {
1818

19+
private static final String INSTALLATION_DELETED = "installation.deleted";
20+
1921
private final Long installationId;
2022

2123
private final Optional<String> appName;
@@ -64,6 +66,11 @@ public Long getInstallationId() {
6466
return installationId;
6567
}
6668

69+
@Override
70+
public boolean supportsInstallation() {
71+
return installationId != null && !INSTALLATION_DELETED.equals(eventAction);
72+
}
73+
6774
@Override
6875
public Optional<String> getAppName() {
6976
return appName;

runtime/src/main/java/io/quarkiverse/githubapp/runtime/telemetry/opentelemetry/OpenTelemetryDecoratedGitHubEvent.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ public Long getInstallationId() {
2424
return originalGitHubEvent.getInstallationId();
2525
}
2626

27+
@Override
28+
public boolean supportsInstallation() {
29+
return originalGitHubEvent.supportsInstallation();
30+
}
31+
2732
@Override
2833
public Optional<String> getAppName() {
2934
return originalGitHubEvent.getAppName();

0 commit comments

Comments
 (0)