softdelete experimentation

andrewazores · andrewazores · commit 185c280f6d33 · 2025-04-17T17:14:00.000-04:00
diff --git a/src/main/java/io/cryostat/discovery/ContainerDiscovery.java b/src/main/java/io/cryostat/discovery/ContainerDiscovery.java
@@ -259,9 +259,8 @@ private Target toTarget(ContainerSpec desc) {
             return null;
         }
 
-        Target target = new Target();
+        Target target = Target.createOrUndelete(connectUrl);
         target.activeRecordings = new ArrayList<>();
-        target.connectUrl = connectUrl;
         target.alias = Optional.ofNullable(desc.Names.get(0)).orElse(desc.Id);
         target.labels = desc.Labels;
         target.annotations =
diff --git a/src/main/java/io/cryostat/discovery/CustomDiscovery.java b/src/main/java/io/cryostat/discovery/CustomDiscovery.java
@@ -115,8 +115,7 @@ public RestResponse<Target> createForm(
             @RestForm String password,
             @RestQuery boolean dryrun,
             @RestQuery boolean storeCredentials) {
-        var target = new Target();
-        target.connectUrl = connectUrl;
+        var target = Target.createOrUndelete(connectUrl);
         target.alias = alias;
 
         Credential credential = null;
diff --git a/src/main/java/io/cryostat/discovery/JDPDiscovery.java b/src/main/java/io/cryostat/discovery/JDPDiscovery.java
@@ -113,9 +113,8 @@ void handleJdpEvent(JvmDiscoveryEvent evt) {
 
         switch (evt.getEventKind()) {
             case FOUND:
-                Target target = new Target();
+                Target target = Target.createOrUndelete(connectUrl);
                 target.activeRecordings = new ArrayList<>();
-                target.connectUrl = connectUrl;
                 target.alias = evt.getJvmDescriptor().getMainClass();
                 target.annotations =
                         new Annotations(
diff --git a/src/main/java/io/cryostat/discovery/KubeApiDiscovery.java b/src/main/java/io/cryostat/discovery/KubeApiDiscovery.java
@@ -616,9 +616,8 @@ public Target toTarget() {
                                 "/jndi/rmi://" + host + ':' + port.getPort() + "/jmxrmi");
                 URI connectUrl = URI.create(jmxUrl.toString());
 
-                Target target = new Target();
+                Target target = Target.createOrUndelete(connectUrl);
                 target.activeRecordings = new ArrayList<>();
-                target.connectUrl = connectUrl;
                 target.alias = objRef.getName();
                 target.labels = (obj != null ? obj.getMetadata().getLabels() : new HashMap<>());
                 target.annotations =
diff --git a/src/main/java/io/cryostat/expressions/MatchExpressionEvaluator.java b/src/main/java/io/cryostat/expressions/MatchExpressionEvaluator.java
@@ -302,7 +302,7 @@ public List<ProgramOption> getProgramOptions() {
         }
 
         private String[] getJfrEventTypeIds(SimplifiedTarget st) {
-            Target target = Target.find("id", st.id()).singleResult();
+            var target = Target.getTargetById(st.id());
             try {
                 return connectionManager.executeConnectedTask(
                         target,
diff --git a/src/main/java/io/cryostat/targets/Target.java b/src/main/java/io/cryostat/targets/Target.java
@@ -33,6 +33,7 @@
 
 import io.cryostat.discovery.DiscoveryNode;
 import io.cryostat.recordings.ActiveRecording;
+import io.cryostat.recordings.RecordingHelper;
 import io.cryostat.util.URIUtil;
 import io.cryostat.ws.MessagingServer;
 import io.cryostat.ws.Notification;
@@ -126,6 +127,37 @@ public String targetId() {
         return this.connectUrl.toString();
     }
 
+    public static Target createOrUndelete(URI connectUrl) {
+        Objects.requireNonNull(connectUrl);
+        var target =
+                Panache.getSession()
+                        // ignore soft deletion field
+                        .createNativeQuery(
+                                "select * from Target where connectUrl = :connectUrl", Target.class)
+                        .setParameter("connectUrl", connectUrl.toString().getBytes())
+                        .uniqueResult();
+        if (target == null) {
+            target = new Target();
+            target.connectUrl = connectUrl;
+        } else {
+            int updates =
+                    Panache.getSession()
+                            .createNativeQuery(
+                                    "update target set deleted = false where id = :id",
+                                    Target.class)
+                            .setParameter("id", target.id)
+                            .executeUpdate();
+            if (updates != 1) {
+                Logger.getLogger(Target.class)
+                        .warnv(
+                                "Attempted to undelete Target {0} with connectUrl={1}, but update"
+                                        + " affected {2} rows",
+                                target.id, connectUrl, updates);
+            }
+        }
+        return target;
+    }
+
     public static List<Target> getTargetsIncludingDeleted() {
         return Panache.getSession()
                 // ignore soft deletion field
@@ -134,27 +166,34 @@ public static List<Target> getTargetsIncludingDeleted() {
     }
 
     public static Target getTargetById(long targetId) {
-        return Panache.getSession()
-                // ignore soft deletion field
-                .createNativeQuery("select * from Target where id = :id", Target.class)
-                .setParameter("id", targetId)
-                .getSingleResult();
+        return getTargetById(targetId, false);
+    }
+
+    public static Target getTargetById(long targetId, boolean includeDeleted) {
+        if (includeDeleted) {
+            return Panache.getSession()
+                    .createNativeQuery("select * from Target where id = :id", Target.class)
+                    .setParameter("id", targetId)
+                    .getSingleResult();
+        } else {
+            return Target.find("id", targetId).singleResult();
+        }
     }
 
     public static Target getTargetByConnectUrl(URI connectUrl) {
         return Panache.getSession()
                 // ignore soft deletion field
                 .createNativeQuery(
                         "select * from Target where connectUrl = :connectUrl", Target.class)
-                .setParameter("connectUrl", connectUrl.toString())
+                .setParameter("connectUrl", connectUrl.toString().getBytes())
                 .getSingleResult();
     }
 
     public static Optional<Target> getTargetByJvmId(String jvmId) {
         return Panache.getSession()
                 // ignore soft deletion field
                 .createNativeQuery("select * from Target where jvmId = :jvmId", Target.class)
-                .setParameter("jvmId", jvmId)
+                .setParameter("jvmId", jvmId.getBytes())
                 .uniqueResultOptional();
     }
 
@@ -309,8 +348,9 @@ public record TargetDiscovery(EventKind kind, Target serviceRef, String jvmId) {
     static class Listener {
 
         @Inject URIUtil uriUtil;
-        @Inject Logger logger;
+        @Inject RecordingHelper recordingHelper;
         @Inject EventBus bus;
+        @Inject Logger logger;
 
         @PrePersist
         void prePersist(Target target) {
@@ -356,6 +396,7 @@ void postUpdate(Target target) {
         @PostRemove
         void postRemove(Target target) {
             notify(EventKind.LOST, target);
+            target.activeRecordings.forEach(recordingHelper::deleteRecording);
         }
 
         private void notify(EventKind eventKind, Target target) {
diff --git a/src/main/java/io/cryostat/targets/TargetUpdateJob.java b/src/main/java/io/cryostat/targets/TargetUpdateJob.java
@@ -48,7 +48,11 @@ public void execute(JobExecutionContext context) throws JobExecutionException {
         List<Target> targets;
         Long targetId = (Long) context.getJobDetail().getJobDataMap().get("targetId");
         if (targetId != null) {
-            targets = List.of(Target.getTargetById(targetId));
+            Target target = Target.findById(targetId);
+            if (target == null) {
+                return;
+            }
+            targets = List.of();
         } else {
             targets = Target.<Target>find("#Target.unconnected").list();
         }
diff --git a/src/main/resources/db/migration/V4.1.0__cryostat.sql b/src/main/resources/db/migration/V4.1.0__cryostat.sql
@@ -13,3 +13,6 @@ alter table Rule add column metadata jsonb default '{"labels":{}}';
 
 delete from DiscoveryNode where nodeType not in ('Universe', 'Realm');
 delete from Target where true;
+
+alter table Target drop constraint FKl0dhd7qeayg54dcoblpww6x34;
+alter table Target drop constraint target_connecturl_key;
diff --git a/src/test/java/itest/TargetRecordingPatchTest.java b/src/test/java/itest/TargetRecordingPatchTest.java
@@ -15,6 +15,7 @@
  */
 package itest;
 
+import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
@@ -34,7 +35,6 @@
 import itest.util.ITestCleanupFailedException;
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 @QuarkusTest
@@ -121,7 +121,7 @@ void testSaveEmptyRecordingDoesNotArchiveRecordingFile() throws Exception {
                                 }
                             });
             JsonArray listResp = listRespFuture1.get(REQUEST_TIMEOUT_SECONDS, TimeUnit.SECONDS);
-            Assertions.assertTrue(listResp.isEmpty());
+            MatcherAssert.assertThat(listResp.getList(), Matchers.equalTo(List.of()));
 
         } finally {
             // Clean up recording

Original file line number	Diff line number	Diff line change
`@@ -302,7 +302,7 @@ public List<ProgramOption> getProgramOptions() {`
`302`	`302`	`}`
`303`	`303`
`304`	`304`	`private String[] getJfrEventTypeIds(SimplifiedTarget st) {`
`305`		`- Target target = Target.find("id", st.id()).singleResult();`
	`305`	`+ var target = Target.getTargetById(st.id());`
`306`	`306`	`try {`
`307`	`307`	`return connectionManager.executeConnectedTask(`
`308`	`308`	`target,`