From 8ca42ed047eae6e0407f2cca661b3f51dba7e219 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 24 Apr 2022 10:37:29 +0200 Subject: [PATCH] Do not use NPE for consistency checks We are using Preconditions.checkNotNull() when guarding against illegal state. Improve this by using verifyNotNull(), which results in VerifyException, making the intent clearer. Also migrate a verify(foo != null) to a proper verifyNotNull(), using the resoluting value. Change-Id: I2689852d396315714f126743b70469ceef75ce45 Signed-off-by: Robert Varga --- .../dds/LocalReadOnlyProxyTransaction.java | 4 ++-- .../dds/LocalReadWriteProxyTransaction.java | 23 ++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadOnlyProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadOnlyProxyTransaction.java index bab72d69f8..2954d22245 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadOnlyProxyTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadOnlyProxyTransaction.java @@ -7,8 +7,8 @@ */ package org.opendaylight.controller.cluster.databroker.actors.dds; -import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Verify.verify; +import static com.google.common.base.Verify.verifyNotNull; import static java.util.Objects.requireNonNull; import java.util.Optional; @@ -49,7 +49,7 @@ final class LocalReadOnlyProxyTransaction extends LocalProxyTransaction { @Override DataTreeSnapshot readOnlyView() { - return checkNotNull(snapshot, "Transaction %s is DONE", getIdentifier()); + return verifyNotNull(snapshot, "Transaction %s is DONE", getIdentifier()); } @Override diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java index 4d6a644e92..ecb914549d 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.java @@ -7,8 +7,10 @@ */ package org.opendaylight.controller.cluster.databroker.actors.dds; -import com.google.common.base.Preconditions; -import com.google.common.base.Verify; +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.base.Verify.verify; +import static com.google.common.base.Verify.verifyNotNull; + import java.util.Optional; import java.util.OptionalLong; import java.util.function.BiConsumer; @@ -177,7 +179,7 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction { } private void sealModification() { - Preconditions.checkState(sealedModification == null, "Transaction %s is already sealed", this); + checkState(sealedModification == null, "Transaction %s is already sealed", this); final CursorAwareDataTreeModification mod = getModification(); mod.ready(); sealedModification = mod; @@ -221,7 +223,7 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction { } CursorAwareDataTreeSnapshot getSnapshot() { - Preconditions.checkState(sealedModification != null, "Proxy %s is not sealed yet", getIdentifier()); + checkState(sealedModification != null, "Proxy %s is not sealed yet", getIdentifier()); return sealedModification; } @@ -254,23 +256,23 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction { final Optional maybeProtocol = request.getPersistenceProtocol(); if (maybeProtocol.isPresent()) { - Verify.verify(callback != null, "Request %s has null callback", request); + final var cb = verifyNotNull(callback, "Request %s has null callback", request); if (markSealed()) { sealOnly(); } switch (maybeProtocol.get()) { case ABORT: - sendMethod.accept(new AbortLocalTransactionRequest(getIdentifier(), localActor()), callback); + sendMethod.accept(new AbortLocalTransactionRequest(getIdentifier(), localActor()), cb); break; case READY: // No-op, as we have already issued a sealOnly() and we are not transmitting anything break; case SIMPLE: - sendMethod.accept(commitRequest(false), callback); + sendMethod.accept(commitRequest(false), cb); break; case THREE_PHASE: - sendMethod.accept(commitRequest(true), callback); + sendMethod.accept(commitRequest(true), cb); break; default: throw new IllegalArgumentException("Unhandled protocol " + maybeProtocol.get()); @@ -336,7 +338,7 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction { } private static LocalReadWriteProxyTransaction verifyLocalReadWrite(final LocalProxyTransaction successor) { - Verify.verify(successor instanceof LocalReadWriteProxyTransaction, "Unexpected successor %s", successor); + verify(successor instanceof LocalReadWriteProxyTransaction, "Unexpected successor %s", successor); return (LocalReadWriteProxyTransaction) successor; } @@ -357,8 +359,7 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction { if (closedException != null) { throw closedException.get(); } - - return Preconditions.checkNotNull(modification, "Transaction %s is DONE", getIdentifier()); + return verifyNotNull(modification, "Transaction %s is DONE", getIdentifier()); } private void sendRebased(final CommitLocalTransactionRequest request, final Consumer> callback) { -- 2.36.6