Fix Acl.getAccessListEntries() NPE

This reworks interactions with Acl.getAccessListEntries(), so that
it's non-presence is uniformly treated in all cases. This fixes at
least:

java.lang.NullPointerException: null
	at org.opendaylight.netvirt.aclservice.listeners.AclEventListener.remove(AclEventListener.java:110) ~[?:?]
	at org.opendaylight.netvirt.aclservice.listeners.AclEventListener.remove(AclEventListener.java:50) ~[?:?]
	at org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase$DataTreeChangeHandler.run(AsyncDataTreeChangeListenerBase.java:158) ~[274:org.opendaylight.genius.mdsalutil-api:0.7.1]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) ~[?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) ~[?:?]
	at java.lang.Thread.run(Thread.java:748) [?:?]

While also consilading duplicate code.

JIRA: NETVIRT-1636
Change-Id: Ifc33c63aa41c0fb75453deeee2093906c2ad5e95
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 6a1bd2bd02d4612c8f468d179d5cb0dc795f1d84)

diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java
index afeff3f09c9d8bfa8f44002e1d7c54d178edfccf..15cc7eac8bce550ab744de774ffabbca49484e9f 100644 (file)
--- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java
+++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java
@@ -52,7 +52,6 @@ import org.opendaylight.netvirt.aclservice.utils.AclDataUtil;
 import org.opendaylight.netvirt.aclservice.utils.AclServiceOFFlowBuilder;
 import org.opendaylight.netvirt.aclservice.utils.AclServiceUtils;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.Acl;
-import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.acl.AccessListEntries;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.acl.access.list.entries.Ace;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.acl.access.list.entries.ace.Matches;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.access.lists.acl.access.list.entries.ace.matches.ace.type.AceIp;
@@ -386,11 +385,8 @@ public abstract class AbstractAclServiceImpl implements AclServiceListener {
                 LOG.warn("The ACL {} not found in cache", aclUuid.getValue());
                 continue;
             }
-            AccessListEntries accessListEntries = acl.getAccessListEntries();
-            if (accessListEntries != null && accessListEntries.getAce() != null) {
-                for (Ace ace: accessListEntries.getAce()) {
-                    programAceRule(flowEntries, port, aclUuid.getValue(), ace, addOrRemove);
-                }
+            for (Ace ace : AclServiceUtils.aceList(acl)) {
+                programAceRule(flowEntries, port, aclUuid.getValue(), ace, addOrRemove);
             }
         }
         return true;
@@ -410,7 +406,7 @@ public abstract class AbstractAclServiceImpl implements AclServiceListener {
         SecurityRuleAttr aceAttr = AclServiceUtils.getAccessListAttributes(ace);
         if (aceAttr == null) {
             LOG.error("Ace {} of Acl {} is either null or not having SecurityRuleAttr",
-                    ((ace == null) ? null : ace.getRuleName()), aclName);
+                    ace == null ? null : ace.getRuleName(), aclName);
             return;
         }
         if (addOrRemove == NwConstants.ADD_FLOW && aceAttr.isDeleted()) {

diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
index 57ca86af115c374a3033ccd7b88acbefd530de76..6b7ce297ed7306bb77ca3398f56fb7762c6dd726 100644 (file)
--- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
+++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
@@ -21,7 +21,6 @@ import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import org.eclipse.jdt.annotation.NonNull;
-import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.controller.md.sal.binding.api.ClusteredDataTreeChangeListener;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
@@ -107,8 +106,8 @@ public class AclEventListener extends AsyncDataTreeChangeListenerBase<Acl, AclEv
                 this.aclServiceUtils.releaseAclTag(aclName);
             }
             // Handle Rule deletion If SG Remove event is received before SG Rule delete event
