Skip to content

Commit

Permalink
fix(api): Fix location header for archived report generation (#775)
Browse files Browse the repository at this point in the history
  • Loading branch information
Josh-Matsuoka authored Jan 14, 2025
1 parent 630b100 commit f371a03
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 9 deletions.
10 changes: 4 additions & 6 deletions src/main/java/io/cryostat/reports/Reports.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,7 @@ public Response get(HttpServerResponse response, @RestPath String encodedKey) {
return Response.ok(request.getId(), MediaType.TEXT_PLAIN)
.status(202)
.location(
UriBuilder.fromUri(
String.format(
"/api/v4/targets/%d/reports/%d",
pair.getLeft(), pair.getRight()))
.build())
UriBuilder.fromUri(String.format("/api/v4/reports/%s", encodedKey)).build())
.build();
}

Expand All @@ -129,7 +125,9 @@ public Response getActive(

// Check if we've already cached a result for this report, return it if so
if (reportsService.keyExists(recording)) {
return Response.ok(reportsService.reportFor(recording).await().atMost(timeout))
return Response.ok(
reportsService.reportFor(recording).await().atMost(timeout),
MediaType.APPLICATION_JSON)
.status(200)
.build();
}
Expand Down
19 changes: 17 additions & 2 deletions src/test/java/io/cryostat/reports/ReportsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,17 @@ void testGetReportByTargetAndRemoteId() {
.assertThat()
.statusCode(202)
.contentType(ContentType.TEXT)
.body(Matchers.any(String.class));
.body(Matchers.any(String.class))
.assertThat()
// 202 Indicates report generation is in progress and sends an intermediate
// response.
// Verify we get a location header from a 202.
.header(
"Location",
"http://localhost:8081/api/v4/targets/"
+ targetId
+ "/reports/"
+ remoteId);

given().log()
.all()
Expand Down Expand Up @@ -165,7 +175,12 @@ void testGetReportByUrl() {
.assertThat()
.statusCode(202)
.contentType(ContentType.TEXT)
.body(Matchers.any(String.class));
.body(Matchers.any(String.class))
.assertThat()
// 202 Indicates report generation is in progress and sends an intermediate
// response.
// Verify we get a location header from a 202.
.header("Location", "http://localhost:8081" + reportUrl);

given().log()
.all()
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/itest/RecordingWorkflowTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ public void testWorkflow() throws Exception {
Matchers.matchesRegex(
TARGET_ALIAS + "_" + TEST_RECORDING_NAME + "_[\\d]{8}T[\\d]{6}Z.jfr"));
String savedDownloadUrl = recordingInfo.getString("downloadUrl");

Thread.sleep(3_000L); // wait for the dump to complete

// verify the in-memory recording list has not changed, except recording is now stopped
Expand Down Expand Up @@ -250,6 +249,7 @@ public void testWorkflow() throws Exception {
MatcherAssert.assertThat(
reportResponse.statusCode(),
Matchers.both(Matchers.greaterThanOrEqualTo(200)).and(Matchers.lessThan(300)));
MatcherAssert.assertThat(reportResponse.getHeader("Location"), Matchers.notNullValue());
MatcherAssert.assertThat(reportResponse.bodyAsString(), Matchers.notNullValue());

// Check that report generation concludes
Expand Down

0 comments on commit f371a03

Please sign in to comment.