From d245a9a83d9e8c2508e652424ddc812c4ec9fa8b Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Tue, 6 Oct 2026 23:55:44 +0200 Subject: [PATCH] feat: support the base path and the new API prefix in the storage client Storage plugin v3 mounts the routes under the server base path, while v2 always serves them at the server root (see appium/appium#22906). Both versions serve the `/appium/storage` prefix, plugins older than 1.2.0 only the legacy `/storage` one. The client now probes the candidate route roots (base path first, then the server root, for the new prefix and then the legacy one), moving on on 404 only. A route is remembered only after a successful response, so a transient error cannot pin a wrong layout. The web socket paths returned by the plugin are server-root-relative in all versions, so they are now resolved against the server root rather than appended to the base URL path. The URL helper also passed the authority where the user info is expected, which is fixed as well. `StorageClient.PREFIX` now equals `/appium/storage`. Co-Authored-By: Claude Sonnet 5.5 --- .../plugins/storage/StorageClient.java | 76 ++++++++++++++----- .../java_client/plugin/FakeStorageServer.java | 57 +++++++++++--- .../java_client/plugin/StorageTest.java | 52 +++++++++++++ 3 files changed, 155 insertions(+), 30 deletions(-) diff --git a/src/main/java/io/appium/java_client/plugins/storage/StorageClient.java b/src/main/java/io/appium/java_client/plugins/storage/StorageClient.java index 8529c0036..4e1b9e3c9 100644 --- a/src/main/java/io/appium/java_client/plugins/storage/StorageClient.java +++ b/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>() { }.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 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 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(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 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> items = requireResponseValue(httpResponse); return items.stream().map(item -> new StorageItem( (String) item.get("name"), @@ -155,10 +167,9 @@ public List 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); } @@ -166,24 +177,49 @@ public boolean delete(String name) { * 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 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 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(); diff --git a/src/test/java/io/appium/java_client/plugin/FakeStorageServer.java b/src/test/java/io/appium/java_client/plugin/FakeStorageServer.java index 4a198b085..7b898ba41 100644 --- a/src/test/java/io/appium/java_client/plugin/FakeStorageServer.java +++ b/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>() { }.getType(); @@ -57,7 +56,12 @@ final class FakeStorageServer implements AutoCloseable { private final Map items = new LinkedHashMap<>(); private final Map 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,6 +75,20 @@ 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); @@ -78,7 +96,7 @@ private Upload(String name, String sha1) { } 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. */ @@ -86,6 +104,16 @@ 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,18 +162,26 @@ 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 deleteArgs = gson.fromJson(body, MAP_TYPE); boolean deleted; synchronized (this) { @@ -153,7 +189,7 @@ private void respond(OutputStream out, String method, String path, String body) } json = "{\"value\":" + deleted + "}"; break; - case "POST /storage/add": + case "POST /add": json = gson.toJson(Map.of("value", startUpload(body))); break; default: @@ -167,7 +203,7 @@ private void respond(OutputStream out, String method, String path, String body) private synchronized List> list() { var result = new ArrayList>(); 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; } @@ -175,8 +211,9 @@ private Map startUpload(String body) { Map 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); diff --git a/src/test/java/io/appium/java_client/plugin/StorageTest.java b/src/test/java/io/appium/java_client/plugin/StorageTest.java index caea4fcc9..1b38e4ba7 100644 --- a/src/test/java/io/appium/java_client/plugin/StorageTest.java +++ b/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()); }