-            if (null != acl.getAccessListEntries() && null != acl.getAccessListEntries().getAce()) {
-                List<Ace> aceList = acl.getAccessListEntries().getAce();
+            List<Ace> aceList = AclServiceUtils.aceList(acl);
+            if (!aceList.isEmpty()) {
                 Collection<AclInterface> aclInterfaces =
                         ImmutableSet.copyOf(aclDataUtil.getInterfaceList(new Uuid(aclName)));
                 updateAceRules(aclInterfaces, aclName, aceList, AclServiceManager.Action.REMOVE);
@@ -175,7 +174,7 @@ public class AclEventListener extends AsyncDataTreeChangeListenerBase<Acl, AclEv
      * @param aclName the acl name
      * @param action the action
      */
-    private void updateRemoteAclCache(@Nullable List<Ace> aceList, String aclName, AclServiceManager.Action action) {
+    private void updateRemoteAclCache(@NonNull List<Ace> aceList, String aclName, AclServiceManager.Action action) {
         for (Ace ace : aceList) {
             SecurityRuleAttr aceAttributes = ace.augmentation(SecurityRuleAttr.class);
             if (AclServiceUtils.doesAceHaveRemoteGroupId(aceAttributes)) {
@@ -248,22 +247,17 @@ public class AclEventListener extends AsyncDataTreeChangeListenerBase<Acl, AclEv
         return this;
     }
 
-    @NonNull
-    private List<Ace> getChangedAceList(Acl updatedAcl, Acl currentAcl) {
+    private static @NonNull List<Ace> getChangedAceList(Acl updatedAcl, Acl currentAcl) {
         if (updatedAcl == null) {
             return Collections.emptyList();
         }
-        List<Ace> updatedAceList =
-            updatedAcl.getAccessListEntries() == null || updatedAcl.getAccessListEntries().getAce() == null
-                ? new ArrayList<>()
-                : new ArrayList<>(updatedAcl.getAccessListEntries().getAce());
+        List<Ace> updatedAceList = AclServiceUtils.aceList(updatedAcl);
         if (currentAcl == null) {
             return updatedAceList;
         }
-        List<Ace> currentAceList =
-            currentAcl.getAccessListEntries() == null || currentAcl.getAccessListEntries().getAce() == null
-                ? new ArrayList<>()
-                : new ArrayList<>(currentAcl.getAccessListEntries().getAce());
+
+        List<Ace> currentAceList = AclServiceUtils.aceList(currentAcl);
+        updatedAceList = new ArrayList<>(updatedAceList);
         for (Iterator<Ace> iterator = updatedAceList.iterator(); iterator.hasNext();) {
             Ace ace1 = iterator.next();
             for (Ace ace2 : currentAceList) {

diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
index 2402f25f8e403462b294c8189096723a7f712198..c1e0f7b2c8b55a0b5c6df792d166aa954a8626c8 100644 (file)
--- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
+++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
@@ -33,6 +33,7 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import javax.inject.Inject;
 import javax.inject.Singleton;
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
@@ -961,13 +962,15 @@ public final class AclServiceUtils {
         return flowMatches;
     }
 
-    public static List<Ace> getAceListFromAcl(Acl acl) {
-        if (acl.getAccessListEntries() != null) {
-            List<Ace> aceList = acl.getAccessListEntries().getAce();
-            if (aceList != null && !aceList.isEmpty()
-                    && aceList.get(0).augmentation(SecurityRuleAttr.class) != null) {
-                return aceList;
-            }
+    public static @NonNull List<Ace> aceList(@NonNull Acl acl) {
+        final AccessListEntries ale = acl.getAccessListEntries();
+        return ale == null ? Collections.emptyList() : ale.nonnullAce();
+    }
+
+    public static @NonNull List<Ace> getAceListFromAcl(Acl acl) {
+        List<Ace> aceList = aceList(acl);
+        if (!aceList.isEmpty() && aceList.get(0).augmentation(SecurityRuleAttr.class) != null) {
+            return aceList;
         }
         return Collections.emptyList();
     }
@@ -1024,14 +1027,11 @@ public final class AclServiceUtils {
 
     public static Set<Uuid> getRemoteAclIdsByDirection(Acl acl, Class<? extends DirectionBase> direction) {
         Set<Uuid> remoteAclIds = new HashSet<>();
-        AccessListEntries accessListEntries = acl.getAccessListEntries();
-        if (accessListEntries != null && accessListEntries.getAce() != null) {
-            for (Ace ace : accessListEntries.getAce()) {
-                SecurityRuleAttr aceAttr = AclServiceUtils.getAccessListAttributes(ace);
-                if (aceAttr != null && Objects.equals(aceAttr.getDirection(), direction)
-                        && doesAceHaveRemoteGroupId(aceAttr)) {
-                    remoteAclIds.add(aceAttr.getRemoteGroupId());
-                }
+        for (Ace ace : aceList(acl)) {
+            SecurityRuleAttr aceAttr = AclServiceUtils.getAccessListAttributes(ace);
+            if (aceAttr != null && Objects.equals(aceAttr.getDirection(), direction)
+                    && doesAceHaveRemoteGroupId(aceAttr)) {
+                remoteAclIds.add(aceAttr.getRemoteGroupId());
             }
         }
         return remoteAclIds;