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 +- .../io/fd/honeycomb/data/impl/DataBrokerTest.java | 71 +++++++++++++- .../data/impl/PersistingDataTreeAdapterTest.java | 68 +++++++++++++ .../data/impl/ReadableDataTreeDelegatorTest.java | 27 ++++++ 7 files changed, 238 insertions(+), 49 deletions(-) (limited to 'infra/data-impl/src') 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); diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/DataBrokerTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/DataBrokerTest.java index 08cb3320f..f1bc3105a 100644 --- a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/DataBrokerTest.java +++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/DataBrokerTest.java @@ -107,5 +107,74 @@ public class DataBrokerTest { assertTrue(supportedExtensions.isEmpty()); } - + public static class DataBrokerForContextTest { + + @Mock + private ModifiableDataManager contextDataTree; + @Mock + private DataModification contextSnapshot; + private DataBroker broker; + + @Before + public void setUp() { + initMocks(this); + when(contextDataTree.newModification()).thenReturn(contextSnapshot); + broker = DataBroker.create(contextDataTree); + } + + @Test + public void testNewReadWriteTransaction() { + final DOMDataReadWriteTransaction readWriteTx = broker.newReadWriteTransaction(); + final YangInstanceIdentifier path = mock(YangInstanceIdentifier.class); + readWriteTx.read(LogicalDatastoreType.OPERATIONAL, path); + + verify(contextSnapshot).read(path); + verify(contextDataTree).newModification(); + } + + @Test + public void testNewWriteOnlyTransaction() { + broker.newWriteOnlyTransaction(); + verify(contextDataTree).newModification(); + } + + @Test + public void testNewReadOnlyTransaction() { + final DOMDataReadOnlyTransaction readTx = broker.newReadOnlyTransaction(); + final YangInstanceIdentifier path = mock(YangInstanceIdentifier.class); + readTx.read(LogicalDatastoreType.OPERATIONAL, path); + + // operational data are read directly from data tree + verify(contextDataTree).read(path); + } + + @Test(expected = IllegalArgumentException.class) + public void testReadConfig() { + final DOMDataReadOnlyTransaction readTx = broker.newReadOnlyTransaction(); + + final YangInstanceIdentifier path = mock(YangInstanceIdentifier.class); + readTx.read(LogicalDatastoreType.CONFIGURATION, path); + } + + @Test(expected = UnsupportedOperationException.class) + public void testRegisterDataChangeListener() { + final YangInstanceIdentifier path = mock(YangInstanceIdentifier.class); + final DOMDataChangeListener listener = mock(DOMDataChangeListener.class); + broker.registerDataChangeListener(LogicalDatastoreType.OPERATIONAL, path, listener, + AsyncDataBroker.DataChangeScope.BASE); + } + + @Test(expected = UnsupportedOperationException.class) + public void testCreateTransactionChain() { + final TransactionChainListener listener = mock(TransactionChainListener.class); + broker.createTransactionChain(listener); + } + + @Test + public void testGetSupportedExtensions() { + final Map, DOMDataBrokerExtension> supportedExtensions = + broker.getSupportedExtensions(); + assertTrue(supportedExtensions.isEmpty()); + } + } } \ No newline at end of file diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/PersistingDataTreeAdapterTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/PersistingDataTreeAdapterTest.java index 305166421..8430073ac 100644 --- a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/PersistingDataTreeAdapterTest.java +++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/PersistingDataTreeAdapterTest.java @@ -16,12 +16,17 @@ package io.fd.honeycomb.data.impl; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import com.google.common.base.Optional; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import org.junit.Before; @@ -29,9 +34,11 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.opendaylight.controller.sal.core.api.model.SchemaService; +import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; public class PersistingDataTreeAdapterTest { @@ -41,6 +48,8 @@ public class PersistingDataTreeAdapterTest { private SchemaService schemaService; @Mock private DataTreeSnapshot snapshot; + @Mock + private PersistingDataTreeAdapter.JsonPersister persister; private Path tmpPersistFile; @@ -49,6 +58,8 @@ public class PersistingDataTreeAdapterTest { @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); + doReturn(snapshot).when(delegatingDataTree).takeSnapshot(); + doNothing().when(persister).persistCurrentData(any(Optional.class)); tmpPersistFile = Files.createTempFile("testing-hc-persistence", "json"); persistingDataTreeAdapter = new PersistingDataTreeAdapter(delegatingDataTree, schemaService, tmpPersistFile); } @@ -66,4 +77,61 @@ public class PersistingDataTreeAdapterTest { } } + @Test + public void testPersist() throws Exception { + persistingDataTreeAdapter = new PersistingDataTreeAdapter(delegatingDataTree, persister); + persistingDataTreeAdapter.commit(null); + verify(delegatingDataTree).takeSnapshot(); + verify(persister).persistCurrentData(any(Optional.class)); + } + + @Test + public void testTakeSnapshot() throws Exception { + persistingDataTreeAdapter.takeSnapshot(); + verify(delegatingDataTree).takeSnapshot(); + } + + @Test + public void testSetSchema() throws Exception { + persistingDataTreeAdapter.setSchemaContext(null); + verify(delegatingDataTree).setSchemaContext(null); + } + + @Test + public void testValidate() throws Exception { + persistingDataTreeAdapter.validate(null); + verify(delegatingDataTree).validate(null); + } + + @Test + public void testPrepare() throws Exception { + persistingDataTreeAdapter.prepare(null); + verify(delegatingDataTree).prepare(null); + } + + @Test + public void testGetRootPath() throws Exception { + persistingDataTreeAdapter.getRootPath(); + verify(delegatingDataTree).getRootPath(); + } + + @Test(expected = IllegalStateException.class) + public void testPersistFailure() throws Exception { + doThrow(IOException.class).when(schemaService).getGlobalContext(); + final PersistingDataTreeAdapter.JsonPersister jsonPersister = + new PersistingDataTreeAdapter.JsonPersister(tmpPersistFile, schemaService); + // Nothing + jsonPersister.persistCurrentData(Optional.absent()); + // Exception + jsonPersister.persistCurrentData(Optional.of(ImmutableNodes.leafNode(QName.create("namespace", "leaf"), "value"))); + } + + @Test + public void testPersisterCreateFile() throws Exception { + // Delete to test file creation + Files.delete(tmpPersistFile); + final PersistingDataTreeAdapter.JsonPersister jsonPersister = + new PersistingDataTreeAdapter.JsonPersister(tmpPersistFile, schemaService); + assertTrue(Files.exists(tmpPersistFile)); + } } \ No newline at end of file diff --git a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ReadableDataTreeDelegatorTest.java b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ReadableDataTreeDelegatorTest.java index 2b4cc2a06..e795fbe44 100644 --- a/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ReadableDataTreeDelegatorTest.java +++ b/infra/data-impl/src/test/java/io/fd/honeycomb/data/impl/ReadableDataTreeDelegatorTest.java @@ -16,6 +16,7 @@ package io.fd.honeycomb.data.impl; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -32,13 +33,16 @@ import static org.mockito.MockitoAnnotations.initMocks; import com.google.common.base.Optional; import com.google.common.collect.Iterables; import com.google.common.collect.LinkedListMultimap; +import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; import io.fd.honeycomb.translate.read.ReadContext; import io.fd.honeycomb.translate.read.registry.ReaderRegistry; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -48,6 +52,7 @@ import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; import org.opendaylight.controller.md.sal.dom.api.DOMDataBroker; import org.opendaylight.controller.md.sal.dom.api.DOMDataReadOnlyTransaction; import org.opendaylight.yangtools.binding.data.codec.api.BindingNormalizedNodeSerializer; +import org.opendaylight.yangtools.util.UnmodifiableCollection; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.common.QName; @@ -55,7 +60,9 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; +import org.opendaylight.yangtools.yang.model.api.ListSchemaNode; import org.opendaylight.yangtools.yang.model.api.SchemaContext; public class ReadableDataTreeDelegatorTest { @@ -189,4 +196,24 @@ public class ReadableDataTreeDelegatorTest { assertEquals(SchemaContext.NAME, rootNode.getIdentifier().getNodeType()); assertEquals(vppStateContainer, Iterables.getOnlyElement(rootNode.getValue())); } + + + @Test + public void testWrapMixin() throws Exception { + final QName nodeQName = QName.create("namespace", "node"); + final QName keyQName = QName.create("namespace", "key"); + final List> mapNodes = Lists.newArrayList("one", "two", "three").stream() + .map(value -> ImmutableNodes.mapEntry(nodeQName, keyQName, value)) + .collect(Collectors.toList()); + final ListSchemaNode listSchema = mock(ListSchemaNode.class); + doReturn(Collections.singletonList(keyQName)).when(listSchema).getKeyDefinition(); + doReturn(true).when(listSchema).isUserOrdered(); + doReturn(nodeQName).when(listSchema).getQName(); + + final DataContainerChild dataContainerChild = + ReadableDataTreeDelegator.wrapListIntoMixinNode(mapNodes, listSchema); + + // asserting as arrays, since UnmodifiableCollection has no equals + assertArrayEquals(mapNodes.toArray(), ((UnmodifiableCollection) dataContainerChild.getValue()).toArray()); + } } \ No newline at end of file -- cgit 1.2.3-korg