Re: [PR] fix(cache): add configurable refresh buffer to StorageCredentialCache [polaris]

["visit2rahul (via GitHub)" <gi...@apache.org>]

visit2rahul commented on code in PR #5129:
URL: https://github.com/apache/polaris/pull/5129#discussion_r3696659132


##########
polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java:
##########
@@ -442,6 +443,74 @@ private static List<StorageAccessConfig> getFakeAccessConfigs(int number, boolea
     return res;
   }
 
+  @Test
+  public void testRefreshBufferEvictsNearExpiryCredentials() {
+    // Credentials expire in 120 seconds from now.
+    // With a 300-second refresh buffer, effective TTL = 120s - 300s = -180s → clamped to 0.
+    // The entry must be absent immediately after load.
+    long expiresAt = System.currentTimeMillis() + 120_000;
+    StorageAccessConfig nearExpiryConfig =
+        StorageAccessConfig.builder()
+            .put(StorageAccessProperty.AWS_KEY_ID, "key")
+            .put(StorageAccessProperty.AWS_SECRET_KEY, "secret")
+            .put(
+                StorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS,
+                String.valueOf(expiresAt))
+            .put(StorageAccessProperty.EXPIRATION_TIME, String.valueOf(expiresAt))
+            .build();
+
+    // RealmConfig with STORAGE_CREDENTIAL_REFRESH_BUFFER_SECONDS = 300
+    RealmConfig realmConfigWithBuffer =
+        new RealmConfigImpl(
+            (rc, name) ->
+                FeatureConfiguration.STORAGE_CREDENTIAL_REFRESH_BUFFER_SECONDS.key().equals(name)
+                    ? 300
+                    : null,
+            realmContext);
+
+    StorageCredentialCacheKey key =
+        new TestKey(
+            realmContext.getRealmIdentifier(),
+            "bufferConfig",
+            true,
+            Set.of("s3://bucket/path"),
+            Set.of(),
+            Optional.empty(),
+            realmConfigWithBuffer,
+            () -> nearExpiryConfig);
+
+    storageCredentialCache.getOrLoad(key);
+    Assertions.assertThat(storageCredentialCache.getIfPresent(key)).isNull();
+  }
+
+  @Test
+  public void testRefreshBufferZeroPreservesHalfLifetimeBehavior() {
+    // Credentials expire in 120 seconds from now.
+    // With buffer = 0 (default), effective TTL = 120s / 2 = 60s > 0 → entry is kept in cache.
+    long expiresAt = System.currentTimeMillis() + 120_000;
+    StorageAccessConfig config =
+        StorageAccessConfig.builder()
+            .put(StorageAccessProperty.AWS_KEY_ID, "key")
+            .put(StorageAccessProperty.AWS_SECRET_KEY, "secret")
+            .put(
+                StorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS,
+                String.valueOf(expiresAt))
+            .put(StorageAccessProperty.EXPIRATION_TIME, String.valueOf(expiresAt))
+            .build();
+
+    StorageCredentialCacheKey key =
+        buildKey(
+            "bufferZeroConfig",
+            true,
+            Set.of("s3://bucket/path"),
+            Set.of(),
+            Optional.empty(),
+            () -> config);
+
+    storageCredentialCache.getOrLoad(key);
+    Assertions.assertThat(storageCredentialCache.getIfPresent(key)).isNotNull();

Review Comment:
   Good point -- renamed to what the assert actually proves.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org