diff --git a/CHANGELOG.md b/CHANGELOG.md index e22aac59cdb1..740064bc7e4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -292,6 +292,9 @@ - [Added `Table.geo_distance` to calculate the distance between two points.][12393] - [The reload button clears the Enso Cloud request cache.][12526] +- [The reload button clears the AuthenticationProvider, EnsoSecretReader and + AuditLog caches.][12541] + [11235]: https://github.com/enso-org/enso/pull/11235 [11255]: https://github.com/enso-org/enso/pull/11255 @@ -306,6 +309,7 @@ [12017]: https://github.com/enso-org/enso/pull/12017 [12393]: https://github.com/enso-org/enso/pull/12393 [12526]: https://github.com/enso-org/enso/pull/12526 +[12541]: https://github.com/enso-org/enso/pull/12526 #### Enso Language & Runtime diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Internal/Authentication.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Internal/Authentication.enso index cce928233f9f..96cd96d91332 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Internal/Authentication.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Internal/Authentication.enso @@ -46,13 +46,13 @@ polyglot java import org.enso.base.enso_cloud.AuthenticationProvider and a new one will be returned. Because of that, this method may make network requests. get_access_token : Text -get_access_token = AuthenticationProvider.getAccessToken +get_access_token = AuthenticationProvider.INSTANCE.getAccessToken ## PRIVATE Forcibly refreshes the access token. refresh_access_token : Nothing refresh_access_token = - AuthenticationProvider.getAuthenticationServiceEnsoInstance.force_refresh + AuthenticationProvider.INSTANCE.getAuthenticationServiceEnsoInstance.force_refresh ## PRIVATE credentials_file : File diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/AuthenticationProvider.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/AuthenticationProvider.java index 01f2e64ad2ab..3e9793479a51 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/AuthenticationProvider.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/AuthenticationProvider.java @@ -1,9 +1,15 @@ package org.enso.base.enso_cloud; +import org.enso.base.cache.ReloadDetector; import org.enso.base.polyglot.EnsoMeta; import org.graalvm.polyglot.Value; -public class AuthenticationProvider { +public class AuthenticationProvider implements ReloadDetector.HasClearableCache { + public static AuthenticationProvider INSTANCE = new AuthenticationProvider(); + + private AuthenticationProvider() { + ReloadDetector.register(this); + } public interface AuthenticationService { String get_access_token(); @@ -11,27 +17,27 @@ public interface AuthenticationService { void force_refresh(); } - private static Value authenticationServiceAsEnso = null; - private static AuthenticationService authenticationServiceAsJava = null; + private Value authenticationServiceAsEnso = null; + private AuthenticationService authenticationServiceAsJava = null; - public static void reset() { + public void reset() { authenticationServiceAsEnso = null; authenticationServiceAsJava = null; } - private static Value createAuthenticationService() { + private Value createAuthenticationService() { return EnsoMeta.callStaticModuleMethod( "Standard.Base.Enso_Cloud.Internal.Authentication", "instantiate_authentication_service"); } - private static void ensureServicesSetup() { + private void ensureServicesSetup() { var ensoInstance = createAuthenticationService(); var javaInstance = ensoInstance.as(AuthenticationService.class); authenticationServiceAsEnso = ensoInstance; authenticationServiceAsJava = javaInstance; } - static AuthenticationService getAuthenticationService() { + AuthenticationService getAuthenticationService() { if (authenticationServiceAsJava == null) { ensureServicesSetup(); } @@ -39,7 +45,9 @@ static AuthenticationService getAuthenticationService() { return authenticationServiceAsJava; } - public static Value getAuthenticationServiceEnsoInstance() { + public Value getAuthenticationServiceEnsoInstance() { + ReloadDetector.clearOnReload(this); + if (authenticationServiceAsEnso == null) { ensureServicesSetup(); } @@ -47,7 +55,26 @@ public static Value getAuthenticationServiceEnsoInstance() { return authenticationServiceAsEnso; } - public static String getAccessToken() { + public String getAccessToken() { + ReloadDetector.clearOnReload(this); + return getAuthenticationService().get_access_token(); } + + @Override /* HasClearableCache */ + public void clearCache() { + reset(); + } + + /** Public for testing. */ + public boolean isCachedTestOnly() { + return authenticationServiceAsEnso != null; + } + + /** Public for testing. */ + // This is necessary because there is no other way to trigger a reload cache + // clear without re-filling the cache. + public void clearOnReloadTestOnly() { + ReloadDetector.clearOnReload(this); + } } diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/CloudAPI.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/CloudAPI.java index 71ed6b8f1bea..bea8339e9d43 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/CloudAPI.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/CloudAPI.java @@ -37,8 +37,8 @@ public static String getCloudSessionId() { public static void flushCloudCaches() { CloudRequestCache.INSTANCE.clear(); - AuthenticationProvider.reset(); - EnsoSecretReader.flushCache(); + AuthenticationProvider.INSTANCE.reset(); + EnsoSecretReader.INSTANCE.flushCache(); AuditLog.resetCache(); } } diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java index 0f3fdad129b2..bb47c47fb253 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java @@ -16,6 +16,7 @@ import java.util.Comparator; import java.util.List; import java.util.Properties; +import org.enso.base.cache.ReloadDetector; import org.enso.base.cache.ResponseTooLargeException; import org.enso.base.net.URISchematic; import org.enso.base.net.URIWithSecrets; @@ -102,7 +103,7 @@ public static EnsoHttpResponse makeRequest( } public static void deleteSecretFromCache(String secretId) { - EnsoSecretReader.removeFromCache(secretId); + EnsoSecretReader.INSTANCE.removeFromCache(secretId); } private static class RequestMaker implements EnsoHTTPResponseCache.RequestMaker { @@ -192,6 +193,16 @@ public static EnsoHTTPResponseCache getOrCreateCache() { return cache; } + /** Visible for testing */ + public static int getEnsoSecretReaderCacheSize() { + return EnsoSecretReader.INSTANCE.getCacheSize(); + } + + /** Visible for testing */ + public static void simulateEnsoSecretReaderReload() { + ReloadDetector.simulateReloadTestOnly(EnsoSecretReader.INSTANCE); + } + private static final Comparator> headerNameComparator = Comparator.comparing((Pair pair) -> pair.getLeft()) .thenComparing(Comparator.comparing(pair -> pair.getRight())); diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretReader.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretReader.java index 9cd2f226d1fb..2d19db5b9fd1 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretReader.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretReader.java @@ -7,16 +7,25 @@ import java.net.http.HttpResponse; import java.util.HashMap; import java.util.Map; +import org.enso.base.cache.ReloadDetector; /** * Internal class to read secrets from the Enso Cloud. */ -class EnsoSecretReader { - private static final Map secrets = new HashMap<>(); +class EnsoSecretReader implements ReloadDetector.HasClearableCache { + static final EnsoSecretReader INSTANCE = new EnsoSecretReader(); - static void flushCache() { + private final Map secrets = new HashMap<>(); + + private EnsoSecretReader() { + ReloadDetector.register(this); + } + + void flushCache() { secrets.clear(); } - static void removeFromCache(String secretId) { + void removeFromCache(String secretId) { + ReloadDetector.clearOnReloadIfRegistered(this); + secrets.remove(secretId); } @@ -26,7 +35,9 @@ static void removeFromCache(String secretId) { * @param secretId the ID of the secret to read. * @return the secret value. */ - static String readSecret(String secretId) { + String readSecret(String secretId) { + ReloadDetector.clearOnReloadIfRegistered(this); + if (secrets.containsKey(secretId)) { return secrets.get(secretId); } @@ -34,13 +45,13 @@ static String readSecret(String secretId) { return fetchSecretValue(secretId, 3); } - private static String fetchSecretValue(String secretId, int retryCount) { + private String fetchSecretValue(String secretId, int retryCount) { var apiUri = CloudAPI.getAPIRootURI() + "s3cr3tz/" + secretId; var client = HttpClient.newBuilder().followRedirects(HttpClient.Redirect.ALWAYS).build(); var request = HttpRequest.newBuilder() .uri(URI.create(apiUri)) - .header("Authorization", "Bearer " + AuthenticationProvider.getAccessToken()) + .header("Authorization", "Bearer " + AuthenticationProvider.INSTANCE.getAccessToken()) .GET() .build(); @@ -64,7 +75,7 @@ private static String fetchSecretValue(String secretId, int retryCount) { "Unable to read secret - numerous " + kind + " failures (status code " + status + ")."); } else { // We forcibly refresh the access token and try again. - AuthenticationProvider.getAuthenticationService().force_refresh(); + AuthenticationProvider.INSTANCE.getAuthenticationService().force_refresh(); return fetchSecretValue(secretId, retryCount - 1); } } @@ -80,9 +91,19 @@ private static String fetchSecretValue(String secretId, int retryCount) { return secretValue; } - private static String readValueFromString(String json) { + private String readValueFromString(String json) { var base64 = json.substring(1, json.length() - 1).translateEscapes(); return new String( java.util.Base64.getDecoder().decode(base64), java.nio.charset.StandardCharsets.UTF_8); } + + @Override /* HasClearableCache */ + public void clearCache() { + flushCache(); + } + + /** Visible for testing */ + public int getCacheSize() { + return secrets.size(); + } } diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/SecretValueResolver.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/SecretValueResolver.java index cbdb6897a6f2..fc88491473a3 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/SecretValueResolver.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/SecretValueResolver.java @@ -11,7 +11,7 @@ protected static String resolveValue(HideableValue value) { return switch (value) { case HideableValue.PlainValue plainValue -> plainValue.value(); case HideableValue.SecretValue secretValue -> { - yield EnsoSecretReader.readSecret(secretValue.secretId()); + yield EnsoSecretReader.INSTANCE.readSecret(secretValue.secretId()); } case HideableValue.ConcatValues concatValues -> { String left = resolveValue(concatValues.left()); diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/audit/AuditLogApiAccess.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/audit/AuditLogApiAccess.java index 372ad014ef78..8fba49ee6bf9 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/audit/AuditLogApiAccess.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/audit/AuditLogApiAccess.java @@ -16,6 +16,7 @@ import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; +import org.enso.base.cache.ReloadDetector; import org.enso.base.enso_cloud.AuthenticationProvider; import org.enso.base.enso_cloud.CloudAPI; @@ -23,7 +24,7 @@ * Gives access to the low-level log event API in the Cloud and manages asynchronously submitting * the logs. */ -class AuditLogApiAccess { +public final class AuditLogApiAccess implements ReloadDetector.HasClearableCache { private static final Logger logger = Logger.getLogger(AuditLogApiAccess.class.getName()); /** @@ -45,9 +46,12 @@ private AuditLogApiAccess() { // If the thread is idle for 60 seconds, it will be shut down. backgroundThreadService = new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue<>()); + ReloadDetector.register(this); } public Future logWithConfirmation(LogMessage message) { + ReloadDetector.clearOnReload(this); + var currentRequestConfig = getRequestConfig(); CompletableFuture completionNotification = new CompletableFuture<>(); enqueueJob(new LogJob(message, completionNotification, currentRequestConfig)); @@ -55,6 +59,8 @@ public Future logWithConfirmation(LogMessage message) { } public void logWithoutConfirmation(LogMessage message) { + ReloadDetector.clearOnReload(this); + var currentRequestConfig = getRequestConfig(); enqueueJob(new LogJob(message, null, currentRequestConfig)); } @@ -195,7 +201,7 @@ private RequestConfig getRequestConfig() { } var uri = URI.create(CloudAPI.getAPIRootURI() + "logs"); - var config = new RequestConfig(uri, AuthenticationProvider.getAccessToken()); + var config = new RequestConfig(uri, AuthenticationProvider.INSTANCE.getAccessToken()); cachedRequestConfig = config; return config; } @@ -265,4 +271,21 @@ record LogJob( void resetCache() { cachedRequestConfig = null; } + + @Override /* HasClearableCache */ + public void clearCache() { + resetCache(); + } + + /** Public for testing. */ + public boolean isCachedTestOnly() { + return cachedRequestConfig != null; + } + + /** Public for testing. */ + // This is necessary because there is no other way to trigger a reload cache + // clear without re-filling the cache. + public void clearOnReloadTestOnly() { + ReloadDetector.clearOnReload(this); + } } diff --git a/std-bits/table/src/main/java/org/enso/table/excel/ExcelConnectionPool.java b/std-bits/table/src/main/java/org/enso/table/excel/ExcelConnectionPool.java index 85a111c22c64..de5ddd519d46 100644 --- a/std-bits/table/src/main/java/org/enso/table/excel/ExcelConnectionPool.java +++ b/std-bits/table/src/main/java/org/enso/table/excel/ExcelConnectionPool.java @@ -229,8 +229,8 @@ public void clearCache() { LOGGER.error("Unable to close " + record, e); } } - records.clear(); } + records.clear(); } /** Public for testing. */ diff --git a/test/Base_Tests/src/Network/Enso_Cloud/Audit_Log_Spec.enso b/test/Base_Tests/src/Network/Enso_Cloud/Audit_Log_Spec.enso index a5c6f06e3f33..1fb4979ec7fa 100644 --- a/test/Base_Tests/src/Network/Enso_Cloud/Audit_Log_Spec.enso +++ b/test/Base_Tests/src/Network/Enso_Cloud/Audit_Log_Spec.enso @@ -10,6 +10,8 @@ import Standard.Test.Test_Environment import project.Network.Enso_Cloud.Cloud_Tests_Setup.Cloud_Tests_Setup +polyglot java import org.enso.base.cache.ReloadDetector +polyglot java import org.enso.base.enso_cloud.logging.LogApiAccess add_specs suite_builder = ## By default, these tests are run only on the Cloud mock, not on the real deployment. @@ -65,6 +67,15 @@ add_specs suite_builder = events = get_audit_log_events . filter ev-> (ev.metadata.get "my_field") == random_payload events.length . should_equal 121 + suite_builder.group "Clear cache on reload" group_builder-> + group_builder.specify "LogApiAccess cache should be cleared when a reload is detected" <| setup.with_prepared_environment <| + Audit_Log.report_event "TestEvent" "Message" + LogApiAccess.INSTANCE.isCachedTestOnly . should_be_true + ReloadDetector.simulateReloadTestOnly LogApiAccess.INSTANCE + LogApiAccess.INSTANCE.clearOnReloadTestOnly . should_succeed # Triggers cache clearing + LogApiAccess.INSTANCE.isCachedTestOnly . should_be_false + + main filter=Nothing = suite = Test.build suite_builder-> add_specs suite_builder diff --git a/test/Base_Tests/src/Network/Enso_Cloud/Enso_Cloud_Spec.enso b/test/Base_Tests/src/Network/Enso_Cloud/Enso_Cloud_Spec.enso index b5496e9ea6ba..32f68a845b1f 100644 --- a/test/Base_Tests/src/Network/Enso_Cloud/Enso_Cloud_Spec.enso +++ b/test/Base_Tests/src/Network/Enso_Cloud/Enso_Cloud_Spec.enso @@ -104,7 +104,7 @@ add_specs suite_builder setup:Cloud_Tests_Setup = get_current_token = ## We cannot just use `getAccessToken` here, because it would trigger the refresh too early. Instead, we want to see the token as is currently set. - AuthenticationProvider.getAuthenticationServiceEnsoInstance.auth_data.get.access_token + AuthenticationProvider.INSTANCE.getAuthenticationServiceEnsoInstance.auth_data.get.access_token base_credentials = Lazy_Ref.Value <| Mock_Credentials.default Cloud_Tests_Setup.prepare_mock_setup.httpbin_uri group_builder.specify "refreshes an expired token" <| run_with_and_without_output <| @@ -156,16 +156,22 @@ add_specs suite_builder setup:Cloud_Tests_Setup = err.to_display_text . should_contain "Expected field `TokenType` to be of type Text, but got Integer." suite_builder.group "Clear cache on reload" pending=setup.httpbin_pending group_builder-> - group_builder.specify "CloudRequestCache should be cleared when a reload is detected" <| + group_builder.specify "CloudRequestCache should be cleared when a reload is detected" pending=setup.pending <| setup.with_prepared_environment <| dur = Java_Duration.ofDays 1 cache = CloudRequestCache.INSTANCE cache.getOrCompute "key1" (_-> ["key1"]) dur cache.isCachedTestOnly "key1" . should_be_true ReloadDetector.simulateReloadTestOnly cache - ## Another cache action must be called to trigger cache cleaering - cache.getOrCompute "key2" (_-> ["key2"]) dur + cache.getOrCompute "key2" (_-> ["key2"]) dur # Triggers cache clearing cache.isCachedTestOnly "key1" . should_be_false + group_builder.specify "AuthenticationProvider cache should be cleared when a reload is detected" pending=setup.pending <| setup.with_prepared_environment <| + AuthenticationProvider.INSTANCE.getAccessToken . should_succeed + AuthenticationProvider.INSTANCE.isCachedTestOnly . should_be_true + ReloadDetector.simulateReloadTestOnly AuthenticationProvider.INSTANCE + AuthenticationProvider.INSTANCE.clearOnReloadTestOnly . should_succeed # Triggers cache clearing + AuthenticationProvider.INSTANCE.isCachedTestOnly . should_be_false + type Lazy_Ref Value ~get diff --git a/test/Base_Tests/src/Network/URI_Spec.enso b/test/Base_Tests/src/Network/URI_Spec.enso index 8b0d4959dcdd..f17fd02373ab 100644 --- a/test/Base_Tests/src/Network/URI_Spec.enso +++ b/test/Base_Tests/src/Network/URI_Spec.enso @@ -9,6 +9,8 @@ from Standard.Test import all import project.Network.Enso_Cloud.Cloud_Tests_Setup.Cloud_Tests_Setup +polyglot java import org.enso.base.enso_cloud.EnsoSecretHelper + add_specs suite_builder = ## To run this test locally: $ sbt 'http-test-helper/run localhost 8080' @@ -338,6 +340,23 @@ add_specs suite_builder = r1.should_fail_with Illegal_Argument r1.catch.to_display_text . should_contain "Secrets are not allowed in HTTP connections, use HTTPS instead." + with_secret ~action = + secret = Enso_Secret.create "my_test_secret-clear-cache-1-"+Random.uuid "My Value" + secret.should_succeed + Panic.with_finalizer secret.delete (action secret) + + group_builder.specify "EnsoSecreetReader cache should be cleared when a reload is detected" pending=cloud_setup.pending <| Test.with_retries <| + with_secret secret1-> with_secret secret2-> + base_uri = URI.from 'https://httpbin.org/bytes/50' + uri1 = base_uri . add_query_argument "arg1" secret1 + uri2 = base_uri . add_query_argument "arg1" secret2 + uri1.fetch . should_succeed + uri2.fetch . should_succeed + (EnsoSecretHelper.getEnsoSecretReaderCacheSize >= 2) . should_be_true + EnsoSecretHelper.simulateEnsoSecretReaderReload + uri1.fetch . should_succeed + (EnsoSecretHelper.getEnsoSecretReaderCacheSize == 1) . should_be_true + main filter=Nothing = suite = Test.build suite_builder-> add_specs suite_builder