From eea9edf76debdbdebda32793254ce0a4aac17730 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Wed, 16 Mar 2022 11:31:06 +0100 Subject: Change Ethernet API to use OutcomeReceiver This change addresses API review feedback. In addition, it changes the returned result from Network to the interface name. The current API returning a Network object is racy and cannot be implemented correctly. Users should instead use the ConnectivityManager#requestNetwork() API to get hold of the Network for a given interface. Bug: 220017952 Test: TH Change-Id: I7c46545a47034be409071c2ec007d9e1480c6ed0 --- packages/ConnectivityT/framework-t/Android.bp | 2 +- .../src/android/net/EthernetManager.java | 102 ++++++++++++--------- .../src/android/net/IEthernetManager.aidl | 9 +- .../net/IEthernetNetworkManagementListener.aidl | 25 ----- .../net/INetworkInterfaceOutcomeReceiver.aidl | 25 +++++ 5 files changed, 88 insertions(+), 75 deletions(-) delete mode 100644 packages/ConnectivityT/framework-t/src/android/net/IEthernetNetworkManagementListener.aidl create mode 100644 packages/ConnectivityT/framework-t/src/android/net/INetworkInterfaceOutcomeReceiver.aidl diff --git a/packages/ConnectivityT/framework-t/Android.bp b/packages/ConnectivityT/framework-t/Android.bp index 217a1f67c336..bc278528570f 100644 --- a/packages/ConnectivityT/framework-t/Android.bp +++ b/packages/ConnectivityT/framework-t/Android.bp @@ -131,8 +131,8 @@ filegroup { "src/android/net/EthernetNetworkUpdateRequest.java", "src/android/net/EthernetNetworkUpdateRequest.aidl", "src/android/net/IEthernetManager.aidl", - "src/android/net/IEthernetNetworkManagementListener.aidl", "src/android/net/IEthernetServiceListener.aidl", + "src/android/net/INetworkInterfaceOutcomeReceiver.aidl", "src/android/net/ITetheredInterfaceCallback.aidl", ], path: "src", diff --git a/packages/ConnectivityT/framework-t/src/android/net/EthernetManager.java b/packages/ConnectivityT/framework-t/src/android/net/EthernetManager.java index 793f28d5aa59..4f61dbf38b24 100644 --- a/packages/ConnectivityT/framework-t/src/android/net/EthernetManager.java +++ b/packages/ConnectivityT/framework-t/src/android/net/EthernetManager.java @@ -30,6 +30,7 @@ import android.compat.annotation.UnsupportedAppUsage; import android.content.Context; import android.content.pm.PackageManager; import android.os.Build; +import android.os.OutcomeReceiver; import android.os.RemoteException; import com.android.internal.annotations.GuardedBy; @@ -40,7 +41,6 @@ import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Objects; import java.util.concurrent.Executor; -import java.util.function.BiConsumer; /** * A class that manages and configures Ethernet interfaces. @@ -443,41 +443,45 @@ public class EthernetManager { return new TetheredInterfaceRequest(mService, cbInternal); } - private static final class InternalNetworkManagementListener - extends IEthernetNetworkManagementListener.Stub { + private static final class NetworkInterfaceOutcomeReceiver + extends INetworkInterfaceOutcomeReceiver.Stub { @NonNull private final Executor mExecutor; @NonNull - private final BiConsumer mListener; + private final OutcomeReceiver mCallback; - InternalNetworkManagementListener( + NetworkInterfaceOutcomeReceiver( @NonNull final Executor executor, - @NonNull final BiConsumer listener) { + @NonNull final OutcomeReceiver + callback) { Objects.requireNonNull(executor, "Pass a non-null executor"); - Objects.requireNonNull(listener, "Pass a non-null listener"); + Objects.requireNonNull(callback, "Pass a non-null callback"); mExecutor = executor; - mListener = listener; + mCallback = callback; } @Override - public void onComplete( - @Nullable final Network network, - @Nullable final EthernetNetworkManagementException e) { - mExecutor.execute(() -> mListener.accept(network, e)); + public void onResult(@NonNull String iface) { + mExecutor.execute(() -> mCallback.onResult(iface)); + } + + @Override + public void onError(@NonNull EthernetNetworkManagementException e) { + mExecutor.execute(() -> mCallback.onError(e)); } } - private InternalNetworkManagementListener getInternalNetworkManagementListener( + private NetworkInterfaceOutcomeReceiver makeNetworkInterfaceOutcomeReceiver( @Nullable final Executor executor, - @Nullable final BiConsumer listener) { - if (null != listener) { - Objects.requireNonNull(executor, "Pass a non-null executor, or a null listener"); + @Nullable final OutcomeReceiver callback) { + if (null != callback) { + Objects.requireNonNull(executor, "Pass a non-null executor, or a null callback"); } - final InternalNetworkManagementListener proxy; - if (null == listener) { + final NetworkInterfaceOutcomeReceiver proxy; + if (null == callback) { proxy = null; } else { - proxy = new InternalNetworkManagementListener(executor, listener); + proxy = new NetworkInterfaceOutcomeReceiver(executor, callback); } return proxy; } @@ -492,14 +496,17 @@ public class EthernetManager { * Similarly, use {@link NetworkCapabilities.Builder} to build a {@code NetworkCapabilities} * object for this network to put inside the {@code request}. * - * If non-null, the listener will be called exactly once after this is called, unless - * a synchronous exception was thrown. + * This function accepts an {@link OutcomeReceiver} that is called once the operation has + * finished execution. * * @param iface the name of the interface to act upon. * @param request the {@link EthernetNetworkUpdateRequest} used to set an ethernet network's * {@link StaticIpConfiguration} and {@link NetworkCapabilities} values. - * @param executor an {@link Executor} to execute the listener on. Optional if listener is null. - * @param listener an optional {@link BiConsumer} to listen for completion of the operation. + * @param executor an {@link Executor} to execute the callback on. Optional if callback is null. + * @param callback an optional {@link OutcomeReceiver} to listen for completion of the + * operation. On success, {@link OutcomeReceiver#onResult} is called with the + * interface name. On error, {@link OutcomeReceiver#onError} is called with more + * information about the error. * @throws SecurityException if the process doesn't hold * {@link android.Manifest.permission.MANAGE_ETHERNET_NETWORKS}. * @throws UnsupportedOperationException if called on a non-automotive device or on an @@ -515,11 +522,11 @@ public class EthernetManager { @NonNull String iface, @NonNull EthernetNetworkUpdateRequest request, @Nullable @CallbackExecutor Executor executor, - @Nullable BiConsumer listener) { + @Nullable OutcomeReceiver callback) { Objects.requireNonNull(iface, "iface must be non-null"); Objects.requireNonNull(request, "request must be non-null"); - final InternalNetworkManagementListener proxy = getInternalNetworkManagementListener( - executor, listener); + final NetworkInterfaceOutcomeReceiver proxy = makeNetworkInterfaceOutcomeReceiver( + executor, callback); try { mService.updateConfiguration(iface, request, proxy); } catch (RemoteException e) { @@ -530,15 +537,17 @@ public class EthernetManager { /** * Set an ethernet network's link state up. * - * When the link is successfully turned up, the listener will be called with the resulting - * network. If any error or unexpected condition happens while the system tries to turn the - * interface up, the listener will be called with an appropriate exception. - * The listener is guaranteed to be called exactly once for each call to this method, but this - * may take an unbounded amount of time depending on the actual network conditions. + * When the link is successfully turned up, the callback will be called with the network + * interface was torn down, if any. If any error or unexpected condition happens while the + * system tries to turn the interface down, the callback will be called with an appropriate + * exception. The callback is guaranteed to be called exactly once for each call to this method. * * @param iface the name of the interface to act upon. - * @param executor an {@link Executor} to execute the listener on. Optional if listener is null. - * @param listener an optional {@link BiConsumer} to listen for completion of the operation. + * @param executor an {@link Executor} to execute the callback on. Optional if callback is null. + * @param callback an optional {@link OutcomeReceiver} to listen for completion of the + * operation. On success, {@link OutcomeReceiver#onResult} is called with the + * interface name. On error, {@link OutcomeReceiver#onError} is called with more + * information about the error. * @throws SecurityException if the process doesn't hold * {@link android.Manifest.permission.MANAGE_ETHERNET_NETWORKS}. * @throws UnsupportedOperationException if called on a non-automotive device. @@ -553,10 +562,10 @@ public class EthernetManager { public void connectNetwork( @NonNull String iface, @Nullable @CallbackExecutor Executor executor, - @Nullable BiConsumer listener) { + @Nullable OutcomeReceiver callback) { Objects.requireNonNull(iface, "iface must be non-null"); - final InternalNetworkManagementListener proxy = getInternalNetworkManagementListener( - executor, listener); + final NetworkInterfaceOutcomeReceiver proxy = makeNetworkInterfaceOutcomeReceiver( + executor, callback); try { mService.connectNetwork(iface, proxy); } catch (RemoteException e) { @@ -567,14 +576,17 @@ public class EthernetManager { /** * Set an ethernet network's link state down. * - * When the link is successfully turned down, the listener will be called with the network that - * was torn down, if any. If any error or unexpected condition happens while the system tries to - * turn the interface down, the listener will be called with an appropriate exception. - * The listener is guaranteed to be called exactly once for each call to this method. + * When the link is successfully turned down, the callback will be called with the network + * interface was torn down, if any. If any error or unexpected condition happens while the + * system tries to turn the interface down, the callback will be called with an appropriate + * exception. The callback is guaranteed to be called exactly once for each call to this method. * * @param iface the name of the interface to act upon. - * @param executor an {@link Executor} to execute the listener on. Optional if listener is null. - * @param listener an optional {@link BiConsumer} to listen for completion of the operation. + * @param executor an {@link Executor} to execute the callback on. Optional if callback is null. + * @param callback an optional {@link OutcomeReceiver} to listen for completion of the + * operation. On success, {@link OutcomeReceiver#onResult} is called with the + * interface name. On error, {@link OutcomeReceiver#onError} is called with more + * information about the error. * @throws SecurityException if the process doesn't hold * {@link android.Manifest.permission.MANAGE_ETHERNET_NETWORKS}. * @throws UnsupportedOperationException if called on a non-automotive device. @@ -589,10 +601,10 @@ public class EthernetManager { public void disconnectNetwork( @NonNull String iface, @Nullable @CallbackExecutor Executor executor, - @Nullable BiConsumer listener) { + @Nullable OutcomeReceiver callback) { Objects.requireNonNull(iface, "iface must be non-null"); - final InternalNetworkManagementListener proxy = getInternalNetworkManagementListener( - executor, listener); + final NetworkInterfaceOutcomeReceiver proxy = makeNetworkInterfaceOutcomeReceiver( + executor, callback); try { mService.disconnectNetwork(iface, proxy); } catch (RemoteException e) { diff --git a/packages/ConnectivityT/framework-t/src/android/net/IEthernetManager.aidl b/packages/ConnectivityT/framework-t/src/android/net/IEthernetManager.aidl index 544d02ba76ff..95ae907a3d21 100644 --- a/packages/ConnectivityT/framework-t/src/android/net/IEthernetManager.aidl +++ b/packages/ConnectivityT/framework-t/src/android/net/IEthernetManager.aidl @@ -18,8 +18,9 @@ package android.net; import android.net.IpConfiguration; import android.net.IEthernetServiceListener; -import android.net.IEthernetNetworkManagementListener; +import android.net.EthernetNetworkManagementException; import android.net.EthernetNetworkUpdateRequest; +import android.net.INetworkInterfaceOutcomeReceiver; import android.net.ITetheredInterfaceCallback; /** @@ -39,7 +40,7 @@ interface IEthernetManager void requestTetheredInterface(in ITetheredInterfaceCallback callback); void releaseTetheredInterface(in ITetheredInterfaceCallback callback); void updateConfiguration(String iface, in EthernetNetworkUpdateRequest request, - in IEthernetNetworkManagementListener listener); - void connectNetwork(String iface, in IEthernetNetworkManagementListener listener); - void disconnectNetwork(String iface, in IEthernetNetworkManagementListener listener); + in INetworkInterfaceOutcomeReceiver listener); + void connectNetwork(String iface, in INetworkInterfaceOutcomeReceiver listener); + void disconnectNetwork(String iface, in INetworkInterfaceOutcomeReceiver listener); } diff --git a/packages/ConnectivityT/framework-t/src/android/net/IEthernetNetworkManagementListener.aidl b/packages/ConnectivityT/framework-t/src/android/net/IEthernetNetworkManagementListener.aidl deleted file mode 100644 index 93edccfdafd9..000000000000 --- a/packages/ConnectivityT/framework-t/src/android/net/IEthernetNetworkManagementListener.aidl +++ /dev/null @@ -1,25 +0,0 @@ -/** - * Copyright (c) 2021, 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; - -import android.net.EthernetNetworkManagementException; -import android.net.Network; - -/** @hide */ -oneway interface IEthernetNetworkManagementListener { - void onComplete(in Network network, in EthernetNetworkManagementException exception); -} \ No newline at end of file diff --git a/packages/ConnectivityT/framework-t/src/android/net/INetworkInterfaceOutcomeReceiver.aidl b/packages/ConnectivityT/framework-t/src/android/net/INetworkInterfaceOutcomeReceiver.aidl new file mode 100644 index 000000000000..85795ead7aea --- /dev/null +++ b/packages/ConnectivityT/framework-t/src/android/net/INetworkInterfaceOutcomeReceiver.aidl @@ -0,0 +1,25 @@ +/** + * Copyright (c) 2021, 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; + +import android.net.EthernetNetworkManagementException; + +/** @hide */ +oneway interface INetworkInterfaceOutcomeReceiver { + void onResult(in String iface); + void onError(in EthernetNetworkManagementException e); +} \ No newline at end of file -- cgit v1.2.3-59-g8ed1b