From d70f76a3626eb1b3786a5cfac4528a449b476583 Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Mon, 23 May 2016 09:26:27 +0200 Subject: HONEYCOMB-61: Fix outstanding issues Change-Id: I2dec6bbd8db656663029ad0f59da1b583c890565 Signed-off-by: Maros Marsalek --- v3po/api/src/main/yang/naming-context.yang | 3 +- .../data/impl/ModifiableDataTreeDelegator.java | 6 +-- .../v3po/data/impl/PersistingDataTreeAdapter.java | 27 +++++----- .../v3po/data/impl/ReadOnlyTransaction.java | 10 ++-- .../v3po/data/impl/ReadableDataTreeDelegator.java | 2 +- .../honeycomb/v3po/data/impl/WriteTransaction.java | 3 +- .../impl/rev160411/InMemoryDataTreeModule.java | 7 ++- v3po/data-impl/src/main/yang/data-impl.yang | 3 ++ .../honeycomb/v3po/translate/util/JsonUtils.java | 4 +- .../util/write/TransactionWriteContext.java | 2 +- .../v3po/initializers/InterfacesInitializer.java | 2 +- .../v3po/vpp/data/init/RestoringInitializer.java | 21 ++++---- v3po/vpp-cfg-init/src/main/yang/vpp-cfg-init.yang | 2 + .../v3po/translate/v3po/util/NamingContext.java | 58 ++++++++++++++++++++-- 14 files changed, 102 insertions(+), 48 deletions(-) (limited to 'v3po') diff --git a/v3po/api/src/main/yang/naming-context.yang b/v3po/api/src/main/yang/naming-context.yang index aa4ced29d..fa44bdd12 100644 --- a/v3po/api/src/main/yang/naming-context.yang +++ b/v3po/api/src/main/yang/naming-context.yang @@ -1,7 +1,7 @@ module naming-context { yang-version 1; namespace "urn:honeycomb:params:xml:ns:yang:naming:context"; - prefix "vpp-u"; + prefix "nc"; description "This module contains data definition for naming mapping context"; @@ -25,6 +25,7 @@ module naming-context { list mapping { key "name"; + unique "index"; leaf name { type string; 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>, ReadFailedException> read( + public synchronized CheckedFuture>, 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 r) { + final Consumer r) { switch (store) { case CONFIGURATION: checkArgument(configModification != null, "Modification of %s is not supported", store); diff --git a/v3po/data-impl/src/main/java/org/opendaylight/yang/gen/v1/urn/honeycomb/params/xml/ns/yang/data/impl/rev160411/InMemoryDataTreeModule.java b/v3po/data-impl/src/main/java/org/opendaylight/yang/gen/v1/urn/honeycomb/params/xml/ns/yang/data/impl/rev160411/InMemoryDataTreeModule.java index 956aa90f4..99e5b396c 100644 --- a/v3po/data-impl/src/main/java/org/opendaylight/yang/gen/v1/urn/honeycomb/params/xml/ns/yang/data/impl/rev160411/InMemoryDataTreeModule.java +++ b/v3po/data-impl/src/main/java/org/opendaylight/yang/gen/v1/urn/honeycomb/params/xml/ns/yang/data/impl/rev160411/InMemoryDataTreeModule.java @@ -31,11 +31,10 @@ public class InMemoryDataTreeModule extends org.opendaylight.yang.gen.v1.urn.hon return new CloseableConfigDataTree(getSchemaServiceDependency().getGlobalContext(), getType()); } - private static class CloseableConfigDataTree implements AutoCloseable, - org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree { - private final org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree dataTree; + private static class CloseableConfigDataTree implements AutoCloseable, DataTree { + private final DataTree dataTree; - public CloseableConfigDataTree(final SchemaContext schemaContext, final DatatreeType type) { + CloseableConfigDataTree(final SchemaContext schemaContext, final DatatreeType type) { this.dataTree = InMemoryDataTreeFactory.getInstance().create( type == DatatreeType.Config ? TreeType.CONFIGURATION : TreeType.OPERATIONAL ); diff --git a/v3po/data-impl/src/main/yang/data-impl.yang b/v3po/data-impl/src/main/yang/data-impl.yang index ebca239a2..0ea76cf0e 100644 --- a/v3po/data-impl/src/main/yang/data-impl.yang +++ b/v3po/data-impl/src/main/yang/data-impl.yang @@ -39,6 +39,7 @@ module data-impl { leaf type { type dapi:datatree-type; + mandatory true; } } } @@ -73,6 +74,8 @@ module data-impl { leaf persist-file-path { type string; + mandatory true; + description "Path to a file to be used as data storage"; } } } diff --git a/v3po/translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/util/JsonUtils.java b/v3po/translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/util/JsonUtils.java index be6ffa235..d9798d07d 100644 --- a/v3po/translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/util/JsonUtils.java +++ b/v3po/translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/util/JsonUtils.java @@ -41,7 +41,7 @@ import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContaine import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.opendaylight.yangtools.yang.model.api.SchemaPath; -public class JsonUtils { +public final class JsonUtils { private JsonUtils() {} @@ -80,7 +80,7 @@ public class JsonUtils { final NormalizedNodeStreamWriter writer = ImmutableNormalizedNodeStreamWriter.from(builder); final JsonParserStream jsonParser = JsonParserStream.create(writer, schemaContext); - final JsonReader reader = new JsonReader(new InputStreamReader(stream)); + final JsonReader reader = new JsonReader(new InputStreamReader(stream, Charsets.UTF_8)); jsonParser.parse(reader); return builder.build(); diff --git a/v3po/translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/util/write/TransactionWriteContext.java b/v3po/translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/util/write/TransactionWriteContext.java index 20bbf19e0..47498f594 100644 --- a/v3po/translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/util/write/TransactionWriteContext.java +++ b/v3po/translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/util/write/TransactionWriteContext.java @@ -43,7 +43,7 @@ public final class TransactionWriteContext implements WriteContext { private final DOMDataReadOnlyTransaction afterTx; private final ModificationCache ctx; private final BindingNormalizedNodeSerializer serializer; - private MappingContext mappingContext; + private final MappingContext mappingContext; public TransactionWriteContext(final BindingNormalizedNodeSerializer serializer, final DOMDataReadOnlyTransaction beforeTx, diff --git a/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/initializers/InterfacesInitializer.java b/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/initializers/InterfacesInitializer.java index faaf06306..bd10e5f55 100644 --- a/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/initializers/InterfacesInitializer.java +++ b/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/initializers/InterfacesInitializer.java @@ -74,7 +74,7 @@ public class InterfacesInitializer extends AbstractDataTreeConverter dataContainerChild : containerNode .getValue()) { final YangInstanceIdentifier iid = YangInstanceIdentifier.create(dataContainerChild.getIdentifier()); + LOG.trace("Restoring {} from {}", iid, path); + switch (restorationType) { case Merge: domDataWriteTransaction.merge(datastoreType, iid, dataContainerChild); @@ -98,6 +100,7 @@ public class RestoringInitializer implements DataTreeInitializer { // Block here to prevent subsequent initializers processing before context is fully restored domDataWriteTransaction.submit().checkedGet(); + LOG.debug("Data from {} restored successfully", path); } catch (IOException | TransactionCommitFailedException e) { throw new InitializeException("Unable to restore data from " + path, e); diff --git a/v3po/vpp-cfg-init/src/main/yang/vpp-cfg-init.yang b/v3po/vpp-cfg-init/src/main/yang/vpp-cfg-init.yang index 8d305685e..52750d926 100644 --- a/v3po/vpp-cfg-init/src/main/yang/vpp-cfg-init.yang +++ b/v3po/vpp-cfg-init/src/main/yang/vpp-cfg-init.yang @@ -98,6 +98,7 @@ module vpp-cfg-init { leaf persist-file-path { type string; + mandatory true; } leaf restoration-type { @@ -108,6 +109,7 @@ module vpp-cfg-init { leaf datastore-type { type dapi:datatree-type; + mandatory true; } } } diff --git a/v3po/vpp-translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/v3po/util/NamingContext.java b/v3po/vpp-translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/v3po/util/NamingContext.java index d32f27203..34c810462 100644 --- a/v3po/vpp-translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/v3po/util/NamingContext.java +++ b/v3po/vpp-translate-utils/src/main/java/io/fd/honeycomb/v3po/translate/v3po/util/NamingContext.java @@ -36,14 +36,15 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Utility adapter on top of {@link MappingContext} + * Utility adapter on top of {@link MappingContext} storing integer to string mappings according to naming-context yang + * model */ public final class NamingContext implements AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(NamingContext.class); private final String artificialNamePrefix; - private KeyedInstanceIdentifier + private final KeyedInstanceIdentifier namingContextIid; /** @@ -58,15 +59,30 @@ public final class NamingContext implements AutoCloseable { return list.get(0); }); - public NamingContext(final String artificialNamePrefix, final String instanceName) { + /** + * Create new naming context + * + * @param artificialNamePrefix artificial name to be used for items without a name in VPP (or not provided) + * @param instanceName name of this context instance. Will be used as list item identifier within context data tree + */ + public NamingContext(@Nonnull final String artificialNamePrefix, @Nonnull final String instanceName) { this.artificialNamePrefix = artificialNamePrefix; namingContextIid = InstanceIdentifier.create(Contexts.class).child( org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.NamingContext.class, new NamingContextKey(instanceName)); } + /** + * Retrieve name for mapping stored provided mappingContext instance. If not present, artificial name will be + * generated. + * + * @param index index of a mapped item + * @param mappingContext mapping context providing context data for current transaction + * + * @return name mapped to provided index + */ @Nonnull - public synchronized String getName(final int index, final MappingContext mappingContext) { + public synchronized String getName(final int index, @Nonnull final MappingContext mappingContext) { if (!containsName(index, mappingContext)) { final String artificialName = getArtificialName(index); LOG.info("Assigning artificial name: {} for index: {}", artificialName, index); @@ -81,13 +97,29 @@ public final class NamingContext implements AutoCloseable { .collect(SINGLE_ITEM_COLLECTOR).getName(); } - public synchronized boolean containsName(final int index, final MappingContext mappingContext) { + /** + * Check whether mapping is present for index. + * + * @param index index of a mapped item + * @param mappingContext mapping context providing context data for current transaction + * + * @return true if present, false otherwise + */ + public synchronized boolean containsName(final int index, @Nonnull final MappingContext mappingContext) { final Optional read = mappingContext.read(namingContextIid.child(Mappings.class)); return read.isPresent() ? read.get().getMapping().stream().anyMatch(mapping -> mapping.getIndex().equals(index)) : false; } + + /** + * Add mapping to current context + * + * @param index index of a mapped item + * @param name name of a mapped item + * @param mappingContext mapping context providing context data for current transaction + */ public synchronized void addName(final int index, final String name, final MappingContext mappingContext) { final KeyedInstanceIdentifier mappingIid = getMappingIid(name); mappingContext.put(mappingIid, new MappingBuilder().setIndex(index).setName(name).build()); @@ -97,6 +129,12 @@ public final class NamingContext implements AutoCloseable { return namingContextIid.child(Mappings.class).child(Mapping.class, new MappingKey(name)); } + /** + * Remove mapping from current context + * + * @param name name of a mapped item + * @param mappingContext mapping context providing context data for current transaction + */ public synchronized void removeName(final String name, final MappingContext mappingContext) { mappingContext.delete(getMappingIid(name)); } @@ -105,6 +143,8 @@ public final class NamingContext implements AutoCloseable { * Returns index value associated with the given name. * * @param name the name whose associated index value is to be returned + * @param mappingContext mapping context providing context data for current transaction + * * @return integer index value matching supplied name * @throws IllegalArgumentException if name was not found */ @@ -115,6 +155,14 @@ public final class NamingContext implements AutoCloseable { } + /** + * Check whether mapping is present for name. + * + * @param name name of a mapped item + * @param mappingContext mapping context providing context data for current transaction + * + * @return true if present, false otherwise + */ public synchronized boolean containsIndex(final String name, final MappingContext mappingContext) { return mappingContext.read(getMappingIid(name)).isPresent(); } -- cgit 1.2.3-korg