Removing deprecated ServiceDiscovererEvent methods (#1962)

Motivation: Recently, `ServiceDiscovererEvent`'s (and consequently `DnsServiceDiscovererObserver`'s) API has changed and the deprecations can be removed in `main` branch. Modifications: * Removed deprecated methods and reverted default implementations for new methods, * Moved ServiceDiscovererUtils to dns-discovery-netty module. Result: Cleaner API with less deprecations.
apple · Nov 22, 2021 · 8b578f7 · 8b578f7
1 parent 6901e0a
commit 8b578f7
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 67 deletions.
diff --git a/...h/java/io/servicetalk/benchmark/loadbalancer/RoundRobinLoadBalancerSDEventsBenchmark.java b/...h/java/io/servicetalk/benchmark/loadbalancer/RoundRobinLoadBalancerSDEventsBenchmark.java
@@ -40,6 +40,8 @@
 import java.util.List;
 import javax.annotation.Nullable;
 
+import static io.servicetalk.client.api.ServiceDiscovererEvent.Status.AVAILABLE;
+import static io.servicetalk.client.api.ServiceDiscovererEvent.Status.UNAVAILABLE;
 import static io.servicetalk.concurrent.api.Completable.completed;
 import static io.servicetalk.concurrent.api.Publisher.fromIterable;
 import static io.servicetalk.concurrent.api.Single.succeeded;
@@ -63,11 +65,14 @@ public void setup() {
         mixedEvents = new ArrayList<>(ops);
         for (int i = 1; i <= ops; ++i) {
             if (i % removalStride == 0) {
-                mixedEvents.add(new DefaultServiceDiscovererEvent<>(createUnresolved("127.0.0." + (i - 1), 0), false));
+                mixedEvents.add(new DefaultServiceDiscovererEvent<>(
+                        createUnresolved("127.0.0." + (i - 1), 0), UNAVAILABLE));
             } else {
-                mixedEvents.add(new DefaultServiceDiscovererEvent<>(createUnresolved("127.0.0." + i, 0), true));
+                mixedEvents.add(new DefaultServiceDiscovererEvent<>(
+                        createUnresolved("127.0.0." + i, 0), AVAILABLE));
             }
-            availableEvents.add(new DefaultServiceDiscovererEvent<>(createUnresolved("127.0.0." + i, 0), true));
+            availableEvents.add(new DefaultServiceDiscovererEvent<>(
+                    createUnresolved("127.0.0." + i, 0), AVAILABLE));
         }
     }
 

diff --git a/...alk-client-api/src/main/java/io/servicetalk/client/api/DefaultServiceDiscovererEvent.java b/...alk-client-api/src/main/java/io/servicetalk/client/api/DefaultServiceDiscovererEvent.java
@@ -15,8 +15,6 @@
  */
 package io.servicetalk.client.api;
 
-import static io.servicetalk.client.api.ServiceDiscovererEvent.Status.AVAILABLE;
-import static io.servicetalk.client.api.ServiceDiscovererEvent.Status.UNAVAILABLE;
 import static java.util.Objects.requireNonNull;
 
 /**
@@ -27,19 +25,6 @@ public final class DefaultServiceDiscovererEvent<T> implements ServiceDiscoverer
     private final T address;
     private final Status status;
 
-    /**
-     * Create a new instance.
-     * @param address The address returned by {@link #address()}.
-     * @param available Value used to determine {@link #status()}.
-     * @deprecated Use
-     * {@link #DefaultServiceDiscovererEvent(Object, io.servicetalk.client.api.ServiceDiscovererEvent.Status)}.
-     */
-    @Deprecated
-    public DefaultServiceDiscovererEvent(T address, boolean available) {
-        this.address = requireNonNull(address);
-        this.status = available ? AVAILABLE : UNAVAILABLE;
-    }
-
     /**
      * Create a new instance.
      * @param address The address returned by {@link #address()}.
@@ -60,11 +45,6 @@ public Status status() {
         return status;
     }
 
-    @Override
-    public boolean isAvailable() {
-        return AVAILABLE.equals(status);
-    }
-
     @Override
     public boolean equals(final Object o) {
         if (this == o) {
@@ -89,7 +69,6 @@ public String toString() {
         return "DefaultServiceDiscovererEvent{" +
                 "address=" + address +
                 ", status=" + status +
-                ", available=" + isAvailable() +
                 '}';
     }
 }
diff --git a/servicetalk-client-api/src/main/java/io/servicetalk/client/api/ServiceDiscovererEvent.java b/servicetalk-client-api/src/main/java/io/servicetalk/client/api/ServiceDiscovererEvent.java
@@ -17,9 +17,6 @@
 
 import java.util.Locale;
 
-import static io.servicetalk.client.api.ServiceDiscovererEvent.Status.AVAILABLE;
-import static io.servicetalk.client.api.ServiceDiscovererEvent.Status.UNAVAILABLE;
-
 /**
  * Notification from the Service Discovery system that availability for an address has changed.
  * @param <ResolvedAddress> the type of address after resolution.
@@ -38,27 +35,9 @@ public interface ServiceDiscovererEvent<ResolvedAddress> {
     /**
      * {@link Status Status} of the event instructing the {@link ServiceDiscoverer} what actions
      * to take upon the associated {@link #address() address}.
-     * <p>
-     * Note, the default implementation calls {@link #isAvailable()} to allow frictionless adoption, but once the
-     * implementing class removes the override for the deprecated method {@link #isAvailable()},
-     * it will be also necessary to override {@link #status()}.
      * @return {@link Status Status} of the associated {@link #address()}.
      */
