From: Robert Varga Date: Thu, 11 Oct 2018 19:13:53 +0000 (+0200) Subject: Enable spotbugs in mdsal-binding-dom-codec X-Git-Tag: v3.0.1~11 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=ae568d740b27aa53316b15e72430952fef905516;p=mdsal.git Enable spotbugs in mdsal-binding-dom-codec This fixes up the issues reported and flips enforcement to on. Change-Id: I7e5b5c20a8b9f1ce3de42aac512ac3b31779821f Signed-off-by: Robert Varga --- diff --git a/binding/mdsal-binding-dom-codec/pom.xml b/binding/mdsal-binding-dom-codec/pom.xml index 27c29d04e4..bfd16493d8 100644 --- a/binding/mdsal-binding-dom-codec/pom.xml +++ b/binding/mdsal-binding-dom-codec/pom.xml @@ -80,6 +80,13 @@ checkstyle.violationSeverity=error + + com.github.spotbugs + spotbugs-maven-plugin + + true + + diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/gen/impl/AbstractStreamWriterGenerator.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/gen/impl/AbstractStreamWriterGenerator.java index 8e34080d43..847260010d 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/gen/impl/AbstractStreamWriterGenerator.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/gen/impl/AbstractStreamWriterGenerator.java @@ -15,6 +15,8 @@ import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Map.Entry; import javassist.CannotCompileException; import javassist.CtClass; @@ -49,28 +51,38 @@ abstract class AbstractStreamWriterGenerator extends AbstractGenerator implement protected static final String SERIALIZE_METHOD_NAME = "serialize"; protected static final AugmentableDispatchSerializer AUGMENTABLE = new AugmentableDispatchSerializer(); - private static final Field FIELD_MODIFIERS; + private static final Field FIELD_MODIFIERS = getModifiersField(); private final LoadingCache, DataObjectSerializerImplementation> implementations; private final CtClass[] serializeArguments; private final JavassistUtils javassist; private BindingRuntimeContext context; - static { + private static Field getModifiersField() { /* * Cache reflection access to field modifiers field. We need this to set * fix the static declared fields to final once we initialize them. If we * cannot get access, that's fine, too. */ - Field field = null; + final Field field; try { field = Field.class.getDeclaredField("modifiers"); - field.setAccessible(true); } catch (NoSuchFieldException | SecurityException e) { - LOG.warn("Could not get Field modifiers field, serializers run at decreased efficiency", e); + LOG.warn("Could not get modifiers field, serializers run at decreased efficiency", e); + return null; } - FIELD_MODIFIERS = field; + try { + AccessController.doPrivileged((PrivilegedAction) () -> { + field.setAccessible(true); + return null; + }); + } catch (SecurityException e) { + LOG.warn("Could not get access to modifiers field, serializers run at decreased efficiency", e); + return null; + } + + return field; } protected AbstractStreamWriterGenerator(final JavassistUtils utils) { diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BindingCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BindingCodecContext.java index 16f2d971b2..9a72ce2e7d 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BindingCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BindingCodecContext.java @@ -7,7 +7,10 @@ */ package org.opendaylight.mdsal.binding.dom.codec.impl; -import com.google.common.base.Preconditions; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static java.util.Objects.requireNonNull; + import com.google.common.collect.ImmutableMap; import java.lang.reflect.Method; import java.lang.reflect.ParameterizedType; @@ -75,11 +78,11 @@ final class BindingCodecContext implements CodecContextFactory, BindingCodecTree private final SchemaRootCodecContext root; BindingCodecContext(final BindingRuntimeContext context, final BindingNormalizedNodeCodecRegistry registry) { - this.context = Preconditions.checkNotNull(context, "Binding Runtime Context is required."); + this.context = requireNonNull(context, "Binding Runtime Context is required."); this.root = SchemaRootCodecContext.create(this); this.identityCodec = new IdentityCodec(context); this.instanceIdentifierCodec = new InstanceIdentifierCodec(this); - this.registry = Preconditions.checkNotNull(registry); + this.registry = requireNonNull(registry); } @Override @@ -128,7 +131,7 @@ final class BindingCodecContext implements CodecContextFactory, BindingCodecTree DataContainerCodecContext currentNode = root; for (final InstanceIdentifier.PathArgument bindingArg : binding.getPathArguments()) { currentNode = currentNode.bindingPathArgumentChild(bindingArg, builder); - Preconditions.checkArgument(currentNode != null, "Supplied Instance Identifier %s is not valid.", binding); + checkArgument(currentNode != null, "Supplied Instance Identifier %s is not valid.", binding); } return currentNode; } @@ -151,9 +154,9 @@ final class BindingCodecContext implements CodecContextFactory, BindingCodecTree ListNodeCodecContext currentList = null; for (final YangInstanceIdentifier.PathArgument domArg : dom.getPathArguments()) { - Preconditions.checkArgument(currentNode instanceof DataContainerCodecContext, + checkArgument(currentNode instanceof DataContainerCodecContext, "Unexpected child of non-container node %s", currentNode); - final DataContainerCodecContext previous = (DataContainerCodecContext) currentNode; + final DataContainerCodecContext previous = (DataContainerCodecContext) currentNode; final NodeCodecContext nextNode = previous.yangPathArgumentChild(domArg); /* @@ -165,7 +168,7 @@ final class BindingCodecContext implements CodecContextFactory, BindingCodecTree * Identifier as Item or IdentifiableItem */ if (currentList != null) { - Preconditions.checkArgument(currentList == nextNode, + checkArgument(currentList == nextNode, "List should be referenced two times in YANG Instance Identifier %s", dom); // We entered list, so now we have all information to emit @@ -183,13 +186,13 @@ final class BindingCodecContext implements CodecContextFactory, BindingCodecTree // We do not add path argument for choice, since // it is not supported by binding instance identifier. currentNode = nextNode; - } else if (nextNode instanceof DataContainerCodecContext) { + } else if (nextNode instanceof DataContainerCodecContext) { if (bindingArguments != null) { - bindingArguments.add(((DataContainerCodecContext) nextNode).getBindingPathArgument(domArg)); + bindingArguments.add(((DataContainerCodecContext) nextNode).getBindingPathArgument(domArg)); } currentNode = nextNode; } else if (nextNode instanceof LeafNodeCodecContext) { - LOG.debug("Instance identifier referencing a leaf is not representable (%s)", dom); + LOG.debug("Instance identifier referencing a leaf is not representable ({})", dom); return null; } } @@ -197,11 +200,11 @@ final class BindingCodecContext implements CodecContextFactory, BindingCodecTree // Algorithm ended in list as whole representation // we sill need to emit identifier for list if (currentNode instanceof ChoiceNodeCodecContext) { - LOG.debug("Instance identifier targeting a choice is not representable (%s)", dom); + LOG.debug("Instance identifier targeting a choice is not representable ({})", dom); return null; } if (currentNode instanceof CaseNodeCodecContext) { - LOG.debug("Instance identifier targeting a case is not representable (%s)", dom); + LOG.debug("Instance identifier targeting a case is not representable ({})", dom); return null; } @@ -282,8 +285,7 @@ final class BindingCodecContext implements CodecContextFactory, BindingCodecTree } private Codec getCodec(final Class valueType, final DataSchemaNode schema) { - Preconditions.checkArgument(schema instanceof TypedDataSchemaNode, "Unsupported leaf node type %s", schema); - + checkArgument(schema instanceof TypedDataSchemaNode, "Unsupported leaf node type %s", schema); return getCodec(valueType, ((TypedDataSchemaNode)schema).getType()); } @@ -323,7 +325,7 @@ final class BindingCodecContext implements CodecContextFactory, BindingCodecTree } else if (typeDef instanceof LeafrefTypeDefinition) { final Entry typeWithSchema = context.getTypeWithSchema(valueType); final WithStatus schema = typeWithSchema.getValue(); - Preconditions.checkState(schema instanceof TypeDefinition); + checkState(schema instanceof TypeDefinition); return getCodec(valueType, (TypeDefinition) schema); } return ValueTypeCodec.getCodecFor(valueType, typeDef); diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BindingNormalizedNodeCodecRegistry.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BindingNormalizedNodeCodecRegistry.java index 7829d83a15..37c16b45b8 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BindingNormalizedNodeCodecRegistry.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/BindingNormalizedNodeCodecRegistry.java @@ -9,10 +9,10 @@ package org.opendaylight.mdsal.binding.dom.codec.impl; import static java.util.Objects.requireNonNull; -import com.google.common.base.Preconditions; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; import java.util.AbstractMap.SimpleEntry; import java.util.ArrayList; @@ -73,7 +73,7 @@ public class BindingNormalizedNodeCodecRegistry implements DataObjectSerializerR private volatile BindingCodecContext codecContext; public BindingNormalizedNodeCodecRegistry(final DataObjectSerializerGenerator generator) { - this.generator = Preconditions.checkNotNull(generator); + this.generator = requireNonNull(generator); this.serializers = CacheBuilder.newBuilder().weakKeys().build(new GeneratorLoader()); } @@ -123,6 +123,7 @@ public class BindingNormalizedNodeCodecRegistry implements DataObjectSerializerR } @Override + @SuppressFBWarnings("BC_UNCONFIRMED_CAST") public ContainerNode toNormalizedNodeNotification(final Notification data) { // FIXME: Should the cast to DataObject be necessary? return serializeDataObject((DataObject) data, diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ChoiceNodeCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ChoiceNodeCodecContext.java index 0d9699e686..76853ed572 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ChoiceNodeCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ChoiceNodeCodecContext.java @@ -7,7 +7,8 @@ */ package org.opendaylight.mdsal.binding.dom.codec.impl; -import com.google.common.base.Preconditions; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; @@ -35,7 +36,6 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; import org.opendaylight.yangtools.yang.data.impl.schema.SchemaUtils; import org.opendaylight.yangtools.yang.model.api.AugmentationSchemaNode; import org.opendaylight.yangtools.yang.model.api.CaseSchemaNode; @@ -215,7 +215,7 @@ final class ChoiceNodeCodecContext extends DataContainerCo return DataContainerCodecPrototype.from(childClass, childSchema.get(), factory()); } - LOG.debug("Supplied class %s is not valid case in schema %s", childClass, getSchema()); + LOG.debug("Supplied class {} is not valid case in schema {}", childClass, getSchema()); return null; } @@ -228,16 +228,15 @@ final class ChoiceNodeCodecContext extends DataContainerCo cazeProto = byYangCaseChild.get(arg); } - return childNonNull(cazeProto, arg,"Argument %s is not valid child of %s", arg, getSchema()).get() + return childNonNull(cazeProto, arg, "Argument %s is not valid child of %s", arg, getSchema()).get() .yangPathArgumentChild(arg); } @SuppressWarnings("unchecked") @Override public D deserialize(final NormalizedNode data) { - Preconditions.checkArgument(data instanceof ChoiceNode); - final NormalizedNodeContainer> casted = - (NormalizedNodeContainer>) data; + checkArgument(data instanceof ChoiceNode); + final ChoiceNode casted = (ChoiceNode) data; final NormalizedNode first = Iterables.getFirst(casted.getValue(), null); if (first == null) { @@ -255,7 +254,7 @@ final class ChoiceNodeCodecContext extends DataContainerCo @Override public PathArgument deserializePathArgument(final YangInstanceIdentifier.PathArgument arg) { - Preconditions.checkArgument(getDomPathArgument().equals(arg)); + checkArgument(getDomPathArgument().equals(arg)); return null; } diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java index 3b615f25b0..88731f31cd 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java @@ -12,6 +12,7 @@ import static com.google.common.base.Preconditions.checkArgument; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedMap; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; @@ -130,6 +131,7 @@ abstract class DataObjectCodecContext augment : possibleAugmentations.entrySet()) { final DataContainerCodecPrototype augProto = getAugmentationPrototype(augment.getValue()); diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyDataObject.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyDataObject.java index 0c7a16586c..a535eab2ee 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyDataObject.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyDataObject.java @@ -7,9 +7,11 @@ */ package org.opendaylight.mdsal.binding.dom.codec.impl; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects.ToStringHelper; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; @@ -51,8 +53,8 @@ class LazyDataObject implements InvocationHandler, Augment @SuppressWarnings({ "rawtypes", "unchecked" }) LazyDataObject(final DataObjectCodecContext ctx, final NormalizedNodeContainer data) { - this.context = Preconditions.checkNotNull(ctx, "Context must not be null"); - this.data = Preconditions.checkNotNull(data, "Data must not be null"); + this.context = requireNonNull(ctx, "Context must not be null"); + this.data = requireNonNull(data, "Data must not be null"); } @Override @@ -146,12 +148,13 @@ class LazyDataObject implements InvocationHandler, Augment Object cached = cachedData.get(method); if (cached == null) { final Object readedValue = context.getBindingChildValue(method, data); - if (readedValue == null) { - cached = NULL_VALUE; - } else { - cached = readedValue; + cached = readedValue == null ? NULL_VALUE : readedValue; + + final Object raced = cachedData.putIfAbsent(method, cached); + if (raced != null) { + // Load/store raced, we should return the stored value + cached = raced; } - cachedData.putIfAbsent(method, cached); } return cached == NULL_VALUE ? null : cached; @@ -174,14 +177,14 @@ class LazyDataObject implements InvocationHandler, Augment @Override public Map>, Augmentation> getAugmentations(final Object obj) { - Preconditions.checkArgument(this == Proxy.getInvocationHandler(obj), + checkArgument(this == Proxy.getInvocationHandler(obj), "Supplied object is not associated with this proxy handler"); return getAugmentationsImpl(); } private Object getAugmentationImpl(final Class cls) { - Preconditions.checkNotNull(cls, "Supplied augmentation must not be null."); + requireNonNull(cls, "Supplied augmentation must not be null."); final ImmutableMap>, Augmentation> aug = cachedAugmentations; if (aug != null) { diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/MissingSchemaForClassException.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/MissingSchemaForClassException.java index 87a5781945..c2f0e676c6 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/MissingSchemaForClassException.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/MissingSchemaForClassException.java @@ -8,6 +8,7 @@ package org.opendaylight.mdsal.binding.dom.codec.impl; import com.google.common.base.Preconditions; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.opendaylight.mdsal.binding.generator.util.BindingRuntimeContext; import org.opendaylight.yangtools.yang.binding.Augmentation; @@ -21,6 +22,7 @@ public final class MissingSchemaForClassException extends MissingSchemaException private static final long serialVersionUID = 1L; + @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") private final transient Class bindingClass; private MissingSchemaForClassException(final Class clz) { diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/NotificationCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/NotificationCodecContext.java index 61d7eecf41..514fead67e 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/NotificationCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/NotificationCodecContext.java @@ -7,12 +7,12 @@ */ package org.opendaylight.mdsal.binding.dom.codec.impl; -import com.google.common.base.Preconditions; +import static com.google.common.base.Preconditions.checkState; + import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.Notification; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; import org.opendaylight.yangtools.yang.model.api.NotificationDefinition; final class NotificationCodecContext @@ -25,8 +25,8 @@ final class NotificationCodecContext @Override public D deserialize(final NormalizedNode data) { - Preconditions.checkState(data instanceof ContainerNode); - return createBindingProxy((NormalizedNodeContainer) data); + checkState(data instanceof ContainerNode); + return createBindingProxy((ContainerNode) data); } @Override diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/util/AbstractBindingLazyContainerNode.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/util/AbstractBindingLazyContainerNode.java index 0527de2b0e..da6ab8cdb3 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/util/AbstractBindingLazyContainerNode.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/util/AbstractBindingLazyContainerNode.java @@ -104,7 +104,7 @@ public abstract class AbstractBindingLazyContainerNode return false; } final ContainerNode other = (ContainerNode) obj; - return delegate().equals(obj); + return delegate().equals(other); } @Override