Refactor MatchExtensionHelper

This is a rather simple utility class. Since we are cleaning up
deprecation warnings, we might as well completely refactor it.

This results in much more expressive code as well as performance
wins, as we propagate invariants as early as possible -- untangling
the Optional.ofNullable().map().orElse(new ...) mess.

Change-Id: Ia92d36626c6c449558a3cd91f4f52b708b0d9645
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/extension/MatchExtensionHelper.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/extension/MatchExtensionHelper.java
index d04f9900cb6bf514d798b5fe7bbce447a11f9a7f..5f5e34e43aab88c626d9077e3bd4b5d7a5c9d899 100644 (file)
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/extension/MatchExtensionHelper.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/extension/MatchExtensionHelper.java
@@ -7,23 +7,16 @@
  */
 package org.opendaylight.openflowplugin.openflow.md.core.extension;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.List;
-import java.util.Optional;
 import org.opendaylight.openflowjava.protocol.api.keys.MatchEntrySerializerKey;
 import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion;
 import org.opendaylight.openflowplugin.extension.api.AugmentTuple;
-import org.opendaylight.openflowplugin.extension.api.ConvertorFromOFJava;
-import org.opendaylight.openflowplugin.extension.api.ExtensionAugment;
 import org.opendaylight.openflowplugin.extension.api.path.MatchPath;
 import org.opendaylight.openflowplugin.openflow.md.core.session.OFSessionUtil;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.flow.MatchBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.augments.rev150225.oxm.container.match.entry.value.ExperimenterIdCase;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.oxm.rev150225.ExperimenterClass;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.oxm.rev150225.MatchField;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.oxm.rev150225.OxmClassBase;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.oxm.rev150225.match.entries.grouping.MatchEntry;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.GeneralAugMatchNodesNodeTableFlow;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.GeneralAugMatchNodesNodeTableFlowBuilder;
@@ -37,21 +30,21 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.ge
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.GeneralAugMatchPacketInMessageBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.GeneralAugMatchRpcOutputFlowStats;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.GeneralAugMatchRpcOutputFlowStatsBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.general.extension.grouping.Extension;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.general.extension.grouping.ExtensionBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.general.extension.list.grouping.ExtensionList;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.general.extension.list.grouping.ExtensionListBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.general.extension.list.grouping.ExtensionListKey;
 import org.opendaylight.yangtools.yang.binding.Augmentable;