-    default Status status() {
-        return isAvailable() ? AVAILABLE : UNAVAILABLE;
-    }
-
-    /**
-     * Determine if {@link #address()} is now available or unavailable.
-     * @return {@code true} if {@link #address()} is now available or false if the {@link #address()} is now
-     * unavailable.
-     * @deprecated Implement and use {@link #status()}. This method will be removed.
-     */
-    @Deprecated
-    default boolean isAvailable() {
-        throw new UnsupportedOperationException("Migrate to status() method. This method may be implemented" +
-                " temporarily until migration to status() is complete.");
-    }
+    Status status();
 
     /**
      * Status provided by the {@link ServiceDiscoverer} system that guides the actions of {@link LoadBalancer} upon the

diff --git a/...ns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java b/...ns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java
@@ -70,7 +70,6 @@
 import static io.netty.handler.codec.dns.DefaultDnsRecordDecoder.decodeName;
 import static io.netty.handler.codec.dns.DnsRecordType.SRV;
 import static io.servicetalk.client.api.ServiceDiscovererEvent.Status.AVAILABLE;
-import static io.servicetalk.client.api.internal.ServiceDiscovererUtils.calculateDifference;
 import static io.servicetalk.concurrent.api.AsyncCloseables.toAsyncCloseable;
 import static io.servicetalk.concurrent.api.Completable.completed;
 import static io.servicetalk.concurrent.api.Publisher.defer;
@@ -86,6 +85,7 @@
 import static io.servicetalk.concurrent.internal.SubscriberUtils.safeOnError;
 import static io.servicetalk.concurrent.internal.ThrowableUtils.unknownStackTrace;
 import static io.servicetalk.dns.discovery.netty.DnsClients.mapEventList;
+import static io.servicetalk.dns.discovery.netty.ServiceDiscovererUtils.calculateDifference;
 import static io.servicetalk.transport.netty.internal.BuilderUtils.datagramChannel;
 import static io.servicetalk.transport.netty.internal.BuilderUtils.socketChannel;
 import static io.servicetalk.transport.netty.internal.EventLoopAwareNettyIoExecutors.toEventLoopAwareNettyIoExecutor;

diff --git a/...-netty/src/main/java/io/servicetalk/dns/discovery/netty/DnsServiceDiscovererObserver.java b/...-netty/src/main/java/io/servicetalk/dns/discovery/netty/DnsServiceDiscovererObserver.java
@@ -94,26 +94,11 @@ interface ResolutionResult {
          */
         int nAvailable();
 
-        /**
-         * Number of resolved records that became {@link ServiceDiscovererEvent.Status#UNAVAILABLE unavailable}.
-         *
-         * @return the number of resolved records that became
-         * {@link ServiceDiscovererEvent.Status#UNAVAILABLE unavailable}
-         * @deprecated Implement and use {@link #nMissing()} method.
-         */
-        @Deprecated
-        default int nUnavailable() {
-            return nMissing();
-        }
-
         /**
          * Number of missing records compared to the previous resolution result.
          *
          * @return number of missing records compared to the previous resolution result.
          */
-        default int nMissing() {
-            throw new UnsupportedOperationException("Method nMissing is not yet implemented by "
-                    + getClass().getName());
-        }
+        int nMissing();
     }
 }
diff --git a/.../api/internal/ServiceDiscovererUtils.java → ...scovery/netty/ServiceDiscovererUtils.java b/.../api/internal/ServiceDiscovererUtils.java → ...scovery/netty/ServiceDiscovererUtils.java
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2018 Apple Inc. and the ServiceTalk project authors
+ * Copyright © 2021 Apple Inc. and the ServiceTalk project authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package io.servicetalk.client.api.internal;
+package io.servicetalk.dns.discovery.netty;
 
 import io.servicetalk.client.api.DefaultServiceDiscovererEvent;
 import io.servicetalk.client.api.ServiceDiscoverer;

diff --git a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/HttpClientBuilderTest.java b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/HttpClientBuilderTest.java
@@ -34,6 +34,7 @@
 import java.util.Collections;
 import java.util.concurrent.ExecutionException;
 
+import static io.servicetalk.client.api.ServiceDiscovererEvent.Status.AVAILABLE;
 import static io.servicetalk.concurrent.api.Completable.completed;
 import static io.servicetalk.http.api.HttpExecutionStrategies.noOffloadsStrategy;
 import static io.servicetalk.transport.netty.internal.AddressUtils.serverHostAndPort;
@@ -49,7 +50,7 @@ class HttpClientBuilderTest extends AbstractEchoServerBasedHttpRequesterTest {
     void httpClientWithStaticLoadBalancing() throws Exception {
 
         DefaultServiceDiscovererEvent<InetSocketAddress> sdEvent = new DefaultServiceDiscovererEvent<>(
-                (InetSocketAddress) serverContext.listenAddress(), true);
+                (InetSocketAddress) serverContext.listenAddress(), AVAILABLE);
 
         sendRequestAndValidate(Publisher.from(sdEvent));
     }
@@ -60,7 +61,7 @@ void httpClientWithDynamicLoadBalancing() throws Exception {
         TestPublisher<ServiceDiscovererEvent<InetSocketAddress>> sdPub = new TestPublisher<>();
 
         DefaultServiceDiscovererEvent<InetSocketAddress> sdEvent = new DefaultServiceDiscovererEvent<>(
-                (InetSocketAddress) serverContext.listenAddress(), true);
+                (InetSocketAddress) serverContext.listenAddress(), AVAILABLE);
 
         // Simulate delayed discovery
         CTX.executor().schedule(() -> {