Improve InstanceIdentifierBuilderImpl safety

Add a constructor which explicitly takes (Identifiable)Item, removing the
need to use type arguments and hiding the intermediate empty constructor
as well as the methods to add items.

Change-Id: Ib45485b0ab3b10ab0666b4ad6797dc710cd00130
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

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 d3e476cf835f1365aa357d60d66f4fb42a31c9d1..6d17f1e8c6588aebb10c7e84d4e8c29706fe6cda 100644 (file)
--- 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
@@ -439,7 +439,7 @@ public class InstanceIdentifier<T extends DataObject>
      */
     public static <T extends ChildOf<? extends DataRoot>> @NonNull InstanceIdentifierBuilder<T> builder(
             final Class<T> container) {
-        return new InstanceIdentifierBuilderImpl<T>().addWildNode(Item.of(container));
+        return new InstanceIdentifierBuilderImpl<>(Item.of(container));
     }
 
     /**
@@ -455,7 +455,7 @@ public class InstanceIdentifier<T extends DataObject>
      */
     public static <C extends ChoiceIn<? extends DataRoot> & DataObject, T extends ChildOf<? super C>>
             @NonNull InstanceIdentifierBuilder<T> builder(final Class<C> caze, final Class<T> container) {
-        return new InstanceIdentifierBuilderImpl<T>().addWildNode(Item.of(caze, container));
+        return new InstanceIdentifierBuilderImpl<>(Item.of(caze, container));
     }
 
     /**
@@ -472,7 +472,7 @@ public class InstanceIdentifier<T extends DataObject>
     public static <N extends Identifiable<K> & ChildOf<? extends DataRoot>,
             K extends Identifier<N>> @NonNull InstanceIdentifierBuilder<N> builder(final Class<N> listItem,
                     final K listKey) {
-        return new InstanceIdentifierBuilderImpl<N>().addNode(IdentifiableItem.of(listItem, listKey));
+        return new InstanceIdentifierBuilderImpl<>(IdentifiableItem.of(listItem, listKey));
     }
 
     /**
@@ -492,13 +492,13 @@ public class InstanceIdentifier<T extends DataObject>
             N extends Identifiable<K> & ChildOf<? super C>, K extends Identifier<N>>
             @NonNull InstanceIdentifierBuilder<N> builder(final Class<C> caze, final Class<N> listItem,
                     final K listKey) {
-        return new InstanceIdentifierBuilderImpl<N>().addNode(IdentifiableItem.of(caze, listItem, listKey));
+        return new InstanceIdentifierBuilderImpl<>(IdentifiableItem.of(caze, listItem, listKey));
     }
 
     public static <R extends DataRoot & DataObject, T extends ChildOf<? super R>>
             @NonNull InstanceIdentifierBuilder<T> builderOfInherited(final Class<R> root, final Class<T> container) {
         // FIXME: we are losing root identity, hence namespaces may not work correctly
-        return new InstanceIdentifierBuilderImpl<T>().addWildNode(Item.of(container));
+        return new InstanceIdentifierBuilderImpl<>(Item.of(container));
     }
 
     public static <R extends DataRoot & DataObject, C extends ChoiceIn<? super R> & DataObject,
@@ -506,7 +506,7 @@ public class InstanceIdentifier<T extends DataObject>
             @NonNull InstanceIdentifierBuilder<T> builderOfInherited(final Class<R> root,
                 final Class<C> caze, final Class<T> container) {
         // FIXME: we are losing root identity, hence namespaces may not work correctly
-        return new InstanceIdentifierBuilderImpl<T>().addWildNode(Item.of(caze, container));
+        return new InstanceIdentifierBuilderImpl<>(Item.of(caze, container));
     }
 
     public static <R extends DataRoot & DataObject, N extends Identifiable<K> & ChildOf<? super R>,
@@ -514,7 +514,7 @@ public class InstanceIdentifier<T extends DataObject>
             @NonNull InstanceIdentifierBuilder<N> builderOfInherited(final Class<R> root,
                 final Class<N> listItem, final K listKey) {
         // FIXME: we are losing root identity, hence namespaces may not work correctly
-        return new InstanceIdentifierBuilderImpl<N>().addNode(IdentifiableItem.of(listItem, listKey));
+        return new InstanceIdentifierBuilderImpl<>(IdentifiableItem.of(listItem, listKey));
     }
 
     public static <R extends DataRoot & DataObject, C extends ChoiceIn<? super R> & DataObject,
@@ -522,7 +522,7 @@ public class InstanceIdentifier<T extends DataObject>
             @NonNull InstanceIdentifierBuilder<N> builderOfInherited(final Class<R> root,
                 final Class<C> caze, final Class<N> listItem, final K listKey) {
         // FIXME: we are losing root identity, hence namespaces may not work correctly
-        return new InstanceIdentifierBuilderImpl<N>().addNode(IdentifiableItem.of(caze, listItem, listKey));
+        return new InstanceIdentifierBuilderImpl<>(IdentifiableItem.of(caze, listItem, listKey));
     }
 
     /**

diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifierBuilderImpl.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifierBuilderImpl.java
index bb207ff52ad221bbe8162562f5a1d1d5dd1b427d..c097fdcc422dcc590ac52b845e01a9e27ee79dd2 100644 (file)
--- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifierBuilderImpl.java
+++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifierBuilderImpl.java
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.yangtools.yang.binding;
 
+import static java.util.Objects.requireNonNull;
+
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
@@ -22,20 +24,35 @@ final class InstanceIdentifierBuilderImpl<T extends DataObject> implements Insta
     private final ImmutableList.Builder<PathArgument> pathBuilder = ImmutableList.builder();
     private final HashCodeBuilder<PathArgument> hashBuilder;
     private final Iterable<? extends PathArgument> basePath;
-    private boolean wildcard = false;
-    private PathArgument arg = null;
+    private boolean wildcard;
+    private PathArgument arg;
+
+    private InstanceIdentifierBuilderImpl(final PathArgument item) {
+        hashBuilder = new HashCodeBuilder<>();
+        basePath = null;
+        arg = requireNonNull(item);
+        hashBuilder.addArgument(item);
+        pathBuilder.add(item);
+        wildcard = false;
+    }
+
+    InstanceIdentifierBuilderImpl(final IdentifiableItem<? super T, ?> item) {
+        this((PathArgument) item);
+    }
 
-    InstanceIdentifierBuilderImpl() {
-        this.hashBuilder = new HashCodeBuilder<>();
-        this.basePath = null;
+    InstanceIdentifierBuilderImpl(final Item<T> item) {
+        this((PathArgument) item);
+        if (Identifiable.class.isAssignableFrom(item.getType())) {
+            wildcard = true;
+        }
     }
 
     InstanceIdentifierBuilderImpl(final PathArgument item, final Iterable<? extends PathArgument> pathArguments,
             final int hash, final boolean wildcard) {
-        this.hashBuilder = new HashCodeBuilder<>(hash);
-        this.basePath = pathArguments;
+        hashBuilder = new HashCodeBuilder<>(hash);
+        basePath = pathArguments;
         this.wildcard = wildcard;
-        this.arg = item;
+        arg = item;
     }
 
     @Override
@@ -110,7 +127,7 @@ final class InstanceIdentifierBuilderImpl<T extends DataObject> implements Insta
         return InstanceIdentifier.trustedCreate(arg, pathArguments, hashBuilder.build(), wildcard);
     }
 
-    <N extends DataObject> @NonNull InstanceIdentifierBuilderImpl<N> addWildNode(final PathArgument newArg) {
+    private <N extends DataObject> @NonNull InstanceIdentifierBuilderImpl<N> addWildNode(final PathArgument newArg) {
         if (Identifiable.class.isAssignableFrom(newArg.getType())) {
             wildcard = true;
         }
@@ -118,7 +135,7 @@ final class InstanceIdentifierBuilderImpl<T extends DataObject> implements Insta
     }
 
     @SuppressWarnings("unchecked")
-    <N extends DataObject> @NonNull InstanceIdentifierBuilderImpl<N> addNode(final PathArgument newArg) {
+    private <N extends DataObject> @NonNull InstanceIdentifierBuilderImpl<N> addNode(final PathArgument newArg) {
         arg = newArg;
         hashBuilder.addArgument(newArg);
         pathBuilder.add(newArg);