follow up comments in custom TTL CLS (aosp/2870274 and aosp/2606573)
Summary of major changes in this CL:
1. update javadoc in AdvertisingRequest to start with one line
2. update ttl API document to ve clearer that values smaller than 1
second is discarded
3. Remove redundant constructors of MdnsServiceInfo
4. add tests for MdnsRecordRepository to verify that addService works
with non-null TTL
Other comments needs more consideration and relatively larger change, so
tracked in separated child issue of b/327619631
Bug: 327619631
Change-Id: I040b8451337d88d96da7120455c228cef204f768
diff --git a/framework-t/src/android/net/nsd/AdvertisingRequest.java b/framework-t/src/android/net/nsd/AdvertisingRequest.java
index 2895b0c..6afb2d5 100644
--- a/framework-t/src/android/net/nsd/AdvertisingRequest.java
+++ b/framework-t/src/android/net/nsd/AdvertisingRequest.java
@@ -110,8 +110,9 @@
}
/**
- * Returns the time interval that the resource records may be cached on a DNS resolver or
- * {@code null} if not specified.
+ * Returns the time interval that the resource records may be cached on a DNS resolver.
+ *
+ * The value will be {@code null} if it's not specified with the {@link #Builder}.
*
* @hide
*/
@@ -161,7 +162,7 @@
dest.writeParcelable(mServiceInfo, flags);
dest.writeInt(mProtocolType);
dest.writeLong(mAdvertisingConfig);
- dest.writeLong(mTtl == null ? -1 : mTtl.getSeconds());
+ dest.writeLong(mTtl == null ? -1L : mTtl.getSeconds());
}
// @FlaggedApi(NsdManager.Flags.ADVERTISE_REQUEST_API)
@@ -205,7 +206,9 @@
* When registering a service, {@link NsdManager#FAILURE_BAD_PARAMETERS} will be returned
* if {@code ttl} is smaller than 30 seconds.
*
- * Note: only number of seconds of {@code ttl} is used.
+ * Note: the value after the decimal point (in unit of seconds) will be discarded. For
+ * example, {@code 30} seconds will be used when {@code Duration.ofSeconds(30L, 50_000L)}
+ * is provided.
*
* @param ttl the maximum duration that the DNS resource records will be cached
*
diff --git a/framework-t/src/android/net/nsd/NsdServiceInfo.java b/framework-t/src/android/net/nsd/NsdServiceInfo.java
index f4cc2ac..dba08a1 100644
--- a/framework-t/src/android/net/nsd/NsdServiceInfo.java
+++ b/framework-t/src/android/net/nsd/NsdServiceInfo.java
@@ -70,7 +70,8 @@
private int mInterfaceIndex;
- // The timestamp that all resource records associated with this service are considered invalid.
+ // The timestamp that one or more resource records associated with this service are considered
+ // invalid.
@Nullable
private Instant mExpirationTime;
@@ -497,7 +498,9 @@
/**
* Sets the timestamp after when this service is expired.
*
- * Note: only number of seconds of {@code expirationTime} is used.
+ * Note: the value after the decimal point (in unit of seconds) will be discarded. For
+ * example, {@code 30} seconds will be used when {@code Duration.ofSeconds(30L, 50_000L)}
+ * is provided.
*
* @hide
*/
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceInfo.java b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceInfo.java
index f60a95e..1ec9e39 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceInfo.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceInfo.java
@@ -94,57 +94,6 @@
@NonNull
private final Instant expirationTime;
- /** Constructs a {@link MdnsServiceInfo} object with default values. */
- public MdnsServiceInfo(
- String serviceInstanceName,
- String[] serviceType,
- @Nullable List<String> subtypes,
- String[] hostName,
- int port,
- @Nullable String ipv4Address,
- @Nullable String ipv6Address,
- @Nullable List<String> textStrings) {
- this(
- serviceInstanceName,
- serviceType,
- subtypes,
- hostName,
- port,
- List.of(ipv4Address),
- List.of(ipv6Address),
- textStrings,
- /* textEntries= */ null,
- /* interfaceIndex= */ INTERFACE_INDEX_UNSPECIFIED,
- /* network= */ null,
- /* expirationTime= */ Instant.MAX);
- }
-
- /** Constructs a {@link MdnsServiceInfo} object with default values. */
- public MdnsServiceInfo(
- String serviceInstanceName,
- String[] serviceType,
- List<String> subtypes,
- String[] hostName,
- int port,
- @Nullable String ipv4Address,
- @Nullable String ipv6Address,
- @Nullable List<String> textStrings,
- @Nullable List<TextEntry> textEntries) {
- this(
- serviceInstanceName,
- serviceType,
- subtypes,
- hostName,
- port,
- List.of(ipv4Address),
- List.of(ipv6Address),
- textStrings,
- textEntries,
- /* interfaceIndex= */ INTERFACE_INDEX_UNSPECIFIED,
- /* network= */ null,
- /* expirationTime= */ Instant.MAX);
- }
-
/**
* Constructs a {@link MdnsServiceInfo} object with default values.
*
diff --git a/tests/unit/java/android/net/nsd/AdvertisingRequestTest.kt b/tests/unit/java/android/net/nsd/AdvertisingRequestTest.kt
index 332f2a3..c491f37 100644
--- a/tests/unit/java/android/net/nsd/AdvertisingRequestTest.kt
+++ b/tests/unit/java/android/net/nsd/AdvertisingRequestTest.kt
@@ -54,17 +54,17 @@
assertEquals(beforeParcel.advertisingConfig, afterParcel.advertisingConfig)
}
-@Test
-fun testBuilder_setNullTtl_success() {
- val info = NsdServiceInfo().apply {
- serviceType = "_ipp._tcp"
- }
- val request = AdvertisingRequest.Builder(info, PROTOCOL_DNS_SD)
- .setTtl(null)
- .build()
+ @Test
+ fun testBuilder_setNullTtl_success() {
+ val info = NsdServiceInfo().apply {
+ serviceType = "_ipp._tcp"
+ }
+ val request = AdvertisingRequest.Builder(info, PROTOCOL_DNS_SD)
+ .setTtl(null)
+ .build()
- assertNull(request.ttl)
-}
+ assertNull(request.ttl)
+ }
@Test
fun testBuilder_setPropertiesSuccess() {
diff --git a/tests/unit/java/android/net/nsd/NsdManagerTest.java b/tests/unit/java/android/net/nsd/NsdManagerTest.java
index 76a649e..27c4561 100644
--- a/tests/unit/java/android/net/nsd/NsdManagerTest.java
+++ b/tests/unit/java/android/net/nsd/NsdManagerTest.java
@@ -240,7 +240,7 @@
AdvertisingRequest capturedRequest = getAdvertisingRequest(
req -> verify(mServiceConn).registerService(anyInt(), req.capture()));
- assertEquals(request, capturedRequest);
+ assertEquals(request.getTtl(), capturedRequest.getTtl());
}
private void doTestRegisterService() throws Exception {
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
index 8d1dff6..c69b1e1 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
@@ -38,6 +38,7 @@
import java.net.InetSocketAddress
import java.net.NetworkInterface
import java.util.Collections
+import java.time.Duration
import kotlin.test.assertContentEquals
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
@@ -146,7 +147,7 @@
val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
assertEquals(0, repository.servicesCount)
assertEquals(-1,
- repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, null /* ttl */))
+ repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, Duration.ofSeconds(50)))
assertEquals(1, repository.servicesCount)
val probingInfo = repository.setServiceProbing(TEST_SERVICE_ID_1)
@@ -169,7 +170,7 @@
assertEquals(MdnsServiceRecord(expectedName,
0L /* receiptTimeMillis */,
false /* cacheFlush */,
- SHORT_TTL /* ttlMillis */,
+ 50_000L /* ttlMillis */,
0 /* servicePriority */, 0 /* serviceWeight */,
TEST_PORT, TEST_HOSTNAME), packet.authorityRecords[0])
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceInfoTest.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceInfoTest.java
index 8740e80..4ce8ba6 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceInfoTest.java
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceInfoTest.java
@@ -54,7 +54,8 @@
"192.168.1.1",
"2001::1",
List.of("vn=Google Inc.", "mn=Google Nest Hub Max"),
- /* textEntries= */ null);
+ /* textEntries= */ null,
+ INTERFACE_INDEX_UNSPECIFIED);
assertTrue(info.getAttributeByKey("vn").equals("Google Inc."));
assertTrue(info.getAttributeByKey("mn").equals("Google Nest Hub Max"));
@@ -73,7 +74,8 @@
"2001::1",
/* textStrings= */ null,
List.of(MdnsServiceInfo.TextEntry.fromString("vn=Google Inc."),
- MdnsServiceInfo.TextEntry.fromString("mn=Google Nest Hub Max")));
+ MdnsServiceInfo.TextEntry.fromString("mn=Google Nest Hub Max")),
+ INTERFACE_INDEX_UNSPECIFIED);
assertTrue(info.getAttributeByKey("vn").equals("Google Inc."));
assertTrue(info.getAttributeByKey("mn").equals("Google Nest Hub Max"));
@@ -93,7 +95,8 @@
List.of("vn=Alphabet Inc.", "mn=Google Nest Hub Max", "id=12345"),
List.of(
MdnsServiceInfo.TextEntry.fromString("vn=Google Inc."),
- MdnsServiceInfo.TextEntry.fromString("mn=Google Nest Hub Max")));
+ MdnsServiceInfo.TextEntry.fromString("mn=Google Nest Hub Max")),
+ INTERFACE_INDEX_UNSPECIFIED);
assertEquals(Map.of("vn", "Google Inc.", "mn", "Google Nest Hub Max"),
info.getAttributes());
@@ -113,7 +116,8 @@
List.of("vn=Alphabet Inc.", "mn=Google Nest Hub Max", "id=12345"),
List.of(MdnsServiceInfo.TextEntry.fromString("vn=Google Inc."),
MdnsServiceInfo.TextEntry.fromString("mn=Google Nest Hub Max"),
- MdnsServiceInfo.TextEntry.fromString("mn=Google WiFi Router")));
+ MdnsServiceInfo.TextEntry.fromString("mn=Google WiFi Router")),
+ INTERFACE_INDEX_UNSPECIFIED);
assertEquals(Map.of("vn", "Google Inc.", "mn", "Google Nest Hub Max"),
info.getAttributes());
@@ -131,7 +135,8 @@
"192.168.1.1",
"2001::1",
List.of("KEY=Value"),
- /* textEntries= */ null);
+ /* textEntries= */ null,
+ INTERFACE_INDEX_UNSPECIFIED);
assertEquals("Value", info.getAttributeByKey("key"));
assertEquals("Value", info.getAttributeByKey("KEY"));
@@ -150,7 +155,9 @@
12345,
"192.168.1.1",
"2001::1",
- List.of());
+ List.of(),
+ /* textEntries= */ null,
+ INTERFACE_INDEX_UNSPECIFIED);
assertEquals(info.getInterfaceIndex(), INTERFACE_INDEX_UNSPECIFIED);
}