Fix Preferences related tests, document limitations.

- The default Preferences factory methods are entirely
  useless on android. "java.home" points to a location
  in a read-only partition and "user.home" is set to "/"
  which apps are not allowed to write to. A strong warning
  has been added against these methods.

- We continue to support the underlying default preferences
  implementation (even though there's no way to use the impl.
  directly except by reflection) to help apps transition *just
  in case* they were manually massing "user.home" and "java.home".

- Add a fake preferences factory that allows us to
  use a new (temporary) preferences root for every test.
  Fixed a test interdependency issue that was uncovered
  by this change.

Change-Id: I6019441c65b398376d84264f1e4c9776507533eb

6 files changed, 115 insertions, 29 deletions

diff --git a/luni/src/main/java/java/util/prefs/FilePreferencesFactoryImpl.java b/luni/src/main/java/java/util/prefs/FilePreferencesFactoryImpl.java
index 8f2d0aa..154c71e 100644
--- a/luni/src/main/java/java/util/prefs/FilePreferencesFactoryImpl.java
+++ b/luni/src/main/java/java/util/prefs/FilePreferencesFactoryImpl.java
@@ -24,10 +24,12 @@ package java.util.prefs;
  */
 class FilePreferencesFactoryImpl implements PreferencesFactory {
     //  user root preferences
-    private static final Preferences USER_ROOT = new FilePreferencesImpl(true);
+    private static final Preferences USER_ROOT = new FilePreferencesImpl(
+            System.getProperty("user.home") + "/.java/.userPrefs", true);
 
     //  system root preferences
-    private static final Preferences SYSTEM_ROOT = new FilePreferencesImpl(false);
+    private static final Preferences SYSTEM_ROOT = new FilePreferencesImpl(
+            System.getProperty("java.home") + "/.systemPrefs", false);
 
     public FilePreferencesFactoryImpl() {
     }
@@ -39,5 +41,4 @@ class FilePreferencesFactoryImpl implements PreferencesFactory {
     public Preferences systemRoot() {
         return SYSTEM_ROOT;
     }
-
 }
diff --git a/luni/src/main/java/java/util/prefs/FilePreferencesImpl.java b/luni/src/main/java/java/util/prefs/FilePreferencesImpl.java
index bd367a6..de1ead4 100644
--- a/luni/src/main/java/java/util/prefs/FilePreferencesImpl.java
+++ b/luni/src/main/java/java/util/prefs/FilePreferencesImpl.java
@@ -30,20 +30,15 @@ import java.util.Set;
  * TODO some sync mechanism with backend, Performance - check file edit date
  *
  * @since 1.4
+ *
+ * @hide
  */
-class FilePreferencesImpl extends AbstractPreferences {
-
+public class FilePreferencesImpl extends AbstractPreferences {
     //prefs file name
     private static final String PREFS_FILE_NAME = "prefs.xml";
 
-    //home directory for user prefs
-    private static String USER_HOME = System.getProperty("user.home") + "/.java/.userPrefs";
-
-    //home directory for system prefs
-    private static String SYSTEM_HOME = System.getProperty("java.home") + "/.systemPrefs";
-
     //file path for this preferences node
-    private String path;
+    private final String path;
 
     //internal cache for prefs key-value pair
     private Properties prefs;
@@ -67,13 +62,15 @@ class FilePreferencesImpl extends AbstractPreferences {
      */
 
     /**
-     * Construct root <code>FilePreferencesImpl</code> instance, construct
-     * user root if userNode is true, system root otherwise
+     * Construct root <code>FilePreferencesImpl</code> instance rooted
+     * at the given path.
+     *
+     * @hide
      */
-    FilePreferencesImpl(boolean userNode) {
+    public FilePreferencesImpl(String path, boolean isUserNode) {
         super(null, "");
-        this.userNode = userNode;
-        path = userNode ? USER_HOME : SYSTEM_HOME;
+        this.path = path;
+        this.userNode = isUserNode;
         initPrefs();
     }
 
diff --git a/luni/src/main/java/java/util/prefs/Preferences.java b/luni/src/main/java/java/util/prefs/Preferences.java
index b808052..342be70 100644
--- a/luni/src/main/java/java/util/prefs/Preferences.java
+++ b/luni/src/main/java/java/util/prefs/Preferences.java
@@ -98,8 +98,17 @@ public abstract class Preferences {
      */
     public static final int MAX_VALUE_LENGTH = 8192;
 
-    //factory used to get user/system prefs root
-    private static final PreferencesFactory factory = findPreferencesFactory();
+    // factory used to get user/system prefs root
+    private static volatile PreferencesFactory factory = findPreferencesFactory();
+
+    /**
+     * @hide for testing only.
+     */
+    public static PreferencesFactory setPreferencesFactory(PreferencesFactory pf) {
+        PreferencesFactory previous = factory;
+        factory = pf;
+        return previous;
+    }
 
     private static PreferencesFactory findPreferencesFactory() {
         // Try the system property first...
@@ -780,6 +789,11 @@ public abstract class Preferences {
     public abstract void sync() throws BackingStoreException;
 
     /**
+     * <strong>Legacy code; do not use.</strong> On Android, the Preference nodes
+     * corresponding to the "system" and "user" preferences are stored in sections
+     * of the file system that are inaccessible to apps. Further, allowing apps to set
+     * "system wide" preferences is contrary to android's security model.
+     *
      * Returns the system preference node for the package of the given class.
      * The absolute path of the returned node is one slash followed by the given
      * class's full package name, replacing each period character ('.') with
@@ -796,11 +810,16 @@ public abstract class Preferences {
      * @throws NullPointerException
      *             if the given class is {@code null}.
      */
-    public static Preferences systemNodeForPackage (Class<?> c) {
+    public static Preferences systemNodeForPackage(Class<?> c) {
         return factory.systemRoot().node(getNodeName(c));
     }
 
     /**
+     * <strong>Legacy code; do not use.</strong> On Android, the Preference nodes
+     * corresponding to the "system" and "user" preferences are stored in sections
+     * of the file system that are inaccessible to apps. Further, allowing apps to set
+     * "system wide" preferences is contrary to android's security model.
+     *
      * Returns the root node of the system preference hierarchy.
      *
      * @return the system preference hierarchy root node.
@@ -810,6 +829,13 @@ public abstract class Preferences {
     }
 
     /**
+     *
+     * <strong>Legacy code; do not use.</strong> On Android, the Preference nodes
+     * corresponding to the "system" and "user" preferences are stored in sections
+     * of the file system that are inaccessible to apps. Further, allowing apps to set
+     * "system wide" preferences is contrary to android's security model.
+     *
+     * <p>
      * Returns the user preference node for the package of the given class.
      * The absolute path of the returned node is one slash followed by the given
      * class's full package name, replacing each period character ('.') with
@@ -820,13 +846,11 @@ public abstract class Preferences {
      * by this method won't necessarily be persisted until the method {@code
      * flush()} is invoked.
      *
-     * @param c
-     *            the given class.
      * @return the user preference node for the package of the given class.
      * @throws NullPointerException
      *             if the given class is {@code null}.
      */
-    public static Preferences userNodeForPackage (Class<?> c) {
+    public static Preferences userNodeForPackage(Class<?> c) {
         return factory.userRoot().node(getNodeName(c));
     }
 
@@ -840,6 +864,11 @@ public abstract class Preferences {
     }
 
     /**
+     * <strong>Legacy code; do not use.</strong> On Android, the Preference nodes
+     * corresponding to the "system" and "user" preferences are stored in sections
+     * of the file system that are inaccessible to apps. Further, allowing apps to set
+     * "system wide" preferences is contrary to android's security model.
+     *
      * Returns the root node of the user preference hierarchy.
      *
      * @return the user preference hierarchy root node.
diff --git a/luni/src/test/java/libcore/java/util/prefs/OldAbstractPreferencesTest.java b/luni/src/test/java/libcore/java/util/prefs/OldAbstractPreferencesTest.java
index b9d3f1d..6384059 100644
--- a/luni/src/test/java/libcore/java/util/prefs/OldAbstractPreferencesTest.java
+++ b/luni/src/test/java/libcore/java/util/prefs/OldAbstractPreferencesTest.java
@@ -18,6 +18,7 @@ package libcore.java.util.prefs;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.File;
 import java.io.IOException;
 import java.util.prefs.AbstractPreferences;
 import java.util.prefs.BackingStoreException;
@@ -28,6 +29,7 @@ import java.util.prefs.PreferenceChangeEvent;
 import java.util.prefs.PreferenceChangeListener;
 import java.util.prefs.Preferences;
 import junit.framework.TestCase;
+import libcore.io.IoUtils;
 
 public final class OldAbstractPreferencesTest extends TestCase {
 
@@ -40,11 +42,13 @@ public final class OldAbstractPreferencesTest extends TestCase {
     protected void setUp() throws Exception {
         super.setUp();
 
+        File rootDir = IoUtils.createTemporaryDirectory("OldAbstractPreferencesTest");
+        Preferences.setPreferencesFactory(
+                new PreferencesTest.TestPreferencesFactory(rootDir.getAbsolutePath()));
+
         root = (AbstractPreferences) Preferences.userRoot();
-        for (String child : root.childrenNames()) {
-            root.node(child).removeNode();
-        }
-        root.clear();
+        assertEquals(0, root.childrenNames().length);
+        assertEquals(0, root.keys().length);
 
         parent = (AbstractPreferences) Preferences.userNodeForPackage(getClass());
         pref = (AbstractPreferences) parent.node(nodeName);
diff --git a/luni/src/test/java/libcore/java/util/prefs/OldPreferencesTest.java b/luni/src/test/java/libcore/java/util/prefs/OldPreferencesTest.java
index f8a8154..7245af7 100644
--- a/luni/src/test/java/libcore/java/util/prefs/OldPreferencesTest.java
+++ b/luni/src/test/java/libcore/java/util/prefs/OldPreferencesTest.java
@@ -16,6 +16,7 @@
 
 package libcore.java.util.prefs;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.util.Arrays;
@@ -25,7 +26,9 @@ import java.util.prefs.NodeChangeListener;
 import java.util.prefs.PreferenceChangeEvent;
 import java.util.prefs.PreferenceChangeListener;
 import java.util.prefs.Preferences;
+import java.util.prefs.PreferencesFactory;
 import junit.framework.TestCase;
+import libcore.io.IoUtils;
 
 public final class OldPreferencesTest extends TestCase {
 
@@ -48,8 +51,14 @@ public final class OldPreferencesTest extends TestCase {
         longValue = value.toString();
     }
 
-    @Override protected void setUp() throws Exception {
+    private PreferencesFactory defaultFactory;
+
+    @Override
+    protected void setUp() throws Exception {
         super.setUp();
+        final File tmpDir = IoUtils.createTemporaryDirectory("OldPreferenceTest");
+        defaultFactory = Preferences.setPreferencesFactory(
+                new PreferencesTest.TestPreferencesFactory(tmpDir.getAbsolutePath()));
 
         Preferences pref = Preferences.userNodeForPackage(Preferences.class);
         for (String child : pref.childrenNames()) {
@@ -58,6 +67,11 @@ public final class OldPreferencesTest extends TestCase {
         pref.clear();
     }
 
+    @Override
+    protected void tearDown() throws Exception {
+        Preferences.setPreferencesFactory(defaultFactory);
+    }
+
     public void testAbstractMethods() throws IOException, BackingStoreException {
         Preferences p = new MockPreferences();
         p.absolutePath();
diff --git a/luni/src/test/java/libcore/java/util/prefs/PreferencesTest.java b/luni/src/test/java/libcore/java/util/prefs/PreferencesTest.java
index 1560fbe..6c57ef9 100644
--- a/luni/src/test/java/libcore/java/util/prefs/PreferencesTest.java
+++ b/luni/src/test/java/libcore/java/util/prefs/PreferencesTest.java
@@ -19,17 +19,58 @@ package libcore.java.util.prefs;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileWriter;
+import java.util.prefs.FilePreferencesImpl;
 import java.util.prefs.Preferences;
+import java.util.prefs.PreferencesFactory;
 import junit.framework.TestCase;
+import libcore.io.IoUtils;
 
 public final class PreferencesTest extends TestCase {
 
     /**
+     * A preferences factory rooted at a given path.
+     */
+    public static final class TestPreferencesFactory implements PreferencesFactory {
+        private final Preferences userPrefs;
+        private final Preferences systemPrefs;
+
+        public TestPreferencesFactory(String root) {
+            userPrefs = new FilePreferencesImpl(root + "/user", true);
+            systemPrefs = new FilePreferencesImpl(root + "/system", false);
+        }
+
+        public Preferences userRoot() {
+            return userPrefs;
+        }
+
+        public Preferences systemRoot() {
+            return systemPrefs;
+        }
+    }
+
+    private PreferencesFactory defaultFactory;
+    private File temporaryDirectory;
+
+    @Override
+    public void setUp() throws Exception {
+        temporaryDirectory = IoUtils.createTemporaryDirectory("PreferencesTest");
+        defaultFactory = Preferences.setPreferencesFactory(
+                new TestPreferencesFactory(temporaryDirectory.getAbsolutePath()));
+    }
+
+    @Override
+    public void tearDown() throws Exception {
+        Preferences.setPreferencesFactory(defaultFactory);
+    }
+
+    /**
      * The preferences API is designed to be hostile towards files that exist
      * where it wants to store its XML data. http://b/3431233
      */
     public void testPreferencesClobbersExistingFiles() throws Exception {
-        File userPrefs = new File(System.getProperty("user.home") + "/.java/.userPrefs/prefs.xml");
+        final File userPrefsDir = new File(temporaryDirectory + "/user");
+        final File userPrefs = new File(userPrefsDir, "prefs.xml");
+        assertTrue(userPrefs.createNewFile());
         FileWriter writer = new FileWriter(userPrefs);
         writer.write("lamb");
         writer.close();