Merge pull request #4797 from nextcloud/ezaquarii/replace-shared-preferences-with-app-preferences-in-base-activity

Replace BaseActivity.sharedPreferences with app preferences

src/main/java/com/nextcloud/client/preferences/AppPreferences.java

@@ -23,8 +23,42 @@ package com.nextcloud.client.preferences;
 import com.owncloud.android.datamodel.OCFile;
 import com.owncloud.android.utils.FileSortOrder;
 
+import androidx.annotation.Nullable;
+
+/**
+ * This interface provides single point of entry for access to all application
+ * preferences and allows clients to subscribe for specific configuration
+ * changes.
+ */
 public interface AppPreferences {
 
+    /**
+     * Preferences listener. Callbacks should be invoked on main thread.
+     *
+     * Miantainers should extend this interface with callbacks for specific
+     * events.
+     */
+    interface Listener {
+        default void onDarkThemeEnabledChanged(boolean enabled) {
+            /* default empty implementation */
+        };
+    }
+
+    /**
+     * Registers preferences listener. It no-ops if listener
+     * is already registered.
+     *
+     * @param listener application preferences listener
+     */
+    void addListener(@Nullable Listener listener);
+
+    /**
+     * Unregister listener. It no-ops if listener is not registered.
+     *
+     * @param listener application preferences listener
+     */
+    void removeListener(@Nullable Listener listener);
+
     void setKeysReInitEnabled();
     boolean isKeysReInitEnabled();
 
@@ -239,6 +273,18 @@ public interface AppPreferences {
      */
     int getUploaderBehaviour();
 
+    /**
+     * Enable dark theme.
+     *
+     * This is reactive property. Listeners will be invoked if registered.
+     *
+     * @param enabled true to turn dark theme on, false to turn it off
+     */
+    void setDarkThemeEnabled(boolean enabled);
+
+    /**
+     * @return true if application uses dark UI theme, false otherwise
+     */
     boolean isDarkThemeEnabled();
 
     /**

src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java

@@ -35,12 +35,21 @@ import com.owncloud.android.ui.activity.PassCodeActivity;
 import com.owncloud.android.ui.activity.SettingsActivity;
 import com.owncloud.android.utils.FileSortOrder;
 
+import java.util.Set;
+import java.util.concurrent.CopyOnWriteArraySet;
+
+import androidx.annotation.Nullable;
+
 import static com.owncloud.android.ui.fragment.OCFileListFragment.FOLDER_LAYOUT_LIST;
 
 /**
- * Helper to simplify reading of Preferences all around the app
+ * Implementation of application-wide preferences using {@link SharedPreferences}.
+ *
+ * Users should not use this class directly. Please use {@link AppPreferences} interafce
+ * instead.
  */
 public final class AppPreferencesImpl implements AppPreferences {
+
     /**
      * Constant to access value of last path selected by the user to upload a file shared from other app.
      * Value handled by the app without direct access in the UI.
@@ -48,6 +57,7 @@ public final class AppPreferencesImpl implements AppPreferences {
     public static final String AUTO_PREF__LAST_SEEN_VERSION_CODE = "lastSeenVersionCode";
     public static final String STORAGE_PATH = "storage_path";
     public static final float DEFAULT_GRID_COLUMN = 4.0f;
+
     private static final String AUTO_PREF__LAST_UPLOAD_PATH = "last_upload_path";
     private static final String AUTO_PREF__UPLOAD_FROM_LOCAL_LAST_PATH = "upload_from_local_last_path";
     private static final String AUTO_PREF__UPLOAD_FILE_EXTENSION_MAP_URL = "prefs_upload_file_extension_map_url";
@@ -68,7 +78,7 @@ public final class AppPreferencesImpl implements AppPreferences {
     private static final String PREF__AUTO_UPLOAD_INIT = "autoUploadInit";
     private static final String PREF__FOLDER_SORT_ORDER = "folder_sort_order";
     private static final String PREF__FOLDER_LAYOUT = "folder_layout";
-    public static final String PREF__THEME = "darkTheme";
+    static final String PREF__DARK_THEME_ENABLED = "dark_theme_enabled";
 
     private static final String PREF__LOCK_TIMESTAMP = "lock_timestamp";
     private static final String PREF__SHOW_MEDIA_SCAN_NOTIFICATIONS = "show_media_scan_notifications";
@@ -81,7 +91,54 @@ public final class AppPreferencesImpl implements AppPreferences {
     private final Context context;
     private final SharedPreferences preferences;
     private final CurrentAccountProvider currentAccountProvider;
+    private final ListenerRegistry listeners;
+
+    /**
+     * Adapter delegating raw {@link SharedPreferences.OnSharedPreferenceChangeListener} calls
+     * with key-value pairs to respective {@link com.nextcloud.client.preferences.AppPreferences.Listener} method.
+     */
+    static class ListenerRegistry implements SharedPreferences.OnSharedPreferenceChangeListener {
+        private final AppPreferences preferences;
+        private final Set<Listener> listeners;
+
+        ListenerRegistry(AppPreferences preferences) {
+            this.preferences = preferences;
+            this.listeners = new CopyOnWriteArraySet<>();
+        }
+
+        void add(@Nullable final Listener listener) {
+            if (listener != null) {
+                listeners.add(listener);
+            }
+        }
+
+        void remove(@Nullable  final Listener listener) {
+            if (listener != null) {
+                listeners.remove(listener);
+            }
+        }
+
+        @Override
+        public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key) {
+            if(PREF__DARK_THEME_ENABLED.equals(key)) {
+                boolean enabled = preferences.isDarkThemeEnabled();
+                for(Listener l : listeners) {
+                    l.onDarkThemeEnabledChanged(enabled);
+                }
+            }
+        }
+    }
 
+    /**
+     * This is a temporary workaround to access app preferences in places that cannot use
+     * dependency injection yet. Use injected component via {@link AppPreferences} interface.
+     *
+     * WARNING: this creates new instance! it does not return app-wide singleton
+     *
+     * @param context Context used to create shared preferences
+     * @return New instance of app preferences component
+     */
+    @Deprecated
     public static AppPreferences fromContext(Context context) {
         final CurrentAccountProvider currentAccountProvider = UserAccountManagerImpl.fromContext(context);
         final SharedPreferences prefs = android.preference.PreferenceManager.getDefaultSharedPreferences(context);
@@ -92,6 +149,18 @@ public final class AppPreferencesImpl implements AppPreferences {
         this.context = appContext;
         this.preferences = preferences;
         this.currentAccountProvider = currentAccountProvider;
+        this.listeners = new ListenerRegistry(this);
+        this.preferences.registerOnSharedPreferenceChangeListener(listeners);
+    }
+
+    @Override
+    public void addListener(@Nullable AppPreferences.Listener listener) {
+        this.listeners.add(listener);
+    }
+
+    @Override
+    public void removeListener(@Nullable AppPreferences.Listener listener) {
+        this.listeners.remove(listener);
     }
 
     @Override
@@ -342,9 +411,14 @@ public final class AppPreferencesImpl implements AppPreferences {
         return preferences.getInt(AUTO_PREF__UPLOADER_BEHAVIOR, 1);
     }
 
+    @Override
+    public void setDarkThemeEnabled(boolean enabled) {
+        preferences.edit().putBoolean(PREF__DARK_THEME_ENABLED, enabled).apply();
+    }
+
     @Override
     public boolean isDarkThemeEnabled() {
-        return preferences.getBoolean(PREF__THEME, false);
+        return preferences.getBoolean(PREF__DARK_THEME_ENABLED, false);
     }
 
     @Override

src/main/java/com/owncloud/android/ui/activity/BaseActivity.java

@@ -12,6 +12,7 @@ import android.os.Handler;
 
 import com.nextcloud.client.account.UserAccountManager;
 import com.nextcloud.client.di.Injectable;
+import com.nextcloud.client.preferences.AppPreferences;
 import com.nextcloud.client.preferences.AppPreferencesImpl;
 import com.owncloud.android.MainApp;
 import com.owncloud.android.R;
@@ -28,9 +29,7 @@ import androidx.appcompat.app.AppCompatActivity;
 /**
  * Base activity with common behaviour for activities dealing with ownCloud {@link Account}s .
  */
-public abstract class BaseActivity
-    extends AppCompatActivity
-    implements Injectable, SharedPreferences.OnSharedPreferenceChangeListener {
+public abstract class BaseActivity extends AppCompatActivity implements Injectable {
 
     private static final String TAG = BaseActivity.class.getSimpleName();
 
@@ -56,7 +55,14 @@ public abstract class BaseActivity
     private boolean paused;
 
     @Inject UserAccountManager accountManager;
-    @Inject SharedPreferences sharedPreferences;
+    @Inject AppPreferences preferences;
+
+    private AppPreferences.Listener onPreferencesChanged = new AppPreferences.Listener() {
+        @Override
+        public void onDarkThemeEnabledChanged(boolean enabled) {
+            BaseActivity.this.onThemeSettingsChanged();
+        }
+    };
 
     public UserAccountManager getUserAccountManager() {
         return accountManager;
@@ -65,13 +71,13 @@ public abstract class BaseActivity
     @Override
     protected void onPostCreate(@Nullable Bundle savedInstanceState) {
         super.onPostCreate(savedInstanceState);
-        sharedPreferences.registerOnSharedPreferenceChangeListener(this);
+        preferences.addListener(onPreferencesChanged);
     }
 
     @Override
     protected void onDestroy() {
         super.onDestroy();
-        sharedPreferences.unregisterOnSharedPreferenceChangeListener(this);
+        preferences.removeListener(onPreferencesChanged);
     }
 
     @Override
@@ -122,17 +128,12 @@ public abstract class BaseActivity
         Log_OC.v(TAG, "onRestart() end");
     }
 
-    @Override
-    public void onSharedPreferenceChanged(final SharedPreferences sharedPreferences, final String key) {
-       if (!AppPreferencesImpl.PREF__THEME.equals(key)) {
-            return;
-        }
-
+    private void onThemeSettingsChanged() {
         if(paused) {
             themeChangePending = true;
-            return;
+        } else {
+            recreate();
         }
-        recreate();
     }
 
     /**

src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java

@@ -692,13 +692,13 @@ public class SettingsActivity extends ThemedPreferenceActivity
 
         loadStoragePath();
 
-        SwitchPreference themePref = (SwitchPreference) findPreference(AppPreferencesImpl.PREF__THEME);
-
-        themePref.setSummary(preferences.isDarkThemeEnabled() ?
-                            getString(R.string.prefs_value_theme_dark) : getString(R.string.prefs_value_theme_light));
+        SwitchPreference themePref = (SwitchPreference) findPreference("dark_theme_enabled");
+        boolean darkThemeEnabled = preferences.isDarkThemeEnabled();
+        int summaryResId = darkThemeEnabled ? R.string.prefs_value_theme_dark : R.string.prefs_value_theme_light;
+        themePref.setSummary(summaryResId);
         themePref.setOnPreferenceChangeListener((preference, newValue) -> {
-            MainApp.setAppTheme((Boolean) newValue);
-
+            boolean enabled = (Boolean)newValue;
+            MainApp.setAppTheme(enabled);
             return true;
         });
     }

src/main/java/com/owncloud/android/ui/activity/ThemedPreferenceActivity.java

@@ -20,17 +20,16 @@
 
 package com.owncloud.android.ui.activity;
 
-import android.content.SharedPreferences;
 import android.os.Bundle;
 import android.preference.PreferenceActivity;
 
+import com.nextcloud.client.preferences.AppPreferences;
+
 import javax.inject.Inject;
 
 import androidx.annotation.Nullable;
 
-public class ThemedPreferenceActivity
-    extends PreferenceActivity
-    implements SharedPreferences.OnSharedPreferenceChangeListener {
+public class ThemedPreferenceActivity extends PreferenceActivity {
 
     /**
      * Tracks whether the activity should be recreate()'d after a theme change
@@ -38,18 +37,29 @@ public class ThemedPreferenceActivity
     private boolean themeChangePending;
     private boolean paused;
 
-    @Inject SharedPreferences sharedPreferences;
+    @Inject AppPreferences preferences;
+
+    private AppPreferences.Listener onThemeChangedListener = new AppPreferences.Listener() {
+        @Override
+        public void onDarkThemeEnabledChanged(boolean enabled) {
+            if(paused) {
+                themeChangePending = true;
+                return;
+            }
+            recreate();
+        }
+    };
 
     @Override
     protected void onPostCreate(@Nullable Bundle savedInstanceState) {
         super.onPostCreate(savedInstanceState);
-        sharedPreferences.registerOnSharedPreferenceChangeListener(this);
+        preferences.addListener(onThemeChangedListener);
     }
 
     @Override
     protected void onDestroy() {
         super.onDestroy();
-        sharedPreferences.unregisterOnSharedPreferenceChangeListener(this);
+        preferences.removeListener(onThemeChangedListener);
     }
 
     @Override
@@ -67,14 +77,4 @@ public class ThemedPreferenceActivity
             recreate();
         }
     }
-
-    @Override
-    public void onSharedPreferenceChanged(final SharedPreferences sharedPreferences, final String key) {
-        if(paused) {
-            themeChangePending = true;
-            return;
-        }
-
-        recreate();
-    }
 }

src/main/res/xml/preferences.xml

@@ -27,8 +27,9 @@
             android:title="@string/prefs_storage_path"
             android:key="storage_path"/>
         <com.owncloud.android.ui.ThemeableSwitchPreference
+            android:id="@+id/dark_theme_preference"
             android:defaultValue="@string/prefs_value_theme_light"
-            android:key="darkTheme"
+            android:key="dark_theme_enabled"
             android:summary="%s"
             android:title="@string/prefs_theme_title"
             android:theme="@style/SwitchPreference"/>

src/test/java/com/nextcloud/client/preferences/TestAppPreferences.java

@@ -0,0 +1,156 @@
+package com.nextcloud.client.preferences;
+
+import android.content.Context;
+import android.content.SharedPreferences;
+
+import com.nextcloud.client.account.CurrentAccountProvider;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+import org.mockito.InOrder;
+import org.mockito.Mock;
+import static org.mockito.Mockito.*;
+
+import org.mockito.MockitoAnnotations;
+
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+    TestAppPreferences.Preferences.class,
+    TestAppPreferences.ListenerRegistery.class
+})
+public class TestAppPreferences {
+
+    public static class ListenerRegistery {
+        private static final SharedPreferences NOT_USED_NULL = null;
+
+        @Mock
+        private AppPreferences.Listener listener1;
+
+        @Mock
+        private AppPreferences.Listener listener2;
+
+        @Mock
+        private AppPreferences.Listener listener3;
+
+        @Mock
+        private AppPreferences.Listener listener4;
+
+        @Mock
+        AppPreferences appPreferences;
+
+        private AppPreferencesImpl.ListenerRegistry registry;
+
+        @Before
+        public void setUp() {
+            MockitoAnnotations.initMocks(this);
+            when(appPreferences.isDarkThemeEnabled()).thenReturn(true);
+            registry = new AppPreferencesImpl.ListenerRegistry(appPreferences);
+        }
+
+        @Test
+        public void canRemoveListenersFromCallback() {
+
+            // GIVEN
+            //      registery has few listeners
+            //      one listener will try to remove itself and other listener
+            registry.add(listener1);
+            registry.add(listener2);
+            registry.add(listener3);
+            registry.add(listener4);
+
+            doAnswer((i) -> {
+                registry.remove(listener2);
+                registry.remove(listener3);
+                return null;
+            }).when(listener2).onDarkThemeEnabledChanged(anyBoolean());
+
+            // WHEN
+            //      callback is called twice
+            registry.onSharedPreferenceChanged(NOT_USED_NULL, AppPreferencesImpl.PREF__DARK_THEME_ENABLED);
+            registry.onSharedPreferenceChanged(NOT_USED_NULL, AppPreferencesImpl.PREF__DARK_THEME_ENABLED);
+
+            // THEN
+            //      no ConcurrentModificationException
+            //      1st time, all listeners (including removed) are called
+            //      2nd time removed callbacks are not called
+            verify(listener1, times(2)).onDarkThemeEnabledChanged(anyBoolean());
+            verify(listener2).onDarkThemeEnabledChanged(anyBoolean());
+            verify(listener3).onDarkThemeEnabledChanged(anyBoolean());
+            verify(listener4, times(2)).onDarkThemeEnabledChanged(anyBoolean());
+        }
+
+        @Test
+        public void nullsAreNotAddedToRegistry() {
+            // GIVEN
+            //      registry has no listeners
+            //      attempt to add null listener was made
+            registry.add(null);
+
+            // WHEN
+            //      callback is called
+            registry.onSharedPreferenceChanged(NOT_USED_NULL, AppPreferencesImpl.PREF__DARK_THEME_ENABLED);
+
+            // THEN
+            //      nothing happens
+            //      null was not added to registry
+        }
+
+        @Test
+        public void nullsAreNotRemovedFromRegistry() {
+            // GIVEN
+            //      registry has no listeners
+
+            // WHEN
+            //      attempt to remove null listener was made
+            registry.remove(null);
+
+            // THEN
+            //      null is ignored
+        }
+    }
+
+    public static class Preferences {
+        @Mock
+        private Context testContext;
+
+        @Mock
+        private SharedPreferences sharedPreferences;
+
+        @Mock
+        private SharedPreferences.Editor editor;
+
+        @Mock
+        private CurrentAccountProvider accountProvider;
+
+        private AppPreferencesImpl appPreferences;
+
+        @Before
+        public void setUp() {
+            MockitoAnnotations.initMocks(this);
+            when(editor.remove(anyString())).thenReturn(editor);
+            when(sharedPreferences.edit()).thenReturn(editor);
+            appPreferences = new AppPreferencesImpl(testContext, sharedPreferences, accountProvider);
+        }
+
+        @Test
+        public void removeLegacyPreferences() {
+            appPreferences.removeLegacyPreferences();
+            InOrder inOrder = inOrder(editor);
+            inOrder.verify(editor).remove("instant_uploading");
+            inOrder.verify(editor).remove("instant_video_uploading");
+            inOrder.verify(editor).remove("instant_upload_path");
+            inOrder.verify(editor).remove("instant_upload_path_use_subfolders");
+            inOrder.verify(editor).remove("instant_upload_on_wifi");
+            inOrder.verify(editor).remove("instant_upload_on_charging");
+            inOrder.verify(editor).remove("instant_video_upload_path");
+            inOrder.verify(editor).remove("instant_video_upload_path_use_subfolders");
+            inOrder.verify(editor).remove("instant_video_upload_on_wifi");
+            inOrder.verify(editor).remove("instant_video_uploading");
+            inOrder.verify(editor).remove("instant_video_upload_on_charging");
+            inOrder.verify(editor).remove("prefs_instant_behaviour");
+            inOrder.verify(editor).apply();
+        }
+    }
+}

src/test/java/com/nextcloud/client/preferences/TestPreferenceManager.java

@@ -1,58 +0,0 @@
-package com.nextcloud.client.preferences;
-
-import android.content.Context;
-import android.content.SharedPreferences;
-
-import com.nextcloud.client.account.CurrentAccountProvider;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.InOrder;
-import org.mockito.Mock;
-import static org.mockito.Mockito.*;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class TestPreferenceManager {
-
-    @Mock
-    private Context testContext;
-
-    @Mock
-    private SharedPreferences sharedPreferences;
-
-    @Mock
-    private SharedPreferences.Editor editor;
-
-    @Mock
-    private CurrentAccountProvider accountProvider;
-
-    private AppPreferencesImpl appPreferences;
-
-    @Before
-    public void setUp() {
-        when(editor.remove(anyString())).thenReturn(editor);
-        when(sharedPreferences.edit()).thenReturn(editor);
-        appPreferences = new AppPreferencesImpl(testContext, sharedPreferences, accountProvider);
-    }
-
-    @Test
-    public void removeLegacyPreferences() {
-        appPreferences.removeLegacyPreferences();
-        InOrder inOrder = inOrder(editor);
-        inOrder.verify(editor).remove("instant_uploading");
-        inOrder.verify(editor).remove("instant_video_uploading");
-        inOrder.verify(editor).remove("instant_upload_path");
-        inOrder.verify(editor).remove("instant_upload_path_use_subfolders");
-        inOrder.verify(editor).remove("instant_upload_on_wifi");
-        inOrder.verify(editor).remove("instant_upload_on_charging");
-        inOrder.verify(editor).remove("instant_video_upload_path");
-        inOrder.verify(editor).remove("instant_video_upload_path_use_subfolders");
-        inOrder.verify(editor).remove("instant_video_upload_on_wifi");
-        inOrder.verify(editor).remove("instant_video_uploading");
-        inOrder.verify(editor).remove("instant_video_upload_on_charging");
-        inOrder.verify(editor).remove("prefs_instant_behaviour");
-        inOrder.verify(editor).apply();
-    }
-}