Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Java: Support for sending firefox addon directory as temporary in remote sessions #10566

Merged
merged 7 commits into from
Apr 22, 2022
Merged

Java: Support for sending firefox addon directory as temporary in remote sessions #10566

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a139724
Sending firefox addon directory as temporary in remote sessions
TamsilAmani Apr 20, 2022
b56a962
Merge branch 'trunk' into firefox-addon-support
pujagani Apr 21, 2022
a3c5999
Added separate test for firefox addon directory as temp
TamsilAmani Apr 21, 2022
6550267
Merge branch 'trunk' into firefox-addon-support
pujagani Apr 21, 2022
900643c
Merge branch 'trunk' into firefox-addon-support
pujagani Apr 21, 2022
bf3c0a9
Added separate method to zip the directory and using try-with-resources
TamsilAmani Apr 21, 2022
27fe0a3
Merge branch 'trunk' into firefox-addon-support
pujagani Apr 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions java/src/org/openqa/selenium/firefox/AddHasExtensions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@
import org.openqa.selenium.remote.ExecuteMethod;
import org.openqa.selenium.remote.http.HttpMethod;

import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IntelliJ tends to do this by default when optimizing imports. But it is good practice to avoid importing entire package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I'll correct that.

import java.nio.file.attribute.BasicFileAttributes;
import java.util.Base64;
import java.util.Map;
import java.util.function.Predicate;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;

import static org.openqa.selenium.remote.Browser.FIREFOX;

Expand Down Expand Up @@ -77,7 +80,23 @@ public String installExtension(Path path, Boolean temporary) {

String encoded;
try {
encoded = Base64.getEncoder().encodeToString(Files.readAllBytes(path));
if (Files.isDirectory(path)){
Path extZip = Paths.get(path.getFileName().toString()+".zip");
ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(extZip.toFile()));
Files.walkFileTree(path, new SimpleFileVisitor<Path>() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fine if Path is not passed explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing Path is breaking the code. I'll look into it further.

public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
zos.putNextEntry(new ZipEntry(path.relativize(file).toString()));
Files.copy(file, zos);
zos.closeEntry();
return FileVisitResult.CONTINUE;
}
});
zos.close();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be done in a Try with Resources block?
Maybe we can extract those lines for the unzipping into another method so it looks more readable.

Copy link
Contributor Author

@TamsilAmani TamsilAmani Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorporated the changes. Please check to verify and resolve :)

encoded = Base64.getEncoder().encodeToString(Files.readAllBytes(extZip));
}
else{
encoded = Base64.getEncoder().encodeToString(Files.readAllBytes(path));
}
} catch (IOException e) {
throw new InvalidArgumentException(path + " is an invalid path", e);
}
Expand Down
3 changes: 2 additions & 1 deletion java/test/org/openqa/selenium/firefox/FirefoxDriverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
public class FirefoxDriverTest extends JUnit4TestBase {

private static final String EXT_PATH = "common/extensions/webextensions-selenium-example.xpi";
private static final String EXT_PATH_DIR = "common/extensions/webextensions-selenium-example";
private static char[] CHARS =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890!\"§$%&/()+*~#',.-_:;\\"
.toCharArray();
Expand Down Expand Up @@ -544,7 +545,7 @@ public void canAddRemoveExtensions() {

@Test
public void canAddRemoveTempExtensions() {
Path extension = InProject.locate(EXT_PATH);
Path extension = InProject.locate(EXT_PATH_DIR);

String id = ((HasExtensions) driver).installExtension(extension, true);
assertThat(id).isEqualTo("webextensions-selenium-example@example.com");
Expand Down