Show file modification time (#16589)

* split method LocalNode.downloadFile() to 3 smaller methods

* GET /se/files -> list all downloaded files
* POST /se/files -> get specific file by name
* DELETE /se/files -> delete downloaded files

* replace `ImmutableMap.of` by `Map.of`

It's available from Java 9, and it also returns an immutable map.

* add method to get downloaded files with meta-info from Grid

To track the downloading progress and wait for the full download completion,
we need to know this info about downloaded files:
1. File modification time
2. File size

---------

Co-authored-by: Diego Molina <diemol@users.noreply.github.com>

Author: asolntsev

java/src/org/openqa/selenium/HasDownloads.java

@@ -54,9 +54,18 @@ static boolean isDownloadsEnabled(Capabilities capabilities) {
    * Gets the downloadable files.
    *
    * @return a list of downloadable files for each key
+   * @deprecated Use method {@link #getDownloadedFiles()} instead
    */
+  @Deprecated
   List<String> getDownloadableFiles();
 
+  /**
+   * Gets all files downloaded by browser.
+   *
+   * @return a list of files with their name, size and time.
+   */
+  List<DownloadedFile> getDownloadedFiles();
+
   /**
    * Downloads a file to a given location.
    *
@@ -68,4 +77,34 @@ static boolean isDownloadsEnabled(Capabilities capabilities) {
 
   /** Deletes the downloadable files. */
   void deleteDownloadableFiles();
+
+  class DownloadedFile {
+    private final String name;
+    private final long creationTime;
+    private final long lastModifiedTime;
+    private final long size;
+
+    public DownloadedFile(String name, long creationTime, long lastModifiedTime, long size) {
+      this.name = name;
+      this.creationTime = creationTime;
+      this.lastModifiedTime = lastModifiedTime;
+      this.size = size;
+    }
+
+    public String getName() {
+      return name;
+    }
+
+    public long getCreationTime() {
+      return creationTime;
+    }
+
+    public long getLastModifiedTime() {
+      return lastModifiedTime;
+    }
+
+    public long getSize() {
+      return size;
+    }
+  }
 }

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

@@ -18,6 +18,8 @@
 package org.openqa.selenium.grid.node.local;
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static java.nio.file.Files.readAttributes;
+import static org.openqa.selenium.HasDownloads.DownloadedFile;
 import static org.openqa.selenium.concurrent.ExecutorServices.shutdownGracefully;
 import static org.openqa.selenium.grid.data.Availability.DOWN;
 import static org.openqa.selenium.grid.data.Availability.DRAINING;
@@ -37,14 +39,14 @@
 import com.github.benmanes.caffeine.cache.Ticker;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.io.Serializable;
 import java.io.UncheckedIOException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
@@ -580,8 +582,8 @@ private boolean managedDownloadsRequested(Capabilities capabilities) {
   private Capabilities setDownloadsDirectory(TemporaryFilesystem downloadsTfs, Capabilities caps) {
     File tempDir = downloadsTfs.createTempDir("download", "");
     if (Browser.CHROME.is(caps) || Browser.EDGE.is(caps)) {
-      ImmutableMap<String, Serializable> map =
-          ImmutableMap.of(
+      Map<String, Serializable> map =
+          Map.of(
               "download.prompt_for_download",
               false,
               "download.default_directory",
@@ -592,8 +594,8 @@ private Capabilities setDownloadsDirectory(TemporaryFilesystem downloadsTfs, Cap
       return appendPrefs(caps, optionsKey, map);
     }
     if (Browser.FIREFOX.is(caps)) {
-      ImmutableMap<String, Serializable> map =
-          ImmutableMap.of(
+      Map<String, Serializable> map =
+          Map.of(
               "browser.download.folderList", 2, "browser.download.dir", tempDir.getAbsolutePath());
       return appendPrefs(caps, "moz:firefoxOptions", map);
     }
@@ -738,25 +740,50 @@ public HttpResponse downloadFile(HttpRequest req, SessionId id) {
     }
     File downloadsDirectory =
         Optional.ofNullable(tempFS.getBaseDir().listFiles()).orElse(new File[] {})[0];
-    if (req.getMethod().equals(HttpMethod.GET)) {
-      // User wants to list files that can be downloaded
-      List<String> collected =
-          Arrays.stream(Optional.ofNullable(downloadsDirectory.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));
-    }
-    if (req.getMethod().equals(HttpMethod.DELETE)) {
-      File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {});
-      for (File file : files) {
-        FileHandler.delete(file);
+
+    try {
+      if (req.getMethod().equals(HttpMethod.GET)) {
+        return listDownloadedFiles(downloadsDirectory);
       }
-      Map<String, Object> toReturn = new HashMap<>();
-      toReturn.put("value", null);
-      return new HttpResponse().setContent(asJson(toReturn));
+      if (req.getMethod().equals(HttpMethod.DELETE)) {
+        return deleteDownloadedFile(downloadsDirectory);
+      }
+      return getDownloadedFile(req, downloadsDirectory);
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
     }
+  }
+
+  /** User wants to list files that can be downloaded */
+  private HttpResponse listDownloadedFiles(File downloadsDirectory) {
+    File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {});
+    List<String> fileNames = Arrays.stream(files).map(File::getName).collect(Collectors.toList());
+    List<DownloadedFile> fileInfos =
+        Arrays.stream(files).map(this::getFileInfo).collect(Collectors.toList());
+
+    Map<String, Object> data =
+        Map.of(
+            "names", fileNames,
+            "files", fileInfos);
+    Map<String, Map<String, Object>> result = Map.of("value", data);
+    return new HttpResponse().setContent(asJson(result));
+  }
+
+  private DownloadedFile getFileInfo(File file) {
+    try {
+      BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class);
+      return new DownloadedFile(
+          file.getName(),
+          attributes.creationTime().toMillis(),
+          attributes.lastModifiedTime().toMillis(),
+          attributes.size());
+    } catch (IOException e) {
+      throw new UncheckedIOException("Failed to get file attributes: " + file.getAbsolutePath(), e);
+    }
+  }
+
+  private HttpResponse getDownloadedFile(HttpRequest req, File downloadsDirectory)
+      throws IOException {
     String raw = string(req);
     if (raw.isEmpty()) {
       throw new WebDriverException(
@@ -771,30 +798,37 @@ public HttpResponse downloadFile(HttpRequest req, SessionId id) {
                     new WebDriverException(
                         "Please specify file to download in payload as {\"name\":"
                             + " \"fileToDownload\"}"));
-    try {
-      File[] allFiles =
-          Optional.ofNullable(downloadsDirectory.listFiles((dir, name) -> name.equals(filename)))
-              .orElse(new File[] {});
-      if (allFiles.length == 0) {
-        throw new WebDriverException(
-            String.format(
-                "Cannot find file [%s] in directory %s.",
-                filename, downloadsDirectory.getAbsolutePath()));
-      }
-      if (allFiles.length != 1) {
-        throw new WebDriverException(
-            String.format("Expected there to be only 1 file. There were: %s.", allFiles.length));
-      }
-      String content = Zip.zip(allFiles[0]);
-      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);
+    File[] allFiles =
+        Optional.ofNullable(downloadsDirectory.listFiles((dir, name) -> name.equals(filename)))
+            .orElse(new File[] {});
+    if (allFiles.length == 0) {
+      throw new WebDriverException(
+          String.format(
+              "Cannot find file [%s] in directory %s.",
+              filename, downloadsDirectory.getAbsolutePath()));
+    }
+    if (allFiles.length != 1) {
+      throw new WebDriverException(
+          String.format("Expected there to be only 1 file. There were: %s.", allFiles.length));
+    }
+    String content = Zip.zip(allFiles[0]);
+    Map<String, Object> data =
+        Map.of(
+            "filename", filename,
+            "file", getFileInfo(allFiles[0]),
+            "contents", content);
+    Map<String, Map<String, Object>> result = Map.of("value", data);
+    return new HttpResponse().setContent(asJson(result));
+  }
+
+  private HttpResponse deleteDownloadedFile(File downloadsDirectory) {
+    File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {});
+    for (File file : files) {
+      FileHandler.delete(file);
     }
+    Map<String, Object> toReturn = new HashMap<>();
+    toReturn.put("value", null);
+    return new HttpResponse().setContent(asJson(toReturn));
   }
 
   @Override
@@ -829,7 +863,7 @@ public HttpResponse uploadFile(HttpRequest req, SessionId id) {
           String.format("Expected there to be only 1 file. There were: %s", allFiles.length));
     }
 
-    ImmutableMap<String, Object> result = ImmutableMap.of("value", allFiles[0].getAbsolutePath());
+    Map<String, Object> result = Map.of("value", allFiles[0].getAbsolutePath());
 
     return new HttpResponse().setContent(asJson(result));
   }
@@ -1063,13 +1097,17 @@ private boolean decrementSessionCount() {
   }
 
   private Map<String, Object> toJson() {
-    return ImmutableMap.of(
-        "id", getId(),
-        "uri", externalUri,
-        "maxSessions", maxSessionCount,
-        "draining", isDraining(),
+    return Map.of(
+        "id",
+        getId(),
+        "uri",
+        externalUri,
+        "maxSessions",
+        maxSessionCount,
+        "draining",
+        isDraining(),
         "capabilities",
-            factories.stream().map(SessionSlot::getStereotype).collect(Collectors.toSet()));
+        factories.stream().map(SessionSlot::getStereotype).collect(Collectors.toSet()));
   }
 
   public static class Builder {

java/src/org/openqa/selenium/remote/RemoteWebDriver.java

@@ -20,6 +20,7 @@
 import static java.util.Collections.singleton;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.logging.Level.SEVERE;
+import static org.openqa.selenium.HasDownloads.DownloadedFile;
 import static org.openqa.selenium.remote.CapabilityType.PLATFORM_NAME;
 
 import java.io.IOException;
@@ -672,19 +673,46 @@ public boolean isDownloadsEnabled() {
   }
 
   /**
-   * Retrieves the names of the downloadable files.
+   * Retrieves the names of the files downloaded by browser.
    *
-   * @return A list containing the names of the downloadable files.
+   * @return A list containing the names of the downloaded files.
    * @throws WebDriverException if capability to enable downloads is not set
+   * @deprecated Use method {@link #getDownloadedFiles()} instead
    */
   @Override
   @SuppressWarnings("unchecked")
+  @Deprecated
   public List<String> getDownloadableFiles() {
     requireDownloadsEnabled(capabilities);
 
     Response response = execute(DriverCommand.GET_DOWNLOADABLE_FILES);
-    Map<String, List<String>> value = (Map<String, List<String>>) response.getValue();
-    return value.get("names");
+    Map<String, Object> value = (Map<String, Object>) response.getValue();
+    return (List<String>) value.get("names");
+  }
+
+  /**
+   * Retrieves the list of files downloaded by browser.
+   *
+   * @return A list containing the names, size etc. of the downloaded files.
+   * @throws WebDriverException if capability to enable downloads is not set
+   */
+  @Override
+  @SuppressWarnings("unchecked")
+  public List<DownloadedFile> getDownloadedFiles() {
+    requireDownloadsEnabled(capabilities);
+
+    Response response = execute(DriverCommand.GET_DOWNLOADABLE_FILES);
+    Map<String, Object> value = (Map<String, Object>) response.getValue();
+    List<Map<String, Object>> files = (List<Map<String, Object>>) value.get("files");
+    return files.stream()
+        .map(
+            file ->
+                new DownloadedFile(
+                    (String) file.get("name"),
+                    (Long) file.get("creationTime"),
+                    (Long) file.get("lastModifiedTime"),
+                    (Long) file.get("size")))
+        .collect(Collectors.toUnmodifiableList());
   }
 
   /**

java/test/org/openqa/selenium/grid/router/RemoteWebDriverDownloadTest.java

@@ -18,10 +18,12 @@
 package org.openqa.selenium.grid.router;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.openqa.selenium.HasDownloads.DownloadedFile;
 import static org.openqa.selenium.remote.CapabilityType.ENABLE_DOWNLOADS;
 import static org.openqa.selenium.testing.drivers.Browser.IE;
 import static org.openqa.selenium.testing.drivers.Browser.SAFARI;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.StringReader;
 import java.net.URL;
@@ -34,6 +36,7 @@
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.stream.Collectors;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -114,20 +117,24 @@ void canListDownloadedFiles() {
     driver.findElement(By.id("file-1")).click();
     driver.findElement(By.id("file-2")).click();
 
+    HasDownloads hasDownloads = (HasDownloads) driver;
     new WebDriverWait(driver, Duration.ofSeconds(5))
         .until(
             d ->
-                ((HasDownloads) d)
-                        .getDownloadableFiles().stream()
-                            // ensure we hit no temporary file created by the browser while
-                            // downloading
-                            .filter((f) -> FILE_EXTENSIONS.stream().anyMatch(f::endsWith))
-                            .count()
+                hasDownloads.getDownloadableFiles().stream()
+                        // ensure we hit no temporary file created by the browser while
+                        // downloading
+                        .filter((f) -> FILE_EXTENSIONS.stream().anyMatch(f::endsWith))
+                        .count()
                     == 2);
 
-    List<String> downloadableFiles = ((HasDownloads) driver).getDownloadableFiles();
+    List<String> downloadableFiles = hasDownloads.getDownloadableFiles();
     assertThat(downloadableFiles).contains("file_1.txt", "file_2.jpg");
 
+    List<DownloadedFile> downloadedFiles = hasDownloads.getDownloadedFiles();
+    assertThat(downloadedFiles.stream().map(f -> f.getName()).collect(Collectors.toList()))
+        .contains("file_1.txt", "file_2.jpg");
+
     driver.quit();
   }
 
@@ -150,13 +157,15 @@ void canDownloadFiles() throws IOException {
                         // ensure we hit no temporary file created by the browser while downloading
                         .anyMatch((f) -> FILE_EXTENSIONS.stream().anyMatch(f::endsWith)));
 
-    String fileName = ((HasDownloads) driver).getDownloadableFiles().get(0);
+    DownloadedFile file = ((HasDownloads) driver).getDownloadedFiles().get(0);
 
     Path targetLocation = Files.createTempDirectory("download");
-    ((HasDownloads) driver).downloadFile(fileName, targetLocation);
+    ((HasDownloads) driver).downloadFile(file.getName(), targetLocation);
 
-    String fileContent = String.join("", Files.readAllLines(targetLocation.resolve(fileName)));
-    assertThat(fileContent).isEqualTo("Hello, World!");
+    File localFile = targetLocation.resolve(file.getName()).toFile();
+    assertThat(localFile).hasName(file.getName());
+    assertThat(localFile).hasSize(file.getSize());
+    assertThat(localFile).content().isEqualToIgnoringNewLines("Hello, World!");
 
     driver.quit();
   }