Normalize route objects stored in RouteTable ConsistentMultimap to avoid inconsistent serialization for different IpPrefix/IpAddress types
Change-Id: I3e792fb92afdf388d2eaec5bd04dc47347f910f5
(cherry picked from commit 30ffafd7883b752fbc579582f6de5dc2e3c829d1)
diff --git a/apps/route-service/app/src/main/java/org/onosproject/routeservice/store/DefaultRouteTable.java b/apps/route-service/app/src/main/java/org/onosproject/routeservice/store/DefaultRouteTable.java
index 476f3c0..b52618c 100644
--- a/apps/route-service/app/src/main/java/org/onosproject/routeservice/store/DefaultRouteTable.java
+++ b/apps/route-service/app/src/main/java/org/onosproject/routeservice/store/DefaultRouteTable.java
@@ -23,10 +23,10 @@
import java.util.function.Consumer;
import java.util.stream.Collectors;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import org.onlab.packet.IpAddress;
import org.onlab.packet.IpPrefix;
+import org.onosproject.cluster.NodeId;
import org.onlab.util.KryoNamespace;
import org.onosproject.routeservice.InternalRouteEvent;
import org.onosproject.routeservice.Route;
@@ -50,7 +50,12 @@
public class DefaultRouteTable implements RouteTable {
private final RouteTableId id;
- private final ConsistentMultimap<IpPrefix, Route> routes;
+
+ // The route map stores RawRoute instead of Route to translate the polymorphic IpPrefix and IpAddress types
+ // into monomorphic types (specifically String). Using strings in the stored RawRoute is necessary to ensure
+ // the serialized bytes are consistent whether e.g. IpAddress or Ip4Address is used when storing a route.
+ private final ConsistentMultimap<String, RawRoute> routes;
+
private final RouteStoreDelegate delegate;
private final ExecutorService executor;
private final RouteTableListener listener = new RouteTableListener();
@@ -89,13 +94,14 @@
new InternalRouteEvent(InternalRouteEvent.Type.ROUTE_ADDED, routeSet)));
}
- private ConsistentMultimap<IpPrefix, Route> buildRouteMap(StorageService storageService) {
+ private ConsistentMultimap<String, RawRoute> buildRouteMap(StorageService storageService) {
KryoNamespace routeTableSerializer = KryoNamespace.newBuilder()
.register(KryoNamespaces.API)
.register(Route.class)
.register(Route.Source.class)
+ .register(RawRoute.class)
.build();
- return storageService.<IpPrefix, Route>consistentMultimapBuilder()
+ return storageService.<String, RawRoute>consistentMultimapBuilder()
.withName("onos-routes-" + id.name())
.withRelaxedReadConsistency()
.withSerializer(Serializer.using(routeTableSerializer))
@@ -121,36 +127,37 @@
@Override
public void update(Route route) {
- routes.put(route.prefix(), route);
+ routes.put(route.prefix().toString(), new RawRoute(route));
}
@Override
public void remove(Route route) {
- routes.remove(route.prefix(), route);
+ routes.remove(route.prefix().toString(), new RawRoute(route));
}
@Override
public void replace(Route route) {
- routes.replaceValues(route.prefix(), Sets.newHashSet(route));
+ routes.replaceValues(route.prefix().toString(), Sets.newHashSet(new RawRoute(route)));
}
@Override
public Collection<RouteSet> getRoutes() {
return routes.stream()
.map(Map.Entry::getValue)
- .collect(Collectors.groupingBy(Route::prefix))
+ .collect(Collectors.groupingBy(RawRoute::prefix))
.entrySet()
.stream()
- .map(entry -> new RouteSet(id, entry.getKey(), ImmutableSet.copyOf(entry.getValue())))
+ .map(entry -> new RouteSet(id,
+ IpPrefix.valueOf(entry.getKey()),
+ entry.getValue().stream().map(RawRoute::route).collect(Collectors.toSet())))
.collect(Collectors.toList());
}
@Override
public RouteSet getRoutes(IpPrefix prefix) {
- Versioned<Collection<? extends Route>> routeSet = routes.get(prefix);
-
+ Versioned<Collection<? extends RawRoute>> routeSet = routes.get(prefix.toString());
if (routeSet != null) {
- return new RouteSet(id, prefix, ImmutableSet.copyOf(routeSet.value()));
+ return new RouteSet(id, prefix, routeSet.value().stream().map(RawRoute::route).collect(Collectors.toSet()));
}
return null;
}
@@ -159,22 +166,25 @@
public Collection<Route> getRoutesForNextHop(IpAddress nextHop) {
return routes.stream()
.map(Map.Entry::getValue)
- .filter(r -> r.nextHop().equals(nextHop))
+ .filter(r -> IpAddress.valueOf(r.nextHop()).equals(nextHop))
+ .map(RawRoute::route)
.collect(Collectors.toSet());
}
private class RouteTableListener
- implements MultimapEventListener<IpPrefix, Route> {
+ implements MultimapEventListener<String, RawRoute> {
private InternalRouteEvent createRouteEvent(
- InternalRouteEvent.Type type, MultimapEvent<IpPrefix, Route> event) {
- Collection<? extends Route> currentRoutes = Versioned.valueOrNull(routes.get(event.key()));
+ InternalRouteEvent.Type type, MultimapEvent<String, RawRoute> event) {
+ Collection<? extends RawRoute> currentRoutes = Versioned.valueOrNull(routes.get(event.key()));
return new InternalRouteEvent(type, new RouteSet(
- id, event.key(), currentRoutes != null ? ImmutableSet.copyOf(currentRoutes) : Collections.emptySet()));
+ id, IpPrefix.valueOf(event.key()), currentRoutes != null ?
+ currentRoutes.stream().map(RawRoute::route).collect(Collectors.toSet())
+ : Collections.emptySet()));
}
@Override
- public void event(MultimapEvent<IpPrefix, Route> event) {
+ public void event(MultimapEvent<String, RawRoute> event) {
InternalRouteEvent ire = null;
switch (event.type()) {
case INSERT:
@@ -190,4 +200,32 @@
}
}
+ /**
+ * Represents a route object stored in the underlying ConsistentMultimap.
+ */
+ private static class RawRoute {
+ private Route.Source source;
+ private String prefix;
+ private String nextHop;
+ private NodeId sourceNode;
+
+ RawRoute(Route route) {
+ this.source = route.source();
+ this.prefix = route.prefix().toString();
+ this.nextHop = route.nextHop().toString();
+ this.sourceNode = route.sourceNode();
+ }
+
+ String prefix() {
+ return prefix;
+ }
+
+ String nextHop() {
+ return nextHop;
+ }
+
+ Route route() {
+ return new Route(source, IpPrefix.valueOf(prefix), IpAddress.valueOf(nextHop), sourceNode);
+ }
+ }
}