From 833611f1df5e9cd886d82c49bd7d597736d062a3 Mon Sep 17 00:00:00 2001 From: Julian Knutsen Date: Fri, 15 Nov 2019 09:10:21 -0800 Subject: [PATCH] [PR COMMENTS] Make maxSequenceNumberBeforePurge final 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. --- .../main/java/bisq/network/p2p/P2PModule.java | 1 + .../network/p2p/storage/P2PDataStorage.java | 11 ++++++--- .../P2PDataStorageRemoveExpiredTest.java | 2 +- .../bisq/network/p2p/storage/TestState.java | 24 ++++--------------- 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/p2p/src/main/java/bisq/network/p2p/P2PModule.java b/p2p/src/main/java/bisq/network/p2p/P2PModule.java index 6356515f18b..723c70fbdce 100644 --- a/p2p/src/main/java/bisq/network/p2p/P2PModule.java +++ b/p2p/src/main/java/bisq/network/p2p/P2PModule.java @@ -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); } } 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 04313763d17..7ed3ee77f6f 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -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; @@ -121,7 +123,9 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers private final Set 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 @@ -134,12 +138,14 @@ public P2PDataStorage(NetworkNode networkNode, ProtectedDataStoreService protectedDataStoreService, ResourceDataStoreService resourceDataStoreService, Storage 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); @@ -147,7 +153,6 @@ public P2PDataStorage(NetworkNode networkNode, this.sequenceNumberMapStorage = sequenceNumberMapStorage; sequenceNumberMapStorage.setNumMaxBackupFiles(5); - this.maxSequenceNumberMapSizeBeforePurge = 1000; } @Override diff --git a/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRemoveExpiredTest.java b/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRemoveExpiredTest.java index 7814004cd2e..aec700c8ab5 100644 --- a/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRemoveExpiredTest.java +++ b/p2p/src/test/java/bisq/network/p2p/storage/P2PDataStorageRemoveExpiredTest.java @@ -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); 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 b13ec2a97a4..942cd668e1c 100644 --- a/p2p/src/test/java/bisq/network/p2p/storage/TestState.java +++ b/p2p/src/test/java/bisq/network/p2p/storage/TestState.java @@ -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; @@ -72,34 +74,16 @@ public class TestState { private final Storage 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 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);