chore(http): remove Apache HttpClient5, replace with RESTEasy client (c…

…ryostatio#383)
mwangggg · Apr 24, 2024 · 19d6926 · 19d6926
1 parent 1342270
commit 19d6926
Show file tree

Hide file tree

Showing 12 changed files with 102 additions and 134 deletions.
diff --git a/compose/reports.yml b/compose/reports.yml
@@ -2,7 +2,7 @@ version: "3"
 services:
   cryostat:
     environment:
-      - CRYOSTAT_SERVICES_REPORTS_URL=http://reports:10001
+      - QUARKUS_REST_CLIENT_REPORTS_URL=http://reports:10001
   reports:
     image: ${CRYOSTAT_REPORTS_IMAGE:-quay.io/cryostat/cryostat-reports:latest}
     hostname: reports

diff --git a/pom.xml b/pom.xml
@@ -171,11 +171,6 @@
       <artifactId>commons-collections4</artifactId>
       <version>${org.apache.commons.collections.version}</version>
     </dependency>
-    <dependency>
-      <groupId>org.apache.httpcomponents.client5</groupId>
-      <artifactId>httpclient5</artifactId>
-      <version>${org.apache.httpcomponents.version}</version>
-    </dependency>
     <dependency>
       <groupId>org.projectnessie.cel</groupId>
       <artifactId>cel-tools</artifactId>

diff --git a/src/main/java/io/cryostat/ConfigProperties.java b/src/main/java/io/cryostat/ConfigProperties.java
@@ -33,7 +33,7 @@ public class ConfigProperties {
     public static final String CONNECTIONS_FAILED_BACKOFF = "cryostat.connections.failed-backoff";
     public static final String CONNECTIONS_FAILED_TIMEOUT = "cryostat.connections.failed-timeout";
 
-    public static final String REPORTS_SIDECAR_URL = "cryostat.services.reports.url";
+    public static final String REPORTS_SIDECAR_URL = "quarkus.rest-client.reports.url";
     public static final String REPORTS_MEMORY_CACHE_ENABLED =
             "cryostat.services.reports.memory-cache.enabled";
     public static final String REPORTS_STORAGE_CACHE_ENABLED =

diff --git a/src/main/java/io/cryostat/events/S3TemplateService.java b/src/main/java/io/cryostat/events/S3TemplateService.java
@@ -54,9 +54,9 @@
 import jakarta.enterprise.event.Observes;
 import jakarta.inject.Inject;
 import jakarta.inject.Named;
+import jakarta.ws.rs.core.MediaType;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang3.tuple.Pair;
-import org.apache.hc.core5.http.ContentType;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
 import org.jboss.logging.Logger;
 import org.jsoup.Jsoup;
@@ -239,7 +239,7 @@ public Template addTemplate(InputStream stream)
                     PutObjectRequest.builder()
                             .bucket(bucket)
                             .key(templateName)
-                            .contentType(ContentType.APPLICATION_XML.getMimeType())
+                            .contentType(MediaType.APPLICATION_XML)
                             .tagging(createTemplateTagging(templateName, description, provider))
                             .build(),
                     RequestBody.fromString(model.toString()));

diff --git a/src/main/java/io/cryostat/graphql/GraphQL.java b/src/main/java/io/cryostat/graphql/GraphQL.java
@@ -23,7 +23,6 @@
 import jakarta.ws.rs.Path;
 import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.UriInfo;
-import org.apache.hc.core5.net.URIBuilder;
 import org.jboss.resteasy.reactive.RestResponse;
 
 @Path("")
@@ -33,22 +32,11 @@ public class GraphQL {
     @Path("/api/v2.2/graphql")
     @RolesAllowed("write")
     public Response redirectGet(UriInfo info) throws Exception {
-        var uriBuilder = new URIBuilder();
-        info.getQueryParameters()
-                .entrySet()
-                .forEach(
-                        entry -> {
-                            if (entry.getValue().size() != 1) {
-                                return;
-                            }
-                            uriBuilder.addParameter(entry.getKey(), entry.getValue().get(0));
-                        });
         return Response.status(RestResponse.Status.PERMANENT_REDIRECT)
                 .location(
                         URI.create(
                                 String.format(
-                                        "/api/v3/graphql?%s",
-                                        String.join("&", uriBuilder.build().getRawQuery()))))
+                                        "/api/v3/graphql?%s", info.getRequestUri().getRawQuery())))
                 .build();
     }
 

diff --git a/src/main/java/io/cryostat/jmcagent/S3ProbeTemplateService.java b/src/main/java/io/cryostat/jmcagent/S3ProbeTemplateService.java
@@ -36,9 +36,9 @@
 import jakarta.enterprise.event.Observes;
 import jakarta.inject.Inject;
 import jakarta.inject.Named;
+import jakarta.ws.rs.core.MediaType;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang3.tuple.Pair;
-import org.apache.hc.core5.http.ContentType;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
 import org.jboss.logging.Logger;
 import org.xml.sax.SAXException;
