Rework prependAS() 81/85181/1
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 17 Oct 2019 13:00:59 +0000 (15:00 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 17 Oct 2019 13:00:59 +0000 (15:00 +0200)
We cannot modify lists in-place, so make a proper copy and create
a new list. Fixes the following splat:

java.lang.UnsupportedOperationException: null
at com.google.common.collect.ImmutableList.remove(ImmutableList.java:534) ~[36:com.google.guava:27.1.0.jre]
at org.opendaylight.protocol.bgp.openconfig.routing.policy.statement.actions.AbstractPrependAsPath.prependAS(AbstractPrependAsPath.java:38) ~[232:org.opendaylight.bgpcep.bgp-openconfig-rp-statement:0.13.0.SNAPSHOT]
at org.opendaylight.protocol.bgp.openconfig.routing.policy.statement.actions.LocalAsPathPrependHandler.applyImportAction(LocalAsPathPrependHandler.java:37) ~[232:org.opendaylight.bgpcep.bgp-openconfig-rp-statement:0.13.0.SNAPSHOT]
at org.opendaylight.protocol.bgp.openconfig.routing.policy.statement.actions.LocalAsPathPrependHandler.applyImportAction(LocalAsPathPrependHandler.java:18) ~[232:org.opendaylight.bgpcep.bgp-openconfig-rp-statement:0.13.0.SNAPSHOT]
at org.opendaylight.protocol.bgp.openconfig.routing.policy.spi.registry.ActionsRegistryImpl.applyImportAction(ActionsRegistryImpl.java:314) ~[231:org.opendaylight.bgpcep.bgp-openconfig-rp-spi:0.13.0.SNAPSHOT]
at org.opendaylight.protocol.bgp.openconfig.routing.policy.spi.registry.StatementRegistry.applyImportStatement(StatementRegistry.java:82) ~[231:org.opendaylight.bgpcep.bgp-openconfig-rp-spi:0.13.0.SNAPSHOT]

Change-Id: I6f604e8abdb223fe09e44ac92a752bb6e410ed7f
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/actions/AbstractPrependAsPath.java

index c1662a6f1c28dbcf7e5258bfd9d6810c5e74191f..ac732b5d2621eab2d83a306abf7bfa56abdf83cd 100644 (file)
@@ -7,7 +7,9 @@
  */
 package org.opendaylight.protocol.bgp.openconfig.routing.policy.statement.actions;
 
+import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import org.opendaylight.protocol.util.Values;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.AsNumber;
@@ -20,33 +22,47 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.mess
 abstract class AbstractPrependAsPath {
 
     static final Attributes prependAS(final Attributes attributes, final AsNumber as) {
-        final List<Segments> oldSegments = attributes.getAsPath().getSegments();
+        return new AttributesBuilder(attributes)
+                .setAsPath(new AsPathBuilder()
+                    .setSegments(prependAS(attributes.getAsPath().getSegments(), as))
+                    .build())
+                .build();
+    }
+
+    private static List<Segments> prependAS(final List<Segments> oldSegments, final AsNumber as) {
+        if (oldSegments == null || oldSegments.isEmpty()) {
+            return ImmutableList.of(singleSequence(as));
+        }
+
         /*
          * We need to check the first segment.
          * If it has as-set then new as-sequence with local AS is prepended.
          * If it has as-sequence, we may add local AS when it has less than 255 elements.
          * Otherwise we need to create new as-sequence for local AS.
          */
-        final ArrayList<AsNumber> newAsSequence = new ArrayList<>();
-        newAsSequence.add(new AsNumber(as));
+        final Iterator<Segments> it = oldSegments.iterator();
+        final Segments firstSegment = it.next();
+        final List<AsNumber> firstAsSequence = firstSegment.getAsSequence();
 
-        List<Segments> newSegments = new ArrayList<>();
-        if (oldSegments == null || oldSegments.isEmpty()) {
-            newSegments = new ArrayList<>();
+        final List<Segments> newSegments;
+        if (firstAsSequence != null && firstAsSequence.size() < Values.UNSIGNED_BYTE_MAX_VALUE) {
+            final ArrayList<AsNumber> newAsSequence = new ArrayList<>(firstAsSequence.size() + 1);
+            newAsSequence.add(as);
+            newAsSequence.addAll(firstAsSequence);
+
+            newSegments = new ArrayList<>(oldSegments.size());
             newSegments.add(new SegmentsBuilder().setAsSequence(newAsSequence).build());
         } else {
-            final Segments firstSegment = oldSegments.remove(0);
-            final List<AsNumber> firstAsSequence = firstSegment.getAsSequence();
-            if (firstAsSequence != null && firstAsSequence.size() < Values.UNSIGNED_BYTE_MAX_VALUE) {
-                newAsSequence.addAll(firstAsSequence);
-                newSegments.add(new SegmentsBuilder().setAsSequence(newAsSequence).build());
-            } else {
-                newSegments.add(new SegmentsBuilder().setAsSequence(newAsSequence).build());
-                newSegments.add(firstSegment);
-            }
-            newSegments.addAll(oldSegments);
+            newSegments = new ArrayList<>(oldSegments.size() + 1);
+            newSegments.add(singleSequence(as));
+            newSegments.add(firstSegment);
         }
-        return new AttributesBuilder(attributes).setAsPath(new AsPathBuilder()
-                .setSegments(newSegments).build()).build();
+
+        it.forEachRemaining(newSegments::add);
+        return newSegments;
+    }
+
+    private static Segments singleSequence(final AsNumber as) {
+        return new SegmentsBuilder().setAsSequence(ImmutableList.of(as)).build();
     }
 }