From f662ce8b1fa94b77ba66f7ece8bcaff91dee809e Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 21 May 2019 10:47:34 +0200 Subject: [PATCH] Do not leak DataTree from backend actor YANG tools is now exposing the read-only part of DataTree as ReadOnlyDataTree. Use this interface to restrict the set of methods accessible to frontend. Change-Id: I9426d4cee1e68e3cc21f7a39322a0df09739e770 Signed-off-by: Robert Varga --- .../access/commands/ConnectClientSuccess.java | 14 ++++----- .../commands/ConnectClientSuccessProxyV1.java | 3 +- .../commands/ConnectClientSuccessTest.java | 3 +- .../databroker/actors/dds/ProxyHistory.java | 30 ++++++++++--------- .../actors/dds/ShardBackendInfo.java | 18 ++++++----- .../AbstractTransactionContextFactory.java | 6 ++-- .../datastore/LocalTransactionChain.java | 9 +++--- .../LocalTransactionFactoryImpl.java | 9 +++--- .../datastore/TransactionChainProxy.java | 4 +-- .../datastore/TransactionContextFactory.java | 4 +-- .../messages/LocalPrimaryShardFound.java | 8 ++--- .../datastore/messages/PrimaryShardInfo.java | 8 ++--- .../messages/ShardLeaderStateChanged.java | 9 +++--- .../shardmanager/ShardInformation.java | 8 ++--- .../cluster/datastore/utils/ActorUtils.java | 4 +-- 15 files changed, 71 insertions(+), 66 deletions(-) diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccess.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccess.java index 1ec81c8b5b..43fdb3c3c2 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccess.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccess.java @@ -22,7 +22,7 @@ import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.controller.cluster.access.ABIVersion; import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier; import org.opendaylight.controller.cluster.access.concepts.RequestSuccess; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ReadOnlyDataTree; /** * Successful reply to an {@link ConnectClientRequest}. Client actor which initiated this connection should use @@ -41,24 +41,24 @@ public final class ConnectClientSuccess extends RequestSuccess alternates; @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "See justification above.") - private final DataTree dataTree; + private final ReadOnlyDataTree dataTree; private final @NonNull ActorRef backend; private final int maxMessages; ConnectClientSuccess(final ClientIdentifier target, final long sequence, final ActorRef backend, - final List alternates, final Optional dataTree, final int maxMessages) { + final List alternates, final int maxMessages, final ReadOnlyDataTree dataTree) { super(target, sequence); this.backend = requireNonNull(backend); this.alternates = ImmutableList.copyOf(alternates); - this.dataTree = dataTree.orElse(null); + this.dataTree = dataTree; checkArgument(maxMessages > 0, "Maximum messages has to be positive, not %s", maxMessages); this.maxMessages = maxMessages; } public ConnectClientSuccess(final @NonNull ClientIdentifier target, final long sequence, final @NonNull ActorRef backend, final @NonNull List alternates, - final @NonNull DataTree dataTree, final int maxMessages) { - this(target, sequence, backend, alternates, Optional.of(dataTree), maxMessages); + final @NonNull ReadOnlyDataTree dataTree, final int maxMessages) { + this(target, sequence, backend, alternates, maxMessages, requireNonNull(dataTree)); } /** @@ -74,7 +74,7 @@ public final class ConnectClientSuccess extends RequestSuccess getDataTree() { + public Optional getDataTree() { return Optional.ofNullable(dataTree); } diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccessProxyV1.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccessProxyV1.java index fb44e07c10..8dd40ac2c4 100644 --- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccessProxyV1.java +++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccessProxyV1.java @@ -17,7 +17,6 @@ import java.io.ObjectInput; import java.io.ObjectOutput; import java.util.ArrayList; import java.util.List; -import java.util.Optional; import org.opendaylight.controller.cluster.access.concepts.AbstractSuccessProxy; import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier; @@ -78,7 +77,7 @@ final class ConnectClientSuccessProxyV1 extends AbstractSuccessProxy { @@ -62,7 +63,7 @@ public class ConnectClientSuccessTest extends AbstractRequestSuccessTest { private abstract static class AbstractLocal extends ProxyHistory { - private final DataTree dataTree; + private final ReadOnlyDataTree dataTree; AbstractLocal(final AbstractClientHistory parent, final AbstractClientConnection connection, - final LocalHistoryIdentifier identifier, final DataTree dataTree) { + final LocalHistoryIdentifier identifier, final ReadOnlyDataTree dataTree) { super(parent, connection, identifier); - this.dataTree = Preconditions.checkNotNull(dataTree); + this.dataTree = requireNonNull(dataTree); } final DataTreeSnapshot takeSnapshot() { @@ -80,14 +82,14 @@ abstract class ProxyHistory implements Identifiable { private volatile LocalReadWriteProxyTransaction lastSealed; Local(final AbstractClientHistory parent, final AbstractClientConnection connection, - final LocalHistoryIdentifier identifier, final DataTree dataTree) { + final LocalHistoryIdentifier identifier, final ReadOnlyDataTree dataTree) { super(parent, connection, identifier, dataTree); } @Override AbstractProxyTransaction doCreateTransactionProxy(final AbstractClientConnection connection, final TransactionIdentifier txId, final boolean snapshotOnly, final boolean isDone) { - Preconditions.checkState(lastOpen == null, "Proxy %s has %s currently open", this, lastOpen); + checkState(lastOpen == null, "Proxy %s has %s currently open", this, lastOpen); if (isDone) { // Done transactions do not register on our radar on should not have any state associated. @@ -136,7 +138,7 @@ abstract class ProxyHistory implements Identifiable { @Override void onTransactionSealed(final AbstractProxyTransaction tx) { - Preconditions.checkState(tx.equals(lastOpen)); + checkState(tx.equals(lastOpen)); lastSealed = lastOpen; lastOpen = null; } @@ -144,7 +146,7 @@ abstract class ProxyHistory implements Identifiable { private static final class LocalSingle extends AbstractLocal { LocalSingle(final AbstractClientHistory parent, final AbstractClientConnection connection, - final LocalHistoryIdentifier identifier, final DataTree dataTree) { + final LocalHistoryIdentifier identifier, final ReadOnlyDataTree dataTree) { super(parent, connection, identifier, dataTree); } @@ -328,14 +330,14 @@ abstract class ProxyHistory implements Identifiable { private ProxyHistory(final AbstractClientHistory parent, final AbstractClientConnection connection, final LocalHistoryIdentifier identifier) { - this.parent = Preconditions.checkNotNull(parent); - this.connection = Preconditions.checkNotNull(connection); - this.identifier = Preconditions.checkNotNull(identifier); + this.parent = requireNonNull(parent); + this.connection = requireNonNull(connection); + this.identifier = requireNonNull(identifier); } static ProxyHistory createClient(final AbstractClientHistory parent, final AbstractClientConnection connection, final LocalHistoryIdentifier identifier) { - final Optional dataTree = connection.getBackendInfo().flatMap(ShardBackendInfo::getDataTree); + final Optional dataTree = connection.getBackendInfo().flatMap(ShardBackendInfo::getDataTree); return dataTree.isPresent() ? new Local(parent, connection, identifier, dataTree.get()) : new Remote(parent, connection, identifier); } @@ -343,7 +345,7 @@ abstract class ProxyHistory implements Identifiable { static ProxyHistory createSingle(final AbstractClientHistory parent, final AbstractClientConnection connection, final LocalHistoryIdentifier identifier) { - final Optional dataTree = connection.getBackendInfo().flatMap(ShardBackendInfo::getDataTree); + final Optional dataTree = connection.getBackendInfo().flatMap(ShardBackendInfo::getDataTree); return dataTree.isPresent() ? new LocalSingle(parent, connection, identifier, dataTree.get()) : new RemoteSingle(parent, connection, identifier); } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ShardBackendInfo.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ShardBackendInfo.java index c23fc3fd7d..0958aade71 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ShardBackendInfo.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ShardBackendInfo.java @@ -7,15 +7,17 @@ */ package org.opendaylight.controller.cluster.databroker.actors.dds; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + import akka.actor.ActorRef; import com.google.common.base.MoreObjects.ToStringHelper; -import com.google.common.base.Preconditions; import com.google.common.primitives.UnsignedLong; import java.util.Optional; import org.opendaylight.controller.cluster.access.ABIVersion; import org.opendaylight.controller.cluster.access.client.BackendInfo; import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifier; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ReadOnlyDataTree; /** * Combined backend tracking. Aside from usual {@link BackendInfo}, this object also tracks the cookie assigned @@ -24,26 +26,26 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; * @author Robert Varga */ final class ShardBackendInfo extends BackendInfo { - private final Optional dataTree; + private final Optional dataTree; private final UnsignedLong cookie; ShardBackendInfo(final ActorRef actor, final long sessionId, final ABIVersion version, final String shardName, - final UnsignedLong cookie, final Optional dataTree, final int maxMessages) { + final UnsignedLong cookie, final Optional dataTree, final int maxMessages) { super(actor, shardName, sessionId, version, maxMessages); - this.cookie = Preconditions.checkNotNull(cookie); - this.dataTree = Preconditions.checkNotNull(dataTree); + this.cookie = requireNonNull(cookie); + this.dataTree = requireNonNull(dataTree); } UnsignedLong getCookie() { return cookie; } - Optional getDataTree() { + Optional getDataTree() { return dataTree; } LocalHistoryIdentifier brandHistory(final LocalHistoryIdentifier id) { - Preconditions.checkArgument(id.getCookie() == 0, "History %s is already branded", id); + checkArgument(id.getCookie() == 0, "History %s is already branded", id); return new LocalHistoryIdentifier(id.getClientId(), id.getHistoryId(), cookie.longValue()); } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractTransactionContextFactory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractTransactionContextFactory.java index a415a8c087..6d573dedf5 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractTransactionContextFactory.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractTransactionContextFactory.java @@ -25,7 +25,7 @@ import org.opendaylight.controller.cluster.datastore.utils.ActorUtils; import org.opendaylight.mdsal.dom.spi.store.DOMStoreReadTransaction; import org.opendaylight.mdsal.dom.spi.store.DOMStoreReadWriteTransaction; import org.opendaylight.mdsal.dom.spi.store.DOMStoreWriteTransaction; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ReadOnlyDataTree; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import scala.concurrent.Future; @@ -143,7 +143,7 @@ abstract class AbstractTransactionContextFactory maybeDataTree = primaryShardInfo.getLocalShardDataTree(); + final Optional maybeDataTree = primaryShardInfo.getLocalShardDataTree(); if (maybeDataTree.isPresent()) { if (!knownLocal.containsKey(shardName)) { LOG.debug("Shard {} resolved to local data tree - adding local factory", shardName); @@ -190,7 +190,7 @@ abstract class AbstractTransactionContextFactory getLocalShardDataTree() { + public @NonNull Optional getLocalShardDataTree() { return Optional.ofNullable(localShardDataTree); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/ShardLeaderStateChanged.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/ShardLeaderStateChanged.java index 679d421eaf..cbf2cf9e0f 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/ShardLeaderStateChanged.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/ShardLeaderStateChanged.java @@ -13,7 +13,7 @@ import java.util.Optional; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.controller.cluster.notifications.LeaderStateChanged; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ReadOnlyDataTree; /** * A local message derived from LeaderStateChanged containing additional Shard-specific info that is sent @@ -23,11 +23,10 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; * @author Thomas Pantelis */ public class ShardLeaderStateChanged extends LeaderStateChanged { - - private final DataTree localShardDataTree; + private final ReadOnlyDataTree localShardDataTree; public ShardLeaderStateChanged(@NonNull String memberId, @Nullable String leaderId, - @NonNull DataTree localShardDataTree, short leaderPayloadVersion) { + @NonNull ReadOnlyDataTree localShardDataTree, short leaderPayloadVersion) { super(memberId, leaderId, leaderPayloadVersion); this.localShardDataTree = requireNonNull(localShardDataTree); } @@ -38,7 +37,7 @@ public class ShardLeaderStateChanged extends LeaderStateChanged { this.localShardDataTree = null; } - public @NonNull Optional getLocalShardDataTree() { + public @NonNull Optional getLocalShardDataTree() { return Optional.ofNullable(localShardDataTree); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardInformation.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardInformation.java index 5b1f1b780e..97cbc5b0e0 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardInformation.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardInformation.java @@ -31,7 +31,7 @@ import org.opendaylight.controller.cluster.datastore.messages.PeerUp; import org.opendaylight.controller.cluster.datastore.shardmanager.ShardManager.OnShardInitialized; import org.opendaylight.controller.cluster.datastore.shardmanager.ShardManager.OnShardReady; import org.opendaylight.controller.cluster.raft.RaftState; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ReadOnlyDataTree; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,7 +51,7 @@ final class ShardInformation { private final AtomicShardContextProvider schemaContextProvider = new AtomicShardContextProvider(); private ActorRef actor; - private Optional localShardDataTree; + private Optional localShardDataTree; private boolean leaderAvailable = false; // flag that determines if the actor is ready for business @@ -101,11 +101,11 @@ final class ShardInformation { return shardId; } - void setLocalDataTree(final Optional dataTree) { + void setLocalDataTree(final Optional dataTree) { this.localShardDataTree = dataTree; } - Optional getLocalShardDataTree() { + Optional getLocalShardDataTree() { return localShardDataTree; } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/utils/ActorUtils.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/utils/ActorUtils.java index 3d45f647a7..a1efbf2d9b 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/utils/ActorUtils.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/utils/ActorUtils.java @@ -51,7 +51,7 @@ import org.opendaylight.controller.cluster.datastore.shardstrategy.ShardStrategy import org.opendaylight.controller.cluster.raft.client.messages.Shutdown; import org.opendaylight.controller.cluster.reporting.MetricsReporter; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ReadOnlyDataTree; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -242,7 +242,7 @@ public class ActorUtils { } private PrimaryShardInfo onPrimaryShardFound(final String shardName, final String primaryActorPath, - final short primaryVersion, final DataTree localShardDataTree) { + final short primaryVersion, final ReadOnlyDataTree localShardDataTree) { ActorSelection actorSelection = actorSystem.actorSelection(primaryActorPath); PrimaryShardInfo info = localShardDataTree == null ? new PrimaryShardInfo(actorSelection, primaryVersion) : new PrimaryShardInfo(actorSelection, primaryVersion, localShardDataTree); -- 2.36.6