fix: use the correct ClassLoader

When looking for HttpClient classes (e.g.
org.apache.http.client.utils.URIBuilder), use the current thread's class
loader, rather than the default.

Author: apotterri

agent/bin/test_run

@@ -3,8 +3,21 @@ set -e
 
 export PATH="$BATS_DIR"/bin:"$BASHUNIT_DIR":$PATH
 
+shopt -s extglob
 set -x
-# bats has a mechanism to discover test files recursively, but it doesn't report errors properly.
-bats $(find "${@-test}" -name \*.bats)
+
+# Use -r to discover tests. It comes with a couple of caveats:
+#
+#   * it has problems reporting test scripts errors, i.e. problems in the scripts themselves, not
+#     with commands they run. If you see failures that don't have sensible errors messages, try
+#     something like
+#
+#     $ bats $(find test/!(http_client) -name \*.bats)
+#
+#   * just doing bats -r test doesn't discover a setup_suite.bash file correctly. http_client uses
+#     one, so it needs to be run separately
+
+bats -r test/!(http_client)
+bats -r test/http_client
 
 bashunit 'test/**/*_test.sh'

agent/src/main/java/com/appland/appmap/process/hooks/http/HttpClientRequest.java

@@ -80,15 +80,16 @@ private static void execute(Event event, String method, URIBuilder builder) {
 
   static private URIBuilder newBuilder(URI uri) {
     try {
-      Class<?> cls = Class.forName("org.apache.http.client.utils.URIBuilder");
+      Class<?> cls = Class.forName("org.apache.http.client.utils.URIBuilder", true,
+          Thread.currentThread().getContextClassLoader());
       Constructor<?> ctor = cls.getConstructor(java.net.URI.class);
       return new URIBuilder(ctor.newInstance(uri));
     } catch (ClassNotFoundException | NoSuchMethodException | SecurityException | InstantiationException
         | IllegalAccessException
         | IllegalArgumentException | InvocationTargetException e) {
       logger.error(e, "failed creating a URIBuilder");
       // This shouldn't ever happen: we've hooked a method in
-      // org.apachage.http.client, so URIBuilder should always be available.
+      // org.apache.http.client, so URIBuilder should always be available.
       throw new InternalError(e);
     }
   }

agent/test/http_client/appmap.yml

@@ -1,3 +1,3 @@
 name: HTTP client test
 packages:
-  - path: http_client
+  - path: httpclient

agent/test/http_client/http_client.bats

@@ -1,57 +1 @@
-#!/usr/bin/env bats
-
-load '../../build/bats/bats-support/load'
-load '../../build/bats/bats-assert/load'
-load '../helper'
-load '../petclinic-shared/shared-setup.bash'
-
-
-setup_file() {
-  export FIXTURE_DIR="build/fixtures/spring-petclinic"
-  _shared_setup
-  start_petclinic >&3
-
-  cd test/http_client
-}
-
-teardown_file() {
-  stop_ws
-}
-
-@test "request without query" {
-  run ./gradlew -q -PmainClass=http_client.HttpClientTest run ${DEBUG} --args "${WS_URL}/vets"
-
-  assert_json_eq '.events[1].http_client_request.request_method' "GET"
-  assert_json_eq '.events[1].http_client_request.url' "${WS_URL}/vets"
-
-  # Neither message nor parameters should set set
-  assert_json_eq '.events[1].message' ''
-  assert_json_eq '.events[1].parameters' ''
-}
-
-@test "request with query" {
-  run ./gradlew -q -PmainClass=http_client.HttpClientTest run ${DEBUG} --args "${WS_URL}/owners?lastName=davis"
-
-  assert_json_eq '.events[1].http_client_request.url' "${WS_URL}/owners"
-  assert_json_eq '.events[1].message | length' 1
-  assert_json_eq '.events[1].message[0] | "\(.name) \(.value)"' "lastName davis"
-
-  assert_json_eq '.events[2].http_client_response.status' "200"
-}
-
-@test "request without Content-Type" {
-  run ./gradlew -q -PmainClass=http_client.HttpClientTest run ${DEBUG} --args "${WS_URL}/no-content"
-
-  assert_json_eq '.events[1].http_client_request.url' "${WS_URL}/no-content"
-  assert_json_eq '.events[2].http_client_response.status' "200"
-}
-
-@test "request with HttpHost" {
-  run ./gradlew -q -PmainClass=http_client.HttpHostTest run ${DEBUG} --args "${WS_HOST} ${WS_PORT} /owners?lastName=davis"
-
-  assert_json_eq '.events[1].http_client_request.url' "${WS_URL}/owners"
-  assert_json_eq '.events[1].message | length' 1
-  assert_json_eq '.events[1].message[0] | "\(.name) \(.value)"' "lastName davis"
-
-  assert_json_eq '.events[2].http_client_response.status' "200"
-}
+# intentionally empty, bats requires a test file to enable detection of setup_suite.bash

agent/test/http_client/build.gradle renamed to agent/test/http_client/httpclient/build.gradle

@@ -1,3 +1,4 @@
+
 plugins {
   id 'application'
   id 'java'
@@ -16,13 +17,13 @@ sourceSets {
 }
 
 // TODO: look at consolidating this with the gradle config in httpcore
-def appmapJar = fileTree('../../build/libs').matching {
+def appmapJar = fileTree('../../../build/libs').matching {
       include {
         it.file.name ==~ /.*appmap-[0-9.]*(-SNAPSHOT)*.jar$/
       }
     }.getFiles()[0]
 
-def annotationJar = fileTree('../../../annotation/build/libs').matching {
+def annotationJar = fileTree('../../../../annotation/build/libs').matching {
       include {
         it.file.name ==~ /.*annotation-[0-9.]*(-SNAPSHOT)*.jar$/
       }
@@ -47,7 +48,6 @@ application {
     System.env.JAVA_OUTPUT_OPTIONS,
     "-javaagent:${appmapJar}",
     "-Djava.util.logging.config.file=${System.env.JUL_CONFIG}",
-    "-Dappmap.config.file=appmap.yml",
     //"-Dappmap.debug=true",
     //"-Dappmap.debug.file=../../build/log/httpclient-appmap.log"
   ]

agent/test/http_client/httpclient/gradle/wrapper/gradle-wrapper.jar

@@ -0,0 +1,6 @@
+distributionBase=GRADLE_USER_HOME
+distributionPath=wrapper/dists
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
+networkTimeout=10000
+zipStoreBase=GRADLE_USER_HOME
+zipStorePath=wrapper/dists

agent/test/http_client/httpclient/gradle/wrapper/gradle-wrapper.properties

@@ -0,0 +1,49 @@
+#!/usr/bin/env bats
+
+load '../../../build/bats/bats-support/load'
+load '../../../build/bats/bats-assert/load'
+load '../../helper'
+load '../../petclinic-shared/shared-setup.bash'
+
+
+setup_file() {
+  cd test/http_client/httpclient
+}
+
+@test "request without query" {
+  run ./gradlew -q -PmainClass=httpclient.HttpClientTest run ${DEBUG} --args "${WS_URL}/vets"
+
+  assert_json_eq '.events[1].http_client_request.request_method' "GET"
+  assert_json_eq '.events[1].http_client_request.url' "${WS_URL}/vets"
+
+  # Neither message nor parameters should be set
+  assert_json_eq '.events[1].message' ''
+  assert_json_eq '.events[1].parameters' ''
+}
+
+@test "request with query" {
+  run ./gradlew -q -PmainClass=httpclient.HttpClientTest run ${DEBUG} --args "${WS_URL}/owners?lastName=davis"
+
+  assert_json_eq '.events[1].http_client_request.url' "${WS_URL}/owners"
+  assert_json_eq '.events[1].message | length' 1
+  assert_json_eq '.events[1].message[0] | "\(.name) \(.value)"' "lastName davis"
+
+  assert_json_eq '.events[2].http_client_response.status' "200"
+}
+
+@test "request without Content-Type" {
+  run ./gradlew -q -PmainClass=httpclient.HttpClientTest run ${DEBUG} --args "${WS_URL}/no-content"
+
+  assert_json_eq '.events[1].http_client_request.url' "${WS_URL}/no-content"
+  assert_json_eq '.events[2].http_client_response.status' "200"
+}
+
+@test "request with HttpHost" {
+  run ./gradlew -q -PmainClass=httpclient.HttpHostTest run ${DEBUG} --args "${WS_HOST} ${WS_PORT} /owners?lastName=davis"
+
+  assert_json_eq '.events[1].http_client_request.url' "${WS_URL}/owners"
+  assert_json_eq '.events[1].message | length' 1
+  assert_json_eq '.events[1].message[0] | "\(.name) \(.value)"' "lastName davis"
+
+  assert_json_eq '.events[2].http_client_response.status' "200"
+}