From 5356e0c0a95c1693bfafcf413233cc85449451de Mon Sep 17 00:00:00 2001 From: menghanli Date: Mon, 11 Jul 2022 16:09:10 +0800 Subject: [PATCH] Refactor CaptionAppearanceFragment to improve maintainability (4/n) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: There is a bunch of different logic of preferences in CaptionAppearanceFragment. It’s hard to implement new features and hard to maintain and hard to be testable. Solution: Move out preset preference logic of CaptionAppearanceFragment into controllers to reduce the complexity of the relationship between preference and fragment. Bug: 197695932 Test: make RunSettingsRoboTests ROBOTEST_FILTER=CaptionPresetControllerTest CaptionAppearanceFragmentTest Change-Id: I5409c1e8a6bdfc633abc304d8cf800ea0943de78 --- res/xml/captioning_appearance.xml | 3 +- .../CaptionAppearanceFragment.java | 107 +++++------- .../settings/accessibility/CaptionHelper.java | 15 ++ .../CaptionPresetController.java | 62 +++++++ .../CaptionAppearanceFragmentTest.java | 159 ++++++++++++++++++ .../accessibility/CaptionHelperTest.java | 12 ++ .../CaptionPresetControllerTest.java | 115 +++++++++++++ 7 files changed, 405 insertions(+), 68 deletions(-) create mode 100644 src/com/android/settings/accessibility/CaptionPresetController.java create mode 100644 tests/robotests/src/com/android/settings/accessibility/CaptionAppearanceFragmentTest.java create mode 100644 tests/robotests/src/com/android/settings/accessibility/CaptionPresetControllerTest.java diff --git a/res/xml/captioning_appearance.xml b/res/xml/captioning_appearance.xml index 66c74e6d3d1..b29d5a71104 100644 --- a/res/xml/captioning_appearance.xml +++ b/res/xml/captioning_appearance.xml @@ -38,7 +38,8 @@ + android:title="@string/captioning_preset" + settings:controller="com.android.settings.accessibility.CaptionPresetController"/> CAPTIONING_FEATURE_KEYS = Arrays.asList( + Settings.Secure.ACCESSIBILITY_CAPTIONING_PRESET + ); + private final Handler mHandler = new Handler(Looper.getMainLooper()); + @VisibleForTesting + AccessibilitySettingsContentObserver mSettingsContentObserver; private CaptioningManager mCaptioningManager; - private CaptionHelper mCaptionHelper; - - // Standard options. - private PresetPreference mPreset; - - // Custom options. private PreferenceCategory mCustom; - private boolean mShowingCustom; - @Override public int getMetricsCategory() { return SettingsEnums.ACCESSIBILITY_CAPTION_APPEARANCE; @@ -60,14 +61,24 @@ public class CaptionAppearanceFragment extends DashboardFragment @Override public void onCreatePreferences(Bundle savedInstanceState, String rootKey) { super.onCreatePreferences(savedInstanceState, rootKey); - - mCaptioningManager = (CaptioningManager) getSystemService(Context.CAPTIONING_SERVICE); - mCaptionHelper = new CaptionHelper(getContext()); - - initializeAllPreferences(); - updateAllPreferences(); + mCaptioningManager = getContext().getSystemService(CaptioningManager.class); + mSettingsContentObserver = new AccessibilitySettingsContentObserver(mHandler); + mSettingsContentObserver.registerKeysToObserverCallback(CAPTIONING_FEATURE_KEYS, + key -> refreshShowingCustom()); + mCustom = findPreference(PREF_CUSTOM); refreshShowingCustom(); - installUpdateListeners(); + } + + @Override + public void onStart() { + super.onStart(); + mSettingsContentObserver.register(getContext().getContentResolver()); + } + + @Override + public void onStop() { + super.onStop(); + getContext().getContentResolver().unregisterContentObserver(mSettingsContentObserver); } @Override @@ -80,55 +91,17 @@ public class CaptionAppearanceFragment extends DashboardFragment return TAG; } - private void initializeAllPreferences() { - - final Resources res = getResources(); - final int[] presetValues = res.getIntArray(R.array.captioning_preset_selector_values); - final String[] presetTitles = res.getStringArray(R.array.captioning_preset_selector_titles); - mPreset = (PresetPreference) findPreference(PREF_PRESET); - mPreset.setValues(presetValues); - mPreset.setTitles(presetTitles); - - mCustom = (PreferenceCategory) findPreference(PREF_CUSTOM); - mShowingCustom = true; - } - - private void installUpdateListeners() { - mPreset.setOnValueChangedListener(this); - } - - private void updateAllPreferences() { - final int preset = mCaptioningManager.getRawUserStyle(); - mPreset.setValue(preset); - } - - private void refreshShowingCustom() { - final boolean customPreset = - mPreset.getValue() == CaptioningManager.CaptionStyle.PRESET_CUSTOM; - if (!customPreset && mShowingCustom) { - getPreferenceScreen().removePreference(mCustom); - mShowingCustom = false; - } else if (customPreset && !mShowingCustom) { - getPreferenceScreen().addPreference(mCustom); - mShowingCustom = true; - } - } - - @Override - public void onValueChanged(ListDialogPreference preference, int value) { - final ContentResolver cr = getActivity().getContentResolver(); - if (mPreset == preference) { - Settings.Secure.putInt(cr, Settings.Secure.ACCESSIBILITY_CAPTIONING_PRESET, value); - refreshShowingCustom(); - } - mCaptionHelper.setEnabled(true); - } - @Override public int getHelpResource() { return R.string.help_url_caption; } + private void refreshShowingCustom() { + final boolean isCustomPreset = + mCaptioningManager.getRawUserStyle() == CaptionStyle.PRESET_CUSTOM; + mCustom.setVisible(isCustomPreset); + } + public static final BaseSearchIndexProvider SEARCH_INDEX_DATA_PROVIDER = new BaseSearchIndexProvider(R.xml.captioning_appearance); } diff --git a/src/com/android/settings/accessibility/CaptionHelper.java b/src/com/android/settings/accessibility/CaptionHelper.java index 4c9127fa951..eb76b6d0ec2 100644 --- a/src/com/android/settings/accessibility/CaptionHelper.java +++ b/src/com/android/settings/accessibility/CaptionHelper.java @@ -186,4 +186,19 @@ public class CaptionHelper { final CaptionStyle attrs = CaptionStyle.getCustomStyle(mContentResolver); return attrs.edgeType; } + + /** + * Sets the caption raw user style. + * + * @param type The caption raw user style + */ + public void setRawUserStyle(int type) { + Settings.Secure.putInt(mContentResolver, + Settings.Secure.ACCESSIBILITY_CAPTIONING_PRESET, type); + } + + /** Returns the caption raw user style.*/ + public int getRawUserStyle() { + return mCaptioningManager.getRawUserStyle(); + } } diff --git a/src/com/android/settings/accessibility/CaptionPresetController.java b/src/com/android/settings/accessibility/CaptionPresetController.java new file mode 100644 index 00000000000..db21f0b29b6 --- /dev/null +++ b/src/com/android/settings/accessibility/CaptionPresetController.java @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.accessibility; + +import android.content.Context; +import android.content.res.Resources; + +import androidx.preference.PreferenceScreen; + +import com.android.settings.R; +import com.android.settings.core.BasePreferenceController; + +/** Preference controller for caption preset. */ +public class CaptionPresetController extends BasePreferenceController + implements ListDialogPreference.OnValueChangedListener { + + private final CaptionHelper mCaptionHelper; + + public CaptionPresetController(Context context, String preferenceKey) { + super(context, preferenceKey); + mCaptionHelper = new CaptionHelper(context); + } + + @Override + public int getAvailabilityStatus() { + return AVAILABLE; + } + + @Override + public void displayPreference(PreferenceScreen screen) { + super.displayPreference(screen); + final PresetPreference preference = screen.findPreference(getPreferenceKey()); + final Resources res = mContext.getResources(); + final int[] presetValues = res.getIntArray(R.array.captioning_preset_selector_values); + final String[] presetTitles = res.getStringArray(R.array.captioning_preset_selector_titles); + preference.setTitles(presetTitles); + preference.setValues(presetValues); + final int preset = mCaptionHelper.getRawUserStyle(); + preference.setValue(preset); + preference.setOnValueChangedListener(this); + } + + @Override + public void onValueChanged(ListDialogPreference preference, int value) { + mCaptionHelper.setRawUserStyle(value); + mCaptionHelper.setEnabled(true); + } +} diff --git a/tests/robotests/src/com/android/settings/accessibility/CaptionAppearanceFragmentTest.java b/tests/robotests/src/com/android/settings/accessibility/CaptionAppearanceFragmentTest.java new file mode 100644 index 00000000000..3c320e2c6c6 --- /dev/null +++ b/tests/robotests/src/com/android/settings/accessibility/CaptionAppearanceFragmentTest.java @@ -0,0 +1,159 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.accessibility; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.app.settings.SettingsEnums; +import android.content.ContentResolver; +import android.content.Context; +import android.os.Bundle; +import android.provider.Settings; +import android.view.accessibility.CaptioningManager.CaptionStyle; + +import androidx.preference.PreferenceCategory; +import androidx.preference.PreferenceManager; +import androidx.preference.PreferenceScreen; +import androidx.test.core.app.ApplicationProvider; + +import com.android.settings.R; +import com.android.settings.SettingsActivity; +import com.android.settings.testutils.XmlTestUtils; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.util.ReflectionHelpers; + +import java.util.List; + +/** Tests for {@link CaptionAppearanceFragment}. */ +@RunWith(RobolectricTestRunner.class) +public class CaptionAppearanceFragmentTest { + + @Rule + public MockitoRule mMockitoRule = MockitoJUnit.rule(); + @Mock + private SettingsActivity mActivity; + @Mock + private PreferenceScreen mScreen; + @Mock + private PreferenceManager mPreferenceManager; + @Mock + private ContentResolver mContentResolver; + @Mock + private PreferenceCategory mCustomPref; + @Spy + private Context mContext = ApplicationProvider.getApplicationContext(); + private TestCaptionAppearanceFragment mFragment; + + @Before + public void setUp() { + mFragment = spy(new TestCaptionAppearanceFragment()); + doReturn(mActivity).when(mFragment).getActivity(); + doReturn(mContext).when(mFragment).getContext(); + doReturn(mCustomPref).when(mFragment).findPreference(mFragment.PREF_CUSTOM); + when(mPreferenceManager.getPreferenceScreen()).thenReturn(mScreen); + ReflectionHelpers.setField(mFragment, "mPreferenceManager", mPreferenceManager); + } + + @Test + public void onCreatePreferences_shouldPreferenceIsInvisible() { + mFragment.onAttach(mContext); + + mFragment.onCreatePreferences(Bundle.EMPTY, /* rootKey */ null); + + verify(mCustomPref).setVisible(false); + } + + @Test + public void onCreatePreferences_customValue_shouldPreferenceIsVisible() { + Settings.Secure.putInt(mContext.getContentResolver(), + Settings.Secure.ACCESSIBILITY_CAPTIONING_PRESET, CaptionStyle.PRESET_CUSTOM); + mFragment.onAttach(mContext); + + mFragment.onCreatePreferences(Bundle.EMPTY, /* rootKey */ null); + + verify(mCustomPref).setVisible(true); + } + + @Test + public void onStart_registerSpecificContentObserverForSpecificKeys() { + when(mContext.getContentResolver()).thenReturn(mContentResolver); + mFragment.onAttach(mContext); + mFragment.onCreatePreferences(Bundle.EMPTY, /* rootKey */ null); + + mFragment.onStart(); + + for (String key : mFragment.CAPTIONING_FEATURE_KEYS) { + verify(mContentResolver).registerContentObserver(Settings.Secure.getUriFor(key), + /* notifyForDescendants= */ false, mFragment.mSettingsContentObserver); + } + } + + @Test + public void onStop_unregisterContentObserver() { + when(mContext.getContentResolver()).thenReturn(mContentResolver); + mFragment.onAttach(mContext); + mFragment.onCreatePreferences(Bundle.EMPTY, /* rootKey */ null); + mFragment.onStart(); + + mFragment.onStop(); + + verify(mContentResolver).unregisterContentObserver(mFragment.mSettingsContentObserver); + } + + @Test + public void getMetricsCategory_returnsCorrectCategory() { + assertThat(mFragment.getMetricsCategory()).isEqualTo( + SettingsEnums.ACCESSIBILITY_CAPTION_APPEARANCE); + } + + @Test + public void getLogTag_returnsCorrectTag() { + assertThat(mFragment.getLogTag()).isEqualTo("CaptionAppearanceFragment"); + } + + @Test + public void getNonIndexableKeys_existInXmlLayout() { + final List niks = CaptionAppearanceFragment.SEARCH_INDEX_DATA_PROVIDER + .getNonIndexableKeys(mContext); + final List keys = XmlTestUtils.getKeysFromPreferenceXml(mContext, + R.xml.captioning_appearance); + + assertThat(keys).containsAtLeastElementsIn(niks); + } + + private static class TestCaptionAppearanceFragment extends CaptionAppearanceFragment { + + @Override + public int getPreferenceScreenResId() { + return R.xml.placeholder_prefs; + } + } +} diff --git a/tests/robotests/src/com/android/settings/accessibility/CaptionHelperTest.java b/tests/robotests/src/com/android/settings/accessibility/CaptionHelperTest.java index 3047433db42..4700ecdfd27 100644 --- a/tests/robotests/src/com/android/settings/accessibility/CaptionHelperTest.java +++ b/tests/robotests/src/com/android/settings/accessibility/CaptionHelperTest.java @@ -24,6 +24,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.content.ContentResolver; import android.content.Context; import android.provider.Settings; import android.view.View; @@ -61,12 +62,14 @@ public class CaptionHelperTest { private View mPreviewWindow; @Spy private final Context mContext = ApplicationProvider.getApplicationContext(); + private ContentResolver mContentResolver; private CaptionHelper mCaptionHelper; @Before public void setUp() { when(mContext.getSystemService(CaptioningManager.class)).thenReturn(mCaptioningManager); mCaptionHelper = new CaptionHelper(mContext); + mContentResolver = mContext.getContentResolver(); } @Test @@ -170,4 +173,13 @@ public class CaptionHelperTest { final int edgeType = mCaptionHelper.getEdgeType(); assertThat(edgeType).isEqualTo(CaptionStyle.EDGE_TYPE_OUTLINE); } + + @Test + public void setRawUserStyle_shouldReturnSpecificStyle() { + mCaptionHelper.setRawUserStyle(CaptionStyle.PRESET_CUSTOM); + + final int style = Settings.Secure.getInt(mContentResolver, + Settings.Secure.ACCESSIBILITY_CAPTIONING_PRESET, 0); + assertThat(style).isEqualTo(CaptionStyle.PRESET_CUSTOM); + } } diff --git a/tests/robotests/src/com/android/settings/accessibility/CaptionPresetControllerTest.java b/tests/robotests/src/com/android/settings/accessibility/CaptionPresetControllerTest.java new file mode 100644 index 00000000000..0b832d3f1b0 --- /dev/null +++ b/tests/robotests/src/com/android/settings/accessibility/CaptionPresetControllerTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.accessibility; + +import static com.android.settings.accessibility.AccessibilityUtil.State.OFF; +import static com.android.settings.accessibility.AccessibilityUtil.State.ON; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.provider.Settings; +import android.util.AttributeSet; +import android.view.accessibility.CaptioningManager; +import android.view.accessibility.CaptioningManager.CaptionStyle; + +import androidx.preference.PreferenceScreen; +import androidx.test.core.app.ApplicationProvider; + +import com.android.settings.core.BasePreferenceController; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.robolectric.Robolectric; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.shadow.api.Shadow; +import org.robolectric.shadows.ShadowCaptioningManager; + +/** Tests for {@link CaptionPresetController}. */ +@RunWith(RobolectricTestRunner.class) +public class CaptionPresetControllerTest { + + @Rule + public final MockitoRule mMockitoRule = MockitoJUnit.rule(); + @Mock + private PreferenceScreen mScreen; + private final Context mContext = ApplicationProvider.getApplicationContext(); + private CaptionPresetController mController; + private PresetPreference mPreference; + private ShadowCaptioningManager mShadowCaptioningManager; + + @Before + public void setUp() { + mController = new CaptionPresetController(mContext, "captioning_preset"); + final AttributeSet attributeSet = Robolectric.buildAttributeSet().build(); + mPreference = new PresetPreference(mContext, attributeSet); + when(mScreen.findPreference(mController.getPreferenceKey())).thenReturn(mPreference); + CaptioningManager captioningManager = mContext.getSystemService(CaptioningManager.class); + mShadowCaptioningManager = Shadow.extract(captioningManager); + } + + @Test + public void getAvailabilityStatus_shouldReturnAvailable() { + assertThat(mController.getAvailabilityStatus()) + .isEqualTo(BasePreferenceController.AVAILABLE); + } + + @Test + public void getSummary_defaultValue_shouldReturnWhiteOnBlack() { + mController.displayPreference(mScreen); + + assertThat(mPreference.getSummary().toString()).isEqualTo("White on black"); + } + + @Test + public void getSummary_customValue_shouldReturnCustom() { + Settings.Secure.putInt(mContext.getContentResolver(), + Settings.Secure.ACCESSIBILITY_CAPTIONING_PRESET, CaptionStyle.PRESET_CUSTOM); + + mController.displayPreference(mScreen); + + assertThat(mPreference.getSummary().toString()).isEqualTo("Custom"); + } + + @Test + public void setCustomValue_shouldReturnCustom() { + mController.displayPreference(mScreen); + + mPreference.setValue(CaptionStyle.PRESET_CUSTOM); + + assertThat(mPreference.getSummary().toString()).isEqualTo("Custom"); + } + + @Test + public void onValueChanged_shouldSetCaptionEnabled() { + mShadowCaptioningManager.setEnabled(false); + mController.displayPreference(mScreen); + + mController.onValueChanged(mPreference, CaptionStyle.PRESET_CUSTOM); + + final boolean isCaptionEnabled = Settings.Secure.getInt(mContext.getContentResolver(), + Settings.Secure.ACCESSIBILITY_CAPTIONING_ENABLED, OFF) == ON; + assertThat(isCaptionEnabled).isTrue(); + } +}