Make ActionInfo immutable

This subtly changes the behaviour of ActionInfo's actionKey. Please
carefully code review to make sure that no code anywhere assumes that
ActionInfo's actionKey gets automagically updated when FlowEntity's
getFlowBuilder() is used.  From a pure Java development point of view
this seems logical, because you created the ActionInfo with a certain
actionKey given to the ActionInfo (or the default 0), and would not
expect it to just change by itself!  (Or is it perhaps simply wrong to
have actionKey in ActionInfo all together, and this numeric index should
only be present in Action?)

Immutable objects are a Good Thing(TM).  They are clear and thread safe
by default.  All YANG gen. objects are immutable, and created by
*Builder, for a very good reason.  We should make all hand-crafted beans
the same (or, ideally, better, just code generate them all as well, use
YANG, or using Google AutoValue; if YANG for simple Beans is somehow not
good; see
https://github.com/google/auto/blob/master/value/userguide/index.md).

This solves a specific problem I ran into when writing tests, where I
found it very un-intutive that what looked like a "getter", the
getFlowBuilder(), would change objects.  The new
ActionInfoImmutableTest illustrates the specific problem I ran into, and
which originally motived me to do this - but immutable beans is a good
idea generally anyway.

This also makes the intention of the code in these classes much clearer
to read (the idea of the API as-is seems to be that always only either
String actionValues or BI bigActionValues is set; never both). Removal
of setActionKey is only consistent then.

Similar to what was already done in
https://git.opendaylight.org/gerrit/#/c/41524/ for InstructionInfo and
(Nx)MatchInfo.

Change-Id: Id0619b1ab6013bee53f392603acb85f9c4f18f7c
Signed-off-by: Michael Vorburger <vorburger@redhat.com>

diff --git a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/AbstractActionInfoList.java b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/AbstractActionInfoList.java
new file mode 100644 (file)
index 0000000..956cc31
--- /dev/null
+++ b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/AbstractActionInfoList.java
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2016 Red Hat and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.genius.mdsalutil;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.list.Action;
+
+/**
+ * List<ActionInfo> with method to build List<Action> from it.
+ *
+ * @author Michael Vorburger
+ */
+public abstract class AbstractActionInfoList {
+
+    private final List<ActionInfo> actionInfos;
+
+    protected AbstractActionInfoList(List<ActionInfo> actionInfos) {
+        super();
+        this.actionInfos = actionInfos;
+    }
+
+    public List<ActionInfo> getActionInfos() {
+        return actionInfos;
+    }
+
+    public List<Action> buildActions() {
+        if (actionInfos == null)
+            return Collections.emptyList();
+        int newActionKey = 0;
+        List<Action> actions = new ArrayList<>(actionInfos.size());
+        for (ActionInfo actionInfo: actionInfos) {
+            ActionType actionType = actionInfo.getActionType();
+            actions.add(actionType.buildAction(newActionKey++, actionInfo));
+        }
+        return actions;
+    }
+}

diff --git a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionInfo.java b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionInfo.java
index 9ab13e237d82287f5a60fac7cfe0921071ee5104..d627f4ec7bb37580ac9949bb9f8ec31baa8d0762 100644 (file)
--- a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionInfo.java
+++ b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionInfo.java
@@ -19,44 +19,44 @@ public class ActionInfo implements Serializable {
     private static final long serialVersionUID = 1L;
 
     private final ActionType m_actionType;
-
-    private String[] m_asActionValues = null;
-    private BigInteger[] m_aBigIntValues;
-    private int m_actionKey = 0;
+    private final String[] m_asActionValues;
+    private final BigInteger[] m_aBigIntValues;
+    private final int m_actionKey;
 
     public ActionInfo(ActionInfo action) {
         super();
         m_actionType = action.m_actionType;
         m_actionKey = action.m_actionKey;
         m_asActionValues = Arrays.copyOf(action.m_asActionValues, action.m_asActionValues.length);
+        m_aBigIntValues = null;
     }
 
     public ActionInfo(ActionType actionType, String[] asActionValues) {
         m_actionType = actionType;
         m_actionKey = 0;
         m_asActionValues = asActionValues;
+        m_aBigIntValues = null;
     }
 
     public ActionInfo(ActionType actionType, String[] asActionValues, int actionKey) {
         m_actionType = actionType;
         m_actionKey = actionKey;
         m_asActionValues = asActionValues;
+        m_aBigIntValues = null;
     }
 
     public ActionInfo(ActionType actionType, BigInteger[] aBigIntValues) {
         m_actionType = actionType;
         m_actionKey = 0;
         m_aBigIntValues = aBigIntValues;
+        m_asActionValues = null;
     }
 
     public ActionInfo(ActionType actionType, BigInteger[] aBigIntValues, int actionKey) {
         m_actionType = actionType;
         m_actionKey = actionKey;
         m_aBigIntValues = aBigIntValues;
-    }
-
-    public void setActionKey(int key) {
-        m_actionKey = key;
+        m_asActionValues = null;
     }
 
     public int getActionKey() {
@@ -64,7 +64,7 @@ public class ActionInfo implements Serializable {
     }
 
     public Action buildAction() {
-        return m_actionType.buildAction(this);
+        return m_actionType.buildAction(getActionKey(), this);
     }
 
     public ActionType getActionType() {

diff --git a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionType.java b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionType.java
index a94058fe29462a0e5121e2a91edb0ab336e38e24..44f5cca7e030da775511c73e9fc408d97ff558fa 100644 (file)
--- a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionType.java
+++ b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionType.java
@@ -8,8 +8,6 @@
 package org.opendaylight.genius.mdsalutil;
 
 import java.math.BigInteger;
-import java.net.InetAddress;
-
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Uri;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.MacAddress;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.GroupActionCaseBuilder;
@@ -54,26 +52,25 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.acti
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.drop.action._case.DropActionBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.add.group.input.buckets.bucket.action.action.NxActionResubmitRpcAddGroupCaseBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nodes.node.table.flow.instructions.instruction.instruction.apply.actions._case.apply.actions.action.action.NxActionConntrackNodesNodeTableFlowApplyActionsCaseBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nx.action.conntrack.grouping.NxConntrack;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nx.action.conntrack.grouping.NxConntrackBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nx.action.resubmit.grouping.NxResubmitBuilder;
 
 public enum ActionType {
     group {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             long groupId = Long.parseLong(actionInfo.getActionValues()[0]);
 
             return new ActionBuilder().setAction(
                             new GroupActionCaseBuilder().setGroupAction(
                                     new GroupActionBuilder().setGroupId(groupId).build()).build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     output {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             String port = actionValues[0];
             int maxLength = 0;
@@ -86,88 +83,88 @@ public enum ActionType {
                     new OutputActionCaseBuilder().setOutputAction(
                             new OutputActionBuilder().setMaxLength(Integer.valueOf(maxLength))
                                             .setOutputNodeConnector(new Uri(port)).build()).build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     pop_mpls {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             return new ActionBuilder().setAction(
                     new PopMplsActionCaseBuilder().setPopMplsAction(
                             new PopMplsActionBuilder().setEthernetType(
                                     Integer.valueOf(NwConstants.ETHTYPE_IPV4)).build()).build())
 
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     pop_pbb {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             return new ActionBuilder()
                     .setAction(new PopPbbActionCaseBuilder().setPopPbbAction(new PopPbbActionBuilder().build()).build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     pop_vlan {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             return new ActionBuilder().setAction(
                     new PopVlanActionCaseBuilder().setPopVlanAction(new PopVlanActionBuilder().build()).build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     push_mpls {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             return new ActionBuilder().setAction(new PushMplsActionCaseBuilder().setPushMplsAction(
                                     new PushMplsActionBuilder().setEthernetType(
                                             Integer.valueOf(NwConstants.ETHTYPE_MPLS_UC)).build()).build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     push_pbb {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             return new ActionBuilder().setAction(
                     new PushPbbActionCaseBuilder().setPushPbbAction(
                                     new PushPbbActionBuilder()
                                             .setEthernetType(Integer.valueOf(NwConstants.ETHTYPE_PBB)).build()).build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     push_vlan {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             return new ActionBuilder().setAction(
                     new PushVlanActionCaseBuilder().setPushVlanAction(
                                     new PushVlanActionBuilder().setEthernetType(
                                             Integer.valueOf(NwConstants.ETHTYPE_802_1Q)).build()).build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     set_field_mpls_label {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             long label = Long.valueOf(actionValues[0]);
 
             return new ActionBuilder().setAction(
                     new SetFieldCaseBuilder().setSetField(new SetFieldBuilder().setProtocolMatchFields(
                                             new ProtocolMatchFieldsBuilder().setMplsLabel(label).build()).build())
-                                    .build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                                    .build()).setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     set_field_pbb_isid {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             long label = Long.valueOf(actionValues[0]);
 
@@ -176,13 +173,13 @@ public enum ActionType {
                             new SetFieldBuilder().setProtocolMatchFields(
                                             new ProtocolMatchFieldsBuilder().setPbb(
                                                     new PbbBuilder().setPbbIsid(label).build()).build()).build())
-                                    .build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                                    .build()).setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     set_field_vlan_vid {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             int vlanId = Integer.valueOf(actionValues[0]);
 
@@ -192,13 +189,13 @@ public enum ActionType {
                                     new VlanMatchBuilder().setVlanId(
                                                     new VlanIdBuilder().setVlanId(new VlanId(vlanId))
                                                             .setVlanIdPresent(true).build()).build()).build()).build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
         }
     },
 
     set_field_tunnel_id {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             BigInteger [] actionValues = actionInfo.getBigActionValues();
             if (actionValues.length == 2) {
                 return new ActionBuilder().setAction(
@@ -207,7 +204,7 @@ public enum ActionType {
                             .setTunnel(new TunnelBuilder().setTunnelId(actionValues[0])
                                            .setTunnelMask(actionValues[1]).build()).build())
                         .build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
             } else {
                 return new ActionBuilder().setAction(
                     new SetFieldCaseBuilder().setSetField(
@@ -215,7 +212,7 @@ public enum ActionType {
                             .setTunnel(new TunnelBuilder().setTunnelId(actionValues[0])
                                            .build()).build())
                         .build())
-                    .setKey(new ActionKey(actionInfo.getActionKey())).build();
+                    .setKey(new ActionKey(newActionKey)).build();
             }
 
         }
@@ -225,7 +222,7 @@ public enum ActionType {
     set_field_eth_dest {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             MacAddress mac = new MacAddress(actionValues[0]);
 
@@ -234,7 +231,7 @@ public enum ActionType {
                             new SetFieldBuilder().setEthernetMatch(
                                     new EthernetMatchBuilder().setEthernetDestination(
                                                     new EthernetDestinationBuilder().setAddress(mac).build()).build())
-                                            .build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                                            .build()).build()).setKey(new ActionKey(newActionKey)).build();
 
         }
 
@@ -243,26 +240,26 @@ public enum ActionType {
     set_udp_protocol {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             return new ActionBuilder().setAction(
                     new SetFieldCaseBuilder().setSetField(
                             new SetFieldBuilder().setIpMatch(
                                     new IpMatchBuilder().setIpProtocol((short) 17).build()).
-                                    build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                                    build()).build()).setKey(new ActionKey(newActionKey)).build();
 
         }
 
     },
     punt_to_controller {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             ActionBuilder ab = new ActionBuilder();
             OutputActionBuilder output = new OutputActionBuilder();
             output.setMaxLength(0xffff);
             Uri value = new Uri(OutputPortValues.CONTROLLER.toString());
             output.setOutputNodeConnector(value);
             ab.setAction(new OutputActionCaseBuilder().setOutputAction(output.build()).build());
-            ab.setKey(new ActionKey(actionInfo.getActionKey()));
+            ab.setKey(new ActionKey(newActionKey));
             return ab.build();
         }
 
@@ -270,7 +267,7 @@ public enum ActionType {
     set_udp_destination_port {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             Integer portNumber = new Integer(actionValues[0]);
 
@@ -279,7 +276,7 @@ public enum ActionType {
                             new SetFieldBuilder().setLayer4Match(
                                     new UdpMatchBuilder().setUdpDestinationPort(
                                             new PortNumber(portNumber)).build())
-                            .build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                            .build()).build()).setKey(new ActionKey(newActionKey)).build();
 
         }
 
@@ -287,7 +284,7 @@ public enum ActionType {
     set_udp_source_port {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             Integer portNumber = new Integer(actionValues[0]);
 
@@ -296,7 +293,7 @@ public enum ActionType {
                             new SetFieldBuilder().setLayer4Match(
                                     new UdpMatchBuilder().setUdpSourcePort(
                                             new PortNumber(portNumber)).build())
-                            .build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                            .build()).build()).setKey(new ActionKey(newActionKey)).build();
 
         }
 
@@ -304,7 +301,7 @@ public enum ActionType {
     set_tcp_destination_port {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             Integer portNumber = new Integer(actionValues[0]);
 
@@ -313,7 +310,7 @@ public enum ActionType {
                             new SetFieldBuilder().setLayer4Match(
                                     new TcpMatchBuilder().setTcpDestinationPort(
                                             new PortNumber(portNumber)).build())
-                            .build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                            .build()).build()).setKey(new ActionKey(newActionKey)).build();
 
         }
 
@@ -321,7 +318,7 @@ public enum ActionType {
     set_tcp_source_port {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             Integer portNumber = new Integer(actionValues[0]);
 
@@ -330,14 +327,14 @@ public enum ActionType {
                             new SetFieldBuilder().setLayer4Match(
                                     new TcpMatchBuilder().setTcpSourcePort(
                                             new PortNumber(portNumber)).build())
-                            .build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                            .build()).build()).setKey(new ActionKey(newActionKey)).build();
 
         }
 
     },
     set_source_ip {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             String sourceIp = actionValues[0];
             String sourceMask = (actionValues.length > 1) ? actionValues[1] : "32";
@@ -347,7 +344,7 @@ public enum ActionType {
                                                 new SetFieldBuilder().setLayer3Match(
                                                         new Ipv4MatchBuilder().setIpv4Source(
                                                                 new Ipv4Prefix(source)).build()).
-                                                                build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                                                                build()).build()).setKey(new ActionKey(newActionKey)).build();
 
 
         }
@@ -356,7 +353,7 @@ public enum ActionType {
     set_destination_ip {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             String destIp = actionValues[0];
             String destMask = (actionValues.length > 1) ? actionValues[1] : "32";
@@ -365,8 +362,8 @@ public enum ActionType {
                     new SetFieldCaseBuilder().setSetField(
                             new SetFieldBuilder().setLayer3Match(
                                     new Ipv4MatchBuilder().setIpv4Destination(
-                                            new Ipv4Prefix(destination)).build()).
-                                            build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                                            new Ipv4Prefix(destination)).build())
+                                            .build()).build()).setKey(new ActionKey(newActionKey)).build();
 
         }
 
@@ -374,7 +371,7 @@ public enum ActionType {
     set_field_eth_src {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             MacAddress mac = new MacAddress(actionValues[0]);
 
@@ -383,7 +380,7 @@ public enum ActionType {
                             new SetFieldBuilder().setEthernetMatch(
                                     new EthernetMatchBuilder().setEthernetSource(
                                                     new EthernetSourceBuilder().setAddress(mac).build()).build())
-                                            .build()).build()).setKey(new ActionKey(actionInfo.getActionKey())).build();
+                                            .build()).build()).setKey(new ActionKey(newActionKey)).build();
 
         }
     },
@@ -391,12 +388,12 @@ public enum ActionType {
     drop_action {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             DropActionBuilder dab = new DropActionBuilder();
             DropAction dropAction = dab.build();
             ActionBuilder ab = new ActionBuilder();
             ab.setAction(new DropActionCaseBuilder().setDropAction(dropAction).build());
-            ab.setKey(new ActionKey(actionInfo.getActionKey())).build();
+            ab.setKey(new ActionKey(newActionKey)).build();
             return ab.build();
         }
     },
@@ -404,13 +401,13 @@ public enum ActionType {
     nx_resubmit {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             NxResubmitBuilder nxarsb = new NxResubmitBuilder();
             nxarsb.setTable(Short.parseShort(actionValues[0]));
             ActionBuilder ab = new ActionBuilder();
             ab.setAction(new NxActionResubmitRpcAddGroupCaseBuilder().setNxResubmit(nxarsb.build()).build());
-            ab.setKey(new ActionKey(actionInfo.getActionKey()));
+            ab.setKey(new ActionKey(newActionKey));
             return ab.build();
         }
     },
@@ -418,14 +415,14 @@ public enum ActionType {
     goto_table {
 
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
-            ActionBuilder ab = new ActionBuilder();
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
+            new ActionBuilder();
             return null;
         }
     },
     nx_conntrack {
         @Override
-        public Action buildAction(ActionInfo actionInfo) {
+        public Action buildAction(int newActionKey, ActionInfo actionInfo) {
             String[] actionValues = actionInfo.getActionValues();
             Integer flags = new Integer(actionValues[0]);
             Long zoneSrc = new Long(actionValues[1]);
@@ -439,7 +436,7 @@ public enum ActionType {
             ActionBuilder ab = new ActionBuilder();
             ab.setAction(new NxActionConntrackNodesNodeTableFlowApplyActionsCaseBuilder()
                 .setNxConntrack(ctb.build()).build());
-            ab.setKey(new ActionKey(actionInfo.getActionKey()));
+            ab.setKey(new ActionKey(newActionKey));
             return ab.build();
 
         }
@@ -447,5 +444,6 @@ public enum ActionType {
     };
 
     private static final int RADIX_HEX = 16;
-    public abstract Action buildAction(ActionInfo actionInfo);
+
+    public abstract Action buildAction(int newActionKey, ActionInfo actionInfo);
 }

diff --git a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/BucketInfo.java b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/BucketInfo.java
index 465cf6cdbf1be3b4f76774aa7325bdbe5fdc3949..ed6a1bc0a1e53ce8a665f04a5b0b59ef8aa3b8f6 100644 (file)
--- a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/BucketInfo.java
+++ b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/BucketInfo.java
@@ -13,38 +13,25 @@ import java.util.List;
 
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.list.Action;
 
-public class BucketInfo implements Serializable {
+public class BucketInfo extends AbstractActionInfoList implements Serializable {
 
     private static final long serialVersionUID = 1L;
 
-    private final List<ActionInfo> m_listActionInfo;
-
     private Integer weight = 0;
     private Long watchPort = 0xffffffffL;
     private Long watchGroup = 0xffffffffL;
 
     public BucketInfo(List<ActionInfo> listActions) {
-        m_listActionInfo = listActions;
+        super(listActions);
     }
 
-    public BucketInfo(List<ActionInfo> m_listActionInfo, Integer weight, Long watchPort, Long watchGroup) {
-        super();
-        this.m_listActionInfo = m_listActionInfo;
+    public BucketInfo(List<ActionInfo> actionInfos, Integer weight, Long watchPort, Long watchGroup) {
+        super(actionInfos);
         this.weight = weight;
         this.watchPort = watchPort;
         this.watchGroup = watchGroup;
     }
 
-    public void buildAndAddActions(List<Action> listActionOut) {
-        int key = 0;
-        if (m_listActionInfo != null) {
-            for (ActionInfo actionInfo : m_listActionInfo) {
-                actionInfo.setActionKey(key++);
-                listActionOut.add(actionInfo.buildAction());
-            }
-        }
-    }
-
     public void setWeight(Integer bucketWeight) {
         weight = bucketWeight;
     }
@@ -53,10 +40,6 @@ public class BucketInfo implements Serializable {
         return weight;
     }
 
-    public List<ActionInfo> getActionInfoList() {
-        return m_listActionInfo;
-    }
-
     public Long getWatchPort() {
         return watchPort;
     }
@@ -75,7 +58,7 @@ public class BucketInfo implements Serializable {
 
     @Override
     public String toString() {
-        return MoreObjects.toStringHelper(this).add("actionInfoList", m_listActionInfo).add("weight", weight)
+        return MoreObjects.toStringHelper(this).add("actionInfos", getActionInfos()).add("weight", weight)
                 .add("watchPort", watchPort).add("watchGroup", watchGroup).toString();
     }
 }

diff --git a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionInfo.java b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionInfo.java
index 6e085a2a8f75338587c4a68706e152d378d6c9ce..9a9f9fbe5ffa3dd5e3053d655e88ed8d30c39727 100644 (file)
--- a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionInfo.java
+++ b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionInfo.java
@@ -15,42 +15,41 @@ import java.util.List;
 
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.instruction.list.Instruction;
 
-public class InstructionInfo implements Serializable {
+public class InstructionInfo extends AbstractActionInfoList implements Serializable {
 
     private static final long serialVersionUID = 1L;
 
     private final InstructionType m_instructionType;
     private final long[] m_alInstructionValues;
     private final BigInteger[] m_alBigInstructionValues;
-    private final List<ActionInfo> m_actionInfos;
 
     // This constructor should be used incase of clearAction
     public InstructionInfo(InstructionType instructionType) {
+        super(null);
         m_instructionType = instructionType;
         m_alInstructionValues = null;
         m_alBigInstructionValues = null;
-        m_actionInfos = null;
     }
 
     public InstructionInfo(InstructionType instructionType, long[] instructionValues) {
+        super(null);
         m_instructionType = instructionType;
         m_alInstructionValues = instructionValues;
         m_alBigInstructionValues = null;
-        m_actionInfos = null;
     }
 
     public InstructionInfo(InstructionType instructionType, BigInteger[] instructionValues) {
+        super(null);
         m_instructionType = instructionType;
         m_alInstructionValues = null;
         m_alBigInstructionValues = instructionValues;
-        m_actionInfos = null;
     }
 
     public InstructionInfo(InstructionType instructionType, List<ActionInfo> actionInfos) {
+        super(actionInfos);
         m_instructionType = instructionType;
         m_alInstructionValues = null;
         m_alBigInstructionValues = null;
-        m_actionInfos = actionInfos;
     }
 
     public Instruction buildInstruction(int instructionKey) {
@@ -69,15 +68,11 @@ public class InstructionInfo implements Serializable {
         return m_alBigInstructionValues;
     }
 
-    public List<ActionInfo> getActionInfos() {
-        return m_actionInfos;
-    }
-
     @Override
     public String toString() {
         return MoreObjects.toStringHelper(this).omitNullValues().add("instructionType", m_instructionType)
                 .add("instructionValues", Arrays.toString(m_alInstructionValues))
                 .add("bigInstructionValues", Arrays.deepToString(m_alBigInstructionValues))
-                .add("actionInfos", m_actionInfos).toString();
+                .add("actionInfos", getActionInfos()).toString();
     }
 }

diff --git a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionType.java b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionType.java
index f5b4cdca78193e8824019741dc7033b792aac472..cccce5a37eb091a8855bed91b0d69290030e5b69 100644 (file)
--- a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionType.java
+++ b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionType.java
@@ -8,7 +8,6 @@
 package org.opendaylight.genius.mdsalutil;
 
 import java.math.BigInteger;
-import java.util.ArrayList;
 import java.util.List;
 
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.list.Action;
@@ -34,14 +33,7 @@ public enum InstructionType {
     apply_actions {
         @Override
         public Instruction buildInstruction(InstructionInfo instructionInfo, int instructionKey) {
-            List<ActionInfo> mkActions = instructionInfo.getActionInfos();
-            List<Action> listAction = new ArrayList <Action> ();
-            int actionKey = 0 ;
-            for(ActionInfo mkAction: mkActions) {
-                ActionType actionType = mkAction.getActionType();
-                mkAction.setActionKey(actionKey++);
-                listAction.add(actionType.buildAction(mkAction));
-            }
+            List<Action> listAction = instructionInfo.buildActions();
             ApplyActions applyActions = new ApplyActionsBuilder().setAction(listAction).build();
             ApplyActionsCase applyActionsCase = new ApplyActionsCaseBuilder().setApplyActions(applyActions).build();
             InstructionBuilder instructionBuilder = new InstructionBuilder();
@@ -69,14 +61,7 @@ public enum InstructionType {
     write_actions {
         @Override
         public Instruction buildInstruction(InstructionInfo instructionInfo, int instructionKey) {
-            List<ActionInfo> mkActions = instructionInfo.getActionInfos();
-            List<Action> listAction = new ArrayList <Action> ();
-            int actionKey = 0 ;
-            for(ActionInfo mkAction: mkActions) {
-                ActionType actionType = mkAction.getActionType();
-                mkAction.setActionKey(actionKey++);
-                listAction.add(actionType.buildAction(mkAction));
-            }
+            List<Action> listAction = instructionInfo.buildActions();
             WriteActions writeActions = new WriteActionsBuilder().setAction(listAction).build();
             WriteActionsCase writeActionsCase = new WriteActionsCaseBuilder().setWriteActions(writeActions).build();
             InstructionBuilder instructionBuilder = new InstructionBuilder();

diff --git a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/MDSALUtil.java b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/MDSALUtil.java
index 05efdd0644112c112fda4b7ac8a8d28ee935c749..2536a27af2dd3a221da0baa714ed8468c7c64751 100644 (file)
--- a/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/MDSALUtil.java
+++ b/mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/MDSALUtil.java
@@ -210,7 +210,7 @@ public class MDSALUtil {
     }
 
     public static List<Action> buildActions(List<ActionInfo> actions) {
-        List<Action> actionsList = new ArrayList<Action>();
+        List<Action> actionsList = new ArrayList<>();
         for (ActionInfo actionInfo : actions) {
             actionsList.add(actionInfo.buildAction());
         }
@@ -243,14 +243,11 @@ public class MDSALUtil {
         long i = 0;
         if (listBucketInfo != null) {
             BucketsBuilder bucketsBuilder = new BucketsBuilder();
-            List<Bucket> bucketList = new ArrayList<Bucket>();
+            List<Bucket> bucketList = new ArrayList<>();
 
             for (BucketInfo bucketInfo : listBucketInfo) {
                 BucketBuilder bucketBuilder = new BucketBuilder();
-                List<Action> actionsList = new ArrayList<Action>();
-
-                bucketInfo.buildAndAddActions(actionsList);
-                bucketBuilder.setAction(actionsList);
+                bucketBuilder.setAction(bucketInfo.buildActions());
                 bucketBuilder.setWeight(bucketInfo.getWeight());
                 bucketBuilder.setBucketId(new BucketId(i++));
                 bucketBuilder.setWeight(bucketInfo.getWeight()).setWatchPort(bucketInfo.getWatchPort())
@@ -267,7 +264,7 @@ public class MDSALUtil {
 
     public static Instructions buildInstructions(List<InstructionInfo> listInstructionInfo) {
         if (listInstructionInfo != null) {
-            List<Instruction> instructions = new ArrayList<Instruction>();
+            List<Instruction> instructions = new ArrayList<>();
             int instructionKey = 0;
 
             for (InstructionInfo instructionInfo : listInstructionInfo) {
@@ -284,7 +281,7 @@ public class MDSALUtil {
     public static Match buildMatches(List<? extends MatchInfoBase> listMatchInfoBase) {
         if (listMatchInfoBase != null) {
             MatchBuilder matchBuilder = new MatchBuilder();
-            Map<Class<?>, Object> mapMatchBuilder = new HashMap<Class<?>, Object>();
+            Map<Class<?>, Object> mapMatchBuilder = new HashMap<>();
 
             for (MatchInfoBase MatchInfoBase : listMatchInfoBase) {
                 MatchInfoBase.createInnerMatchBuilder(mapMatchBuilder);
@@ -383,7 +380,7 @@ public class MDSALUtil {
         Action popVlanAction = new ActionBuilder().setAction(
                 new PopVlanActionCaseBuilder().setPopVlanAction(new PopVlanActionBuilder().build()).build())
                 .setKey(new ActionKey(actionKey)).build();
-        List<Action> listAction = new ArrayList<Action> ();
+        List<Action> listAction = new ArrayList<> ();
         listAction.add(popVlanAction);
         return buildApplyActionsInstruction(listAction, instructionKey);
     }
@@ -407,8 +404,8 @@ public class MDSALUtil {
     }
 
     public static List<Instruction> buildInstructionsDrop(int instructionKey) {
-        List<Instruction> mkInstructions = new ArrayList<Instruction>();
-        List <Action> actionsInfos = new ArrayList <Action> ();
+        List<Instruction> mkInstructions = new ArrayList<>();
+        List <Action> actionsInfos = new ArrayList <> ();
         actionsInfos.add(new ActionInfo(ActionType.drop_action, new String[]{}).buildAction());
         mkInstructions.add(getWriteActionsInstruction(actionsInfos, instructionKey));
         return mkInstructions;

diff --git a/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/tests/ActionInfoImmutableTest.java b/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/tests/ActionInfoImmutableTest.java
new file mode 100644 (file)
index 0000000..26f8b94
--- /dev/null
+++ b/mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/tests/ActionInfoImmutableTest.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2016 Red Hat and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.genius.mdsalutil.tests;
+
+import static org.junit.Assert.assertEquals;
+
+import java.math.BigInteger;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.Test;
+import org.opendaylight.genius.mdsalutil.ActionInfo;
+import org.opendaylight.genius.mdsalutil.ActionType;
+import org.opendaylight.genius.mdsalutil.FlowEntity;
+import org.opendaylight.genius.mdsalutil.InstructionInfo;
+import org.opendaylight.genius.mdsalutil.InstructionType;
+
+public class ActionInfoImmutableTest {
+
+    @Test
+    public void actionInfoActionKeyDoesNotMagicallyChangeOnFlowEntityGetFlowBuilder() {
+        FlowEntity flowEntity = new FlowEntity(BigInteger.valueOf(123L));
+        flowEntity.setFlowId("TEST");
+        flowEntity.setCookie(BigInteger.valueOf(110100480L));
+        ActionInfo actionInfo = new ActionInfo(ActionType.nx_conntrack, new String[] { "1", "0", "0", "255" }, 27);
+        List<ActionInfo> actionInfos = new ArrayList<>();
+        actionInfos.add(actionInfo);
+        flowEntity.getInstructionInfoList().add(new InstructionInfo(InstructionType.apply_actions, actionInfos));
+        assertEquals(27, flowEntity.getInstructionInfoList().get(0).getActionInfos().get(0).getActionKey());
+
+        flowEntity.getFlowBuilder();
+        assertEquals(27, flowEntity.getInstructionInfoList().get(0).getActionInfos().get(0).getActionKey());
+        flowEntity.getFlowBuilder();
+        assertEquals(27, flowEntity.getInstructionInfoList().get(0).getActionInfos().get(0).getActionKey());
+    }
+
+}