fix(jmc-agent): Agent plugin updates and cleanup (#156) (#163)

* Basic agent functionality (TODO: Method & field capturing xml model)

* Added more classes for agent probe xml modelling, added support for deleting templates

* Cleaning up localProbetemplateService, adding XML Validation

* Cleanup, adding tests, implementing getTemplate

* Remove extraneous non-nls tags, cleanup, rework probeTemplate.deserialize

* Remove extraneous non-nls tags

* clean up unnecessary catches, turn ProbeTemplateService into an interface, clean up getLocalTemplates

* Renaming ProbeTemplateService

* Adding invalid converter/relation key to exception message, rethrowing exceptions in AgentJMXHelper

* running spotless

* Agent Plugin updates

* Fixing visibility of MBenServerConnection

* Fix template serialization

* Cleanup of XML stream, removing unnecessary toString override

* Add check for connection before returning from getHandle

* Running spotless

(cherry picked from commit da0f563)

Co-authored-by: Joshua Matsuoka <Josh.matsuoka@gmail.com>

src/main/java/io/cryostat/core/agent/AgentXMLStream.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright The Cryostat Authors
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or data
+ * (collectively the "Software"), free of charge and under any and all copyright
+ * rights in the Software, and any and all patent rights owned or freely
+ * licensable by each licensor hereunder covering either (i) the unmodified
+ * Software as contributed to or provided by such licensor, or (ii) the Larger
+ * Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software (each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ * The above copyright notice and either this complete permission notice or at
+ * a minimum a reference to the UPL must be included in all copies or
+ * substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package io.cryostat.core.agent;
+
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class AgentXMLStream extends BufferedInputStream {
+
+    public AgentXMLStream(InputStream is) {
+        super(is);
+    }
+
+    @Override
+    public void close() {
+        // The XML Validator closes the stream when it finishes validation,
+        // we want to keep it open for further processing after validating.
+    }
+
+    public void trulyClose() throws IOException {
+        super.close();
+    }
+}

src/main/java/io/cryostat/core/agent/LocalProbeTemplateService.java

@@ -37,11 +37,8 @@
  */
 package io.cryostat.core.agent;
 
-import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -100,11 +97,7 @@ public void addTemplate(InputStream inputStream, String filename)
             if (fs.exists(path)) {
                 throw new FileAlreadyExistsException(template.getFileName());
             }
-            fs.writeString(
-                    path,
-                    new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))
-                            .lines()
-                            .collect(Collectors.joining("\n")));
+            fs.writeString(path, template.serialize());
         }
     }
 
@@ -138,10 +131,18 @@ public List<ProbeTemplate> getTemplates() throws FlightRecorderException {
         try {
             List<ProbeTemplate> templates = new ArrayList<>();
             for (Path path : getLocalTemplates()) {
-                try (InputStream stream = fs.newInputStream(path)) {
-                    ProbeTemplate template = new ProbeTemplate();
-                    template.deserialize(stream);
-                    templates.add(template);
+                if (path != null) {
+                    try (InputStream stream = fs.newInputStream(path)) {
+                        Path fileName = path.getFileName();
+                        if (fileName != null) {
+                            ProbeTemplate template = new ProbeTemplate();
+                            template.deserialize(stream);
+                            template.setFileName(fileName.toString());
+                            templates.add(template);
+                        }
+                    }
+                } else {
+                    throw new IOException("getLocalTemplates returned null path");
                 }
             }
             return templates;

src/main/java/io/cryostat/core/agent/ProbeTemplate.java

@@ -92,11 +92,11 @@ public ProbeTemplate() {
     }
 
     public void deserialize(InputStream xmlStream) throws IOException, SAXException {
-        BufferedInputStream stream = new BufferedInputStream(xmlStream);
-        xmlStream.mark(1); // arbitrary readLimit > 0
+        AgentXMLStream stream = new AgentXMLStream(xmlStream);
+        stream.mark(1); // arbitrary readLimit > 0
         ProbeValidator validator = new ProbeValidator();
         validator.validate(new StreamSource(stream));
-        xmlStream.reset();
+        stream.reset();
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
         DocumentBuilder builder;
 
@@ -106,10 +106,8 @@ public void deserialize(InputStream xmlStream) throws IOException, SAXException
             throw new IllegalStateException(e);
         }
 
-        if (xmlStream.markSupported()) {
-            xmlStream.reset();
-        }
-        Document document = builder.parse(xmlStream);
+        Document document = builder.parse(stream);
+        stream.close();
         NodeList elements;
 
         // parse global configurations
@@ -245,4 +243,8 @@ public Event[] getEvents() {
     public String getFileName() {
         return fileName;
     }
+
+    public void setFileName(String name) {
+        this.fileName = name;
+    }
 }

src/main/java/io/cryostat/core/net/JFRConnection.java

@@ -110,8 +110,18 @@ public class JFRConnection implements AutoCloseable {
         this(cw, fs, env, cd, List.of());
     }
 
-    public synchronized IConnectionHandle getHandle() {
-        return this.handle;
+    public synchronized IConnectionHandle getHandle() throws ConnectionException, IOException {
+        if (!isConnected()) {
+            connect();
+        }
+        IConnectionHandle handle = this.handle;
+        if (handle == null || !isConnected()) {
+            throw new ConnectionException(
+                    String.format(
+                            "Could not connect to remote target %s",
+                            this.connectionDescriptor.createJMXServiceURL().toString()));
+        }
+        return handle;
     }
 
     public synchronized IFlightRecorderService getService()