fix(jmc-agent): fix template validation error handling and notification content (#1563)

* chore(agent): update agent template utilities

Signed-off-by: Thuan Vo <[email protected]>

* deps(core): bump core version

---------

Signed-off-by: Thuan Vo <[email protected]>

Author: tthvo

pom.xml

@@ -83,7 +83,7 @@
   <com.google.dagger.version>2.45</com.google.dagger.version>
   <com.google.dagger.compiler.version>2.45</com.google.dagger.compiler.version>
 
-  <io.cryostat.core.version>2.21.0</io.cryostat.core.version>
+  <io.cryostat.core.version>2.21.1</io.cryostat.core.version>
 
   <org.openjdk.nashorn.core.version>15.4</org.openjdk.nashorn.core.version>
   <org.apache.commons.lang3.version>3.12.0</org.apache.commons.lang3.version>

src/main/java/io/cryostat/net/web/http/api/v2/ProbeTemplateUploadHandler.java

@@ -38,6 +38,7 @@
 package io.cryostat.net.web.http.api.v2;
 
 import java.io.InputStream;
+import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Path;
 import java.util.EnumSet;
 import java.util.List;
@@ -48,6 +49,7 @@
 
 import io.cryostat.configuration.CredentialsManager;
 import io.cryostat.core.agent.LocalProbeTemplateService;
+import io.cryostat.core.agent.ProbeTemplate;
 import io.cryostat.core.agent.ProbeValidationException;
 import io.cryostat.core.log.Logger;
 import io.cryostat.core.sys.FileSystem;
@@ -128,25 +130,22 @@ public IntermediateResponse<Void> handle(RequestParameters requestParams) throws
             for (FileUpload u : requestParams.getFileUploads()) {
                 String templateName = requestParams.getPathParams().get("probetemplateName");
                 Path path = fs.pathOf(u.uploadedFileName());
-                if (!"probeTemplate".equals(u.name())) {
+                if (!u.name().equals("probeTemplate")) {
                     fs.deleteIfExists(path);
                     continue;
                 }
                 try (InputStream is = fs.newInputStream(path)) {
-                    probeTemplateService.addTemplate(is, templateName);
-                    String template = probeTemplateService.getTemplate(templateName);
+                    ProbeTemplate template = probeTemplateService.addTemplate(is, templateName);
                     notificationFactory
                             .createBuilder()
                             .metaCategory(NOTIFICATION_CATEGORY)
                             .metaType(HttpMimeType.JSON)
                             .message(
                                     Map.of(
                                             "probeTemplate",
-                                            u.uploadedFileName(),
-                                            "templateName",
                                             templateName,
                                             "templateContent",
-                                            template))
+                                            template.serialize()))
                             .build()
                             .send();
                 } finally {
@@ -156,6 +155,9 @@ public IntermediateResponse<Void> handle(RequestParameters requestParams) throws
         } catch (ProbeValidationException pve) {
             logger.error(pve.getMessage());
             throw new ApiException(400, pve.getMessage(), pve);
+        } catch (FileAlreadyExistsException faee) {
+            logger.error(faee.getMessage());
+            throw new ApiException(400, faee.getMessage(), faee);
         } catch (Exception e) {
             logger.error(e.getMessage());
             throw new ApiException(500, e.getMessage(), e);

src/main/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandler.java

@@ -166,7 +166,7 @@ public IntermediateResponse<Void> handle(RequestParameters requestParams) throws
                 getConnectionDescriptorFromParams(requestParams),
                 connection -> {
                     AgentJMXHelper helper = new AgentJMXHelper(connection.getHandle());
-                    String templateContent = probeTemplateService.getTemplate(probeTemplate);
+                    String templateContent = probeTemplateService.getTemplateContent(probeTemplate);
                     helper.defineEventProbes(templateContent);
                     ProbeTemplate template = new ProbeTemplate();
                     template.deserialize(

src/test/java/io/cryostat/net/web/http/api/v2/ProbeTemplateUploadHandlerTest.java

@@ -41,6 +41,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Path;
 import java.util.List;
 import java.util.Map;
@@ -49,6 +50,7 @@
 import io.cryostat.MainModule;
 import io.cryostat.configuration.CredentialsManager;
 import io.cryostat.core.agent.LocalProbeTemplateService;
+import io.cryostat.core.agent.ProbeTemplate;
 import io.cryostat.core.agent.ProbeValidationException;
 import io.cryostat.core.log.Logger;
 import io.cryostat.core.sys.FileSystem;
@@ -202,7 +204,7 @@ void shouldRespond400IfXmlInvalid() throws Exception {
         }
 
         @Test
-        void shouldProcessGoodRequest() throws Exception {
+        void shouldRespond400IfFileExists() throws Exception {
             FileUpload upload = Mockito.mock(FileUpload.class);
             Mockito.when(upload.name()).thenReturn("probeTemplate");
             Mockito.when(upload.uploadedFileName()).thenReturn("/file-uploads/abcd-1234");
@@ -214,23 +216,52 @@ void shouldProcessGoodRequest() throws Exception {
             Path uploadPath = Mockito.mock(Path.class);
             Mockito.when(fs.pathOf("/file-uploads/abcd-1234")).thenReturn(uploadPath);
 
+            InputStream stream = Mockito.mock(InputStream.class);
+            Mockito.when(fs.newInputStream(Mockito.any())).thenReturn(stream);
+
+            Mockito.doThrow(FileAlreadyExistsException.class)
+                    .when(templateService)
+                    .addTemplate(stream, "foo.xml");
+
+            ApiException ex =
+                    Assertions.assertThrows(
+                            ApiException.class, () -> handler.handle(requestParams));
+            MatcherAssert.assertThat(ex.getStatusCode(), Matchers.equalTo(400));
+            Mockito.verify(fs).deleteIfExists(uploadPath);
+        }
+
+        @Test
+        void shouldProcessGoodRequest() throws Exception {
+            String templateName = "foo.xml";
+            FileUpload upload = Mockito.mock(FileUpload.class);
+            Mockito.when(upload.name()).thenReturn("probeTemplate");
+            Mockito.when(upload.uploadedFileName()).thenReturn("/file-uploads/abcd-1234");
+
+            Mockito.when(requestParams.getFileUploads()).thenReturn(Set.of(upload));
+            Mockito.when(requestParams.getPathParams())
+                    .thenReturn(Map.of("probetemplateName", templateName));
+
+            Path uploadPath = Mockito.mock(Path.class);
+            Mockito.when(fs.pathOf("/file-uploads/abcd-1234")).thenReturn(uploadPath);
+
             InputStream stream = Mockito.mock(InputStream.class);
             Mockito.when(fs.newInputStream(uploadPath)).thenReturn(stream);
-            Mockito.when(templateService.getTemplate(Mockito.anyString()))
-                    .thenReturn("someContent");
+
+            ProbeTemplate template = Mockito.mock(ProbeTemplate.class);
+            String templateContent = "someContent";
+            Mockito.when(templateService.addTemplate(stream, templateName)).thenReturn(template);
+            Mockito.when(template.serialize()).thenReturn(templateContent);
 
             IntermediateResponse<Void> response = handler.handle(requestParams);
 
-            Mockito.verify(templateService).addTemplate(stream, "foo.xml");
+            Mockito.verify(templateService).addTemplate(stream, templateName);
             Mockito.verify(notificationBuilder)
                     .message(
                             Map.of(
                                     "probeTemplate",
-                                    "/file-uploads/abcd-1234",
-                                    "templateName",
-                                    "foo.xml",
+                                    templateName,
                                     "templateContent",
-                                    "someContent"));
+                                    templateContent));
             Mockito.verifyNoMoreInteractions(templateService);
             Mockito.verify(fs).deleteIfExists(uploadPath);
 

src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java

@@ -190,7 +190,7 @@ public void shouldRespondOK() throws Exception {
                         + " <name>handleAuthenticated</name>"
                         + " <descriptor>(Lio/vertx/ext/web/RoutingContext;)V</descriptor> </method>"
                         + " </event> </events> </jfragent>";
-            Mockito.when(templateService.getTemplate(Mockito.anyString()))
+            Mockito.when(templateService.getTemplateContent(Mockito.anyString()))
                     .thenReturn(templateContent);
             Mockito.when(connection.getHandle()).thenReturn(handle);
             Mockito.when(handle.getServiceOrDummy(MBeanServerConnection.class)).thenReturn(mbsc);