From c3a45f7ab0de305efdcfe84af218806f9763d03a Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 19 Oct 2023 15:28:07 -0400 Subject: [PATCH] chore(recordings): replace subdirectory with jvmId in fromPathHandler (#1692) (#1731) (cherry picked from commit 70cf51f0cd1bb19c41b98e80c03b947cab49092a) Co-authored-by: Ming Yu Wang <90855268+mwangggg@users.noreply.github.com> --- .../beta/RecordingDeleteFromPathHandler.java | 9 ++- .../RecordingGetFromPathWithJwtHandler.java | 9 ++- ...dingMetadataLabelsPostFromPathHandler.java | 10 +++- .../RecordingUploadPostFromPathHandler.java | 9 ++- .../api/beta/ReportGetFromPathHandler.java | 9 ++- .../beta/ReportGetFromPathWithJwtHandler.java | 9 ++- .../recordings/RecordingArchiveHelper.java | 3 +- .../RecordingDeleteFromPathHandlerTest.java | 26 ++++----- ...ecordingGetFromPathWithJwtHandlerTest.java | 21 +++++-- ...MetadataLabelsPostFromPathHandlerTest.java | 19 ++++-- ...ecordingUploadPostFromPathHandlerTest.java | 58 ++++++++----------- .../beta/ReportGetFromPathHandlerTest.java | 53 ++++++----------- .../ReportGetFromPathWithJwtHandlerTest.java | 23 ++++++-- .../itest/FileSystemArchivedRequestsIT.java | 13 ++--- .../java/itest/TargetRecordingPatchIT.java | 3 +- 15 files changed, 147 insertions(+), 127 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingDeleteFromPathHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingDeleteFromPathHandler.java index 493d676750..2feac0c86f 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingDeleteFromPathHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingDeleteFromPathHandler.java @@ -31,6 +31,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.net.web.http.api.v2.IntermediateResponse; import io.cryostat.net.web.http.api.v2.RequestParameters; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingNotFoundException; import io.cryostat.rules.ArchivePathException; @@ -40,17 +41,20 @@ public class RecordingDeleteFromPathHandler extends AbstractV2RequestHandler { - static final String PATH = "fs/recordings/:subdirectoryName/:recordingName"; + static final String PATH = "fs/recordings/:jvmId/:recordingName"; private final RecordingArchiveHelper recordingArchiveHelper; + private final JvmIdHelper jvmIdHelper; @Inject RecordingDeleteFromPathHandler( AuthManager auth, CredentialsManager credentialsManager, Gson gson, + JvmIdHelper jvmIdHelper, RecordingArchiveHelper recordingArchiveHelper) { super(auth, credentialsManager, gson); + this.jvmIdHelper = jvmIdHelper; this.recordingArchiveHelper = recordingArchiveHelper; } @@ -91,9 +95,10 @@ public boolean isAsync() { @Override public IntermediateResponse handle(RequestParameters params) throws Exception { - String subdirectoryName = params.getPathParams().get("subdirectoryName"); String recordingName = params.getPathParams().get("recordingName"); + String jvmId = params.getPathParams().get("jvmId"); try { + String subdirectoryName = jvmIdHelper.jvmIdToSubdirectoryName(jvmId); recordingArchiveHelper.deleteRecordingFromPath(subdirectoryName, recordingName).get(); return new IntermediateResponse().body(null); } catch (ExecutionException e) { diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingGetFromPathWithJwtHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingGetFromPathWithJwtHandler.java index c8f34179a9..8639e1c27a 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingGetFromPathWithJwtHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingGetFromPathWithJwtHandler.java @@ -32,6 +32,7 @@ import io.cryostat.net.web.http.api.ApiVersion; import io.cryostat.net.web.http.api.v2.AbstractAssetJwtConsumingHandler; import io.cryostat.net.web.http.api.v2.ApiException; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingNotFoundException; import io.cryostat.rules.ArchivePathException; @@ -44,8 +45,9 @@ public class RecordingGetFromPathWithJwtHandler extends AbstractAssetJwtConsumingHandler { - static final String PATH = "fs/recordings/:subdirectoryName/:recordingName/jwt"; + static final String PATH = "fs/recordings/:jvmId/:recordingName/jwt"; + private final JvmIdHelper jvmIdHelper; private final RecordingArchiveHelper recordingArchiveHelper; @Inject @@ -54,9 +56,11 @@ public class RecordingGetFromPathWithJwtHandler extends AbstractAssetJwtConsumin CredentialsManager credentialsManager, AssetJwtHelper jwtFactory, Lazy webServer, + JvmIdHelper jvmIdHelper, RecordingArchiveHelper recordingArchiveHelper, Logger logger) { super(auth, credentialsManager, jwtFactory, webServer, logger); + this.jvmIdHelper = jvmIdHelper; this.recordingArchiveHelper = recordingArchiveHelper; } @@ -87,9 +91,10 @@ public boolean isAsync() { @Override public void handleWithValidJwt(RoutingContext ctx, JWT jwt) throws Exception { - String subdirectoryName = ctx.pathParam("subdirectoryName"); + String jvmId = ctx.pathParam("jvmId"); String recordingName = ctx.pathParam("recordingName"); try { + String subdirectoryName = jvmIdHelper.jvmIdToSubdirectoryName(jvmId); Path archivedRecording = recordingArchiveHelper .getRecordingPathFromPath(subdirectoryName, recordingName) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingMetadataLabelsPostFromPathHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingMetadataLabelsPostFromPathHandler.java index 0a50c1cbc2..c538c89451 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingMetadataLabelsPostFromPathHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingMetadataLabelsPostFromPathHandler.java @@ -30,6 +30,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.net.web.http.api.v2.IntermediateResponse; import io.cryostat.net.web.http.api.v2.RequestParameters; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingMetadataManager; import io.cryostat.recordings.RecordingMetadataManager.Metadata; @@ -41,21 +42,24 @@ public class RecordingMetadataLabelsPostFromPathHandler extends AbstractV2RequestHandler { - static final String PATH = "fs/recordings/:subdirectoryName/:recordingName/metadata/labels"; + static final String PATH = "fs/recordings/:jvmId/:recordingName/metadata/labels"; private final RecordingArchiveHelper recordingArchiveHelper; private final RecordingMetadataManager recordingMetadataManager; + private final JvmIdHelper jvmIdHelper; @Inject RecordingMetadataLabelsPostFromPathHandler( AuthManager auth, CredentialsManager credentialsManager, Gson gson, + JvmIdHelper jvmIdHelper, RecordingArchiveHelper recordingArchiveHelper, RecordingMetadataManager recordingMetadataManager) { super(auth, credentialsManager, gson); this.recordingArchiveHelper = recordingArchiveHelper; this.recordingMetadataManager = recordingMetadataManager; + this.jvmIdHelper = jvmIdHelper; } @Override @@ -96,9 +100,9 @@ public boolean requiresAuthentication() { @Override public IntermediateResponse handle(RequestParameters params) throws Exception { String recordingName = params.getPathParams().get("recordingName"); - String subdirectoryName = params.getPathParams().get("subdirectoryName"); - + String jvmId = params.getPathParams().get("jvmId"); try { + String subdirectoryName = jvmIdHelper.jvmIdToSubdirectoryName(jvmId); Metadata metadata = new Metadata(recordingMetadataManager.parseRecordingLabels(params.getBody())); diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingUploadPostFromPathHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingUploadPostFromPathHandler.java index bbfc0840d1..6e5a7b2150 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingUploadPostFromPathHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingUploadPostFromPathHandler.java @@ -41,6 +41,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.net.web.http.api.v2.IntermediateResponse; import io.cryostat.net.web.http.api.v2.RequestParameters; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingNotFoundException; import io.cryostat.rules.ArchivePathException; @@ -56,11 +57,12 @@ class RecordingUploadPostFromPathHandler extends AbstractV2RequestHandler { - static final String PATH = "fs/recordings/:subdirectoryName/:recordingName/upload"; + static final String PATH = "fs/recordings/:jvmId/:recordingName/upload"; private final Environment env; private final long httpTimeoutSeconds; private final WebClient webClient; + private final JvmIdHelper jvmIdHelper; private final RecordingArchiveHelper recordingArchiveHelper; @Inject @@ -70,12 +72,14 @@ class RecordingUploadPostFromPathHandler extends AbstractV2RequestHandler handle(RequestParameters params) throws Exception { - String subdirectoryName = params.getPathParams().get("subdirectoryName"); String recordingName = params.getPathParams().get("recordingName"); + String jvmId = params.getPathParams().get("jvmId"); try { + String subdirectoryName = jvmIdHelper.jvmIdToSubdirectoryName(jvmId); URL uploadUrl = new URL(env.getEnv(Variables.GRAFANA_DATASOURCE_ENV)); boolean isValidUploadUrl = new UrlValidator(UrlValidator.ALLOW_LOCAL_URLS).isValid(uploadUrl.toString()); diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathHandler.java index 95b6e26204..283ece7304 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathHandler.java @@ -38,6 +38,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.net.web.http.api.v2.IntermediateResponse; import io.cryostat.net.web.http.api.v2.RequestParameters; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingNotFoundException; import io.cryostat.rules.ArchivePathException; @@ -47,8 +48,9 @@ public class ReportGetFromPathHandler extends AbstractV2RequestHandler { - static final String PATH = "fs/reports/:subdirectoryName/:recordingName"; + static final String PATH = "fs/reports/:jvmId/:recordingName"; + private final JvmIdHelper jvmIdHelper; private final ReportService reportService; private final long reportGenerationTimeoutSeconds; @@ -57,10 +59,12 @@ public class ReportGetFromPathHandler extends AbstractV2RequestHandler { AuthManager auth, CredentialsManager credentialsManager, Gson gson, + JvmIdHelper jvmIdHelper, ReportService reportService, @Named(ReportsModule.REPORT_GENERATION_TIMEOUT_SECONDS) long reportGenerationTimeoutSeconds) { super(auth, credentialsManager, gson); + this.jvmIdHelper = jvmIdHelper; this.reportService = reportService; this.reportGenerationTimeoutSeconds = reportGenerationTimeoutSeconds; } @@ -105,9 +109,10 @@ public boolean isAsync() { @Override public IntermediateResponse handle(RequestParameters params) throws Exception { - String subdirectoryName = params.getPathParams().get("subdirectoryName"); + String jvmId = params.getPathParams().get("jvmId"); String recordingName = params.getPathParams().get("recordingName"); try { + String subdirectoryName = jvmIdHelper.jvmIdToSubdirectoryName(jvmId); List queriedFilter = params.getQueryParams().getAll("filter"); String rawFilter = queriedFilter.isEmpty() ? "" : queriedFilter.get(0); Path report = diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathWithJwtHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathWithJwtHandler.java index f13bd3b513..a6e8ca5a13 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathWithJwtHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathWithJwtHandler.java @@ -38,6 +38,7 @@ import io.cryostat.net.web.http.api.ApiVersion; import io.cryostat.net.web.http.api.v2.AbstractAssetJwtConsumingHandler; import io.cryostat.net.web.http.api.v2.ApiException; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingNotFoundException; import io.cryostat.rules.ArchivePathException; @@ -51,10 +52,11 @@ class ReportGetFromPathWithJwtHandler extends AbstractAssetJwtConsumingHandler { - static final String PATH = "fs/reports/:subdirectoryName/:recordingName/jwt"; + static final String PATH = "fs/reports/:jvmId/:recordingName/jwt"; private final ReportService reportService; private final long generationTimeoutSeconds; + private final JvmIdHelper jvmIdHelper; @Inject ReportGetFromPathWithJwtHandler( @@ -62,11 +64,13 @@ class ReportGetFromPathWithJwtHandler extends AbstractAssetJwtConsumingHandler { CredentialsManager credentialsManager, AssetJwtHelper jwtFactory, Lazy webServer, + JvmIdHelper jvmIdHelper, ReportService reportService, RecordingArchiveHelper recordingArchiveHelper, @Named(ReportsModule.REPORT_GENERATION_TIMEOUT_SECONDS) long generationTimeoutSeconds, Logger logger) { super(auth, credentialsManager, jwtFactory, webServer, logger); + this.jvmIdHelper = jvmIdHelper; this.reportService = reportService; this.generationTimeoutSeconds = generationTimeoutSeconds; } @@ -111,9 +115,10 @@ public boolean isOrdered() { @Override public void handleWithValidJwt(RoutingContext ctx, JWT jwt) throws Exception { - String subdirectoryName = ctx.pathParam("subdirectoryName"); + String jvmId = ctx.pathParam("jvmId"); String recordingName = ctx.pathParam("recordingName"); try { + String subdirectoryName = jvmIdHelper.jvmIdToSubdirectoryName(jvmId); List queriedFilter = ctx.queryParam("filter"); String rawFilter = queriedFilter.isEmpty() ? "" : queriedFilter.get(0); Path report = diff --git a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java index 606b3dc676..9f032db364 100644 --- a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java @@ -709,8 +709,7 @@ public Future> getRecordingsAndDirectories() { "beta/recordings", "beta/fs/recordings"), webServer - .getArchivedReportURL( - subdirectoryName, file) + .getArchivedReportURL(jvmId, file) .replace( "beta/reports", "beta/fs/reports"), diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingDeleteFromPathHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingDeleteFromPathHandlerTest.java index 14566da79a..0e18f7b481 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingDeleteFromPathHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingDeleteFromPathHandlerTest.java @@ -32,6 +32,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.net.web.http.api.v2.IntermediateResponse; import io.cryostat.net.web.http.api.v2.RequestParameters; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingNotFoundException; import io.cryostat.rules.ArchivedRecordingInfo; @@ -56,13 +57,14 @@ class RecordingDeleteFromPathHandlerTest { @Mock AuthManager auth; @Mock CredentialsManager credentialsManager; @Mock Gson gson; + @Mock JvmIdHelper jvmIdHelper; @Mock RecordingArchiveHelper recordingArchiveHelper; @BeforeEach void setup() { this.handler = new RecordingDeleteFromPathHandler( - auth, credentialsManager, gson, recordingArchiveHelper); + auth, credentialsManager, gson, jvmIdHelper, recordingArchiveHelper); } @Nested @@ -94,7 +96,7 @@ void shouldHaveExpectedRequiredPermissions() { void shouldHandleCorrectPath() { MatcherAssert.assertThat( handler.path(), - Matchers.equalTo("/api/beta/fs/recordings/:subdirectoryName/:recordingName")); + Matchers.equalTo("/api/beta/fs/recordings/:jvmId/:recordingName")); } @Test @@ -117,14 +119,12 @@ class Behaviour { @Test void shouldThrow404IfNoMatchingRecordingFound() throws Exception { String recordingName = "someRecording"; + String jvmId = "id"; String subdirectoryName = "someSubdirectory"; + + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); Future future = CompletableFuture.failedFuture( @@ -141,14 +141,12 @@ void shouldThrow404IfNoMatchingRecordingFound() throws Exception { @Test void shouldHandleSuccessfulDELETERequest() throws Exception { String recordingName = "someRecording"; + String jvmId = "id"; String subdirectoryName = "someSubdirectory"; + + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(params.getPathParams()) - .thenReturn( - Map.of( - "recordingName", - recordingName, - "subdirectoryName", - subdirectoryName)); + .thenReturn(Map.of("recordingName", recordingName, "jvmId", jvmId)); CompletableFuture future = Mockito.mock(CompletableFuture.class); when(recordingArchiveHelper.deleteRecordingFromPath( diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingGetFromPathWithJwtHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingGetFromPathWithJwtHandlerTest.java index cc1169ff7e..3bb4bfae10 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingGetFromPathWithJwtHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingGetFromPathWithJwtHandlerTest.java @@ -30,6 +30,7 @@ import io.cryostat.net.web.WebServer; import io.cryostat.net.web.http.api.ApiVersion; import io.cryostat.net.web.http.api.v2.ApiException; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingNotFoundException; @@ -58,6 +59,7 @@ class RecordingGetFromPathWithJwtHandlerTest { @Mock CredentialsManager credentialsManager; @Mock AssetJwtHelper jwt; @Mock WebServer webServer; + @Mock JvmIdHelper jvmIdHelper; @Mock RecordingArchiveHelper archive; @Mock Logger logger; @@ -65,7 +67,13 @@ class RecordingGetFromPathWithJwtHandlerTest { void setup() { this.handler = new RecordingGetFromPathWithJwtHandler( - auth, credentialsManager, jwt, () -> webServer, archive, logger); + auth, + credentialsManager, + jwt, + () -> webServer, + jvmIdHelper, + archive, + logger); } @Nested @@ -85,8 +93,7 @@ void shouldUseHttpGetVerb() { void shouldUseExpectedPath() { MatcherAssert.assertThat( handler.path(), - Matchers.equalTo( - "/api/beta/fs/recordings/:subdirectoryName/:recordingName/jwt")); + Matchers.equalTo("/api/beta/fs/recordings/:jvmId/:recordingName/jwt")); } @Test @@ -115,8 +122,10 @@ class Behaviour { @Test void shouldRespond404IfNotFound() throws Exception { - when(ctx.pathParam("subdirectoryName")).thenReturn("mysubdirectory"); + when(ctx.pathParam("jvmId")).thenReturn("id"); when(ctx.pathParam("recordingName")).thenReturn("myrecording"); + when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) + .thenReturn("mysubdirectory"); Future future = CompletableFuture.failedFuture( new RecordingNotFoundException("mysubdirectory", "myrecording")); @@ -132,8 +141,10 @@ void shouldRespond404IfNotFound() throws Exception { void shouldSendFileIfFound() throws Exception { HttpServerResponse resp = Mockito.mock(HttpServerResponse.class); when(ctx.response()).thenReturn(resp); - when(ctx.pathParam("subdirectoryName")).thenReturn("mysubdirectory"); + when(ctx.pathParam("jvmId")).thenReturn("id"); when(ctx.pathParam("recordingName")).thenReturn("myrecording"); + when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) + .thenReturn("mysubdirectory"); Path path = Mockito.mock(Path.class); when(path.toAbsolutePath()).thenReturn(path); when(path.toString()).thenReturn("foo.jfr"); diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingMetadataLabelsPostFromPathHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingMetadataLabelsPostFromPathHandlerTest.java index ef3de0607f..37b5f14638 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingMetadataLabelsPostFromPathHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingMetadataLabelsPostFromPathHandlerTest.java @@ -36,6 +36,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.net.web.http.api.v2.IntermediateResponse; import io.cryostat.net.web.http.api.v2.RequestParameters; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingMetadataManager; import io.cryostat.recordings.RecordingMetadataManager.Metadata; @@ -61,6 +62,7 @@ public class RecordingMetadataLabelsPostFromPathHandlerTest { @Mock AuthManager authManager; @Mock CredentialsManager credentialsManager; @Mock Gson gson; + @Mock JvmIdHelper jvmIdHelper; @Mock RecordingArchiveHelper recordingArchiveHelper; @Mock RecordingMetadataManager recordingMetadataManager; @Mock RequestParameters params; @@ -77,6 +79,7 @@ void setup() { authManager, credentialsManager, gson, + jvmIdHelper, recordingArchiveHelper, recordingMetadataManager); } @@ -104,7 +107,7 @@ void shouldHaveTargetsPath() { MatcherAssert.assertThat( handler.path(), Matchers.equalTo( - "/api/beta/fs/recordings/:subdirectoryName/:recordingName/metadata/labels")); + "/api/beta/fs/recordings/:jvmId/:recordingName/metadata/labels")); } @Test @@ -139,15 +142,17 @@ class Behaviour { @Test void shouldUpdateLabels() throws Exception { String recordingName = "someRecording"; - String subdirectoryName = "someTarget"; + String jvmId = "id"; Map labels = Map.of("key", "value"); Metadata metadata = new Metadata(labels); String requestLabels = labels.toString(); + String subdirectoryName = "someSubdirectory"; Map params = Mockito.mock(Map.class); + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(requestParameters.getPathParams()).thenReturn(params); when(params.get("recordingName")).thenReturn(recordingName); - when(params.get("subdirectoryName")).thenReturn(subdirectoryName); + when(params.get("jvmId")).thenReturn(jvmId); when(requestParameters.getBody()).thenReturn(requestLabels); when(recordingArchiveHelper.getRecordingPathFromPath(subdirectoryName, recordingName)) @@ -169,7 +174,7 @@ void shouldThrow400OnEmptyLabels() throws Exception { Map params = Mockito.mock(Map.class); when(requestParameters.getPathParams()).thenReturn(params); when(params.get("recordingName")).thenReturn("someRecording"); - when(params.get("subdirectoryName")).thenReturn("subdirectoryName"); + when(params.get("jvmId")).thenReturn("id"); when(requestParameters.getBody()).thenReturn("invalid"); Mockito.doThrow(new IllegalArgumentException()) .when(recordingMetadataManager) @@ -182,14 +187,16 @@ void shouldThrow400OnEmptyLabels() throws Exception { @Test void shouldThrowWhenRecordingNotFound() throws Exception { - String subdirectoryName = "someSubdirectory"; + String jvmId = "id"; String recordingName = "someNonExistentRecording"; + String subdirectoryName = "someSubdirectory"; String labels = Map.of("key", "value").toString(); Map params = Mockito.mock(Map.class); + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(requestParameters.getPathParams()).thenReturn(params); when(params.get("recordingName")).thenReturn(recordingName); - when(params.get("subdirectoryName")).thenReturn(subdirectoryName); + when(params.get("jvmId")).thenReturn(jvmId); when(requestParameters.getBody()).thenReturn(labels); when(recordingArchiveHelper.getRecordingPathFromPath(subdirectoryName, recordingName)) diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingUploadPostFromPathHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingUploadPostFromPathHandlerTest.java index fec828e1b6..bcdd5e231f 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingUploadPostFromPathHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingUploadPostFromPathHandlerTest.java @@ -34,6 +34,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.net.web.http.api.v2.IntermediateResponse; import io.cryostat.net.web.http.api.v2.RequestParameters; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingNotFoundException; @@ -70,11 +71,13 @@ class RecordingUploadPostFromPathHandlerTest { @Mock CredentialsManager credentialsManager; @Mock Environment env; @Mock WebClient webClient; + @Mock JvmIdHelper jvmIdHelper; @Mock RecordingArchiveHelper recordingArchiveHelper; @Mock Gson gson; static final String DATASOURCE_URL = "http://localhost:8080"; + static final String jvmId = "id"; static final String subdirectoryName = "foo"; static final String recordingName = "bar"; @@ -82,7 +85,14 @@ class RecordingUploadPostFromPathHandlerTest { void setup() { this.handler = new RecordingUploadPostFromPathHandler( - auth, credentialsManager, env, 30, webClient, recordingArchiveHelper, gson); + auth, + credentialsManager, + env, + 30, + webClient, + jvmIdHelper, + recordingArchiveHelper, + gson); } @Nested @@ -114,8 +124,7 @@ void shouldHaveExpectedRequiredPermissions() { void shouldHandleCorrectPath() { MatcherAssert.assertThat( handler.path(), - Matchers.equalTo( - "/api/beta/fs/recordings/:subdirectoryName/:recordingName/upload")); + Matchers.equalTo("/api/beta/fs/recordings/:jvmId/:recordingName/upload")); } @Test @@ -155,13 +164,9 @@ void shouldThrow501IfDatasourceUrlMalformed(String rawUrl) { @Test void shouldThrowExceptionIfRecordingNotFound() throws Exception { + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(env.getEnv("GRAFANA_DATASOURCE_URL")).thenReturn(DATASOURCE_URL); CompletableFuture future = Mockito.mock(CompletableFuture.class); @@ -170,8 +175,7 @@ void shouldThrowExceptionIfRecordingNotFound() throws Exception { .thenReturn(future); ExecutionException e = Mockito.mock(ExecutionException.class); when(future.get()).thenThrow(e); - when(e.getCause()) - .thenReturn(new RecordingNotFoundException(subdirectoryName, recordingName)); + when(e.getCause()).thenReturn(new RecordingNotFoundException("foo", recordingName)); ApiException ex = Assertions.assertThrows(ApiException.class, () -> handler.handle(params)); @@ -180,13 +184,9 @@ void shouldThrowExceptionIfRecordingNotFound() throws Exception { @Test void shouldDoUpload() throws Exception { + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn("foo"); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(env.getEnv("GRAFANA_DATASOURCE_URL")).thenReturn(DATASOURCE_URL); CompletableFuture future = Mockito.mock(CompletableFuture.class); @@ -234,13 +234,9 @@ public Void answer(InvocationOnMock args) throws Throwable { @Test void shouldHandleInvalidResponseStatusCode() throws Exception { + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn("someSubdirectory"); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(env.getEnv("GRAFANA_DATASOURCE_URL")).thenReturn(DATASOURCE_URL); CompletableFuture future = Mockito.mock(CompletableFuture.class); @@ -295,13 +291,9 @@ public Void answer(InvocationOnMock args) throws Throwable { @Test void shouldHandleNullStatusMessage() throws Exception { + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn("someSubdirectory"); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(env.getEnv("GRAFANA_DATASOURCE_URL")).thenReturn(DATASOURCE_URL); CompletableFuture future = Mockito.mock(CompletableFuture.class); @@ -356,13 +348,9 @@ public Void answer(InvocationOnMock args) throws Throwable { @Test void shouldHandleNullResponseBody() throws Exception { + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn("someSubdirectory"); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(env.getEnv("GRAFANA_DATASOURCE_URL")).thenReturn(DATASOURCE_URL); CompletableFuture future = Mockito.mock(CompletableFuture.class); diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathHandlerTest.java index 1741942fb4..6072b9a50b 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathHandlerTest.java @@ -33,6 +33,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.net.web.http.api.v2.IntermediateResponse; import io.cryostat.net.web.http.api.v2.RequestParameters; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingNotFoundException; import com.google.gson.Gson; @@ -56,13 +57,18 @@ class ReportGetFromPathHandlerTest { @Mock AuthManager authManager; @Mock CredentialsManager credentialsManager; @Mock Gson gson; + @Mock JvmIdHelper jvmIdHelper; @Mock ReportService reportService; + static final String jvmId = "id"; + static final String recordingName = "someRecording"; + static final String subdirectoryName = "someSubdirectory"; + @BeforeEach void setup() { this.handler = new ReportGetFromPathHandler( - authManager, credentialsManager, gson, reportService, 30); + authManager, credentialsManager, gson, jvmIdHelper, reportService, 30); } @Nested @@ -97,8 +103,7 @@ void shouldHaveExpectedRequiredPermissions() { @Test void shouldHandleCorrectPath() { MatcherAssert.assertThat( - handler.path(), - Matchers.equalTo("/api/beta/fs/reports/:subdirectoryName/:recordingName")); + handler.path(), Matchers.equalTo("/api/beta/fs/reports/:jvmId/:recordingName")); } @Test @@ -121,15 +126,9 @@ class Behaviour { @Test void shouldThrow404IfNoMatchingRecordingFound() throws Exception { MultiMap queryParams = MultiMap.caseInsensitiveMultiMap(); - String recordingName = "someRecording"; - String subdirectoryName = "someDirectory"; + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(params.getQueryParams()).thenReturn(queryParams); Future future = @@ -149,15 +148,9 @@ void shouldThrow404IfNoMatchingRecordingFound() throws Exception { @Test void shouldRespondBySendingFile() throws Exception { MultiMap queryParams = MultiMap.caseInsensitiveMultiMap(); - String recordingName = "someRecording"; - String subdirectoryName = "subdirectoryName"; + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(params.getQueryParams()).thenReturn(queryParams); Path fakePath = Mockito.mock(Path.class); @@ -178,15 +171,10 @@ void shouldRespondBySendingFile() throws Exception { void shouldRespondBySendingFileFiltered() throws Exception { MultiMap queryParams = MultiMap.caseInsensitiveMultiMap(); queryParams.add("filter", "someFilter"); - String recordingName = "someRecording"; - String subdirectoryName = "subdirectoryName"; + + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(params.getQueryParams()).thenReturn(queryParams); Path fakePath = Mockito.mock(Path.class); @@ -207,15 +195,10 @@ void shouldRespondBySendingFileFiltered() throws Exception { void shouldRespondBySendingFileUnformatted() throws Exception { MultiMap queryParams = MultiMap.caseInsensitiveMultiMap(); queryParams.add("filter", "someFilter"); - String recordingName = "someRecording"; - String subdirectoryName = "subdirectoryName"; + + when(jvmIdHelper.jvmIdToSubdirectoryName(jvmId)).thenReturn(subdirectoryName); when(params.getPathParams()) - .thenReturn( - Map.of( - "subdirectoryName", - subdirectoryName, - "recordingName", - recordingName)); + .thenReturn(Map.of("jvmId", jvmId, "recordingName", recordingName)); when(params.getQueryParams()).thenReturn(queryParams); Path fakePath = Mockito.mock(Path.class); diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathWithJwtHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathWithJwtHandlerTest.java index 394b40e6f6..8e5e1ccd13 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathWithJwtHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/ReportGetFromPathWithJwtHandlerTest.java @@ -34,6 +34,7 @@ import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.net.web.http.api.ApiVersion; import io.cryostat.net.web.http.api.v2.ApiException; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingNotFoundException; @@ -62,6 +63,7 @@ class ReportGetFromPathWithJwtHandlerTest { @Mock CredentialsManager credentialsManager; @Mock AssetJwtHelper jwt; @Mock WebServer webServer; + @Mock JvmIdHelper jvmIdHelper; @Mock ReportService reports; @Mock RecordingArchiveHelper archiveHelper; @Mock Logger logger; @@ -74,6 +76,7 @@ void setup() { credentialsManager, jwt, () -> webServer, + jvmIdHelper, reports, archiveHelper, 30, @@ -108,7 +111,7 @@ void shouldRequireResourceActions() { void shouldUseExpectedPath() { MatcherAssert.assertThat( handler.path(), - Matchers.equalTo("/api/beta/fs/reports/:subdirectoryName/:recordingName/jwt")); + Matchers.equalTo("/api/beta/fs/reports/:jvmId/:recordingName/jwt")); } @Test @@ -137,12 +140,14 @@ class Behaviour { @Test void shouldRespond404IfNotFound() throws Exception { - when(ctx.pathParam("subdirectoryName")).thenReturn("mydirectory"); + when(ctx.pathParam("jvmId")).thenReturn("id"); when(ctx.pathParam("recordingName")).thenReturn("myrecording"); + when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) + .thenReturn("mydirectory"); Future future = CompletableFuture.failedFuture( - new RecordingNotFoundException("mytarget", "myrecording")); + new RecordingNotFoundException("mydirectory", "myrecording")); when(reports.getFromPath(Mockito.anyString(), Mockito.anyString(), Mockito.anyString())) .thenReturn(future); ApiException ex = @@ -155,8 +160,10 @@ void shouldRespond404IfNotFound() throws Exception { void shouldSendFileIfFound() throws Exception { when(ctx.getAcceptableContentType()).thenReturn(HttpMimeType.JSON.mime()); when(ctx.response()).thenReturn(resp); - when(ctx.pathParam("subdirectoryName")).thenReturn("mydirectory"); + when(ctx.pathParam("jvmId")).thenReturn("id"); when(ctx.pathParam("recordingName")).thenReturn("myrecording"); + when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) + .thenReturn("mydirectory"); Path path = Mockito.mock(Path.class); when(path.toAbsolutePath()).thenReturn(path); when(path.toString()).thenReturn("foo.jfr"); @@ -177,8 +184,10 @@ void shouldSendFileIfFound() throws Exception { void shouldSendFileIfFoundFiltered() throws Exception { when(ctx.getAcceptableContentType()).thenReturn(HttpMimeType.JSON.mime()); when(ctx.response()).thenReturn(resp); - when(ctx.pathParam("subdirectoryName")).thenReturn("mydirectory"); + when(ctx.pathParam("jvmId")).thenReturn("id"); when(ctx.pathParam("recordingName")).thenReturn("myrecording"); + when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) + .thenReturn("mydirectory"); Path path = Mockito.mock(Path.class); when(path.toAbsolutePath()).thenReturn(path); when(path.toString()).thenReturn("foo.jfr"); @@ -200,8 +209,10 @@ void shouldSendFileIfFoundFiltered() throws Exception { void shouldSendFileIfFoundUnformatted() throws Exception { when(ctx.getAcceptableContentType()).thenReturn(HttpMimeType.JSON.mime()); when(ctx.response()).thenReturn(resp); - when(ctx.pathParam("subdirectoryName")).thenReturn("mydirectory"); + when(ctx.pathParam("jvmId")).thenReturn("id"); when(ctx.pathParam("recordingName")).thenReturn("myrecording"); + when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) + .thenReturn("mydirectory"); Path path = Mockito.mock(Path.class); when(path.toAbsolutePath()).thenReturn(path); when(path.toString()).thenReturn("foo.jfr"); diff --git a/src/test/java/itest/FileSystemArchivedRequestsIT.java b/src/test/java/itest/FileSystemArchivedRequestsIT.java index cfd01bbfc9..1dbe7c595f 100644 --- a/src/test/java/itest/FileSystemArchivedRequestsIT.java +++ b/src/test/java/itest/FileSystemArchivedRequestsIT.java @@ -18,7 +18,6 @@ import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.Map; @@ -39,7 +38,6 @@ import io.vertx.core.json.JsonObject; import itest.bases.JwtAssetsSelfTest; import itest.util.Podman; -import org.apache.commons.codec.binary.Base32; import org.apache.http.client.utils.URIBuilder; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; @@ -48,7 +46,6 @@ public class FileSystemArchivedRequestsIT extends JwtAssetsSelfTest { private static final Gson gson = MainModule.provideGson(Logger.INSTANCE); - private static final Base32 base32 = new Base32(); static final String TEST_RECORDING_NAME = "FileSystemArchivedRequestsIT"; @@ -57,7 +54,7 @@ void testGetRecordingsAndDirectories() throws Exception { URL resource = null; URL archivedResource = null; Path assetDownload = null; - String subdirectoryName = null; + String jvmId = null; try { JsonObject creationResponse = createRecording(); resource = new URL(creationResponse.getString("downloadUrl")); @@ -97,8 +94,7 @@ void testGetRecordingsAndDirectories() throws Exception { MatcherAssert.assertThat(labels, Matchers.equalTo(expectedLabels)); // post metadata fromPath - subdirectoryName = - base32.encodeAsString(dir.getString("jvmId").getBytes(StandardCharsets.UTF_8)); + jvmId = dir.getString("jvmId"); String recordingName = archivedRecording.getString("name"); Map uploadMetadata = Map.of("label", "test"); CompletableFuture metadataFuture = new CompletableFuture<>(); @@ -106,7 +102,7 @@ void testGetRecordingsAndDirectories() throws Exception { .post( String.format( "/api/beta/fs/recordings/%s/%s/metadata/labels", - subdirectoryName, recordingName)) + jvmId, recordingName)) .sendBuffer( Buffer.buffer(gson.toJson(uploadMetadata, Map.class)), ar -> { @@ -182,8 +178,7 @@ void testGetRecordingsAndDirectories() throws Exception { .getPath() .replaceFirst( "/api/v1/recordings", - String.format( - "/api/beta/fs/recordings/%s", subdirectoryName)); + String.format("/api/beta/fs/recordings/%s", jvmId)); cleanupCreatedResources(updatedArchivedPath); } if (assetDownload != null) { diff --git a/src/test/java/itest/TargetRecordingPatchIT.java b/src/test/java/itest/TargetRecordingPatchIT.java index f192bec6d1..8489a3d2e5 100644 --- a/src/test/java/itest/TargetRecordingPatchIT.java +++ b/src/test/java/itest/TargetRecordingPatchIT.java @@ -28,7 +28,6 @@ import itest.bases.StandardSelfTest; import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; public class TargetRecordingPatchIT extends StandardSelfTest { @@ -109,7 +108,7 @@ void testSaveEmptyRecordingDoesNotArchiveRecordingFile() throws Exception { } }); JsonArray listResp = listRespFuture1.get(REQUEST_TIMEOUT_SECONDS, TimeUnit.SECONDS); - Assertions.assertTrue(listResp.isEmpty()); + MatcherAssert.assertThat(listResp, Matchers.equalTo(new JsonArray())); } finally {