Introduce type parameters where applicable for xDS classes.

Motivation: Following the discussion in this [comment](#5336 (comment)), incorporating type parameters in xDS classes offers advantages such as improved type safety and enhanced readability. Modifications: - Introduce type parameters where applicable. - Split `AbstractResourceNode` into `AbstractResourceNode` and `AbstractResourceNodeWithPrimer` to accommodate the absence of a primer in the listener. - Rename `AbstractResourceHolder` to `ResourceHolderWithPrimer` for consistency due to the absence of a primer in the listener. - Conduct clean-up in some APIs. Result: - Enhanced type safety, improved readability, and a more structured representation in xDS classes.
line · Jan 26, 2024 · 012b9d1 · 012b9d1
1 parent fbcd997
commit 012b9d1
Show file tree

Hide file tree

Showing 41 changed files with 296 additions and 229 deletions.
diff --git a/xds/src/main/java/com/linecorp/armeria/xds/AbstractResourceHolder.java b/xds/src/main/java/com/linecorp/armeria/xds/AbstractResourceHolder.java
diff --git a/xds/src/main/java/com/linecorp/armeria/xds/AbstractResourceNode.java b/xds/src/main/java/com/linecorp/armeria/xds/AbstractResourceNode.java
@@ -24,7 +24,7 @@
 import io.envoyproxy.envoy.config.core.v3.ConfigSource;
 import io.grpc.Status;
 
-abstract class AbstractResourceNode<T> implements ResourceNode<AbstractResourceHolder> {
+abstract class AbstractResourceNode<T extends ResourceHolder<?>> implements ResourceNode<T> {
 
     private final Deque<ResourceNode<?>> children = new ArrayDeque<>();
 
@@ -33,21 +33,18 @@ abstract class AbstractResourceNode<T> implements ResourceNode<AbstractResourceH
     private final ConfigSource configSource;
     private final XdsType type;
     private final String resourceName;
-    @Nullable
-    private final ResourceHolder primer;
-    private final SnapshotWatcher<? super T> parentWatcher;
+    private final SnapshotWatcher<?> parentWatcher;
     private final ResourceNodeType resourceNodeType;
     @Nullable
-    private AbstractResourceHolder current;
+    private T current;
 
     AbstractResourceNode(XdsBootstrapImpl xdsBootstrap, @Nullable ConfigSource configSource,
-                         XdsType type, String resourceName, @Nullable ResourceHolder primer,
-                         SnapshotWatcher<? super T> parentWatcher, ResourceNodeType resourceNodeType) {
+                         XdsType type, String resourceName,
+                         SnapshotWatcher<?> parentWatcher, ResourceNodeType resourceNodeType) {
         this.xdsBootstrap = xdsBootstrap;
         this.configSource = configSource;
         this.type = type;
         this.resourceName = resourceName;
-        this.primer = primer;
         this.parentWatcher = parentWatcher;
         this.resourceNodeType = resourceNodeType;
     }
@@ -56,12 +53,17 @@ XdsBootstrapImpl xdsBootstrap() {
         return xdsBootstrap;
     }
 
-    private void setCurrent(@Nullable AbstractResourceHolder current) {
+    @Override
+    public ConfigSource configSource() {
+        return configSource;
+    }
+
+    private void setCurrent(@Nullable T current) {
         this.current = current;
     }
 
     @Override
-    public AbstractResourceHolder currentResourceHolder() {
+    public T currentResourceHolder() {
         return current;
     }
 
@@ -82,10 +84,8 @@ public void onResourceDoesNotExist(XdsType type, String resourceName) {
     }
 
     @Override
-    public final void onChanged(AbstractResourceHolder update) {
+    public void onChanged(T update) {
         assert update.type() == type();
-
-        update = update.withPrimer(primer);
         setCurrent(update);
 
         final Deque<ResourceNode<?>> prevChildren = new ArrayDeque<>(children);
@@ -98,7 +98,7 @@ public final void onChanged(AbstractResourceHolder update) {
         }
     }
 
-    abstract void doOnChanged(ResourceHolder update);
+    abstract void doOnChanged(T update);
 
     @Override
     public void close() {
@@ -107,18 +107,14 @@ public void close() {
         }
         children.clear();
         if (resourceNodeType == ResourceNodeType.DYNAMIC) {
-            xdsBootstrap.unsubscribe(configSource, this);
+            xdsBootstrap.unsubscribe(this);
         }
     }
 
     Deque<ResourceNode<?>> children() {
         return children;
     }
 
-    SnapshotWatcher<? super T> parentWatcher() {
-        return parentWatcher;
-    }
-
     @Override
     public XdsType type() {
         return type;

diff --git a/xds/src/main/java/com/linecorp/armeria/xds/AbstractResourceNodeWithPrimer.java b/xds/src/main/java/com/linecorp/armeria/xds/AbstractResourceNodeWithPrimer.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2023 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you 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:
+ *
+ *   https://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 com.linecorp.armeria.xds;
+
+import com.linecorp.armeria.common.annotation.Nullable;
+
+import io.envoyproxy.envoy.config.core.v3.ConfigSource;
+
+abstract class AbstractResourceNodeWithPrimer
+        <T extends ResourceHolderWithPrimer<T, ?, U>, U extends ResourceHolder<?>>
+        extends AbstractResourceNode<T> {
+
+    @Nullable
+    private final U primer;
+
+    AbstractResourceNodeWithPrimer(XdsBootstrapImpl xdsBootstrap, @Nullable ConfigSource configSource,
+                                   XdsType type, String resourceName, @Nullable U primer,
+                                   SnapshotWatcher<?> parentWatcher, ResourceNodeType resourceNodeType) {
+        super(xdsBootstrap, configSource, type, resourceName, parentWatcher, resourceNodeType);
+        this.primer = primer;
+    }
+
+    @Override
+    public void onChanged(T update) {
+        assert update.type() == type();
+        super.onChanged(update.withPrimer(primer));
+    }
+}
diff --git a/xds/src/main/java/com/linecorp/armeria/xds/AbstractRoot.java b/xds/src/main/java/com/linecorp/armeria/xds/AbstractRoot.java
@@ -34,7 +34,7 @@
 import io.grpc.Status;
 import io.netty.util.concurrent.EventExecutor;
 
-abstract class AbstractRoot<T extends Snapshot<? extends ResourceHolder>>
+abstract class AbstractRoot<T extends Snapshot<? extends ResourceHolder<?>>>
         implements SnapshotWatcher<T>, SafeCloseable {
 
     private static final Logger logger = LoggerFactory.getLogger(AbstractRoot.class);

diff --git a/xds/src/main/java/com/linecorp/armeria/xds/BootstrapApiConfigs.java b/xds/src/main/java/com/linecorp/armeria/xds/BootstrapApiConfigs.java
@@ -50,21 +50,21 @@ final class BootstrapApiConfigs {
         }
     }
 
-    ConfigSource remapConfigSource(XdsType type, @Nullable ConfigSource configSource,
-                                   String resourceName) {
+    ConfigSource configSource(XdsType type, String resourceName, ResourceNode<?> node) {
         if (type == XdsType.LISTENER) {
-            return ldsConfigSource(configSource);
+            return ldsConfigSource(node);
         } else if (type == XdsType.ROUTE) {
-            return rdsConfigSource(configSource, resourceName);
+            return rdsConfigSource(node, resourceName);
         } else if (type == XdsType.CLUSTER) {
-            return cdsConfigSource(configSource, resourceName);
+            return cdsConfigSource(node, resourceName);
         } else {
             assert type == XdsType.ENDPOINT;
-            return edsConfigSource(configSource, resourceName);
+            return edsConfigSource(node, resourceName);
         }
     }
 
-    ConfigSource edsConfigSource(@Nullable ConfigSource configSource, String resourceName) {
+    ConfigSource edsConfigSource(ResourceNode<?> node, String resourceName) {
+        final ConfigSource configSource = node.configSource();
         if (configSource != null && configSource.hasApiConfigSource()) {
             return configSource;
         }
@@ -74,7 +74,8 @@ ConfigSource edsConfigSource(@Nullable ConfigSource configSource, String resourc
         throw new IllegalArgumentException("Cannot find an EDS config source for " + resourceName);
     }
 
-    ConfigSource cdsConfigSource(@Nullable ConfigSource configSource, String resourceName) {
+    ConfigSource cdsConfigSource(ResourceNode<?> node, String resourceName) {
+        final ConfigSource configSource = node.configSource();
         if (configSource != null && configSource.hasApiConfigSource()) {
             return configSource;
         }
@@ -90,7 +91,8 @@ ConfigSource cdsConfigSource(@Nullable ConfigSource configSource, String resourc
         throw new IllegalArgumentException("Cannot find a CDS config source for route: " + resourceName);
     }
 
-    ConfigSource rdsConfigSource(@Nullable ConfigSource configSource, String resourceName) {
+    ConfigSource rdsConfigSource(ResourceNode<?> node, String resourceName) {
+        final ConfigSource configSource = node.configSource();
         if (configSource != null && configSource.hasApiConfigSource()) {
             return configSource;
         }
@@ -100,7 +102,8 @@ ConfigSource rdsConfigSource(@Nullable ConfigSource configSource, String resourc
         throw new IllegalArgumentException("Cannot find an RDS config source for route: " + resourceName);
     }
 
-    ConfigSource ldsConfigSource(@Nullable ConfigSource configSource) {
+    ConfigSource ldsConfigSource(ResourceNode<?> node) {
+        final ConfigSource configSource = node.configSource();
         if (configSource != null && configSource.hasApiConfigSource()) {
             return configSource;
         }

diff --git a/xds/src/main/java/com/linecorp/armeria/xds/BootstrapClusters.java b/xds/src/main/java/com/linecorp/armeria/xds/BootstrapClusters.java
@@ -28,7 +28,7 @@
 import io.envoyproxy.envoy.config.cluster.v3.Cluster;
 import io.grpc.Status;
 
-final class BootstrapClusters implements SnapshotWatcher<Snapshot<?>> {
+final class BootstrapClusters implements SnapshotWatcher<ClusterSnapshot> {
 
     private final Map<String, ClusterSnapshot> clusterSnapshots = new HashMap<>();
 
@@ -46,10 +46,8 @@ final class BootstrapClusters implements SnapshotWatcher<Snapshot<?>> {
     }
 
     @Override
-    public void snapshotUpdated(Snapshot<?> newSnapshot) {
-        assert newSnapshot instanceof ClusterSnapshot;
-        final ClusterSnapshot clusterSnapshot = (ClusterSnapshot) newSnapshot;
-        clusterSnapshots.put(clusterSnapshot.holder().name(), clusterSnapshot);
+    public void snapshotUpdated(ClusterSnapshot newSnapshot) {
+        clusterSnapshots.put(newSnapshot.holder().name(), newSnapshot);
     }
 
     @Nullable

diff --git a/xds/src/main/java/com/linecorp/armeria/xds/ClusterResourceHolder.java b/xds/src/main/java/com/linecorp/armeria/xds/ClusterResourceHolder.java
@@ -29,7 +29,8 @@
 /**
  * A resource holder object for a {@link Cluster}.
  */
-public final class ClusterResourceHolder extends AbstractResourceHolder {
+public final class ClusterResourceHolder
+        extends ResourceHolderWithPrimer<ClusterResourceHolder, Cluster, RouteResourceHolder> {
 
     private final Cluster cluster;
     @Nullable
@@ -45,12 +46,11 @@ public final class ClusterResourceHolder extends AbstractResourceHolder {
     }
 
     @Override
-    ClusterResourceHolder withPrimer(@Nullable ResourceHolder primer) {
+    ClusterResourceHolder withPrimer(@Nullable RouteResourceHolder primer) {
         if (primer == null) {
             return this;
         }
-        checkArgument(primer instanceof RouteResourceHolder);
-        return new ClusterResourceHolder(cluster, (RouteResourceHolder) primer);
+        return new ClusterResourceHolder(cluster, primer);
     }
 
     @Nullable

diff --git a/xds/src/main/java/com/linecorp/armeria/xds/ClusterResourceNode.java b/xds/src/main/java/com/linecorp/armeria/xds/ClusterResourceNode.java
@@ -30,38 +30,42 @@
 import io.envoyproxy.envoy.config.route.v3.VirtualHost;
 import io.grpc.Status;
 
-final class ClusterResourceNode extends AbstractResourceNode<ClusterSnapshot> {
+final class ClusterResourceNode
+        extends AbstractResourceNodeWithPrimer<ClusterResourceHolder, RouteResourceHolder> {
 
     @Nullable
     private final VirtualHost virtualHost;
     @Nullable
     private final Route route;
     private final int index;
     private final EndpointSnapshotWatcher snapshotWatcher = new EndpointSnapshotWatcher();
+    private final SnapshotWatcher<ClusterSnapshot> parentWatcher;
 
     ClusterResourceNode(@Nullable ConfigSource configSource,
                         String resourceName, XdsBootstrapImpl xdsBootstrap,
-                        @Nullable ResourceHolder primer, SnapshotWatcher<? super ClusterSnapshot> parentWatcher,
+                        @Nullable RouteResourceHolder primer,
+                        SnapshotWatcher<ClusterSnapshot> parentWatcher,
                         ResourceNodeType resourceNodeType) {
         super(xdsBootstrap, configSource, CLUSTER, resourceName, primer, parentWatcher, resourceNodeType);
+        this.parentWatcher = parentWatcher;
         virtualHost = null;
         route = null;
         index = -1;
     }
 
     ClusterResourceNode(@Nullable ConfigSource configSource,
                         String resourceName, XdsBootstrapImpl xdsBootstrap,
-                        @Nullable ResourceHolder primer, SnapshotWatcher<ClusterSnapshot> parentWatcher,
+                        @Nullable RouteResourceHolder primer, SnapshotWatcher<ClusterSnapshot> parentWatcher,
                         VirtualHost virtualHost, Route route, int index, ResourceNodeType resourceNodeType) {
         super(xdsBootstrap, configSource, CLUSTER, resourceName, primer, parentWatcher, resourceNodeType);
+        this.parentWatcher = parentWatcher;
         this.virtualHost = requireNonNull(virtualHost, "virtualHost");
         this.route = requireNonNull(route, "route");
         this.index = index;
     }
 
     @Override
-    public void doOnChanged(ResourceHolder update) {
-        final ClusterResourceHolder holder = (ClusterResourceHolder) update;
+    public void doOnChanged(ClusterResourceHolder holder) {
         final Cluster cluster = holder.resource();
         if (cluster.hasLoadAssignment()) {
             final ClusterLoadAssignment loadAssignment = cluster.getLoadAssignment();
@@ -75,17 +79,12 @@ public void doOnChanged(ResourceHolder update) {
                     new EndpointResourceNode(configSource, cluster.getName(), xdsBootstrap(), holder,
                                              snapshotWatcher, ResourceNodeType.DYNAMIC);
             children().add(node);
-            xdsBootstrap().subscribe(configSource, node);
+            xdsBootstrap().subscribe(node);
         } else {
-            parentWatcher().snapshotUpdated(new ClusterSnapshot(holder));
+            parentWatcher.snapshotUpdated(new ClusterSnapshot(holder));
         }
     }
 
-    @Override
-    public ClusterResourceHolder currentResourceHolder() {
-        return (ClusterResourceHolder) super.currentResourceHolder();
-    }
-
     private class EndpointSnapshotWatcher implements SnapshotWatcher<EndpointSnapshot> {
         @Override
         public void snapshotUpdated(EndpointSnapshot newSnapshot) {
@@ -96,18 +95,18 @@ public void snapshotUpdated(EndpointSnapshot newSnapshot) {
             if (!Objects.equals(newSnapshot.holder().primer(), current)) {
                 return;
             }
-            parentWatcher().snapshotUpdated(
+            parentWatcher.snapshotUpdated(
                     new ClusterSnapshot(current, newSnapshot, virtualHost, route, index));
         }
 
         @Override
         public void onError(XdsType type, Status status) {
-            parentWatcher().onError(type, status);
+            parentWatcher.onError(type, status);
         }
 
         @Override
         public void onMissing(XdsType type, String resourceName) {
-            parentWatcher().onMissing(type, resourceName);
+            parentWatcher.onMissing(type, resourceName);
         }
     }
 }
diff --git a/xds/src/main/java/com/linecorp/armeria/xds/ClusterResourceParser.java b/xds/src/main/java/com/linecorp/armeria/xds/ClusterResourceParser.java
@@ -22,7 +22,7 @@
 import io.envoyproxy.envoy.config.cluster.v3.Cluster.EdsClusterConfig;
 import io.envoyproxy.envoy.config.cluster.v3.ClusterOrBuilder;
 
-final class ClusterResourceParser extends ResourceParser {
+final class ClusterResourceParser extends ResourceParser<ClusterResourceHolder> {
 
     static final ClusterResourceParser INSTANCE = new ClusterResourceParser();
 

diff --git a/xds/src/main/java/com/linecorp/armeria/xds/ConfigSourceClient.java b/xds/src/main/java/com/linecorp/armeria/xds/ConfigSourceClient.java
@@ -90,14 +90,14 @@ void updateResources(XdsType type) {
     }
 
     void addSubscriber(XdsType type, String resourceName,
-                       ResourceWatcher<AbstractResourceHolder> watcher) {
+                       ResourceWatcher<?> watcher) {
         if (subscriberStorage.register(type, resourceName, watcher)) {
             updateResources(type);
         }
     }
 
     boolean removeSubscriber(XdsType type, String resourceName,
-                             ResourceWatcher<AbstractResourceHolder> watcher) {
+                             ResourceWatcher<?> watcher) {
         if (subscriberStorage.unregister(type, resourceName, watcher)) {
             updateResources(type);
         }