Skip to content

Commit

Permalink
mobile: Fix onDefaultNetworkChanged implementation on Android (#35998)
Browse files Browse the repository at this point in the history 
This PR updates the implementation of `onDefaultNetworkChanged` for
Android to only call the `onDefaultNetworkChanged` callback when there
is a change in the network / transport type, such as when switching from
WiFi to cellular, etc and not every network capability change.

Risk Level: medium
Testing: unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: mobile (android)

---------

Signed-off-by: Fredy Wijaya <[email protected]>
  • Loading branch information
@fredyw
fredyw authored Sep 6, 2024
1 parent b89afa2 commit 1bef676
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 15 deletions.
2 changes: 1 addition & 1 deletion mobile/library/common/internal_engine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
void onDefaultNetworkAvailable();

/**
* This function does the following when the default network configuration was changed.
* This function does the following when the default network was changed.
*
* - Sets the preferred network.
* - Check for IPv6 connectivity. If there is no IPv6 no connectivity, it will call
Expand Down
58 changes: 48 additions & 10 deletions mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
* <li>When the internet is not available: call the
* <code>InternalEngine::onDefaultNetworkUnavailable</code> callback.</li>
*
* <li>When the capabilities are changed: call the
* <li>When the network is changed: call the
* <code>EnvoyEngine::onDefaultNetworkChanged</code>.</li>
* </ul>
*/
Expand Down Expand Up @@ -57,10 +57,20 @@ public static void shutdown() {
instance = null;
}

private static class DefaultNetworkCallback extends NetworkCallback {
@VisibleForTesting
static class DefaultNetworkCallback extends NetworkCallback {
private static final int[] TRANSPORT_TYPES = new int[] {
NetworkCapabilities.TRANSPORT_CELLULAR, NetworkCapabilities.TRANSPORT_WIFI,
NetworkCapabilities.TRANSPORT_BLUETOOTH, NetworkCapabilities.TRANSPORT_ETHERNET,
NetworkCapabilities.TRANSPORT_VPN, NetworkCapabilities.TRANSPORT_WIFI_AWARE,
NetworkCapabilities.TRANSPORT_LOWPAN,
};
private static final int EMPTY_TRANSPORT_TYPE = -1;

private final EnvoyEngine envoyEngine;
@VisibleForTesting int transportType = EMPTY_TRANSPORT_TYPE;

private DefaultNetworkCallback(EnvoyEngine envoyEngine) { this.envoyEngine = envoyEngine; }
DefaultNetworkCallback(EnvoyEngine envoyEngine) { this.envoyEngine = envoyEngine; }

@Override
public void onAvailable(@NonNull Network network) {
Expand All @@ -70,20 +80,48 @@ public void onAvailable(@NonNull Network network) {
@Override
public void onCapabilitiesChanged(@NonNull Network network,
@NonNull NetworkCapabilities networkCapabilities) {
if (networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) {
if (networkCapabilities.hasCapability(NetworkCapabilities.TRANSPORT_WIFI)) {
envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.WLAN);
} else if (networkCapabilities.hasCapability(NetworkCapabilities.TRANSPORT_CELLULAR)) {
envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.WWAN);
} else {
envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.GENERIC);
// `onCapabilities` is guaranteed to be called immediately after `onAvailable`
// starting with Android O, so this logic may not work on older Android versions.
// https://developer.android.com/reference/android/net/ConnectivityManager.NetworkCallback#onCapabilitiesChanged(android.net.Network,%20android.net.NetworkCapabilities)
if (transportType == EMPTY_TRANSPORT_TYPE) {
// The network was lost previously, see `onLost`.
onDefaultNetworkChanged(networkCapabilities);
} else {
// Only call the `onDefaultNetworkChanged` callback when there is a change in the
// transport type.
if (!networkCapabilities.hasTransport(transportType)) {
onDefaultNetworkChanged(networkCapabilities);
}
}
}

@Override
public void onLost(@NonNull Network network) {
envoyEngine.onDefaultNetworkUnavailable();
transportType = EMPTY_TRANSPORT_TYPE;
}

private static int getTransportType(NetworkCapabilities networkCapabilities) {
for (int type : TRANSPORT_TYPES) {
if (networkCapabilities.hasTransport(type)) {
return type;
}
}
return EMPTY_TRANSPORT_TYPE;
}

private void onDefaultNetworkChanged(NetworkCapabilities networkCapabilities) {
if (networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) {
if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) ||
networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI_AWARE)) {
envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.WLAN);
} else if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) {
envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.WWAN);
} else {
envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.GENERIC);
}
}
transportType = getTransportType(networkCapabilities);
}
}

