Skip to content

Commit

Permalink
[PR COMMENTS] Make maxSequenceNumberBeforePurge final
Browse files Browse the repository at this point in the history
Instead of using a subclass that overwrites a value, utilize Guice
to inject the real value of 10000 in the app and let the tests overwrite
it with their own.
  • Loading branch information
julianknutsen committed Nov 18, 2019
1 parent 906a252 commit 833611f
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 24 deletions.
1 change: 1 addition & 0 deletions p2p/src/main/java/bisq/network/p2p/P2PModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,6 @@ protected void configure() {
bindConstant().annotatedWith(named(NetworkOptionKeys.MSG_THROTTLE_PER_10_SEC)).to(environment.getRequiredProperty(NetworkOptionKeys.MSG_THROTTLE_PER_10_SEC));
bindConstant().annotatedWith(named(NetworkOptionKeys.SEND_MSG_THROTTLE_TRIGGER)).to(environment.getRequiredProperty(NetworkOptionKeys.SEND_MSG_THROTTLE_TRIGGER));
bindConstant().annotatedWith(named(NetworkOptionKeys.SEND_MSG_THROTTLE_SLEEP)).to(environment.getRequiredProperty(NetworkOptionKeys.SEND_MSG_THROTTLE_SLEEP));
bindConstant().annotatedWith(named("MAX_SEQUENCE_NUMBER_MAP_SIZE_BEFORE_PURGE")).to(10000);
}
}
11 changes: 8 additions & 3 deletions p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@

import com.google.protobuf.ByteString;

import com.google.inject.name.Named;

import javax.inject.Inject;

import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -121,7 +123,9 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
private final Set<ProtectedDataStoreListener> protectedDataStoreListeners = new CopyOnWriteArraySet<>();
private final Clock clock;

protected int maxSequenceNumberMapSizeBeforePurge;
/// The maximum number of items that must exist in the SequenceNumberMap before it is scheduled for a purge
/// which removes entries after PURGE_AGE_DAYS.
private final int maxSequenceNumberMapSizeBeforePurge;

///////////////////////////////////////////////////////////////////////////////////////////
// Constructor
Expand All @@ -134,20 +138,21 @@ public P2PDataStorage(NetworkNode networkNode,
ProtectedDataStoreService protectedDataStoreService,
ResourceDataStoreService resourceDataStoreService,
Storage<SequenceNumberMap> sequenceNumberMapStorage,
Clock clock) {
Clock clock,
@Named("MAX_SEQUENCE_NUMBER_MAP_SIZE_BEFORE_PURGE") int maxSequenceNumberBeforePurge) {
this.broadcaster = broadcaster;
this.appendOnlyDataStoreService = appendOnlyDataStoreService;
this.protectedDataStoreService = protectedDataStoreService;
this.resourceDataStoreService = resourceDataStoreService;
this.clock = clock;
this.maxSequenceNumberMapSizeBeforePurge = maxSequenceNumberBeforePurge;


networkNode.addMessageListener(this);
networkNode.addConnectionListener(this);

this.sequenceNumberMapStorage = sequenceNumberMapStorage;
sequenceNumberMapStorage.setNumMaxBackupFiles(5);
this.maxSequenceNumberMapSizeBeforePurge = 1000;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public void removeExpiredEntries_PurgeSeqNrMap() throws CryptoException, NoSuchA

Assert.assertTrue(testState.mockedStorage.addProtectedStorageEntry(purgedProtectedStorageEntry, TestState.getTestNodeAddress(), null, true));

for (int i = 0; i < 4; ++i) {
for (int i = 0; i < MAX_SEQUENCE_NUMBER_MAP_SIZE_BEFORE_PURGE - 1; ++i) {
KeyPair ownerKeys = TestUtils.generateKeyPair();
ProtectedStoragePayload protectedStoragePayload = new PersistableExpirableProtectedStoragePayloadStub(ownerKeys.getPublic(), 0);
ProtectedStorageEntry tmpEntry = testState.mockedStorage.getProtectedStorageEntry(protectedStoragePayload, ownerKeys);
Expand Down
24 changes: 4 additions & 20 deletions p2p/src/test/java/bisq/network/p2p/storage/TestState.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@
* Used in the P2PDataStorage*Test(s) in order to leverage common test set up and validation.
*/
public class TestState {
static final int MAX_SEQUENCE_NUMBER_MAP_SIZE_BEFORE_PURGE = 5;

final P2PDataStorage mockedStorage;
final Broadcaster mockBroadcaster;

Expand All @@ -72,34 +74,16 @@ public class TestState {
private final Storage<SequenceNumberMap> mockSeqNrStorage;
final ClockFake clockFake;

/**
* Subclass of P2PDataStorage that allows for easier testing, but keeps all functionality
*/
static class P2PDataStorageForTest extends P2PDataStorage {

P2PDataStorageForTest(NetworkNode networkNode,
Broadcaster broadcaster,
AppendOnlyDataStoreService appendOnlyDataStoreService,
ProtectedDataStoreService protectedDataStoreService,
ResourceDataStoreService resourceDataStoreService,
Storage<SequenceNumberMap> sequenceNumberMapStorage,
Clock clock) {
super(networkNode, broadcaster, appendOnlyDataStoreService, protectedDataStoreService, resourceDataStoreService, sequenceNumberMapStorage, clock);

this.maxSequenceNumberMapSizeBeforePurge = 5;
}
}

TestState() {
this.mockBroadcaster = mock(Broadcaster.class);
this.mockSeqNrStorage = mock(Storage.class);
this.clockFake = new ClockFake();

this.mockedStorage = new P2PDataStorageForTest(mock(NetworkNode.class),
this.mockedStorage = new P2PDataStorage(mock(NetworkNode.class),
this.mockBroadcaster,
new AppendOnlyDataStoreServiceFake(),
new ProtectedDataStoreServiceFake(), mock(ResourceDataStoreService.class),
this.mockSeqNrStorage, this.clockFake);
this.mockSeqNrStorage, this.clockFake, MAX_SEQUENCE_NUMBER_MAP_SIZE_BEFORE_PURGE);

this.appendOnlyDataStoreListener = mock(AppendOnlyDataStoreListener.class);
this.protectedDataStoreListener = mock(ProtectedDataStoreListener.class);
Expand Down

0 comments on commit 833611f

Please sign in to comment.