sap: Merge e3c83182ed1778266ded5bb8e4be01e03a73025c

application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
  * 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -111,7 +111,7 @@ private boolean shouldEvaluate(IRule rule) throws InterruptedException {
 				if (dependencyType != null) {
 					while (true) {
 						if (evaluatedRules.containsKey(dependencyType)) {
-							if (evaluatedRules.get(dependencyType).compareTo(dependency.severity()) < 0) {
+							if (!evaluatedRules.get(dependencyType).isAtLeast(dependency.severity())) {
 								return false;
 							}
 							return true;
@@ -304,8 +304,8 @@ public Collection<IRule> getRules(String ... topics) {
 	}
 
 	public Severity getMaxSeverity(String ... topics) {
-		return getResults(topics).parallelStream().map(IResult::getSeverity).max(Comparator.naturalOrder())
-				.orElse(Severity.NA);
+		return getResults(topics).parallelStream().map(IResult::getSeverity)
+				.max(Comparator.comparingDouble(Severity::getLimit)).orElse(Severity.NA);
 	}
 
 	public Collection<IResult> getUnmappedResults() {

application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/AbstractDataPage.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
  * 
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -144,7 +144,7 @@ public AbstractDataPage(IPageDefinition definition, StreamModel model, IPageCont
 
 	private long getNumberOfInterestingResults() {
 		return editor.getRuleManager().getResults(definition.getTopics()).parallelStream()
-				.filter(r -> r.getSeverity().compareTo(Severity.INFO) >= 0).count();
+				.filter(r -> r.getSeverity().isAtLeast(Severity.INFO)).count();
 	}
 
 	private String getRulesStatistics() {

application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/DataPageToolkit.java

@@ -823,12 +823,11 @@ public static class ShowResultAction extends Action {
 			maxSeverity = pageContainer.getRuleManager().getMaxSeverity(topics);
 			for (String topic : topics) {
 				Consumer<IResult> listener = result -> {
-					Severity severity = result.getSeverity();
-					if (severity.compareTo(maxSeverity) > 0) {
-						maxSeverity = severity;
+					Severity updatedMaxSeverity = Severity.nextMax(maxSeverity, result.getSeverity(),
+							() -> pageContainer.getRuleManager().getMaxSeverity(topics));
+					if (updatedMaxSeverity != maxSeverity) {
+						maxSeverity = updatedMaxSeverity;
 						setImageDescriptor(getResultIcon(maxSeverity));
-					} else if (severity.compareTo(maxSeverity) < 0) { // severity could be less than previous max
-						maxSeverity = pageContainer.getRuleManager().getMaxSeverity(topics);
 					}
 					setToolTipText(tooltip.get());
 				};

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/Severity.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
  *
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -32,10 +32,26 @@
  */
 package org.openjdk.jmc.flightrecorder.rules;
 
+import java.util.function.Supplier;
+
 import org.openjdk.jmc.flightrecorder.rules.messages.internal.Messages;
 
+/**
+ * The severity of a rule result.
+ * <p>
+ * The constants are declared in ascending order of severity, so that the natural ordering of this
+ * enum agrees with the ordering of the severity scores returned by {@link #getLimit()}. Keep it
+ * that way when adding constants: code that compares severities relies on it. Prefer
+ * {@link #isAtLeast(Severity)} over {@link #compareTo(Object)} when filtering, since it states the
+ * intent and compares the scores directly.
+ */
 public enum Severity {
 
+	/**
+	 * Results with this severity score should not be presented at all, because the rule does not
+	 * apply in a way that is worth reporting.
+	 */
+	IGNORE(-3, Messages.getString(Messages.Severity_IGNORE)),
 	/**
 	 * Results with this severity score are not applicable to the recording, but it should be
 	 * possible to view them so the user can see which results have not been possible to evaluate.
@@ -52,9 +68,7 @@ public enum Severity {
 	/**
 	 * Results with this severity score should be presented as warnings.
 	 */
-	WARNING(75, Messages.getString(Messages.Severity_WARNING)),
-
-	IGNORE(-3, Messages.getString(Messages.Severity_IGNORE));
+	WARNING(75, Messages.getString(Messages.Severity_WARNING));
 
 	private final double score;
 	private final String localizedName;
@@ -72,6 +86,47 @@ public double getLimit() {
 		return score;
 	}
 
+	/**
+	 * Checks whether this severity is at least as severe as the given one, comparing the severity
+	 * scores. Use this rather than comparing severities directly when applying a minimum severity
+	 * threshold.
+	 *
+	 * @param other
+	 *            the severity to compare against
+	 * @return true if this severity's score is greater than or equal to the other's
+	 */
+	public boolean isAtLeast(Severity other) {
+		return score >= other.score;
+	}
+
+	/**
+	 * Determines the new maximum severity after a result with {@code resultSeverity} arrives, given
+	 * the {@code currentMax} tracked so far. A result more severe than the current max simply
+	 * becomes the new max. A result less severe than the current max means the previous max may no
+	 * longer be in effect (e.g. its rule was re-evaluated with a lower severity), so the true max
+	 * is recomputed from scratch via {@code recompute}.
+	 *
+	 * @param currentMax
+	 *            the maximum severity tracked so far
+	 * @param resultSeverity
+	 *            the severity of the result that just arrived
+	 * @param recompute
+	 *            supplies the true maximum severity across all results, used when
+	 *            {@code resultSeverity} is less severe than {@code currentMax}
+	 * @return the new maximum severity
+	 */
+	public static Severity nextMax(Severity currentMax, Severity resultSeverity, Supplier<Severity> recompute) {
+		if (resultSeverity.getLimit() > currentMax.getLimit()) {
+			return resultSeverity;
+		} else if (resultSeverity.getLimit() < currentMax.getLimit()) {
+			return recompute.get();
+		}
+		return currentMax;
+	}
+
+	/**
+	 * The constants in descending order of severity score, as required by {@link #get(double)}.
+	 */
 	private static final Severity[] VALUES = {WARNING, INFO, OK, NA, IGNORE};
 
 	public static Severity get(double score) {

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/report/JfrRulesReport.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
  *
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -265,7 +265,7 @@ public int compare(Entry<IRule, ?> o1, Entry<IRule, ?> o2) {
 					continue;
 				}
 
-				if (result != null && result.getSeverity().compareTo(minSeverity) >= 0) {
+				if (result != null && result.getSeverity().isAtLeast(minSeverity)) {
 					Element ruleNode = createRuleNode(parent, reportNode, result.getRule());
 
 					ruleNode.appendChild(createValueNode(parent.getOwnerDocument(), "severity", //$NON-NLS-1$

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/report/html/internal/RulesHtmlToolkit.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
  *
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -278,7 +278,7 @@ public int compare(IResult r1, IResult r2) {
 
 		@Override
 		public int compare(IResult r1, IResult r2) {
-			return r1.getSeverity().compareTo(r2.getSeverity());
+			return Double.compare(r1.getSeverity().getLimit(), r2.getSeverity().getLimit());
 		}
 	};
 
@@ -302,7 +302,7 @@ public static String generateSinglePageHtml(Collection<IResult> results, boolean
 			html.append("</p>"); //$NON-NLS-1$
 		} else {
 			for (IResult result : resultList) {
-				boolean expand = result.getSeverity().compareTo(Severity.INFO) >= 0;
+				boolean expand = result.getSeverity().isAtLeast(Severity.INFO);
 				html.append(createRuleHtml(result, expand, 0));
 			}
 		}

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java

@@ -1423,7 +1423,7 @@
 	private static boolean shouldEvaluate(IRule rule, IResult depResult) {
 		DependsOn dependency = rule.getClass().getAnnotation(DependsOn.class);
 		if (dependency != null) {
-			if (depResult.getSeverity().compareTo(dependency.severity()) < 0) {
+			if (!depResult.getSeverity().isAtLeast(dependency.severity())) {
 				return false;
 			}
 		}
@@ -1492,7 +1492,7 @@
 	 * @return a stack trace frame
 	 * @deprecated Use {@link #getTopNFramesInMostCommonTrace(IItemCollection, int)} instead.
 	 */
 	public static String getSecondFrameInMostCommonTrace(IItemCollection items) {
 		List<StacktraceFrame> frames = getTopNFramesInMostCommonTrace(items, 2);
 		if (frames.size() < 2) {
 			return null;

core/tests/org.openjdk.jmc.flightrecorder.rules.jdk.test/src/main/java/org/openjdk/jmc/flightrecorder/test/rules/jdk/TestRulesWithJfr.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
  *
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -257,7 +257,7 @@ private static boolean shouldEvaluate(Map<Class<? extends IRule>, Severity> eval
 			if (dependencyType != null) {
 				while (true) {
 					if (evaluatedRules.containsKey(dependencyType)) {
-						if (evaluatedRules.get(dependencyType).compareTo(dependency.severity()) < 0) {
+						if (!evaluatedRules.get(dependencyType).isAtLeast(dependency.severity())) {
 							return false;
 						}
 						return true;
@@ -289,7 +289,7 @@ private static Report generateReport(IOResource jfr, boolean verbose, Severity m
 						}
 						evaluatedRules.put(rule.getClass(), result.getSeverity());
 						rp.addResults(result);
-						if (minSeverity == null || result.getSeverity().compareTo(minSeverity) >= 0) {
+						if (minSeverity == null || result.getSeverity().isAtLeast(minSeverity)) {
 							ItemSet itemSet = null;
 							IItemQuery itemQuery = result.getResult(TypedResult.ITEM_QUERY);
 							if (verbose && itemQuery != null && !itemQuery.getAttributes().isEmpty()) {

core/tests/org.openjdk.jmc.flightrecorder.rules.test/src/main/java/org/openjdk/jmc/flightrecorder/rules/SeverityTest.java

@@ -0,0 +1,157 @@
+/*
+ * Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2026, Datadog, Inc. All rights reserved.
+ *
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * The contents of this file are subject to the terms of either the Universal Permissive License
+ * v 1.0 as shown at https://oss.oracle.com/licenses/upl
+ *
+ * or the following license:
+ *
+ * Redistribution and use in source and binary forms, with or without modification, are permitted
+ * provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this list of conditions
+ * and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice, this list of
+ * conditions and the following disclaimer in the documentation and/or other materials provided with
+ * the distribution.
+ *
+ * 3. Neither the name of the copyright holder nor the names of its contributors may be used to
+ * endorse or promote products derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
+ * WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.openjdk.jmc.flightrecorder.rules;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class SeverityTest {
+
+	/**
+	 * The severities in ascending order of severity. This is the order the rest of these tests, and
+	 * the severity filtering throughout the code base, are written against.
+	 */
+	private static final Severity[] ASCENDING = {Severity.IGNORE, Severity.NA, Severity.OK, Severity.INFO,
+			Severity.WARNING};
+
+	/**
+	 * The declaration order of the enum must agree with the severity scores, so that the natural
+	 * ordering of the enum is also the severity ordering. IGNORE used to be declared last, which
+	 * made compareTo rank it as the most severe value and let ignored results through every
+	 * severity filter.
+	 */
+	@Test
+	public void declarationOrderMatchesScoreOrder() {
+		Assert.assertArrayEquals("Severity constants are no longer declared in ascending severity order", ASCENDING,
+				Severity.values());
+		for (int i = 1; i < ASCENDING.length; i++) {
+			Assert.assertTrue(ASCENDING[i - 1] + " should score lower than " + ASCENDING[i],
+					ASCENDING[i - 1].getLimit() < ASCENDING[i].getLimit());
+		}
+	}
+
+	@Test
+	public void ignoreIsTheLeastSevere() {
+		for (Severity severity : Severity.values()) {
+			if (severity != Severity.IGNORE) {
+				Assert.assertFalse("IGNORE must not satisfy a minimum of " + severity,
+						Severity.IGNORE.isAtLeast(severity));
+				Assert.assertTrue(severity + " must satisfy a minimum of IGNORE", severity.isAtLeast(Severity.IGNORE));
+			}
+		}
+	}
+
+	@Test
+	public void isAtLeastMatchesScoreOrder() {
+		for (int i = 0; i < ASCENDING.length; i++) {
+			for (int j = 0; j < ASCENDING.length; j++) {
+				Severity severity = ASCENDING[i];
+				Severity minimum = ASCENDING[j];
+				Assert.assertEquals(severity + ".isAtLeast(" + minimum + ")", i >= j, severity.isAtLeast(minimum));
+			}
+		}
+	}
+
+	@Test
+	public void isAtLeastIsReflexive() {
+		for (Severity severity : Severity.values()) {
+			Assert.assertTrue(severity + " should be at least itself", severity.isAtLeast(severity));
+		}
+	}
+
+	@Test
+	public void getMapsScoresToTheHighestMatchingSeverity() {
+		Assert.assertEquals(Severity.WARNING, Severity.get(100));
+		Assert.assertEquals(Severity.WARNING, Severity.get(75));
+		Assert.assertEquals(Severity.INFO, Severity.get(74));
+		Assert.assertEquals(Severity.INFO, Severity.get(25));
+		Assert.assertEquals(Severity.OK, Severity.get(24));
+		Assert.assertEquals(Severity.OK, Severity.get(0));
+		Assert.assertEquals(Severity.NA, Severity.get(-1));
+		Assert.assertEquals(Severity.IGNORE, Severity.get(-3));
+	}
+
+	/**
+	 * Every constant must be reachable from {@link Severity#get(double)}, which iterates a separate
+	 * array that has to be kept in descending score order.
+	 */
+	@Test
+	public void getReturnsEverySeverity() {
+		for (Severity severity : Severity.values()) {
+			Assert.assertEquals("Severity.get should return " + severity + " for its own score", severity,
+					Severity.get(severity.getLimit()));
+		}
+	}
+
+	/**
+	 * Regression tests for
+	 * {@link Severity#nextMax(Severity, Severity, java.util.function.Supplier)}, which drives the
+	 * icon shown by rule result actions in the UI. A prior version of that logic recomputed the max
+	 * severity when a result dropped below the current max, but never told the caller to refresh
+	 * the icon, leaving a stale (too severe) icon displayed.
+	 */
+	@Test
+	public void moreSevereResultBecomesTheNewMax() {
+		Severity result = Severity.nextMax(Severity.OK, Severity.WARNING, () -> {
+			throw new AssertionError("recompute should not be needed when severity increases");
+		});
+		Assert.assertEquals(Severity.WARNING, result);
+	}
+
+	@Test
+	public void lessSevereResultTriggersRecompute() {
+		Severity result = Severity.nextMax(Severity.WARNING, Severity.OK, () -> Severity.INFO);
+		Assert.assertEquals("the recomputed value must be used so the icon can be refreshed", Severity.INFO, result);
+	}
+
+	@Test
+	public void sameSeverityKeepsCurrentMaxWithoutRecomputing() {
+		Severity result = Severity.nextMax(Severity.INFO, Severity.INFO, () -> {
+			throw new AssertionError("recompute should not be needed when severity is unchanged");
+		});
+		Assert.assertEquals(Severity.INFO, result);
+	}
+
+	/**
+	 * Pins the bug: when severity drops, the caller must be told the max changed (by getting back a
+	 * different reference) so it knows to refresh the icon, even if the recomputed max happens to
+	 * differ from the reported result's own severity.
+	 */
+	@Test
+	public void recomputedMaxCanDifferFromTheReportedResult() {
+		Severity result = Severity.nextMax(Severity.WARNING, Severity.IGNORE, () -> Severity.OK);
+		Assert.assertNotSame(Severity.IGNORE, result);
+		Assert.assertSame(Severity.OK, result);
+	}
+}