Expand Down
2 changes: 1 addition & 1 deletion mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public interface EnvoyEngine {
void onDefaultNetworkAvailable();

/**
* A callback into the Envoy Engine when the default network configuration was changed.
* A callback into the Envoy Engine when the default network type was changed.
*/
void onDefaultNetworkChanged(EnvoyNetworkType network);

Expand Down
2 changes: 1 addition & 1 deletion mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ protected static native int registerStringAccessor(String accessorName,
protected static native void onDefaultNetworkAvailable(long engine);

/**
* A callback into the Envoy Engine when the default network configuration was changed.
* A callback into the Envoy Engine when the default network was changed.
*/
protected static native void onDefaultNetworkChanged(long engine, int networkType);

Expand Down
106 changes: 104 additions & 2 deletions mobile/test/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitorTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package io.envoyproxy.envoymobile.engine;

import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.robolectric.Shadows.shadowOf;

Expand Down Expand Up @@ -94,7 +96,7 @@ public void testOnDefaultNetworkChangedWlan() {
shadowOf(connectivityManager).getNetworkCallbacks().forEach(callback -> {
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addCapability(NetworkCapabilities.TRANSPORT_WIFI);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_WIFI);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);
});

Expand All @@ -106,7 +108,7 @@ public void testOnDefaultNetworkChangedWwan() {
shadowOf(connectivityManager).getNetworkCallbacks().forEach(callback -> {
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addCapability(NetworkCapabilities.TRANSPORT_CELLULAR);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);
});

Expand All @@ -118,9 +120,109 @@ public void testOnDefaultNetworkChangedGeneric() {
shadowOf(connectivityManager).getNetworkCallbacks().forEach(callback -> {
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);
});

verify(mockEnvoyEngine).onDefaultNetworkChanged(EnvoyNetworkType.GENERIC);
}

@Test
public void testOnCapabilitiesChangedPreviousNetworkIsEmptyCallbackIsCalledWlan() {
AndroidNetworkMonitor.DefaultNetworkCallback callback =
new AndroidNetworkMonitor.DefaultNetworkCallback(mockEnvoyEngine);
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_WIFI);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);

verify(mockEnvoyEngine).onDefaultNetworkChanged(EnvoyNetworkType.WLAN);
}

@Test
public void testOnCapabilitiesChangedPreviousNetworkIsEmptyCallbackIsCalledWwan() {
AndroidNetworkMonitor.DefaultNetworkCallback callback =
new AndroidNetworkMonitor.DefaultNetworkCallback(mockEnvoyEngine);
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);

verify(mockEnvoyEngine).onDefaultNetworkChanged(EnvoyNetworkType.WWAN);
}

@Test
public void testOnCapabilitiesChangedPreviousNetworkIsEmptyCallbackIsCalledGeneric() {
AndroidNetworkMonitor.DefaultNetworkCallback callback =
new AndroidNetworkMonitor.DefaultNetworkCallback(mockEnvoyEngine);
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);

verify(mockEnvoyEngine).onDefaultNetworkChanged(EnvoyNetworkType.GENERIC);
}

@Test
public void testOnCapabilitiesChangedPreviousNetworkNotEmptyCallbackIsCalledWwan() {
AndroidNetworkMonitor.DefaultNetworkCallback callback =
new AndroidNetworkMonitor.DefaultNetworkCallback(mockEnvoyEngine);
callback.transportType = NetworkCapabilities.TRANSPORT_WIFI;
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);

verify(mockEnvoyEngine).onDefaultNetworkChanged(EnvoyNetworkType.WWAN);
}

@Test
public void testOnCapabilitiesChangedPreviousNetworkNotEmptyCallCallbackIsCalledWlan() {
AndroidNetworkMonitor.DefaultNetworkCallback callback =
new AndroidNetworkMonitor.DefaultNetworkCallback(mockEnvoyEngine);
callback.transportType = NetworkCapabilities.TRANSPORT_CELLULAR;
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_WIFI);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);

verify(mockEnvoyEngine).onDefaultNetworkChanged(EnvoyNetworkType.WLAN);
}

@Test
public void testOnCapabilitiesChangedPreviousNetworkNotEmptyCallbackIsCalledGeneric() {
AndroidNetworkMonitor.DefaultNetworkCallback callback =
new AndroidNetworkMonitor.DefaultNetworkCallback(mockEnvoyEngine);
callback.transportType = NetworkCapabilities.TRANSPORT_BLUETOOTH;
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);

verify(mockEnvoyEngine).onDefaultNetworkChanged(EnvoyNetworkType.GENERIC);
}

@Test
public void testOnCapabilitiesChangedPreviousNetworkNotEmptyCallbackIsNotCalled() {
AndroidNetworkMonitor.DefaultNetworkCallback callback =
new AndroidNetworkMonitor.DefaultNetworkCallback(mockEnvoyEngine);
callback.transportType = NetworkCapabilities.TRANSPORT_WIFI;
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_WIFI);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);

verify(mockEnvoyEngine, never()).onDefaultNetworkChanged(any());
}

@Test
public void testOnCapabilitiesChangedNoInternetCallbackIsNotCalled() {
AndroidNetworkMonitor.DefaultNetworkCallback callback =
new AndroidNetworkMonitor.DefaultNetworkCallback(mockEnvoyEngine);
NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance();
shadowOf(capabilities).addTransportType(NetworkCapabilities.TRANSPORT_WIFI);
callback.onCapabilitiesChanged(ShadowNetwork.newInstance(0), capabilities);

verify(mockEnvoyEngine, never()).onDefaultNetworkChanged(any());
}
}

0 comments on commit 1bef676

Please sign in to comment.