diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java index c160fbcc59..1c4a9f9301 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java @@ -53,6 +53,8 @@ import java.util.Optional; import lombok.Value; import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; + import javax.annotation.Nullable; import static com.google.common.base.Preconditions.checkNotNull; @@ -63,7 +65,7 @@ import static com.google.common.base.Preconditions.checkNotNull; @Slf4j @Value -public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayload, PersistableNetworkPayload, PersistableEnvelope, CapabilityRequiringPayload { +public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayload, PersistableNetworkPayload, PersistableEnvelope, CapabilityRequiringPayload, Comparable { public static final String MEDIATOR_ADDRESS = "medAddr"; public static final String REFUND_AGENT_ADDRESS = "refAddr"; @@ -282,10 +284,30 @@ public final class TradeStatistics2 implements ProcessOncePersistableNetworkPayl // Since the trade wasn't executed it's better to filter it out to avoid it having an undue influence on the // BSQ trade stats. boolean excludedFailedTrade = offerId.equals("6E5KOI6O-3a06a037-6f03-4bfa-98c2-59f49f73466a-112"); - boolean depositTxIdValid = depositTxId == null || (tradeDate < CUT_OFF_DATE_FOR_DEPOSIT_TX_ID.getTime() && !depositTxId.isEmpty()); + boolean depositTxIdValid = depositTxId == null || !depositTxId.isEmpty(); return tradeAmount > 0 && tradePrice > 0 && !excludedFailedTrade && depositTxIdValid; } + // TODO: Can be removed as soon as everyone uses v1.2.6+ + @Override + public int compareTo(@NotNull TradeStatistics2 o) { + if (direction.equals(o.direction) && + baseCurrency.equals(o.baseCurrency) && + counterCurrency.equals(o.counterCurrency) && + offerPaymentMethod.equals(o.offerPaymentMethod) && + offerDate == o.offerDate && + offerUseMarketBasedPrice == o.offerUseMarketBasedPrice && + offerAmount == o.offerAmount && + offerMinAmount == o.offerMinAmount && + offerId.equals(o.offerId) && + tradePrice == o.tradePrice && + tradeAmount == o.tradeAmount) { + return 0; + } + + return -1; + } + @Override public String toString() { return "TradeStatistics2{" + diff --git a/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java b/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java index 0ef608aa2c..ee0fbc03d4 100644 --- a/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java +++ b/core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java @@ -30,6 +30,7 @@ import bisq.common.storage.JsonFileManager; import bisq.common.util.Utilities; import com.google.inject.Inject; + import javax.inject.Named; import javafx.collections.FXCollections; @@ -39,6 +40,7 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -93,9 +95,22 @@ public class TradeStatisticsManager { } private void addToSet(TradeStatistics2 tradeStatistics) { + if (!observableTradeStatisticsSet.contains(tradeStatistics)) { - if (observableTradeStatisticsSet.stream().anyMatch(e -> e.getOfferId().equals(tradeStatistics.getOfferId()))) { - return; + Optional duplicate = observableTradeStatisticsSet.stream().filter( + e -> e.getOfferId().equals(tradeStatistics.getOfferId())).findAny(); + + if (duplicate.isPresent()) { + // TODO: Can be removed as soon as everyone uses v1.2.6+ + // Removes an existing object with a trade id if the new one matches the existing except + // for the deposit tx id + if (tradeStatistics.getDepositTxId() == null && + tradeStatistics.isValid() && + duplicate.get().compareTo(tradeStatistics) == 0) { + observableTradeStatisticsSet.remove(duplicate.get()); + } else { + return; + } } if (!tradeStatistics.isValid()) { diff --git a/core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Maker.java b/core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Maker.java index ae93ac58cc..3c49ce2d5a 100644 --- a/core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Maker.java +++ b/core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Maker.java @@ -36,6 +36,7 @@ public class TradeStatistics2Maker { public static final Property date = new Property<>(); public static final Property depositTxId = new Property<>(); + public static final Property tradeAmount = new Property<>(); public static final Instantiator TradeStatistic2 = lookup -> { Calendar calendar = Calendar.getInstance(); @@ -81,7 +82,7 @@ public class TradeStatistics2Maker { null, 0), Price.valueOf("BTC", 100000L), - Coin.SATOSHI, + lookup.valueOf(tradeAmount, Coin.SATOSHI), lookup.valueOf(date, new Date(calendar.getTimeInMillis())), lookup.valueOf(depositTxId, "123456"), Collections.emptyMap()); diff --git a/core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Test.java b/core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Test.java index 67f128c9be..725d18d5c7 100644 --- a/core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Test.java +++ b/core/src/test/java/bisq/core/trade/statistics/TradeStatistics2Test.java @@ -17,24 +17,19 @@ package bisq.core.trade.statistics; -import org.apache.commons.lang3.time.DateUtils; - import org.junit.Test; -import static bisq.core.trade.statistics.TradeStatistics2Maker.date; import static bisq.core.trade.statistics.TradeStatistics2Maker.dayZeroTrade; import static bisq.core.trade.statistics.TradeStatistics2Maker.depositTxId; import static com.natpryce.makeiteasy.MakeItEasy.make; -import static com.natpryce.makeiteasy.MakeItEasy.with; import static com.natpryce.makeiteasy.MakeItEasy.withNull; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class TradeStatistics2Test { @Test - public void isValid_WithDepositTxIdBeforeCutOffDate() { + public void isValid_WithDepositTxId() { TradeStatistics2 tradeStatistic = make(dayZeroTrade); @@ -42,20 +37,8 @@ public class TradeStatistics2Test { } @Test - public void isValid_Not_WithDepositTxIdAfterCutOffDate() { - TradeStatistics2 tradeStatistic = make(dayZeroTrade.but( - with(date, DateUtils.addDays(TradeStatistics2.CUT_OFF_DATE_FOR_DEPOSIT_TX_ID, 1)) - )); - - assertFalse(tradeStatistic.isValid()); - } - - @Test - public void isValid_WithEmptyDepositTxIdAfterCutOffDate() { - TradeStatistics2 tradeStatistic = make(dayZeroTrade.but( - with(date, DateUtils.addDays(TradeStatistics2.CUT_OFF_DATE_FOR_DEPOSIT_TX_ID, 1)), - withNull(depositTxId) - )); + public void isValid_WithEmptyDepositTxId() { + TradeStatistics2 tradeStatistic = make(dayZeroTrade.but(withNull(depositTxId))); assertTrue(tradeStatistic.isValid()); } diff --git a/core/src/test/java/bisq/core/trade/statistics/TradeStatisticsManagerTest.java b/core/src/test/java/bisq/core/trade/statistics/TradeStatisticsManagerTest.java new file mode 100644 index 0000000000..44b84d1384 --- /dev/null +++ b/core/src/test/java/bisq/core/trade/statistics/TradeStatisticsManagerTest.java @@ -0,0 +1,114 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.core.trade.statistics; + +import bisq.core.provider.price.PriceFeedService; + +import bisq.network.p2p.P2PService; +import bisq.network.p2p.storage.P2PDataStorage; +import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreListener; +import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreService; + +import org.bitcoinj.core.Coin; + +import java.io.File; + +import org.mockito.ArgumentCaptor; + +import org.junit.Before; +import org.junit.Test; + +import static bisq.core.trade.statistics.TradeStatistics2Maker.dayZeroTrade; +import static bisq.core.trade.statistics.TradeStatistics2Maker.depositTxId; +import static bisq.core.trade.statistics.TradeStatistics2Maker.tradeAmount; +import static com.natpryce.makeiteasy.MakeItEasy.make; +import static com.natpryce.makeiteasy.MakeItEasy.with; +import static com.natpryce.makeiteasy.MakeItEasy.withNull; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class TradeStatisticsManagerTest { + + private TradeStatisticsManager manager; + private TradeStatistics2 tradeWithNullDepositTxId; + private ArgumentCaptor listenerArgumentCaptor; + + @Before + public void prepareMocksAndObjects() { + P2PService p2PService = mock(P2PService.class); + P2PDataStorage p2PDataStorage = mock(P2PDataStorage.class); + File storageDir = mock(File.class); + TradeStatistics2StorageService tradeStatistics2StorageService = mock(TradeStatistics2StorageService.class); + PriceFeedService priceFeedService = mock(PriceFeedService.class); + + AppendOnlyDataStoreService appendOnlyDataStoreService = mock(AppendOnlyDataStoreService.class); + when(p2PService.getP2PDataStorage()).thenReturn(p2PDataStorage); + + manager = new TradeStatisticsManager(p2PService, priceFeedService, + tradeStatistics2StorageService, appendOnlyDataStoreService, storageDir, false); + + tradeWithNullDepositTxId = make(dayZeroTrade.but(withNull(depositTxId))); + + manager.onAllServicesInitialized(); + listenerArgumentCaptor = ArgumentCaptor.forClass(AppendOnlyDataStoreListener.class); + verify(p2PDataStorage).addAppendOnlyDataStoreListener(listenerArgumentCaptor.capture()); + + } + + @Test + public void addToSet_ObjectWithNullDepositTxId() { + listenerArgumentCaptor.getValue().onAdded(tradeWithNullDepositTxId); + assertTrue(manager.getObservableTradeStatisticsSet().contains(tradeWithNullDepositTxId)); + } + + @Test + public void addToSet_RemoveExistingObjectIfObjectWithNullDepositTxIdIsAdded() { + TradeStatistics2 tradeWithDepositTxId = make(dayZeroTrade); + + listenerArgumentCaptor.getValue().onAdded(tradeWithDepositTxId); + listenerArgumentCaptor.getValue().onAdded(tradeWithNullDepositTxId); + + assertFalse(manager.getObservableTradeStatisticsSet().contains(tradeWithDepositTxId)); + assertTrue(manager.getObservableTradeStatisticsSet().contains(tradeWithNullDepositTxId)); + } + + @Test + public void addToSet_NotRemoveExistingObjectIfObjectsNotEqual() { + TradeStatistics2 tradeWithDepositTxId = make(dayZeroTrade.but(with(tradeAmount, Coin.FIFTY_COINS))); + + listenerArgumentCaptor.getValue().onAdded(tradeWithDepositTxId); + listenerArgumentCaptor.getValue().onAdded(tradeWithNullDepositTxId); + + assertTrue(manager.getObservableTradeStatisticsSet().contains(tradeWithDepositTxId)); + assertFalse(manager.getObservableTradeStatisticsSet().contains(tradeWithNullDepositTxId)); + } + + @Test + public void addToSet_IgnoreObjectIfObjectWithNullDepositTxIdAlreadyExists() { + TradeStatistics2 tradeWithDepositTxId = make(dayZeroTrade); + + listenerArgumentCaptor.getValue().onAdded(tradeWithNullDepositTxId); + listenerArgumentCaptor.getValue().onAdded(tradeWithDepositTxId); + + assertTrue(manager.getObservableTradeStatisticsSet().contains(tradeWithNullDepositTxId)); + assertFalse(manager.getObservableTradeStatisticsSet().contains(tradeWithDepositTxId)); + } +}