HONEYCOMB-61: Fix outstanding issues

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

5 files changed, 23 insertions, 25 deletions

diff --git a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java
index 2b9010747..75ffb702a 100644
--- a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java
+++ b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java
@@ -125,14 +125,14 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
                 // Blocking on context data update
                 contextUpdateResult.checkedGet();
 
-            } catch (io.fd.honeycomb.v3po.translate.write.WriterRegistry.BulkUpdateException e) {
+            } catch (WriterRegistry.BulkUpdateException e) {
                 LOG.warn("Failed to apply all changes", e);
                 LOG.info("Trying to revert successful changes for current transaction");
 
                 try {
                     e.revertChanges();
                     LOG.info("Changes successfully reverted");
-                } catch (io.fd.honeycomb.v3po.translate.write.WriterRegistry.Reverter.RevertFailedException revertFailedException) {
+                } catch (WriterRegistry.Reverter.RevertFailedException revertFailedException) {
                     // fail with failed revert
                     LOG.error("Failed to revert successful changes", revertFailedException);
                     throw revertFailedException;
@@ -140,7 +140,7 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager
 
                 throw e; // fail with success revert
             } catch (TransactionCommitFailedException e) {
-                // FIXME revert should probably occur when context is not written successfully, but can that even happen ?
+                // FIXME revert should probably occur when context is not written successfully
                 final String msg = "Error while updating mapping context data";
                 LOG.error(msg, e);
                 throw new TranslationException(msg, e);
diff --git a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/PersistingDataTreeAdapter.java b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/PersistingDataTreeAdapter.java
index df5bbee4b..9b71dfd62 100644
--- a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/PersistingDataTreeAdapter.java
+++ b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/PersistingDataTreeAdapter.java
@@ -17,6 +17,7 @@
 package io.fd.honeycomb.v3po.data.impl;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.base.Optional;
 import io.fd.honeycomb.v3po.translate.util.JsonUtils;
@@ -41,32 +42,32 @@ import org.slf4j.LoggerFactory;
  * Adapter for a DataTree that stores current state of data in backing DataTree on each successful commit.
  * Uses JSON format.
  */
-public class PersistingDataTreeAdapter implements org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree, AutoCloseable {
+public class PersistingDataTreeAdapter implements DataTree, AutoCloseable {
 
     private static final Logger LOG = LoggerFactory.getLogger(PersistingDataTreeAdapter.class);
 
     private final DataTree delegateDependency;
-    private SchemaService schemaServiceDependency;
+    private final SchemaService schemaServiceDependency;
     private final Path path;
 
     /**
      * Create new Persisting DataTree adapter
      *
-     * @param delegateDependency backing data tree that actually handles all the operations
+     * @param delegate backing data tree that actually handles all the operations
      * @param persistPath path to a file (existing or not) to be used as storage for persistence. Full control over
      *                    a file at peristPath is expected
-     * @param schemaServiceDependency schemaContext provier
+     * @param schemaService schemaContext provier
      */
-    public PersistingDataTreeAdapter(@Nonnull final DataTree delegateDependency,
-                                     @Nonnull final SchemaService schemaServiceDependency,
+    public PersistingDataTreeAdapter(@Nonnull final DataTree delegate,
+                                     @Nonnull final SchemaService schemaService,
                                      @Nonnull final Path persistPath) {
-        this.path = testPersistPath(persistPath);
-        this.delegateDependency = delegateDependency;
-        this.schemaServiceDependency = schemaServiceDependency;
+        this.path = testPersistPath(checkNotNull(persistPath, "persistPath is null"));
+        this.delegateDependency = checkNotNull(delegate, "delegate is null");
+        this.schemaServiceDependency = checkNotNull(schemaService, "schemaService is null");
     }
 
     /**
-     * Test whether file at persistPath is a file and can be created/deleted
+     * Test whether file at persistPath exists and is readable or create it along with its parent structure
      */
     private Path testPersistPath(final Path persistPath) {
         try {
@@ -115,12 +116,8 @@ public class PersistingDataTreeAdapter implements org.opendaylight.yangtools.yan
         if(currentRoot.isPresent()) {
             try {
                 LOG.trace("Persisting current data: {} into: {}", currentRoot.get(), path);
-                // Make sure the file gets overwritten
-                if(Files.exists(path)) {
-                    Files.delete(path);
-                }
                 JsonUtils.writeJsonRoot(currentRoot.get(), schemaServiceDependency.getGlobalContext(),
-                    Files.newOutputStream(path, StandardOpenOption.CREATE));
+                    Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING));
                 LOG.trace("Data persisted successfully in {}", path);
             } catch (IOException e) {
                 throw new IllegalStateException("Unable to persist current data", e);
diff --git a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ReadOnlyTransaction.java b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ReadOnlyTransaction.java
index c38fa27ca..2850a0d9a 100644
--- a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ReadOnlyTransaction.java
+++ b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ReadOnlyTransaction.java
@@ -41,11 +41,11 @@ final class ReadOnlyTransaction implements DOMDataReadOnlyTransaction {
     private static final Logger LOG = LoggerFactory.getLogger(ReadOnlyTransaction.class);
 
     @Nullable
-    private volatile ReadableDataManager operationalData;
+    private ReadableDataManager operationalData;
     @Nullable
-    private volatile ReadableDataManager configSnapshot;
+    private ReadableDataManager configSnapshot;
 
-    private volatile boolean closed = false;
+    private boolean closed = false;
 
     /**
      * @param configData config data tree manager. Null if config reads are not to be supported
@@ -58,14 +58,14 @@ final class ReadOnlyTransaction implements DOMDataReadOnlyTransaction {
     }
 
     @Override
-    public void close() {
+    public synchronized void close() {
         closed = true;
         configSnapshot = null;
         operationalData = null;
     }
 
     @Override
-    public CheckedFuture<Optional<NormalizedNode<?, ?>>, ReadFailedException> read(
+    public synchronized CheckedFuture<Optional<NormalizedNode<?, ?>>, ReadFailedException> read(
             final LogicalDatastoreType store,
             final YangInstanceIdentifier path) {
         LOG.debug("ReadOnlyTransaction.read(), store={}, path={}", store, path);
diff --git a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ReadableDataTreeDelegator.java b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ReadableDataTreeDelegator.java
index 46903a1f0..8dc7f8f1c 100644
--- a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ReadableDataTreeDelegator.java
+++ b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ReadableDataTreeDelegator.java
@@ -114,7 +114,7 @@ public final class ReadableDataTreeDelegator implements ReadableDataManager {
                 new org.opendaylight.controller.md.sal.common.api.data.ReadFailedException(
                     "Failed to read VPP data", e));
         } catch (TransactionCommitFailedException e) {
-            // FIXME revert should probably occur when context is not written successfully, but can that even happen ?
+            // FIXME revert should probably occur when context is not written successfully
             final String msg = "Error while updating mapping context data";
             LOG.error(msg, e);
             return Futures.immediateFailedCheckedFuture(
diff --git a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/WriteTransaction.java b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/WriteTransaction.java
index ac4ba9049..c8f9bd3db 100644
--- a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/WriteTransaction.java
+++ b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/WriteTransaction.java
@@ -30,6 +30,7 @@ import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import io.fd.honeycomb.v3po.data.DataModification;
 import io.fd.honeycomb.v3po.translate.TranslationException;
+import java.util.function.Consumer;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
@@ -74,7 +75,7 @@ final class WriteTransaction implements DOMDataWriteTransaction {
     }
 
     private void handleOperation(final LogicalDatastoreType store,
-                                 final java.util.function.Consumer<DataModification> r) {
+                                 final Consumer<DataModification> r) {
         switch (store) {
             case CONFIGURATION:
                 checkArgument(configModification != null, "Modification of %s is not supported", store);