From 5ffec23b2f3d367e8e04d1462357b1aca441d38e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 13 Jun 2018 14:15:42 +0200 Subject: [PATCH] set default values of preferences only in preferences.xml This removes a layer of redundancy where there were defaults set in the Preferences class, as well as in preferences.xml. This makes it possible for whitelabel versions to change the default values of the preferences by changing it only in preferences.xml. --- app/src/basic/res/xml/preferences.xml | 3 + .../java/org/fdroid/fdroid/Preferences.java | 112 +++++++------ .../views/fragments/PreferencesFragment.java | 3 - app/src/main/res/values/donottranslate.xml | 4 + app/src/main/res/xml/preferences.xml | 3 + .../org/fdroid/fdroid/PreferencesTest.java | 156 ++++++++++++++++++ 6 files changed, 230 insertions(+), 51 deletions(-) create mode 100644 app/src/test/java/org/fdroid/fdroid/PreferencesTest.java diff --git a/app/src/basic/res/xml/preferences.xml b/app/src/basic/res/xml/preferences.xml index d1a123d4f..c1e62f5ca 100644 --- a/app/src/basic/res/xml/preferences.xml +++ b/app/src/basic/res/xml/preferences.xml @@ -30,10 +30,12 @@ + * Copyright (C) 2013-2016 Daniel Martí + * Copyright (C) 2014-2018 Hans-Christoph Steiner + * Copyright (C) 2018 Senecto Limited + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 3 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + package org.fdroid.fdroid; import android.annotation.SuppressLint; @@ -26,6 +48,14 @@ import java.util.concurrent.TimeUnit; * (using {@link Preferences#setup(android.content.Context)} before it gets * accessed via the {@link org.fdroid.fdroid.Preferences#get()} * singleton method. + *

+ * All defaults should be set in {@code res/xml/preferences.xml}. The one + * exception is {@link Preferences#PREF_LOCAL_REPO_NAME} since it needs to be + * generated per install. The preferences are only written out explicitly when + * the user changes the preferences. So the default values need to be reloaded + * every time F-Droid starts. The various {@link SharedPreferences} getters are + * using {@code false} and {@code -1} as fallback default values to help catch + * problems with the proper default loading as quickly as possible. */ public final class Preferences implements SharedPreferences.OnSharedPreferenceChangeListener { @@ -34,6 +64,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh private final SharedPreferences preferences; private Preferences(Context context) { + PreferenceManager.setDefaultValues(context, R.xml.preferences, true); preferences = PreferenceManager.getDefaultSharedPreferences(context); preferences.registerOnSharedPreferenceChangeListener(this); if (preferences.getString(PREF_LOCAL_REPO_NAME, null) == null) { @@ -79,30 +110,16 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh public static final int OVER_NETWORK_ON_DEMAND = 1; public static final int OVER_NETWORK_ALWAYS = 2; - private static final boolean DEFAULT_SHOW_INCOMPAT_VERSIONS = false; - private static final boolean DEFAULT_SHOW_ROOT_APPS = true; - private static final boolean DEFAULT_SHOW_ANTI_FEATURE_APPS = true; - public static final int DEFAULT_OVER_WIFI = OVER_NETWORK_ALWAYS; - public static final int DEFAULT_OVER_DATA = OVER_NETWORK_ON_DEMAND; - public static final int DEFAULT_UPDATE_INTERVAL = 3; - private static final boolean DEFAULT_PRIVILEGED_INSTALLER = true; - //private static final boolean DEFAULT_LOCAL_REPO_BONJOUR = true; - private static final long DEFAULT_KEEP_CACHE_TIME = TimeUnit.DAYS.toMillis(1); - private static final boolean DEFAULT_UNSTABLE_UPDATES = false; - private static final boolean DEFAULT_KEEP_INSTALL_HISTORY = false; - //private static final boolean DEFAULT_LOCAL_REPO_HTTPS = false; - private static final boolean DEFAULT_EXPERT = false; - private static final boolean DEFAULT_ENABLE_PROXY = false; - public static final String DEFAULT_THEME = "light"; + // these preferences are not listed in preferences.xml so the defaults are set here @SuppressWarnings("PMD.AvoidUsingHardCodedIP") - public static final String DEFAULT_PROXY_HOST = "127.0.0.1"; - public static final int DEFAULT_PROXY_PORT = 8118; + public static final String DEFAULT_PROXY_HOST = "127.0.0.1"; // TODO move to preferences.xml + public static final int DEFAULT_PROXY_PORT = 8118; // TODO move to preferences.xml private static final boolean DEFAULT_SHOW_NFC_DURING_SWAP = true; - private static final boolean DEFAULT_FORCE_OLD_INDEX = false; private static final boolean DEFAULT_POST_PRIVILEGED_INSTALL = false; - private static final boolean DEFAULT_PREVENT_SCREENSHOTS = false; private static final boolean DEFAULT_PANIC_EXIT = true; - private static final boolean DEFAULT_HIDE_ON_LONG_PRESS_SEARCH = false; + + private static final boolean IGNORED_B = false; + private static final int IGNORED_I = -1; @Deprecated private static final String OLD_PREF_UPDATE_INTERVAL = "updateInterval"; @@ -126,8 +143,8 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh DateUtils.HOUR_IN_MILLIS, }; - private boolean showAppsRequiringRoot = DEFAULT_SHOW_ROOT_APPS; - private boolean showAppsWithAntiFeatures = DEFAULT_SHOW_ANTI_FEATURE_APPS; + private boolean showAppsRequiringRoot; + private boolean showAppsWithAntiFeatures; private final Map initialized = new HashMap<>(); @@ -150,7 +167,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public boolean isForceOldIndexEnabled() { - return preferences.getBoolean(PREF_FORCE_OLD_INDEX, DEFAULT_FORCE_OLD_INDEX); + return preferences.getBoolean(PREF_FORCE_OLD_INDEX, IGNORED_B); } public void setForceOldIndex(boolean flag) { @@ -166,7 +183,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh * @see org.fdroid.fdroid.views.fragments.PreferencesFragment#initPrivilegedInstallerPreference() */ public boolean isPrivilegedInstallerEnabled() { - return preferences.getBoolean(PREF_PRIVILEGED_INSTALLER, DEFAULT_PRIVILEGED_INSTALLER); + return preferences.getBoolean(PREF_PRIVILEGED_INSTALLER, IGNORED_B); } public boolean isPostPrivilegedInstall() { @@ -189,7 +206,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh if (getOverData() == OVER_NETWORK_NEVER && getOverWifi() == OVER_NETWORK_NEVER) { return UPDATE_INTERVAL_VALUES[0]; } else { - int position = preferences.getInt(PREF_UPDATE_INTERVAL, DEFAULT_UPDATE_INTERVAL); + int position = preferences.getInt(PREF_UPDATE_INTERVAL, IGNORED_I); return UPDATE_INTERVAL_VALUES[position]; } } @@ -215,7 +232,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh if (!preferences.contains(OLD_PREF_UPDATE_INTERVAL)) { return false; // already completed } - int updateInterval = DEFAULT_UPDATE_INTERVAL; + int updateInterval = 3; String value = preferences.getString(OLD_PREF_UPDATE_INTERVAL, String.valueOf(24)); if ("1".equals(value)) { // 1 hour updateInterval = 6; @@ -265,8 +282,8 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh * Time in millis to keep cached files. Anything that has been around longer will be deleted */ public long getKeepCacheTime() { - String value = preferences.getString(PREF_KEEP_CACHE_TIME, - String.valueOf(DEFAULT_KEEP_CACHE_TIME)); + String value = preferences.getString(PREF_KEEP_CACHE_TIME, null); + long fallbackValue = TimeUnit.DAYS.toMillis(1); // the first time this was migrated, it was botched, so reset to default switch (value) { @@ -279,7 +296,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh SharedPreferences.Editor editor = preferences.edit(); editor.remove(PREF_KEEP_CACHE_TIME); editor.apply(); - return Preferences.DEFAULT_KEEP_CACHE_TIME; + return fallbackValue; } if (preferences.contains(PREF_CACHE_APK)) { @@ -295,7 +312,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh try { return Long.parseLong(value); } catch (NumberFormatException e) { - return DEFAULT_KEEP_CACHE_TIME; + return fallbackValue; } } @@ -306,7 +323,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh * asking), or whether there is no apps because we have never actually asked to update the repos. */ public boolean hasTriedEmptyUpdate() { - return preferences.getBoolean(PREF_TRIED_EMPTY_UPDATE, false); + return preferences.getBoolean(PREF_TRIED_EMPTY_UPDATE, IGNORED_B); } public void setTriedEmptyUpdate(boolean value) { @@ -314,7 +331,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public boolean getUnstableUpdates() { - return preferences.getBoolean(PREF_UNSTABLE_UPDATES, DEFAULT_UNSTABLE_UPDATES); + return preferences.getBoolean(PREF_UNSTABLE_UPDATES, IGNORED_B); } public void setUnstableUpdates(boolean value) { @@ -322,11 +339,11 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public boolean isKeepingInstallHistory() { - return preferences.getBoolean(PREF_KEEP_INSTALL_HISTORY, DEFAULT_KEEP_INSTALL_HISTORY); + return preferences.getBoolean(PREF_KEEP_INSTALL_HISTORY, IGNORED_B); } public boolean showIncompatibleVersions() { - return preferences.getBoolean(PREF_SHOW_INCOMPAT_VERSIONS, DEFAULT_SHOW_INCOMPAT_VERSIONS); + return preferences.getBoolean(PREF_SHOW_INCOMPAT_VERSIONS, IGNORED_B); } public boolean showNfcDuringSwap() { @@ -338,7 +355,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public boolean expertMode() { - return preferences.getBoolean(PREF_EXPERT, DEFAULT_EXPERT); + return preferences.getBoolean(PREF_EXPERT, IGNORED_B); } public void setExpertMode(boolean flag) { @@ -346,11 +363,11 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public boolean forceTouchApps() { - return preferences.getBoolean(Preferences.PREF_FORCE_TOUCH_APPS, false); + return preferences.getBoolean(Preferences.PREF_FORCE_TOUCH_APPS, IGNORED_B); } public Theme getTheme() { - return Theme.valueOf(preferences.getString(Preferences.PREF_THEME, Preferences.DEFAULT_THEME)); + return Theme.valueOf(preferences.getString(Preferences.PREF_THEME, null)); } public boolean isLocalRepoHttpsEnabled() { @@ -379,7 +396,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public boolean isAutoDownloadEnabled() { - return preferences.getBoolean(PREF_AUTO_DOWNLOAD_INSTALL_UPDATES, false); + return preferences.getBoolean(PREF_AUTO_DOWNLOAD_INSTALL_UPDATES, IGNORED_B); } /** @@ -405,11 +422,11 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public int getOverWifi() { - return preferences.getInt(PREF_OVER_WIFI, DEFAULT_OVER_WIFI); + return preferences.getInt(PREF_OVER_WIFI, IGNORED_I); } public int getOverData() { - return preferences.getInt(PREF_OVER_DATA, DEFAULT_OVER_DATA); + return preferences.getInt(PREF_OVER_DATA, IGNORED_I); } /** @@ -419,11 +436,11 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh public boolean isTorEnabled() { // TODO enable once Orbot can auto-start after first install //return preferences.getBoolean(PREF_USE_TOR, OrbotHelper.requestStartTor(context)); - return preferences.getBoolean(PREF_USE_TOR, false); + return preferences.getBoolean(PREF_USE_TOR, IGNORED_B); } private boolean isProxyEnabled() { - return preferences.getBoolean(PREF_ENABLE_PROXY, DEFAULT_ENABLE_PROXY); + return preferences.getBoolean(PREF_ENABLE_PROXY, IGNORED_B); } /** @@ -457,7 +474,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public boolean preventScreenshots() { - return preferences.getBoolean(PREF_PREVENT_SCREENSHOTS, DEFAULT_PREVENT_SCREENSHOTS); + return preferences.getBoolean(PREF_PREVENT_SCREENSHOTS, IGNORED_B); } public boolean panicExit() { @@ -465,11 +482,11 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } public boolean panicHide() { - return preferences.getBoolean(PREF_PANIC_HIDE, false); + return preferences.getBoolean(PREF_PANIC_HIDE, IGNORED_B); } public boolean hideOnLongPressSearch() { - return preferences.getBoolean(PREF_HIDE_ON_LONG_PRESS_SEARCH, DEFAULT_HIDE_ON_LONG_PRESS_SEARCH); + return preferences.getBoolean(PREF_HIDE_ON_LONG_PRESS_SEARCH, IGNORED_B); } /** @@ -481,7 +498,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh public boolean showAppsRequiringRoot() { if (!isInitialized(PREF_SHOW_ROOT_APPS)) { initialize(PREF_SHOW_ROOT_APPS); - showAppsRequiringRoot = preferences.getBoolean(PREF_SHOW_ROOT_APPS, DEFAULT_SHOW_ROOT_APPS); + showAppsRequiringRoot = preferences.getBoolean(PREF_SHOW_ROOT_APPS, IGNORED_B); } return showAppsRequiringRoot; } @@ -500,8 +517,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh } if (!isInitialized(PREF_SHOW_ANTI_FEATURE_APPS)) { initialize(PREF_SHOW_ANTI_FEATURE_APPS); - showAppsWithAntiFeatures = preferences.getBoolean(PREF_SHOW_ANTI_FEATURE_APPS, - DEFAULT_SHOW_ANTI_FEATURE_APPS); + showAppsWithAntiFeatures = preferences.getBoolean(PREF_SHOW_ANTI_FEATURE_APPS, IGNORED_B); } return showAppsWithAntiFeatures; } diff --git a/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java b/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java index 574474f24..a3752021d 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java +++ b/app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java @@ -206,21 +206,18 @@ public class PreferencesFragment extends PreferenceFragment case Preferences.PREF_UPDATE_INTERVAL: updateIntervalSeekBar.setMax(Preferences.UPDATE_INTERVAL_VALUES.length - 1); - updateIntervalSeekBar.setDefaultValue(Preferences.DEFAULT_UPDATE_INTERVAL); int seekBarPosition = updateIntervalSeekBar.getValue(); updateIntervalSeekBar.setSummary(UPDATE_INTERVAL_NAMES[seekBarPosition]); break; case Preferences.PREF_OVER_WIFI: overWifiSeekBar.setMax(Preferences.OVER_NETWORK_ALWAYS); - overWifiSeekBar.setDefaultValue(Preferences.DEFAULT_OVER_WIFI); setNetworkSeekBarSummary(overWifiSeekBar); enableUpdateInverval(); break; case Preferences.PREF_OVER_DATA: overDataSeekBar.setMax(Preferences.OVER_NETWORK_ALWAYS); - overDataSeekBar.setDefaultValue(Preferences.DEFAULT_OVER_DATA); setNetworkSeekBarSummary(overDataSeekBar); enableUpdateInverval(); break; diff --git a/app/src/main/res/values/donottranslate.xml b/app/src/main/res/values/donottranslate.xml index c49c85d9b..a255f6148 100644 --- a/app/src/main/res/values/donottranslate.xml +++ b/app/src/main/res/values/donottranslate.xml @@ -37,4 +37,8 @@ night + 1 + 2 + + 3 diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index 75d6dd58e..84b3c138d 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -30,10 +30,12 @@ 0); + + SharedPreferences sharedPreferences = PreferenceManager.getDefaultSharedPreferences(CONTEXT); + Log.d(TAG, "Clearing DefaultSharedPreferences containing: " + sharedPreferences.getAll().size()); + sharedPreferences.edit().clear().commit(); + assertEquals(0, sharedPreferences.getAll().size()); + + SharedPreferences defaultValueSp = CONTEXT.getSharedPreferences(PreferenceManager.KEY_HAS_SET_DEFAULT_VALUES, + Context.MODE_PRIVATE); + defaultValueSp.edit().remove(PreferenceManager.KEY_HAS_SET_DEFAULT_VALUES).commit(); + } + + /** + * Check that the defaults are being set when using + * {@link PreferenceManager#getDefaultSharedPreferences(Context)}, and that + * the values match. {@link Preferences#Preferences(Context)} sets the + * value of {@link Preferences#PREF_LOCAL_REPO_NAME} dynamically, so there + * is one more preference. + */ + @Test + public void testSetDefaultValues() { + Preferences.setupForTests(CONTEXT); + + SharedPreferences sharedPreferences = PreferenceManager.getDefaultSharedPreferences(CONTEXT); + assertEquals(defaults.getAll().size() + 1, sharedPreferences.getAll().size()); + assertTrue(sharedPreferences.contains(Preferences.PREF_LOCAL_REPO_NAME)); + + Map entries = sharedPreferences.getAll(); + for (Map.Entry entry : defaults.getAll().entrySet()) { + String key = entry.getKey(); + Object value = entry.getValue(); + assertTrue(sharedPreferences.contains(entry.getKey())); + assertEquals(entries.get(key), value); + } + + String key = Preferences.PREF_EXPERT; + boolean defaultValue = defaults.getBoolean(key, false); + sharedPreferences.edit().putBoolean(key, !defaultValue).commit(); + assertNotEquals(defaultValue, sharedPreferences.getBoolean(key, false)); + } + + @Test + public void testMethodsUseDefaults() { + Preferences.setupForTests(CONTEXT); + Preferences preferences = Preferences.get(); + + assertEquals(defaults.getBoolean(Preferences.PREF_AUTO_DOWNLOAD_INSTALL_UPDATES, false), + preferences.isAutoDownloadEnabled()); + assertEquals(defaults.getBoolean(Preferences.PREF_EXPERT, false), + preferences.expertMode()); + assertEquals(defaults.getBoolean(Preferences.PREF_FORCE_TOUCH_APPS, false), + preferences.isForceOldIndexEnabled()); + assertEquals(defaults.getBoolean(Preferences.PREF_FORCE_OLD_INDEX, false), + preferences.isForceOldIndexEnabled()); + assertEquals(defaults.getBoolean(Preferences.PREF_PREVENT_SCREENSHOTS, false), + preferences.preventScreenshots()); + assertEquals(defaults.getBoolean(Preferences.PREF_SHOW_ANTI_FEATURE_APPS, false), + preferences.showAppsWithAntiFeatures()); + assertEquals(defaults.getBoolean(Preferences.PREF_SHOW_INCOMPAT_VERSIONS, false), + preferences.showIncompatibleVersions()); + assertEquals(defaults.getBoolean(Preferences.PREF_SHOW_ROOT_APPS, false), + preferences.showAppsRequiringRoot()); + assertEquals(defaults.getBoolean(Preferences.PREF_UPDATE_NOTIFICATION_ENABLED, false), + preferences.isUpdateNotificationEnabled()); + + assertEquals(Long.parseLong(defaults.getString(Preferences.PREF_KEEP_CACHE_TIME, null)), + preferences.getKeepCacheTime()); + + assertEquals(Preferences.Theme.valueOf(defaults.getString(Preferences.PREF_THEME, null)), + preferences.getTheme()); + + // now test setting the prefs + boolean defaultValue = defaults.getBoolean(Preferences.PREF_EXPERT, false); + preferences.setExpertMode(!defaultValue); + assertNotEquals(defaultValue, preferences.expertMode()); + } + + /** + * When {@link Preferences#Preferences(Context)} calls + * {@link PreferenceManager#setDefaultValues(Context, int, boolean)}, any + * existing preference values should not be overridden. + */ + @Test + public void testMigrationWithSetDefaultValues() { + SharedPreferences sharedPreferences = PreferenceManager.getDefaultSharedPreferences(CONTEXT); + long testValue = (long) (Math.random() * Long.MAX_VALUE); + String testValueString = String.valueOf(testValue); + assertNotEquals(testValueString, defaults.getString(Preferences.PREF_KEEP_CACHE_TIME, "not a long")); + sharedPreferences.edit().putString(Preferences.PREF_KEEP_CACHE_TIME, testValueString).commit(); + + Preferences.setupForTests(CONTEXT); + assertEquals(testValue, Preferences.get().getKeepCacheTime()); + assertNotEquals(Long.parseLong(defaults.getString(Preferences.PREF_KEEP_CACHE_TIME, null)), + Preferences.get().getKeepCacheTime()); + } +}