HONEYCOMB-270 Remove read (presence) workarounds for nat

Change-Id: Ia0986dbe173123a4dca4f7f3bf65e7ba4851b1b7
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>

4 files changed, 18 insertions, 120 deletions

diff --git a/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/AbstractInterfaceNatCustomizer.java b/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/AbstractInterfaceNatCustomizer.java
index e48e7eb3d..f4d45900c 100644
--- a/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/AbstractInterfaceNatCustomizer.java
+++ b/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/AbstractInterfaceNatCustomizer.java
@@ -47,6 +47,11 @@ abstract class AbstractInterfaceNatCustomizer<C extends DataObject, B extends Bu
     public void readCurrentAttributes(@Nonnull final InstanceIdentifier<C> id,
                                       @Nonnull final B builder,
                                       @Nonnull final ReadContext ctx) throws ReadFailedException {
+        // NOOP
+    }
+
+    @Override
+    public boolean isPresent(final InstanceIdentifier<C> id, final C built, final ReadContext ctx) throws ReadFailedException {
         final String ifcName = id.firstKeyOf(Interface.class).getName();
         getLog().debug("Reading NAT features on interface: {}", ifcName);
         final int index = ifcContext.getIndex(ifcName, ctx.getMappingContext());
@@ -56,17 +61,15 @@ abstract class AbstractInterfaceNatCustomizer<C extends DataObject, B extends Bu
                 dumpMgr.getDump(id, ctx.getModificationCache(), null);
 
         // Find entries for current ifc and if is marked as inside set the builder to return presence container
-        dump.or(new SnatInterfaceDetailsReplyDump()).snatInterfaceDetails.stream()
+        return dump.or(new SnatInterfaceDetailsReplyDump()).snatInterfaceDetails.stream()
                 .filter(snatIfcDetail -> snatIfcDetail.swIfIndex == index)
                 .filter(this::isExpectedNatType)
                 .findFirst()
-                .ifPresent(snatIfcDetail -> setBuilderPresence(builder));
+                .isPresent();
         // Not setting data, just marking the builder to propagate empty container to indicate presence
     }
 
     protected abstract Logger getLog();
 
-    abstract void setBuilderPresence(@Nonnull final B builder);
-
     abstract boolean isExpectedNatType(final SnatInterfaceDetails snatInterfaceDetails);
 }
diff --git a/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/InterfaceInboundNatCustomizer.java b/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/InterfaceInboundNatCustomizer.java
index 52467a19a..3e4b60fb4 100644
--- a/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/InterfaceInboundNatCustomizer.java
+++ b/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/InterfaceInboundNatCustomizer.java
@@ -17,6 +17,7 @@
 package io.fd.honeycomb.nat.read.ifc;
 
 import io.fd.honeycomb.translate.read.ReadContext;
+import io.fd.honeycomb.translate.read.ReadFailedException;
 import io.fd.honeycomb.translate.spi.read.Initialized;
 import io.fd.honeycomb.translate.util.read.cache.DumpCacheManager;
 import io.fd.honeycomb.translate.vpp.util.NamingContext;
@@ -32,8 +33,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interfa
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.nat.rev161214._interface.nat.attributes.nat.Inbound;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.nat.rev161214._interface.nat.attributes.nat.InboundBuilder;
 import org.opendaylight.yangtools.concepts.Builder;
-import org.opendaylight.yangtools.yang.binding.Augmentation;
-import org.opendaylight.yangtools.yang.binding.DataContainer;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
@@ -55,8 +54,10 @@ final class InterfaceInboundNatCustomizer extends AbstractInterfaceNatCustomizer
     }
 
     @Override
