From 039d8111e0cdbb8e2910b7c1fb26da9f1f5aff6b Mon Sep 17 00:00:00 2001 From: Paul Hu Date: Tue, 4 Oct 2022 14:20:04 +0800 Subject: [PATCH] Send MdnsServiceInfo on onServiceRemoved callback Send MdnsServiceInfo on onServiceRemoved callback which include service name, service type and other info instead of service name only. These extra info is necessary for service lost callback on NsdService on the following changes. Bug: 254166302 Test: atest FrameworksNetTests Change-Id: Ibacbef4b893209bbc2a37376bf26d713ce41429c --- .../mdns/MdnsResponseDecoder.java | 12 ++-- .../mdns/MdnsServiceBrowserListener.java | 4 +- .../mdns/MdnsServiceTypeClient.java | 35 +++++++---- .../mdns/MdnsServiceTypeClientTests.java | 62 +++++++++++++++---- 4 files changed, 80 insertions(+), 33 deletions(-) diff --git a/service/mdns/com/android/server/connectivity/mdns/MdnsResponseDecoder.java b/service/mdns/com/android/server/connectivity/mdns/MdnsResponseDecoder.java index 861acbb8b0..6c2bc19545 100644 --- a/service/mdns/com/android/server/connectivity/mdns/MdnsResponseDecoder.java +++ b/service/mdns/com/android/server/connectivity/mdns/MdnsResponseDecoder.java @@ -256,6 +256,9 @@ public class MdnsResponseDecoder { response = new MdnsResponse(now); responses.add(response); } + // Set interface index earlier because some responses have PTR record only. + // Need to know every response is getting from which interface. + response.setInterfaceIndex(interfaceIndex); response.addPointerRecord((MdnsPointerRecord) record); } } @@ -286,13 +289,13 @@ public class MdnsResponseDecoder { List matchingResponses = findResponsesWithHostName(responses, inetRecord.getName()); for (MdnsResponse response : matchingResponses) { - assignInetRecord(response, inetRecord, interfaceIndex); + assignInetRecord(response, inetRecord); } } else { MdnsResponse response = findResponseWithHostName(responses, inetRecord.getName()); if (response != null) { - assignInetRecord(response, inetRecord, interfaceIndex); + assignInetRecord(response, inetRecord); } } } @@ -301,14 +304,11 @@ public class MdnsResponseDecoder { return SUCCESS; } - private static void assignInetRecord( - MdnsResponse response, MdnsInetAddressRecord inetRecord, int interfaceIndex) { + private static void assignInetRecord(MdnsResponse response, MdnsInetAddressRecord inetRecord) { if (inetRecord.getInet4Address() != null) { response.setInet4AddressRecord(inetRecord); - response.setInterfaceIndex(interfaceIndex); } else if (inetRecord.getInet6Address() != null) { response.setInet6AddressRecord(inetRecord); - response.setInterfaceIndex(interfaceIndex); } } diff --git a/service/mdns/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java b/service/mdns/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java index 53e58d1247..7a8fcc0cc7 100644 --- a/service/mdns/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java +++ b/service/mdns/com/android/server/connectivity/mdns/MdnsServiceBrowserListener.java @@ -44,9 +44,9 @@ public interface MdnsServiceBrowserListener { /** * Called when an mDNS service instance is no longer valid and removed. * - * @param serviceInstanceName The service instance name of the removed mDNS service. + * @param serviceInfo The service instance of the removed mDNS service. */ - void onServiceRemoved(@NonNull String serviceInstanceName); + void onServiceRemoved(@NonNull MdnsServiceInfo serviceInfo); /** * Called when searching for mDNS service has stopped because of an error. diff --git a/service/mdns/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java b/service/mdns/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java index 8ca71b9a07..0fd60250ed 100644 --- a/service/mdns/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java +++ b/service/mdns/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java @@ -91,8 +91,12 @@ public class MdnsServiceTypeClient { private static MdnsServiceInfo buildMdnsServiceInfoFromResponse( @NonNull MdnsResponse response, @NonNull String[] serviceTypeLabels) { - String[] hostName = response.getServiceRecord().getServiceHost(); - int port = response.getServiceRecord().getServicePort(); + String[] hostName = null; + int port = 0; + if (response.hasServiceRecord()) { + hostName = response.getServiceRecord().getServiceHost(); + port = response.getServiceRecord().getServicePort(); + } String ipv4Address = null; String ipv6Address = null; @@ -104,15 +108,17 @@ public class MdnsServiceTypeClient { Inet6Address inet6Address = response.getInet6AddressRecord().getInet6Address(); ipv6Address = (inet6Address == null) ? null : inet6Address.getHostAddress(); } - if (ipv4Address == null && ipv6Address == null) { - throw new IllegalArgumentException( - "Either ipv4Address or ipv6Address must be non-null"); - } String serviceInstanceName = response.getServiceInstanceName(); if (serviceInstanceName == null) { throw new IllegalStateException( "mDNS response must have non-null service instance name"); } + List textStrings = null; + List textEntries = null; + if (response.hasTextRecord()) { + textStrings = response.getTextRecord().getStrings(); + textEntries = response.getTextRecord().getEntries(); + } // TODO: Throw an error message if response doesn't have Inet6 or Inet4 address. return new MdnsServiceInfo( serviceInstanceName, @@ -122,8 +128,8 @@ public class MdnsServiceTypeClient { port, ipv4Address, ipv6Address, - response.getTextRecord().getStrings(), - response.getTextRecord().getEntries(), + textStrings, + textEntries, response.getInterfaceIndex()); } @@ -246,12 +252,14 @@ public class MdnsServiceTypeClient { } private void onGoodbyeReceived(@Nullable String serviceInstanceName) { - if (serviceInstanceName == null) { + final MdnsResponse response = instanceNameToResponse.remove(serviceInstanceName); + if (response == null) { return; } - instanceNameToResponse.remove(serviceInstanceName); for (MdnsServiceBrowserListener listener : listeners) { - listener.onServiceRemoved(serviceInstanceName); + final MdnsServiceInfo serviceInfo = + buildMdnsServiceInfoFromResponse(response, serviceTypeLabels); + listener.onServiceRemoved(serviceInfo); } } @@ -425,7 +433,10 @@ public class MdnsServiceTypeClient { String serviceInstanceName = existingResponse.getServiceInstanceName(); if (serviceInstanceName != null) { - listener.onServiceRemoved(serviceInstanceName); + final MdnsServiceInfo serviceInfo = + buildMdnsServiceInfoFromResponse( + existingResponse, serviceTypeLabels); + listener.onServiceRemoved(serviceInfo); } } } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java index 6b10c7176c..6f8b85a423 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -35,6 +36,7 @@ import static org.mockito.Mockito.when; import static java.nio.charset.StandardCharsets.UTF_8; import android.annotation.NonNull; +import android.text.TextUtils; import com.android.server.connectivity.mdns.MdnsServiceInfo.TextEntry; import com.android.server.connectivity.mdns.MdnsServiceTypeClient.QueryTaskConfig; @@ -57,6 +59,7 @@ import java.net.Inet4Address; import java.net.Inet6Address; import java.net.SocketAddress; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -71,6 +74,7 @@ import java.util.concurrent.TimeUnit; public class MdnsServiceTypeClientTests { private static final String SERVICE_TYPE = "_googlecast._tcp.local"; + private static final String[] SERVICE_TYPE_LABELS = TextUtils.split(SERVICE_TYPE, "\\."); @Mock private MdnsServiceBrowserListener mockListenerOne; @@ -491,17 +495,43 @@ public class MdnsServiceTypeClientTests { } @Test - public void processResponse_goodBye() { + public void processResponse_goodBye() throws Exception { client.startSendAndReceive(mockListenerOne, MdnsSearchOptions.getDefaultOptions()); client.startSendAndReceive(mockListenerTwo, MdnsSearchOptions.getDefaultOptions()); + final String serviceName = "service-instance-1"; + final String ipV6Address = "2000:3333::da6c:63ff:fe7c:7483"; + final int interfaceIndex = 999; + // Process the initial response. + final MdnsResponse initialResponse = + createResponse( + serviceName, + ipV6Address, + 5353 /* port */, + Collections.singletonList("ABCDE"), + Collections.emptyMap(), + interfaceIndex); + client.processResponse(initialResponse); MdnsResponse response = mock(MdnsResponse.class); - when(response.getServiceInstanceName()).thenReturn("goodbye-service-instance-name"); - when(response.isGoodbye()).thenReturn(true); + doReturn("goodbye-service").when(response).getServiceInstanceName(); + doReturn(interfaceIndex).when(response).getInterfaceIndex(); + doReturn(true).when(response).isGoodbye(); client.processResponse(response); + // Verify onServiceRemoved won't be called if the service is not existed. + verify(mockListenerOne, never()).onServiceRemoved(any()); + verify(mockListenerTwo, never()).onServiceRemoved(any()); - verify(mockListenerOne).onServiceRemoved("goodbye-service-instance-name"); - verify(mockListenerTwo).onServiceRemoved("goodbye-service-instance-name"); + // Verify onServiceRemoved would be called. + doReturn(serviceName).when(response).getServiceInstanceName(); + client.processResponse(response); + verify(mockListenerOne).onServiceRemoved(argThat( + info -> serviceName.equals(info.getServiceInstanceName()) + && Arrays.equals(SERVICE_TYPE_LABELS, info.getServiceType()) + && info.getInterfaceIndex() == interfaceIndex)); + verify(mockListenerTwo).onServiceRemoved(argThat( + info -> serviceName.equals(info.getServiceInstanceName()) + && Arrays.equals(SERVICE_TYPE_LABELS, info.getServiceType()) + && info.getInterfaceIndex() == interfaceIndex)); } @Test @@ -563,7 +593,7 @@ public class MdnsServiceTypeClientTests { firstMdnsTask.run(); // Verify onServiceRemoved was not called. - verify(mockListenerOne, never()).onServiceRemoved(serviceInstanceName); + verify(mockListenerOne, never()).onServiceRemoved(any()); } @Test @@ -586,7 +616,7 @@ public class MdnsServiceTypeClientTests { MdnsResponse initialResponse = createResponse( serviceInstanceName, "192.168.1.1", 5353, List.of("ABCDE"), - Map.of()); + Map.of(), 999 /* interfaceIndex */); client.processResponse(initialResponse); // Clear the scheduled runnable. @@ -597,14 +627,17 @@ public class MdnsServiceTypeClientTests { firstMdnsTask.run(); // Verify onServiceRemoved was not called. - verify(mockListenerOne, never()).onServiceRemoved(serviceInstanceName); + verify(mockListenerOne, never()).onServiceRemoved(any()); // Simulate the case where the response is after TTL. when(initialResponse.getServiceRecord().getRemainingTTL(anyLong())).thenReturn((long) 0); firstMdnsTask.run(); // Verify onServiceRemoved was called. - verify(mockListenerOne, times(1)).onServiceRemoved(serviceInstanceName); + verify(mockListenerOne, times(1)).onServiceRemoved(argThat( + info -> serviceInstanceName.equals(info.getServiceInstanceName()) + && Arrays.equals(SERVICE_TYPE_LABELS, info.getServiceType()) + && info.getInterfaceIndex() == 999)); } @Test @@ -636,7 +669,7 @@ public class MdnsServiceTypeClientTests { firstMdnsTask.run(); // Verify onServiceRemoved was not called. - verify(mockListenerOne, never()).onServiceRemoved(serviceInstanceName); + verify(mockListenerOne, never()).onServiceRemoved(any()); } @Test @@ -659,7 +692,7 @@ public class MdnsServiceTypeClientTests { MdnsResponse initialResponse = createResponse( serviceInstanceName, "192.168.1.1", 5353, List.of("ABCDE"), - Map.of()); + Map.of(), 999 /* interfaceIndex */); client.processResponse(initialResponse); // Clear the scheduled runnable. @@ -669,8 +702,11 @@ public class MdnsServiceTypeClientTests { when(initialResponse.getServiceRecord().getRemainingTTL(anyLong())).thenReturn((long) 0); firstMdnsTask.run(); - // Verify onServiceRemoved was not called. - verify(mockListenerOne, times(1)).onServiceRemoved(serviceInstanceName); + // Verify onServiceRemoved was called. + verify(mockListenerOne, times(1)).onServiceRemoved(argThat( + info -> serviceInstanceName.equals(info.getServiceInstanceName()) + && Arrays.equals(SERVICE_TYPE_LABELS, info.getServiceType()) + && info.getInterfaceIndex() == 999)); } // verifies that the right query was enqueued with the right delay, and send query by executing