Skip to content

Commit

Permalink
chore(refactor): improve null safety of various structures (#50)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewazores authored Aug 25, 2023
1 parent b598524 commit 428c8ce
Show file tree
Hide file tree
Showing 12 changed files with 151 additions and 84 deletions.
14 changes: 12 additions & 2 deletions src/main/java/io/cryostat/V2Response.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,23 @@
*/
package io.cryostat;

import java.util.Objects;

import jakarta.annotation.Nullable;

public record V2Response(Meta meta, Data data) {
public static V2Response json(Object payload, String status) {
return new V2Response(new Meta("application/json", status), new Data(payload));
}

public record Meta(String type, String status) {}
// FIXME the type and status should both come from an enum and be non-null
public record Meta(String type, String status) {
public Meta {
Objects.requireNonNull(type);
Objects.requireNonNull(status);
}
}

public record Data(Object result) {}
public record Data(@Nullable Object result) {}
}
// {"meta":{"type":"application/json","status":"OK"},"data":{"result":{"name":"Test_Rule","description":"This is a rule for testing","matchExpression":"target.alias=='io.cryostat.Cryostat'","eventSpecifier":"template=Continuous,type=TARGET","archivalPeriodSeconds":30,"preservedArchives":1,"maxAgeSeconds":30,"maxSizeBytes":-1}}}
2 changes: 1 addition & 1 deletion src/main/java/io/cryostat/discovery/CustomDiscovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public Response create(Target target, @RestQuery boolean dryrun) {
target.activeRecordings = new ArrayList<>();
target.labels = Map.of();
target.annotations = new Annotations();
target.annotations.cryostat.putAll(Map.of("REALM", REALM));
target.annotations.cryostat().putAll(Map.of("REALM", REALM));

DiscoveryNode node = DiscoveryNode.target(target);
target.discoveryNode = node;
Expand Down
22 changes: 12 additions & 10 deletions src/main/java/io/cryostat/discovery/JDPDiscovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,18 @@ public synchronized void handleJdpEvent(JvmDiscoveryEvent evt) {
target.alias = evt.getJvmDescriptor().getMainClass();
target.labels = Map.of();
target.annotations = new Annotations();
target.annotations.cryostat.putAll(
Map.of(
"REALM", // AnnotationKey.REALM,
REALM,
"JAVA_MAIN", // AnnotationKey.JAVA_MAIN,
evt.getJvmDescriptor().getMainClass(),
"HOST", // AnnotationKey.HOST,
rmiTarget.getHost(),
"PORT", // "AnnotationKey.PORT,
Integer.toString(rmiTarget.getPort())));
target.annotations
.cryostat()
.putAll(
Map.of(
"REALM", // AnnotationKey.REALM,
REALM,
"JAVA_MAIN", // AnnotationKey.JAVA_MAIN,
evt.getJvmDescriptor().getMainClass(),
"HOST", // AnnotationKey.HOST,
rmiTarget.getHost(),
"PORT", // "AnnotationKey.PORT,
Integer.toString(rmiTarget.getPort())));

DiscoveryNode node = DiscoveryNode.target(target);

Expand Down
18 changes: 10 additions & 8 deletions src/main/java/io/cryostat/discovery/PodmanDiscovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -270,14 +270,16 @@ public void handlePodmanEvent(ContainerSpec desc, EventKind evtKind) {
target.alias = Optional.ofNullable(desc.Names.get(0)).orElse(desc.Id);
target.labels = desc.Labels;
target.annotations = new Annotations();
target.annotations.cryostat.putAll(
Map.of(
"REALM", // AnnotationKey.REALM,
REALM,
"HOST", // AnnotationKey.HOST,
hostname,
"PORT", // "AnnotationKey.PORT,
Integer.toString(jmxPort)));
target.annotations
.cryostat()
.putAll(
Map.of(
"REALM", // AnnotationKey.REALM,
REALM,
"HOST", // AnnotationKey.HOST,
hostname,
"PORT", // "AnnotationKey.PORT,
Integer.toString(jmxPort)));

DiscoveryNode node = DiscoveryNode.target(target);
target.discoveryNode = node;
Expand Down
44 changes: 15 additions & 29 deletions src/main/java/io/cryostat/events/SerializableEventTypeInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
package io.cryostat.events;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
Expand All @@ -30,6 +30,20 @@ public record SerializableEventTypeInfo(
String[] category,
Map<String, SerializableOptionDescriptor> options) {

public SerializableEventTypeInfo {
Objects.requireNonNull(name);
Objects.requireNonNull(typeId);
if (description == null) {
description = "";
}
if (category == null) {
category = new String[0];
}
if (options == null) {
options = Collections.emptyMap();
}
}

public static SerializableEventTypeInfo fromEventTypeInfo(IEventTypeInfo info) {
var name = info.getName();
var typeId = info.getEventTypeID().getFullKey();
Expand All @@ -46,32 +60,4 @@ public static SerializableEventTypeInfo fromEventTypeInfo(IEventTypeInfo info) {

return new SerializableEventTypeInfo(name, typeId, description, category, options);
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + Arrays.hashCode(category);
result = prime * result + Objects.hash(description, name, options, typeId);
return result;
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
SerializableEventTypeInfo other = (SerializableEventTypeInfo) obj;
return Arrays.equals(category, other.category)
&& Objects.equals(description, other.description)
&& Objects.equals(name, other.name)
&& Objects.equals(options, other.options)
&& Objects.equals(typeId, other.typeId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,16 @@
*/
package io.cryostat.events;

import java.util.Objects;

import org.openjdk.jmc.common.unit.IOptionDescriptor;

public record SerializableOptionDescriptor(String name, String description, String defaultValue) {
public SerializableOptionDescriptor {
Objects.requireNonNull(name);
Objects.requireNonNull(description);
Objects.requireNonNull(defaultValue);
}

public static SerializableOptionDescriptor fromOptionDescriptor(IOptionDescriptor<?> desc) {
var name = desc.getName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package io.cryostat.expressions;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -25,6 +26,7 @@

import io.cryostat.expressions.MatchExpression.ExpressionEvent;
import io.cryostat.targets.Target;
import io.cryostat.targets.Target.Annotations;

import io.quarkus.cache.Cache;
import io.quarkus.cache.CacheInvalidate;
Expand Down Expand Up @@ -201,6 +203,18 @@ private static record SimplifiedTarget(
String jvmId,
Map<String, String> labels,
Target.Annotations annotations) {
SimplifiedTarget {
Objects.requireNonNull(connectUrl);
Objects.requireNonNull(alias);
Objects.requireNonNull(jvmId);
if (labels == null) {
labels = Collections.emptyMap();
}
if (annotations == null) {
annotations = new Annotations();
}
}

static SimplifiedTarget from(Target target) {
return new SimplifiedTarget(
target.connectUrl.toString(),
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/io/cryostat/recordings/ActiveRecording.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.io.IOException;
import java.net.URI;
import java.util.Objects;

import org.openjdk.jmc.common.unit.UnitLookup;
import org.openjdk.jmc.rjmx.ServiceNotAvailableException;
Expand Down Expand Up @@ -222,6 +223,11 @@ private void notify(String category, ActiveRecording recording) {
recordingHelper.toExternalForm(recording))));
}

public record RecordingEvent(URI target, Object recording) {}
public record RecordingEvent(URI target, Object recording) {
public RecordingEvent {
Objects.requireNonNull(target);
Objects.requireNonNull(recording);
}
}
}
}
46 changes: 43 additions & 3 deletions src/main/java/io/cryostat/recordings/Recordings.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -840,18 +841,57 @@ public record LinkedRecordingDescriptor(
String name,
String downloadUrl,
String reportUrl,
Metadata metadata) {}
Metadata metadata) {
public LinkedRecordingDescriptor {
Objects.requireNonNull(state);
Objects.requireNonNull(name);
Objects.requireNonNull(downloadUrl);
Objects.requireNonNull(reportUrl);
Objects.requireNonNull(metadata);
}

public static LinkedRecordingDescriptor from(ActiveRecording recording) {
return new LinkedRecordingDescriptor(
recording.remoteId,
recording.state,
recording.duration,
recording.startTime,
recording.continuous,
recording.toDisk,
recording.maxSize,
recording.maxAge,
recording.name,
"TODO",
"TODO",
recording.metadata);
}
}

public record ArchivedRecording(
String name,
String downloadUrl,
String reportUrl,
Metadata metadata,
long size,
long archivedTime) {}
long archivedTime) {
public ArchivedRecording {
Objects.requireNonNull(name);
Objects.requireNonNull(downloadUrl);
Objects.requireNonNull(reportUrl);
Objects.requireNonNull(metadata);
}
}

public record ArchivedRecordingDirectory(
String connectUrl, String jvmId, List<ArchivedRecording> recordings) {}
String connectUrl, String jvmId, List<ArchivedRecording> recordings) {
public ArchivedRecordingDirectory {
Objects.requireNonNull(connectUrl);
Objects.requireNonNull(jvmId);
if (recordings == null) {
recordings = Collections.emptyList();
}
}
}

public record Metadata(Map<String, String> labels) {
public Metadata {
Expand Down
49 changes: 21 additions & 28 deletions src/main/java/io/cryostat/targets/Target.java
Original file line number Diff line number Diff line change
Expand Up @@ -120,34 +120,18 @@ public ActiveRecording getRecordingById(long remoteId) {
.orElse(null);
}

public static class Annotations {
public Map<String, String> platform;
public Map<String, String> cryostat;

public Annotations() {
this.platform = new HashMap<>();
this.cryostat = new HashMap<>();
}

@Override
public int hashCode() {
return Objects.hash(cryostat, platform);
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
public static record Annotations(Map<String, String> platform, Map<String, String> cryostat) {
public Annotations {
if (platform == null) {
platform = new HashMap<>();
}
if (getClass() != obj.getClass()) {
return false;
if (cryostat == null) {
cryostat = new HashMap<>();
}
Annotations other = (Annotations) obj;
return Objects.equals(cryostat, other.cryostat)
&& Objects.equals(platform, other.platform);
}

public Annotations() {
this(new HashMap<>(), new HashMap<>());
}
}

Expand Down Expand Up @@ -182,7 +166,12 @@ public enum EventKind {
;
}

public record TargetDiscovery(EventKind kind, Target serviceRef) {}
public record TargetDiscovery(EventKind kind, Target serviceRef) {
public TargetDiscovery {
Objects.requireNonNull(kind);
Objects.requireNonNull(serviceRef);
}
}

@ApplicationScoped
static class Listener {
Expand Down Expand Up @@ -269,6 +258,10 @@ private void notify(EventKind eventKind, Target target) {
bus.publish(TARGET_JVM_DISCOVERY, new TargetDiscovery(eventKind, target));
}

public record TargetDiscoveryEvent(TargetDiscovery event) {}
public record TargetDiscoveryEvent(TargetDiscovery event) {
public TargetDiscoveryEvent {
Objects.requireNonNull(event);
}
}
}
}
9 changes: 8 additions & 1 deletion src/main/java/io/cryostat/ws/Notification.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,11 @@
*/
package io.cryostat.ws;

public record Notification(String category, Object message) {}
import java.util.Objects;

// FIXME the category should come from an enum and be non-null
public record Notification(String category, Object message) {
public Notification {
Objects.requireNonNull(category);
}
}
2 changes: 1 addition & 1 deletion src/test/java/itest/TargetEventsGetIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void testGetTargetEventsV2WithQueryReturnsRequestedEvents() throws Except
expectedResults.put("name", "Target Connection Opened");
expectedResults.put(
"typeId", "io.cryostat.net.TargetConnectionManager.TargetConnectionOpened");
expectedResults.put("description", null);
expectedResults.put("description", "");
expectedResults.put("category", List.of("Cryostat"));
expectedResults.put(
"options",
Expand Down

0 comments on commit 428c8ce

Please sign in to comment.