From 3ca8a0d5395fa8950234957f1057d4491a274427 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 7 Dec 2016 14:49:55 +0900 Subject: [PATCH 1/2] DO NOT MERGE Unit tests for NetworkNotificationManager Test: new unit test, no functional changes. Bug: 32198726 (cherry picked from commit 74264329da5c52fbbafc1d20457056fdeabe19dc) Change-Id: Ib8a725cdd8c708ccb9cffad62321e0db8b27e593 --- .../NetworkNotificationManager.java | 5 +- .../NetworkNotificationManagerTest.java | 142 ++++++++++++++++++ 2 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java diff --git a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java index c6bf4c5fcd..9ffa40b752 100644 --- a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java +++ b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java @@ -27,7 +27,7 @@ import android.net.NetworkCapabilities; import android.os.UserHandle; import android.telephony.TelephonyManager; import android.util.Slog; - +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.R; import static android.net.NetworkCapabilities.*; @@ -37,7 +37,8 @@ public class NetworkNotificationManager { public static enum NotificationType { SIGN_IN, NO_INTERNET, LOST_INTERNET, NETWORK_SWITCH }; - private static final String NOTIFICATION_ID = "Connectivity.Notification"; + @VisibleForTesting + static final String NOTIFICATION_ID = "Connectivity.Notification"; private static final String TAG = NetworkNotificationManager.class.getSimpleName(); private static final boolean DBG = true; diff --git a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java new file mode 100644 index 0000000000..813e928118 --- /dev/null +++ b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java @@ -0,0 +1,142 @@ +/* + * Copyright (C) 2016 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.connectivity; + +import android.app.Notification; +import android.app.NotificationManager; +import android.content.Context; +import android.content.pm.ApplicationInfo; +import android.content.pm.PackageManager; +import android.content.res.Resources; +import android.net.NetworkCapabilities; +import android.net.NetworkInfo; +import android.telephony.TelephonyManager; +import android.test.suitebuilder.annotation.SmallTest; +import com.android.server.connectivity.NetworkNotificationManager.NotificationType; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import junit.framework.TestCase; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; + +import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.*; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyInt; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class NetworkNotificationManagerTest extends TestCase { + + static final String NOTIFICATION_ID = NetworkNotificationManager.NOTIFICATION_ID; + + static final NetworkCapabilities CELL_CAPABILITIES = new NetworkCapabilities(); + static final NetworkCapabilities WIFI_CAPABILITIES = new NetworkCapabilities(); + static { + CELL_CAPABILITIES.addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR); + CELL_CAPABILITIES.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + + WIFI_CAPABILITIES.addTransportType(NetworkCapabilities.TRANSPORT_WIFI); + WIFI_CAPABILITIES.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + } + + @Mock Context mCtx; + @Mock Resources mResources; + @Mock PackageManager mPm; + @Mock TelephonyManager mTelephonyManager; + @Mock NotificationManager mNotificationManager; + @Mock NetworkAgentInfo mWifiNai; + @Mock NetworkAgentInfo mCellNai; + @Mock NetworkInfo mNetworkInfo; + ArgumentCaptor mCaptor; + + NetworkNotificationManager mManager; + + public void setUp() { + MockitoAnnotations.initMocks(this); + mCaptor = ArgumentCaptor.forClass(Notification.class); + mWifiNai.networkCapabilities = WIFI_CAPABILITIES; + mWifiNai.networkInfo = mNetworkInfo; + mCellNai.networkCapabilities = CELL_CAPABILITIES; + mCellNai.networkInfo = mNetworkInfo; + when(mCtx.getResources()).thenReturn(mResources); + when(mCtx.getPackageManager()).thenReturn(mPm); + when(mCtx.getApplicationInfo()).thenReturn(new ApplicationInfo()); + when(mResources.getColor(anyInt(), any())).thenReturn(0xFF607D8B); + + mManager = new NetworkNotificationManager(mCtx, mTelephonyManager, mNotificationManager); + } + + @SmallTest + public void testNotificationsShownAndCleared() { + final int NETWORK_ID_BASE = 100; + List types = Arrays.asList(NotificationType.values()); + List ids = new ArrayList<>(types.size()); + for (int i = 0; i < ids.size(); i++) { + ids.add(NETWORK_ID_BASE + i); + } + Collections.shuffle(ids); + Collections.shuffle(types); + + for (int i = 0; i < ids.size(); i++) { + mManager.showNotification(ids.get(i), types.get(i), mWifiNai, mCellNai, null, false); + } + + Collections.shuffle(ids); + for (int i = 0; i < ids.size(); i++) { + mManager.clearNotification(ids.get(i)); + } + + for (int i = 0; i < ids.size(); i++) { + final int expectedId = NETWORK_ID_BASE + i; + verify(mNotificationManager, times(1)) + .notifyAsUser(eq(NOTIFICATION_ID), eq(expectedId), any(), any()); + verify(mNotificationManager, times(1)) + .cancelAsUser(eq(NOTIFICATION_ID), eq(expectedId), any()); + } + } + + @SmallTest + public void testNoInternetNotificationsNotShownForCellular() { + mManager.showNotification(100, NO_INTERNET, mCellNai, mWifiNai, null, false); + mManager.showNotification(101, LOST_INTERNET, mCellNai, mWifiNai, null, false); + + verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any()); + + mManager.showNotification(102, NO_INTERNET, mWifiNai, mCellNai, null, false); + + verify(mNotificationManager, times(1)) + .notifyAsUser(eq(NOTIFICATION_ID), eq(102), any(), any()); + } + + @SmallTest + public void testNotificationsNotShownIfNoInternetCapability() { + mWifiNai.networkCapabilities = new NetworkCapabilities(); + mWifiNai.networkCapabilities .addTransportType(NetworkCapabilities.TRANSPORT_WIFI); + mManager.showNotification(102, NO_INTERNET, mWifiNai, mCellNai, null, false); + mManager.showNotification(103, LOST_INTERNET, mWifiNai, mCellNai, null, false); + mManager.showNotification(104, NETWORK_SWITCH, mWifiNai, mCellNai, null, false); + + verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any()); + } +} From e9c9d4bf0cb0275dd2b27ce2990fb27fe59650b3 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Thu, 8 Dec 2016 09:36:52 +0900 Subject: [PATCH 2/2] DO NOT MERGE Network notifications: revamp keying scheme This patch changes the (tag: String, id: Int) keying scheme for network notifications so that TRON notification counters can count network related notifications unambiguously. TRON instruments all notifications shown for package "android" as well as user interactions with these Notifications. These counters are grouped by id. However the network notifications ("no internet" dialog, "captive portal sign in" dialog, ...) use a static tag and a dynamic id for keying notifications, preventing the counters to correctly aggregate. In addition there is also the risk of collision with other SystemUi notification ids not managed by NetworkNotificationManager. In order to make the TRON counters useful for network notifications, the id is now encoding the network notification type in a stable way while the tag is used to uniquely identify network notifications. Test: change covered by previously added new unit tests. Bug: 32198726 Bug: 33030620 (cherry picked from commit fb2609d3eee1c7a4dda889c000f32183a044978a) Change-Id: Iadf7f15da38de28587090ed0395f15c24d4ad442 --- .../NetworkNotificationManager.java | 78 +++++++++++++++---- .../NetworkNotificationManagerTest.java | 17 ++-- 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java index 9ffa40b752..c051642a21 100644 --- a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java +++ b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java @@ -19,7 +19,6 @@ package com.android.server.connectivity; import android.app.Notification; import android.app.NotificationManager; import android.app.PendingIntent; -import android.widget.Toast; import android.content.Context; import android.content.Intent; import android.content.res.Resources; @@ -27,18 +26,40 @@ import android.net.NetworkCapabilities; import android.os.UserHandle; import android.telephony.TelephonyManager; import android.util.Slog; -import com.android.internal.annotations.VisibleForTesting; +import android.util.SparseArray; +import android.util.SparseIntArray; +import android.widget.Toast; import com.android.internal.R; +import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.logging.MetricsProto.MetricsEvent; -import static android.net.NetworkCapabilities.*; - +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; +import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; +import static android.net.NetworkCapabilities.TRANSPORT_WIFI; public class NetworkNotificationManager { - public static enum NotificationType { SIGN_IN, NO_INTERNET, LOST_INTERNET, NETWORK_SWITCH }; + public static enum NotificationType { + LOST_INTERNET(MetricsEvent.NOTIFICATION_NETWORK_LOST_INTERNET), + NETWORK_SWITCH(MetricsEvent.NOTIFICATION_NETWORK_SWITCH), + NO_INTERNET(MetricsEvent.NOTIFICATION_NETWORK_NO_INTERNET), + SIGN_IN(MetricsEvent.NOTIFICATION_NETWORK_SIGN_IN); - @VisibleForTesting - static final String NOTIFICATION_ID = "Connectivity.Notification"; + public final int eventId; + + NotificationType(int eventId) { + this.eventId = eventId; + Holder.sIdToTypeMap.put(eventId, this); + } + + private static class Holder { + private static SparseArray sIdToTypeMap = new SparseArray<>(); + } + + public static NotificationType getFromId(int id) { + return Holder.sIdToTypeMap.get(id); + } + }; private static final String TAG = NetworkNotificationManager.class.getSimpleName(); private static final boolean DBG = true; @@ -47,11 +68,14 @@ public class NetworkNotificationManager { private final Context mContext; private final TelephonyManager mTelephonyManager; private final NotificationManager mNotificationManager; + // Tracks the types of notifications managed by this instance, from creation to cancellation. + private final SparseIntArray mNotificationTypeMap; public NetworkNotificationManager(Context c, TelephonyManager t, NotificationManager n) { mContext = c; mTelephonyManager = t; mNotificationManager = n; + mNotificationTypeMap = new SparseIntArray(); } // TODO: deal more gracefully with multi-transport networks. @@ -101,8 +125,10 @@ public class NetworkNotificationManager { */ public void showNotification(int id, NotificationType notifyType, NetworkAgentInfo nai, NetworkAgentInfo switchToNai, PendingIntent intent, boolean highPriority) { - int transportType; - String extraInfo; + final String tag = tagFor(id); + final int eventId = notifyType.eventId; + final int transportType; + final String extraInfo; if (nai != null) { transportType = getFirstTransportType(nai); extraInfo = nai.networkInfo.getExtraInfo(); @@ -115,9 +141,9 @@ public class NetworkNotificationManager { } if (DBG) { - Slog.d(TAG, "showNotification id=" + id + " " + notifyType - + " transportType=" + getTransportName(transportType) - + " extraInfo=" + extraInfo + " highPriority=" + highPriority); + Slog.d(TAG, String.format( + "showNotification tag=%s event=%s transport=%s extraInfo=%d highPrioriy=%s", + tag, nameOf(eventId), getTransportName(transportType), extraInfo, highPriority)); } Resources r = Resources.getSystem(); @@ -185,22 +211,31 @@ public class NetworkNotificationManager { Notification notification = builder.build(); + mNotificationTypeMap.put(id, eventId); try { - mNotificationManager.notifyAsUser(NOTIFICATION_ID, id, notification, UserHandle.ALL); + mNotificationManager.notifyAsUser(tag, eventId, notification, UserHandle.ALL); } catch (NullPointerException npe) { Slog.d(TAG, "setNotificationVisible: visible notificationManager error", npe); } } public void clearNotification(int id) { + final String tag = tagFor(id); + if (mNotificationTypeMap.indexOfKey(id) < 0) { + Slog.e(TAG, "cannot clear unknown notification with tag=" + tag); + return; + } + final int eventId = mNotificationTypeMap.get(id); if (DBG) { - Slog.d(TAG, "clearNotification id=" + id); + Slog.d(TAG, String.format("clearing notification tag=%s event=", tag, nameOf(eventId))); } try { - mNotificationManager.cancelAsUser(NOTIFICATION_ID, id, UserHandle.ALL); + mNotificationManager.cancelAsUser(tag, eventId, UserHandle.ALL); } catch (NullPointerException npe) { - Slog.d(TAG, "setNotificationVisible: cancel notificationManager error", npe); + Slog.d(TAG, String.format( + "failed to clear notification tag=%s event=", tag, nameOf(eventId)), npe); } + mNotificationTypeMap.delete(id); } /** @@ -223,4 +258,15 @@ public class NetworkNotificationManager { R.string.network_switch_metered_toast, fromTransport, toTransport); Toast.makeText(mContext, text, Toast.LENGTH_LONG).show(); } + + @VisibleForTesting + static String tagFor(int id) { + return String.format("ConnectivityNotification:%d", id); + } + + @VisibleForTesting + static String nameOf(int eventId) { + NotificationType t = NotificationType.getFromId(eventId); + return (t != null) ? t.name() : "UNKNOWN"; + } } diff --git a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java index 813e928118..98073ce1e5 100644 --- a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java +++ b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java @@ -48,8 +48,6 @@ import static org.mockito.Mockito.when; public class NetworkNotificationManagerTest extends TestCase { - static final String NOTIFICATION_ID = NetworkNotificationManager.NOTIFICATION_ID; - static final NetworkCapabilities CELL_CAPABILITIES = new NetworkCapabilities(); static final NetworkCapabilities WIFI_CAPABILITIES = new NetworkCapabilities(); static { @@ -108,11 +106,11 @@ public class NetworkNotificationManagerTest extends TestCase { } for (int i = 0; i < ids.size(); i++) { - final int expectedId = NETWORK_ID_BASE + i; - verify(mNotificationManager, times(1)) - .notifyAsUser(eq(NOTIFICATION_ID), eq(expectedId), any(), any()); - verify(mNotificationManager, times(1)) - .cancelAsUser(eq(NOTIFICATION_ID), eq(expectedId), any()); + final int id = ids.get(i); + final int eventId = types.get(i).eventId; + final String tag = NetworkNotificationManager.tagFor(id); + verify(mNotificationManager, times(1)).notifyAsUser(eq(tag), eq(eventId), any(), any()); + verify(mNotificationManager, times(1)).cancelAsUser(eq(tag), eq(eventId), any()); } } @@ -125,8 +123,9 @@ public class NetworkNotificationManagerTest extends TestCase { mManager.showNotification(102, NO_INTERNET, mWifiNai, mCellNai, null, false); - verify(mNotificationManager, times(1)) - .notifyAsUser(eq(NOTIFICATION_ID), eq(102), any(), any()); + final int eventId = NO_INTERNET.eventId; + final String tag = NetworkNotificationManager.tagFor(102); + verify(mNotificationManager, times(1)).notifyAsUser(eq(tag), eq(eventId), any(), any()); } @SmallTest