Permalink
Browse files

Fixing OperaProfile to work with RemoteWebDriver

Currently fromJson is never called, because the base64-encoded JSON
string in {opera.profile: "foobar="} is a String, so is treated like a
file path by OperaSettings.PROFILE.sanitize.  Instead, encode the
base64-encoded string as a value in a map, so we can distinguish between
it and a file path.

Also, when using the base64-encoded string, currently we create a
TemporaryFileSystem, delete it, and seem to try to access the deleted
file somewhere; instead, just read the file once in to a buffer, and
manipulate that as needed.  Note: I'm assuming that the preferences file
is always stored as UTF-8, as is implied by the comment in
OperaFilePreferences.

And a little tidying up here and there.

Tested by running `ant test` - the same failures exist as in a clean
client, see issue 76.  Added a test that profiles can be decoded
properly.  Manually tested that a profile can be properly loaded through
a RemoteWebDriver server.

Note that this commit technically breaks wire-compatibility from
previously released versions, but the previous versions suffered from a
bug meaning that this never actually worked, so no clients should be
broken by this change (that weren't broken already).
  • Loading branch information...
1 parent c991c0c commit bdd7d5d3178858207e4d0215881c9c7a168430b6 @illicitonion committed May 15, 2012
@@ -16,17 +16,20 @@
package com.opera.core.systems;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.opera.core.systems.preferences.OperaFilePreferences;
import com.opera.core.systems.preferences.OperaPreferences;
+import org.json.JSONObject;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.io.TemporaryFilesystem;
import org.openqa.selenium.io.Zip;
import java.io.File;
import java.io.IOException;
-import java.util.Collection;
-import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
import static com.google.common.base.Preconditions.checkArgument;
@@ -65,6 +68,7 @@
*/
public class OperaProfile {
+ public static final String BASE64_JSON_KEY = "base64";
private File directory;
private File preferenceFile;
private boolean randomProfile = false;
@@ -169,8 +173,13 @@ public void cleanUp() {
* @return the JSON representation of this profile
* @throws IOException if an I/O error occurs
*/
- public String toJson() throws IOException {
- return new Zip().zip(getDirectory());
+ public JSONObject toJson() throws IOException {
+ String base64 = new Zip().zip(getDirectory());
+ Map<String, String> map = ImmutableMap.of(
+ "className", this.getClass().getName(),
+ BASE64_JSON_KEY, base64);
+
+ return new JSONObject(map);
}
/**
@@ -196,22 +205,18 @@ public static OperaProfile fromJson(String json) throws IOException {
* @param directory the directory to look for a preference file
* @return a preference file
*/
- private static File getPreferenceFile(File directory) {
- final String directoryPath = directory.getAbsolutePath() + File.separator;
-
- @SuppressWarnings("serial")
- Collection<File> candidates = new LinkedHashSet<File>() {{
- add(new File(directoryPath + "operaprefs.ini"));
- add(new File(directoryPath + "opera.ini"));
- }};
+ private static File getPreferenceFile(final File directory) {
+ List<File> candidates = ImmutableList.of(
+ new File(directory, "operaprefs.ini"),
+ new File(directory, "opera.ini"));
for (File candidate : candidates) {
if (candidate.exists()) {
return candidate;
}
}
- return new File(directoryPath + "operaprefs.ini");
+ return candidates.get(0);
}
}
@@ -17,6 +17,7 @@
package com.opera.core.systems;
import com.google.common.base.CaseFormat;
+import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableMap;
import com.opera.core.systems.arguments.OperaCoreArguments;
@@ -277,6 +278,16 @@ Object sanitize(Object value) {
}
} else if (value instanceof OperaProfile) {
return value;
+ } else if (value instanceof Map) {
+ @SuppressWarnings("rawtypes")
+ Map map = (Map)value;
+ if (map.containsKey("base64")) {
+ try {
+ return OperaProfile.fromJson((String)map.get("base64"));
+ } catch (IOException e) {
+ Throwables.propagate(e);
+ }
+ }
}
return new OperaProfile();
@@ -20,15 +20,18 @@
import org.ini4j.Profile;
import org.ini4j.Wini;
import org.openqa.selenium.WebDriverException;
-import org.openqa.selenium.io.TemporaryFilesystem;
-import java.io.BufferedReader;
-import java.io.BufferedWriter;
+import com.google.common.base.Charsets;
+import com.google.common.base.Joiner;
+import com.google.common.base.Predicate;
+import com.google.common.base.Throwables;
+import com.google.common.collect.Iterables;
+import com.google.common.io.Files;
+
import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.FileReader;
-import java.io.FileWriter;
import java.io.IOException;
+import java.io.StringReader;
+import java.util.List;
import java.util.Map;
/**
@@ -61,6 +64,17 @@ public OperaFilePreferences(File preferenceFile) {
return;
}
+ Ini ini = getIniForPreferenceFile(preferenceFile);
+
+ // Add each preference entry
+ for (Map.Entry<String, Profile.Section> section : ini.entrySet()) {
+ for (Map.Entry<String, String> entry : section.getValue().entrySet()) {
+ set(section.getValue().getName(), entry.getKey(), entry.getValue());
+ }
+ }
+ }
+
+ private Ini getIniForPreferenceFile(File preferenceFile) {
// Due to the sucky nature of Opera's invalid preference files, we are forced to remove the
// first line of the file.
//
@@ -76,47 +90,17 @@ public OperaFilePreferences(File preferenceFile) {
//
// &c.
- TemporaryFilesystem fs = null;
- File temporaryPreferenceFile;
- Ini ini;
-
try {
- fs = TemporaryFilesystem.getDefaultTmpFS();
- temporaryPreferenceFile = new File(fs.createTempDir("operadriver", "preferences")
- .getAbsolutePath() + File.separator + "opera.ini");
-
- BufferedReader reader = new BufferedReader(new FileReader(preferenceFile));
- BufferedWriter writer = new BufferedWriter(new FileWriter(temporaryPreferenceFile));
-
- String newLine = System.getProperty("line.separator");
- String currentLine;
- while ((currentLine = reader.readLine()) != null) {
- if (!currentLine.contains("Opera Preferences version")) {
- writer.write(currentLine + newLine);
+ List<String> lines = Files.readLines(preferenceFile, Charsets.UTF_8);
+ Iterable<String> filteredLines = Iterables.filter(lines, new Predicate<String>() {
+ public boolean apply(String line) {
+ return !line.contains("Opera Preferences version");
}
- }
-
- // Make sure channels are closed so that last line is flushed
- reader.close();
- writer.close();
-
- // Read new preference file
- ini = new Ini(temporaryPreferenceFile);
- } catch (FileNotFoundException e) {
- throw new WebDriverException("Unknown file: " + preferenceFile.getAbsolutePath());
+ });
+
+ return new Ini(new StringReader(Joiner.on("\n").join(filteredLines)));
} catch (IOException e) {
- throw new WebDriverException("Unable to read file: " + preferenceFile.getPath());
- } finally {
- if (fs != null) {
- fs.deleteTemporaryFiles();
- }
- }
-
- // Add each preference entry
- for (Map.Entry<String, Profile.Section> section : ini.entrySet()) {
- for (Map.Entry<String, String> entry : section.getValue().entrySet()) {
- set(section.getValue().getName(), entry.getKey(), entry.getValue());
- }
+ throw Throwables.propagate(e);
}
}
@@ -16,6 +16,7 @@
package com.opera.core.systems;
+import com.google.common.collect.ImmutableMap;
import com.google.common.io.Files;
import com.opera.core.systems.testing.NoDriver;
@@ -24,14 +25,18 @@
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
+import org.json.JSONException;
+import org.json.JSONObject;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.openqa.selenium.io.Zip;
+import org.openqa.selenium.remote.DesiredCapabilities;
import java.io.File;
import java.io.IOException;
+import java.util.Map;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
@@ -41,6 +46,8 @@
@NoDriver
public class OperaProfileTest extends OperaDriverTestCase {
+ private static final String VALID_BASE64_ENCODED_PROFILE =
+ "UEsDBBQACAAIAGWKr0AAAAAAAAAAAAAAAAAOAAAAb3BlcmFwcmVmcy5pbmmLDi1OLVIIKEpNK47l\n8kzPyy9KVQjNK0otLE0tLklNUQjILygtKFawVTDg4gIAUEsHCExnhFouAAAALAAAAFBLAQIUABQA\nCAAIAGWKr0BMZ4RaLgAAACwAAAAOAAAAAAAAAAAAAAAAAAAAAABvcGVyYXByZWZzLmluaVBLBQYA\nAAAAAQABADwAAABqAAAAAAA=";
public OperaProfile profile;
public File temporaryProfile;
public File existingProfile;
@@ -138,30 +145,41 @@ public void testPreferences() {
}
@Test
- public void testToJson() throws IOException {
+ public void testToJson() throws IOException, JSONException {
profile = new OperaProfile(existingProfile);
- new Zip().unzip(profile.toJson(), temporaryProfile);
+ String json = profile.toJson().getString(OperaProfile.BASE64_JSON_KEY);
+ new Zip().unzip(json, temporaryProfile);
OperaProfile extractedProfile = new OperaProfile(temporaryProfile.getPath());
assertThat(profile, matchesProfile(extractedProfile));
}
@Test
- public void testFromJson() throws IOException {
+ public void testFromJson() throws IOException, JSONException {
OperaProfile data = new OperaProfile(existingProfile);
- profile = TestOperaProfile.fromJson(temporaryProfile, data.toJson());
+ profile = TestOperaProfile.fromJson(temporaryProfile, data.toJson().getString(OperaProfile.BASE64_JSON_KEY));
assertThat(profile, matchesProfile(data));
}
+ @Test
+ public void fromCapabilitiesWithProfile() throws Throwable {
+ DesiredCapabilities caps = DesiredCapabilities.opera();
+ Map<String, String> profileMap = ImmutableMap.of(OperaProfile.BASE64_JSON_KEY, VALID_BASE64_ENCODED_PROFILE);
+ caps.setCapability("opera.profile", profileMap);
+
+ // Shouldn't throw
+ new OperaSettings().merge(caps);
+ }
+
private Matcher matchesProfile(final OperaProfile expected) {
return new BaseMatcher() {
public boolean matches(Object o) {
OperaProfile actual = (OperaProfile) o;
try {
- if ((expected.toJson().equals(actual.toJson())) &&
+ if ((expected.toJson().toString().equals(actual.toJson().toString())) &&
(expected.preferences().size() == actual.preferences().size())) {
return true;
}

0 comments on commit bdd7d5d

Please sign in to comment.