From 6e2af66b29dba1d0debebfc48136b9874b23ca04 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 25 Oct 2023 17:27:22 -0400 Subject: [PATCH] chore(spotbugs): resolve spotbugs warnings (#120) --- .../discovery/ContainerDiscovery.java | 3 +- .../io/cryostat/discovery/DiscoveryNode.java | 40 ++++++++---------- .../cryostat/discovery/DiscoveryPlugin.java | 41 ++++++++++++------- .../events/SerializableEventTypeInfo.java | 3 ++ .../cryostat/expressions/MatchExpression.java | 3 ++ .../expressions/MatchExpressionEvaluator.java | 6 +-- .../recordings/EventOptionsBuilder.java | 4 +- .../cryostat/recordings/RecordingHelper.java | 14 +++++-- .../io/cryostat/recordings/Recordings.java | 13 +++++- src/main/java/io/cryostat/rules/Rule.java | 7 ++++ .../java/io/cryostat/rules/RuleService.java | 2 + src/main/java/io/cryostat/targets/Target.java | 3 ++ 12 files changed, 89 insertions(+), 50 deletions(-) diff --git a/src/main/java/io/cryostat/discovery/ContainerDiscovery.java b/src/main/java/io/cryostat/discovery/ContainerDiscovery.java index 06a6c37ae..4c73cd17b 100644 --- a/src/main/java/io/cryostat/discovery/ContainerDiscovery.java +++ b/src/main/java/io/cryostat/discovery/ContainerDiscovery.java @@ -338,9 +338,8 @@ public void handleContainerEvent(ContainerSpec desc, EventKind evtKind) { DiscoveryNode node = DiscoveryNode.target(target); target.discoveryNode = node; String podName = desc.PodName; - DiscoveryNode pod = new DiscoveryNode(); if (StringUtils.isNotBlank(podName)) { - pod = DiscoveryNode.environment(podName, DiscoveryNode.POD); + DiscoveryNode pod = DiscoveryNode.environment(podName, DiscoveryNode.POD); if (!realm.children.contains(pod)) { pod.children.add(node); realm.children.add(pod); diff --git a/src/main/java/io/cryostat/discovery/DiscoveryNode.java b/src/main/java/io/cryostat/discovery/DiscoveryNode.java index 16eb8a286..14ecfecc3 100644 --- a/src/main/java/io/cryostat/discovery/DiscoveryNode.java +++ b/src/main/java/io/cryostat/discovery/DiscoveryNode.java @@ -24,14 +24,11 @@ import java.util.function.Predicate; import io.cryostat.targets.Target; -import io.cryostat.targets.Target.TargetDiscovery; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; import com.fasterxml.jackson.annotation.JsonView; import io.quarkus.hibernate.orm.panache.PanacheEntity; -import io.quarkus.vertx.ConsumeEvent; -import io.smallrye.common.annotation.Blocking; import io.vertx.mutiny.core.eventbus.EventBus; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; @@ -46,7 +43,6 @@ import jakarta.persistence.PostRemove; import jakarta.persistence.PostUpdate; import jakarta.persistence.PrePersist; -import jakarta.transaction.Transactional; import org.hibernate.annotations.JdbcTypeCode; import org.hibernate.type.SqlTypes; import org.jboss.logging.Logger; @@ -55,10 +51,10 @@ @EntityListeners(DiscoveryNode.Listener.class) public class DiscoveryNode extends PanacheEntity { - public static String NODE_TYPE = "nodeType"; - public static String UNIVERSE = "Universe"; - public static String REALM = "Realm"; - public static String POD = "Pod"; + public static final String NODE_TYPE = "nodeType"; + public static final String UNIVERSE = "Universe"; + public static final String REALM = "Realm"; + public static final String POD = "Pod"; @Column(unique = false, nullable = false, updatable = false) @JsonView(Views.Flat.class) @@ -151,20 +147,20 @@ static class Listener { @Inject Logger logger; @Inject EventBus bus; - @Transactional - @Blocking - @ConsumeEvent(Target.TARGET_JVM_DISCOVERY) - void onMessage(TargetDiscovery event) { - switch (event.kind()) { - case LOST: - break; - case FOUND: - break; - default: - // no-op - break; - } - } + // @Transactional + // @Blocking + // @ConsumeEvent(Target.TARGET_JVM_DISCOVERY) + // void onMessage(TargetDiscovery event) { + // switch (event.kind()) { + // case LOST: + // break; + // case FOUND: + // break; + // default: + // // no-op + // break; + // } + // } @PrePersist void prePersist(DiscoveryNode node) {} diff --git a/src/main/java/io/cryostat/discovery/DiscoveryPlugin.java b/src/main/java/io/cryostat/discovery/DiscoveryPlugin.java index 52e5d6d12..c49f3c986 100644 --- a/src/main/java/io/cryostat/discovery/DiscoveryPlugin.java +++ b/src/main/java/io/cryostat/discovery/DiscoveryPlugin.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.UUID; @@ -36,6 +37,7 @@ import jakarta.persistence.Id; import jakarta.persistence.OneToOne; import jakarta.persistence.PrePersist; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.GET; import jakarta.ws.rs.Path; import jakarta.ws.rs.client.ClientRequestContext; @@ -119,30 +121,39 @@ public void filter(ClientRequestContext requestContext) throws IOException { return; } - Credential credential = null; if (StringUtils.isNotBlank(userInfo) && userInfo.contains(":")) { String[] parts = userInfo.split(":"); - if ("storedcredentials".equals(parts[0])) { + if (parts.length == 2 && "storedcredentials".equals(parts[0])) { logger.infov( "Using stored credentials id:{0} referenced in ping callback" + " userinfo", parts[1]); - credential = Credential.find("id", Long.parseLong(parts[1])).singleResult(); + Credential credential = + Credential.find("id", Long.parseLong(parts[1])).singleResult(); + + requestContext + .getHeaders() + .add( + HttpHeaders.AUTHORIZATION, + "Basic " + + Base64.getEncoder() + .encodeToString( + (credential.username + + ":" + + credential + .password) + .getBytes( + StandardCharsets + .UTF_8))); + } else { + throw new IllegalStateException("Unexpected credential format"); } + } else { + throw new IOException( + new BadRequestException( + "No credentials provided and none found in storage")); } - - requestContext - .getHeaders() - .add( - HttpHeaders.AUTHORIZATION, - "Basic " - + Base64.getEncoder() - .encodeToString( - (credential.username - + ":" - + credential.password) - .getBytes())); } } } diff --git a/src/main/java/io/cryostat/events/SerializableEventTypeInfo.java b/src/main/java/io/cryostat/events/SerializableEventTypeInfo.java index 2bfe3b356..b6a489784 100644 --- a/src/main/java/io/cryostat/events/SerializableEventTypeInfo.java +++ b/src/main/java/io/cryostat/events/SerializableEventTypeInfo.java @@ -23,6 +23,9 @@ import org.openjdk.jmc.common.unit.IOptionDescriptor; import org.openjdk.jmc.rjmx.services.jfr.IEventTypeInfo; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +@SuppressFBWarnings("EI_EXPOSE_REP") public record SerializableEventTypeInfo( String name, String typeId, diff --git a/src/main/java/io/cryostat/expressions/MatchExpression.java b/src/main/java/io/cryostat/expressions/MatchExpression.java index 447305df2..b576d9b94 100644 --- a/src/main/java/io/cryostat/expressions/MatchExpression.java +++ b/src/main/java/io/cryostat/expressions/MatchExpression.java @@ -27,6 +27,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.quarkus.hibernate.orm.panache.PanacheEntity; import io.vertx.mutiny.core.eventbus.EventBus; import jakarta.annotation.Nullable; @@ -89,6 +90,7 @@ public MatchedExpression match(MatchExpression expr) throws ScriptException { } } + @SuppressFBWarnings("EI_EXPOSE_REP") public static record MatchedExpression( @Nullable Long id, String expression, Collection targets) { public MatchedExpression { @@ -145,6 +147,7 @@ private void notify(ExpressionEventCategory category, MatchExpression expr) { } } + @SuppressFBWarnings("EI_EXPOSE_REP") public record ExpressionEvent(ExpressionEventCategory category, MatchExpression expression) { public ExpressionEvent { Objects.requireNonNull(category); diff --git a/src/main/java/io/cryostat/expressions/MatchExpressionEvaluator.java b/src/main/java/io/cryostat/expressions/MatchExpressionEvaluator.java index 88fe2898f..e25a5350c 100644 --- a/src/main/java/io/cryostat/expressions/MatchExpressionEvaluator.java +++ b/src/main/java/io/cryostat/expressions/MatchExpressionEvaluator.java @@ -28,6 +28,7 @@ import io.cryostat.targets.Target; import io.cryostat.targets.Target.Annotations; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.quarkus.cache.Cache; import io.quarkus.cache.CacheInvalidate; import io.quarkus.cache.CacheManager; @@ -171,10 +172,7 @@ public List getMatchedTargets(MatchExpression matchExpression) { @Name("io.cryostat.rules.MatchExpressionEvaluator.MatchExpressionAppliesEvent") @Label("Match Expression Evaluation") @Category("Cryostat") - // @SuppressFBWarnings( - // value = "URF_UNREAD_FIELD", - // justification = "The event fields are recorded with JFR instead of accessed - // directly") + @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "URF_UNREAD_FIELD"}) public static class MatchExpressionAppliesEvent extends Event { String matchExpression; diff --git a/src/main/java/io/cryostat/recordings/EventOptionsBuilder.java b/src/main/java/io/cryostat/recordings/EventOptionsBuilder.java index cf3865647..b4c50345a 100644 --- a/src/main/java/io/cryostat/recordings/EventOptionsBuilder.java +++ b/src/main/java/io/cryostat/recordings/EventOptionsBuilder.java @@ -32,6 +32,8 @@ import io.cryostat.core.net.JFRConnection; import io.cryostat.core.tui.ClientWriter; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + public class EventOptionsBuilder { private final boolean isV2; @@ -82,7 +84,7 @@ static V capture(T t) { return (V) t; } - // @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "Field is never mutated") + @SuppressFBWarnings("EI_EXPOSE_REP") public IConstrainedMap build() { if (!isV2) { return null; diff --git a/src/main/java/io/cryostat/recordings/RecordingHelper.java b/src/main/java/io/cryostat/recordings/RecordingHelper.java index 225b86831..0f0fceb4b 100644 --- a/src/main/java/io/cryostat/recordings/RecordingHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingHelper.java @@ -109,7 +109,7 @@ public class RecordingHelper { Pattern.compile("^template=([\\w]+)(?:,type=([\\w]+))?$"); public static final String DATASOURCE_FILENAME = "cryostat-analysis.jfr"; - private final long httpTimeoutSeconds = 5; // TODO: configurable client timeout + private static final long httpTimeoutSeconds = 5; // TODO: configurable client timeout @Inject Logger logger; @Inject EntityManager entityManager; @@ -608,10 +608,16 @@ private Tagging createMetadataTagging(Metadata metadata) { Tag.builder() .key( base64Url.encodeAsString( - e.getKey().getBytes())) + e.getKey() + .getBytes( + StandardCharsets + .UTF_8))) .value( base64Url.encodeAsString( - e.getValue().getBytes())) + e.getValue() + .getBytes( + StandardCharsets + .UTF_8))) .build()) .toList()); if (metadata.expiry() != null) { @@ -623,7 +629,7 @@ private Tagging createMetadataTagging(Metadata metadata) { metadata.expiry() .atOffset(ZoneOffset.UTC) .toString() - .getBytes())) + .getBytes(StandardCharsets.UTF_8))) .build()); } return Tagging.builder().tagSet(tags).build(); diff --git a/src/main/java/io/cryostat/recordings/Recordings.java b/src/main/java/io/cryostat/recordings/Recordings.java index 26c80ec26..62453e052 100644 --- a/src/main/java/io/cryostat/recordings/Recordings.java +++ b/src/main/java/io/cryostat/recordings/Recordings.java @@ -55,6 +55,7 @@ import com.arjuna.ats.jta.exceptions.NotImplementedException; import com.fasterxml.jackson.databind.ObjectMapper; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.quarkus.runtime.StartupEvent; import io.smallrye.common.annotation.Blocking; import io.vertx.core.json.JsonObject; @@ -849,10 +850,16 @@ private Tagging createMetadataTagging(Metadata metadata) { Tag.builder() .key( base64Url.encodeAsString( - e.getKey().getBytes())) + e.getKey() + .getBytes( + StandardCharsets + .UTF_8))) .value( base64Url.encodeAsString( - e.getValue().getBytes())) + e.getValue() + .getBytes( + StandardCharsets + .UTF_8))) .build()) .toList()) .build(); @@ -954,6 +961,7 @@ public record ArchivedRecording( } } + @SuppressFBWarnings("EI_EXPOSE_REP") public record ArchivedRecordingDirectory( String connectUrl, String jvmId, List recordings) { public ArchivedRecordingDirectory { @@ -965,6 +973,7 @@ public record ArchivedRecordingDirectory( } } + @SuppressFBWarnings("EI_EXPOSE_REP") public record Metadata(Map labels, Instant expiry) { public Metadata { Objects.requireNonNull(labels); diff --git a/src/main/java/io/cryostat/rules/Rule.java b/src/main/java/io/cryostat/rules/Rule.java index 083afda96..c371472bd 100644 --- a/src/main/java/io/cryostat/rules/Rule.java +++ b/src/main/java/io/cryostat/rules/Rule.java @@ -21,6 +21,7 @@ import io.cryostat.ws.MessagingServer; import io.cryostat.ws.Notification; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.quarkus.hibernate.orm.panache.PanacheEntity; import io.vertx.mutiny.core.eventbus.EventBus; import jakarta.enterprise.context.ApplicationScoped; @@ -41,6 +42,11 @@ @Entity @EntityListeners(Rule.Listener.class) +@SuppressFBWarnings( + value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD", + justification = + "rule.description is not used directly anywhere, but it is serialized and may be" + + " displayed by clients") public class Rule extends PanacheEntity { public static final String RULE_ADDRESS = "io.cryostat.rules.Rule"; @@ -121,6 +127,7 @@ private void notify(RuleEventCategory category, Rule rule) { } } + @SuppressFBWarnings("EI_EXPOSE_REP") public record RuleEvent(RuleEventCategory category, Rule rule) { public RuleEvent { Objects.requireNonNull(category); diff --git a/src/main/java/io/cryostat/rules/RuleService.java b/src/main/java/io/cryostat/rules/RuleService.java index 8c3e95174..1601d5998 100644 --- a/src/main/java/io/cryostat/rules/RuleService.java +++ b/src/main/java/io/cryostat/rules/RuleService.java @@ -44,6 +44,7 @@ import io.cryostat.targets.Target; import io.cryostat.targets.TargetConnectionManager; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.quarkus.runtime.StartupEvent; import io.quarkus.vertx.ConsumeEvent; import io.smallrye.common.annotation.Blocking; @@ -281,6 +282,7 @@ private void cancelTasksForRule(Rule rule) { } } + @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "EI_EXPOSE_REP2"}) public record RuleRecording(Rule rule, ActiveRecording recording) { public RuleRecording { Objects.requireNonNull(rule); diff --git a/src/main/java/io/cryostat/targets/Target.java b/src/main/java/io/cryostat/targets/Target.java index 51bf35226..50dad8780 100644 --- a/src/main/java/io/cryostat/targets/Target.java +++ b/src/main/java/io/cryostat/targets/Target.java @@ -35,6 +35,7 @@ import io.cryostat.ws.Notification; import com.fasterxml.jackson.annotation.JsonIgnore; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.quarkus.hibernate.orm.panache.PanacheEntity; import io.quarkus.vertx.ConsumeEvent; import io.smallrye.common.annotation.Blocking; @@ -125,6 +126,7 @@ public ActiveRecording getRecordingById(long remoteId) { .orElse(null); } + @SuppressFBWarnings("EI_EXPOSE_REP") public static record Annotations(Map platform, Map cryostat) { public Annotations { if (platform == null) { @@ -171,6 +173,7 @@ public enum EventKind { ; } + @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "EI_EXPOSE_REP2"}) public record TargetDiscovery(EventKind kind, Target serviceRef) { public TargetDiscovery { Objects.requireNonNull(kind);