diff options
8 files changed, 112 insertions, 35 deletions
diff --git a/service/java/com/android/server/wifi/SoftApBackupRestore.java b/service/java/com/android/server/wifi/SoftApBackupRestore.java index 6034e8f1a..441321cd3 100644 --- a/service/java/com/android/server/wifi/SoftApBackupRestore.java +++ b/service/java/com/android/server/wifi/SoftApBackupRestore.java @@ -25,6 +25,7 @@ import android.util.BackupUtils; import android.util.Log; import com.android.server.wifi.util.ApConfigUtil; +import com.android.server.wifi.util.SettingsMigrationDataHolder; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -54,9 +55,12 @@ public class SoftApBackupRestore { private static final int ETHER_ADDR_LEN = 6; // Byte array size of MacAddress private final Context mContext; + private final SettingsMigrationDataHolder mSettingsMigrationDataHolder; - public SoftApBackupRestore(Context context) { + public SoftApBackupRestore(Context context, + SettingsMigrationDataHolder settingsMigrationDataHolder) { mContext = context; + mSettingsMigrationDataHolder = settingsMigrationDataHolder; } /** @@ -163,7 +167,7 @@ public class SoftApBackupRestore { } else { // Migrate data out of settings. WifiMigration.SettingsMigrationData migrationData = - WifiMigration.loadFromSettings(mContext); + mSettingsMigrationDataHolder.retrieveData(); if (migrationData == null) { Log.e(TAG, "No migration data present"); } else { diff --git a/service/java/com/android/server/wifi/SoftApStoreData.java b/service/java/com/android/server/wifi/SoftApStoreData.java index ac32ae66a..ae9a40af9 100644 --- a/service/java/com/android/server/wifi/SoftApStoreData.java +++ b/service/java/com/android/server/wifi/SoftApStoreData.java @@ -25,6 +25,7 @@ import android.text.TextUtils; import android.util.Log; import com.android.server.wifi.util.ApConfigUtil; +import com.android.server.wifi.util.SettingsMigrationDataHolder; import com.android.server.wifi.util.WifiConfigStoreEncryptionUtil; import com.android.server.wifi.util.XmlUtil; @@ -59,6 +60,7 @@ public class SoftApStoreData implements WifiConfigStore.StoreData { private static final String XML_TAG_ALLOWED_CLIENT_LIST = "AllowedClientList"; private final Context mContext; + private final SettingsMigrationDataHolder mSettingsMigrationDataHolder; private final DataSource mDataSource; /** @@ -95,8 +97,10 @@ public class SoftApStoreData implements WifiConfigStore.StoreData { * * @param dataSource The DataSource that implements the update and retrieval of the SSID set. */ - SoftApStoreData(Context context, DataSource dataSource) { + SoftApStoreData(Context context, SettingsMigrationDataHolder settingsMigrationDataHolder, + DataSource dataSource) { mContext = context; + mSettingsMigrationDataHolder = settingsMigrationDataHolder; mDataSource = dataSource; } @@ -265,7 +269,7 @@ public class SoftApStoreData implements WifiConfigStore.StoreData { if (!autoShutdownEnabledTagPresent) { // Migrate data out of settings. WifiMigration.SettingsMigrationData migrationData = - WifiMigration.loadFromSettings(mContext); + mSettingsMigrationDataHolder.retrieveData(); if (migrationData == null) { Log.e(TAG, "No migration data present"); } else { diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java index 87e161612..f388d2370 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -54,6 +54,7 @@ import com.android.server.wifi.p2p.WifiP2pMonitor; import com.android.server.wifi.p2p.WifiP2pNative; import com.android.server.wifi.rtt.RttMetrics; import com.android.server.wifi.util.NetdWrapper; +import com.android.server.wifi.util.SettingsMigrationDataHolder; import com.android.server.wifi.util.TelephonyUtil; import com.android.server.wifi.util.WifiPermissionsUtil; import com.android.server.wifi.util.WifiPermissionsWrapper; @@ -160,6 +161,7 @@ public class WifiInjector { private final WifiSettingsConfigStore mSettingsConfigStore; private final WifiScanAlwaysAvailableSettingsCompatibility mWifiScanAlwaysAvailableSettingsCompatibility; + private final SettingsMigrationDataHolder mSettingsMigrationDataHolder; public WifiInjector(Context context) { if (context == null) { @@ -185,6 +187,7 @@ public class WifiInjector { mFrameworkFacade = new FrameworkFacade(); mMacAddressUtil = new MacAddressUtil(); mContext = context; + mSettingsMigrationDataHolder = new SettingsMigrationDataHolder(mContext); mConnectionFailureNotificationBuilder = new ConnectionFailureNotificationBuilder( mContext, getWifiStackPackageName(), mFrameworkFacade); mBatteryStats = context.getSystemService(BatteryStatsManager.class); @@ -198,7 +201,7 @@ public class WifiInjector { mWifiPermissionsUtil = new WifiPermissionsUtil(mWifiPermissionsWrapper, mContext, mUserManager, this); mWifiBackupRestore = new WifiBackupRestore(mWifiPermissionsUtil); - mSoftApBackupRestore = new SoftApBackupRestore(mContext); + mSoftApBackupRestore = new SoftApBackupRestore(mContext, mSettingsMigrationDataHolder); mWifiStateTracker = new WifiStateTracker(mBatteryStats); mWifiThreadRunner = new WifiThreadRunner(wifiHandler); mWifiP2pServiceHandlerThread = new HandlerThread("WifiP2pService"); @@ -264,8 +267,8 @@ public class WifiInjector { new NetworkListUserStoreData(mContext), new RandomizedMacStoreData(), mFrameworkFacade, wifiHandler, mDeviceConfigFacade, mWifiScoreCard); - mSettingsConfigStore = new WifiSettingsConfigStore(context, wifiHandler, mWifiConfigManager, - mWifiConfigStore); + mSettingsConfigStore = new WifiSettingsConfigStore(context, wifiHandler, + mSettingsMigrationDataHolder, mWifiConfigManager, mWifiConfigStore); mSettingsStore = new WifiSettingsStore(mContext, mSettingsConfigStore); mWifiMetrics.setWifiConfigManager(mWifiConfigManager); @@ -679,7 +682,7 @@ public class WifiInjector { */ public SoftApStoreData makeSoftApStoreData( SoftApStoreData.DataSource dataSource) { - return new SoftApStoreData(mContext, dataSource); + return new SoftApStoreData(mContext, mSettingsMigrationDataHolder, dataSource); } public WifiPermissionsUtil getWifiPermissionsUtil() { diff --git a/service/java/com/android/server/wifi/WifiSettingsConfigStore.java b/service/java/com/android/server/wifi/WifiSettingsConfigStore.java index 872d0d437..d77aa4803 100644 --- a/service/java/com/android/server/wifi/WifiSettingsConfigStore.java +++ b/service/java/com/android/server/wifi/WifiSettingsConfigStore.java @@ -25,6 +25,7 @@ import android.text.TextUtils; import android.util.Log; import com.android.internal.annotations.GuardedBy; +import com.android.server.wifi.util.SettingsMigrationDataHolder; import com.android.server.wifi.util.WifiConfigStoreEncryptionUtil; import com.android.server.wifi.util.XmlUtil; @@ -85,6 +86,7 @@ public class WifiSettingsConfigStore { private final Context mContext; private final Handler mHandler; + private final SettingsMigrationDataHolder mSettingsMigrationDataHolder; private final WifiConfigManager mWifiConfigManager; private final Object mLock = new Object(); @@ -112,10 +114,12 @@ public class WifiSettingsConfigStore { } public WifiSettingsConfigStore(@NonNull Context context, @NonNull Handler handler, + @NonNull SettingsMigrationDataHolder settingsMigrationDataHolder, @NonNull WifiConfigManager wifiConfigManager, @NonNull WifiConfigStore wifiConfigStore) { mContext = context; mHandler = handler; + mSettingsMigrationDataHolder = settingsMigrationDataHolder; mWifiConfigManager = wifiConfigManager; // Register our data store. @@ -176,7 +180,7 @@ public class WifiSettingsConfigStore { private void migrateFromSettingsIfNeeded() { if (!mSettings.isEmpty()) return; // already migrated. - mCachedMigrationData = WifiMigration.loadFromSettings(mContext); + mCachedMigrationData = mSettingsMigrationDataHolder.retrieveData(); if (mCachedMigrationData == null) { Log.e(TAG, "No settings data to migrate"); return; diff --git a/service/java/com/android/server/wifi/util/SettingsMigrationDataHolder.java b/service/java/com/android/server/wifi/util/SettingsMigrationDataHolder.java new file mode 100644 index 000000000..cdd0391be --- /dev/null +++ b/service/java/com/android/server/wifi/util/SettingsMigrationDataHolder.java @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2020 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.server.wifi.util; + +import android.annotation.Nullable; +import android.content.Context; +import android.net.wifi.WifiMigration; + +/** + * Holder for storing the migration settings data retrieved from + * {@link android.net.wifi.WifiMigration#loadFromSettings(Context)} to avoid invoking the method + * multiple times. + */ +public class SettingsMigrationDataHolder { + private final Context mContext; + private WifiMigration.SettingsMigrationData mData = null; + private boolean mRetrieved = false; + + public SettingsMigrationDataHolder(Context context) { + mContext = context; + } + + private void retrieveDataIfNecessary() { + if (mRetrieved) return; + mData = WifiMigration.loadFromSettings(mContext); + mRetrieved = true; + } + + /** + * Retrieve the cached data returned from {@link WifiMigration#loadFromSettings(Context)}. + */ + @Nullable + public WifiMigration.SettingsMigrationData retrieveData() { + retrieveDataIfNecessary(); + return mData; + } +} diff --git a/tests/wifitests/src/com/android/server/wifi/SoftApBackupRestoreTest.java b/tests/wifitests/src/com/android/server/wifi/SoftApBackupRestoreTest.java index 56ba8878f..a1b917431 100644 --- a/tests/wifitests/src/com/android/server/wifi/SoftApBackupRestoreTest.java +++ b/tests/wifitests/src/com/android/server/wifi/SoftApBackupRestoreTest.java @@ -30,16 +30,14 @@ import android.util.BackupUtils; import androidx.test.filters.SmallTest; -import com.android.dx.mockito.inline.extended.ExtendedMockito; import com.android.server.wifi.util.ApConfigUtil; +import com.android.server.wifi.util.SettingsMigrationDataHolder; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import org.mockito.MockitoSession; -import org.mockito.quality.Strictness; import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; @@ -55,6 +53,7 @@ import java.util.List; public class SoftApBackupRestoreTest extends WifiBaseTest { @Mock private Context mContext; + @Mock private SettingsMigrationDataHolder mSettingsMigrationDataHolder; @Mock private WifiMigration.SettingsMigrationData mOemMigrationData; private SoftApBackupRestore mSoftApBackupRestore; private final ArrayList<MacAddress> mTestBlockedList = new ArrayList<>(); @@ -72,8 +71,6 @@ public class SoftApBackupRestoreTest extends WifiBaseTest { private static final int TEST_CHANNEL = 40; private static final boolean TEST_HIDDEN = false; - MockitoSession mSession; - /** * Asserts that the WifiConfigurations equal to SoftApConfiguration. * This only compares the elements saved @@ -100,20 +97,14 @@ public class SoftApBackupRestoreTest extends WifiBaseTest { public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - mSession = ExtendedMockito.mockitoSession() - .mockStatic(WifiMigration.class, withSettings().lenient()) - .strictness(Strictness.LENIENT) - .startMocking(); - when(WifiMigration.loadFromSettings(any(Context.class))) - .thenReturn(mOemMigrationData); + when(mSettingsMigrationDataHolder.retrieveData()).thenReturn(mOemMigrationData); when(mOemMigrationData.isSoftApTimeoutEnabled()).thenReturn(true); - mSoftApBackupRestore = new SoftApBackupRestore(mContext); + mSoftApBackupRestore = new SoftApBackupRestore(mContext, mSettingsMigrationDataHolder); } @After public void tearDown() throws Exception { - mSession.finishMocking(); } /** diff --git a/tests/wifitests/src/com/android/server/wifi/SoftApStoreDataTest.java b/tests/wifitests/src/com/android/server/wifi/SoftApStoreDataTest.java index 04955f806..739987106 100644 --- a/tests/wifitests/src/com/android/server/wifi/SoftApStoreDataTest.java +++ b/tests/wifitests/src/com/android/server/wifi/SoftApStoreDataTest.java @@ -26,7 +26,6 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.mockito.Mockito.withSettings; import android.content.Context; import android.net.MacAddress; @@ -37,8 +36,8 @@ import android.util.Xml; import androidx.test.filters.SmallTest; -import com.android.dx.mockito.inline.extended.ExtendedMockito; import com.android.internal.util.FastXmlSerializer; +import com.android.server.wifi.util.SettingsMigrationDataHolder; import com.android.server.wifi.util.WifiConfigStoreEncryptionUtil; import org.junit.After; @@ -47,8 +46,6 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import org.mockito.MockitoSession; -import org.mockito.quality.Strictness; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlSerializer; @@ -187,21 +184,17 @@ public class SoftApStoreDataTest extends WifiBaseTest { @Mock private Context mContext; @Mock SoftApStoreData.DataSource mDataSource; @Mock private WifiMigration.SettingsMigrationData mOemMigrationData; - MockitoSession mSession; + @Mock private SettingsMigrationDataHolder mSettingsMigrationDataHolder; SoftApStoreData mSoftApStoreData; @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - mSession = ExtendedMockito.mockitoSession() - .mockStatic(WifiMigration.class, withSettings().lenient()) - .strictness(Strictness.LENIENT) - .startMocking(); - when(WifiMigration.loadFromSettings(any(Context.class))) + when(mSettingsMigrationDataHolder.retrieveData()) .thenReturn(mOemMigrationData); when(mOemMigrationData.isSoftApTimeoutEnabled()).thenReturn(true); - mSoftApStoreData = new SoftApStoreData(mContext, mDataSource); + mSoftApStoreData = new SoftApStoreData(mContext, mSettingsMigrationDataHolder, mDataSource); TEST_BLOCKEDLIST.add(MacAddress.fromString(TEST_BLOCKED_CLIENT)); TEST_ALLOWEDLIST.add(MacAddress.fromString(TEST_ALLOWED_CLIENT)); } @@ -213,7 +206,6 @@ public class SoftApStoreDataTest extends WifiBaseTest { public void cleanup() { TEST_BLOCKEDLIST.clear(); TEST_ALLOWEDLIST.clear(); - mSession.finishMocking(); } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiSettingsConfigStoreTest.java b/tests/wifitests/src/com/android/server/wifi/WifiSettingsConfigStoreTest.java index 2471e090e..e2e2575d8 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiSettingsConfigStoreTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiSettingsConfigStoreTest.java @@ -25,8 +25,10 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.validateMockitoUsage; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import android.content.Context; +import android.net.wifi.WifiMigration; import android.os.Handler; import android.os.test.TestLooper; import android.util.Xml; @@ -35,6 +37,7 @@ import androidx.test.filters.SmallTest; import com.android.internal.util.FastXmlSerializer; import com.android.server.wifi.WifiSettingsConfigStore.Key; +import com.android.server.wifi.util.SettingsMigrationDataHolder; import com.android.server.wifi.util.XmlUtil; import org.junit.After; @@ -61,6 +64,8 @@ public class WifiSettingsConfigStoreTest extends WifiBaseTest { @Mock private Context mContext; @Mock + private SettingsMigrationDataHolder mSettingsMigrationDataHolder; + @Mock private WifiConfigStore mWifiConfigStore; @Mock private WifiConfigManager mWifiConfigManager; @@ -75,7 +80,7 @@ public class WifiSettingsConfigStoreTest extends WifiBaseTest { mLooper = new TestLooper(); mWifiSettingsConfigStore = new WifiSettingsConfigStore(mContext, new Handler(mLooper.getLooper()), - mWifiConfigManager, mWifiConfigStore); + mSettingsMigrationDataHolder, mWifiConfigManager, mWifiConfigStore); } /** @@ -124,6 +129,30 @@ public class WifiSettingsConfigStoreTest extends WifiBaseTest { storeDataCaptor.getValue().deserializeData(in, in.getDepth(), -1, null); assertTrue(mWifiSettingsConfigStore.get(WIFI_VERBOSE_LOGGING_ENABLED)); + // verify that we did not trigger migration. + verifyNoMoreInteractions(mSettingsMigrationDataHolder); + } + + @Test + public void testLoadFromMigration() throws Exception { + ArgumentCaptor<WifiConfigStore.StoreData> storeDataCaptor = ArgumentCaptor.forClass( + WifiConfigStore.StoreData.class); + verify(mWifiConfigStore).registerStoreData(storeDataCaptor.capture()); + assertNotNull(storeDataCaptor.getValue()); + + WifiMigration.SettingsMigrationData migrationData = mock( + WifiMigration.SettingsMigrationData.class); + when(mSettingsMigrationDataHolder.retrieveData()).thenReturn(migrationData); + when(migrationData.isVerboseLoggingEnabled()).thenReturn(true); + + // indicate that there is not data in the store file to trigger migration. + storeDataCaptor.getValue().resetData(); + storeDataCaptor.getValue().deserializeData(null, -1, -1, null); + mLooper.dispatchAll(); + + assertTrue(mWifiSettingsConfigStore.get(WIFI_VERBOSE_LOGGING_ENABLED)); + // Trigger store file write after migration. + verify(mWifiConfigManager).saveToStore(true); } private XmlPullParser createSettingsTestXmlForParsing(Key key, Object value) |