diff --git a/src/main/java/com/nextcloud/client/preferences/AppPreferences.java b/src/main/java/com/nextcloud/client/preferences/AppPreferences.java index aad701982c..5c6d9107a6 100644 --- a/src/main/java/com/nextcloud/client/preferences/AppPreferences.java +++ b/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(); /** diff --git a/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java b/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java index a78ee9aa2f..1ac83f8873 100644 --- a/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java +++ b/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 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 diff --git a/src/main/java/com/owncloud/android/ui/activity/BaseActivity.java b/src/main/java/com/owncloud/android/ui/activity/BaseActivity.java index f10bc8a290..e65b103160 100644 --- a/src/main/java/com/owncloud/android/ui/activity/BaseActivity.java +++ b/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(); } /** diff --git a/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java b/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java index ae8474d5eb..e51d028e5f 100644 --- a/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java +++ b/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; }); } diff --git a/src/main/java/com/owncloud/android/ui/activity/ThemedPreferenceActivity.java b/src/main/java/com/owncloud/android/ui/activity/ThemedPreferenceActivity.java index 1acc839a4f..058a1bed8a 100644 --- a/src/main/java/com/owncloud/android/ui/activity/ThemedPreferenceActivity.java +++ b/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(); - } } diff --git a/src/main/res/xml/preferences.xml b/src/main/res/xml/preferences.xml index 4dae1b4f4b..67e479c50c 100644 --- a/src/main/res/xml/preferences.xml +++ b/src/main/res/xml/preferences.xml @@ -27,8 +27,9 @@ android:title="@string/prefs_storage_path" android:key="storage_path"/> diff --git a/src/test/java/com/nextcloud/client/preferences/TestAppPreferences.java b/src/test/java/com/nextcloud/client/preferences/TestAppPreferences.java new file mode 100644 index 0000000000..9e7fef2602 --- /dev/null +++ b/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(); + } + } +} diff --git a/src/test/java/com/nextcloud/client/preferences/TestPreferenceManager.java b/src/test/java/com/nextcloud/client/preferences/TestPreferenceManager.java deleted file mode 100644 index 64bdca3e5e..0000000000 --- a/src/test/java/com/nextcloud/client/preferences/TestPreferenceManager.java +++ /dev/null @@ -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(); - } -}