Skip to content

Commit

Permalink
Filedownloads url - Adhere to w3c standards (#11646)
Browse files Browse the repository at this point in the history 
* Filedownloads url - Adhere to w3c standards

Fixes: #11466, #11458


New format

`POST /se/files`


Request:

`{ "name": "file" }`


Response:
```
{
	"value": {
	"filename": "filename",
	"contents": "ContentsGoHere"
	}
}
```

`GET /se/files`

Request:
NONE

Response:
```
{
	"value": {
		"names":[
		"1",
		"2",
		"3"
		]
	}
}
```
  • Loading branch information
@krmahadevan
krmahadevan authored Feb 17, 2023
1 parent d95e96f commit b1300a6
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 14 deletions.
5 changes: 4 additions & 1 deletion java/src/org/openqa/selenium/grid/node/Node.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
28 changes: 26 additions & 2 deletions java/src/org/openqa/selenium/grid/node/local/LocalNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand All @@ -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);
Expand Down
121 changes: 110 additions & 11 deletions java/test/org/openqa/selenium/grid/node/NodeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();

Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit b1300a6

Please sign in to comment.