diff --git a/calm-hub/src/main/java/org/finos/calm/store/github/util/GitHubApiResponseCache.java b/calm-hub/src/main/java/org/finos/calm/store/github/util/GitHubApiResponseCache.java new file mode 100644 index 000000000..89484929e --- /dev/null +++ b/calm-hub/src/main/java/org/finos/calm/store/github/util/GitHubApiResponseCache.java @@ -0,0 +1,129 @@ +package org.finos.calm.store.github.util; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.Ticker; +import io.quarkus.arc.lookup.LookupIfProperty; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import org.eclipse.microprofile.config.inject.ConfigProperty; + +import java.time.Duration; +import java.util.List; +import java.util.Optional; + +/** + * Caches responses from the GitHub REST API on behalf of {@code GitHubVersionService}: + * version lists for a file (5 minutes) and file content at an immutable commit SHA + * (365 days). + * + *

This is a per-JVM, in-memory Caffeine cache with no cross-instance coordination + * and no invalidation broadcast — in a multi-instance deployment, each calm-hub + * instance holds its own independent copy and instances can disagree with each other + * for up to a TTL window. That is safe here, structurally, for two reasons: + *

+ * + *

Do not reuse this pattern for Mongo/Nitrite-backed data. That + * backend is the primary one and supports full CRUD through the REST API — a Mongo + * write on one instance would never invalidate another instance's cached read there, + * which is a real correctness problem, not a benign staleness window. This class's API + * is deliberately GitHub-shaped (no generic key/type/TTL parameters) specifically so it + * can't be reached for as a general-purpose cache; see + * #3073 for the + * fuller discussion. + */ +@LookupIfProperty(name = "calm.database.mode", stringValue = "github") +@ApplicationScoped +public class GitHubApiResponseCache { + + private static final Duration VERSIONS_TTL = Duration.ofMinutes(5); + private static final Duration CONTENT_TTL = Duration.ofDays(365); + + private final Cache> versionsCache; + private final Cache contentCache; + + @Inject + public GitHubApiResponseCache(@ConfigProperty(name = "calm.github.cache.max-size", defaultValue = "10000") long maxSize) { + this(maxSize, Ticker.systemTicker()); + } + + // Package-private: lets tests drive expiry deterministically with a fake Ticker + // instead of Thread.sleep, the same pattern used by SchemaMigrationInProgressFilter's + // injectable LongSupplier. + GitHubApiResponseCache(long maxSize, Ticker ticker) { + this.versionsCache = buildCache(maxSize, ticker, VERSIONS_TTL); + this.contentCache = buildCache(maxSize, ticker, CONTENT_TTL); + } + + private static Cache buildCache(long maxSize, Ticker ticker, Duration ttl) { + return Caffeine.newBuilder() + .maximumSize(maxSize) + .ticker(ticker) + .expireAfterWrite(ttl) + .build(); + } + + /** + * Reads the cached commit-SHA version list for a file, if present and not + * expired. + */ + public Optional> getVersions(String repoFullName, String filePath) { + return read(versionsCache, versionsKey(repoFullName, filePath)); + } + + /** + * Caches the commit-SHA version list for a file for {@link #VERSIONS_TTL}. A + * {@code null} list is silently ignored. Stores an immutable copy, so a caller + * mutating the list it passed in — or held onto after a {@link #getVersions} + * call — can never corrupt the cached entry. + */ + public void putVersions(String repoFullName, String filePath, List versions) { + write(versionsCache, versionsKey(repoFullName, filePath), versions == null ? null : List.copyOf(versions)); + } + + /** + * Reads the cached file content at a commit SHA, if present and not expired. + */ + public Optional getContentAtSha(String repoFullName, String filePath, String sha) { + return read(contentCache, contentKey(repoFullName, filePath, sha)); + } + + /** + * Caches file content at a commit SHA for {@link #CONTENT_TTL}. A {@code null} + * content value is silently ignored. Safe to cache for a long TTL because a + * commit SHA is immutable — the same SHA always resolves to the same content. + */ + public void putContentAtSha(String repoFullName, String filePath, String sha, String content) { + write(contentCache, contentKey(repoFullName, filePath, sha), content); + } + + private static Optional read(Cache cache, String key) { + return Optional.ofNullable(cache.getIfPresent(key)); + } + + private static void write(Cache cache, String key, V value) { + if (value == null) { + return; + } + cache.put(key, value); + } + + private static String versionsKey(String repoFullName, String filePath) { + return "versions:" + repoFullName + ":" + filePath; + } + + private static String contentKey(String repoFullName, String filePath, String sha) { + return "content:" + repoFullName + ":" + filePath + ":" + sha; + } +} diff --git a/calm-hub/src/main/resources/application.properties b/calm-hub/src/main/resources/application.properties index 127bd6f21..7152fd985 100644 --- a/calm-hub/src/main/resources/application.properties +++ b/calm-hub/src/main/resources/application.properties @@ -17,6 +17,10 @@ calm.standalone.seed-demo-data=false # Read-only mode: when true, opens Nitrite with readOnly(true) and rejects mutating HTTP verbs calm.readonly=false +# Maximum number of entries GitHubApiResponseCache holds per cache (versions and +# content are sized independently) — applies when calm.database.mode=github. +calm.github.cache.max-size=10000 + # Audit logging: independently toggle persistence to the auditLogs store and # emission of a structured log line under the org.finos.calm.audit category. # Both default to true. sourceIp capture defaults to false and has its own diff --git a/calm-hub/src/test/java/org/finos/calm/store/github/util/TestGitHubApiResponseCacheShould.java b/calm-hub/src/test/java/org/finos/calm/store/github/util/TestGitHubApiResponseCacheShould.java new file mode 100644 index 000000000..f9ed4c505 --- /dev/null +++ b/calm-hub/src/test/java/org/finos/calm/store/github/util/TestGitHubApiResponseCacheShould.java @@ -0,0 +1,216 @@ +package org.finos.calm.store.github.util; + +import com.github.benmanes.caffeine.cache.Ticker; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.stream.IntStream; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class TestGitHubApiResponseCacheShould { + + private static final long MAX_SIZE = 10_000; + + private final FakeTicker ticker = new FakeTicker(); + private GitHubApiResponseCache cache; + + @BeforeEach + void setup() { + cache = new GitHubApiResponseCache(MAX_SIZE, ticker); + } + + @Test + void construct_via_the_config_property_constructor() { + GitHubApiResponseCache service = new GitHubApiResponseCache(MAX_SIZE); + service.putVersions("org/repo", "path/file.json", List.of("abc1234")); + assertThat(service.getVersions("org/repo", "path/file.json").orElse(null), contains("abc1234")); + } + + @Test + void return_empty_versions_for_a_missing_key() { + assertThat(cache.getVersions("org/repo", "path/file.json").isEmpty(), is(true)); + } + + @Test + void store_and_retrieve_versions() { + cache.putVersions("org/repo", "path/file.json", List.of("abc1234", "def5678")); + + Optional> result = cache.getVersions("org/repo", "path/file.json"); + + assertThat(result.isPresent(), is(true)); + assertThat(result.get(), contains("abc1234", "def5678")); + } + + @Test + void keep_versions_for_different_files_independent() { + cache.putVersions("org/repo", "path/a.json", List.of("aaa")); + cache.putVersions("org/repo", "path/b.json", List.of("bbb")); + + assertThat(cache.getVersions("org/repo", "path/a.json").orElse(null), contains("aaa")); + assertThat(cache.getVersions("org/repo", "path/b.json").orElse(null), contains("bbb")); + } + + @Test + void overwrite_existing_versions_entry() { + cache.putVersions("org/repo", "path/file.json", List.of("old")); + cache.putVersions("org/repo", "path/file.json", List.of("new")); + + assertThat(cache.getVersions("org/repo", "path/file.json").orElse(null), contains("new")); + } + + @Test + void ignore_a_null_versions_value() { + cache.putVersions("org/repo", "path/file.json", null); + assertThat(cache.getVersions("org/repo", "path/file.json").isEmpty(), is(true)); + } + + @Test + void store_a_defensive_copy_so_a_caller_cannot_mutate_the_cached_entry() { + List mutable = new ArrayList<>(List.of("original")); + cache.putVersions("org/repo", "path/file.json", mutable); + + mutable.add("mutated-after-put"); + + assertThat(cache.getVersions("org/repo", "path/file.json").orElse(null), contains("original")); + } + + @Test + void return_an_immutable_versions_list_so_a_caller_cannot_corrupt_the_cache() { + cache.putVersions("org/repo", "path/file.json", List.of("abc1234")); + + List result = cache.getVersions("org/repo", "path/file.json").orElseThrow(); + + assertThrows(UnsupportedOperationException.class, () -> result.add("should-fail")); + } + + @Test + void expire_versions_after_five_minutes() { + cache.putVersions("org/repo", "path/file.json", List.of("abc1234")); + assertThat(cache.getVersions("org/repo", "path/file.json").isPresent(), is(true)); + + ticker.advance(Duration.ofMinutes(5).plusSeconds(1)); + + assertThat(cache.getVersions("org/repo", "path/file.json").isEmpty(), is(true)); + } + + @Test + void not_refresh_versions_ttl_on_read() { + cache.putVersions("org/repo", "path/file.json", List.of("abc1234")); + ticker.advance(Duration.ofMinutes(3)); + assertThat(cache.getVersions("org/repo", "path/file.json").isPresent(), is(true)); + + ticker.advance(Duration.ofMinutes(3)); + + assertThat(cache.getVersions("org/repo", "path/file.json").isEmpty(), is(true)); + } + + @Test + void return_empty_content_for_a_missing_key() { + assertThat(cache.getContentAtSha("org/repo", "path/file.json", "abc1234").isEmpty(), is(true)); + } + + @Test + void store_and_retrieve_content_at_sha() { + cache.putContentAtSha("org/repo", "path/file.json", "abc1234", "file contents"); + + Optional result = cache.getContentAtSha("org/repo", "path/file.json", "abc1234"); + + assertThat(result.isPresent(), is(true)); + assertThat(result.get(), equalTo("file contents")); + } + + @Test + void keep_content_for_different_shas_independent() { + cache.putContentAtSha("org/repo", "path/file.json", "sha1", "content-at-sha1"); + cache.putContentAtSha("org/repo", "path/file.json", "sha2", "content-at-sha2"); + + assertThat(cache.getContentAtSha("org/repo", "path/file.json", "sha1").orElse(null), equalTo("content-at-sha1")); + assertThat(cache.getContentAtSha("org/repo", "path/file.json", "sha2").orElse(null), equalTo("content-at-sha2")); + } + + @Test + void ignore_a_null_content_value() { + cache.putContentAtSha("org/repo", "path/file.json", "abc1234", null); + assertThat(cache.getContentAtSha("org/repo", "path/file.json", "abc1234").isEmpty(), is(true)); + } + + @Test + void expire_content_after_365_days() { + cache.putContentAtSha("org/repo", "path/file.json", "abc1234", "file contents"); + assertThat(cache.getContentAtSha("org/repo", "path/file.json", "abc1234").isPresent(), is(true)); + + ticker.advance(Duration.ofDays(365).plusSeconds(1)); + + assertThat(cache.getContentAtSha("org/repo", "path/file.json", "abc1234").isEmpty(), is(true)); + } + + @Test + void expire_versions_and_content_independently_of_each_other() { + cache.putVersions("org/repo", "path/file.json", List.of("abc1234")); + cache.putContentAtSha("org/repo", "path/file.json", "abc1234", "file contents"); + + ticker.advance(Duration.ofMinutes(5).plusSeconds(1)); + + assertThat(cache.getVersions("org/repo", "path/file.json").isEmpty(), is(true)); + assertThat(cache.getContentAtSha("org/repo", "path/file.json", "abc1234").isPresent(), is(true)); + } + + @Test + void handle_concurrent_access_safely() throws Exception { + int threadCount = 10; + int iterationsPerThread = 100; + ExecutorService executor = Executors.newFixedThreadPool(threadCount); + + List> futures = IntStream.range(0, threadCount) + .>mapToObj(threadId -> executor.submit(() -> { + for (int i = 0; i < iterationsPerThread; i++) { + String filePath = "path/" + threadId + "/" + i + ".json"; + cache.putVersions("org/repo", filePath, List.of("sha-" + i)); + cache.getVersions("org/repo", filePath); + } + })) + .toList(); + + for (Future future : futures) { + future.get(10, TimeUnit.SECONDS); + } + executor.shutdown(); + + for (int threadId = 0; threadId < threadCount; threadId++) { + for (int i = 0; i < iterationsPerThread; i++) { + String filePath = "path/" + threadId + "/" + i + ".json"; + assertThat(cache.getVersions("org/repo", filePath).orElse(null), contains("sha-" + i)); + } + } + } + + /** + * A manually-advanced {@link Ticker} so expiry tests are deterministic + * instead of relying on {@code Thread.sleep}. + */ + private static final class FakeTicker implements Ticker { + private long nanos = 0; + + @Override + public long read() { + return nanos; + } + + void advance(Duration duration) { + nanos += duration.toNanos(); + } + } +}