Skip to content

Commit 4c7d2d4

Browse files
krmahadevankrmahadevan
committed
Filedownloads url - Adhere to w3c standards
Fixes: SeleniumHQ#11466, SeleniumHQ#11458 New format `POST /se/files` Request: `{ "filename": "file" }` Response: ``` { "value": { "filename": "filename", "contents": "ContentsGoHere" } } ``` `GET /se/files` Request: NONE Response: ``` { "value": { "files":[ "1", "2", "3" ] } } ```
1 parent 1e2a4c4 commit 4c7d2d4

File tree

3 files changed

+140
-14
lines changed

3 files changed

+140
-14
lines changed

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,10 @@ protected Node(Tracer tracer, NodeId id, URI uri, Secret registrationSecret) {
150150
post("/session/{sessionId}/se/file")
151151
.to(params -> new UploadFile(this, sessionIdFrom(params)))
152152
.with(spanDecorator("node.upload_file")),
153-
get("/session/{sessionId}/se/file")
153+
get("/session/{sessionId}/se/files")
154+
.to(params -> new DownloadFile(this, sessionIdFrom(params)))
155+
.with(spanDecorator("node.download_file")),
156+
post("/session/{sessionId}/se/files")
154157
.to(params -> new DownloadFile(this, sessionIdFrom(params)))
155158
.with(spanDecorator("node.download_file")),
156159
get("/se/grid/node/owner/{sessionId}")

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

+26-2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.common.collect.ImmutableList;
2727
import com.google.common.collect.ImmutableMap;
2828

29+
import java.util.Arrays;
2930
import org.openqa.selenium.Capabilities;
3031
import org.openqa.selenium.ImmutableCapabilities;
3132
import org.openqa.selenium.MutableCapabilities;
@@ -63,6 +64,7 @@
6364
import org.openqa.selenium.io.Zip;
6465
import org.openqa.selenium.json.Json;
6566
import org.openqa.selenium.remote.SessionId;
67+
import org.openqa.selenium.remote.http.HttpMethod;
6668
import org.openqa.selenium.remote.http.HttpRequest;
6769
import org.openqa.selenium.remote.http.HttpResponse;
6870
import org.openqa.selenium.remote.tracing.AttributeKey;
@@ -495,7 +497,28 @@ public HttpResponse downloadFile(HttpRequest req, SessionId id) {
495497
if (!dir.isDirectory()) {
496498
throw new WebDriverException(String.format("Invalid directory: %s.", downloadsPath));
497499
}
498-
String filename = req.getQueryParameter("filename");
500+
if (req.getMethod().equals(HttpMethod.GET)) {
501+
//User wants to list files that can be downloaded
502+
List<String> collected = Arrays.stream(
503+
Optional.ofNullable(dir.listFiles()).orElse(new File[]{})
504+
).map(File::getName).collect(Collectors.toList());
505+
ImmutableMap<String, Object> data = ImmutableMap.of("filenames", collected);
506+
ImmutableMap<String, Map<String, Object>> result = ImmutableMap.of("value",data);
507+
return new HttpResponse().setContent(asJson(result));
508+
}
509+
510+
String raw = string(req);
511+
if (raw.isEmpty()) {
512+
throw new WebDriverException(
513+
"Please specify file to download in payload as {\"filename\": \"fileToDownload\"}");
514+
}
515+
516+
Map<String, Object> incoming = JSON.toType(raw, Json.MAP_TYPE);
517+
String filename = Optional.ofNullable(incoming.get("filename"))
518+
.map(Object::toString)
519+
.orElseThrow(
520+
() -> new WebDriverException(
521+
"Please specify file to download in payload as {\"filename\": \"fileToDownload\"}"));
499522
try {
500523
File[] allFiles = Optional.ofNullable(
501524
dir.listFiles((dir1, name) -> name.equals(filename))
@@ -509,9 +532,10 @@ public HttpResponse downloadFile(HttpRequest req, SessionId id) {
509532
String.format("Expected there to be only 1 file. There were: %s.", allFiles.length));
510533
}
511534
String content = Zip.zip(allFiles[0]);
512-
ImmutableMap<String, Object> result = ImmutableMap.of(
535+
ImmutableMap<String, Object> data = ImmutableMap.of(
513536
"filename", filename,
514537
"contents", content);
538+
ImmutableMap<String, Map<String, Object>> result = ImmutableMap.of("value",data);
515539
return new HttpResponse().setContent(asJson(result));
516540
} catch (IOException e) {
517541
throw new UncheckedIOException(e);

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

+110-11
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static java.util.concurrent.TimeUnit.SECONDS;
2222
import static org.assertj.core.api.Assertions.assertThat;
2323
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
24+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2425
import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
2526
import static org.openqa.selenium.json.Json.MAP_TYPE;
2627
import static org.openqa.selenium.remote.http.Contents.string;
@@ -31,8 +32,12 @@
3132
import com.google.common.collect.ImmutableSet;
3233

3334
import java.nio.file.Path;
35+
import java.util.List;
36+
import java.util.Optional;
3437
import org.junit.jupiter.api.BeforeEach;
38+
import org.junit.jupiter.api.DisplayName;
3539
import org.junit.jupiter.api.Test;
40+
import org.junit.jupiter.api.TestInfo;
3641
import org.openqa.selenium.Capabilities;
3742
import org.openqa.selenium.ImmutableCapabilities;
3843
import org.openqa.selenium.NoSuchSessionException;
@@ -49,6 +54,7 @@
4954
import org.openqa.selenium.grid.data.Session;
5055
import org.openqa.selenium.grid.data.SessionClosedEvent;
5156
import org.openqa.selenium.grid.node.local.LocalNode;
57+
import org.openqa.selenium.grid.node.local.LocalNode.Builder;
5258
import org.openqa.selenium.grid.node.remote.RemoteNode;
5359
import org.openqa.selenium.grid.security.Secret;
5460
import org.openqa.selenium.grid.testing.EitherAssert;
@@ -106,7 +112,7 @@ private static <A, B> EitherAssert<A, B> assertThatEither(Either<A, B> either) {
106112
}
107113

108114
@BeforeEach
109-
public void setUp() throws URISyntaxException {
115+
public void setUp(TestInfo testInfo) throws URISyntaxException {
110116
tracer = DefaultTestTracer.createTracer();
111117
bus = new GuavaEventBus();
112118

@@ -129,13 +135,15 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
129135
}
130136
}
131137

132-
local = LocalNode.builder(tracer, bus, uri, uri, registrationSecret)
133-
.add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
134-
.add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
135-
.add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
136-
.downloadsPath(downloadsPath.getAbsolutePath())
137-
.maximumConcurrentSessions(2)
138-
.build();
138+
Builder builder = LocalNode.builder(tracer, bus, uri, uri, registrationSecret)
139+
.add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
140+
.add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
141+
.add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
142+
.maximumConcurrentSessions(2);
143+
if (!testInfo.getDisplayName().equalsIgnoreCase("BootWithoutDownloadsDir")) {
144+
builder = builder.downloadsPath(downloadsPath.getAbsolutePath());
145+
}
146+
local = builder.build();
139147

140148
node = new RemoteNode(
141149
tracer,
@@ -495,20 +503,111 @@ void canDownloadAFile() throws IOException {
495503
Session session = response.right().getSession();
496504
String hello = "Hello, world!";
497505

498-
HttpRequest req = new HttpRequest(GET, String.format("/session/%s/se/file", session.getId()));
506+
HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
499507
File zip = createTmpFile(hello);
500-
req.addQueryParameter("filename", zip.getName());
508+
String payload = new Json().toJson(Collections.singletonMap("filename", zip.getName()));
509+
req.setContent(() -> new ByteArrayInputStream(payload.getBytes()));
501510
HttpResponse rsp = node.execute(req);
511+
Map<String, Object> raw = new Json().toType(string(rsp), Json.MAP_TYPE);
502512
node.stop(session.getId());
503-
Map<String, Object> map = new Json().toType(string(rsp), Json.MAP_TYPE);
513+
assertThat(raw).isNotNull();
504514
File baseDir = getTemporaryFilesystemBaseDir(TemporaryFilesystem.getDefaultTmpFS());
515+
Map<String, Object> map = Optional.ofNullable(
516+
raw.get("value")
517+
).map(data -> (Map<String, Object>) data)
518+
.orElseThrow(() -> new IllegalStateException("Could not find value attribute"));
505519
String encodedContents = map.get("contents").toString();
506520
Zip.unzip(encodedContents, baseDir);
507521
Path path = new File(baseDir.getAbsolutePath() + "/" + map.get("filename")).toPath();
508522
String decodedContents = String.join("", Files.readAllLines(path));
509523
assertThat(decodedContents).isEqualTo(hello);
510524
}
511525

526+
@Test
527+
void canListFilesToDownload() {
528+
Either<WebDriverException, CreateSessionResponse> response =
529+
node.newSession(createSessionRequest(caps));
530+
assertThatEither(response).isRight();
531+
Session session = response.right().getSession();
532+
String hello = "Hello, world!";
533+
File zip = createTmpFile(hello);
534+
HttpRequest req = new HttpRequest(GET, String.format("/session/%s/se/files", session.getId()));
535+
HttpResponse rsp = node.execute(req);
536+
Map<String, Object> raw = new Json().toType(string(rsp), Json.MAP_TYPE);
537+
node.stop(session.getId());
538+
assertThat(raw).isNotNull();
539+
Map<String, Object> map = Optional.ofNullable(
540+
raw.get("value")
541+
).map(data -> (Map<String, Object>) data)
542+
.orElseThrow(() -> new IllegalStateException("Could not find value attribute"));
543+
List<String> files = (List<String>) map.get("filenames");
544+
assertThat(files).contains(zip.getName());
545+
}
546+
547+
@Test
548+
@DisplayName("BootWithoutDownloadsDir")
549+
void canDownloadFileThrowsErrorMsgWhenDownloadsDirNotSpecified() {
550+
Either<WebDriverException, CreateSessionResponse> response =
551+
node.newSession(createSessionRequest(caps));
552+
assertThatEither(response).isRight();
553+
Session session = response.right().getSession();
554+
createTmpFile("Hello, world!");
555+
556+
HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
557+
String msg = "Please specify the path wherein the files downloaded using the browser "
558+
+ "would be available via the command line arg [--downloads-path] and restart the node";
559+
assertThatThrownBy(() -> node.execute(req))
560+
.hasMessageContaining(msg);
561+
}
562+
563+
@Test
564+
void canDownloadFileThrowsErrorMsgWhenPayloadIsMissing() {
565+
Either<WebDriverException, CreateSessionResponse> response =
566+
node.newSession(createSessionRequest(caps));
567+
assertThatEither(response).isRight();
568+
Session session = response.right().getSession();
569+
createTmpFile("Hello, world!");
570+
571+
HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
572+
String msg = "Please specify file to download in payload as {\"filename\": \"fileToDownload\"}";
573+
assertThatThrownBy(() -> node.execute(req))
574+
.hasMessageContaining(msg);
575+
}
576+
577+
@Test
578+
void canDownloadFileThrowsErrorMsgWhenWrongPayloadIsGiven() {
579+
Either<WebDriverException, CreateSessionResponse> response =
580+
node.newSession(createSessionRequest(caps));
581+
assertThatEither(response).isRight();
582+
Session session = response.right().getSession();
583+
createTmpFile("Hello, world!");
584+
585+
HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
586+
String payload = new Json().toJson(Collections.singletonMap("my-file", "README.md"));
587+
req.setContent(() -> new ByteArrayInputStream(payload.getBytes()));
588+
589+
String msg = "Please specify file to download in payload as {\"filename\": \"fileToDownload\"}";
590+
assertThatThrownBy(() -> node.execute(req))
591+
.hasMessageContaining(msg);
592+
}
593+
594+
@Test
595+
void canDownloadFileThrowsErrorMsgWhenFileNotFound() {
596+
Either<WebDriverException, CreateSessionResponse> response =
597+
node.newSession(createSessionRequest(caps));
598+
assertThatEither(response).isRight();
599+
Session session = response.right().getSession();
600+
createTmpFile("Hello, world!");
601+
602+
HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId()));
603+
String payload = new Json().toJson(Collections.singletonMap("filename", "README.md"));
604+
req.setContent(() -> new ByteArrayInputStream(payload.getBytes()));
605+
606+
String msg = "Cannot find file [README.md] in directory";
607+
assertThatThrownBy(() -> node.execute(req))
608+
.hasMessageContaining(msg);
609+
}
610+
512611
@Test
513612
void shouldNotCreateSessionIfDraining() {
514613
node.drain();

0 commit comments

Comments
 (0)