diff options
| -rw-r--r-- | src/android/net/ip/IpClient.java | 82 | ||||
| -rw-r--r-- | src/android/net/ip/IpClientLinkObserver.java | 378 | ||||
| -rw-r--r-- | src/com/android/server/NetworkObserver.java | 5 | ||||
| -rw-r--r-- | src/com/android/server/NetworkObserverRegistry.java | 10 | ||||
| -rw-r--r-- | src/com/android/server/NetworkStackService.java | 8 | ||||
| -rw-r--r-- | tests/src/android/net/ip/IpClientTest.java | 41 |
6 files changed, 439 insertions, 85 deletions
diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java index f20e016..612ebf7 100644 --- a/src/android/net/ip/IpClient.java +++ b/src/android/net/ip/IpClient.java @@ -40,15 +40,12 @@ import android.net.ip.IIpClientCallbacks; import android.net.metrics.IpConnectivityLog; import android.net.metrics.IpManagerEvent; import android.net.shared.InitialConfiguration; -import android.net.shared.NetdService; import android.net.shared.ProvisioningConfiguration; import android.net.util.InterfaceParams; import android.net.util.SharedLog; import android.os.ConditionVariable; -import android.os.INetworkManagementService; import android.os.Message; import android.os.RemoteException; -import android.os.ServiceManager; import android.os.SystemClock; import android.text.TextUtils; import android.util.LocalLog; @@ -64,7 +61,7 @@ import com.android.internal.util.Preconditions; import com.android.internal.util.State; import com.android.internal.util.StateMachine; import com.android.internal.util.WakeupMessage; -import com.android.server.net.NetlinkTracker; +import com.android.server.NetworkObserverRegistry; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -338,8 +335,9 @@ public class IpClient extends StateMachine { private final Dependencies mDependencies; private final CountDownLatch mShutdownLatch; private final ConnectivityManager mCm; - private final INetworkManagementService mNwService; - private final NetlinkTracker mNetlinkTracker; + private final INetd mNetd; + private final NetworkObserverRegistry mObserverRegistry; + private final IpClientLinkObserver mLinkObserver; private final WakeupMessage mProvisioningTimeoutAlarm; private final WakeupMessage mDhcpActionTimeoutAlarm; private final SharedLog mLog; @@ -373,15 +371,6 @@ public class IpClient extends StateMachine { private final ConditionVariable mApfDataSnapshotComplete = new ConditionVariable(); public static class Dependencies { - public INetworkManagementService getNMS() { - return INetworkManagementService.Stub.asInterface( - ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE)); - } - - public INetd getNetd() { - return NetdService.getInstance(); - } - /** * Get interface parameters for the specified interface. */ @@ -390,26 +379,14 @@ public class IpClient extends StateMachine { } } - public IpClient(Context context, String ifName, IIpClientCallbacks callback) { - this(context, ifName, callback, new Dependencies()); - } - - /** - * An expanded constructor, useful for dependency injection. - * TODO: migrate all test users to mock IpClient directly and remove this ctor. - */ public IpClient(Context context, String ifName, IIpClientCallbacks callback, - INetworkManagementService nwService) { - this(context, ifName, callback, new Dependencies() { - @Override - public INetworkManagementService getNMS() { - return nwService; - } - }); + NetworkObserverRegistry observerRegistry) { + this(context, ifName, callback, observerRegistry, new Dependencies()); } @VisibleForTesting - IpClient(Context context, String ifName, IIpClientCallbacks callback, Dependencies deps) { + IpClient(Context context, String ifName, IIpClientCallbacks callback, + NetworkObserverRegistry observerRegistry, Dependencies deps) { super(IpClient.class.getSimpleName() + "." + ifName); Preconditions.checkNotNull(ifName); Preconditions.checkNotNull(callback); @@ -422,7 +399,7 @@ public class IpClient extends StateMachine { mDependencies = deps; mShutdownLatch = new CountDownLatch(1); mCm = mContext.getSystemService(ConnectivityManager.class); - mNwService = deps.getNMS(); + mObserverRegistry = observerRegistry; sSmLogs.putIfAbsent(mInterfaceName, new SharedLog(MAX_LOG_RECORDS, mTag)); mLog = sSmLogs.get(mInterfaceName); @@ -433,19 +410,15 @@ public class IpClient extends StateMachine { // TODO: Consider creating, constructing, and passing in some kind of // InterfaceController.Dependencies class. - mInterfaceCtrl = new InterfaceController(mInterfaceName, deps.getNetd(), mLog); + mNetd = mContext.getSystemService(INetd.class); + mInterfaceCtrl = new InterfaceController(mInterfaceName, mNetd, mLog); - mNetlinkTracker = new NetlinkTracker( + mLinkObserver = new IpClientLinkObserver( mInterfaceName, - new NetlinkTracker.Callback() { - @Override - public void update() { - sendMessage(EVENT_NETLINK_LINKPROPERTIES_CHANGED); - } - }) { + () -> sendMessage(EVENT_NETLINK_LINKPROPERTIES_CHANGED)) { @Override - public void interfaceAdded(String iface) { - super.interfaceAdded(iface); + public void onInterfaceAdded(String iface) { + super.onInterfaceAdded(iface); if (mClatInterfaceName.equals(iface)) { mCallback.setNeighborDiscoveryOffload(false); } else if (!mInterfaceName.equals(iface)) { @@ -457,8 +430,8 @@ public class IpClient extends StateMachine { } @Override - public void interfaceRemoved(String iface) { - super.interfaceRemoved(iface); + public void onInterfaceRemoved(String iface) { + super.onInterfaceRemoved(iface); // TODO: Also observe mInterfaceName going down and take some // kind of appropriate action. if (mClatInterfaceName.equals(iface)) { @@ -570,19 +543,11 @@ public class IpClient extends StateMachine { } private void startStateMachineUpdaters() { - try { - mNwService.registerObserver(mNetlinkTracker); - } catch (RemoteException e) { - logError("Couldn't register NetlinkTracker: %s", e); - } + mObserverRegistry.registerObserver(mLinkObserver, getHandler()); } private void stopStateMachineUpdaters() { - try { - mNwService.unregisterObserver(mNetlinkTracker); - } catch (RemoteException e) { - logError("Couldn't unregister NetlinkTracker: %s", e); - } + mObserverRegistry.unregisterObserver(mLinkObserver); } @Override @@ -805,7 +770,7 @@ public class IpClient extends StateMachine { // we should only call this if we know for sure that there are no IP addresses // assigned to the interface, etc. private void resetLinkProperties() { - mNetlinkTracker.clearLinkProperties(); + mLinkObserver.clearLinkProperties(); mConfiguration = null; mDhcpResults = null; mTcpBufferSizes = ""; @@ -984,10 +949,10 @@ public class IpClient extends StateMachine { // - IPv6 DNS servers // // N.B.: this is fundamentally race-prone and should be fixed by - // changing NetlinkTracker from a hybrid edge/level model to an + // changing IpClientLinkObserver from a hybrid edge/level model to an // edge-only model, or by giving IpClient its own netlink socket(s) // so as to track all required information directly. - LinkProperties netlinkLinkProperties = mNetlinkTracker.getLinkProperties(); + LinkProperties netlinkLinkProperties = mLinkObserver.getLinkProperties(); newLp.setLinkAddresses(netlinkLinkProperties.getLinkAddresses()); for (RouteInfo route : netlinkLinkProperties.getRoutes()) { newLp.addRoute(route); @@ -1166,8 +1131,7 @@ public class IpClient extends StateMachine { // necessary or does reading from settings at startup suffice?). final int numSolicits = 5; final int interSolicitIntervalMs = 750; - setNeighborParameters(mDependencies.getNetd(), mInterfaceName, - numSolicits, interSolicitIntervalMs); + setNeighborParameters(mNetd, mInterfaceName, numSolicits, interSolicitIntervalMs); } catch (Exception e) { mLog.e("Failed to adjust neighbor parameters", e); // Carry on using the system defaults (currently: 3, 1000); diff --git a/src/android/net/ip/IpClientLinkObserver.java b/src/android/net/ip/IpClientLinkObserver.java new file mode 100644 index 0000000..8ad99aa --- /dev/null +++ b/src/android/net/ip/IpClientLinkObserver.java @@ -0,0 +1,378 @@ +/* + * Copyright (C) 2014 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 android.net.ip; + +import android.net.InetAddresses; +import android.net.LinkAddress; +import android.net.LinkProperties; +import android.net.RouteInfo; +import android.util.Log; + +import com.android.server.NetworkObserver; + +import java.net.InetAddress; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Set; + +/** + * Keeps track of link configuration received from Netd. + * + * An instance of this class is constructed by passing in an interface name and a callback. The + * owner is then responsible for registering the tracker with NetworkObserverRegistry. When the + * class receives update notifications, it applies the update to its local LinkProperties, and if + * something has changed, notifies its owner of the update via the callback. + * + * The owner can then call {@code getLinkProperties()} in order to find out + * what changed. If in the meantime the LinkProperties stored here have changed, + * this class will return the current LinkProperties. Because each change + * triggers an update callback after the change is made, the owner may get more + * callbacks than strictly necessary (some of which may be no-ops), but will not + * be out of sync once all callbacks have been processed. + * + * Threading model: + * + * - The owner of this class is expected to create it, register it, and call + * getLinkProperties or clearLinkProperties on its thread. + * - Most of the methods in the class are implementing NetworkObserver and are called + * on the handler used to register the observer. + * - All accesses to mLinkProperties must be synchronized(this). All the other + * member variables are immutable once the object is constructed. + * + * @hide + */ +public class IpClientLinkObserver implements NetworkObserver { + private final String mTag; + + /** + * Callback used by {@link IpClientLinkObserver} to send update notifications. + */ + public interface Callback { + /** + * Called when some properties of the link were updated. + */ + void update(); + } + + private final String mInterfaceName; + private final Callback mCallback; + private final LinkProperties mLinkProperties; + private DnsServerRepository mDnsServerRepository; + + private static final boolean DBG = false; + + public IpClientLinkObserver(String iface, Callback callback) { + mTag = "NetlinkTracker/" + iface; + mInterfaceName = iface; + mCallback = callback; + mLinkProperties = new LinkProperties(); + mLinkProperties.setInterfaceName(mInterfaceName); + mDnsServerRepository = new DnsServerRepository(); + } + + private void maybeLog(String operation, String iface, LinkAddress address) { + if (DBG) { + Log.d(mTag, operation + ": " + address + " on " + iface + + " flags " + address.getFlags() + " scope " + address.getScope()); + } + } + + private void maybeLog(String operation, Object o) { + if (DBG) { + Log.d(mTag, operation + ": " + o.toString()); + } + } + + @Override + public void onInterfaceRemoved(String iface) { + maybeLog("interfaceRemoved", iface); + if (mInterfaceName.equals(iface)) { + // Our interface was removed. Clear our LinkProperties and tell our owner that they are + // now empty. Note that from the moment that the interface is removed, any further + // interface-specific messages (e.g., RTM_DELADDR) will not reach us, because the netd + // code that parses them will not be able to resolve the ifindex to an interface name. + clearLinkProperties(); + mCallback.update(); + } + } + + @Override + public void onInterfaceAddressUpdated(LinkAddress address, String iface) { + if (mInterfaceName.equals(iface)) { + maybeLog("addressUpdated", iface, address); + boolean changed; + synchronized (this) { + changed = mLinkProperties.addLinkAddress(address); + } + if (changed) { + mCallback.update(); + } + } + } + + @Override + public void onInterfaceAddressRemoved(LinkAddress address, String iface) { + if (mInterfaceName.equals(iface)) { + maybeLog("addressRemoved", iface, address); + boolean changed; + synchronized (this) { + changed = mLinkProperties.removeLinkAddress(address); + } + if (changed) { + mCallback.update(); + } + } + } + + @Override + public void onRouteUpdated(RouteInfo route) { + if (mInterfaceName.equals(route.getInterface())) { + maybeLog("routeUpdated", route); + boolean changed; + synchronized (this) { + changed = mLinkProperties.addRoute(route); + } + if (changed) { + mCallback.update(); + } + } + } + + @Override + public void onRouteRemoved(RouteInfo route) { + if (mInterfaceName.equals(route.getInterface())) { + maybeLog("routeRemoved", route); + boolean changed; + synchronized (this) { + changed = mLinkProperties.removeRoute(route); + } + if (changed) { + mCallback.update(); + } + } + } + + @Override + public void onInterfaceDnsServerInfo(String iface, long lifetime, String[] addresses) { + if (mInterfaceName.equals(iface)) { + maybeLog("interfaceDnsServerInfo", Arrays.toString(addresses)); + boolean changed = mDnsServerRepository.addServers(lifetime, addresses); + if (changed) { + synchronized (this) { + mDnsServerRepository.setDnsServersOn(mLinkProperties); + } + mCallback.update(); + } + } + } + + /** + * Returns a copy of this object's LinkProperties. + */ + public synchronized LinkProperties getLinkProperties() { + return new LinkProperties(mLinkProperties); + } + + /** + * Reset this object's LinkProperties. + */ + public synchronized void clearLinkProperties() { + // Clear the repository before clearing mLinkProperties. That way, if a clear() happens + // while interfaceDnsServerInfo() is being called, we'll end up with no DNS servers in + // mLinkProperties, as desired. + mDnsServerRepository = new DnsServerRepository(); + mLinkProperties.clear(); + mLinkProperties.setInterfaceName(mInterfaceName); + } + + /** + * Tracks DNS server updates received from Netlink. + * + * The network may announce an arbitrary number of DNS servers in Router Advertisements at any + * time. Each announcement has a lifetime; when the lifetime expires, the servers should not be + * used any more. In this way, the network can gracefully migrate clients from one set of DNS + * servers to another. Announcements can both raise and lower the lifetime, and an announcement + * can expire servers by announcing them with a lifetime of zero. + * + * Typically the system will only use a small number (2 or 3; {@code NUM_CURRENT_SERVERS}) of + * DNS servers at any given time. These are referred to as the current servers. In case all the + * current servers expire, the class also keeps track of a larger (but limited) number of + * servers that are promoted to current servers when the current ones expire. In order to + * minimize updates to the rest of the system (and potentially expensive cache flushes) this + * class attempts to keep the list of current servers constant where possible. More + * specifically, the list of current servers is only updated if a new server is learned and + * there are not yet {@code NUM_CURRENT_SERVERS} current servers, or if one or more of the + * current servers expires or is pushed out of the set. Therefore, the current servers will not + * necessarily be the ones with the highest lifetime, but the ones learned first. + * + * This is by design: if instead the class always preferred the servers with the highest + * lifetime, a (misconfigured?) network where two or more routers announce more than + * {@code NUM_CURRENT_SERVERS} unique servers would cause persistent oscillations. + * + * TODO: Currently servers are only expired when a new DNS update is received. + * Update them using timers, or possibly on every notification received by NetlinkTracker. + * + * Threading model: run by NetlinkTracker. Methods are synchronized(this) just in case netlink + * notifications are sent by multiple threads. If future threads use alarms to expire, those + * alarms must also be synchronized(this). + * + */ + private static class DnsServerRepository { + + /** How many DNS servers we will use. 3 is suggested by RFC 6106. */ + static final int NUM_CURRENT_SERVERS = 3; + + /** How many DNS servers we'll keep track of, in total. */ + static final int NUM_SERVERS = 12; + + /** Stores up to {@code NUM_CURRENT_SERVERS} DNS servers we're currently using. */ + private Set<InetAddress> mCurrentServers; + + public static final String TAG = "DnsServerRepository"; + + /** + * Stores all the DNS servers we know about, for use when the current servers expire. + * Always sorted in order of decreasing expiry. The elements in this list are also the + * values of mIndex, and may be elements in mCurrentServers. + */ + private ArrayList<DnsServerEntry> mAllServers; + + /** + * Indexes the servers so we can update their lifetimes more quickly in the common case + * where servers are not being added, but only being refreshed. + */ + private HashMap<InetAddress, DnsServerEntry> mIndex; + + DnsServerRepository() { + mCurrentServers = new HashSet<>(); + mAllServers = new ArrayList<>(NUM_SERVERS); + mIndex = new HashMap<>(NUM_SERVERS); + } + + /** Sets the DNS servers of the provided LinkProperties object to the current servers. */ + public synchronized void setDnsServersOn(LinkProperties lp) { + lp.setDnsServers(mCurrentServers); + } + + /** + * Notifies the class of new DNS server information. + * @param lifetime the time in seconds that the DNS servers are valid. + * @param addresses the string representations of the IP addresses of DNS servers to use. + */ + public synchronized boolean addServers(long lifetime, String[] addresses) { + // The lifetime is actually an unsigned 32-bit number, but Java doesn't have unsigned. + // Technically 0xffffffff (the maximum) is special and means "forever", but 2^32 seconds + // (136 years) is close enough. + long now = System.currentTimeMillis(); + long expiry = now + 1000 * lifetime; + + // Go through the list of servers. For each one, update the entry if one exists, and + // create one if it doesn't. + for (String addressString : addresses) { + InetAddress address; + try { + address = InetAddresses.parseNumericAddress(addressString); + } catch (IllegalArgumentException ex) { + continue; + } + + if (!updateExistingEntry(address, expiry)) { + // There was no entry for this server. Create one, unless it's already expired + // (i.e., if the lifetime is zero; it cannot be < 0 because it's unsigned). + if (expiry > now) { + DnsServerEntry entry = new DnsServerEntry(address, expiry); + mAllServers.add(entry); + mIndex.put(address, entry); + } + } + } + + // Sort the servers by expiry. + Collections.sort(mAllServers); + + // Prune excess entries and update the current server list. + return updateCurrentServers(); + } + + private synchronized boolean updateExistingEntry(InetAddress address, long expiry) { + DnsServerEntry existing = mIndex.get(address); + if (existing != null) { + existing.expiry = expiry; + return true; + } + return false; + } + + private synchronized boolean updateCurrentServers() { + long now = System.currentTimeMillis(); + boolean changed = false; + + // Prune excess or expired entries. + for (int i = mAllServers.size() - 1; i >= 0; i--) { + if (i >= NUM_SERVERS || mAllServers.get(i).expiry < now) { + DnsServerEntry removed = mAllServers.remove(i); + mIndex.remove(removed.address); + changed |= mCurrentServers.remove(removed.address); + } else { + break; + } + } + + // Add servers to the current set, in order of decreasing lifetime, until it has enough. + // Prefer existing servers over new servers in order to minimize updates to the rest of + // the system and avoid persistent oscillations. + for (DnsServerEntry entry : mAllServers) { + if (mCurrentServers.size() < NUM_CURRENT_SERVERS) { + changed |= mCurrentServers.add(entry.address); + } else { + break; + } + } + return changed; + } + } + + /** + * Represents a DNS server entry with an expiry time. + * + * Implements Comparable so DNS server entries can be sorted by lifetime, longest-lived first. + * The ordering of entries with the same lifetime is unspecified, because given two servers with + * identical lifetimes, we don't care which one we use, and only comparing the lifetime is much + * faster than comparing the IP address as well. + * + * Note: this class has a natural ordering that is inconsistent with equals. + */ + private static class DnsServerEntry implements Comparable<DnsServerEntry> { + /** The IP address of the DNS server. */ + public final InetAddress address; + /** The time until which the DNS server may be used. A Java millisecond time as might be + * returned by currentTimeMillis(). */ + public long expiry; + + DnsServerEntry(InetAddress address, long expiry) throws IllegalArgumentException { + this.address = address; + this.expiry = expiry; + } + + public int compareTo(DnsServerEntry other) { + return Long.compare(other.expiry, this.expiry); + } + } +} diff --git a/src/com/android/server/NetworkObserver.java b/src/com/android/server/NetworkObserver.java index d3b40a6..cccec0b 100644 --- a/src/com/android/server/NetworkObserver.java +++ b/src/com/android/server/NetworkObserver.java @@ -17,6 +17,7 @@ package com.android.server; import android.net.LinkAddress; +import android.net.RouteInfo; /** * Observer for network events, to use with {@link NetworkObserverRegistry}. @@ -77,11 +78,11 @@ public interface NetworkObserver { * @see android.net.INetdUnsolicitedEventListener * #onRouteChanged(boolean, String, String, String) */ - default void onRouteUpdated(String route, String gateway, String ifName) {} + default void onRouteUpdated(RouteInfo route) {} /** * @see android.net.INetdUnsolicitedEventListener * #onRouteChanged(boolean, String, String, String) */ - default void onRouteRemoved(String route, String gateway, String ifName) {} + default void onRouteRemoved(RouteInfo route) {} } diff --git a/src/com/android/server/NetworkObserverRegistry.java b/src/com/android/server/NetworkObserverRegistry.java index 14e6c5f..86e5e14 100644 --- a/src/com/android/server/NetworkObserverRegistry.java +++ b/src/com/android/server/NetworkObserverRegistry.java @@ -18,7 +18,10 @@ package com.android.server; import android.annotation.NonNull; import android.net.INetd; import android.net.INetdUnsolicitedEventListener; +import android.net.InetAddresses; +import android.net.IpPrefix; import android.net.LinkAddress; +import android.net.RouteInfo; import android.os.Handler; import android.os.RemoteException; @@ -138,10 +141,13 @@ public class NetworkObserverRegistry extends INetdUnsolicitedEventListener.Stub @Override public void onRouteChanged(boolean updated, String route, String gateway, String ifName) { + final RouteInfo processRoute = new RouteInfo(new IpPrefix(route), + ("".equals(gateway)) ? null : InetAddresses.parseNumericAddress(gateway), + ifName); if (updated) { - invokeForAllObservers(o -> o.onRouteUpdated(route, gateway, ifName)); + invokeForAllObservers(o -> o.onRouteUpdated(processRoute)); } else { - invokeForAllObservers(o -> o.onRouteRemoved(route, gateway, ifName)); + invokeForAllObservers(o -> o.onRouteRemoved(processRoute)); } } diff --git a/src/com/android/server/NetworkStackService.java b/src/com/android/server/NetworkStackService.java index 631ee45..7405c47 100644 --- a/src/com/android/server/NetworkStackService.java +++ b/src/com/android/server/NetworkStackService.java @@ -117,7 +117,11 @@ public class NetworkStackService extends Service { mObserverRegistry = new NetworkObserverRegistry(); mCm = context.getSystemService(ConnectivityManager.class); - // TODO: call mObserverRegistry here after adding sepolicy changes + try { + mObserverRegistry.register(mNetd); + } catch (RemoteException e) { + mLog.e("Error registering observer on Netd", e); + } } @NonNull @@ -158,7 +162,7 @@ public class NetworkStackService extends Service { @Override public void makeIpClient(String ifName, IIpClientCallbacks cb) throws RemoteException { - final IpClient ipClient = new IpClient(mContext, ifName, cb); + final IpClient ipClient = new IpClient(mContext, ifName, cb, mObserverRegistry); synchronized (mIpClients) { final Iterator<WeakReference<IpClient>> it = mIpClients.iterator(); diff --git a/tests/src/android/net/ip/IpClientTest.java b/tests/src/android/net/ip/IpClientTest.java index 4ae044d..40d510a 100644 --- a/tests/src/android/net/ip/IpClientTest.java +++ b/tests/src/android/net/ip/IpClientTest.java @@ -46,7 +46,6 @@ import android.net.RouteInfo; import android.net.shared.InitialConfiguration; import android.net.shared.ProvisioningConfiguration; import android.net.util.InterfaceParams; -import android.os.INetworkManagementService; import android.provider.Settings; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; @@ -54,7 +53,8 @@ import android.test.mock.MockContentResolver; import com.android.internal.R; import com.android.internal.util.test.FakeSettingsProvider; -import com.android.server.net.BaseNetworkObserver; +import com.android.server.NetworkObserver; +import com.android.server.NetworkObserverRegistry; import org.junit.Before; import org.junit.Test; @@ -87,15 +87,15 @@ public class IpClientTest { @Mock private Context mContext; @Mock private ConnectivityManager mCm; - @Mock private INetworkManagementService mNMService; + @Mock private NetworkObserverRegistry mObserverRegistry; @Mock private INetd mNetd; @Mock private Resources mResources; @Mock private IIpClientCallbacks mCb; @Mock private AlarmManager mAlarm; - @Mock private IpClient.Dependencies mDependecies; + @Mock private IpClient.Dependencies mDependencies; private MockContentResolver mContentResolver; - private BaseNetworkObserver mObserver; + private NetworkObserver mObserver; private InterfaceParams mIfParams; @Before @@ -104,6 +104,7 @@ public class IpClientTest { when(mContext.getSystemService(eq(Context.ALARM_SERVICE))).thenReturn(mAlarm); when(mContext.getSystemService(eq(ConnectivityManager.class))).thenReturn(mCm); + when(mContext.getSystemService(INetd.class)).thenReturn(mNetd); when(mContext.getResources()).thenReturn(mResources); when(mResources.getInteger(R.integer.config_networkAvoidBadWifi)) .thenReturn(DEFAULT_AVOIDBADWIFI_CONFIG_VALUE); @@ -113,28 +114,24 @@ public class IpClientTest { when(mContext.getContentResolver()).thenReturn(mContentResolver); mIfParams = null; - - when(mDependecies.getNMS()).thenReturn(mNMService); - when(mDependecies.getNetd()).thenReturn(mNetd); } private void setTestInterfaceParams(String ifname) { mIfParams = (ifname != null) ? new InterfaceParams(ifname, TEST_IFINDEX, TEST_MAC) : null; - when(mDependecies.getInterfaceParams(anyString())).thenReturn(mIfParams); + when(mDependencies.getInterfaceParams(anyString())).thenReturn(mIfParams); } private IpClient makeIpClient(String ifname) throws Exception { setTestInterfaceParams(ifname); - final IpClient ipc = new IpClient(mContext, ifname, mCb, mDependecies); + final IpClient ipc = new IpClient(mContext, ifname, mCb, mObserverRegistry, mDependencies); verify(mNetd, timeout(TEST_TIMEOUT_MS).times(1)).interfaceSetEnableIPv6(ifname, false); verify(mNetd, timeout(TEST_TIMEOUT_MS).times(1)).interfaceClearAddrs(ifname); - ArgumentCaptor<BaseNetworkObserver> arg = - ArgumentCaptor.forClass(BaseNetworkObserver.class); - verify(mNMService, times(1)).registerObserver(arg.capture()); + ArgumentCaptor<NetworkObserver> arg = ArgumentCaptor.forClass(NetworkObserver.class); + verify(mObserverRegistry, times(1)).registerObserver(arg.capture(), any()); mObserver = arg.getValue(); - reset(mNMService); + reset(mObserverRegistry); reset(mNetd); // Verify IpClient doesn't call onLinkPropertiesChange() when it starts. verify(mCb, never()).onLinkPropertiesChange(any()); @@ -152,7 +149,8 @@ public class IpClientTest { public void testNullInterfaceNameMostDefinitelyThrows() throws Exception { setTestInterfaceParams(null); try { - final IpClient ipc = new IpClient(mContext, null, mCb, mDependecies); + final IpClient ipc = new IpClient( + mContext, null, mCb, mObserverRegistry, mDependencies); ipc.shutdown(); fail(); } catch (NullPointerException npe) { @@ -165,7 +163,8 @@ public class IpClientTest { final String ifname = "lo"; setTestInterfaceParams(ifname); try { - final IpClient ipc = new IpClient(mContext, ifname, null, mDependecies); + final IpClient ipc = new IpClient( + mContext, ifname, null, mObserverRegistry, mDependencies); ipc.shutdown(); fail(); } catch (NullPointerException npe) { @@ -176,14 +175,16 @@ public class IpClientTest { @Test public void testInvalidInterfaceDoesNotThrow() throws Exception { setTestInterfaceParams(TEST_IFNAME); - final IpClient ipc = new IpClient(mContext, TEST_IFNAME, mCb, mDependecies); + final IpClient ipc = new IpClient( + mContext, TEST_IFNAME, mCb, mObserverRegistry, mDependencies); ipc.shutdown(); } @Test public void testInterfaceNotFoundFailsImmediately() throws Exception { setTestInterfaceParams(null); - final IpClient ipc = new IpClient(mContext, TEST_IFNAME, mCb, mDependecies); + final IpClient ipc = new IpClient( + mContext, TEST_IFNAME, mCb, mObserverRegistry, mDependencies); ipc.startProvisioning(new ProvisioningConfiguration()); verify(mCb, times(1)).onProvisioningFailure(any()); ipc.shutdown(); @@ -247,13 +248,13 @@ public class IpClientTest { // Add N - 1 addresses for (int i = 0; i < lastAddr; i++) { - mObserver.addressUpdated(iface, new LinkAddress(addresses[i])); + mObserver.onInterfaceAddressUpdated(new LinkAddress(addresses[i]), iface); verify(mCb, timeout(TEST_TIMEOUT_MS)).onLinkPropertiesChange(any()); reset(mCb); } // Add Nth address - mObserver.addressUpdated(iface, new LinkAddress(addresses[lastAddr])); + mObserver.onInterfaceAddressUpdated(new LinkAddress(addresses[lastAddr]), iface); LinkProperties want = linkproperties(links(addresses), routes(prefixes)); want.setInterfaceName(iface); verify(mCb, timeout(TEST_TIMEOUT_MS).times(1)).onProvisioningSuccess(argThat( |