@@ -191,7 +191,7 @@ public ProbeTemplate addTemplate(InputStream stream, String fileName)
                     PutObjectRequest.builder()
                             .bucket(bucket)
                             .key(fileName)
-                            .contentType(ContentType.APPLICATION_XML.getMimeType())
+                            .contentType(MediaType.APPLICATION_XML)
                             .tagging(
                                     createTemplateTagging(
                                             fileName,

diff --git a/src/main/java/io/cryostat/recordings/RecordingHelper.java b/src/main/java/io/cryostat/recordings/RecordingHelper.java
@@ -91,12 +91,12 @@
 import jakarta.ws.rs.BadRequestException;
 import jakarta.ws.rs.NotFoundException;
 import jakarta.ws.rs.ServerErrorException;
+import jakarta.ws.rs.core.Response;
 import jdk.jfr.RecordingState;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.commons.validator.routines.UrlValidator;
-import org.apache.hc.core5.http.HttpStatus;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
 import org.jboss.logging.Logger;
 import org.quartz.Job;
@@ -178,7 +178,7 @@ void onStart(@Observes StartupEvent evt) {
         if (grafanaDatasourceURLProperty.isEmpty()) {
             grafanaDatasourceURL.completeExceptionally(
                     new HttpException(
-                            HttpStatus.SC_BAD_GATEWAY,
+                            Response.Status.BAD_GATEWAY.getStatusCode(),
                             String.format(
                                     "Configuration property %s is not set",
                                     ConfigProperties.GRAFANA_DATASOURCE_URL)));
@@ -192,7 +192,7 @@ void onStart(@Observes StartupEvent evt) {
             if (!isValidUploadUrl) {
                 grafanaDatasourceURL.completeExceptionally(
                         new HttpException(
-                                HttpStatus.SC_BAD_GATEWAY,
+                                Response.Status.BAD_GATEWAY.getStatusCode(),
                                 String.format(
                                         "Configuration property %s=%s is not acceptable",
                                         ConfigProperties.GRAFANA_DATASOURCE_URL,
@@ -203,7 +203,7 @@ void onStart(@Observes StartupEvent evt) {
         } catch (MalformedURLException e) {
             grafanaDatasourceURL.completeExceptionally(
                     new HttpException(
-                            HttpStatus.SC_BAD_GATEWAY,
+                            Response.Status.BAD_GATEWAY.getStatusCode(),
                             String.format(
                                     "Configuration property %s=%s is not a valid URL",
                                     ConfigProperties.GRAFANA_DATASOURCE_URL,

diff --git a/src/main/java/io/cryostat/reports/ReportSidecarService.java b/src/main/java/io/cryostat/reports/ReportSidecarService.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright The Cryostat Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.cryostat.reports;
+
+import java.io.InputStream;
+import java.util.Map;
+
+import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult;
+
+import io.smallrye.mutiny.Uni;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.core.MediaType;
+import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
+import org.jboss.resteasy.reactive.PartType;
+import org.jboss.resteasy.reactive.RestForm;
+
+@Path("/report")
+@RegisterRestClient(configKey = "reports")
+@ApplicationScoped
+public interface ReportSidecarService {
+    @POST
+    @Consumes(MediaType.MULTIPART_FORM_DATA)
+    Uni<Map<String, AnalysisResult>> generate(
+            @RestForm("file") @PartType(MediaType.APPLICATION_OCTET_STREAM) InputStream file);
+}
diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java
@@ -17,11 +17,7 @@
 
 import java.io.BufferedInputStream;
 import java.io.InputStream;
-import java.net.URI;
 import java.util.Map;
-import java.util.Optional;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.Future;
 import java.util.function.Predicate;
 
 import org.openjdk.jmc.flightrecorder.rules.IRule;
@@ -31,88 +27,67 @@
 import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult;
 import io.cryostat.recordings.ActiveRecording;
 import io.cryostat.recordings.RecordingHelper;
-import io.cryostat.util.HttpStatusCodeIdentifier;
 
-import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.smallrye.mutiny.Uni;
-import io.vertx.ext.web.handler.HttpException;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
-import org.apache.hc.client5.http.classic.methods.HttpPost;
-import org.apache.hc.client5.http.entity.mime.MultipartEntityBuilder;
-import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
+import org.eclipse.microprofile.rest.client.inject.RestClient;
 import org.jboss.logging.Logger;
 
 @ApplicationScoped
 class ReportsServiceImpl implements ReportsService {
 
+    private static final String NO_SIDECAR_URL = "http://localhost/";
+
     @ConfigProperty(name = ConfigProperties.REPORTS_SIDECAR_URL)
-    Optional<URI> sidecarUri;
+    String sidecarUri;
 
     @Inject ObjectMapper mapper;
     @Inject RecordingHelper helper;
     @Inject InterruptibleReportGenerator reportGenerator;
+    @Inject @RestClient ReportSidecarService sidecar;
     @Inject Logger logger;
 
     @Override
     public Uni<Map<String, AnalysisResult>> reportFor(
             ActiveRecording recording, Predicate<IRule> predicate) {
-        Future<Map<String, AnalysisResult>> future =
-                sidecarUri
-                        .map(
-                                uri -> {
-                                    logger.tracev(
-                                            "sidecar reportFor active recording {0} {1}",
-                                            recording.target.jvmId, recording.remoteId);
-                                    try {
-                                        return fireRequest(
-                                                uri, helper.getActiveInputStream(recording));
-                                    } catch (Exception e) {
-                                        throw new ReportGenerationException(e);
-                                    }
-                                })
-                        .orElseGet(
-                                () -> {
-                                    logger.tracev(
-                                            "inprocess reportFor active recording {0} {1}",
-                                            recording.target.jvmId, recording.remoteId);
-                                    try {
-                                        return process(
-                                                helper.getActiveInputStream(recording), predicate);
-                                    } catch (Exception e) {
-                                        throw new ReportGenerationException(e);
-                                    }
-                                });
-        return Uni.createFrom().future(future);
+        InputStream stream;
+        try {
+            stream = helper.getActiveInputStream(recording);
+        } catch (Exception e) {
+            throw new ReportGenerationException(e);
+        }
+        if (NO_SIDECAR_URL.equals(sidecarUri)) {
+            logger.tracev(
+                    "inprocess reportFor active recording {0} {1}",
+                    recording.target.jvmId, recording.remoteId);
+            return process(stream, predicate);
+        } else {
+            logger.tracev(
+                    "sidecar reportFor active recording {0} {1}",
+                    recording.target.jvmId, recording.remoteId);
+            return fireRequest(stream);
+        }
     }
 
     @Override
     public Uni<Map<String, AnalysisResult>> reportFor(
             String jvmId, String filename, Predicate<IRule> predicate) {
-        Future<Map<String, AnalysisResult>> future =
-                sidecarUri
-                        .map(
-                                uri -> {
-                                    logger.tracev(
-                                            "sidecar reportFor archived recording {0} {1}",
-                                            jvmId, filename);
-                                    return fireRequest(
-                                            uri,
-                                            helper.getArchivedRecordingStream(jvmId, filename));
-                                })
-                        .orElseGet(
-                                () -> {
-                                    logger.tracev(
-                                            "inprocess reportFor archived recording {0} {1}",
-                                            jvmId, filename);
-                                    return process(
-                                            helper.getArchivedRecordingStream(jvmId, filename),
-                                            predicate);
-                                });
-
-        return Uni.createFrom().future(future);
+        InputStream stream;
+        try {
+            stream = helper.getArchivedRecordingStream(jvmId, filename);
+        } catch (Exception e) {
+            throw new ReportGenerationException(e);
+        }
+        if (NO_SIDECAR_URL.equals(sidecarUri)) {
+            logger.tracev("inprocess reportFor archived recording {0} {1}", jvmId, filename);
+            return process(stream, predicate);
+        } else {
+            logger.tracev("sidecar reportFor archived recording {0} {1}", jvmId, filename);
+            return fireRequest(stream);
+        }
     }
 
     @Override
@@ -125,40 +100,16 @@ public Uni<Map<String, AnalysisResult>> reportFor(String jvmId, String filename)
         return reportFor(jvmId, filename, r -> true);
     }
 
-    private Future<Map<String, AnalysisResult>> process(
+    private Uni<Map<String, AnalysisResult>> process(
             InputStream stream, Predicate<IRule> predicate) {
-        return reportGenerator.generateEvalMapInterruptibly(
-                new BufferedInputStream(stream), predicate);
+        return Uni.createFrom()
+                .future(
+                        reportGenerator.generateEvalMapInterruptibly(
+                                new BufferedInputStream(stream), predicate));
     }
 
-    private Future<Map<String, AnalysisResult>> fireRequest(URI uri, InputStream stream) {
-        var cf = new CompletableFuture<Map<String, AnalysisResult>>();
-        try (var http = HttpClients.createDefault();
-                stream) {
-            var post = new HttpPost(uri.resolve("report"));
-            var form = MultipartEntityBuilder.create().addBinaryBody("file", stream).build();
-            post.setEntity(form);
-            http.execute(
-                    post,
-                    response -> {
-                        if (!HttpStatusCodeIdentifier.isSuccessCode(response.getCode())) {
-                            cf.completeExceptionally(
-                                    new HttpException(
-                                            response.getCode(), response.getReasonPhrase()));
-                            return null;
-                        }
-                        var entity = response.getEntity();
-                        Map<String, AnalysisResult> evaluation =
-                                mapper.readValue(
-                                        entity.getContent(),
-                                        new TypeReference<Map<String, AnalysisResult>>() {});
-                        cf.complete(evaluation);
-                        return null;
-                    });
-        } catch (Exception e) {
-            cf.completeExceptionally(new ReportGenerationException(e));
-        }
-        return cf;
+    private Uni<Map<String, AnalysisResult>> fireRequest(InputStream stream) {
+        return sidecar.generate(stream);
     }
 
     public static class ReportGenerationException extends RuntimeException {