From aa09aa02103954735b432fa3f7c41ee235bac9fd Mon Sep 17 00:00:00 2001 From: Amin Shaikh Date: Mon, 21 Nov 2016 17:27:53 -0800 Subject: [PATCH] Refactor NetworkScoreAppManager for testing. Test: runtest frameworks-services && runtest frameworks-core Change-Id: Ie2172009e9ba0438050488fe0aea6408f852c2c2 --- core/java/android/net/NetworkScoreManager.java | 2 +- core/java/android/net/NetworkScorerAppManager.java | 40 ++++++++++++---------- .../android/net/NetworkScorerAppManagerTest.java | 5 ++- .../com/android/server/NetworkScoreService.java | 28 +++++++++------ 4 files changed, 44 insertions(+), 31 deletions(-) diff --git a/core/java/android/net/NetworkScoreManager.java b/core/java/android/net/NetworkScoreManager.java index 01c160f1bc45..b93bf59d5ac4 100644 --- a/core/java/android/net/NetworkScoreManager.java +++ b/core/java/android/net/NetworkScoreManager.java @@ -144,7 +144,7 @@ public class NetworkScoreManager { * scorer. */ public String getActiveScorerPackage() { - NetworkScorerAppData app = NetworkScorerAppManager.getActiveScorer(mContext); + NetworkScorerAppData app = new NetworkScorerAppManager(mContext).getActiveScorer(); if (app == null) { return null; } diff --git a/core/java/android/net/NetworkScorerAppManager.java b/core/java/android/net/NetworkScorerAppManager.java index 29291ca90467..ebb31c9056a9 100644 --- a/core/java/android/net/NetworkScorerAppManager.java +++ b/core/java/android/net/NetworkScorerAppManager.java @@ -41,14 +41,17 @@ import java.util.List; * * @hide */ -public final class NetworkScorerAppManager { +public class NetworkScorerAppManager { private static final String TAG = "NetworkScorerAppManager"; private static final Intent SCORE_INTENT = new Intent(NetworkScoreManager.ACTION_SCORE_NETWORKS); - /** This class cannot be instantiated. */ - private NetworkScorerAppManager() {} + private final Context mContext; + + public NetworkScorerAppManager(Context context) { + mContext = context; + } public static class NetworkScorerAppData { /** Package name of this scorer app. */ @@ -108,7 +111,7 @@ public final class NetworkScorerAppManager { * * @return the list of scorers, or the empty list if there are no valid scorers. */ - public static Collection getAllValidScorers(Context context) { + public Collection getAllValidScorers() { // Network scorer apps can only run as the primary user so exit early if we're not the // primary user. if (UserHandle.getCallingUserId() != UserHandle.USER_SYSTEM) { @@ -116,7 +119,7 @@ public final class NetworkScorerAppManager { } List scorers = new ArrayList<>(); - PackageManager pm = context.getPackageManager(); + PackageManager pm = mContext.getPackageManager(); // Only apps installed under the primary user of the device can be scorers. // TODO: http://b/23422763 List receivers = @@ -179,10 +182,10 @@ public final class NetworkScorerAppManager { * selected) or if the previously-set scorer is no longer a valid scorer app (e.g. because * it was disabled or uninstalled). */ - public static NetworkScorerAppData getActiveScorer(Context context) { - String scorerPackage = Settings.Global.getString(context.getContentResolver(), + public NetworkScorerAppData getActiveScorer() { + String scorerPackage = Settings.Global.getString(mContext.getContentResolver(), Settings.Global.NETWORK_SCORER_APP); - return getScorer(context, scorerPackage); + return getScorer(scorerPackage); } /** @@ -190,13 +193,12 @@ public final class NetworkScorerAppManager { * *

The caller must have permission to write to {@link android.provider.Settings.Global}. * - * @param context the context of the calling application * @param packageName the packageName of the new scorer to use. If null, scoring will be * disabled. Otherwise, the scorer will only be set if it is a valid scorer application. * @return true if the scorer was changed, or false if the package is not a valid scorer. */ - public static boolean setActiveScorer(Context context, String packageName) { - String oldPackageName = Settings.Global.getString(context.getContentResolver(), + public boolean setActiveScorer(String packageName) { + String oldPackageName = Settings.Global.getString(mContext.getContentResolver(), Settings.Global.NETWORK_SCORER_APP); if (TextUtils.equals(oldPackageName, packageName)) { // No change. @@ -206,13 +208,13 @@ public final class NetworkScorerAppManager { Log.i(TAG, "Changing network scorer from " + oldPackageName + " to " + packageName); if (packageName == null) { - Settings.Global.putString(context.getContentResolver(), + Settings.Global.putString(mContext.getContentResolver(), Settings.Global.NETWORK_SCORER_APP, null); return true; } else { // We only make the change if the new package is valid. - if (getScorer(context, packageName) != null) { - Settings.Global.putString(context.getContentResolver(), + if (getScorer(packageName) != null) { + Settings.Global.putString(mContext.getContentResolver(), Settings.Global.NETWORK_SCORER_APP, packageName); return true; } else { @@ -223,8 +225,8 @@ public final class NetworkScorerAppManager { } /** Determine whether the application with the given UID is the enabled scorer. */ - public static boolean isCallerActiveScorer(Context context, int callingUid) { - NetworkScorerAppData defaultApp = getActiveScorer(context); + public boolean isCallerActiveScorer(int callingUid) { + NetworkScorerAppData defaultApp = getActiveScorer(); if (defaultApp == null) { return false; } @@ -233,16 +235,16 @@ public final class NetworkScorerAppManager { } // To be extra safe, ensure the caller holds the SCORE_NETWORKS permission. It always // should, since it couldn't become the active scorer otherwise, but this can't hurt. - return context.checkCallingPermission(Manifest.permission.SCORE_NETWORKS) == + return mContext.checkCallingPermission(Manifest.permission.SCORE_NETWORKS) == PackageManager.PERMISSION_GRANTED; } /** Returns the {@link NetworkScorerAppData} for the given app, or null if it's not a scorer. */ - public static NetworkScorerAppData getScorer(Context context, String packageName) { + public NetworkScorerAppData getScorer(String packageName) { if (TextUtils.isEmpty(packageName)) { return null; } - Collection applications = getAllValidScorers(context); + Collection applications = getAllValidScorers(); for (NetworkScorerAppData app : applications) { if (packageName.equals(app.mPackageName)) { return app; diff --git a/core/tests/coretests/src/android/net/NetworkScorerAppManagerTest.java b/core/tests/coretests/src/android/net/NetworkScorerAppManagerTest.java index e7aca78a0720..02c25170bb74 100644 --- a/core/tests/coretests/src/android/net/NetworkScorerAppManagerTest.java +++ b/core/tests/coretests/src/android/net/NetworkScorerAppManagerTest.java @@ -42,6 +42,8 @@ public class NetworkScorerAppManagerTest extends InstrumentationTestCase { @Mock private Context mMockContext; @Mock private PackageManager mMockPm; + private NetworkScorerAppManager mNetworkScorerAppManager; + @Override public void setUp() throws Exception { super.setUp(); @@ -54,6 +56,7 @@ public class NetworkScorerAppManagerTest extends InstrumentationTestCase { MockitoAnnotations.initMocks(this); Mockito.when(mMockContext.getPackageManager()).thenReturn(mMockPm); + mNetworkScorerAppManager = new NetworkScorerAppManager(mMockContext); } public void testGetAllValidScorers() throws Exception { @@ -81,7 +84,7 @@ public class NetworkScorerAppManagerTest extends InstrumentationTestCase { setScorers(scorers); Iterator result = - NetworkScorerAppManager.getAllValidScorers(mMockContext).iterator(); + mNetworkScorerAppManager.getAllValidScorers().iterator(); assertTrue(result.hasNext()); NetworkScorerAppData next = result.next(); diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java index 83d6344c95b0..756cb301fefa 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -41,6 +41,7 @@ import android.util.Log; import com.android.internal.R; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.content.PackageMonitor; import java.io.FileDescriptor; @@ -61,6 +62,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { private static final boolean DBG = false; private final Context mContext; + private final NetworkScorerAppManager mNetworkScorerAppManager; private final Map mScoreCaches; /** Lock used to update mPackageMonitor when scorer package changes occur. */ private final Object mPackageMonitorLock = new Object[0]; @@ -131,7 +133,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { + ", forceUnbind=" + forceUnbind); } final NetworkScorerAppData activeScorer = - NetworkScorerAppManager.getActiveScorer(mContext); + mNetworkScorerAppManager.getActiveScorer(); if (activeScorer == null) { // Package change has invalidated a scorer, this will also unbind any service // connection. @@ -152,7 +154,13 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } public NetworkScoreService(Context context) { + this(context, new NetworkScorerAppManager(context)); + } + + @VisibleForTesting + NetworkScoreService(Context context, NetworkScorerAppManager networkScoreAppManager) { mContext = context; + mNetworkScorerAppManager = networkScoreAppManager; mScoreCaches = new HashMap<>(); IntentFilter filter = new IntentFilter(Intent.ACTION_USER_UNLOCKED); // TODO: Need to update when we support per-user scorers. http://b/23422763 @@ -171,7 +179,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { String defaultPackage = mContext.getResources().getString( R.string.config_defaultNetworkScorerPackageName); if (!TextUtils.isEmpty(defaultPackage)) { - NetworkScorerAppManager.setActiveScorer(mContext, defaultPackage); + mNetworkScorerAppManager.setActiveScorer(defaultPackage); } Settings.Global.putInt(cr, Settings.Global.NETWORK_SCORING_PROVISIONED, 1); } @@ -192,7 +200,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { private void registerPackageMonitorIfNeeded() { if (DBG) Log.d(TAG, "registerPackageMonitorIfNeeded"); - NetworkScorerAppData scorer = NetworkScorerAppManager.getActiveScorer(mContext); + NetworkScorerAppData scorer = mNetworkScorerAppManager.getActiveScorer(); synchronized (mPackageMonitorLock) { // Unregister the current monitor if needed. if (mPackageMonitor != null) { @@ -220,7 +228,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { private void bindToScoringServiceIfNeeded() { if (DBG) Log.d(TAG, "bindToScoringServiceIfNeeded"); - NetworkScorerAppData scorerData = NetworkScorerAppManager.getActiveScorer(mContext); + NetworkScorerAppData scorerData = mNetworkScorerAppManager.getActiveScorer(); bindToScoringServiceIfNeeded(scorerData); } @@ -257,7 +265,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public boolean updateScores(ScoredNetwork[] networks) { - if (!NetworkScorerAppManager.isCallerActiveScorer(mContext, getCallingUid())) { + if (!mNetworkScorerAppManager.isCallerActiveScorer(getCallingUid())) { throw new SecurityException("Caller with UID " + getCallingUid() + " is not the active scorer."); } @@ -296,7 +304,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { public boolean clearScores() { // Only the active scorer or the system (who can broadcast BROADCAST_NETWORK_PRIVILEGED) // should be allowed to flush all scores. - if (NetworkScorerAppManager.isCallerActiveScorer(mContext, getCallingUid()) || + if (mNetworkScorerAppManager.isCallerActiveScorer(getCallingUid()) || mContext.checkCallingOrSelfPermission(permission.BROADCAST_NETWORK_PRIVILEGED) == PackageManager.PERMISSION_GRANTED) { clearInternal(); @@ -326,7 +334,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { public void disableScoring() { // Only the active scorer or the system (who can broadcast BROADCAST_NETWORK_PRIVILEGED) // should be allowed to disable scoring. - if (NetworkScorerAppManager.isCallerActiveScorer(mContext, getCallingUid()) || + if (mNetworkScorerAppManager.isCallerActiveScorer(getCallingUid()) || mContext.checkCallingOrSelfPermission(permission.BROADCAST_NETWORK_PRIVILEGED) == PackageManager.PERMISSION_GRANTED) { // The return value is discarded here because at this point, the call should always @@ -350,8 +358,8 @@ public class NetworkScoreService extends INetworkScoreService.Stub { // only be allowing valid apps to be set as scorers, so failure here should be rare. clearInternal(); // Get the scorer that is about to be replaced, if any, so we can notify it directly. - NetworkScorerAppData prevScorer = NetworkScorerAppManager.getActiveScorer(mContext); - boolean result = NetworkScorerAppManager.setActiveScorer(mContext, packageName); + NetworkScorerAppData prevScorer = mNetworkScorerAppManager.getActiveScorer(); + boolean result = mNetworkScorerAppManager.setActiveScorer(packageName); // Unconditionally attempt to bind to the current scorer. If setActiveScorer() failed // then we'll attempt to restore the previous binding (if any), otherwise an attempt // will be made to bind to the new scorer. @@ -409,7 +417,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override protected void dump(FileDescriptor fd, PrintWriter writer, String[] args) { mContext.enforceCallingOrSelfPermission(permission.DUMP, TAG); - NetworkScorerAppData currentScorer = NetworkScorerAppManager.getActiveScorer(mContext); + NetworkScorerAppData currentScorer = mNetworkScorerAppManager.getActiveScorer(); if (currentScorer == null) { writer.println("Scoring is disabled."); return; -- 2.11.0