-import org.opendaylight.yangtools.yang.binding.Augmentation;
+import org.opendaylight.yangtools.yang.binding.util.BindingMap;
+import org.opendaylight.yangtools.yang.binding.util.BindingMap.Builder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public final class MatchExtensionHelper {
-
     private static final Logger LOG = LoggerFactory.getLogger(MatchExtensionHelper.class);
 
     private MatchExtensionHelper() {
-        throw new IllegalAccessError("singleton enforcement");
+        // Hidden on purpose
     }
 
     /**
@@ -66,15 +59,8 @@ public final class MatchExtensionHelper {
             final MatchEntry matchEntry,
             final MatchBuilder matchBuilder,
             final MatchPath matchPath) {
-
         final ExtensionListBuilder extBuilder = processExtension(matchEntry, ofVersion, matchPath);
-
-        // TODO: remove the Optional.ofNullable() here, which will result more performant and easier to understand code
-        final GeneralAugMatchNodesNodeTableFlowBuilder builder = Optional
-                .ofNullable(matchBuilder.augmentation(GeneralAugMatchNodesNodeTableFlow.class))
-                .map(GeneralAugMatchNodesNodeTableFlowBuilder::new)
-                .orElse(new GeneralAugMatchNodesNodeTableFlowBuilder().setExtensionList(new HashMap<>()));
-
+        final GeneralAugMatchNodesNodeTableFlowBuilder builder = flowBuilderFor(matchBuilder);
         if (extBuilder != null) {
             final ExtensionList ext = extBuilder.build();
             builder.getExtensionList().put(ext.key(), ext);
@@ -86,6 +72,15 @@ public final class MatchExtensionHelper {
         matchBuilder.addAugmentation(builder.build());
     }
 
+    private static GeneralAugMatchNodesNodeTableFlowBuilder flowBuilderFor(final MatchBuilder matchBuilder) {
+        final var aug = matchBuilder.augmentation(GeneralAugMatchNodesNodeTableFlow.class);
+        return aug != null
+            // Reuse existing augmentation
+            ? new GeneralAugMatchNodesNodeTableFlowBuilder(aug)
+                // Allocate space for extension
+                : new GeneralAugMatchNodesNodeTableFlowBuilder().setExtensionList(new HashMap<>());
+    }
+
     /**
      * Processes all extensions.
      *
@@ -98,66 +93,44 @@ public final class MatchExtensionHelper {
     @SuppressWarnings("unchecked")
     public static <E extends Augmentable<E>> AugmentTuple<E> processAllExtensions(
             final Collection<MatchEntry> matchEntries, final OpenflowVersion ofVersion, final MatchPath matchPath) {
-        List<ExtensionList> extensionsList = new ArrayList<>();
-
-        if (matchEntries != null) {
-            for (MatchEntry matchEntry : matchEntries) {
-                ExtensionListBuilder extensionListBld = processExtension(matchEntry, ofVersion.getVersion(), matchPath);
-                if (extensionListBld == null) {
-                    continue;
-                }
+        if (matchEntries == null) {
+            return null;
+        }
 
-                extensionsList.add(extensionListBld.build());
+        final Builder<ExtensionListKey, ExtensionList> extensions = BindingMap.orderedBuilder(matchEntries.size());
+        for (MatchEntry matchEntry : matchEntries) {
+            final var extensionListBld = processExtension(matchEntry, ofVersion.getVersion(), matchPath);
+            if (extensionListBld != null) {
+                extensions.add(extensionListBld.build());
             }
         }
 
-        AugmentTuple<E> augmentTuple = null;
-        if (!extensionsList.isEmpty()) {
-            switch (matchPath) {
-                case FLOWS_STATISTICS_UPDATE_MATCH:
-                    GeneralAugMatchNotifUpdateFlowStatsBuilder generalExtMatchAugBld1 =
-                            new GeneralAugMatchNotifUpdateFlowStatsBuilder();
-                    generalExtMatchAugBld1.setExtensionList(extensionsList);
-                    augmentTuple = (AugmentTuple<E>)
-                            new AugmentTuple<>(
-                                    GeneralAugMatchNotifUpdateFlowStats.class, generalExtMatchAugBld1.build());
-                    break;
-                case PACKET_RECEIVED_MATCH:
-                    GeneralAugMatchNotifPacketInBuilder generalExtMatchAugBld2 =
-                            new GeneralAugMatchNotifPacketInBuilder();
-                    generalExtMatchAugBld2.setExtensionList(extensionsList);
-                    augmentTuple = (AugmentTuple<E>)
-                            new AugmentTuple<>(GeneralAugMatchNotifPacketIn.class, generalExtMatchAugBld2.build());
-                    break;
-                case PACKET_IN_MESSAGE_MATCH:
-                    GeneralAugMatchPacketInMessageBuilder generalExtMatchAugBld5 =
-                            new GeneralAugMatchPacketInMessageBuilder();
-                    generalExtMatchAugBld5.setExtensionList(extensionsList);
-                    augmentTuple = (AugmentTuple<E>)
-                            new AugmentTuple<>(GeneralAugMatchPacketInMessage.class, generalExtMatchAugBld5.build());
-                    break;
-                case SWITCH_FLOW_REMOVED_MATCH:
-                    GeneralAugMatchNotifSwitchFlowRemovedBuilder generalExtMatchAugBld3 =
-                            new GeneralAugMatchNotifSwitchFlowRemovedBuilder();
-                    generalExtMatchAugBld3.setExtensionList(extensionsList);
-                    augmentTuple = (AugmentTuple<E>)
-                            new AugmentTuple<>(
-                                    GeneralAugMatchNotifSwitchFlowRemoved.class, generalExtMatchAugBld3.build());
-                    break;
-                case FLOWS_STATISTICS_RPC_MATCH:
-                    GeneralAugMatchRpcOutputFlowStatsBuilder generalExtMatchAugBld4 =
-                           new GeneralAugMatchRpcOutputFlowStatsBuilder();
-                    generalExtMatchAugBld4.setExtensionList(extensionsList);
-                    augmentTuple = (AugmentTuple<E>)
-                            new AugmentTuple<>(
-                                    GeneralAugMatchRpcOutputFlowStats.class, generalExtMatchAugBld4.build());
-                    break;
-                default:
-                    LOG.warn("matchPath not supported: {}", matchPath);
-            }
+        final var extensionsList = extensions.build();
+        if (extensionsList.isEmpty()) {
+            return null;
         }
 
-        return augmentTuple;
+        // TODO: use a switch expression when we have JDK14+
+        switch (matchPath) {
+            case FLOWS_STATISTICS_UPDATE_MATCH:
+                return (AugmentTuple<E>) new AugmentTuple<>(GeneralAugMatchNotifUpdateFlowStats.class,
+                    new GeneralAugMatchNotifUpdateFlowStatsBuilder().setExtensionList(extensionsList).build());
+            case PACKET_RECEIVED_MATCH:
+                return (AugmentTuple<E>) new AugmentTuple<>(GeneralAugMatchNotifPacketIn.class,
+                    new GeneralAugMatchNotifPacketInBuilder().setExtensionList(extensionsList).build());
+            case PACKET_IN_MESSAGE_MATCH:
+                return (AugmentTuple<E>) new AugmentTuple<>(GeneralAugMatchPacketInMessage.class,
+                    new GeneralAugMatchPacketInMessageBuilder().setExtensionList(extensionsList).build());
+            case SWITCH_FLOW_REMOVED_MATCH:
+                return (AugmentTuple<E>)new AugmentTuple<>(GeneralAugMatchNotifSwitchFlowRemoved.class,
+                    new GeneralAugMatchNotifSwitchFlowRemovedBuilder().setExtensionList(extensionsList).build());
+            case FLOWS_STATISTICS_RPC_MATCH:
+                return (AugmentTuple<E>) new AugmentTuple<>(GeneralAugMatchRpcOutputFlowStats.class,
+                    new GeneralAugMatchRpcOutputFlowStatsBuilder().setExtensionList(extensionsList).build());
+            default:
+                LOG.warn("matchPath not supported: {}", matchPath);
+                return null;
+        }
     }
 
     /**
@@ -170,32 +143,28 @@ public final class MatchExtensionHelper {
      */
     private static ExtensionListBuilder processExtension(final MatchEntry matchEntry, final short ofVersion,
             final MatchPath matchPath) {
-        ExtensionListBuilder extListBld = null;
+        final var convertorProvider = OFSessionUtil.getExtensionConvertorProvider();
+        if (convertorProvider == null) {
+            return null;
+        }
 
         // TODO: EXTENSION PROPOSAL (match, OFJava to MD-SAL)
-        MatchEntrySerializerKey<? extends OxmClassBase, ? extends MatchField> key = new MatchEntrySerializerKey<>(
-                ofVersion, matchEntry.getOxmClass(), matchEntry.getOxmMatchField());
+        final var key = new MatchEntrySerializerKey<>(ofVersion, matchEntry.getOxmClass(),
+            matchEntry.getOxmMatchField());
 
         // If entry is experimenter, set experimenter ID to key
         if (matchEntry.getOxmClass().equals(ExperimenterClass.class)) {
-            ExperimenterIdCase experimenterIdCase = (ExperimenterIdCase) matchEntry.getMatchEntryValue();
-            key.setExperimenterId(experimenterIdCase.getExperimenter().getExperimenter().getValue());
+            key.setExperimenterId(
+                ((ExperimenterIdCase) matchEntry.getMatchEntryValue()).getExperimenter().getExperimenter().getValue());
         }
 
-        if (null != OFSessionUtil.getExtensionConvertorProvider()) {
-            ConvertorFromOFJava<MatchEntry, MatchPath> convertor =
-                    OFSessionUtil.getExtensionConvertorProvider().getConverter(key);
-            if (convertor != null) {
-                ExtensionAugment<? extends Augmentation<Extension>> extensionMatch =
-                        convertor.convert(matchEntry, matchPath);
-                ExtensionBuilder extBld = new ExtensionBuilder();
-                extBld.addAugmentation(extensionMatch.getAugmentationObject());
-
-                extListBld = new ExtensionListBuilder();
-                extListBld.setExtension(extBld.build());
-                extListBld.setExtensionKey(extensionMatch.getKey());
-            }
+        final var convertor = convertorProvider.getConverter(key);
+        if (convertor != null) {
+            final var extensionMatch = convertor.convert(matchEntry, matchPath);
+            return new ExtensionListBuilder()
+                .setExtension(new ExtensionBuilder().addAugmentation(extensionMatch.getAugmentationObject()).build())
+                .setExtensionKey(extensionMatch.getKey());
         }
-        return extListBld;
+        return null;
     }
 }