Fix neutron pagination 53/12653/4
authorRobert Varga <rovarga@cisco.com>
Fri, 7 Nov 2014 20:33:28 +0000 (21:33 +0100)
committerRobert Varga <rovarga@cisco.com>
Sat, 8 Nov 2014 10:47:51 +0000 (11:47 +0100)
This patch fixes a logic mistake which results in dead code being
reported. startPos is guaranteed to be non-null by virtue of being
always assigned.

The problem is that the detection that the marker is not found is not
correct, as binarySearch() will report the negated insertion point.

Also improves performance by using a primitive type instead of an
encapsulated object.

Change-Id: I525c2070d2794b1e001465ea925d95d432feca29
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/northbound/networkconfiguration/neutron/src/main/java/org/opendaylight/controller/networkconfig/neutron/northbound/PaginatedRequestFactory.java

index 0e8e605527fb3ca726cfaf07779ff1bad68b5876..7f6c296d98e8403e18070886753e8970bb921dfc 100644 (file)
@@ -40,6 +40,24 @@ public class PaginatedRequestFactory {
         }
     }
 
+    private static final class MarkerObject implements INeutronObject {
+        private final String id;
+
+        MarkerObject(String id) {
+            this.id = id;
+        }
+
+        @Override
+        public String getID() {
+            return id;
+        }
+
+        @Override
+        public void setID(String id) {
+            throw new UnsupportedOperationException("Marker has constant ID");
+        }
+    }
+
     /*
      * SuppressWarnings is needed because the compiler does not understand that we
      * are actually safe here.
@@ -68,7 +86,7 @@ public class PaginatedRequestFactory {
 
     private static <T extends INeutronObject> PaginationResults<T> _paginate(Integer limit, String marker, Boolean pageReverse, UriInfo uriInfo, List<T> collection) {
         List<NeutronPageLink> links = new ArrayList<>();
-        Integer startPos = null;
+        final int startPos;
         String startMarker;
         String endMarker;
         Boolean firstPage = false;
@@ -76,43 +94,21 @@ public class PaginatedRequestFactory {
 
         Collections.sort(collection, NEUTRON_OBJECT_COMPARATOR);
 
-        if (marker == null) {
-            startPos = 0;
-        }
-
-        else {
-
-            class MarkerObject implements INeutronObject {
-                private String id;
-
-                @Override
-                public String getID() {
-                    return id;
-                }
-
-                @Override
-                public void setID(String id) {
-                    this.id = id;
-                }
+        if (marker != null) {
+            int offset = Collections.binarySearch(collection, new MarkerObject(marker), NEUTRON_OBJECT_COMPARATOR);
+            if (offset < 0) {
+                throw new ResourceNotFoundException("UUID for marker: " + marker + " could not be found");
             }
 
-            INeutronObject markerObject = new MarkerObject();
-
-            markerObject.setID(marker);
-
-            startPos = Collections.binarySearch(collection, markerObject, NEUTRON_OBJECT_COMPARATOR);
-
-            if (!pageReverse){
-                startPos = startPos + 1;
+            if (!pageReverse) {
+                startPos = offset + 1;
             }
             else {
-                startPos = startPos - limit;
+                startPos = offset - limit;
             }
-
         }
-
-        if (startPos == null) {
-            throw new ResourceNotFoundException("UUID for marker:" + marker + " could not be found");
+        else {
+            startPos = 0;
         }
 
         if (startPos == 0){