diff --git a/src/main/java/org/commonhaus/automation/Main.java b/src/main/java/org/commonhaus/automation/Main.java index f57f921..acff3ed 100644 --- a/src/main/java/org/commonhaus/automation/Main.java +++ b/src/main/java/org/commonhaus/automation/Main.java @@ -1,5 +1,7 @@ package org.commonhaus.automation; +import java.nio.file.Path; + import jakarta.enterprise.event.Observes; import jakarta.inject.Inject; @@ -14,6 +16,7 @@ public class Main { public static void main(String... args) { + System.out.println("cwd=" + Path.of("").toAbsolutePath()); Quarkus.run(ApplicationRoot.class, args); } @@ -24,7 +27,7 @@ public static class ApplicationRoot implements QuarkusApplication { @Override public int run(String... args) throws Exception { - System.out.println("Do startup logic here. dryRun=" + botConfig.isDryRun()); + System.out.println("dryRun=" + botConfig.isDryRun()); // Reminder: stop can happen elsewhere with Quarkus.asyncExit() Quarkus.waitForExit(); diff --git a/src/main/java/org/commonhaus/automation/github/LabelChanges.java b/src/main/java/org/commonhaus/automation/github/LabelChanges.java index fc0a837..59dcbbe 100644 --- a/src/main/java/org/commonhaus/automation/github/LabelChanges.java +++ b/src/main/java/org/commonhaus/automation/github/LabelChanges.java @@ -3,7 +3,6 @@ import jakarta.inject.Inject; import org.commonhaus.automation.github.QueryHelper.QueryContext; -import org.commonhaus.automation.github.model.ActionType; import org.commonhaus.automation.github.model.DataDiscussion; import org.commonhaus.automation.github.model.DataLabel; import org.commonhaus.automation.github.model.EventPayload; @@ -53,13 +52,7 @@ void onDiscussionLabelChangeEvent(GitHubEvent event, GitHub github, } Log.debugf("LabelChanges (%s): discussion %s changed label %s", event.getEventAction(), discussion.id, label); - - // Don't fetch labels first: only add/remove if it's an item/id we know about - if (initialData.getActionType() == ActionType.labeled) { - queryContext.addCachedLabel(discussion.id, label); - } else { - queryContext.removeCachedLabel(discussion.id, label); - } + queryContext.modifyLabels(discussion.id, label, initialData.getActionType()); } /** @@ -84,14 +77,6 @@ void onRepositoryLabelChange(GitHubEvent event, GitHub github, String cacheId = labelPayload.getRepository().getNodeId(); Log.debugf("LabelChanges (%s): repository %s changed label %s", event.getEventAction(), cacheId, label); - - // Don't fetch labels first: only add/remove if it's an item/id we know about - if (initialData.getActionType() == ActionType.created) { - queryContext.addCachedLabel(cacheId, label); - } else if (initialData.getActionType() == ActionType.edited) { - queryContext.updateCachedLabel(cacheId, label); - } else { - queryContext.removeCachedLabel(cacheId, label); - } + queryContext.modifyLabels(cacheId, label, initialData.getActionType()); } } diff --git a/src/main/java/org/commonhaus/automation/github/QueryHelper.java b/src/main/java/org/commonhaus/automation/github/QueryHelper.java index 6a0458a..eaa9d1b 100644 --- a/src/main/java/org/commonhaus/automation/github/QueryHelper.java +++ b/src/main/java/org/commonhaus/automation/github/QueryHelper.java @@ -9,11 +9,13 @@ import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.function.BiFunction; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import org.commonhaus.automation.AppConfig; +import org.commonhaus.automation.github.model.ActionType; import org.commonhaus.automation.github.model.DataLabel; import org.kohsuke.github.GHIOException; import org.kohsuke.github.GitHub; @@ -29,10 +31,61 @@ @ApplicationScoped public class QueryHelper { - static final String LABEL = "labels"; - static Cache cache = Caffeine.newBuilder() - .expireAfterWrite(6, TimeUnit.HOURS) - .build(); + public enum QueryCache { + LABELS(Caffeine.newBuilder() + .expireAfterWrite(6, TimeUnit.HOURS) + .build()), + GLOB(Caffeine.newBuilder() + .maximumSize(200) + .build()); + + private final Cache cache; + + private QueryCache(Cache cache) { + this.cache = cache; + } + + @SuppressWarnings("unchecked") + public T getCachedValue(String key, Class type) { + T result = (T) cache.getIfPresent(key); + if (result != null) { + Log.debugf(":: HIT :: %s/%s ::: ", this.name(), key); + } else { + Log.debugf(":: MISS :: %s/%s ::: ", this.name(), key); + } + return result; + } + + /** + * Put a value into the cache + * + * @param key + * @param value to be cached + * @return new value + */ + public T putCachedValue(String key, T value) { + Log.debugf(":: PUT :: %s/%s ::: ", this.name(), key); + cache.put(key, value); + return value; + } + + @SuppressWarnings("unchecked") + public T computeIfPresent(String key, BiFunction mappingFunction) { + Log.debugf(":: UPDATE :: %s/%s ::: ", this.name(), key); + Object value = cache.asMap().computeIfPresent(key, mappingFunction); + return (T) value; + } + + public void invalidate(String key) { + Log.debugf(":: INVALIDATE :: %s/%s ::: ", this.name(), key); + cache.invalidate(key); + } + + public void invalidateAll() { + Log.debugf(":: INVALIDATE ALL :: %s ::: ", this.name()); + cache.invalidateAll(); + } + } @Inject AppConfig botConfig; @@ -48,23 +101,6 @@ public QueryContext newQueryContext(EventData eventData, GitHub github) { return newQueryContext(eventData).addExisting(github); } - public static T getCache(String itemId, String key, Class type) { - String idx = itemId + ":" + key; - T result = (T) cache.getIfPresent(idx); - if (result != null) { - System.out.println(":: HIT :: " + idx + " ::: "); - } else { - System.out.println(":: MISS :: " + idx + " ::: "); - } - return result; - } - - public static void putCache(String itemId, String key, Object value) { - String idx = itemId + ":" + key; - System.out.println(":: UPDATE :: " + idx + " ::: "); - cache.put(idx, value); - } - /** * QueryContext is a helper class to encapsulate the GitHub API and GraphQL * client. @@ -143,7 +179,7 @@ protected GitHub getGitHub() { @FunctionalInterface public interface GitHubParameterApiCall { - R apply(GitHub gh) throws GHIOException, IOException; + R apply(GitHub gh, boolean isDryRun) throws GHIOException, IOException; } /** @@ -159,7 +195,7 @@ public R execGitHubSync(GitHubParameterApiCall ghApiCall) { return null; } try { - return ghApiCall.apply(getGitHub()); + return ghApiCall.apply(getGitHub(), botConfig.isDryRun()); } catch (GHIOException e) { // TODO: Config to handle GHIOException (retry? quit? ensure notification?) Log.errorf(e, "[%s] Error making GH Request: %s", evt.installationId(), e.toString()); @@ -235,14 +271,6 @@ public Response execRepoQuerySync(String query, Map variables) { return execQuerySync(query, variables); } - public T getCache(String itemId, String key, Class type) { - return QueryHelper.getCache(itemId, key, type); - } - - public void putCache(String itemId, String key, Object value) { - QueryHelper.putCache(itemId, key, value); - } - /** * Get labels for the labelable item * @@ -250,14 +278,16 @@ public void putCache(String itemId, String key, Object value) { * @return collection of labels for the item */ public Collection getCachedLabels(String itemId) { - Collection labels = getCache(itemId, "labels", Collection.class); + @SuppressWarnings("unchecked") + Set labels = QueryHelper.QueryCache.LABELS.getCachedValue(itemId, Set.class); + if (labels != null) { return labels; } // fresh fetch graphQL fetch labels = DataLabel.queryLabels(this, itemId); if (labels != null) { - putCache(itemId, LABEL, labels); + QueryHelper.QueryCache.LABELS.putCachedValue(itemId, labels); } return labels; } @@ -269,63 +299,33 @@ public Collection getCachedLabels(String itemId) { * @param label Label to add * @return updated collection of labels for the item, or null if not cached */ - public Collection addCachedLabel(String cacheId, DataLabel label) { - Collection labels = getCachedLabels(cacheId); // ensure labels are cached (if not already) - if (labels == null) { - return null; - } - labels.add(label); - return labels; - } - - /** - * Remove a label from the list of known labels if the associated item exists - * - * @param cacheId Labelable item node id - * @param label Label to remove - * @return updated collection of labels for the item, or null if not cached - */ - public Collection removeCachedLabel(String cacheId, DataLabel label) { - Collection labels = getCachedLabels(cacheId); // ensure labels are cached (if not already) - if (labels == null) { - return null; - } - labels.remove(label); - return labels; - } - - /** - * Update a label in the cache if the associated item exists - * - * @param cacheId Labelable item node id - * @param label Label to update/replace - * @return updated collection of labels for the item, or null if not cached - */ - public Collection updateCachedLabel(String cacheId, DataLabel label) { - Collection labels = getCachedLabels(cacheId); // ensure labels are cached (if not already) - if (labels == null) { - return null; - } - // only the id is equals/hashcode -- so remove and add - labels.remove(label); - labels.add(label); - return labels; + public Collection modifyLabels(String cacheId, DataLabel label, ActionType action) { + return QueryHelper.QueryCache.LABELS.computeIfPresent(cacheId, (k, v) -> { + @SuppressWarnings("unchecked") + Set labels = (Set) v; + if (action == ActionType.deleted || action == ActionType.unlabeled || action == ActionType.edited) { + labels.remove(label); + } + if (action == ActionType.created || action == ActionType.labeled || action == ActionType.edited) { + labels.add(label); + } + return labels; + }); } /** - * Add a label from the list of known labels if the associated item has been seen/cached + * Replace cached labels with result of modification (post-mutation) * * @param cacheId Labelable item node id - * @param labels updated Set of labels (e.g. post modify command) - * @return updated collection of labels for the item, or null if not cached + * @param labels updated Set of labels (e.g. post-mutation) + * @return updated collection of labels */ - public Collection modifyLabels(String cacheId, List labels) { + public Collection updateLabels(String cacheId, List labels) { Set newLabels = DataLabel.modifyLabels(this, cacheId, labels); if (newLabels == null) { return null; } - QueryHelper.putCache(cacheId, LABEL, newLabels); - return newLabels; + return QueryHelper.QueryCache.LABELS.putCachedValue(cacheId, newLabels); } } } diff --git a/src/main/java/org/commonhaus/automation/github/actions/LabelAction.java b/src/main/java/org/commonhaus/automation/github/actions/LabelAction.java index e26ff71..354d278 100644 --- a/src/main/java/org/commonhaus/automation/github/actions/LabelAction.java +++ b/src/main/java/org/commonhaus/automation/github/actions/LabelAction.java @@ -50,7 +50,7 @@ public void apply(QueryContext queryContext) { if (!newLabels.isEmpty()) { // Modify labels and update cache - queryContext.modifyLabels(nodeId, newLabels); + queryContext.updateLabels(nodeId, newLabels); } } } diff --git a/src/main/java/org/commonhaus/automation/github/rules/MatchFilePath.java b/src/main/java/org/commonhaus/automation/github/rules/MatchFilePath.java index e8609eb..ff8598e 100644 --- a/src/main/java/org/commonhaus/automation/github/rules/MatchFilePath.java +++ b/src/main/java/org/commonhaus/automation/github/rules/MatchFilePath.java @@ -57,10 +57,9 @@ public boolean matches(QueryContext queryContext) { } MatchingEngine compileGlob(String filenamePattern) { - MatchingEngine me = QueryHelper.getCache("GLOB", filenamePattern, MatchingEngine.class); + MatchingEngine me = QueryHelper.QueryCache.GLOB.getCachedValue(filenamePattern, MatchingEngine.class); if (me == null) { - me = GlobPattern.compile(filenamePattern); - QueryHelper.putCache("GLOB", filenamePattern, me); + me = QueryHelper.QueryCache.GLOB.putCachedValue(filenamePattern, GlobPattern.compile(filenamePattern)); } return me; } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 7af8863..e4fbfa8 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -1,11 +1,11 @@ quarkus.log.category."org.commonhaus".level=DEBUG quarkus.log.category."io.quarkiverse".level=DEBUG quarkus.log.category."io.quarkus.cache".level=DEBUG + quarkus.package.vineflower.enabled=true -quarkus.log.file.enable=true -quarkus.log.file.async=true -quarkus.log.file.path=logs/debug.log -quarkus.log.file.level=TRACE +%prod.quarkus.log.file.enable=true +%prod.quarkus.log.file.async=true +%prod.quarkus.log.file.path=logs/bot.log +%prod.quarkus.log.file.level=INFO -quarkus.cache.caffeine."glob-cache".maximum-size=200 diff --git a/src/test/java/org/commonhaus/automation/github/NotifyLabelsTest.java b/src/test/java/org/commonhaus/automation/github/NotifyLabelsTest.java index d7cb9ff..7492314 100644 --- a/src/test/java/org/commonhaus/automation/github/NotifyLabelsTest.java +++ b/src/test/java/org/commonhaus/automation/github/NotifyLabelsTest.java @@ -33,8 +33,6 @@ import org.kohsuke.github.PagedIterator; import org.mockito.Mockito; -import com.hrakaroo.glob.MatchingEngine; - import io.quarkiverse.githubapp.testing.GitHubAppTest; import io.quarkus.test.junit.QuarkusTest; import io.smallrye.graphql.client.Response; @@ -45,6 +43,8 @@ public class NotifyLabelsTest { final String repoFullName = "commonhaus/automation-test"; final long repoId = 742099370; final long installationId = 46053716; + static final DataLabel bug = new DataLabel.Builder().name("bug").id("LA_kwDOLDuJqs8AAAABfqsdNQ").build(); + static final DataLabel notice = new DataLabel.Builder().name("notice").id("LA_kwDOLDuJqs8AAAABgn2hGA").build(); Response mockResponse(Path filename) { try { @@ -61,8 +61,7 @@ Response mockResponse(Path filename) { @BeforeEach void beforeEach() { - QueryHelper.cache.invalidateAll(); - QueryHelper.cache.cleanUp(); + QueryHelper.QueryCache.LABELS.invalidateAll(); } @Test @@ -142,7 +141,7 @@ void discussionCreatedReviewsLabeled() throws Exception { // preload the cache: no request to fetch repo labels (and check our work) Set existing = new HashSet<>(); existing.add(new DataLabel.Builder().name("notice").build()); - QueryHelper.putCache(repositoryId, QueryHelper.LABEL, existing); + QueryHelper.QueryCache.LABELS.putCachedValue(repositoryId, existing); verifyLabelCache(repositoryId, 1, List.of("notice")); Response bugLabel = mockResponse(Path.of("src/test/resources/github/queryLabelBug.json")); @@ -169,14 +168,13 @@ void discussionCreatedReviewsLabeled() throws Exception { @Test void discussionLabeled() throws Exception { // When a discussion is labeled, ... - // from src/test/resources/github/eventDiscussionLabeled.json String discussionId = "D_kwDOLDuJqs4AXNhB"; // preload the cache: no request to fetch labels (and check our work) Set existing = new HashSet<>(); - existing.add(new DataLabel.Builder().name("bug").build()); - QueryHelper.putCache(discussionId, QueryHelper.LABEL, existing); + existing.add(bug); + QueryHelper.QueryCache.LABELS.putCachedValue(discussionId, existing); verifyLabelCache(discussionId, 1, List.of("bug")); given() @@ -193,25 +191,48 @@ void discussionLabeled() throws Exception { verifyLabelCache(discussionId, 2, List.of("bug", "duplicate")); } + @Test + void discussionUnlabeledUnknown() throws Exception { + // When a discussion is unlabeled, ... + // from src/test/resources/github/eventDiscussionUnlabeled.json + String discussionId = "D_kwDOLDuJqs4AXNhB"; + + given() + .github(mocks -> { + mocks.configFile(RepositoryAppConfig.NAME).fromClasspath("/cf-label.yml"); + }) + .when().payloadFromClasspath("/github/eventDiscussionUnlabeled.json") + .event(GHEvent.DISCUSSION) + .then().github(mocks -> { + verify(mocks.installationGraphQLClient(installationId), times(0)) + .executeSync(anyString(), anyMap()); + verifyNoMoreInteractions(mocks.ghObjects()); + }); + + Assertions.assertNull(QueryHelper.QueryCache.LABELS.getCachedValue(discussionId, Set.class), + "Discussion labels cache should not exist"); + } + @Test void discussionUnlabeled() throws Exception { - // When a discussion is labeled, ... - // If we don't have the labels cached, we fetch them first + // When a discussion is unlabeled, ... // from src/test/resources/github/eventDiscussionUnlabeled.json String discussionId = "D_kwDOLDuJqs4AXNhB"; - Response bugLabel = mockResponse(Path.of("src/test/resources/github/queryLabelBug.json")); + + // preload the cache: no request to fetch labels (and check our work) + Set existing = new HashSet<>(); + existing.add(bug); + QueryHelper.QueryCache.LABELS.putCachedValue(discussionId, existing); + verifyLabelCache(discussionId, 1, List.of("bug")); given() .github(mocks -> { mocks.configFile(RepositoryAppConfig.NAME).fromClasspath("/cf-label.yml"); - when(mocks.installationGraphQLClient(installationId) - .executeSync(anyString(), anyMap())) - .thenReturn(bugLabel); }) .when().payloadFromClasspath("/github/eventDiscussionUnlabeled.json") .event(GHEvent.DISCUSSION) .then().github(mocks -> { - verify(mocks.installationGraphQLClient(installationId)) + verify(mocks.installationGraphQLClient(installationId), times(0)) .executeSync(anyString(), anyMap()); verifyNoMoreInteractions(mocks.ghObjects()); }); @@ -223,6 +244,7 @@ void discussionUnlabeled() throws Exception { void testRelevantPr() throws Exception { String prNodeId = "PR_kwDOLDuJqs5lPq17"; long id = 1698606459; + Response repoLabels = mockResponse(Path.of("src/test/resources/github/queryRepositoryLabelsNotice.json")); Response bugLabel = mockResponse(Path.of("src/test/resources/github/queryLabelBug.json")); Response modifiedLabel = mockResponse(Path.of("src/test/resources/github/addLabelsToLabelableResponse.json")); @@ -248,14 +270,38 @@ void testRelevantPr() throws Exception { }); verifyLabelCache(prNodeId, 1, List.of("notice")); - Assertions.assertNotNull(QueryHelper.getCache("GLOB", "bylaws/*", Object.class), + Assertions.assertNotNull(QueryHelper.QueryCache.GLOB.getCachedValue("bylaws/*", Object.class), "bylaws/* GLOB cache should exist"); - Assertions.assertNull(QueryHelper.getCache("GLOB", "policy/*", Object.class), + Assertions.assertNull(QueryHelper.QueryCache.GLOB.getCachedValue("policy/*", Object.class), "policy/* GLOB cache should not exist"); } - private void verifyLabelCache(String discussionId, int size, List expectedLabels) { - Collection labels = QueryHelper.getCache(discussionId, "labels", Collection.class); + @Test + public void testLabelChanged() throws Exception { + String repoId = "R_kgDOLDuJqg"; + + // preload the cache: no request to fetch labels (and check our work) + Set existing = new HashSet<>(); + existing.add(bug); + QueryHelper.QueryCache.LABELS.putCachedValue(repoId, existing); + verifyLabelCache(repoId, 1, List.of("bug")); + + given() + .github(mocks -> { + mocks.configFile(RepositoryAppConfig.NAME).fromClasspath("/cf-label.yml"); + }) + .when().payloadFromClasspath("/github/eventLabelCreated.json") + .event(GHEvent.LABEL) + .then().github(mocks -> { + verifyNoMoreInteractions(mocks.ghObjects()); + }); + + verifyLabelCache(repoId, 2, List.of("bug", "notice")); + } + + private void verifyLabelCache(String labeledId, int size, List expectedLabels) { + @SuppressWarnings("unchecked") + Set labels = QueryHelper.QueryCache.LABELS.getCachedValue(labeledId, Set.class); Assertions.assertNotNull(labels); Assertions.assertEquals(size, labels.size(), stringify(labels)); @@ -273,17 +319,6 @@ public String stringify(Collection labels) { return labels.stream().map(label -> label.name).collect(Collectors.joining(", ")); } - // public void repositoryLabelMocks(GitHubMockSetupContext mocks) throws IOException { - // GHLabel mockLabel = mock(GHLabel.class); - // when(mockLabel.getName()).thenReturn("notice"); - // when(mockLabel.getId()).thenReturn(1L); - // when(mockLabel.getNodeId()).thenReturn("MDU6TGFiZWwx"); - - // PagedIterable iterableMock = mockPagedIterable(mockLabel); - // when(mocks.repository(repoFullName).listLabels()) - // .thenReturn(iterableMock); - // } - public static GHPullRequestFileDetail mockGHPullRequestFileDetail(String filename) { GHPullRequestFileDetail mock = mock(GHPullRequestFileDetail.class); lenient().when(mock.getFilename()).thenReturn(filename);