Correct Frontend{Client,History}Metadata deserialization 99/98299/2
authorRobert Varga <robert.varga@pantheon.tech>
Sat, 6 Nov 2021 09:08:05 +0000 (10:08 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Sat, 6 Nov 2021 10:20:29 +0000 (11:20 +0100)
We are turning Range.closedOpen() into Range.closed() during
deserialization, which the resulting ends up also covering the upper
bound, which it should not.

JIRA: CONTROLLER-1942
Change-Id: Ib8f9016e2eddcf014ff5e451ac82cd77b66d7019
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendClientMetadata.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendHistoryMetadata.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/persisted/FrontendShardDataTreeSnapshotMetadataTest.java

index a3f41ed65ddfbfb21c4da06d804f1a0278db6c75..c9b243052868bb7f0f8cd7c4b9461f05ad6a9ecc 100644 (file)
@@ -78,7 +78,7 @@ public final class FrontendClientMetadata implements Identifiable<ClientIdentifi
             final UnsignedLong lower = UnsignedLong.fromLongBits(WritableObjects.readFirstLong(in, header));
             final UnsignedLong upper = UnsignedLong.fromLongBits(WritableObjects.readSecondLong(in, header));
 
-            b.add(Range.closed(lower, upper));
+            b.add(Range.closedOpen(lower, upper));
         }
 
         final int currentSize = in.readInt();
index 2cdda9f54716b41924e94a59cfb73c18110a1f12..fc0bd442c8483e7d42f139ac5ff7f318f36584ff 100644 (file)
@@ -103,7 +103,7 @@ public final class FrontendHistoryMetadata implements WritableObject {
             final byte h = WritableObjects.readLongHeader(in);
             final UnsignedLong l = UnsignedLong.fromLongBits(WritableObjects.readFirstLong(in, h));
             final UnsignedLong u = UnsignedLong.fromLongBits(WritableObjects.readSecondLong(in, h));
-            purgedTransactions.add(Range.closed(l, u));
+            purgedTransactions.add(Range.closedOpen(l, u));
         }
 
         return new FrontendHistoryMetadata(historyId, cookie, closed, closedTransactions, purgedTransactions);
index db52111ad22199270bac587fd8ce10cbc476f84a..25b5128dbaf08b0ab29c940b333b58190ac8cdbe 100644 (file)
@@ -10,6 +10,7 @@ package org.opendaylight.controller.cluster.datastore.persisted;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
 import com.google.common.collect.ImmutableMap;
@@ -23,11 +24,10 @@ import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.junit.Test;
 import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier;
 import org.opendaylight.controller.cluster.access.concepts.FrontendIdentifier;
@@ -36,27 +36,27 @@ import org.opendaylight.controller.cluster.access.concepts.MemberName;
 
 public class FrontendShardDataTreeSnapshotMetadataTest {
 
-    @Test(expected = NullPointerException.class)
-    public final void testCreateMetadataSnapshotNullInput() {
-        new FrontendShardDataTreeSnapshotMetadata(null);
+    @Test
+    public void testCreateMetadataSnapshotNullInput() {
+        assertThrows(NullPointerException.class, () -> new FrontendShardDataTreeSnapshotMetadata(null));
     }
 
     @Test
-    public final void testCreateMetadataSnapshotEmptyInput() throws Exception {
+    public void testCreateMetadataSnapshotEmptyInput() throws Exception {
         final FrontendShardDataTreeSnapshotMetadata emptyOrigSnapshot = createEmptyMetadataSnapshot();
         final FrontendShardDataTreeSnapshotMetadata emptyCopySnapshot = copy(emptyOrigSnapshot, 127);
         testMetadataSnapshotEqual(emptyOrigSnapshot, emptyCopySnapshot);
     }
 
     @Test
-    public final void testSerializeMetadataSnapshotWithOneClient() throws Exception {
+    public void testSerializeMetadataSnapshotWithOneClient() throws Exception {
         final FrontendShardDataTreeSnapshotMetadata origSnapshot = createMetadataSnapshot(1);
         final FrontendShardDataTreeSnapshotMetadata copySnapshot = copy(origSnapshot, 162);
         testMetadataSnapshotEqual(origSnapshot, copySnapshot);
     }
 
     @Test
-    public final void testSerializeMetadataSnapshotWithMoreClients() throws Exception {
+    public void testSerializeMetadataSnapshotWithMoreClients() throws Exception {
         final FrontendShardDataTreeSnapshotMetadata origSnapshot = createMetadataSnapshot(5);
         final FrontendShardDataTreeSnapshotMetadata copySnapshot = copy(origSnapshot, 314);
         testMetadataSnapshotEqual(origSnapshot, copySnapshot);
@@ -68,7 +68,7 @@ public class FrontendShardDataTreeSnapshotMetadataTest {
         final List<FrontendClientMetadata> origClientList = origSnapshot.getClients();
         final List<FrontendClientMetadata> copyClientList = copySnapshot.getClients();
 
-        assertTrue(origClientList.size() == copyClientList.size());
+        assertEquals(origClientList.size(), copyClientList.size());
 
         final Map<ClientIdentifier, FrontendClientMetadata> origIdent = new HashMap<>();
         final Map<ClientIdentifier, FrontendClientMetadata> copyIdent = new HashMap<>();
@@ -81,13 +81,13 @@ public class FrontendShardDataTreeSnapshotMetadataTest {
         origIdent.values().forEach(client -> {
             final FrontendClientMetadata copyClient = copyIdent.get(client.getIdentifier());
             testObject(client.getIdentifier(), copyClient.getIdentifier());
-            assertTrue(client.getPurgedHistories().equals(copyClient.getPurgedHistories()));
-            assertTrue(client.getCurrentHistories().equals(copyClient.getCurrentHistories()));
+            assertEquals(client.getPurgedHistories(), copyClient.getPurgedHistories());
+            assertEquals(client.getCurrentHistories(), copyClient.getCurrentHistories());
         });
     }
 
     private static FrontendShardDataTreeSnapshotMetadata createEmptyMetadataSnapshot() {
-        return new FrontendShardDataTreeSnapshotMetadata(Collections.<FrontendClientMetadata>emptyList());
+        return new FrontendShardDataTreeSnapshotMetadata(List.of());
     }
 
     private static FrontendShardDataTreeSnapshotMetadata createMetadataSnapshot(final int size) {
@@ -106,9 +106,9 @@ public class FrontendShardDataTreeSnapshotMetadataTest {
         final ClientIdentifier clientIdentifier = ClientIdentifier.create(frontendIdentifier, num);
 
         final RangeSet<UnsignedLong> purgedHistories = TreeRangeSet.create();
-        purgedHistories.add(Range.closed(UnsignedLong.ZERO, UnsignedLong.ONE));
+        purgedHistories.add(Range.closedOpen(UnsignedLong.ZERO, UnsignedLong.ONE));
 
-        final Collection<FrontendHistoryMetadata> currentHistories = Collections.singleton(
+        final Set<FrontendHistoryMetadata> currentHistories = Set.of(
             new FrontendHistoryMetadata(num, num, true, ImmutableMap.of(UnsignedLong.ZERO, Boolean.TRUE),
                 purgedHistories));