From b589b5bb6fc4b88f74710010782b155c80433740 Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Wed, 7 Sep 2016 17:15:09 +0200 Subject: HONEYCOMB-194 Increase data-impl coverage to 87% Change-Id: I1bd6d6ad2e8d35322346aa658e74413ce2d889f0 Signed-off-by: Maros Marsalek --- .../java/io/fd/honeycomb/data/impl/DataBroker.java | 2 +- .../data/impl/PersistingDataTreeAdapter.java | 108 ++++++++++++--------- .../honeycomb/data/impl/ReadOnlyTransaction.java | 4 +- .../data/impl/ReadableDataTreeDelegator.java | 7 +- 4 files changed, 73 insertions(+), 48 deletions(-) (limited to 'infra/data-impl/src/main/java/io') diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/DataBroker.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/DataBroker.java index eb19c9dd1..5246ca15b 100644 --- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/DataBroker.java +++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/DataBroker.java @@ -166,7 +166,7 @@ public class DataBroker implements DOMDataBroker, Closeable { } /** - * Transaction factory specific for Honeycomb's context pipeline (config: none, operational: read+write) + * Transaction factory specific for Honeycomb's context pipeline (config: none, operational: read+write. */ private static class ContextPipelineTxFactory implements TransactionFactory { private final ModifiableDataManager operationalDataTree; diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/PersistingDataTreeAdapter.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/PersistingDataTreeAdapter.java index 459971436..356ca51c1 100644 --- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/PersistingDataTreeAdapter.java +++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/PersistingDataTreeAdapter.java @@ -19,6 +19,8 @@ package io.fd.honeycomb.data.impl; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.MoreObjects; import com.google.common.base.Optional; import io.fd.honeycomb.translate.util.JsonUtils; import java.io.IOException; @@ -42,13 +44,12 @@ 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 DataTree, AutoCloseable { +public class PersistingDataTreeAdapter implements DataTree { private static final Logger LOG = LoggerFactory.getLogger(PersistingDataTreeAdapter.class); private final DataTree delegateDependency; - private final SchemaService schemaServiceDependency; - private final Path path; + private final JsonPersister persister; /** * Create new Persisting DataTree adapter @@ -61,30 +62,13 @@ public class PersistingDataTreeAdapter implements DataTree, AutoCloseable { public PersistingDataTreeAdapter(@Nonnull final DataTree delegate, @Nonnull final SchemaService schemaService, @Nonnull final Path persistPath) { - this.path = testPersistPath(checkNotNull(persistPath, "persistPath is null")); - this.delegateDependency = checkNotNull(delegate, "delegate is null"); - this.schemaServiceDependency = checkNotNull(schemaService, "schemaService is null"); + this(delegate, new JsonPersister(persistPath, schemaService)); } - /** - * Test whether file at persistPath exists and is readable or create it along with its parent structure - */ - private Path testPersistPath(final Path persistPath) { - try { - checkArgument(!Files.isDirectory(persistPath), "Path %s points to a directory", persistPath); - if(Files.exists(persistPath)) { - checkArgument(Files.isReadable(persistPath), - "Provided path %s points to existing, but non-readable file", persistPath); - return persistPath; - } - Files.createDirectories(persistPath.getParent()); - Files.write(persistPath, new byte[]{}, StandardOpenOption.CREATE); - } catch (IOException | UnsupportedOperationException e) { - LOG.warn("Provided path for persistence: {} is not usable", persistPath, e); - throw new IllegalArgumentException("Path " + persistPath + " cannot be used as "); - } - - return persistPath; + public PersistingDataTreeAdapter(final DataTree delegate, + final JsonPersister persister) { + this.delegateDependency = checkNotNull(delegate, "delegate is null"); + this.persister = persister; } @Override @@ -103,27 +87,68 @@ public class PersistingDataTreeAdapter implements DataTree, AutoCloseable { delegateDependency.commit(dataTreeCandidate); LOG.debug("Delegate commit successful. Persisting data"); - // TODO HONEYCOMB-163 doing full read and full write might not be the fastest way of persisting data here + // FIXME doing full read and full write might not be the fastest way of persisting data here final DataTreeSnapshot dataTreeSnapshot = delegateDependency.takeSnapshot(); - // TODO HONEYCOMB-163 this can be handled in background by a dedicated thread + a limited blocking queue - // TODO HONEYCOMB-163 enable configurable granularity for persists. Maybe doing write on every modification is too much + // TODO this can be handled in background by a dedicated thread + a limited blocking queue + // TODO enable configurable granularity for persists. Maybe doing write on every modification is too much // and we could do bulk persist - persistCurrentData(dataTreeSnapshot.readNode(YangInstanceIdentifier.EMPTY)); + persister.persistCurrentData(dataTreeSnapshot.readNode(YangInstanceIdentifier.EMPTY)); } - private void persistCurrentData(final Optional> currentRoot) { - if (currentRoot.isPresent()) { + @VisibleForTesting + static class JsonPersister { + + private final Path path; + private final SchemaService schemaServiceDependency; + + JsonPersister(final Path persistPath, final SchemaService schemaService) { + this.path = testPersistPath(checkNotNull(persistPath, "persistPath is null")); + this.schemaServiceDependency = checkNotNull(schemaService, "schemaService is null"); + } + + void persistCurrentData(final Optional> currentRoot) { + if (currentRoot.isPresent()) { + try { + LOG.trace("Persisting current data: {} into: {}", currentRoot.get(), path); + JsonUtils.writeJsonRoot(currentRoot.get(), schemaServiceDependency.getGlobalContext(), + 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); + } + } else { + LOG.debug("Skipping persistence, since there's no data to persist"); + } + } + + /** + * Test whether file at persistPath exists and is readable or create it along with its parent structure. + */ + private static Path testPersistPath(final Path persistPath) { try { - LOG.trace("Persisting current data: {} into: {}", currentRoot.get(), path); - JsonUtils.writeJsonRoot(currentRoot.get(), schemaServiceDependency.getGlobalContext(), - 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); + checkArgument(!Files.isDirectory(persistPath), "Path %s points to a directory", persistPath); + if (Files.exists(persistPath)) { + checkArgument(Files.isReadable(persistPath), + "Provided path %s points to existing, but non-readable file", persistPath); + return persistPath; + } + Files.createDirectories(persistPath.getParent()); + Files.write(persistPath, new byte[]{}, StandardOpenOption.CREATE); + } catch (IOException | UnsupportedOperationException e) { + LOG.warn("Provided path for persistence: {} is not usable", persistPath, e); + throw new IllegalArgumentException("Path " + persistPath + " cannot be used as "); } - } else { - LOG.debug("Skipping persistence, since there's no data to persist"); + + return persistPath; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("path", path) + .toString(); } } @@ -143,9 +168,4 @@ public class PersistingDataTreeAdapter implements DataTree, AutoCloseable { return delegateDependency.prepare(dataTreeModification); } - @Override - public void close() throws Exception { - LOG.trace("Closing {} for {}", getClass().getSimpleName(), path); - // NOOP - } } diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ReadOnlyTransaction.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ReadOnlyTransaction.java index 00d105e26..e21098a3b 100644 --- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ReadOnlyTransaction.java +++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ReadOnlyTransaction.java @@ -72,10 +72,10 @@ final class ReadOnlyTransaction implements DOMDataReadOnlyTransaction { checkState(!closed, "Transaction has been closed"); if (store == LogicalDatastoreType.OPERATIONAL) { - checkArgument(operationalData != null, "{} reads not supported", store); + checkArgument(operationalData != null, "%s reads not supported", store); return operationalData.read(path); } else { - checkArgument(configSnapshot != null, "{} reads not supported", store); + checkArgument(configSnapshot != null, "%s reads not supported", store); return configSnapshot.read(path); } } diff --git a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ReadableDataTreeDelegator.java b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ReadableDataTreeDelegator.java index ae29fa08b..a7ad88c9f 100644 --- a/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ReadableDataTreeDelegator.java +++ b/infra/data-impl/src/main/java/io/fd/honeycomb/data/impl/ReadableDataTreeDelegator.java @@ -19,6 +19,7 @@ package io.fd.honeycomb.data.impl; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.Iterables.getOnlyElement; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -178,11 +179,14 @@ public final class ReadableDataTreeDelegator implements ReadableDataManager { } } - private static DataContainerChild wrapListIntoMixinNode( + @VisibleForTesting + static DataContainerChild wrapListIntoMixinNode( final Collection> normalizedRootElements, final ListSchemaNode listSchema) { if (listSchema.getKeyDefinition().isEmpty()) { final CollectionNodeBuilder listBuilder = Builders.unkeyedListBuilder(); + listBuilder.withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(listSchema.getQName())); + for (NormalizedNode normalizedRootElement : normalizedRootElements) { listBuilder.withChild((UnkeyedListEntryNode) normalizedRootElement); } @@ -192,6 +196,7 @@ public final class ReadableDataTreeDelegator implements ReadableDataManager { listSchema.isUserOrdered() ? Builders.orderedMapBuilder() : Builders.mapBuilder(); + listBuilder.withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(listSchema.getQName())); for (NormalizedNode normalizedRootElement : normalizedRootElements) { listBuilder.withChild((MapEntryNode) normalizedRootElement); -- cgit 1.2.3-korg