FrontendClientMetadata(Builder) should not be Identifiable

Expose a clientId() to internal package users instead.

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

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendClientMetadataBuilder.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendClientMetadataBuilder.java
index 142d331e05ed15277acf6cd3c07d0cfe652d0aad..c89627800fc72ef8a275ca9b65cfdc19c08e1e85 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendClientMetadataBuilder.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendClientMetadataBuilder.java
@@ -7,11 +7,11 @@
  */
 package org.opendaylight.controller.cluster.datastore;
 
-import static com.google.common.base.Verify.verify;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.MoreObjects.ToStringHelper;
+import com.google.common.base.VerifyException;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
 import java.util.HashMap;
@@ -21,25 +21,23 @@ import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
 import org.opendaylight.controller.cluster.datastore.persisted.FrontendClientMetadata;
-import org.opendaylight.controller.cluster.datastore.persisted.FrontendHistoryMetadata;
 import org.opendaylight.controller.cluster.datastore.utils.ImmutableUnsignedLongSet;
 import org.opendaylight.controller.cluster.datastore.utils.MutableUnsignedLongSet;
-import org.opendaylight.yangtools.concepts.Identifiable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * This class is NOT thread-safe.
  */
-abstract sealed class FrontendClientMetadataBuilder implements Identifiable<ClientIdentifier> {
+abstract sealed class FrontendClientMetadataBuilder {
     static final class Disabled extends FrontendClientMetadataBuilder {
-        Disabled(final String shardName, final ClientIdentifier identifier) {
-            super(shardName, identifier);
+        Disabled(final String shardName, final ClientIdentifier clientId) {
+            super(shardName, clientId);
         }
 
         @Override
         FrontendClientMetadata build() {
-            return new FrontendClientMetadata(getIdentifier(), ImmutableUnsignedLongSet.of(), ImmutableList.of());
+            return new FrontendClientMetadata(clientId(), ImmutableUnsignedLongSet.of(), ImmutableList.of());
         }
 
         @Override
@@ -79,7 +77,7 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
 
         @Override
         LeaderFrontendState toLeaderState(final Shard shard) {
-            return new LeaderFrontendState.Disabled(shard.persistenceId(), getIdentifier(), shard.getDataStore());
+            return new LeaderFrontendState.Disabled(shard.persistenceId(), clientId(), shard.getDataStore());
         }
     }
 
@@ -88,8 +86,8 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
         private final MutableUnsignedLongSet purgedHistories;
         private final LocalHistoryIdentifier standaloneId;
 
-        Enabled(final String shardName, final ClientIdentifier identifier) {
-            super(shardName, identifier);
+        Enabled(final String shardName, final ClientIdentifier clientId) {
+            super(shardName, clientId);
 
             purgedHistories = MutableUnsignedLongSet.of();
 
@@ -99,33 +97,33 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
         }
 
         Enabled(final String shardName, final FrontendClientMetadata meta) {
-            super(shardName, meta.getIdentifier());
+            super(shardName, meta.clientId());
 
             purgedHistories = meta.getPurgedHistories().mutableCopy();
-            for (FrontendHistoryMetadata h : meta.getCurrentHistories()) {
-                final FrontendHistoryMetadataBuilder b = new FrontendHistoryMetadataBuilder(getIdentifier(), h);
-                currentHistories.put(b.getIdentifier(), b);
+            for (var historyMeta : meta.getCurrentHistories()) {
+                final var builder = new FrontendHistoryMetadataBuilder(clientId(), historyMeta);
+                currentHistories.put(builder.getIdentifier(), builder);
             }
 
             // Sanity check and recovery
             standaloneId = standaloneHistoryId();
             if (!currentHistories.containsKey(standaloneId)) {
                 LOG.warn("{}: Client {} recovered histories {} do not contain stand-alone history, attempting recovery",
-                    shardName, getIdentifier(), currentHistories);
+                    shardName, clientId(), currentHistories);
                 currentHistories.put(standaloneId, new FrontendHistoryMetadataBuilder(standaloneId));
             }
         }
 
         @Override
         FrontendClientMetadata build() {
-            return new FrontendClientMetadata(getIdentifier(), purgedHistories.immutableCopy(),
+            return new FrontendClientMetadata(clientId(), purgedHistories.immutableCopy(),
                 Collections2.transform(currentHistories.values(), FrontendHistoryMetadataBuilder::build));
         }
 
         @Override
         void onHistoryCreated(final LocalHistoryIdentifier historyId) {
-            final FrontendHistoryMetadataBuilder newMeta = new FrontendHistoryMetadataBuilder(historyId);
-            final FrontendHistoryMetadataBuilder oldMeta = currentHistories.putIfAbsent(historyId, newMeta);
+            final var newMeta = new FrontendHistoryMetadataBuilder(historyId);
+            final var oldMeta = currentHistories.putIfAbsent(historyId, newMeta);
             if (oldMeta != null) {
                 // This should not be happening, warn about it
                 LOG.warn("{}: Reused local history {}", shardName(), historyId);
@@ -136,7 +134,7 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
 
         @Override
         void onHistoryClosed(final LocalHistoryIdentifier historyId) {
-            final FrontendHistoryMetadataBuilder builder = currentHistories.get(historyId);
+            final var builder = currentHistories.get(historyId);
             if (builder != null) {
                 builder.onHistoryClosed();
                 LOG.debug("{}: Closed history {}", shardName(), historyId);
@@ -147,7 +145,7 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
 
         @Override
         void onHistoryPurged(final LocalHistoryIdentifier historyId) {
-            final FrontendHistoryMetadataBuilder history = currentHistories.remove(historyId);
+            final var history = currentHistories.remove(historyId);
             final long historyBits = historyId.getHistoryId();
             if (history == null) {
                 if (!purgedHistories.contains(historyBits)) {
@@ -164,7 +162,7 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
 
         @Override
         void onTransactionAborted(final TransactionIdentifier txId) {
-            final FrontendHistoryMetadataBuilder history = getHistory(txId);
+            final var history = getHistory(txId);
             if (history != null) {
                 history.onTransactionAborted(txId);
                 LOG.debug("{}: Aborted transaction {}", shardName(), txId);
@@ -175,7 +173,7 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
 
         @Override
         void onTransactionCommitted(final TransactionIdentifier txId) {
-            final FrontendHistoryMetadataBuilder history = getHistory(txId);
+            final var history = getHistory(txId);
             if (history != null) {
                 history.onTransactionCommitted(txId);
                 LOG.debug("{}: Committed transaction {}", shardName(), txId);
@@ -186,7 +184,7 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
 
         @Override
         void onTransactionPurged(final TransactionIdentifier txId) {
-            final FrontendHistoryMetadataBuilder history = getHistory(txId);
+            final var history = getHistory(txId);
             if (history != null) {
                 history.onTransactionPurged(txId);
                 LOG.debug("{}: Purged transaction {}", shardName(), txId);
@@ -210,26 +208,29 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
         LeaderFrontendState toLeaderState(final Shard shard) {
             // Note: we have to make sure to *copy* all current state and not leak any views, otherwise leader/follower
             //       interactions would get intertwined leading to inconsistencies.
-            final Map<LocalHistoryIdentifier, LocalFrontendHistory> histories = new HashMap<>();
-            for (FrontendHistoryMetadataBuilder e : currentHistories.values()) {
-                if (e.getIdentifier().getHistoryId() != 0) {
-                    final AbstractFrontendHistory state = e.toLeaderState(shard);
-                    verify(state instanceof LocalFrontendHistory, "Unexpected state %s", state);
-                    histories.put(e.getIdentifier(), (LocalFrontendHistory) state);
+            final var histories = new HashMap<LocalHistoryIdentifier, LocalFrontendHistory>();
+            for (var historyMetaBuilder : currentHistories.values()) {
+                final var historyId = historyMetaBuilder.getIdentifier();
+                if (historyId.getHistoryId() != 0) {
+                    final var state = historyMetaBuilder.toLeaderState(shard);
+                    if (state instanceof LocalFrontendHistory localState) {
+                        histories.put(historyId, localState);
+                    } else {
+                        throw new VerifyException("Unexpected state " + state);
+                    }
                 }
             }
 
             final AbstractFrontendHistory singleHistory;
-            final FrontendHistoryMetadataBuilder singleHistoryMeta = currentHistories.get(
-                new LocalHistoryIdentifier(getIdentifier(), 0));
+            final var singleHistoryMeta = currentHistories.get(new LocalHistoryIdentifier(clientId(), 0));
             if (singleHistoryMeta == null) {
-                final ShardDataTree tree = shard.getDataStore();
-                singleHistory = StandaloneFrontendHistory.create(shard.persistenceId(), getIdentifier(), tree);
+                final var tree = shard.getDataStore();
+                singleHistory = StandaloneFrontendHistory.create(shard.persistenceId(), clientId(), tree);
             } else {
                 singleHistory = singleHistoryMeta.toLeaderState(shard);
             }
 
-            return new LeaderFrontendState.Enabled(shard.persistenceId(), getIdentifier(), shard.getDataStore(),
+            return new LeaderFrontendState.Enabled(shard.persistenceId(), clientId(), shard.getDataStore(),
                 purgedHistories.mutableCopy(), singleHistory, histories);
         }
 
@@ -257,30 +258,29 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
         }
 
         private LocalHistoryIdentifier standaloneHistoryId() {
-            return new LocalHistoryIdentifier(getIdentifier(), 0);
+            return new LocalHistoryIdentifier(clientId(), 0);
         }
     }
 
     private static final Logger LOG = LoggerFactory.getLogger(FrontendClientMetadataBuilder.class);
 
-    private final @NonNull ClientIdentifier identifier;
+    private final @NonNull ClientIdentifier clientId;
     private final @NonNull String shardName;
 
-    FrontendClientMetadataBuilder(final String shardName, final ClientIdentifier identifier) {
+    FrontendClientMetadataBuilder(final String shardName, final ClientIdentifier clientId) {
         this.shardName = requireNonNull(shardName);
-        this.identifier = requireNonNull(identifier);
+        this.clientId = requireNonNull(clientId);
     }
 
     static FrontendClientMetadataBuilder of(final String shardName, final FrontendClientMetadata meta) {
         // Completely empty histories imply disabled state, as otherwise we'd have a record of the single history --
         // either purged or active
         return meta.getCurrentHistories().isEmpty() && meta.getPurgedHistories().isEmpty()
-            ? new Disabled(shardName, meta.getIdentifier()) : new Enabled(shardName, meta);
+            ? new Disabled(shardName, meta.clientId()) : new Enabled(shardName, meta);
     }
 
-    @Override
-    public final ClientIdentifier getIdentifier() {
-        return identifier;
+    final ClientIdentifier clientId() {
+        return clientId;
     }
 
     final String shardName() {
@@ -317,6 +317,6 @@ abstract sealed class FrontendClientMetadataBuilder implements Identifiable<Clie
     }
 
     ToStringHelper addToStringAttributes(final ToStringHelper helper) {
-        return helper.add("identifier", identifier);
+        return helper.add("clientId", clientId);
     }
 }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.java
index e3a18997e43e68fc3597936e99a4a58890d73403..abb97e59a43701df5f223e526bc8a3596e8c044f 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.java
@@ -20,7 +20,6 @@ import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.FrontendIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
-import org.opendaylight.controller.cluster.datastore.persisted.FrontendClientMetadata;
 import org.opendaylight.controller.cluster.datastore.persisted.FrontendShardDataTreeSnapshotMetadata;
 import org.opendaylight.controller.cluster.datastore.utils.ImmutableUnsignedLongSet;
 import org.slf4j.Logger;
@@ -59,13 +58,13 @@ final class FrontendMetadata extends ShardDataTreeMetadata<FrontendShardDataTree
         LOG.debug("{}: applying snapshot {} over clients {}", shardName, snapshot, clients);
         clients.clear();
 
-        for (FrontendClientMetadata m : snapshot.getClients()) {
-            LOG.debug("{}: applying metadata {}", shardName, m);
-            final FrontendClientMetadataBuilder b = FrontendClientMetadataBuilder.of(shardName, m);
-            final FrontendIdentifier client = m.getIdentifier().getFrontendId();
+        for (var clientMeta : snapshot.getClients()) {
+            LOG.debug("{}: applying metadata {}", shardName, clientMeta);
+            final var builder = FrontendClientMetadataBuilder.of(shardName, clientMeta);
+            final var frontendId = clientMeta.clientId().getFrontendId();
 
-            LOG.debug("{}: client {} updated to {}", shardName, client, b);
-            clients.put(client, b);
+            LOG.debug("{}: client {} updated to {}", shardName, frontendId, builder);
+            clients.put(frontendId, builder);
         }
     }
 
@@ -76,13 +75,13 @@ final class FrontendMetadata extends ShardDataTreeMetadata<FrontendShardDataTree
     }
 
     private FrontendClientMetadataBuilder ensureClient(final ClientIdentifier id) {
-        final FrontendClientMetadataBuilder existing = clients.get(id.getFrontendId());
-        if (existing != null && id.equals(existing.getIdentifier())) {
+        final var existing = clients.get(id.getFrontendId());
+        if (existing != null && id.equals(existing.clientId())) {
             return existing;
         }
 
-        final FrontendClientMetadataBuilder client = new FrontendClientMetadataBuilder.Enabled(shardName, id);
-        final FrontendClientMetadataBuilder previous = clients.put(id.getFrontendId(), client);
+        final var client = new FrontendClientMetadataBuilder.Enabled(shardName, id);
+        final var previous = clients.put(id.getFrontendId(), client);
         if (previous != null) {
             LOG.debug("{}: Replaced client {} with {}", shardName, previous, client);
         } else {
@@ -136,8 +135,8 @@ final class FrontendMetadata extends ShardDataTreeMetadata<FrontendShardDataTree
     }
 
     void disableTracking(final ClientIdentifier clientId) {
-        final FrontendIdentifier frontendId = clientId.getFrontendId();
-        final FrontendClientMetadataBuilder client = clients.get(frontendId);
+        final var frontendId = clientId.getFrontendId();
+        final var client = clients.get(frontendId);
         if (client == null) {
             // When we have not seen the client before, we still need to disable tracking for him since this only gets
             // triggered once.
@@ -145,7 +144,7 @@ final class FrontendMetadata extends ShardDataTreeMetadata<FrontendShardDataTree
             clients.put(frontendId, new FrontendClientMetadataBuilder.Disabled(shardName, clientId));
             return;
         }
-        if (!clientId.equals(client.getIdentifier())) {
+        if (!clientId.equals(client.clientId())) {
             LOG.debug("{}: disableTracking {} does not match client {}, ignoring", shardName, clientId, client);
             return;
         }
@@ -159,7 +158,7 @@ final class FrontendMetadata extends ShardDataTreeMetadata<FrontendShardDataTree
 
     ImmutableSet<ClientIdentifier> getClients() {
         return clients.values().stream()
-                .map(FrontendClientMetadataBuilder::getIdentifier)
-                .collect(ImmutableSet.toImmutableSet());
+            .map(FrontendClientMetadataBuilder::clientId)
+            .collect(ImmutableSet.toImmutableSet());
     }
 }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendClientMetadata.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendClientMetadata.java
index 35859bb1c947a33497aed15c59023c810d845f89..49573e247c6615e9ff79ef736573a969c749713e 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendClientMetadata.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendClientMetadata.java
@@ -18,21 +18,24 @@ import java.util.Collection;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier;
 import org.opendaylight.controller.cluster.datastore.utils.ImmutableUnsignedLongSet;
-import org.opendaylight.yangtools.concepts.Identifiable;
 import org.opendaylight.yangtools.concepts.WritableObject;
 
-public final class FrontendClientMetadata implements Identifiable<ClientIdentifier>, WritableObject {
+public final class FrontendClientMetadata implements WritableObject {
     private final @NonNull ImmutableList<FrontendHistoryMetadata> currentHistories;
     private final @NonNull ImmutableUnsignedLongSet purgedHistories;
-    private final @NonNull ClientIdentifier identifier;
+    private final @NonNull ClientIdentifier clientId;
 
-    public FrontendClientMetadata(final ClientIdentifier identifier, final ImmutableUnsignedLongSet purgedHistories,
+    public FrontendClientMetadata(final ClientIdentifier clientId, final ImmutableUnsignedLongSet purgedHistories,
             final Collection<FrontendHistoryMetadata> currentHistories) {
-        this.identifier = requireNonNull(identifier);
+        this.clientId = requireNonNull(clientId);
         this.purgedHistories = requireNonNull(purgedHistories);
         this.currentHistories = ImmutableList.copyOf(currentHistories);
     }
 
+    public ClientIdentifier clientId() {
+        return clientId;
+    }
+
     public ImmutableList<FrontendHistoryMetadata> getCurrentHistories() {
         return currentHistories;
     }
@@ -41,14 +44,9 @@ public final class FrontendClientMetadata implements Identifiable<ClientIdentifi
         return purgedHistories;
     }
 
-    @Override
-    public ClientIdentifier getIdentifier() {
-        return identifier;
-    }
-
     @Override
     public void writeTo(final DataOutput out) throws IOException {
-        identifier.writeTo(out);
+        clientId.writeTo(out);
         purgedHistories.writeTo(out);
 
         out.writeInt(currentHistories.size());
@@ -58,7 +56,7 @@ public final class FrontendClientMetadata implements Identifiable<ClientIdentifi
     }
 
     public static FrontendClientMetadata readFrom(final DataInput in) throws IOException {
-        final ClientIdentifier id = ClientIdentifier.readFrom(in);
+        final var clientId = ClientIdentifier.readFrom(in);
         final var purgedHistories = ImmutableUnsignedLongSet.readFrom(in);
 
         final int currentSize = in.readInt();
@@ -67,12 +65,12 @@ public final class FrontendClientMetadata implements Identifiable<ClientIdentifi
             currentBuilder.add(FrontendHistoryMetadata.readFrom(in));
         }
 
-        return new FrontendClientMetadata(id, purgedHistories, currentBuilder.build());
+        return new FrontendClientMetadata(clientId, purgedHistories, currentBuilder.build());
     }
 
     @Override
     public String toString() {
-        return MoreObjects.toStringHelper(FrontendClientMetadata.class).add("identifer", identifier)
-                .add("current", currentHistories).add("purged", purgedHistories).toString();
+        return MoreObjects.toStringHelper(FrontendClientMetadata.class)
+            .add("clientId", clientId).add("current", currentHistories).add("purged", purgedHistories).toString();
     }
 }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendShardDataTreeSnapshotMetadataTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendShardDataTreeSnapshotMetadataTest.java
index 8d4fbe97b9802f162e22b4b1e1b38e8c03fab399..556772456c284c582c96ae7d44c5766466ada6ca 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendShardDataTreeSnapshotMetadataTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendShardDataTreeSnapshotMetadataTest.java
@@ -70,15 +70,15 @@ public class FrontendShardDataTreeSnapshotMetadataTest {
 
         final Map<ClientIdentifier, FrontendClientMetadata> origIdent = new HashMap<>();
         final Map<ClientIdentifier, FrontendClientMetadata> copyIdent = new HashMap<>();
-        origClientList.forEach(client -> origIdent.put(client.getIdentifier(), client));
-        origClientList.forEach(client -> copyIdent.put(client.getIdentifier(), client));
+        origClientList.forEach(client -> origIdent.put(client.clientId(), client));
+        origClientList.forEach(client -> copyIdent.put(client.clientId(), client));
 
         assertTrue(origIdent.keySet().containsAll(copyIdent.keySet()));
         assertTrue(copyIdent.keySet().containsAll(origIdent.keySet()));
 
         origIdent.values().forEach(client -> {
-            final FrontendClientMetadata copyClient = copyIdent.get(client.getIdentifier());
-            testObject(client.getIdentifier(), copyClient.getIdentifier());
+            final var copyClient = copyIdent.get(client.clientId());
+            testObject(client.clientId(), copyClient.clientId());
             assertEquals(client.getPurgedHistories(), copyClient.getPurgedHistories());
             assertEquals(client.getCurrentHistories(), copyClient.getCurrentHistories());
         });