From f2cc78bc85a1bec000e8e400980fb8b2509a7365 Mon Sep 17 00:00:00 2001 From: fjtirado Date: Wed, 29 Oct 2025 09:55:04 +0100 Subject: [PATCH] [Fix #921] Caching resources Signed-off-by: fjtirado --- .../impl/WorkflowDefinition.java | 1 + .../impl/resources/ClasspathResource.java | 15 ++ .../impl/resources/DefaultResourceLoader.java | 33 ++-- .../resources/ExternalResourceHandler.java | 2 +- .../impl/resources/FileResource.java | 29 ++++ .../impl/resources/HttpResource.java | 32 ++++ .../impl/resources/LRUCache.java | 34 ----- .../impl/resources/ResourceLoader.java | 2 +- .../impl/test/OpenAPITest.java | 141 +++++++++--------- .../openapi/get-user-get-request-vars.yaml | 2 +- .../openapi/get-user-get-request.yaml | 2 +- .../openapi/project-post-positive.yaml | 2 +- 12 files changed, 167 insertions(+), 128 deletions(-) delete mode 100644 impl/core/src/main/java/io/serverlessworkflow/impl/resources/LRUCache.java diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowDefinition.java b/impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowDefinition.java index 0948459f6..41c5c2ee9 100644 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowDefinition.java +++ b/impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowDefinition.java @@ -149,5 +149,6 @@ public void addTaskExecutor(WorkflowMutablePosition position, TaskExecutor ta @Override public void close() { safeClose(scheculedConsumer); + safeClose(resourceLoader); } } diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ClasspathResource.java b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ClasspathResource.java index c3ac293ee..119cefe47 100644 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ClasspathResource.java +++ b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ClasspathResource.java @@ -16,6 +16,7 @@ package io.serverlessworkflow.impl.resources; import java.io.InputStream; +import java.util.Objects; public class ClasspathResource implements ExternalResourceHandler { @@ -34,4 +35,18 @@ public InputStream open() { public String name() { return path; } + + @Override + public int hashCode() { + return Objects.hash(path); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + ClasspathResource other = (ClasspathResource) obj; + return Objects.equals(path, other.path); + } } diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/DefaultResourceLoader.java b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/DefaultResourceLoader.java index 4dd975a80..47c6a1c9a 100644 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/DefaultResourceLoader.java +++ b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/DefaultResourceLoader.java @@ -32,9 +32,8 @@ import java.util.Map; import java.util.Optional; import java.util.ServiceLoader; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; public class DefaultResourceLoader implements ResourceLoader { @@ -45,8 +44,7 @@ public class DefaultResourceLoader implements ResourceLoader { private final AtomicReference templateResolver = new AtomicReference(); - private Map resourceCache = new LRUCache<>(100); - private Lock cacheLock = new ReentrantLock(); + private Map resourceCache = new ConcurrentHashMap<>(); protected DefaultResourceLoader(WorkflowApplication application, Path workflowPath) { this.application = application; @@ -108,20 +106,15 @@ public T load( workflowContext, taskContext, model == null ? application.modelFactory().fromNull() : model)); - try { - CachedResource cachedResource; - cacheLock.lock(); - cachedResource = resourceCache.get(resourceHandler); - cacheLock.unlock(); - if (cachedResource == null || resourceHandler.shouldReload(cachedResource.lastReload())) { - cachedResource = new CachedResource(Instant.now(), function.apply(resourceHandler)); - cacheLock.lock(); - resourceCache.put(resourceHandler, cachedResource); - } - return cachedResource.content(); - } finally { - cacheLock.unlock(); - } + return (T) + resourceCache + .compute( + resourceHandler, + (k, v) -> + v == null || k.shouldReload(v.lastReload()) + ? new CachedResource(Instant.now(), function.apply(k)) + : v) + .content(); } @Override @@ -169,4 +162,8 @@ public URI apply(WorkflowContext workflow, TaskContext task, WorkflowModel node) return URI.create(expr.apply(workflow, task, node)); } } + + public void close() { + resourceCache.clear(); + } } diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ExternalResourceHandler.java b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ExternalResourceHandler.java index 9c6c565f2..99a5381ca 100644 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ExternalResourceHandler.java +++ b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ExternalResourceHandler.java @@ -24,7 +24,7 @@ public interface ExternalResourceHandler { InputStream open(); - default boolean shouldReload(Instant lasUpdate) { + default boolean shouldReload(Instant lastUpdate) { return false; } } diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/FileResource.java b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/FileResource.java index 0d3cdb67b..4fb9729f2 100644 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/FileResource.java +++ b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/FileResource.java @@ -20,6 +20,9 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.time.Instant; +import java.util.Objects; class FileResource implements ExternalResourceHandler { @@ -42,4 +45,30 @@ public InputStream open() { public String name() { return path.getFileName().toString(); } + + @Override + public boolean shouldReload(Instant lastUpdate) { + try { + return Files.readAttributes(path, BasicFileAttributes.class) + .lastModifiedTime() + .toInstant() + .isAfter(lastUpdate); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Override + public int hashCode() { + return Objects.hash(path); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + FileResource other = (FileResource) obj; + return Objects.equals(path, other.path); + } } diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/HttpResource.java b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/HttpResource.java index df3db9b2c..7e2cd70d3 100644 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/HttpResource.java +++ b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/HttpResource.java @@ -18,7 +18,10 @@ import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; +import java.net.HttpURLConnection; import java.net.URL; +import java.time.Instant; +import java.util.Objects; public class HttpResource implements ExternalResourceHandler { @@ -40,4 +43,33 @@ public InputStream open() { public String name() { return url.getFile(); } + + @Override + public boolean shouldReload(Instant lastUpdate) { + try { + long millis = lastUpdate.toEpochMilli(); + HttpURLConnection connection = (HttpURLConnection) url.openConnection(); + connection.setUseCaches(true); + connection.setRequestMethod("HEAD"); + connection.setIfModifiedSince(millis); + return connection.getResponseCode() != HttpURLConnection.HTTP_NOT_MODIFIED + && connection.getLastModified() > millis; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Override + public int hashCode() { + return Objects.hash(url); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + HttpResource other = (HttpResource) obj; + return Objects.equals(url, other.url); + } } diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/LRUCache.java b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/LRUCache.java deleted file mode 100644 index 8e1d115fe..000000000 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/LRUCache.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2020-Present The Serverless Workflow Specification Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.serverlessworkflow.impl.resources; - -import java.util.LinkedHashMap; -import java.util.Map; - -public class LRUCache extends LinkedHashMap { - - private final int capacity; - - public LRUCache(int capacity) { - super(capacity, 0.75f, true); - this.capacity = capacity; - } - - @Override - protected boolean removeEldestEntry(Map.Entry eldest) { - return size() > capacity; - } -} diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ResourceLoader.java b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ResourceLoader.java index 8dbb1e0bd..0c17077ee 100644 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ResourceLoader.java +++ b/impl/core/src/main/java/io/serverlessworkflow/impl/resources/ResourceLoader.java @@ -24,7 +24,7 @@ import java.net.URI; import java.util.function.Function; -public interface ResourceLoader { +public interface ResourceLoader extends AutoCloseable { WorkflowValueResolver uriSupplier(Endpoint endpoint); diff --git a/impl/test/src/test/java/io/serverlessworkflow/impl/test/OpenAPITest.java b/impl/test/src/test/java/io/serverlessworkflow/impl/test/OpenAPITest.java index 20a86c9b2..53593851f 100644 --- a/impl/test/src/test/java/io/serverlessworkflow/impl/test/OpenAPITest.java +++ b/impl/test/src/test/java/io/serverlessworkflow/impl/test/OpenAPITest.java @@ -17,36 +17,38 @@ import static io.serverlessworkflow.api.WorkflowReader.readWorkflowFromClasspath; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import com.fasterxml.jackson.databind.ObjectMapper; import io.serverlessworkflow.api.types.Workflow; import io.serverlessworkflow.impl.WorkflowApplication; +import io.serverlessworkflow.impl.WorkflowDefinition; +import io.serverlessworkflow.impl.WorkflowException; import java.io.IOException; -import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; +import java.io.InputStream; +import java.util.Date; import java.util.List; import java.util.Map; -import okhttp3.OkHttpClient; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; +import okio.Buffer; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; public class OpenAPITest { - private static final ObjectMapper MAPPER = new ObjectMapper(); + private static WorkflowApplication app; + private static byte[] yaml; private MockWebServer authServer; private MockWebServer openApiServer; private MockWebServer restServer; - - private OkHttpClient httpClient; + private Buffer yamlBuffer; private static String PROJECT_JSON_SUCCESS = """ @@ -94,6 +96,19 @@ public class OpenAPITest { } """; + @BeforeAll + static void init() throws IOException { + try (InputStream is = OpenAPITest.class.getResourceAsStream("/schema/openapi/openapi.yaml")) { + yaml = is.readAllBytes(); + } + app = WorkflowApplication.builder().build(); + } + + @AfterAll + static void cleanup() { + app.close(); + } + @BeforeEach void setUp() throws IOException { authServer = new MockWebServer(); @@ -105,7 +120,8 @@ void setUp() throws IOException { restServer = new MockWebServer(); restServer.start(8886); - httpClient = new OkHttpClient(); + yamlBuffer = new Buffer(); + yamlBuffer.write(yaml); } @AfterEach @@ -120,14 +136,9 @@ public void testOpenAPIBearerQueryInlinedBodyWithPositiveResponse() throws Excep Workflow workflow = readWorkflowFromClasspath("workflows-samples/openapi/project-post-positive.yaml"); - URL url = this.getClass().getResource("/schema/openapi/openapi.yaml"); - - Path workflowPath = Path.of(url.getPath()); - String yaml = Files.readString(workflowPath, StandardCharsets.UTF_8); - openApiServer.enqueue( new MockResponse() - .setBody(yaml) + .setBody(yamlBuffer) .setHeader("Content-Type", "application/yaml") .setResponseCode(200)); @@ -137,14 +148,8 @@ public void testOpenAPIBearerQueryInlinedBodyWithPositiveResponse() throws Excep .setHeader("Content-Type", "application/json") .setResponseCode(201)); - Map result; - - try (WorkflowApplication app = WorkflowApplication.builder().build()) { - result = - app.workflowDefinition(workflow).instance(Map.of()).start().get().asMap().orElseThrow(); - } catch (Exception e) { - throw new RuntimeException("Workflow execution failed", e); - } + Map result = + app.workflowDefinition(workflow).instance(Map.of()).start().get().asMap().orElseThrow(); RecordedRequest restRequest = restServer.takeRequest(); assertEquals("POST", restRequest.getMethod()); @@ -174,14 +179,9 @@ public void testOpenAPIBearerQueryInlinedBodyWithNegativeResponse() throws Excep Workflow workflow = readWorkflowFromClasspath("workflows-samples/openapi/project-post-positive.yaml"); - URL url = this.getClass().getResource("/schema/openapi/openapi.yaml"); - - Path workflowPath = Path.of(url.getPath()); - String yaml = Files.readString(workflowPath, StandardCharsets.UTF_8); - openApiServer.enqueue( new MockResponse() - .setBody(yaml) + .setBody(yamlBuffer) .setHeader("Content-Type", "application/yaml") .setResponseCode(200)); @@ -191,16 +191,19 @@ public void testOpenAPIBearerQueryInlinedBodyWithNegativeResponse() throws Excep .setHeader("Content-Type", "application/json") .setResponseCode(409)); - Map result; - - Exception exception = null; - - try (WorkflowApplication app = WorkflowApplication.builder().build()) { - result = - app.workflowDefinition(workflow).instance(Map.of()).start().get().asMap().orElseThrow(); - } catch (Exception e) { - exception = e; - } + Exception exception = + assertThrows( + Exception.class, + () -> + app.workflowDefinition(workflow) + .instance(Map.of()) + .start() + .get() + .asMap() + .orElseThrow()); + assertInstanceOf(WorkflowException.class, exception.getCause()); + assertTrue(exception.getMessage().contains("status=409")); + assertTrue(exception.getMessage().contains("title=HTTP 409 Client Error")); RecordedRequest restRequest = restServer.takeRequest(); assertEquals("POST", restRequest.getMethod()); @@ -210,42 +213,48 @@ public void testOpenAPIBearerQueryInlinedBodyWithNegativeResponse() throws Excep assertTrue(restRequest.getPath().contains("lang=en")); assertEquals("application/json", restRequest.getHeader("Content-Type")); assertEquals("Bearer eyJhbnNpc2l0b3IuYm9sdXMubWFnbnVz", restRequest.getHeader("Authorization")); - - assertNotNull(exception); - assertTrue(exception.getMessage().contains("status=409")); - assertTrue(exception.getMessage().contains("title=HTTP 409 Client Error")); } @Test public void testOpenAPIGetWithPositiveResponse() throws Exception { + Workflow workflow = readWorkflowFromClasspath("workflows-samples/openapi/get-user-get-request.yaml"); - URL url = this.getClass().getResource("/schema/openapi/openapi.yaml"); - - Path workflowPath = Path.of(url.getPath()); - String yaml = Files.readString(workflowPath, StandardCharsets.UTF_8); - openApiServer.enqueue( new MockResponse() - .setBody(yaml) + .setBody(yamlBuffer) .setHeader("Content-Type", "application/yaml") .setResponseCode(200)); + openApiServer.enqueue( + new MockResponse() + .setHeader("last-modified", new Date().toGMTString()) + .setResponseCode(304)); + restServer.enqueue( new MockResponse() .setBody(PROJECT_GET_JSON_POSITIVE) .setHeader("Content-Type", "application/json") .setResponseCode(200)); - Map result; - try (WorkflowApplication app = WorkflowApplication.builder().build()) { - result = - app.workflowDefinition(workflow).instance(Map.of()).start().get().asMap().orElseThrow(); - } catch (Exception e) { - throw new RuntimeException("Workflow execution failed", e); - } + restServer.enqueue( + new MockResponse() + .setBody(PROJECT_GET_JSON_POSITIVE) + .setHeader("Content-Type", "application/json") + .setResponseCode(200)); + WorkflowDefinition definition = app.workflowDefinition(workflow); + assertData(definition.instance(Map.of()).start().get().asMap().orElseThrow()); + RecordedRequest openAPIRequest = openApiServer.takeRequest(); + assertEquals("GET", openAPIRequest.getMethod()); + + assertData(definition.instance(Map.of()).start().get().asMap().orElseThrow()); + openAPIRequest = openApiServer.takeRequest(); + assertEquals("HEAD", openAPIRequest.getMethod()); + } + + private void assertData(Map result) throws InterruptedException { RecordedRequest restRequest = restServer.takeRequest(); assertEquals("GET", restRequest.getMethod()); assertTrue(restRequest.getPath().startsWith("/users/40099?")); @@ -262,14 +271,9 @@ public void testOpenAPIGetWithPositiveResponseAndVars() throws Exception { Workflow workflow = readWorkflowFromClasspath("workflows-samples/openapi/get-user-get-request-vars.yaml"); - URL url = this.getClass().getResource("/schema/openapi/openapi.yaml"); - - Path workflowPath = Path.of(url.getPath()); - String yaml = Files.readString(workflowPath, StandardCharsets.UTF_8); - openApiServer.enqueue( new MockResponse() - .setBody(yaml) + .setBody(yamlBuffer) .setHeader("Content-Type", "application/yaml") .setResponseCode(200)); @@ -279,7 +283,6 @@ public void testOpenAPIGetWithPositiveResponseAndVars() throws Exception { .setHeader("Content-Type", "application/json") .setResponseCode(200)); - Map result; Map params = Map.of( "userId", @@ -299,12 +302,8 @@ public void testOpenAPIGetWithPositiveResponseAndVars() throws Exception { "limit", 20); - try (WorkflowApplication app = WorkflowApplication.builder().build()) { - result = - app.workflowDefinition(workflow).instance(params).start().get().asMap().orElseThrow(); - } catch (Exception e) { - throw new RuntimeException("Workflow execution failed", e); - } + Map result = + app.workflowDefinition(workflow).instance(params).start().get().asMap().orElseThrow(); RecordedRequest restRequest = restServer.takeRequest(); assertEquals("GET", restRequest.getMethod()); diff --git a/impl/test/src/test/resources/workflows-samples/openapi/get-user-get-request-vars.yaml b/impl/test/src/test/resources/workflows-samples/openapi/get-user-get-request-vars.yaml index af0325549..71db26b2e 100644 --- a/impl/test/src/test/resources/workflows-samples/openapi/get-user-get-request-vars.yaml +++ b/impl/test/src/test/resources/workflows-samples/openapi/get-user-get-request-vars.yaml @@ -1,7 +1,7 @@ document: dsl: '1.0.1' namespace: test - name: openapi-example + name: openapi-get-user-request-vars version: '0.1.0' do: - getUser: diff --git a/impl/test/src/test/resources/workflows-samples/openapi/get-user-get-request.yaml b/impl/test/src/test/resources/workflows-samples/openapi/get-user-get-request.yaml index d0680b294..3087aa8a3 100644 --- a/impl/test/src/test/resources/workflows-samples/openapi/get-user-get-request.yaml +++ b/impl/test/src/test/resources/workflows-samples/openapi/get-user-get-request.yaml @@ -1,7 +1,7 @@ document: dsl: '1.0.1' namespace: test - name: openapi-example + name: openapi-get-user-request version: '0.1.0' do: - getUser: diff --git a/impl/test/src/test/resources/workflows-samples/openapi/project-post-positive.yaml b/impl/test/src/test/resources/workflows-samples/openapi/project-post-positive.yaml index 8c15dcd50..89f97f1b2 100644 --- a/impl/test/src/test/resources/workflows-samples/openapi/project-post-positive.yaml +++ b/impl/test/src/test/resources/workflows-samples/openapi/project-post-positive.yaml @@ -1,7 +1,7 @@ document: dsl: '1.0.1' namespace: test - name: openapi-example + name: openapi-post-positive version: '0.1.0' do: - addProject: