Filedownloads url - Adhere to w3c standards

java/src/org/openqa/selenium/grid/node/Node.java

@@ -150,7 +150,10 @@ protected Node(Tracer tracer, NodeId id, URI uri, Secret registrationSecret) {
       post("/session/{sessionId}/se/file")
         .to(params -> new UploadFile(this, sessionIdFrom(params)))
         .with(spanDecorator("node.upload_file")),
-      get("/session/{sessionId}/se/file")
+      get("/session/{sessionId}/se/files")
+        .to(params -> new DownloadFile(this, sessionIdFrom(params)))
+        .with(spanDecorator("node.download_file")),
+      post("/session/{sessionId}/se/files")
         .to(params -> new DownloadFile(this, sessionIdFrom(params)))
         .with(spanDecorator("node.download_file")),
       get("/se/grid/node/owner/{sessionId}")

java/src/org/openqa/selenium/grid/node/local/LocalNode.java

@@ -26,6 +26,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 
+import java.util.Arrays;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.MutableCapabilities;
@@ -63,6 +64,7 @@
 import org.openqa.selenium.io.Zip;
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.remote.SessionId;
+import org.openqa.selenium.remote.http.HttpMethod;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
 import org.openqa.selenium.remote.tracing.AttributeKey;
@@ -495,7 +497,28 @@ public HttpResponse downloadFile(HttpRequest req, SessionId id) {
     if (!dir.isDirectory()) {
       throw new WebDriverException(String.format("Invalid directory: %s.", downloadsPath));
     }
-    String filename = req.getQueryParameter("filename");
+    if (req.getMethod().equals(HttpMethod.GET)) {
+      //User wants to list files that can be downloaded
+      List<String> collected = Arrays.stream(
+        Optional.ofNullable(dir.listFiles()).orElse(new File[]{})
+      ).map(File::getName).collect(Collectors.toList());
+      ImmutableMap<String, Object> data = ImmutableMap.of("names", collected);
+      ImmutableMap<String, Map<String, Object>> result = ImmutableMap.of("value",data);
+      return new HttpResponse().setContent(asJson(result));
+    }
+
+    String raw = string(req);
+    if (raw.isEmpty()) {
+      throw new WebDriverException(
+        "Please specify file to download in payload as {\"name\": \"fileToDownload\"}");
+    }
+
+    Map<String, Object> incoming = JSON.toType(raw, Json.MAP_TYPE);
+    String filename = Optional.ofNullable(incoming.get("name"))
+      .map(Object::toString)
+      .orElseThrow(
+        () -> new WebDriverException(
+          "Please specify file to download in payload as {\"name\": \"fileToDownload\"}"));
     try {
       File[] allFiles = Optional.ofNullable(
         dir.listFiles((dir1, name) -> name.equals(filename))
@@ -509,9 +532,10 @@ public HttpResponse downloadFile(HttpRequest req, SessionId id) {
           String.format("Expected there to be only 1 file. There were: %s.", allFiles.length));
       }
       String content = Zip.zip(allFiles[0]);
-      ImmutableMap<String, Object> result = ImmutableMap.of(
+      ImmutableMap<String, Object> data = ImmutableMap.of(
         "filename", filename,
         "contents", content);
+      ImmutableMap<String, Map<String, Object>> result = ImmutableMap.of("value",data);
       return new HttpResponse().setContent(asJson(result));
     } catch (IOException e) {
       throw new UncheckedIOException(e);

java/test/org/openqa/selenium/grid/node/NodeTest.java

@@ -21,6 +21,7 @@
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
 import static org.openqa.selenium.json.Json.MAP_TYPE;
 import static org.openqa.selenium.remote.http.Contents.string;
@@ -31,8 +32,12 @@
 import com.google.common.collect.ImmutableSet;
 
 import java.nio.file.Path;
+import java.util.List;
+import java.util.Optional;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.NoSuchSessionException;
@@ -49,6 +54,7 @@
 import org.openqa.selenium.grid.data.Session;
 import org.openqa.selenium.grid.data.SessionClosedEvent;
 import org.openqa.selenium.grid.node.local.LocalNode;
+import org.openqa.selenium.grid.node.local.LocalNode.Builder;
 import org.openqa.selenium.grid.node.remote.RemoteNode;
 import org.openqa.selenium.grid.security.Secret;
 import org.openqa.selenium.grid.testing.EitherAssert;
@@ -106,7 +112,7 @@ private static <A, B> EitherAssert<A, B> assertThatEither(Either<A, B> either) {
   }
 
   @BeforeEach
-  public void setUp() throws URISyntaxException {
+  public void setUp(TestInfo testInfo) throws URISyntaxException {
     tracer = DefaultTestTracer.createTracer();
     bus = new GuavaEventBus();
 
@@ -129,13 +135,15 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
       }
     }
 
-    local = LocalNode.builder(tracer, bus, uri, uri, registrationSecret)
-        .add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
-        .add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
-        .add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
-        .downloadsPath(downloadsPath.getAbsolutePath())
-        .maximumConcurrentSessions(2)
-        .build();
+    Builder builder = LocalNode.builder(tracer, bus, uri, uri, registrationSecret)
+      .add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
+      .add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
+      .add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
+      .maximumConcurrentSessions(2);
+    if (!testInfo.getDisplayName().equalsIgnoreCase("BootWithoutDownloadsDir")) {
+      builder = builder.downloadsPath(downloadsPath.getAbsolutePath());
+    }
+    local = builder.build();
 
     node = new RemoteNode(
         tracer,
@@ -495,20 +503,111 @@ void canDownloadAFile() throws IOException {
     Session session = response.right().getSession();
     String hello = "Hello, world!";
 
-    HttpRequest req = new HttpRequest(GET, String.format("/session/%s/se/file", session.getId()));
+    HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
     File zip = createTmpFile(hello);
-    req.addQueryParameter("filename", zip.getName());
+    String payload = new Json().toJson(Collections.singletonMap("name", zip.getName()));
+    req.setContent(() -> new ByteArrayInputStream(payload.getBytes()));
     HttpResponse rsp = node.execute(req);
+    Map<String, Object> raw = new Json().toType(string(rsp), Json.MAP_TYPE);
     node.stop(session.getId());
-    Map<String, Object> map = new Json().toType(string(rsp), Json.MAP_TYPE);
+    assertThat(raw).isNotNull();
     File baseDir = getTemporaryFilesystemBaseDir(TemporaryFilesystem.getDefaultTmpFS());
+    Map<String, Object> map = Optional.ofNullable(
+        raw.get("value")
+      ).map(data -> (Map<String, Object>) data)
+      .orElseThrow(() -> new IllegalStateException("Could not find value attribute"));
     String encodedContents = map.get("contents").toString();
     Zip.unzip(encodedContents, baseDir);
     Path path = new File(baseDir.getAbsolutePath() + "/" + map.get("filename")).toPath();
     String decodedContents = String.join("", Files.readAllLines(path));
     assertThat(decodedContents).isEqualTo(hello);
   }
 
+  @Test
+  void canListFilesToDownload() {
+    Either<WebDriverException, CreateSessionResponse> response =
+      node.newSession(createSessionRequest(caps));
+    assertThatEither(response).isRight();
+    Session session = response.right().getSession();
+    String hello = "Hello, world!";
+    File zip = createTmpFile(hello);
+    HttpRequest req = new HttpRequest(GET, String.format("/session/%s/se/files", session.getId()));
+    HttpResponse rsp = node.execute(req);
+    Map<String, Object> raw = new Json().toType(string(rsp), Json.MAP_TYPE);
+    node.stop(session.getId());
+    assertThat(raw).isNotNull();
+    Map<String, Object> map = Optional.ofNullable(
+        raw.get("value")
+      ).map(data -> (Map<String, Object>) data)
+      .orElseThrow(() -> new IllegalStateException("Could not find value attribute"));
+    List<String> files = (List<String>) map.get("names");
+    assertThat(files).contains(zip.getName());
+  }
+
+  @Test
+  @DisplayName("BootWithoutDownloadsDir")
+  void canDownloadFileThrowsErrorMsgWhenDownloadsDirNotSpecified() {
+    Either<WebDriverException, CreateSessionResponse> response =
+      node.newSession(createSessionRequest(caps));
+    assertThatEither(response).isRight();
+    Session session = response.right().getSession();
+    createTmpFile("Hello, world!");
+
+    HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
+    String msg = "Please specify the path wherein the files downloaded using the browser "
+      + "would be available via the command line arg [--downloads-path] and restart the node";
+    assertThatThrownBy(() -> node.execute(req))
+      .hasMessageContaining(msg);
+  }
+
+  @Test
+  void canDownloadFileThrowsErrorMsgWhenPayloadIsMissing() {
+    Either<WebDriverException, CreateSessionResponse> response =
+      node.newSession(createSessionRequest(caps));
+    assertThatEither(response).isRight();
+    Session session = response.right().getSession();
+    createTmpFile("Hello, world!");
+
+    HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
+    String msg = "Please specify file to download in payload as {\"name\": \"fileToDownload\"}";
+    assertThatThrownBy(() -> node.execute(req))
+      .hasMessageContaining(msg);
+  }
+
+  @Test
+  void canDownloadFileThrowsErrorMsgWhenWrongPayloadIsGiven() {
+    Either<WebDriverException, CreateSessionResponse> response =
+      node.newSession(createSessionRequest(caps));
+    assertThatEither(response).isRight();
+    Session session = response.right().getSession();
+    createTmpFile("Hello, world!");
+
+    HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
+    String payload = new Json().toJson(Collections.singletonMap("my-file", "README.md"));
+    req.setContent(() -> new ByteArrayInputStream(payload.getBytes()));
+
+    String msg = "Please specify file to download in payload as {\"name\": \"fileToDownload\"}";
+    assertThatThrownBy(() -> node.execute(req))
+      .hasMessageContaining(msg);
+  }
+
+  @Test
+  void canDownloadFileThrowsErrorMsgWhenFileNotFound() {
+    Either<WebDriverException, CreateSessionResponse> response =
+      node.newSession(createSessionRequest(caps));
+    assertThatEither(response).isRight();
+    Session session = response.right().getSession();
+    createTmpFile("Hello, world!");
+
+    HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
+    String payload = new Json().toJson(Collections.singletonMap("name", "README.md"));
+    req.setContent(() -> new ByteArrayInputStream(payload.getBytes()));
+
+    String msg = "Cannot find file [README.md] in directory";
+    assertThatThrownBy(() -> node.execute(req))
+      .hasMessageContaining(msg);
+  }
+
   @Test
   void shouldNotCreateSessionIfDraining() {
     node.drain();