From 24108cdcdebe73d07055488075f90c35aa01b698 Mon Sep 17 00:00:00 2001 From: Manfred Karrer Date: Tue, 16 Sep 2014 15:05:05 +0200 Subject: [PATCH] Fix remove offer bug. Changed polling, fix problems with empty offer list. --- .../gui/main/trade/orderbook/OrderBook.java | 19 ++- .../main/trade/orderbook/OrderBookModel.java | 7 +- .../java/io/bitsquare/msg/MessageFacade.java | 160 +++++++++--------- .../msg/listeners/AddOfferListener.java | 24 +++ src/main/java/io/bitsquare/trade/Offer.java | 16 +- .../java/io/bitsquare/trade/TradeManager.java | 2 +- .../createoffer/tasks/PublishOfferToDHT.java | 3 +- 7 files changed, 130 insertions(+), 101 deletions(-) create mode 100644 src/main/java/io/bitsquare/msg/listeners/AddOfferListener.java diff --git a/src/main/java/io/bitsquare/gui/main/trade/orderbook/OrderBook.java b/src/main/java/io/bitsquare/gui/main/trade/orderbook/OrderBook.java index 1a6a92e223..3f62c467e6 100644 --- a/src/main/java/io/bitsquare/gui/main/trade/orderbook/OrderBook.java +++ b/src/main/java/io/bitsquare/gui/main/trade/orderbook/OrderBook.java @@ -54,7 +54,7 @@ public class OrderBook { private final ObservableList orderBookListItems = FXCollections.observableArrayList(); private final OrderBookListener orderBookListener; private final ChangeListener bankAccountChangeListener; - private final ChangeListener dirtyListener; + private final ChangeListener invalidationListener; private String fiatCode; private AnimationTimer pollingTimer; private Country country; @@ -71,7 +71,11 @@ public class OrderBook { this.user = user; bankAccountChangeListener = (observableValue, oldValue, newValue) -> setBankAccount(newValue); - dirtyListener = (ov, oldValue, newValue) -> requestOffers(); + invalidationListener = (ov, oldValue, newValue) -> { + log.debug("#### oldValue " + oldValue); + log.debug("#### newValue " + newValue); + requestOffers(); + }; orderBookListener = new OrderBookListener() { @Override public void onOfferAdded(Offer offer) { @@ -87,7 +91,7 @@ public class OrderBook { @Override public void onOfferRemoved(Offer offer) { - orderBookListItems.removeIf(item -> item.getOffer().equals(offer)); + orderBookListItems.removeIf(item -> item.getOffer().getId().equals(offer.getId())); } }; } @@ -137,15 +141,17 @@ public class OrderBook { } private void addListeners() { + log.trace("addListeners "); user.currentBankAccountProperty().addListener(bankAccountChangeListener); messageFacade.addOrderBookListener(orderBookListener); - messageFacade.getIsDirtyProperty().addListener(dirtyListener); + messageFacade.invalidationTimestampProperty().addListener(invalidationListener); } private void removeListeners() { + log.trace("removeListeners "); user.currentBankAccountProperty().removeListener(bankAccountChangeListener); messageFacade.removeOrderBookListener(orderBookListener); - messageFacade.getIsDirtyProperty().removeListener(dirtyListener); + messageFacade.invalidationTimestampProperty().removeListener(invalidationListener); } private void addOfferToOrderBookListItems(Offer offer) { @@ -155,6 +161,7 @@ public class OrderBook { } private void requestOffers() { + log.debug("requestOffers"); messageFacade.getOffers(fiatCode); } @@ -168,7 +175,7 @@ public class OrderBook { addListeners(); setBankAccount(user.getCurrentBankAccount()); pollingTimer = Utilities.setInterval(1000, (animationTimer) -> { - messageFacade.getDirtyFlag(fiatCode); + messageFacade.getInvalidationTimeStamp(fiatCode); return null; }); diff --git a/src/main/java/io/bitsquare/gui/main/trade/orderbook/OrderBookModel.java b/src/main/java/io/bitsquare/gui/main/trade/orderbook/OrderBookModel.java index 9e4f5c093e..2a5b1ee42c 100644 --- a/src/main/java/io/bitsquare/gui/main/trade/orderbook/OrderBookModel.java +++ b/src/main/java/io/bitsquare/gui/main/trade/orderbook/OrderBookModel.java @@ -64,7 +64,7 @@ public class OrderBookModel extends UIModel { private final FilteredList filteredItems; private final SortedList sortedItems; private OrderBookInfo orderBookInfo; - private final ChangeListener bankAccountChangeListener; + private ChangeListener bankAccountChangeListener; private final ObjectProperty amountAsCoin = new SimpleObjectProperty<>(); private final ObjectProperty priceAsFiat = new SimpleObjectProperty<>(); @@ -93,16 +93,16 @@ public class OrderBookModel extends UIModel { filteredItems = new FilteredList<>(orderBook.getOrderBookListItems()); sortedItems = new SortedList<>(filteredItems); - bankAccountChangeListener = (observableValue, oldValue, newValue) -> setBankAccount(newValue); } /////////////////////////////////////////////////////////////////////////////////////////// // Lifecycle /////////////////////////////////////////////////////////////////////////////////////////// - @SuppressWarnings("EmptyMethod") @Override public void initialize() { + bankAccountChangeListener = (observableValue, oldValue, newValue) -> setBankAccount(newValue); + super.initialize(); } @@ -176,7 +176,6 @@ public class OrderBookModel extends UIModel { } boolean isTradable(Offer offer) { - log.debug("### isMatchingRestrictions " + offer); // if user has not registered yet we display all if (user.getCurrentBankAccount() == null) return true; diff --git a/src/main/java/io/bitsquare/msg/MessageFacade.java b/src/main/java/io/bitsquare/msg/MessageFacade.java index e823148f08..8fba26c681 100644 --- a/src/main/java/io/bitsquare/msg/MessageFacade.java +++ b/src/main/java/io/bitsquare/msg/MessageFacade.java @@ -18,6 +18,7 @@ package io.bitsquare.msg; import io.bitsquare.arbitrator.Arbitrator; +import io.bitsquare.msg.listeners.AddOfferListener; import io.bitsquare.msg.listeners.ArbitratorListener; import io.bitsquare.msg.listeners.BootstrapListener; import io.bitsquare.msg.listeners.GetPeerAddressListener; @@ -44,8 +45,8 @@ import javax.annotation.Nullable; import javax.inject.Inject; import javafx.application.Platform; -import javafx.beans.property.BooleanProperty; -import javafx.beans.property.SimpleBooleanProperty; +import javafx.beans.property.LongProperty; +import javafx.beans.property.SimpleLongProperty; import net.tomp2p.dht.FutureGet; import net.tomp2p.dht.FuturePut; @@ -73,27 +74,16 @@ import org.slf4j.LoggerFactory; * TODO: improve callbacks that Platform.runLater is not necessary. We call usually that methods form teh UI thread. */ public class MessageFacade implements MessageBroker { - - public static interface AddOfferListener { - void onComplete(); - - void onFailed(String reason, Throwable throwable); - } - private static final Logger log = LoggerFactory.getLogger(MessageFacade.class); private static final String ARBITRATORS_ROOT = "ArbitratorsRoot"; - public P2PNode getP2pNode() { - return p2pNode; - } - private final P2PNode p2pNode; + private final User user; private final List orderBookListeners = new ArrayList<>(); private final List arbitratorListeners = new ArrayList<>(); private final List incomingTradeMessageListeners = new ArrayList<>(); - private final User user; - private SeedNodeAddress.StaticSeedNodeAddresses defaultStaticSeedNodeAddresses; + private final LongProperty invalidationTimestamp = new SimpleLongProperty(0); /////////////////////////////////////////////////////////////////////////////////////////// @@ -174,13 +164,11 @@ public class MessageFacade implements MessageBroker { try { final Data offerData = new Data(offer); - log.trace("Add offer to DHT requested. Added data: [locationKey: " + locationKey + - ", value: " + offerData + "]"); - // the offer is default 30 days valid int defaultOfferTTL = 30 * 24 * 60 * 60; offerData.ttlSeconds(defaultOfferTTL); - + log.trace("Add offer to DHT requested. Added data: [locationKey: " + locationKey + + ", hash: " + offerData.hash().toString() + "]"); FuturePut futurePut = p2pNode.addProtectedData(locationKey, offerData); futurePut.addListener(new BaseFutureListener() { @Override @@ -201,7 +189,7 @@ public class MessageFacade implements MessageBroker { }); // TODO will be removed when we don't use polling anymore - setDirty(locationKey); + updateInvalidationTimestamp(locationKey); log.trace("Add offer to DHT was successful. Added data: [locationKey: " + locationKey + ", value: " + offerData + "]"); }); @@ -237,10 +225,8 @@ public class MessageFacade implements MessageBroker { Number160 locationKey = Number160.createHash(offer.getCurrency().getCurrencyCode()); try { final Data offerData = new Data(offer); - log.trace("Remove offer from DHT requested. Removed data: [locationKey: " + locationKey + - ", value: " + offerData + "]"); - + ", hash: " + offerData.hash().toString() + "]"); FutureRemove futureRemove = p2pNode.removeFromDataMap(locationKey, offerData); futureRemove.addListener(new BaseFutureListener() { @Override @@ -258,7 +244,7 @@ public class MessageFacade implements MessageBroker { log.error("Remove offer from DHT failed. Error: " + e.getMessage()); } }); - setDirty(locationKey); + updateInvalidationTimestamp(locationKey); }); log.trace("Remove offer from DHT was successful. Removed data: [key: " + locationKey + ", " + @@ -283,6 +269,7 @@ public class MessageFacade implements MessageBroker { public void getOffers(String currencyCode) { Number160 locationKey = Number160.createHash(currencyCode); + log.trace("Get offers from DHT requested for locationKey: " + locationKey); FutureGet futureGet = p2pNode.getDataMap(locationKey); futureGet.addListener(new BaseFutureAdapter() { @Override @@ -306,12 +293,19 @@ public class MessageFacade implements MessageBroker { listener.onOffersReceived(offers))); } - //log.trace("Get offers from DHT was successful"); - /* log.trace("Get offers from DHT was successful. Stored data: [key: " + locationKey - + ", values: " + futureGet.dataMap() + "]");*/ + log.trace("Get offers from DHT was successful. Stored data: [key: " + locationKey + + ", values: " + futureGet.dataMap() + "]"); } else { - log.error("Get offers from DHT was not successful with reason:" + baseFuture.failedReason()); + final Map dataMap = futureGet.dataMap(); + if (dataMap == null || dataMap.size() == 0) { + log.trace("Get offers from DHT delivered empty dataMap."); + Platform.runLater(() -> orderBookListeners.stream().forEach(listener -> + listener.onOffersReceived(new ArrayList<>()))); + } + else { + log.error("Get offers from DHT was not successful with reason:" + baseFuture.failedReason()); + } } } }); @@ -442,83 +436,88 @@ public class MessageFacade implements MessageBroker { public void removeIncomingTradeMessageListener(IncomingTradeMessageListener listener) { incomingTradeMessageListeners.remove(listener); } + + /* + * We store the timestamp of any change of the offer list (add, remove offer) and we poll in intervals for changes. + * If we detect a change we request the offer list from the DHT. + * Polling should be replaced by a push based solution later. + */ /////////////////////////////////////////////////////////////////////////////////////////// - // Check dirty flag for a location key + // Polling /////////////////////////////////////////////////////////////////////////////////////////// - // TODO just temporary - public BooleanProperty getIsDirtyProperty() { - return isDirty; + public void updateInvalidationTimestamp(Number160 locationKey) { + invalidationTimestamp.set(System.currentTimeMillis()); + try { + FuturePut putFuture = p2pNode.putData(getInvalidatedLocationKey(locationKey), + new Data(invalidationTimestamp.get())); + putFuture.addListener(new BaseFutureListener() { + @Override + public void operationComplete(BaseFuture future) throws Exception { + if (putFuture.isSuccess()) + log.trace("Update invalidationTimestamp to DHT was successful. TimeStamp=" + + invalidationTimestamp.get()); + else + log.error("Update invalidationTimestamp to DHT failed with reason:" + putFuture.failedReason()); + } + + @Override + public void exceptionCaught(Throwable t) throws Exception { + log.error("Update invalidationTimestamp to DHT failed with exception:" + t.getMessage()); + } + }); + } catch (IOException | ClassNotFoundException e) { + log.error("Update invalidationTimestamp to DHT failed with exception:" + e.getMessage()); + } } - public void getDirtyFlag(String currencyCode) { + public LongProperty invalidationTimestampProperty() { + return invalidationTimestamp; + } + + public void getInvalidationTimeStamp(String currencyCode) { Number160 locationKey = Number160.createHash(currencyCode); try { - FutureGet getFuture = p2pNode.getData(getDirtyLocationKey(locationKey)); + FutureGet getFuture = p2pNode.getData(getInvalidatedLocationKey(locationKey)); getFuture.addListener(new BaseFutureListener() { @Override public void operationComplete(BaseFuture future) throws Exception { - Data data = getFuture.data(); - if (data != null) { - Object object = data.object(); - if (object instanceof Long) { - Platform.runLater(() -> onGetDirtyFlag((Long) object)); + if (getFuture.isSuccess()) { + Data data = getFuture.data(); + if (data != null && data.object() instanceof Long) { + final Object object = data.object(); + Platform.runLater(() -> { + Long timeStamp = (Long) object; + log.trace("Get invalidationTimestamp from DHT was successful. TimeStamp=" + + timeStamp); + invalidationTimestamp.set(timeStamp); + }); } + else { + log.error("Get invalidationTimestamp from DHT failed. Data = " + data); + } + } + else { + log.error("Get invalidationTimestamp from DHT failed with reason:" + getFuture.failedReason()); } } @Override public void exceptionCaught(Throwable t) throws Exception { - log.error("getFuture exceptionCaught " + t.toString()); + log.error("Get invalidationTimestamp from DHT failed with exception:" + t.getMessage()); + t.printStackTrace(); } }); } catch (IOException | ClassNotFoundException e) { + log.error("Get invalidationTimestamp from DHT failed with exception:" + e.getMessage()); e.printStackTrace(); } } - private Long lastTimeStamp = -3L; - private final BooleanProperty isDirty = new SimpleBooleanProperty(false); - - private void onGetDirtyFlag(long timeStamp) { - // TODO don't get updates at first execute.... - if (lastTimeStamp != timeStamp) { - isDirty.setValue(!isDirty.get()); - } - if (lastTimeStamp > 0) { - lastTimeStamp = timeStamp; - } - else { - lastTimeStamp++; - } - } - - public void setDirty(Number160 locationKey) { - // we don't want to get an update from dirty for own changes, so update the lastTimeStamp to omit a change - // trigger - lastTimeStamp = System.currentTimeMillis(); - try { - FuturePut putFuture = p2pNode.putData(getDirtyLocationKey(locationKey), new Data(lastTimeStamp)); - putFuture.addListener(new BaseFutureListener() { - @Override - public void operationComplete(BaseFuture future) throws Exception { - // log.trace("operationComplete"); - } - - @Override - public void exceptionCaught(Throwable t) throws Exception { - log.warn("Error at writing dirty flag (timeStamp) " + t.toString()); - } - }); - } catch (IOException | ClassNotFoundException e) { - log.warn("Error at writing dirty flag (timeStamp) " + e.getMessage()); - } - } - - private Number160 getDirtyLocationKey(Number160 locationKey) { - return Number160.createHash(locationKey + "Dirty"); + private Number160 getInvalidatedLocationKey(Number160 locationKey) { + return Number160.createHash(locationKey + "invalidated"); } @@ -529,7 +528,6 @@ public class MessageFacade implements MessageBroker { @Override public void handleMessage(Object message, PeerAddress peerAddress) { if (message instanceof TradeMessage) { - log.error("####################"); Platform.runLater(() -> incomingTradeMessageListeners.stream().forEach(e -> e.onMessage((TradeMessage) message, peerAddress))); } diff --git a/src/main/java/io/bitsquare/msg/listeners/AddOfferListener.java b/src/main/java/io/bitsquare/msg/listeners/AddOfferListener.java new file mode 100644 index 0000000000..7a05c60141 --- /dev/null +++ b/src/main/java/io/bitsquare/msg/listeners/AddOfferListener.java @@ -0,0 +1,24 @@ +/* + * This file is part of Bitsquare. + * + * Bitsquare 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. + * + * Bitsquare 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 Bitsquare. If not, see . + */ + +package io.bitsquare.msg.listeners; + +public interface AddOfferListener { + void onComplete(); + + void onFailed(String reason, Throwable throwable); +} diff --git a/src/main/java/io/bitsquare/trade/Offer.java b/src/main/java/io/bitsquare/trade/Offer.java index a210dd0ed4..389c4743e4 100644 --- a/src/main/java/io/bitsquare/trade/Offer.java +++ b/src/main/java/io/bitsquare/trade/Offer.java @@ -47,7 +47,8 @@ public class Offer implements Serializable { private final Date creationDate; - private final Fiat price; + // Fiat cause problems with offer removal (don` found out why, but we want plain objects anyway) + private final long fiatPrice; private final Coin amount; private final Coin minAmount; //TODO use hex string @@ -70,7 +71,7 @@ public class Offer implements Serializable { public Offer(String id, PublicKey messagePublicKey, Direction direction, - Fiat price, + long fiatPrice, Coin amount, Coin minAmount, BankAccountType bankAccountType, @@ -84,7 +85,7 @@ public class Offer implements Serializable { this.id = id; this.messagePublicKey = messagePublicKey; this.direction = direction; - this.price = price; + this.fiatPrice = fiatPrice; this.amount = amount; this.minAmount = minAmount; this.bankAccountType = bankAccountType; @@ -119,7 +120,7 @@ public class Offer implements Serializable { } public Fiat getPrice() { - return price; + return Fiat.valueOf(currency.getCurrencyCode(), fiatPrice); } public Coin getAmount() { @@ -155,9 +156,8 @@ public class Offer implements Serializable { } public Fiat getVolumeByAmount(Coin amount) { - if (price != null && amount != null && !amount.isZero() && !price.isZero()) { - return new ExchangeRate(price).coinToFiat(amount); - } + if (fiatPrice != 0 && amount != null && !amount.isZero()) + return new ExchangeRate(Fiat.valueOf(currency.getCurrencyCode(), fiatPrice)).coinToFiat(amount); else return null; } @@ -197,7 +197,7 @@ public class Offer implements Serializable { "direction=" + direction + ", currency=" + currency + ", uid='" + id + '\'' + - ", price=" + price + + ", fiatPrice=" + fiatPrice + ", amount=" + amount + ", minAmount=" + minAmount + ", messagePubKey=" + messagePublicKey.hashCode() + diff --git a/src/main/java/io/bitsquare/trade/TradeManager.java b/src/main/java/io/bitsquare/trade/TradeManager.java index 42cf664d08..4f7efd7b7f 100644 --- a/src/main/java/io/bitsquare/trade/TradeManager.java +++ b/src/main/java/io/bitsquare/trade/TradeManager.java @@ -170,7 +170,7 @@ public class TradeManager { Offer offer = new Offer(id, user.getMessagePublicKey(), direction, - price, + price.getValue(), amount, minAmount, user.getCurrentBankAccount().getBankAccountType(), diff --git a/src/main/java/io/bitsquare/trade/protocol/createoffer/tasks/PublishOfferToDHT.java b/src/main/java/io/bitsquare/trade/protocol/createoffer/tasks/PublishOfferToDHT.java index e9f65cbd5d..75d4a0af5a 100644 --- a/src/main/java/io/bitsquare/trade/protocol/createoffer/tasks/PublishOfferToDHT.java +++ b/src/main/java/io/bitsquare/trade/protocol/createoffer/tasks/PublishOfferToDHT.java @@ -18,6 +18,7 @@ package io.bitsquare.trade.protocol.createoffer.tasks; import io.bitsquare.msg.MessageFacade; +import io.bitsquare.msg.listeners.AddOfferListener; import io.bitsquare.trade.Offer; import io.bitsquare.trade.handlers.FaultHandler; import io.bitsquare.trade.handlers.ResultHandler; @@ -30,7 +31,7 @@ public class PublishOfferToDHT { public static void run(ResultHandler resultHandler, FaultHandler faultHandler, MessageFacade messageFacade, Offer offer) { - messageFacade.addOffer(offer, new MessageFacade.AddOfferListener() { + messageFacade.addOffer(offer, new AddOfferListener() { @Override public void onComplete() { resultHandler.onResult();