-    void setBuilderPresence(@Nonnull final InboundBuilder builder) {
-        ((PresenceInboundBuilder) builder).setPresent(true);
+    public void readCurrentAttributes(@Nonnull final InstanceIdentifier<Inbound> id,
+                                      @Nonnull final InboundBuilder builder, @Nonnull final ReadContext ctx)
+            throws ReadFailedException {
+        super.readCurrentAttributes(id, builder, ctx);
     }
 
     @Override
@@ -68,7 +69,7 @@ final class InterfaceInboundNatCustomizer extends AbstractInterfaceNatCustomizer
     @Override
     public InboundBuilder getBuilder(@Nonnull final InstanceIdentifier<Inbound> id) {
         // Return not present value by default
-        return new PresenceInboundBuilder(false);
+        return new InboundBuilder();
     }
 
     @Override
@@ -91,53 +92,4 @@ final class InterfaceInboundNatCustomizer extends AbstractInterfaceNatCustomizer
                 .child(Inbound.class);
         return Initialized.create(cfgId, readValue);
     }
-
-    // TODO HONEYCOMB-270, make this better, having to fake a builder + value is just exploitation.
-
-    /**
-     * Special Builder to also propagate empty container into the resulting data.
-     */
-    private static final class PresenceInboundBuilder extends InboundBuilder {
-
-        private volatile boolean isPresent = false;
-
-        PresenceInboundBuilder(final boolean isPresent) {
-            this.isPresent = isPresent;
-        }
-
-        void setPresent(final boolean present) {
-            this.isPresent = present;
-        }
-
-        @Override
-        public Inbound build() {
-            return isPresent
-                    ? super.build()
-                    : NotPresentInbound.NOT_PRESENT_INBOUND;
-        }
-    }
-
-    /**
-     * Fake container that returns false on equals.
-     */
-    private static final class NotPresentInbound implements Inbound {
-
-        private static final NotPresentInbound NOT_PRESENT_INBOUND = new NotPresentInbound();
-
-        @Override
-        public <E extends Augmentation<Inbound>> E getAugmentation(final Class<E> augmentationType) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public Class<? extends DataContainer> getImplementedInterface() {
-            return Inbound.class;
-        }
-
-        @Override
-        public boolean equals(final Object obj) {
-            // This is necessary to fake this.equals(something)
-            return obj == NOT_PRESENT_INBOUND;
-        }
-    }
 }
diff --git a/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/InterfaceOutboundNatCustomizer.java b/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/InterfaceOutboundNatCustomizer.java
index b08a636ca..da940c835 100644
--- a/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/InterfaceOutboundNatCustomizer.java
+++ b/nat/nat2vpp/src/main/java/io/fd/honeycomb/nat/read/ifc/InterfaceOutboundNatCustomizer.java
@@ -32,8 +32,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interfa
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.nat.rev161214._interface.nat.attributes.nat.Outbound;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.nat.rev161214._interface.nat.attributes.nat.OutboundBuilder;
 import org.opendaylight.yangtools.concepts.Builder;
-import org.opendaylight.yangtools.yang.binding.Augmentation;
-import org.opendaylight.yangtools.yang.binding.DataContainer;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
@@ -55,11 +53,6 @@ final class InterfaceOutboundNatCustomizer extends AbstractInterfaceNatCustomize
     }
 
     @Override
-    void setBuilderPresence(@Nonnull final OutboundBuilder builder) {
-        ((PresenceOutboundBuilder) builder).setPresent(true);
-    }
-
-    @Override
     boolean isExpectedNatType(final SnatInterfaceDetails snatInterfaceDetails) {
         return snatInterfaceDetails.isInside == 0;
     }
@@ -67,7 +60,7 @@ final class InterfaceOutboundNatCustomizer extends AbstractInterfaceNatCustomize
     @Nonnull
     @Override
     public OutboundBuilder getBuilder(@Nonnull final InstanceIdentifier<Outbound> id) {
-        return new PresenceOutboundBuilder(false);
+        return new OutboundBuilder();
     }
 
     @Override
@@ -90,54 +83,4 @@ final class InterfaceOutboundNatCustomizer extends AbstractInterfaceNatCustomize
                         .child(Outbound.class);
         return Initialized.create(cfgId, readValue);
     }
