Make ActionInfo immutable 49/41849/1
authorMichael Vorburger <vorburger@redhat.com>
Thu, 14 Jul 2016 17:08:44 +0000 (19:08 +0200)
committerMichael Vorburger <vorburger@redhat.com>
Thu, 14 Jul 2016 18:10:56 +0000 (20:10 +0200)
commit4fd711f266cec95f64130a31d1b7fa748659604b
tree0b1dac1074ae3b0556c2e9b253972bff55d22b37
parent731ab3f3e7586f17122847d8fcd5ce78874882f6
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>
mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/AbstractActionInfoList.java [new file with mode: 0644]
mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionInfo.java
mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/ActionType.java
mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/BucketInfo.java
mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionInfo.java
mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/InstructionType.java
mdsalutil/mdsalutil-api/src/main/java/org/opendaylight/genius/mdsalutil/MDSALUtil.java
mdsalutil/mdsalutil-api/src/test/java/org/opendaylight/genius/mdsalutil/tests/ActionInfoImmutableTest.java [new file with mode: 0644]