feat: support the base path and the new API prefix in the storage client

src/main/java/io/appium/java_client/plugins/storage/StorageClient.java

@@ -32,6 +32,7 @@
 
 import java.io.File;
 import java.lang.reflect.Type;
+import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -44,6 +45,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static io.appium.java_client.plugins.storage.StorageUtils.calcSha1Digest;
 import static io.appium.java_client.plugins.storage.StorageUtils.streamFileToWebSocket;
@@ -54,28 +56,40 @@
  * for more details.
  */
 public class StorageClient {
-    public static final String PREFIX = "/storage";
+    public static final String PREFIX = "/appium/storage";
+    private static final String LEGACY_PREFIX = "/storage";
     private static final Type MAP_TYPE = new TypeToken<Map<String, Object>>() { }.getType();
     private final Gson gson = new Gson();
     private final AppiumW3CHttpResponseCodec responseCodec = new AppiumW3CHttpResponseCodec();
 
     private final URL baseUrl;
     private final HttpClient httpClient;
+    // Candidate route roots, the most recent plugin layout first:
+    // v3+ mounts the routes under the server base path, v2 at the server root, <1.2.0 also under the legacy prefix
+    private final List<URL> routeRoots;
+    private volatile int routeRootIndex = 0;
 
     /**
      * Creates a client of the storage plugin.
      *
-     * @param baseUrl the address of the Appium server
+     * @param baseUrl the address of the Appium server, including the base path if there is one
      */
     public StorageClient(URL baseUrl) {
         this.baseUrl = baseUrl;
         this.httpClient = HttpClient.Factory.createDefault()
                 .createClient(AppiumClientConfig.defaultConfig().baseUrl(baseUrl));
+        this.routeRoots = buildRouteRoots(baseUrl);
     }
 
+    /**
+     * Creates a client of the storage plugin.
+     *
+     * @param clientConfig the client config, which base URL is the address of the Appium server
+     */
     public StorageClient(AppiumClientConfig clientConfig) {
         this.httpClient = HttpClient.Factory.createDefault().createClient(clientConfig);
         this.baseUrl = clientConfig.baseUrl();
+        this.routeRoots = buildRouteRoots(baseUrl);
     }
 
     /**
@@ -95,11 +109,10 @@ public void add(File file) {
      * @param name The remote file name.
      */
     public void add(File file, String name) {
-        var request = new HttpRequest(HttpMethod.POST, formatPath(baseUrl, PREFIX, "add").toString());
-        var httpResponse = httpClient.execute(setJsonPayload(request, Map.of(
+        var httpResponse = execute(HttpMethod.POST, "add", Map.of(
                 "name", name,
                 "sha1", calcSha1Digest(file)
-        )));
+        ));
         Map<String, Object> value = requireResponseValue(httpResponse);
         final var wsTtlMs = (Long) value.get("ttlMs");
         //noinspection unchecked
@@ -109,10 +122,10 @@ public void add(File file, String name) {
         final var completion = new CountDownLatch(1);
         final var lastException = new AtomicReference<Throwable>(null);
         try (var streamWs = httpClient.openSocket(
-                new HttpRequest(HttpMethod.POST, formatPath(baseUrl, streamWsPathname).toString()),
+                new HttpRequest(HttpMethod.POST, withPath(baseUrl, streamWsPathname).toString()),
                 new WebSocket.Listener() {}
         ); var eventsWs = httpClient.openSocket(
-                new HttpRequest(HttpMethod.POST, formatPath(baseUrl, eventWsPathname).toString()),
+                new HttpRequest(HttpMethod.POST, withPath(baseUrl, eventWsPathname).toString()),
                 new EventWsListener(lastException, completion)
         )) {
             streamFileToWebSocket(file, streamWs);
@@ -138,8 +151,7 @@ public void add(File file, String name) {
      * @return All storage items.
      */
     public List<StorageItem> list() {
-        var request = new HttpRequest(HttpMethod.GET, formatPath(baseUrl, PREFIX, "list").toString());
-        var httpResponse = httpClient.execute(request);
+        var httpResponse = execute(HttpMethod.GET, "list", null);
         List<Map<String, Object>> items = requireResponseValue(httpResponse);
         return items.stream().map(item -> new StorageItem(
                 (String) item.get("name"),
@@ -155,35 +167,59 @@ public List<StorageItem> list() {
      * @return true if the dletion was successful.
      */
     public boolean delete(String name) {
-        var request = new HttpRequest(HttpMethod.POST, formatPath(baseUrl, PREFIX, "delete").toString());
-        var httpResponse = httpClient.execute(setJsonPayload(request, Map.of(
+        var httpResponse = execute(HttpMethod.POST, "delete", Map.of(
                 "name", name
-        )));
+        ));
         return requireResponseValue(httpResponse);
     }
 
     /**
      * Resets all items of the server storage.
      */
     public void reset() {
-        var request = new HttpRequest(HttpMethod.POST, formatPath(baseUrl, PREFIX, "reset").toString());
-        var httpResponse = httpClient.execute(request);
+        var httpResponse = execute(HttpMethod.POST, "reset", null);
         requireResponseValue(httpResponse);
     }
 
-    private static URL formatPath(URL url, String... suffixes) {
-        if (suffixes.length == 0) {
-            return url;
+    private static List<URL> buildRouteRoots(URL baseUrl) {
+        var serverRoots = baseUrl.getPath().replace("/", "").isEmpty()
+                ? List.of(baseUrl) : List.of(baseUrl, withPath(baseUrl, "/"));
+        return Stream.of(PREFIX, LEGACY_PREFIX)
+                .flatMap(prefix -> serverRoots.stream().map(root -> withPath(root, root.getPath() + prefix)))
+                .collect(Collectors.toList());
+    }
+
+    /**
+     * Executes the storage endpoint, trying the next route root on 404. Only a successful route is remembered.
+     */
+    private HttpResponse execute(HttpMethod method, String endpoint, Map<String, Object> payload) {
+        HttpResponse response = null;
+        for (int i = routeRootIndex; i < routeRoots.size(); i++) {
+            var root = routeRoots.get(i);
+            var request = new HttpRequest(method, withPath(root, root.getPath() + "/" + endpoint).toString());
+            response = httpClient.execute(payload == null ? request : setJsonPayload(request, payload));
+            if (response.getStatus() != HttpURLConnection.HTTP_NOT_FOUND) {
+                if (response.isSuccessful()) {
+                    routeRootIndex = i;
+                }
+                break;
+            }
         }
+        return response;
+    }
+
+    /**
+     * Replaces the path of the URL. The web socket paths reported by the plugin are always server-root-relative.
+     */
+    private static URL withPath(URL url, String path) {
         try {
             var uri = url.toURI();
-            var updatedPath = (uri.getPath() + "/" + String.join("/", suffixes)).replaceAll("(/{2,})", "/");
             return new URI(
                     uri.getScheme(),
-                    uri.getAuthority(),
+                    uri.getUserInfo(),
                     uri.getHost(),
                     uri.getPort(),
-                    updatedPath,
+                    ("/" + path).replaceAll("(/{2,})", "/"),
                     uri.getQuery(),
                     uri.getFragment()
             ).toURL();

src/test/java/io/appium/java_client/plugin/FakeStorageServer.java

@@ -47,7 +47,6 @@
  */
 final class FakeStorageServer implements AutoCloseable {
     private static final String GUID = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11";
-    private static final String UPLOAD_PREFIX = "/storage/add/";
     private static final long EVENTS_TIMEOUT_SEC = 10;
 
     private static final Type MAP_TYPE = new TypeToken<Map<String, Object>>() { }.getType();
@@ -57,7 +56,12 @@ final class FakeStorageServer implements AutoCloseable {
     private final Map<String, byte[]> items = new LinkedHashMap<>();
     private final Map<String, Upload> uploads = new ConcurrentHashMap<>();
     private final AtomicInteger uploadCounter = new AtomicInteger();
+    private final String basePath;
+    private final String routePrefix;
+    private final String storagePrefix;
+    private final AtomicInteger requestCounter = new AtomicInteger();
     private volatile boolean rejectUploads;
+    private volatile int failureStatus;
 
     private static final class Upload {
         private final String name;
@@ -71,21 +75,45 @@ private Upload(String name, String sha1) {
     }
 
     FakeStorageServer() throws IOException {
+        this("", true, "/appium/storage");
+    }
+
+    /**
+     * Creates a fake server with the given base path.
+     *
+     * @param basePath the base path of the imitated Appium server
+     * @param honorBasePath whether the routes are mounted under the base path (plugin v3+) or at the root (v2)
+     * @param storagePrefix the route prefix: /appium/storage, or /storage for plugins older than 1.2.0
+     */
+    FakeStorageServer(String basePath, boolean honorBasePath, String storagePrefix) throws IOException {
+        this.basePath = basePath;
+        this.storagePrefix = storagePrefix;
+        this.routePrefix = (honorBasePath ? basePath : "") + storagePrefix;
         serverSocket = new ServerSocket(0, 50, InetAddress.getLoopbackAddress());
         var thread = new Thread(this::acceptConnections, "fake-storage-server");
         thread.setDaemon(true);
         thread.start();
     }
 
     URL url() throws IOException {
-        return new URL("http://127.0.0.1:" + serverSocket.getLocalPort());
+        return new URL("http://127.0.0.1:" + serverSocket.getLocalPort() + basePath);
     }
 
     /** Makes the server report a failure for the uploads, as if the content could not be saved. */
     void rejectUploads() {
         rejectUploads = true;
     }
 
+    /** Makes the served routes respond with the given HTTP error status, 0 restores the normal behavior. */
+    void failWith(int status) {
+        failureStatus = status;
+    }
+
+    /** The number of the plain HTTP requests received so far, web socket handshakes excluded. */
+    int requestCount() {
+        return requestCounter.get();
+    }
+
     synchronized byte[] content(String name) {
         return items.get(name);
     }
@@ -134,26 +162,34 @@ private void handle(Socket socket) {
 
     private void respond(OutputStream out, String method, String path, String body) throws IOException {
         var route = method + " " + path;
+        var endpoint = method + " " + (path.startsWith(routePrefix + "/")
+                ? path.substring(routePrefix.length()) : path);
+        requestCounter.incrementAndGet();
+        if (failureStatus != 0 && path.startsWith(routePrefix + "/")) {
+            writeResponse(out, failureStatus,
+                    "{\"value\":{\"error\":\"unknown error\",\"message\":\"boom\",\"stacktrace\":\"\"}}");
+            return;
+        }
         String json;
-        switch (route) {
-            case "POST /storage/reset":
+        switch (endpoint) {
+            case "POST /reset":
                 synchronized (this) {
                     items.clear();
                 }
                 json = "{\"value\":null}";
                 break;
-            case "GET /storage/list":
+            case "GET /list":
                 json = gson.toJson(Map.of("value", list()));
                 break;
-            case "POST /storage/delete":
+            case "POST /delete":
                 Map<String, Object> deleteArgs = gson.fromJson(body, MAP_TYPE);
                 boolean deleted;
                 synchronized (this) {
                     deleted = items.remove((String) deleteArgs.get("name")) != null;
                 }
                 json = "{\"value\":" + deleted + "}";
                 break;
-            case "POST /storage/add":
+            case "POST /add":
                 json = gson.toJson(Map.of("value", startUpload(body)));
                 break;
             default:
@@ -167,16 +203,17 @@ private void respond(OutputStream out, String method, String path, String body)
     private synchronized List<Map<String, Object>> list() {
         var result = new ArrayList<Map<String, Object>>();
         items.forEach((name, content) -> result.add(
-                Map.of("name", name, "path", "/storage/" + name, "size", (long) content.length)));
+                Map.of("name", name, "path", storagePrefix + "/" + name, "size", (long) content.length)));
         return result;
     }
 
     private Map<String, Object> startUpload(String body) {
         Map<String, Object> args = gson.fromJson(body, MAP_TYPE);
         var id = String.valueOf(uploadCounter.incrementAndGet());
         uploads.put(id, new Upload((String) args.get("name"), (String) args.get("sha1")));
+        var wsPrefix = routePrefix + "/add/" + id;
         return Map.of(
-                "ws", Map.of("stream", UPLOAD_PREFIX + id + "/stream", "events", UPLOAD_PREFIX + id + "/events"),
+                "ws", Map.of("stream", wsPrefix + "/stream", "events", wsPrefix + "/events"),
                 "ttlMs", 10000L
         );
     }
@@ -188,7 +225,7 @@ private void handleWebSocket(String path, String key, DataInputStream in, Output
         out.write(("HTTP/1.1 101 Switching Protocols\r\nUpgrade: websocket\r\nConnection: Upgrade\r\n"
                 + "Sec-WebSocket-Accept: " + accept + "\r\n\r\n").getBytes(StandardCharsets.US_ASCII));
         out.flush();
-        var parts = path.substring(UPLOAD_PREFIX.length()).split("/");
+        var parts = path.substring((routePrefix + "/add/").length()).split("/");
         var upload = uploads.get(parts[0]);
         if ("events".equals(parts[1])) {
             upload.events.complete(out);

src/test/java/io/appium/java_client/plugin/StorageTest.java

@@ -90,6 +90,58 @@ void shouldFailIfTheServerReportsAFailedUpload() {
         assertItemsCount(0);
     }
 
+    @Test
+    void shouldUseTheServerBasePath() throws IOException {
+        assertBasicActionsWork(new FakeStorageServer("/wd/hub", true, "/appium/storage"));
+    }
+
+    @Test
+    void shouldFallBackToTheServerRootIfTheBasePathIsIgnored() throws IOException {
+        assertBasicActionsWork(new FakeStorageServer("/wd/hub", false, "/appium/storage"));
+    }
+
+    @Test
+    void shouldFallBackToTheLegacyPrefix() throws IOException {
+        assertBasicActionsWork(new FakeStorageServer("", false, "/storage"));
+    }
+
+    @Test
+    void shouldFallBackToTheLegacyPrefixAtTheServerRoot() throws IOException {
+        assertBasicActionsWork(new FakeStorageServer("/wd/hub", false, "/storage"));
+    }
+
+    @Test
+    void shouldRememberTheRouteOnlyAfterASuccessfulResponse() throws IOException {
+        try (var customServer = new FakeStorageServer("/wd/hub", false, "/storage")) {
+            var client = new StorageClient(customServer.url());
+            // 3 unserved layouts answer with 404, the 4th one fails
+            customServer.failWith(500);
+            assertThrows(WebDriverException.class, client::list);
+            assertEquals(4, customServer.requestCount());
+            // the failed route is not remembered, so the probing starts over
+            customServer.failWith(0);
+            assertTrue(client.list().isEmpty());
+            assertEquals(8, customServer.requestCount());
+            // the successful route is remembered
+            assertTrue(client.list().isEmpty());
+            assertEquals(9, customServer.requestCount());
+        }
+    }
+
+    private void assertBasicActionsWork(FakeStorageServer customServer) throws IOException {
+        try (customServer) {
+            var client = new StorageClient(customServer.url());
+            var testFile = TestUtils.resourcePathToAbsolutePath("html/" + NAME).toFile();
+            assertTrue(client.list().isEmpty());
+            client.add(testFile);
+            assertEquals(NAME, client.list().get(0).getName());
+            assertArrayEquals(Files.readAllBytes(testFile.toPath()), customServer.content(NAME));
+            assertTrue(client.delete(NAME));
+            client.reset();
+            assertTrue(client.list().isEmpty());
+        }
+    }
+
     private void assertItemsCount(int expected) {
         assertEquals(expected, storageClient.list().size());
     }