-
-    // TODO HONEYCOMB-270, make this better, having to fake a builder + value is just exploitation.
-
-    /**
-     * Special Builder to also propagate empty container into the resulting data.
-     */
-    private static final class PresenceOutboundBuilder extends OutboundBuilder {
-
-        private volatile boolean isPresent = false;
-
-        PresenceOutboundBuilder(final boolean isPresent) {
-            this.isPresent = isPresent;
-        }
-
-        void setPresent(final boolean present) {
-            this.isPresent = present;
-        }
-
-        @Override
-        public Outbound build() {
-            final Outbound build = super.build();
-            return isPresent
-                    ? build
-                    : NotPresentOutbound.NOT_PRESENT_OUTBOUND;
-        }
-    }
-
-    /**
-     * Fake container that returns false on equals.
-     */
-    private static final class NotPresentOutbound implements Outbound {
-
-        private static final NotPresentOutbound NOT_PRESENT_OUTBOUND = new NotPresentOutbound();
-
-        @Override
-        public <E extends Augmentation<Outbound>> E getAugmentation(final Class<E> augmentationType) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public Class<? extends DataContainer> getImplementedInterface() {
-            return Outbound.class;
-        }
-
-        @Override
-        public boolean equals(final Object obj) {
-            // This is necessary to fake this.equals(something)
-            return obj == NOT_PRESENT_OUTBOUND;
-        }
-    }
 }
diff --git a/nat/nat2vpp/src/test/java/io/fd/honeycomb/nat/read/ifc/InterfaceInboundNatCustomizerTest.java b/nat/nat2vpp/src/test/java/io/fd/honeycomb/nat/read/ifc/InterfaceInboundNatCustomizerTest.java
index 140d35fec..3cef1880e 100644
--- a/nat/nat2vpp/src/test/java/io/fd/honeycomb/nat/read/ifc/InterfaceInboundNatCustomizerTest.java
+++ b/nat/nat2vpp/src/test/java/io/fd/honeycomb/nat/read/ifc/InterfaceInboundNatCustomizerTest.java
@@ -51,7 +51,7 @@ public class InterfaceInboundNatCustomizerTest
     private static final String CTX_NAME = "ifc";
 
     @Mock
-    private EntityDumpExecutor<SnatInterfaceDetailsReplyDump, Void> abc;
+    private EntityDumpExecutor<SnatInterfaceDetailsReplyDump, Void> natExecutor;
     private DumpCacheManager<SnatInterfaceDetailsReplyDump, Void> dumpMgr;
     private NamingContext ifcContext = new NamingContext(CTX_NAME, CTX_NAME);
     private InstanceIdentifier<Inbound> id;
@@ -65,9 +65,9 @@ public class InterfaceInboundNatCustomizerTest
         id = getId(Inbound.class);
         defineMapping(mappingContext, IFC_NAME, IFC_IDX, CTX_NAME);
         // empty dump
-        Mockito.doReturn(new SnatInterfaceDetailsReplyDump()).when(abc).executeDump(id, null);
+        Mockito.doReturn(new SnatInterfaceDetailsReplyDump()).when(natExecutor).executeDump(id, null);
         dumpMgr = new DumpCacheManager.DumpCacheManagerBuilder<SnatInterfaceDetailsReplyDump, Void>()
-                .withExecutor(abc)
+                .withExecutor(natExecutor)
                 .build();
     }
 
@@ -95,7 +95,7 @@ public class InterfaceInboundNatCustomizerTest
         detail.isInside = 1;
         detail.swIfIndex = IFC_IDX;
         details.snatInterfaceDetails = Lists.newArrayList(detail);
-        Mockito.doReturn(details).when(abc).executeDump(id, null);
+        Mockito.doReturn(details).when(natExecutor).executeDump(id, null);
 
         assertTrue(getReader().read(id, ctx).isPresent());
     }