From 849155a92af779842c3bda8e5160effe3646a622 Mon Sep 17 00:00:00 2001 From: Julian Knutsen Date: Tue, 19 Nov 2019 11:04:44 -0800 Subject: [PATCH] [TESTS] Regression test for #3629 Write a test that shows the incorrect behavior for #3629, the hashmap is rebuilt from disk using the 20-byte key instead of the 32-byte key. --- .../network/p2p/storage/P2PDataStorage.java | 4 +- ...PDataStorageProtectedStorageEntryTest.java | 18 ++++++ .../bisq/network/p2p/storage/TestState.java | 64 +++++++++++++++++-- 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java index 3bfaa946727..ce69d64d8e6 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -121,7 +121,9 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers private Timer removeExpiredEntriesTimer; private final Storage sequenceNumberMapStorage; - private final SequenceNumberMap sequenceNumberMap = new SequenceNumberMap(); + + @VisibleForTesting + final SequenceNumberMap sequenceNumberMap = new SequenceNumberMap(); private final Set appendOnlyDataStoreListeners = new CopyOnWriteArraySet<>(); private final Set protectedDataStoreListeners = new CopyOnWriteArraySet<>(); diff --git a/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageProtectedStorageEntryTest.java b/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageProtectedStorageEntryTest.java index 82e58623ca8..1f582272cb2 100644 --- a/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageProtectedStorageEntryTest.java +++ b/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageProtectedStorageEntryTest.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Optional; import org.junit.Assert; @@ -571,6 +572,23 @@ protected Class getEntryClass() { return ProtectedStorageEntry.class; } + + // Tests that just apply to PersistablePayload objects + + // XXXBUG_3629XXX: Persisted ProtectedStorageEntries are saved to disk via their 20-byte hash. This causes + // the internal hash map to be reloaded with the 20-byte key instead of the 32-byte key. + @Test + public void addProtectedStorageEntry_afterReadFromResourcesWithDuplicate_3629RegressionTest() { + ProtectedStorageEntry protectedStorageEntry = this.getProtectedStorageEntryForAdd(1); + doProtectedStorageAddAndVerify(protectedStorageEntry, true, true); + + Map beforeRestart = this.testState.mockedStorage.getMap(); + + this.testState.simulateRestart(); + + // Should be equal + Assert.assertNotEquals(beforeRestart, this.testState.mockedStorage.getMap()); + } } /** diff --git a/p2p/src/test/java/bisq/network/p2p/storage/TestState.java b/p2p/src/test/java/bisq/network/p2p/storage/TestState.java index 8e74b5845f8..7eef681236d 100644 --- a/p2p/src/test/java/bisq/network/p2p/storage/TestState.java +++ b/p2p/src/test/java/bisq/network/p2p/storage/TestState.java @@ -36,6 +36,7 @@ import bisq.network.p2p.storage.payload.ProtectedStorageEntry; import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreListener; import bisq.network.p2p.storage.persistence.ProtectedDataStoreListener; +import bisq.network.p2p.storage.persistence.ProtectedDataStoreService; import bisq.network.p2p.storage.persistence.ResourceDataStoreService; import bisq.network.p2p.storage.persistence.SequenceNumberMap; @@ -65,33 +66,86 @@ public class TestState { static final int MAX_SEQUENCE_NUMBER_MAP_SIZE_BEFORE_PURGE = 5; - final P2PDataStorage mockedStorage; + P2PDataStorage mockedStorage; final Broadcaster mockBroadcaster; final AppendOnlyDataStoreListener appendOnlyDataStoreListener; private final ProtectedDataStoreListener protectedDataStoreListener; private final HashMapChangedListener hashMapChangedListener; private final Storage mockSeqNrStorage; + private final ProtectedDataStoreService protectedDataStoreService; final ClockFake clockFake; TestState() { this.mockBroadcaster = mock(Broadcaster.class); this.mockSeqNrStorage = mock(Storage.class); this.clockFake = new ClockFake(); + this.protectedDataStoreService = new ProtectedDataStoreServiceFake(); this.mockedStorage = new P2PDataStorage(mock(NetworkNode.class), this.mockBroadcaster, new AppendOnlyDataStoreServiceFake(), - new ProtectedDataStoreServiceFake(), mock(ResourceDataStoreService.class), + this.protectedDataStoreService, mock(ResourceDataStoreService.class), this.mockSeqNrStorage, this.clockFake, MAX_SEQUENCE_NUMBER_MAP_SIZE_BEFORE_PURGE); this.appendOnlyDataStoreListener = mock(AppendOnlyDataStoreListener.class); this.protectedDataStoreListener = mock(ProtectedDataStoreListener.class); this.hashMapChangedListener = mock(HashMapChangedListener.class); - this.mockedStorage.addHashMapChangedListener(this.hashMapChangedListener); - this.mockedStorage.addAppendOnlyDataStoreListener(this.appendOnlyDataStoreListener); - this.mockedStorage.addProtectedDataStoreListener(this.protectedDataStoreListener); + this.mockedStorage = createP2PDataStorageForTest( + this.mockBroadcaster, + this.protectedDataStoreService, + this.mockSeqNrStorage, + this.clockFake, + this.hashMapChangedListener, + this.appendOnlyDataStoreListener, + this.protectedDataStoreListener); + } + + + /** + * Re-initializes the in-memory data structures from the storage objects to simulate a node restarting. Important + * to note that the current TestState uses Test Doubles instead of actual disk storage so this is just "simulating" + * not running the entire storage code paths. + */ + void simulateRestart() { + when(this.mockSeqNrStorage.initAndGetPersisted(any(SequenceNumberMap.class), anyLong())) + .thenReturn(this.mockedStorage.sequenceNumberMap); + + this.mockedStorage = createP2PDataStorageForTest( + this.mockBroadcaster, + this.protectedDataStoreService, + this.mockSeqNrStorage, + this.clockFake, + this.hashMapChangedListener, + this.appendOnlyDataStoreListener, + this.protectedDataStoreListener); + } + + private static P2PDataStorage createP2PDataStorageForTest( + Broadcaster broadcaster, + ProtectedDataStoreService protectedDataStoreService, + Storage sequenceNrMapStorage, + ClockFake clock, + HashMapChangedListener hashMapChangedListener, + AppendOnlyDataStoreListener appendOnlyDataStoreListener, + ProtectedDataStoreListener protectedDataStoreListener) { + + P2PDataStorage p2PDataStorage = new P2PDataStorage(mock(NetworkNode.class), + broadcaster, + new AppendOnlyDataStoreServiceFake(), + protectedDataStoreService, mock(ResourceDataStoreService.class), + sequenceNrMapStorage, clock, MAX_SEQUENCE_NUMBER_MAP_SIZE_BEFORE_PURGE); + + // Currently TestState only supports reading ProtectedStorageEntries off disk. + p2PDataStorage.readFromResources("unused"); + p2PDataStorage.readPersisted(); + + p2PDataStorage.addHashMapChangedListener(hashMapChangedListener); + p2PDataStorage.addAppendOnlyDataStoreListener(appendOnlyDataStoreListener); + p2PDataStorage.addProtectedDataStoreListener(protectedDataStoreListener); + + return p2PDataStorage; } private void resetState() {