Skip to content

Commit

Permalink
[TESTS] Regression test for #3629
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
julianknutsen committed Nov 19, 2019
1 parent a8139f3 commit 849155a
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
private Timer removeExpiredEntriesTimer;

private final Storage<SequenceNumberMap> sequenceNumberMapStorage;
private final SequenceNumberMap sequenceNumberMap = new SequenceNumberMap();

@VisibleForTesting
final SequenceNumberMap sequenceNumberMap = new SequenceNumberMap();

private final Set<AppendOnlyDataStoreListener> appendOnlyDataStoreListeners = new CopyOnWriteArraySet<>();
private final Set<ProtectedDataStoreListener> protectedDataStoreListeners = new CopyOnWriteArraySet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -571,6 +572,23 @@ protected Class<ProtectedStorageEntry> 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<P2PDataStorage.ByteArray, ProtectedStorageEntry> beforeRestart = this.testState.mockedStorage.getMap();

this.testState.simulateRestart();

// Should be equal
Assert.assertNotEquals(beforeRestart, this.testState.mockedStorage.getMap());
}
}

/**
Expand Down
64 changes: 59 additions & 5 deletions p2p/src/test/java/bisq/network/p2p/storage/TestState.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<SequenceNumberMap> 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<SequenceNumberMap> 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() {
Expand Down

0 comments on commit 849155a

Please sign in to comment.