Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cleanup tor files button to tor network settings #1301

Merged
merged 2 commits into from
Jan 31, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions common/src/main/resources/i18n/displayStrings.properties
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ settings.net.roundTripTimeColumn=Roundtrip
settings.net.sentBytesColumn=Sent
settings.net.receivedBytesColumn=Received
settings.net.peerTypeColumn=Peer type
settings.net.openTorSettingsButton=Open Tor network settings
settings.net.openTorSettingsButton=Open Tor settings

settings.net.needRestart=You need to restart the application to apply that change.\nDo you want to do that now?
settings.net.notKnownYet=Not known yet...
Expand Down Expand Up @@ -1248,7 +1248,12 @@ torNetworkSettingWindow.enterBridge=Enter one or more bridge relays (one per lin
torNetworkSettingWindow.enterBridgePrompt=type address:port
torNetworkSettingWindow.restartInfo=You need to restart to apply the changes
torNetworkSettingWindow.openTorWebPage=Open Tor project web page
torNetworkSettingWindow.info=If Tor is blocked by your internet provider or in your country you can try to use Tor bridges.\n\
torNetworkSettingWindow.deleteFiles.header=Connection problems?
torNetworkSettingWindow.deleteFiles.info=If you have repeated connection problems at start up, deleting outdated Tor files might help. To do that click the button below and restart afterwards.
torNetworkSettingWindow.deleteFiles.button=Deleting outdated Tor files and shut down
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Deleting/Delete/

torNetworkSettingWindow.deleteFiles.success=Deleting outdated Tor files wa successful. Please restart.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/wa/was/

torNetworkSettingWindow.bridges.header=Is Tor blocked?
torNetworkSettingWindow.bridges.info=If Tor is blocked by your internet provider or in your country you can try to use Tor bridges.\n\
Visit the Tor web page at: https://bridges.torproject.org/bridges to learn more about \
bridges and pluggable transports.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,7 @@ torNetworkSettingWindow.enterBridge="Bridge relays" angeben (zeilenweise):
torNetworkSettingWindow.enterBridgePrompt=address:port eingeben
torNetworkSettingWindow.restartInfo=Du musst neu starten, um Änderungen anzuwenden
torNetworkSettingWindow.openTorWebPage=Tor Webseite öffnen
torNetworkSettingWindow.info=Falls Tor von deinem Internetanbieter oder Land blockiert wird, kannst du es mit \
torNetworkSettingWindow.bridges.info=Falls Tor von deinem Internetanbieter oder Land blockiert wird, kannst du es mit \
"Tor Bridges" versuchen.\n\
Besuche die Tor Webseite unter: https://bridges.torproject.org/bridges, um mehr über "Bridges" und \
"Pluggable Transports" zu erfahren.
Expand Down
9 changes: 9 additions & 0 deletions gui/src/main/java/io/bisq/gui/bisq.css
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,15 @@ textfield */
-fx-effect: dropshadow(gaussian, #999, 10, 0, 0, 0);
}

#popup-grid-pane-bg {
-fx-font-size: 14;
-fx-text-fill: #333;
-fx-background-color: -bs-content-bg-grey;
-fx-background-radius: 10 10 10 10;
-fx-background-insets: 10;
-fx-effect: dropshadow(gaussian, #999, 10, 0, 0, 0);
}

#notification-popup-headline {
-fx-font-size: 13;
-fx-font-weight: bold;
Expand Down
16 changes: 10 additions & 6 deletions gui/src/main/java/io/bisq/gui/main/MainView.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import io.bisq.gui.main.offer.BuyOfferView;
import io.bisq.gui.main.offer.SellOfferView;
import io.bisq.gui.main.overlays.popups.Popup;
import io.bisq.gui.main.overlays.windows.TorNetworkSettingsWindow;
import io.bisq.gui.main.portfolio.PortfolioView;
import io.bisq.gui.main.settings.SettingsView;
import io.bisq.gui.util.BSFormatter;
Expand Down Expand Up @@ -100,12 +99,15 @@ public static void removeEffect() {
transitions.removeEffect(MainView.rootContainer);
}

private final ToggleGroup navButtons = new ToggleGroup();
private static Transitions transitions;
private static StackPane rootContainer;


private final ViewLoader viewLoader;
private final Navigation navigation;
private static Transitions transitions;
private final BSFormatter formatter;

private final ToggleGroup navButtons = new ToggleGroup();
private ChangeListener<String> walletServiceErrorMsgListener;
private ChangeListener<String> btcSyncIconIdListener;
private ChangeListener<String> splashP2PNetworkErrorMsgListener;
Expand All @@ -117,11 +119,13 @@ public static void removeEffect() {
private Label btcSplashInfo;
private List<String> persistedFilesCorrupted;
private Popup<?> p2PNetworkWarnMsgPopup, btcNetworkWarnMsgPopup;
private static StackPane rootContainer;

@SuppressWarnings("WeakerAccess")
@Inject
public MainView(MainViewModel model, CachingViewLoader viewLoader, Navigation navigation, Transitions transitions,
public MainView(MainViewModel model,
CachingViewLoader viewLoader,
Navigation navigation,
Transitions transitions,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid one per line ctor parameter stacking (see other, more detailed comment)

BSFormatter formatter) {
super(model);
this.viewLoader = viewLoader;
Expand Down Expand Up @@ -478,7 +482,7 @@ private VBox createSplashScreen() {
showTorNetworkSettingsButton.setVisible(false);
showTorNetworkSettingsButton.setManaged(false);
showTorNetworkSettingsButton.setOnAction(e -> {
new TorNetworkSettingsWindow(model.preferences).show();
model.torNetworkSettingsWindow.show();
});

ImageView splashP2PNetworkIcon = new ImageView();
Expand Down
6 changes: 3 additions & 3 deletions gui/src/main/java/io/bisq/gui/main/MainViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public class MainViewModel implements ViewModel {
private final FailedTradesManager failedTradesManager;
private final ClosedTradableManager closedTradableManager;
private final AccountAgeWitnessService accountAgeWitnessService;
final TorNetworkSettingsWindow torNetworkSettingsWindow;
private final BSFormatter formatter;

// BTC network
Expand Down Expand Up @@ -197,7 +198,6 @@ public class MainViewModel implements ViewModel {
private MonadicBinding<String> marketPriceBinding;
@SuppressWarnings({"unused", "FieldCanBeLocal"})
private Subscription priceFeedAllLoadedSubscription;
private TorNetworkSettingsWindow torNetworkSettingsWindow;
private BooleanProperty p2pNetWorkReady;
private final BooleanProperty walletInitialized = new SimpleBooleanProperty();
private boolean allBasicServicesInitialized;
Expand All @@ -219,7 +219,7 @@ public MainViewModel(WalletsManager walletsManager, WalletsSetup walletsSetup,
DaoManager daoManager, EncryptionService encryptionService,
KeyRing keyRing, BisqEnvironment bisqEnvironment, FailedTradesManager failedTradesManager,
ClosedTradableManager closedTradableManager, AccountAgeWitnessService accountAgeWitnessService,
BSFormatter formatter) {
TorNetworkSettingsWindow torNetworkSettingsWindow, BSFormatter formatter) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good example of adding a constructor parameter (see other comments)

this.walletsManager = walletsManager;
this.walletsSetup = walletsSetup;
this.btcWalletService = btcWalletService;
Expand Down Expand Up @@ -247,6 +247,7 @@ public MainViewModel(WalletsManager walletsManager, WalletsSetup walletsSetup,
this.failedTradesManager = failedTradesManager;
this.closedTradableManager = closedTradableManager;
this.accountAgeWitnessService = accountAgeWitnessService;
this.torNetworkSettingsWindow = torNetworkSettingsWindow;
this.formatter = formatter;

TxIdTextField.setPreferences(preferences);
Expand Down Expand Up @@ -332,7 +333,6 @@ private void startBasicServices() {

private void showTorNetworkSettingsWindow() {
MainView.blur();
torNetworkSettingsWindow = new TorNetworkSettingsWindow(preferences).useShutDownButton();
torNetworkSettingsWindow.show();
}

Expand Down
20 changes: 11 additions & 9 deletions gui/src/main/java/io/bisq/gui/main/overlays/Overlay.java
Original file line number Diff line number Diff line change
Expand Up @@ -174,17 +174,19 @@ protected void onShow() {
}

public void hide() {
animateHide(() -> {
removeEffectFromBackground();
if (gridPane != null) {
animateHide(() -> {
removeEffectFromBackground();

if (stage != null)
stage.hide();
else
log.warn("Stage is null");
if (stage != null)
stage.hide();
else
log.warn("Stage is null");

cleanup();
onHidden();
});
cleanup();
onHidden();
});
}
}

protected void onHidden() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,17 @@

package io.bisq.gui.main.overlays.windows;

import com.google.inject.name.Named;
import io.bisq.common.UserThread;
import io.bisq.common.locale.Res;
import io.bisq.common.storage.FileUtil;
import io.bisq.common.util.Tuple2;
import io.bisq.common.util.Utilities;
import io.bisq.core.user.Preferences;
import io.bisq.gui.main.overlays.Overlay;
import io.bisq.gui.main.overlays.popups.Popup;
import io.bisq.gui.util.Layout;
import io.bisq.network.NetworkOptionKeys;
import io.bisq.network.p2p.network.DefaultPluggableTransports;
import javafx.collections.FXCollections;
import javafx.geometry.HPos;
Expand All @@ -56,8 +61,11 @@
import javafx.util.StringConverter;
import lombok.extern.slf4j.Slf4j;

import javax.inject.Inject;
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.concurrent.TimeUnit;

Expand All @@ -80,6 +88,7 @@ public enum Transport {
}

private final Preferences preferences;
private final File torDir;
private RadioButton noBridgesRadioButton, providedBridgesRadioButton, customBridgesRadioButton;
private Label enterBridgeLabel;
private ComboBox<Transport> transportTypeComboBox;
Expand All @@ -89,13 +98,18 @@ public enum Transport {
private Transport selectedTorTransportOrdinal = Transport.OBFS_4;
private String customBridges = "";

public TorNetworkSettingsWindow(Preferences preferences) {
@Inject
public TorNetworkSettingsWindow(Preferences preferences,
@Named(NetworkOptionKeys.TOR_DIR) File torDir) {
this.preferences = preferences;
this.torDir = torDir;

type = Type.Attention;

useShutDownButton();
}


///////////////////////////////////////////////////////////////////////////////////////////
// Public API
///////////////////////////////////////////////////////////////////////////////////////////
Expand All @@ -106,8 +120,6 @@ public void show() {

width = 1000;
createGridPane();
addHeadLine();
addSeparator();
addContent();
addCloseButton();
applyStyles();
Expand Down Expand Up @@ -174,23 +186,46 @@ protected void setupKeyHandler(Scene scene) {
}
}

@Override
protected void applyStyles() {
super.applyStyles();
gridPane.setId("popup-grid-pane-bg");
}

private void addContent() {
gridPane.setStyle("-fx-background-color: #f8f8f8;");
addTitledGroupBg(gridPane, ++rowIndex, 1, Res.get("torNetworkSettingWindow.deleteFiles.header"));

Label deleteFilesLabel = addLabel(gridPane, rowIndex, Res.get("torNetworkSettingWindow.deleteFiles.info"), Layout.FIRST_ROW_DISTANCE);
deleteFilesLabel.setWrapText(true);
GridPane.setColumnIndex(deleteFilesLabel, 0);
GridPane.setColumnSpan(deleteFilesLabel, 2);
GridPane.setHalignment(deleteFilesLabel, HPos.LEFT);
GridPane.setValignment(deleteFilesLabel, VPos.TOP);

Button deleteFilesButton = addButtonAfterGroup(gridPane, ++rowIndex, Res.get("torNetworkSettingWindow.deleteFiles.button"));
deleteFilesButton.setOnAction(e -> {
cleanTorDir();
new Popup<>().feedback(Res.get("torNetworkSettingWindow.deleteFiles.success")).show();
});

Label label = addLabel(gridPane, ++rowIndex, Res.get("torNetworkSettingWindow.info"));
label.setWrapText(true);
GridPane.setColumnIndex(label, 0);
GridPane.setColumnSpan(label, 2);
GridPane.setHalignment(label, HPos.LEFT);
GridPane.setValignment(label, VPos.TOP);

GridPane.setMargin(label, new Insets(0, 0, 20, 0));
addTitledGroupBg(gridPane, ++rowIndex, 7, Res.get("torNetworkSettingWindow.bridges.header"), Layout.GROUP_DISTANCE);

Label bridgesLabel = addLabel(gridPane, rowIndex, Res.get("torNetworkSettingWindow.bridges.info"), Layout.FIRST_ROW_AND_GROUP_DISTANCE);
bridgesLabel.setWrapText(true);
GridPane.setColumnIndex(bridgesLabel, 0);
GridPane.setColumnSpan(bridgesLabel, 2);
GridPane.setHalignment(bridgesLabel, HPos.LEFT);
GridPane.setValignment(bridgesLabel, VPos.TOP);

//addLabelTextArea(gridPane, rowIndex, Res.get("torNetworkSettingWindow.info"), "", Layout.FIRST_ROW_AND_GROUP_DISTANCE);

ToggleGroup toggleGroup = new ToggleGroup();

// noBridges
noBridgesRadioButton = addRadioButton(gridPane, ++rowIndex, toggleGroup, Res.get("torNetworkSettingWindow.noBridges"));
noBridgesRadioButton.setUserData(BridgeOption.NONE);
GridPane.setMargin(noBridgesRadioButton, new Insets(20, 0, 0, 0));

// providedBridges
providedBridgesRadioButton = addRadioButton(gridPane, ++rowIndex, toggleGroup, Res.get("torNetworkSettingWindow.providedBridges"));
Expand Down Expand Up @@ -238,8 +273,8 @@ public Transport fromString(String string) {
GridPane.setColumnIndex(label2, 1);
GridPane.setColumnSpan(label2, 2);
GridPane.setHalignment(label2, HPos.LEFT);
GridPane.setValignment(label, VPos.TOP);
GridPane.setMargin(label, new Insets(10, 10, 20, 0));
GridPane.setValignment(label2, VPos.TOP);
GridPane.setMargin(label2, new Insets(10, 10, 20, 0));

// init persisted values
selectedBridgeOption = BridgeOption.values()[preferences.getBridgeOptionOrdinal()];
Expand Down Expand Up @@ -281,6 +316,17 @@ public Transport fromString(String string) {
);
}

private void cleanTorDir() {
final File hiddenservice = new File(Paths.get(torDir.getAbsolutePath(), "hiddenservice").toString());
try {
FileUtil.deleteDirectory(torDir, hiddenservice);
} catch (IOException e) {
e.printStackTrace();
log.error(e.toString());
new Popup<>().error(e.toString()).show();
}
}

private void applyToggleSelection() {
switch (selectedBridgeOption) {
case PROVIDED:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public class NetworkSettingsView extends ActivatableViewAndModel<GridPane, Activ
private final BitcoinNodes bitcoinNodes;
private final FilterManager filterManager;
private final BisqEnvironment bisqEnvironment;
private final TorNetworkSettingsWindow torNetworkSettingsWindow;
private final Clock clock;
private final BSFormatter formatter;
private final WalletsSetup walletsSetup;
Expand All @@ -109,15 +110,23 @@ public class NetworkSettingsView extends ActivatableViewAndModel<GridPane, Activ
private ChangeListener<Filter> filterPropertyListener;

@Inject
public NetworkSettingsView(WalletsSetup walletsSetup, P2PService p2PService, Preferences preferences, BitcoinNodes bitcoinNodes,
FilterManager filterManager, BisqEnvironment bisqEnvironment, Clock clock, BSFormatter formatter) {
public NetworkSettingsView(WalletsSetup walletsSetup,
P2PService p2PService,
Preferences preferences,
BitcoinNodes bitcoinNodes,
FilterManager filterManager,
BisqEnvironment bisqEnvironment,
TorNetworkSettingsWindow torNetworkSettingsWindow,
Clock clock,
BSFormatter formatter) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to move to one-parameter per line parameter stacking like this. Creates a lot of vertical noise, and constructors with a zillion parameters are usually a signal of something needing to be refactored anyway. I'd prefer that we keep parameters aligned with the opening paren, but not stack one per line. Just go to the 120 margin, and create a new line as necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re typos:
Fixed in next commit.

Re Popup:
The popup is used when Tor cannot start up after 4 minutes at splash screens. So that is a reason why a dedicated Tor view will not work here. It is also considered as "exceptional" screen which should not be needed for normal use case.

Re why it changes anything:
I agree it is not satisfying not knowing more details. Will be a task for our new P2P network dev. There are some consensus files which might get outdated. I just have seen it yesterday with a seed node which kept failing after repeated restarts and after deleting the Tor dir it was fine.

Re ctor params:
I personally prefer it for readability to have one per line if its more then 3 or 4 params...
But not super strong opinion here. we can discuss... I prefer to keep it atm as I usually use it that way in most of the newer code parts.

Re deleteDirectory:
Do you see any risks in that method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, re popup. I didn't realize that before. Thanks.

On deleteDirectory I didn't analyze it line by line, but gave it a good look and then tested it end-to-end on my machine. This is the only place it's used in the codebase, so I'd say the risk is contained for now. Would be good to have tests around it for sure, but I'm ok with it as-is now.

super();
this.walletsSetup = walletsSetup;
this.p2PService = p2PService;
this.preferences = preferences;
this.bitcoinNodes = bitcoinNodes;
this.filterManager = filterManager;
this.bisqEnvironment = bisqEnvironment;
this.torNetworkSettingsWindow = torNetworkSettingsWindow;
this.clock = clock;
this.formatter = formatter;
}
Expand Down Expand Up @@ -260,7 +269,7 @@ public void activate() {
btcNodesInputTextField.textProperty().addListener(btcNodesInputTextFieldListener);
btcNodesInputTextField.focusedProperty().addListener(btcNodesInputTextFieldFocusListener);

openTorSettingsButton.setOnAction(e -> new TorNetworkSettingsWindow(preferences).show());
openTorSettingsButton.setOnAction(e -> torNetworkSettingsWindow.show());
}

@Override
Expand Down