Fix RouteKey and surrounding code

This cleans up RouteKey to be a properly-designed and efficient
DTO:
- prevent useless boxing
- use proper prime in hashCode
- make sure nullness is propagated
- improve compareTo performance

Change-Id: Ibfa1f57fee8dbc4a0a4c1a0f0e47664b48f4d055
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
index 1311ca50ccc3a79d8e973b218e4d1a7b15c68f82..17e4aec849557e7b00645501e2b21a0fe2f81368 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
@@ -82,7 +82,7 @@ public abstract class AddPathAbstractRouteEntry<C extends Routes & DataObject &
             this.offsets.setValue(this.pathsId, offset, ++this.pathIdCounter);
         }
         this.offsets.setValue(this.values, offset, route);
-        LOG.trace("Added route {} from {}", route, key.getRouteId());
+        LOG.trace("Added route {} from {}", route, routerId);
         return offset;
     }
 

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathBestPath.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathBestPath.java
index c87beeb2ac2cc083c9de7bbc19bf2a89d7a1fc82..7fc2a411fa16be612c672f20244c9253d1263b30 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathBestPath.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathBestPath.java
@@ -77,12 +77,12 @@ public final class AddPathBestPath extends AbstractBestPath {
 
     @Override
     public UnsignedInteger getRouterId() {
-        return this.routeKey.getRouteId();
+        return this.routeKey.getRouterId();
     }
 
     @Override
     public PeerId getPeerId() {
-        return RouterIds.createPeerId(this.routeKey.getRouteId());
+        return RouterIds.createPeerId(this.routeKey.getRouterId());
     }
 
     @Override

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathSelector.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathSelector.java
index 65be4c067d78979c89f8604f7f1b864e5137457f..021885c358d08caf61cf89997269cb0f48457723 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathSelector.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathSelector.java
@@ -7,8 +7,6 @@
  */
 package org.opendaylight.protocol.bgp.mode.impl.add;
 
-import static java.util.Objects.requireNonNull;
-
 import com.google.common.primitives.UnsignedInteger;
 import org.opendaylight.protocol.bgp.mode.api.BestPathState;
 import org.opendaylight.protocol.bgp.mode.impl.BestPathStateImpl;
@@ -30,11 +28,10 @@ public final class AddPathSelector extends AbstractBestPathSelector {
     }
 
     void processPath(final Attributes attrs, final RouteKey key, final int offsetPosition, final long pathId) {
-        requireNonNull(key.getRouteId(), "Router ID may not be null");
-
         // Consider only non-null attributes
         if (attrs != null) {
-            final UnsignedInteger originatorId = replaceOriginator(key.getRouteId(), attrs.getOriginatorId());
+            final UnsignedInteger routerId = key.getRouterId();
+            final UnsignedInteger originatorId = replaceOriginator(routerId, attrs.getOriginatorId());
 
             /*
              * Store the new details if we have nothing stored or when the selection algorithm indicates new details
@@ -42,7 +39,7 @@ public final class AddPathSelector extends AbstractBestPathSelector {
              */
             final BestPathState state = new BestPathStateImpl(attrs);
             if (this.bestOriginatorId == null || !isExistingPathBetter(state)) {
-                LOG.trace("Selecting path from router {}", RouterIds.createPeerId(key.getRouteId()).getValue());
+                LOG.trace("Selecting path from router {}", RouterIds.createPeerId(routerId).getValue());
                 this.bestOriginatorId = originatorId;
                 this.bestState = state;
                 this.bestRouteKey = key;

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/RouteKey.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/RouteKey.java
index faf54d9d1d79c0895d30b93da240f2f915028275..27696d59d4e6daead48729e35e6e08daf9f34d0a 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/RouteKey.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/RouteKey.java
@@ -7,65 +7,61 @@
  */
 package org.opendaylight.protocol.bgp.mode.impl.add;
 
+import static java.util.Objects.requireNonNull;
+
 import com.google.common.base.MoreObjects;
 import com.google.common.primitives.UnsignedInteger;
-import javax.annotation.Nonnull;
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 
+@NonNullByDefault
 public final class RouteKey implements Comparable<RouteKey> {
-    private final Long remotePathId;
     private final UnsignedInteger routerId;
+    private final long remotePathId;
 
-    public RouteKey(final UnsignedInteger routerId, final Long remotePathId) {
-        this.routerId = routerId;
-        this.remotePathId = remotePathId != null ? remotePathId : 0;
-    }
-
-    private Long getExternalPathId() {
-        return this.remotePathId;
+    public RouteKey(final UnsignedInteger routerId, final long remotePathId) {
+        this.routerId = requireNonNull(routerId);
+        this.remotePathId = remotePathId;
     }
 
-    public UnsignedInteger getRouteId() {
-        return this.routerId;
-    }
-
-    private MoreObjects.ToStringHelper addToStringAttributes(final MoreObjects.ToStringHelper toStringHelper) {
-        toStringHelper.add("externalPathId", this.remotePathId);
-        toStringHelper.add("routerId", this.routerId);
-        return toStringHelper;
+    public UnsignedInteger getRouterId() {
+        return routerId;
     }
 
     @Override
     public int hashCode() {
-        final int prime = 27;
+        final int prime = 31;
         int result = 1;
-        result = prime * result + this.remotePathId.hashCode();
-        result = prime * result + this.routerId.hashCode();
+        result = prime * result + Long.hashCode(remotePathId);
+        result = prime * result + routerId.hashCode();
         return result;
     }
 
     @Override
-    public boolean equals(final Object obj) {
+    public boolean equals(final @Nullable Object obj) {
         if (this == obj) {
             return true;
         }
-
         if (!(obj instanceof RouteKey)) {
             return false;
         }
 
         final RouteKey other = (RouteKey) obj;
-        return this.remotePathId.equals(other.remotePathId) && this.routerId.equals(other.routerId);
+        return remotePathId == other.remotePathId && routerId.equals(other.routerId);
     }
 
     @Override
     public String toString() {
-        return addToStringAttributes(MoreObjects.toStringHelper(this)).toString();
+        return MoreObjects.toStringHelper(this)
+                .add("externalPathId", remotePathId)
+                .add("routerId", routerId)
+                .toString();
     }
 
     @Override
-    public int compareTo(@Nonnull final RouteKey otherRouteKey) {
-        final int routeIdCompareTo = this.routerId.compareTo(otherRouteKey.getRouteId());
-        return routeIdCompareTo == 0 ? this.remotePathId.compareTo(otherRouteKey.getExternalPathId())
-                : routeIdCompareTo;
+    public int compareTo(final RouteKey otherRouteKey) {
+        int cmp;
+        return (cmp = routerId.compareTo(otherRouteKey.routerId)) != 0 ? cmp
+                : Long.compare(remotePathId, otherRouteKey.remotePathId);
     }
 }

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/all/paths/AllPathsRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/all/paths/AllPathsRouteEntry.java
index ab2f96f6aa31f30dd50f7acf6d04c7e073e7584c..dc2ff974fb05333c968ac30e4ecb82ee35fb603b 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/all/paths/AllPathsRouteEntry.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/all/paths/AllPathsRouteEntry.java
@@ -8,8 +8,6 @@
 
 package org.opendaylight.protocol.bgp.mode.impl.add.all.paths;
 
-import static java.util.Objects.requireNonNull;
-
 import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
 import java.util.List;
@@ -47,7 +45,6 @@ final class AllPathsRouteEntry<C extends Routes & DataObject & ChoiceIn<Tables>,
             for (final RouteKey key : keyList) {
                 final int offset = this.offsets.offsetOf(key);
                 final Route route = this.offsets.getValue(this.values, offset);
-                requireNonNull(key.getRouteId(), "Router ID may not be null");
                 if (route != null) {
                     final BestPathState state = new BestPathStateImpl(route.getAttributes());
                     final AddPathBestPath bestPath = new AddPathBestPath(state, key, offset,