fix(credentials): ensure deletion occurs within transaction context (#…

…755) * fix(credentials): ensure deletion occurs within transaction context * avoid concurrent update exception * log everything in dev mode
cryostatio · Jan 22, 2025 · fceff9d · fceff9d
1 parent 0f44d53
commit fceff9d
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 31 deletions.
diff --git a/src/main/java/io/cryostat/JsonRequestFilter.java b/src/main/java/io/cryostat/JsonRequestFilter.java
@@ -19,9 +19,9 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Pattern;
 
 import com.fasterxml.jackson.databind.JsonNode;
@@ -44,7 +44,7 @@ public class JsonRequestFilter implements ContainerRequestFilter {
                     "/api/v4/matchExpressions",
                     "/api/v4/graphql");
 
-    private final Map<String, Pattern> compiledPatterns = new HashMap<>();
+    private final Map<String, Pattern> compiledPatterns = new ConcurrentHashMap<>();
     @Inject ObjectMapper objectMapper;
 
     @Override

diff --git a/src/main/java/io/cryostat/expressions/MatchExpressionEvaluator.java b/src/main/java/io/cryostat/expressions/MatchExpressionEvaluator.java
@@ -35,6 +35,7 @@
 import io.quarkus.cache.CacheResult;
 import io.quarkus.cache.CompositeCacheKey;
 import io.quarkus.logging.Log;
+import io.quarkus.narayana.jta.QuarkusTransaction;
 import io.quarkus.vertx.ConsumeEvent;
 import jakarta.annotation.Nullable;
 import jakarta.enterprise.context.ApplicationScoped;
@@ -174,34 +175,39 @@ public boolean applies(MatchExpression matchExpression, Target target) throws Sc
     }
 
     public List<Target> getMatchedTargets(MatchExpression matchExpression) {
-        var targets =
-                Target.<Target>listAll().stream()
-                        .filter(
-                                target -> {
-                                    try {
-                                        return applies(matchExpression, target);
-                                    } catch (ScriptException e) {
-                                        logger.error(
-                                                "Error while processing expression: "
-                                                        + matchExpression,
-                                                e);
-                                        return false;
-                                    }
-                                })
-                        .collect(Collectors.toList());
+        return QuarkusTransaction.joiningExisting()
+                .call(
+                        () -> {
+                            var targets =
+                                    Target.<Target>listAll().stream()
+                                            .filter(
+                                                    target -> {
+                                                        try {
+                                                            return applies(matchExpression, target);
+                                                        } catch (ScriptException e) {
+                                                            logger.error(
+                                                                    "Error while processing"
+                                                                            + " expression: "
+                                                                            + matchExpression,
+                                                                    e);
+                                                            return false;
+                                                        }
+                                                    })
+                                            .collect(Collectors.toList());
 
-        var ids = new HashSet<>();
-        var it = targets.iterator();
-        while (it.hasNext()) {
-            var t = it.next();
-            if (ids.contains(t.jvmId)) {
-                it.remove();
-                continue;
-            }
-            ids.add(t.jvmId);
-        }
+                            var ids = new HashSet<>();
+                            var it = targets.iterator();
+                            while (it.hasNext()) {
+                                var t = it.next();
+                                if (ids.contains(t.jvmId)) {
+                                    it.remove();
+                                    continue;
+                                }
+                                ids.add(t.jvmId);
+                            }
 
-        return targets;
+                            return targets;
+                        });
     }
 
     @Name("io.cryostat.rules.MatchExpressionEvaluator.MatchExpressionApplies")

diff --git a/src/main/java/io/cryostat/targets/TargetUpdateService.java b/src/main/java/io/cryostat/targets/TargetUpdateService.java
@@ -31,6 +31,7 @@
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.event.Observes;
 import jakarta.inject.Inject;
+import jakarta.transaction.Transactional;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
 import org.jboss.logging.Logger;
 import org.quartz.JobBuilder;
@@ -77,17 +78,20 @@ void onStop(@Observes ShutdownEvent evt) throws SchedulerException {
         scheduler.shutdown();
     }
 
-    @ConsumeEvent(Credential.CREDENTIALS_STORED)
+    @ConsumeEvent(value = Credential.CREDENTIALS_STORED, blocking = true)
+    @Transactional
     void onCredentialsStored(Credential credential) {
         updateTargetsForExpression(credential);
     }
 
-    @ConsumeEvent(Credential.CREDENTIALS_UPDATED)
+    @ConsumeEvent(value = Credential.CREDENTIALS_UPDATED, blocking = true)
+    @Transactional
     void onCredentialsUpdated(Credential credential) {
         updateTargetsForExpression(credential);
     }
 
-    @ConsumeEvent(Credential.CREDENTIALS_DELETED)
+    @ConsumeEvent(value = Credential.CREDENTIALS_DELETED, blocking = true)
+    @Transactional
     void onCredentialsDeleted(Credential credential) {
         updateTargetsForExpression(credential);
     }

diff --git a/src/main/resources/application-dev.properties b/src/main/resources/application-dev.properties
@@ -12,6 +12,7 @@ quarkus.http.cors.access-control-allow-credentials=true
 # quarkus.http.cors.methods=GET,PUT,POST,PATCH,OPTIONS
 # quarkus.http.cors.access-control-max-age=1s
 
+quarkus.log.level=ALL
 quarkus.hibernate-orm.log.sql=true
 
 quarkus.log.category."org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext".level=DEBUG