spotbugs

cryostatio · Dec 5, 2024 · fa533f4 · fa533f4
1 parent 05fdf61
commit fa533f4
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 40 deletions.
diff --git a/src/main/java/io/cryostat/rules/RuleExecutor.java b/src/main/java/io/cryostat/rules/RuleExecutor.java
@@ -77,12 +77,11 @@ void onStop(@Observes ShutdownEvent evt) throws SchedulerException {
     @ConsumeEvent(blocking = true)
     @Transactional
     void onMessage(ActivationAttempt attempt) throws QuantityConversionException {
-        Rule rule = attempt.rule();
-        Target target = attempt.target();
-        Target attachedTarget = Target.<Target>find("id", target.id).singleResult();
+        Target attachedTarget = Target.<Target>find("id", attempt.target().id).singleResult();
         recordingHelper
                 .getActiveRecording(
-                        attachedTarget, r -> Objects.equals(r.name, rule.getRecordingName()))
+                        attachedTarget,
+                        r -> Objects.equals(r.name, attempt.rule().getRecordingName()))
                 .ifPresent(
                         rec -> {
                             try {
@@ -95,23 +94,25 @@ void onMessage(ActivationAttempt attempt) throws QuantityConversionException {
                             }
                         });
 
-        Pair<String, TemplateType> pair = recordingHelper.parseEventSpecifier(rule.eventSpecifier);
+        Pair<String, TemplateType> pair =
+                recordingHelper.parseEventSpecifier(attempt.rule().eventSpecifier);
         Template template =
-                recordingHelper.getPreferredTemplate(target, pair.getKey(), pair.getValue());
+                recordingHelper.getPreferredTemplate(
+                        attempt.target(), pair.getKey(), pair.getValue());
 
         ActiveRecording recording =
                 recordingHelper
                         .startRecording(
                                 attachedTarget,
                                 RecordingReplace.STOPPED,
                                 template,
-                                createRecordingOptions(rule),
-                                Map.of("rule", rule.name))
+                                createRecordingOptions(attempt.rule()),
+                                Map.of("rule", attempt.rule().name))
                         .await()
                         .atMost(Duration.ofSeconds(10));
 
-        if (rule.isArchiver()) {
-            scheduleArchival(rule, attachedTarget, recording);
+        if (attempt.rule().isArchiver()) {
+            scheduleArchival(attempt.rule(), attachedTarget, recording);
         }
     }
 

diff --git a/src/main/java/io/cryostat/rules/RuleService.java b/src/main/java/io/cryostat/rules/RuleService.java
@@ -190,52 +190,44 @@ public void handleRuleRecordingCleanup(Rule rule) {
     }
 
     private void resetActivations(Rule rule) {
-        synchronized (activations) {
-            Iterator<ActivationAttempt> it = activations.iterator();
-            while (it.hasNext()) {
-                ActivationAttempt attempt = it.next();
-                if (attempt.rule.equals(rule)) {
-                    it.remove();
-                }
+        Iterator<ActivationAttempt> it = activations.iterator();
+        while (it.hasNext()) {
+            ActivationAttempt attempt = it.next();
+            if (attempt.rule.equals(rule)) {
+                it.remove();
             }
         }
     }
 
     private void resetActivations(Target target) {
-        synchronized (activations) {
-            Iterator<ActivationAttempt> it = activations.iterator();
-            while (it.hasNext()) {
-                ActivationAttempt attempt = it.next();
-                if (attempt.target.equals(target)) {
-                    it.remove();
-                }
+        Iterator<ActivationAttempt> it = activations.iterator();
+        while (it.hasNext()) {
+            ActivationAttempt attempt = it.next();
+            if (attempt.target.equals(target)) {
+                it.remove();
             }
         }
     }
 
     void applyRulesToTarget(Target target) {
-        synchronized (activations) {
-            resetActivations(target);
-            for (var rule : Rule.<Rule>find("enabled", true).list()) {
-                try {
-                    if (!evaluator.applies(rule.matchExpression, target)) {
-                        continue;
-                    }
-                    activations.add(new ActivationAttempt(rule, target));
-                } catch (ScriptException se) {
-                    logger.error(se);
+        resetActivations(target);
+        for (var rule : Rule.<Rule>find("enabled", true).list()) {
+            try {
+                if (!evaluator.applies(rule.matchExpression, target)) {
+                    continue;
                 }
+                activations.add(new ActivationAttempt(rule, target));
+            } catch (ScriptException se) {
+                logger.error(se);
             }
         }
     }
 
     void applyRuleToMatchingTargets(Rule rule) {
-        synchronized (activations) {
-            resetActivations(rule);
-            var targets = evaluator.getMatchedTargets(rule.matchExpression);
-            for (var target : targets) {
-                activations.add(new ActivationAttempt(rule, target));
-            }
+        resetActivations(rule);
+        var targets = evaluator.getMatchedTargets(rule.matchExpression);
+        for (var target : targets) {
+            activations.add(new ActivationAttempt(rule, target));
         }
     }
 
@@ -247,6 +239,7 @@ public record RuleRecording(Rule rule, ActiveRecording recording) {
         }
     }
 
+    @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "EI_EXPOSE_REP2"})
     public record ActivationAttempt(Rule rule, Target target, AtomicInteger attempts) {
         public ActivationAttempt(Rule rule, Target target) {
             this(rule, target, new AtomicInteger(0));