diff --git a/src/main/java/io/cryostat/discovery/AbstractNodeTypeAdapter.java b/src/main/java/io/cryostat/discovery/AbstractNodeTypeAdapter.java index 81f345ea9b..bb38cef3ce 100644 --- a/src/main/java/io/cryostat/discovery/AbstractNodeTypeAdapter.java +++ b/src/main/java/io/cryostat/discovery/AbstractNodeTypeAdapter.java @@ -46,6 +46,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.UUID; import io.cryostat.core.log.Logger; import io.cryostat.platform.ServiceRef; @@ -128,6 +129,7 @@ public AbstractNode read(JsonReader reader) throws IOException { break; case "target": reader.beginObject(); + UUID id = null; URI connectUrl = null; String alias = null; Map targetLabels = new HashMap<>(); @@ -136,6 +138,9 @@ public AbstractNode read(JsonReader reader) throws IOException { while (reader.hasNext()) { String targetTokenName = reader.nextName(); switch (targetTokenName) { + case "id": + id = UUID.fromString(reader.nextString()); + break; case "connectUrl": try { connectUrl = new URI(reader.nextString()); @@ -190,7 +195,7 @@ public AbstractNode read(JsonReader reader) throws IOException { } } reader.endObject(); - target = new ServiceRef(connectUrl, alias); + target = new ServiceRef(id, connectUrl, alias); target.setLabels(targetLabels); target.setPlatformAnnotations(platformAnnotations); target.setCryostatAnnotations(cryostatAnnotations); @@ -242,6 +247,9 @@ public void write(JsonWriter writer, AbstractNode node) throws IOException { writer.name("target").beginObject(); + if (sr.getId().isPresent()) { + writer.name("id").value(sr.getId().get().toString()); + } writer.name("connectUrl").value(sr.getServiceUri().toString()); writer.name("alias").value(sr.getAlias().orElse(sr.getServiceUri().toString())); writeMap(writer, "labels", sr.getLabels()); diff --git a/src/main/java/io/cryostat/discovery/DiscoveryStorage.java b/src/main/java/io/cryostat/discovery/DiscoveryStorage.java index 2f666e1598..74d17b89f2 100644 --- a/src/main/java/io/cryostat/discovery/DiscoveryStorage.java +++ b/src/main/java/io/cryostat/discovery/DiscoveryStorage.java @@ -176,11 +176,13 @@ public Optional getById(UUID id) { } public Optional getByRealm(String realm) { + Objects.requireNonNull(realm, "realm"); return dao.getByRealm(realm); } public UUID register(String realm, URI callback) throws RegistrationException { // FIXME this method should return a Future and be performed async + Objects.requireNonNull(realm, "realm"); try { CompletableFuture cf = new CompletableFuture<>(); ping(HttpMethod.GET, callback).onComplete(ar -> cf.complete(ar.succeeded())); @@ -200,6 +202,8 @@ public UUID register(String realm, URI callback) throws RegistrationException { } public Set update(UUID id, Set children) { + Objects.requireNonNull(id, "id"); + Objects.requireNonNull(children, "children"); PluginInfo plugin = dao.get(id).orElseThrow(() -> new NotFoundException(id)); logger.trace("Discovery Update {} ({}): {}", id, plugin.getRealm(), children); EnvironmentNode original = gson.fromJson(plugin.getSubtree(), EnvironmentNode.class); @@ -226,6 +230,7 @@ public Set update(UUID id, Set c } public PluginInfo deregister(UUID id) { + Objects.requireNonNull(id, "id"); PluginInfo plugin = dao.get(id).orElseThrow(() -> new NotFoundException(id)); dao.delete(id); findLeavesFrom(gson.fromJson(plugin.getSubtree(), EnvironmentNode.class)).stream() @@ -245,6 +250,7 @@ public EnvironmentNode getDiscoveryTree() { } public Optional getSubtree(String realm) { + Objects.requireNonNull(realm, "realm"); return getByRealm(realm) .map(PluginInfo::getSubtree) .map(s -> gson.fromJson(s, EnvironmentNode.class)); @@ -254,12 +260,27 @@ private Set getLeafNodes() { return findLeavesFrom(getDiscoveryTree()); } + public Optional getServiceById(UUID id) { + Objects.requireNonNull(id, "id"); + for (ServiceRef sr : listDiscoverableServices()) { + if (sr.getId().isEmpty()) { + logger.warn("ServiceRef {} has a null ID!"); + continue; + } + if (sr.getId().get().equals((id))) { + return Optional.of(sr); + } + } + return Optional.empty(); + } + @Override public List listDiscoverableServices() { return getLeafNodes().stream().map(TargetNode::getTarget).toList(); } public List listDiscoverableServices(String realm) { + Objects.requireNonNull(realm, "realm"); return getSubtree(realm) .map(this::findLeavesFrom) .map(leaves -> leaves.stream().map(TargetNode::getTarget).toList()) diff --git a/src/main/java/io/cryostat/discovery/PluginInfoDao.java b/src/main/java/io/cryostat/discovery/PluginInfoDao.java index ae105060e8..5802bf1b3a 100644 --- a/src/main/java/io/cryostat/discovery/PluginInfoDao.java +++ b/src/main/java/io/cryostat/discovery/PluginInfoDao.java @@ -68,13 +68,13 @@ class PluginInfoDao extends AbstractDao { this.gson = gson; } - public PluginInfo save(String realm, URI callback, EnvironmentNode subtree) { + PluginInfo save(String realm, URI callback, EnvironmentNode subtree) { Objects.requireNonNull(realm); Objects.requireNonNull(subtree); return super.save(new PluginInfo(realm, callback, gson.toJson(subtree))); } - public Optional getByRealm(String realm) { + Optional getByRealm(String realm) { Objects.requireNonNull(realm); CriteriaBuilder cb = entityManager.getCriteriaBuilder(); @@ -94,7 +94,7 @@ public Optional getByRealm(String realm) { } } - public PluginInfo update(UUID id, Set children) { + PluginInfo update(UUID id, Set children) { Objects.requireNonNull(id); Objects.requireNonNull(children); EntityTransaction transaction = entityManager.getTransaction(); diff --git a/src/main/java/io/cryostat/platform/ServiceRef.java b/src/main/java/io/cryostat/platform/ServiceRef.java index eec8954f00..41a5bff252 100644 --- a/src/main/java/io/cryostat/platform/ServiceRef.java +++ b/src/main/java/io/cryostat/platform/ServiceRef.java @@ -44,6 +44,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.UUID; import com.google.gson.annotations.SerializedName; import org.apache.commons.lang3.builder.EqualsBuilder; @@ -52,25 +53,41 @@ public class ServiceRef { + private UUID id; private final @SerializedName("connectUrl") URI serviceUri; private final String alias; private final Map labels; private final Annotations annotations; - public ServiceRef(URI uri, String alias) { + // TODO we generate these IDs manually in application code, and these get stored in the + // Discovery database where ServiceRefs are embedded within TargetNodes in the realms' subtrees. + // Should we create a table just for ServiceRefs, allow the database to generate the ID, and + // have a relationship between the Discovery plugin table and the ServiceRef table? It's a tree + // relationship, not a simple relational one-one or many-many etc. + public ServiceRef(UUID id, URI uri, String alias) { + this.id = id != null ? id : UUID.randomUUID(); this.serviceUri = Objects.requireNonNull(uri); this.alias = alias; this.labels = new HashMap<>(); this.annotations = new Annotations(); } + public ServiceRef(URI uri, String alias) { + this(UUID.randomUUID(), uri, alias); + } + public ServiceRef(ServiceRef sr) { + this.id = sr.id; this.serviceUri = sr.serviceUri; this.alias = sr.alias; this.labels = new HashMap(sr.labels); this.annotations = new Annotations(sr.annotations); } + public Optional getId() { + return Optional.ofNullable(id); + } + public URI getServiceUri() { return serviceUri; } @@ -127,6 +144,7 @@ public boolean equals(Object other) { return false; } ServiceRef sr = (ServiceRef) other; + // id is intentionally ignored return new EqualsBuilder() .append(serviceUri, sr.serviceUri) .append(alias, sr.alias) @@ -137,6 +155,7 @@ public boolean equals(Object other) { @Override public int hashCode() { + // id is intentionally ignored return new HashCodeBuilder() .append(serviceUri) .append(alias) diff --git a/src/test/java/io/cryostat/discovery/DiscoveryStorageTest.java b/src/test/java/io/cryostat/discovery/DiscoveryStorageTest.java index 5a045d12e4..d53907d989 100644 --- a/src/test/java/io/cryostat/discovery/DiscoveryStorageTest.java +++ b/src/test/java/io/cryostat/discovery/DiscoveryStorageTest.java @@ -388,7 +388,6 @@ class Updating { @Test void throwsIfUuidNull() { - Mockito.when(dao.get(Mockito.isNull())).thenThrow(NullPointerException.class); Assertions.assertThrows( NullPointerException.class, () -> storage.update(null, Set.of())); } @@ -401,9 +400,8 @@ void throwsIfInvalidIdGiven() { @Test void throwsIfChildrenNull() { - UUID id = UUID.randomUUID(); - Mockito.when(dao.get(id)).thenReturn(Optional.of(new PluginInfo())); - Assertions.assertThrows(NullPointerException.class, () -> storage.update(id, null)); + Assertions.assertThrows( + NullPointerException.class, () -> storage.update(UUID.randomUUID(), null)); } @Test @@ -456,7 +454,6 @@ class Deregistration { @Test void throwsIfUuidNull() { - Mockito.when(dao.get(Mockito.isNull())).thenThrow(NullPointerException.class); Assertions.assertThrows(NullPointerException.class, () -> storage.deregister(null)); } diff --git a/src/test/java/itest/CustomTargetsIT.java b/src/test/java/itest/CustomTargetsIT.java index 08a9b53f5a..534cc7a3bb 100644 --- a/src/test/java/itest/CustomTargetsIT.java +++ b/src/test/java/itest/CustomTargetsIT.java @@ -50,6 +50,7 @@ import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import itest.bases.StandardSelfTest; +import org.hamcrest.Matcher; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.junit.jupiter.api.MethodOrderer.OrderAnnotation; @@ -133,45 +134,69 @@ void targetShouldAppearInListing() throws ExecutionException, InterruptedExcepti MatcherAssert.assertThat(body, Matchers.notNullValue()); MatcherAssert.assertThat(body.size(), Matchers.equalTo(2)); - JsonObject selfJdp = - new JsonObject( - Map.of( - "alias", - "io.cryostat.Cryostat", - "connectUrl", - "service:jmx:rmi:///jndi/rmi://cryostat-itests:9091/jmxrmi", - "labels", - Map.of(), - "annotations", - Map.of( - "cryostat", - Map.of( - "REALM", - "JDP", - "HOST", - "cryostat-itests", - "PORT", - "9091", - "JAVA_MAIN", - "io.cryostat.Cryostat"), - "platform", - Map.of()))); - JsonObject selfCustom = - new JsonObject( - Map.of( - "alias", - "self", - "connectUrl", - "localhost:0", - "labels", - Map.of(), - "annotations", - Map.of( - "cryostat", - Map.of("REALM", "Custom Targets"), - "platform", - Map.of()))); - MatcherAssert.assertThat(body, Matchers.containsInAnyOrder(selfJdp, selfCustom)); + Matcher selfJdpMatcher = + Matchers.allOf( + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("id"), + Matchers.not(Matchers.blankOrNullString())), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("alias"), + Matchers.equalTo("io.cryostat.Cryostat")), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("connectUrl"), + Matchers.equalTo( + "service:jmx:rmi:///jndi/rmi://cryostat-itests:9091/jmxrmi")), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("labels"), Matchers.equalTo(Map.of())), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("annotations"), + Matchers.equalTo( + Map.of( + "cryostat", + Map.of( + "REALM", + "JDP", + "HOST", + "cryostat-itests", + "PORT", + "9091", + "JAVA_MAIN", + "io.cryostat.Cryostat"), + "platform", Map.of())))); + + Matcher selfCustomMatcher = + Matchers.allOf( + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("id"), + Matchers.not(Matchers.blankOrNullString())), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("alias"), Matchers.equalTo("self")), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("connectUrl"), + Matchers.equalTo("localhost:0")), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("labels"), Matchers.equalTo(Map.of())), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("annotations"), + Matchers.equalTo( + Map.of( + "cryostat", + Map.of("REALM", "Custom Targets"), + "platform", + Map.of())))); + + MatcherAssert.assertThat( + body.getList(), Matchers.containsInAnyOrder(selfJdpMatcher, selfCustomMatcher)); } @Test @@ -260,6 +285,41 @@ void targetShouldNoLongerAppearInListing() throws ExecutionException, Interrupte "io.cryostat.Cryostat"), "platform", Map.of()))); - MatcherAssert.assertThat(body, Matchers.contains(selfJdp)); + + Matcher selfJdpMatcher = + Matchers.allOf( + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("id"), + Matchers.not(Matchers.blankOrNullString())), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("alias"), + Matchers.equalTo("io.cryostat.Cryostat")), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("connectUrl"), + Matchers.equalTo( + "service:jmx:rmi:///jndi/rmi://cryostat-itests:9091/jmxrmi")), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("labels"), Matchers.equalTo(Map.of())), + (Matcher) + Matchers.hasEntry( + Matchers.equalTo("annotations"), + Matchers.equalTo( + Map.of( + "cryostat", + Map.of( + "REALM", + "JDP", + "HOST", + "cryostat-itests", + "PORT", + "9091", + "JAVA_MAIN", + "io.cryostat.Cryostat"), + "platform", Map.of())))); + MatcherAssert.assertThat(body.getList(), Matchers.containsInAnyOrder(selfJdpMatcher)); } }