From eb7ab8e1bb6a28cfafd22a5a62ea66e5f85a8c2d Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 11 Mar 2022 07:48:14 +0100 Subject: [PATCH] Enforce InstanceIdentifier creation We currently allow an InstanceIdentifier to be created for any DataObject, which is wrong, as that includes raw RpcInputs, Notifications and the like. Update create() method to require proper relationship with DataRoot instances. Alternatives can be worked through instantiating a proper builder. Also enforce individual items to always have the proper type. JIRA: MDSAL-370 Change-Id: I38e83760c0ec29613a9ded24c0783bef940c9b7e Signed-off-by: Robert Varga --- .../binding/api/DataTreeIdentifierTest.java | 14 +++-- .../adapter/query/SimpleQueryExecutor.java | 4 +- ...ingDOMDataTreeCommitCohortAdapterTest.java | 8 +-- ...indingDOMMountPointServiceAdapterTest.java | 10 +-- ...DirectGetterRouteContextExtractorTest.java | 28 +++------ .../adapter/LazyDataTreeModificationTest.java | 4 +- .../binding/dom/adapter/Mdsal298Test.java | 20 +++--- .../codec/impl/ExceptionReportingTest.java | 3 +- .../binding/dom/codec/impl/Mdsal724Test.java | 23 +++---- .../impl/TopLevelContainerViaUsesTest.java | 8 +-- .../yang/binding/InstanceIdentifier.java | 62 +++++++++++++++---- .../yang/binding/InstanceIdentifierTest.java | 28 +++++---- .../yang/binding/test/mock/FooRoot.java | 16 +++++ .../yang/binding/test/mock/Nodes.java | 1 - .../mdsal/eos/binding/api/EntityTest.java | 18 +++--- 15 files changed, 148 insertions(+), 99 deletions(-) create mode 100644 binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/test/mock/FooRoot.java diff --git a/binding/mdsal-binding-api/src/test/java/org/opendaylight/mdsal/binding/api/DataTreeIdentifierTest.java b/binding/mdsal-binding-api/src/test/java/org/opendaylight/mdsal/binding/api/DataTreeIdentifierTest.java index 67553639a9..1f6879b9e0 100644 --- a/binding/mdsal-binding-api/src/test/java/org/opendaylight/mdsal/binding/api/DataTreeIdentifierTest.java +++ b/binding/mdsal-binding-api/src/test/java/org/opendaylight/mdsal/binding/api/DataTreeIdentifierTest.java @@ -14,7 +14,9 @@ import static org.junit.Assert.assertTrue; import org.junit.Test; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; +import org.opendaylight.yangtools.yang.binding.ChildOf; import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.DataRoot; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; public class DataTreeIdentifierTest { @@ -56,17 +58,17 @@ public class DataTreeIdentifierTest { assertFalse("Different object", TEST_IDENTIFIER1.equals(new Object())); } - private static class TestDataObject1 implements DataObject { + private interface TestDataObject1 extends ChildOf { @Override - public Class implementedInterface() { - return DataObject.class; + default Class implementedInterface() { + return TestDataObject1.class; } } - private static class TestDataObject2 implements DataObject { + private interface TestDataObject2 extends ChildOf { @Override - public Class implementedInterface() { - return DataObject.class; + default Class implementedInterface() { + return TestDataObject2.class; } } } diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/query/SimpleQueryExecutor.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/query/SimpleQueryExecutor.java index 5f85ed9b00..df281d07b2 100644 --- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/query/SimpleQueryExecutor.java +++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/query/SimpleQueryExecutor.java @@ -60,9 +60,9 @@ public final class SimpleQueryExecutor implements QueryExecutor { } public > @NonNull Builder add(final @NonNull T data) { - @SuppressWarnings("unchecked") + @SuppressWarnings({"rawtypes", "unchecked"}) final BindingDataObjectCodecTreeNode dataCodec = (BindingDataObjectCodecTreeNode) - codec.getSubtreeCodec(InstanceIdentifier.create(data.implementedInterface())); + codec.getSubtreeCodec(InstanceIdentifier.create((Class) data.implementedInterface())); rootBuilder.withChild((DataContainerChild) verifyNotNull(dataCodec).serialize(data)); return this; } diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMDataTreeCommitCohortAdapterTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMDataTreeCommitCohortAdapterTest.java index 6447d76447..89f1148b4b 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMDataTreeCommitCohortAdapterTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMDataTreeCommitCohortAdapterTest.java @@ -15,8 +15,8 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import java.util.Arrays; import java.util.Collection; +import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -28,7 +28,7 @@ import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.mdsal.common.api.PostCanCommitStep; import org.opendaylight.mdsal.dom.api.DOMDataTreeCandidate; import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier; -import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yang.gen.v1.urn.yang.foo.rev160101.BooleanContainer; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.tree.api.DataTreeCandidateNode; @@ -49,7 +49,7 @@ public class BindingDOMDataTreeCommitCohortAdapterTest { final DOMDataTreeCandidate domDataTreeCandidate = mock(DOMDataTreeCandidate.class); final DOMDataTreeIdentifier domDataTreeIdentifier = new DOMDataTreeIdentifier(LogicalDatastoreType.OPERATIONAL, YangInstanceIdentifier.empty()); - doReturn(InstanceIdentifier.create(DataObject.class)).when(registry).fromYangInstanceIdentifier(any()); + doReturn(InstanceIdentifier.create(BooleanContainer.class)).when(registry).fromYangInstanceIdentifier(any()); final BindingDataObjectCodecTreeNode bindingCodecTreeNode = mock(BindingDataObjectCodecTreeNode.class); doReturn(bindingCodecTreeNode).when(registry).getSubtreeCodec(any(InstanceIdentifier.class)); doReturn(domDataTreeIdentifier).when(domDataTreeCandidate).getRootPath(); @@ -59,7 +59,7 @@ public class BindingDOMDataTreeCommitCohortAdapterTest { final Object txId = new Object(); doReturn(PostCanCommitStep.NOOP_SUCCESSFUL_FUTURE).when(cohort).canCommit(any(), any()); - adapter.canCommit(txId, null, Arrays.asList(domDataTreeCandidate, domDataTreeCandidate)); + adapter.canCommit(txId, null, List.of(domDataTreeCandidate, domDataTreeCandidate)); ArgumentCaptor modifications = ArgumentCaptor.forClass(Collection.class); verify(cohort).canCommit(eq(txId), modifications.capture()); assertEquals(2, modifications.getValue().size()); diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMMountPointServiceAdapterTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMMountPointServiceAdapterTest.java index c09f8d67ac..bda9511217 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMMountPointServiceAdapterTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/BindingDOMMountPointServiceAdapterTest.java @@ -20,7 +20,7 @@ import org.opendaylight.mdsal.binding.api.MountPointService.MountPointListener; import org.opendaylight.mdsal.binding.dom.codec.spi.BindingDOMCodecServices; import org.opendaylight.mdsal.dom.api.DOMMountPoint; import org.opendaylight.mdsal.dom.api.DOMMountPointService; -import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yang.gen.v1.urn.yang.foo.rev160101.BooleanContainer; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; @@ -36,12 +36,12 @@ public class BindingDOMMountPointServiceAdapterTest { new BindingDOMMountPointServiceAdapter(codec, mountPointService); doReturn(Optional.empty()).when(mountPointService).getMountPoint(any()); - assertFalse(adapter.getMountPoint(InstanceIdentifier.create(DataObject.class)).isPresent()); + assertFalse(adapter.getMountPoint(InstanceIdentifier.create(BooleanContainer.class)).isPresent()); doReturn(Optional.of(mock(DOMMountPoint.class))).when(mountPointService).getMountPoint(any()); - assertTrue(adapter.getMountPoint(InstanceIdentifier.create(DataObject.class)).isPresent()); + assertTrue(adapter.getMountPoint(InstanceIdentifier.create(BooleanContainer.class)).isPresent()); - assertNotNull(adapter.registerListener(InstanceIdentifier.create(DataObject.class), - mock(MountPointListener.class))); + assertNotNull(adapter.registerListener(InstanceIdentifier.create(BooleanContainer.class), + mock(MountPointListener.class))); } } \ No newline at end of file diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/DirectGetterRouteContextExtractorTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/DirectGetterRouteContextExtractorTest.java index 2a9efeb6b6..317c6a7972 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/DirectGetterRouteContextExtractorTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/DirectGetterRouteContextExtractorTest.java @@ -8,36 +8,28 @@ package org.opendaylight.mdsal.binding.dom.adapter; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.mock; -import java.lang.reflect.Method; import org.junit.Test; -import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yang.gen.v1.urn.yang.foo.rev160101.BooleanContainer; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; public class DirectGetterRouteContextExtractorTest { - - private static final InstanceIdentifier INSTANCE_IDENTIFIER = InstanceIdentifier.create(DataObject.class); + private static final InstanceIdentifier INSTANCE_IDENTIFIER = InstanceIdentifier.create(BooleanContainer.class); private static final String EXCEPTION_TEXT = "testException"; @Test - public void basicTest() throws Exception { - final Method testMthd = this.getClass().getDeclaredMethod("testMethod", DataObject.class); - testMthd.setAccessible(true); - final ContextReferenceExtractor referenceExtractor = DirectGetterRouteContextExtractor.create(testMthd); - assertEquals(testMethod(mock(DataObject.class)), referenceExtractor.extract(mock(DataObject.class))); + public void basicTest() throws IllegalAccessException, NoSuchMethodException { + final ContextReferenceExtractor referenceExtractor = DirectGetterRouteContextExtractor.create( + getClass().getDeclaredMethod("testMethod", BooleanContainer.class)); + assertEquals(INSTANCE_IDENTIFIER, referenceExtractor.extract(mock(BooleanContainer.class))); - try { - referenceExtractor.extract(null); - fail("Expected exception"); - } catch (NullPointerException e) { - assertTrue(e.getMessage().equals(EXCEPTION_TEXT)); - } + assertEquals(EXCEPTION_TEXT, + assertThrows(NullPointerException.class, () -> referenceExtractor.extract(null)).getMessage()); } - private static InstanceIdentifier testMethod(final DataObject data) { + public static InstanceIdentifier testMethod(final BooleanContainer data) { if (data == null) { throw new NullPointerException(EXCEPTION_TEXT); } diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/LazyDataTreeModificationTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/LazyDataTreeModificationTest.java index 8812f7b3ef..9a7b859184 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/LazyDataTreeModificationTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/LazyDataTreeModificationTest.java @@ -18,7 +18,7 @@ import org.opendaylight.mdsal.binding.dom.codec.spi.BindingDOMCodecServices; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.mdsal.dom.api.DOMDataTreeCandidate; import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier; -import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yang.gen.v1.urn.yang.foo.rev160101.BooleanContainer; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.tree.api.DataTreeCandidateNode; @@ -31,7 +31,7 @@ public class LazyDataTreeModificationTest { final DOMDataTreeCandidate domDataTreeCandidate = mock(DOMDataTreeCandidate.class); final DOMDataTreeIdentifier domDataTreeIdentifier = new DOMDataTreeIdentifier(LogicalDatastoreType.OPERATIONAL, YangInstanceIdentifier.empty()); - doReturn(InstanceIdentifier.create(DataObject.class)).when(registry).fromYangInstanceIdentifier(any()); + doReturn(InstanceIdentifier.create(BooleanContainer.class)).when(registry).fromYangInstanceIdentifier(any()); final BindingDataObjectCodecTreeNode bindingCodecTreeNode = mock(BindingDataObjectCodecTreeNode.class); doReturn(bindingCodecTreeNode).when(registry).getSubtreeCodec(any(InstanceIdentifier.class)); doReturn(domDataTreeIdentifier).when(domDataTreeCandidate).getRootPath(); diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Mdsal298Test.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Mdsal298Test.java index 9b8b00743b..63dc96265b 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Mdsal298Test.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Mdsal298Test.java @@ -15,10 +15,9 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.opendaylight.mdsal.common.api.LogicalDatastoreType.CONFIGURATION; -import com.google.common.collect.ImmutableList; import java.util.Collection; -import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.concurrent.ExecutionException; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -48,7 +47,8 @@ import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.con import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.with.choice.Foo; import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.with.choice.foo.addressable._case.Addressable; import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.with.choice.foo.addressable._case.AddressableBuilder; -import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.ChildOf; +import org.opendaylight.yangtools.yang.binding.DataRoot; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.Item; import org.opendaylight.yangtools.yang.common.QName; @@ -119,7 +119,7 @@ public class Mdsal298Test extends AbstractDataBrokerTest { final Container containerAfter = changedContainer.getDataAfter(); assertEquals(new ContainerBuilder() - .setKeyed(ImmutableList.of( + .setKeyed(List.of( new KeyedBuilder().setFoo("foo").withKey(new KeyedKey("foo")).build(), new KeyedBuilder().setFoo("bar").withKey(new KeyedKey("bar")).build())) .build(), containerAfter); @@ -130,13 +130,13 @@ public class Mdsal298Test extends AbstractDataBrokerTest { final Iterator> it = changedChildren.iterator(); final DataObjectModification changedChild1 = it.next(); assertEquals(ModificationType.WRITE, changedChild1.getModificationType()); - assertEquals(Collections.emptyList(), changedChild1.getModifiedChildren()); + assertEquals(List.of(), changedChild1.getModifiedChildren()); final Keyed child1After = (Keyed) changedChild1.getDataAfter(); assertEquals("foo", child1After.getFoo()); final DataObjectModification changedChild2 = it.next(); assertEquals(ModificationType.WRITE, changedChild2.getModificationType()); - assertEquals(Collections.emptyList(), changedChild2.getModifiedChildren()); + assertEquals(List.of(), changedChild2.getModifiedChildren()); final Keyed child2After = (Keyed) changedChild2.getDataAfter(); assertEquals("bar", child2After.getFoo()); } @@ -174,7 +174,7 @@ public class Mdsal298Test extends AbstractDataBrokerTest { final Container containerAfter = changedContainer.getDataAfter(); assertEquals(new ContainerBuilder() - .setUnkeyed(ImmutableList.of( + .setUnkeyed(List.of( new UnkeyedBuilder().setFoo("foo").build(), new UnkeyedBuilder().setFoo("bar").build())) .build(), containerAfter); @@ -309,7 +309,7 @@ public class Mdsal298Test extends AbstractDataBrokerTest { assertEquals(0, choiceChildren.size()); } - private DataTreeChangeListener assertWrittenContainer(final QName qname, + private > DataTreeChangeListener assertWrittenContainer(final QName qname, final Class bindingClass, final T expected) throws InterruptedException, ExecutionException { final DataTreeChangeListener listener = mock(DataTreeChangeListener.class); @@ -339,7 +339,7 @@ public class Mdsal298Test extends AbstractDataBrokerTest { assertEquals(expected, containerAfter); // No further modifications should occur - assertEquals(Collections.emptyList(), changedContainer.getModifiedChildren()); + assertEquals(List.of(), changedContainer.getModifiedChildren()); reset(listener); doNothing().when(listener).onDataTreeChanged(any(Collection.class)); @@ -375,7 +375,7 @@ public class Mdsal298Test extends AbstractDataBrokerTest { assertEquals(new WithChoiceBuilder().build(), containerAfter); // No further modifications should occur - assertEquals(Collections.emptyList(), changedContainer.getModifiedChildren()); + assertEquals(List.of(), changedContainer.getModifiedChildren()); reset(listener); doNothing().when(listener).onDataTreeChanged(any(Collection.class)); diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/ExceptionReportingTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/ExceptionReportingTest.java index 7bfc122b8b..2e0de327aa 100644 --- a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/ExceptionReportingTest.java +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/ExceptionReportingTest.java @@ -64,7 +64,8 @@ public class ExceptionReportingTest { @Test public void testBindingSkippedRoot() { - final var iid = InstanceIdentifier.create(TopLevelList.class); + @SuppressWarnings({"unchecked", "rawtypes"}) + final var iid = InstanceIdentifier.create((Class) TopLevelList.class); assertThrows(IncorrectNestingException.class, () -> FULL_CODEC.toYangInstanceIdentifier(iid)); } diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java index 8f72441c8b..15fa42814b 100644 --- a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java @@ -11,31 +11,28 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import org.junit.Test; -import org.opendaylight.mdsal.binding.dom.codec.api.IncorrectNestingException; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.bi.ba.notification.rev150205.OutOfPixieDustNotification; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.md.sal.knock.knock.rev180723.KnockKnockInput; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; -public class Mdsal724Test extends AbstractBindingCodecTest { +public class Mdsal724Test { @Test + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testNotificationInstanceIdentifier() { // An InstanceIdentifier pointing at a notification, unsafe to create - @SuppressWarnings({ "rawtypes", "unchecked" }) - final var iid = InstanceIdentifier.create((Class) OutOfPixieDustNotification.class); - - final var ex = assertThrows(IllegalArgumentException.class, () -> codecContext.toYangInstanceIdentifier(iid)); + final var ex = assertThrows(IllegalArgumentException.class, + () -> InstanceIdentifier.create((Class) OutOfPixieDustNotification.class)); assertEquals("interface org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test" - + ".bi.ba.notification.rev150205.OutOfPixieDustNotification is not a valid data tree child of " - + "SchemaRootCodecContext [interface org.opendaylight.yangtools.yang.binding.DataRoot]", ex.getMessage()); + + ".bi.ba.notification.rev150205.OutOfPixieDustNotification is not a valid path argument", ex.getMessage()); } @Test + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testRpcInputInstanceIdentifier() { // An InstanceIdentifier pointing at a notification, unsafe to create - @SuppressWarnings({ "rawtypes", "unchecked" }) - final var iid = InstanceIdentifier.create((Class) KnockKnockInput.class); - final var ex = assertThrows(IncorrectNestingException.class, () -> codecContext.toYangInstanceIdentifier(iid)); - assertEquals("interface org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.md.sal.knock.knock." - + "rev180723.KnockKnockInput is not top-level item.", ex.getMessage()); + final var ex = assertThrows(IllegalArgumentException.class, + () -> InstanceIdentifier.create((Class) KnockKnockInput.class)); + assertEquals("interface org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.md.sal.knock.knock" + + ".rev180723.KnockKnockInput is not a valid path argument", ex.getMessage()); } } diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/TopLevelContainerViaUsesTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/TopLevelContainerViaUsesTest.java index 65d6452710..db0797b911 100644 --- a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/TopLevelContainerViaUsesTest.java +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/TopLevelContainerViaUsesTest.java @@ -10,15 +10,15 @@ package org.opendaylight.mdsal.binding.dom.codec.impl; import static org.junit.Assert.assertEquals; import org.junit.Test; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.md.sal.test.top.via.uses.rev151112.OpendaylightBindingTopLevelViaUsesData; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.md.sal.test.top.via.uses.rev151112.container.top.ContainerTop; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; public class TopLevelContainerViaUsesTest extends AbstractBindingCodecTest { - private static final InstanceIdentifier TOP_LEVEL_CONTAINER_FROM_USES = - InstanceIdentifier.create(ContainerTop.class); + InstanceIdentifier.builderOfInherited(OpendaylightBindingTopLevelViaUsesData.class, ContainerTop.class).build(); @Test public void testBindingToDomFirst() { @@ -27,12 +27,10 @@ public class TopLevelContainerViaUsesTest extends AbstractBindingCodecTest { assertEquals(ContainerTop.QNAME, lastArg.getNodeType()); } - @Test public void testDomToBindingFirst() { final YangInstanceIdentifier yangII = YangInstanceIdentifier.of(ContainerTop.QNAME); - InstanceIdentifier bindingII = codecContext.fromYangInstanceIdentifier(yangII); + final InstanceIdentifier bindingII = codecContext.fromYangInstanceIdentifier(yangII); assertEquals(TOP_LEVEL_CONTAINER_FROM_USES, bindingII); } - } diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifier.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifier.java index 416ce36b3d..0c89142533 100644 --- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifier.java +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifier.java @@ -9,6 +9,7 @@ package org.opendaylight.yangtools.yang.binding; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Verify.verify; +import static com.google.common.base.Verify.verifyNotNull; import static java.util.Objects.requireNonNull; import com.google.common.base.MoreObjects; @@ -419,6 +420,7 @@ public class InstanceIdentifier * * @return A builder instance */ + // FIXME: rename this method to 'toBuilder()' public @NonNull InstanceIdentifierBuilder builder() { return new InstanceIdentifierBuilderImpl<>(Item.of(targetType), pathArguments, hash, isWildcarded()); } @@ -489,6 +491,36 @@ public class InstanceIdentifier return new InstanceIdentifierBuilderImpl().addNode(IdentifiableItem.of(caze, listItem, listKey)); } + public static > + @NonNull InstanceIdentifierBuilder builderOfInherited(final Class root, final Class container) { + // FIXME: we are losing root identity, hence namespaces may not work correctly + return new InstanceIdentifierBuilderImpl().addWildNode(Item.of(container)); + } + + public static & DataObject, + T extends ChildOf> + @NonNull InstanceIdentifierBuilder builderOfInherited(final Class root, + final Class caze, final Class container) { + // FIXME: we are losing root identity, hence namespaces may not work correctly + return new InstanceIdentifierBuilderImpl().addWildNode(Item.of(caze, container)); + } + + public static & ChildOf, + K extends Identifier> + @NonNull InstanceIdentifierBuilder builderOfInherited(final Class root, + final Class listItem, final K listKey) { + // FIXME: we are losing root identity, hence namespaces may not work correctly + return new InstanceIdentifierBuilderImpl().addNode(IdentifiableItem.of(listItem, listKey)); + } + + public static & DataObject, + N extends Identifiable & ChildOf, K extends Identifier> + @NonNull InstanceIdentifierBuilder builderOfInherited(final Class root, + final Class caze, final Class listItem, final K listKey) { + // FIXME: we are losing root identity, hence namespaces may not work correctly + return new InstanceIdentifierBuilderImpl().addNode(IdentifiableItem.of(caze, listItem, listKey)); + } + /** * Create an instance identifier for a very specific object type. This method implements {@link #create(Iterable)} * semantics, except it is used by internal callers, which have assured that the argument is an immutable Iterable. @@ -499,24 +531,26 @@ public class InstanceIdentifier * @throws NullPointerException if {@code pathArguments} is null */ private static @NonNull InstanceIdentifier internalCreate(final Iterable pathArguments) { - final Iterator it = requireNonNull(pathArguments, "pathArguments may not be null") - .iterator(); + final var it = requireNonNull(pathArguments, "pathArguments may not be null").iterator(); + checkArgument(it.hasNext(), "pathArguments may not be empty"); + final HashCodeBuilder hashBuilder = new HashCodeBuilder<>(); boolean wildcard = false; - PathArgument arg = null; + PathArgument arg; - while (it.hasNext()) { + do { arg = it.next(); - checkArgument(arg != null, "pathArguments may not contain null elements"); + // Non-null is implied by our callers + final var type = verifyNotNull(arg).getType(); + checkArgument(ChildOf.class.isAssignableFrom(type) || Augmentation.class.isAssignableFrom(type), + "%s is not a valid path argument", type); - // TODO: sanity check ChildOf<>; hashBuilder.addArgument(arg); - if (Identifiable.class.isAssignableFrom(arg.getType()) && !(arg instanceof IdentifiableItem)) { + if (Identifiable.class.isAssignableFrom(type) && !(arg instanceof IdentifiableItem)) { wildcard = true; } - } - checkArgument(arg != null, "pathArguments may not be empty"); + } while (it.hasNext()); return trustedCreate(arg, pathArguments, hashBuilder.build(), wildcard); } @@ -536,6 +570,7 @@ public class InstanceIdentifier * @throws IllegalArgumentException if pathArguments is empty or * contains a null element. */ + // FIXME: rename to 'unsafeOf()' public static @NonNull InstanceIdentifier create(final Iterable pathArguments) { if (pathArguments instanceof ImmutableCollection) { @SuppressWarnings("unchecked") @@ -559,9 +594,11 @@ public class InstanceIdentifier * @param type The type of the object which this instance identifier represents * @return InstanceIdentifier instance */ + // FIXME: considering removing in favor of always going through a builder @SuppressWarnings("unchecked") - public static @NonNull InstanceIdentifier create(final Class<@NonNull T> type) { - return (InstanceIdentifier) create(ImmutableList.of(Item.of(type))); + public static > @NonNull InstanceIdentifier create( + final Class<@NonNull T> type) { + return (InstanceIdentifier) internalCreate(ImmutableList.of(Item.of(type))); } /** @@ -572,6 +609,7 @@ public class InstanceIdentifier * @throws IllegalArgumentException if the supplied identifier type cannot have a key. * @throws NullPointerException if id is null. */ + // FIXME: reconsider naming and design of this method public static & DataObject, K extends Identifier> K keyOf( final InstanceIdentifier id) { requireNonNull(id); @@ -826,6 +864,8 @@ public class InstanceIdentifier } } + // FIXME: rename to 'Builder' + // FIXME: introduce KeyedBuilder with specialized build() method public interface InstanceIdentifierBuilder { /** * Append the specified container as a child of the current InstanceIdentifier referenced by the builder. This diff --git a/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifierTest.java b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifierTest.java index 5148337464..1270d340ea 100644 --- a/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifierTest.java +++ b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifierTest.java @@ -20,11 +20,13 @@ import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import org.junit.Test; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.InstanceIdentifierBuilder; import org.opendaylight.yangtools.yang.binding.test.mock.FooChild; +import org.opendaylight.yangtools.yang.binding.test.mock.FooRoot; import org.opendaylight.yangtools.yang.binding.test.mock.InstantiatedFoo; import org.opendaylight.yangtools.yang.binding.test.mock.Node; import org.opendaylight.yangtools.yang.binding.test.mock.NodeAugmentation; @@ -123,10 +125,10 @@ public class InstanceIdentifierTest { } @Test - public void basicTests() throws Exception { - final InstanceIdentifier instanceIdentifier1 = InstanceIdentifier.create(DataObject.class); - final InstanceIdentifier instanceIdentifier2 = InstanceIdentifier.create(DataObject.class); - final InstanceIdentifier instanceIdentifier4 = InstanceIdentifier.create(DataObject.class); + public void basicTests() { + final InstanceIdentifier instanceIdentifier1 = InstanceIdentifier.create(FooRoot.class); + final InstanceIdentifier instanceIdentifier2 = InstanceIdentifier.create(FooRoot.class); + final InstanceIdentifier instanceIdentifier4 = InstanceIdentifier.create(FooRoot.class); final InstanceIdentifier instanceIdentifier3 = InstanceIdentifier.builder(Nodes.class) .child(Node.class, new NodeKey(10)).child(NodeChild.class).build(); final Object object = new Object(); @@ -143,7 +145,7 @@ public class InstanceIdentifierTest { assertFalse(instanceIdentifier1.equals(instanceIdentifier3)); assertFalse(instanceIdentifier1.equals(instanceIdentifier4)); - final InstanceIdentifier instanceIdentifier5 = InstanceIdentifier.create(Node.class); + final InstanceIdentifier instanceIdentifier5 = InstanceIdentifier.create(Nodes.class).child(Node.class); Whitebox.setInternalState(instanceIdentifier5, "hash", instanceIdentifier1.hashCode()); Whitebox.setInternalState(instanceIdentifier5, "wildcarded", false); @@ -175,24 +177,24 @@ public class InstanceIdentifierTest { public void firstKeyOfTest() { final InstanceIdentifier instanceIdentifier = InstanceIdentifier.builder(Nodes.class).child(Node.class, new NodeKey(10)).build(); - final InstanceIdentifier instanceIdentifier1 = InstanceIdentifier.create(DataObject.class); + final InstanceIdentifier instanceIdentifier1 = InstanceIdentifier.create(FooRoot.class); assertNotNull(instanceIdentifier.firstKeyOf(Node.class)); assertNull(instanceIdentifier1.firstKeyOf(Node.class)); } @Test - public void keyOfTest() throws Exception { + public void keyOfTest() { final Identifier identifier = mock(Identifier.class); assertEquals(identifier, InstanceIdentifier.keyOf( new KeyedInstanceIdentifier(Identifiable.class, ImmutableList.of(), false, 0, identifier))); } @Test - public void serializationTest() throws Exception { + public void serializationTest() throws IOException, ClassNotFoundException { final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); final ObjectOutputStream outputStream = new ObjectOutputStream(byteArrayOutputStream); - final InstanceIdentifier instanceIdentifier = InstanceIdentifier.create(DataObject.class); + final InstanceIdentifier instanceIdentifier = InstanceIdentifier.create(FooRoot.class); outputStream.writeObject(instanceIdentifier); outputStream.flush(); outputStream.close(); @@ -206,8 +208,8 @@ public class InstanceIdentifierTest { @Test public void equalsTest() { - final InstanceIdentifierBuilder builder1 = InstanceIdentifier.create(DataObject.class).builder(); - final InstanceIdentifierBuilder builder2 = InstanceIdentifier.create(DataObject.class).builder(); + final InstanceIdentifierBuilder builder1 = InstanceIdentifier.create(FooRoot.class).builder(); + final InstanceIdentifierBuilder builder2 = InstanceIdentifier.create(FooRoot.class).builder(); final InstanceIdentifierBuilder builder3 = InstanceIdentifier.create(Nodes.class).builder(); final InstanceIdentifierBuilder builder4 = InstanceIdentifier.create(Nodes.class).builder(); final Object obj = new Object(); @@ -241,8 +243,8 @@ public class InstanceIdentifierTest { @Test public void hashCodeTest() { - final InstanceIdentifierBuilder builder1 = InstanceIdentifier.create(DataObject.class).builder(); - final InstanceIdentifierBuilder builder2 = InstanceIdentifier.create(DataObject.class).builder(); + final InstanceIdentifierBuilder builder1 = InstanceIdentifier.create(FooRoot.class).builder(); + final InstanceIdentifierBuilder builder2 = InstanceIdentifier.create(FooRoot.class).builder(); final InstanceIdentifierBuilder builder3 = InstanceIdentifier.create(Nodes.class).builder(); final InstanceIdentifierBuilder builder4 = InstanceIdentifier.create(Nodes.class).builder(); final Object obj = new Object(); diff --git a/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/test/mock/FooRoot.java b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/test/mock/FooRoot.java new file mode 100644 index 0000000000..a109968c31 --- /dev/null +++ b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/test/mock/FooRoot.java @@ -0,0 +1,16 @@ +/* + * Copyright (c) 2014 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.yangtools.yang.binding.test.mock; + +import org.opendaylight.yangtools.yang.binding.ChildOf; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.DataRoot; + +public interface FooRoot extends ChildOf, DataObject { + +} diff --git a/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/test/mock/Nodes.java b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/test/mock/Nodes.java index 1fddcb1e9d..4354d0e2b8 100644 --- a/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/test/mock/Nodes.java +++ b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/test/mock/Nodes.java @@ -13,5 +13,4 @@ import org.opendaylight.yangtools.yang.binding.DataRoot; public interface Nodes extends ChildOf, DataObject { - Iterable getNode(); } diff --git a/entityownership/mdsal-eos-binding-api/src/test/java/org/opendaylight/mdsal/eos/binding/api/EntityTest.java b/entityownership/mdsal-eos-binding-api/src/test/java/org/opendaylight/mdsal/eos/binding/api/EntityTest.java index c015cb89b6..f7acd9901a 100644 --- a/entityownership/mdsal-eos-binding-api/src/test/java/org/opendaylight/mdsal/eos/binding/api/EntityTest.java +++ b/entityownership/mdsal-eos-binding-api/src/test/java/org/opendaylight/mdsal/eos/binding/api/EntityTest.java @@ -13,7 +13,9 @@ import static org.junit.Assert.assertNotNull; import org.apache.commons.lang3.SerializationUtils; import org.junit.Test; +import org.opendaylight.yangtools.yang.binding.ChildOf; import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.DataRoot; import org.opendaylight.yangtools.yang.binding.Identifier; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; @@ -24,8 +26,8 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; */ public class EntityTest { - static String ENTITY_TYPE1 = "type1"; - static String ENTITY_TYPE2 = "type2"; + static final String ENTITY_TYPE1 = "type1"; + static final String ENTITY_TYPE2 = "type2"; static final InstanceIdentifier ID1 = InstanceIdentifier.create(TestDataObject1.class); static final InstanceIdentifier ID2 = InstanceIdentifier.create(TestDataObject2.class); @@ -70,17 +72,17 @@ public class EntityTest { assertNotNull("List key not found", keyID); } - static class TestDataObject1 implements DataObject { + interface TestDataObject1 extends ChildOf { @Override - public Class implementedInterface() { - return DataObject.class; + default Class implementedInterface() { + return TestDataObject1.class; } } - static class TestDataObject2 implements DataObject { + interface TestDataObject2 extends ChildOf { @Override - public Class implementedInterface() { - return DataObject.class; + default Class implementedInterface() { + return TestDataObject2.class; } } } -- 2.36.6