Avoid use of JOpt's #defaultsTo method

Providing an explicit default using the #defaultsTo method ends up
short-circuiting the Spring Environment's hierarchical property resolution
process. for example, if --port.useManualPortForwarding has a default
value of `false`, then the command line property source always returns a
value when its #getProperty method is invoked by the Environment. This
means that a lower-precedence property source never has the opportunity
to return its value. For example, if port.useManualPortForwarding had
been set to true in the filesystem property source
(at ${user.data.dir}/bitsquare.properties), this property value would
never be resolved because the default command line property source always
overrides it (thus the notion of "short circuiting" above).

This change eliminates the use of JOpt's #defaultsTo method in favor of
a simple approach to advertising default values (if any) in the option's
description string. The result is --help output that reads exactly the
same as it did before, but no actual default value is set at the command
line property source level.

Note that the default property source is still created, and default
values are still assigned in BitsquareEnvironment#defaultPropertySource.
This property source has the lowest precedence, and this means that any
and all other property sources have the opportunity to provide a value
and override the default.
This commit is contained in:
Chris Beams 2014-11-17 11:18:21 +01:00
parent 97fc4a728e
commit f6f97d55ed
No known key found for this signature in database
GPG Key ID: 3D214F8F5BC5ED73
3 changed files with 47 additions and 19 deletions

View File

@ -20,6 +20,10 @@ package io.bitsquare.app;
import joptsimple.OptionException;
import joptsimple.OptionParser;
import joptsimple.OptionSet;
import org.springframework.util.StringUtils;
import static java.lang.String.format;
import static java.lang.String.join;
public abstract class BitsquareExecutable {
public static final int EXIT_SUCCESS = 0;
@ -53,5 +57,14 @@ public abstract class BitsquareExecutable {
protected abstract void customizeOptionParsing(OptionParser parser);
protected static String description(String descText, Object defaultValue) {
String description = "";
if (StringUtils.hasText(descText))
description = description.concat(descText);
if (defaultValue != null)
description = join(" ", description, format("(default: %s)", defaultValue));
return description;
}
protected abstract void doExecute(OptionSet options);
}

View File

@ -31,9 +31,12 @@ public class BootstrapNodeMain extends BitsquareExecutable {
}
protected void customizeOptionParsing(OptionParser parser) {
parser.accepts(Node.NAME_KEY, "Name of this node").withRequiredArg().isRequired();
parser.accepts(Node.PORT_KEY, "Port to listen on").withRequiredArg().ofType(int.class)
.defaultsTo(Node.DEFAULT_PORT);
parser.accepts(Node.NAME_KEY, description("Name of this node", null))
.withRequiredArg()
.isRequired();
parser.accepts(Node.PORT_KEY, description("Port to listen on", Node.DEFAULT_PORT))
.withRequiredArg()
.ofType(int.class);
}
protected void doExecute(OptionSet options) {

View File

@ -30,6 +30,7 @@ import joptsimple.OptionSet;
import static io.bitsquare.app.BitsquareEnvironment.*;
import static io.bitsquare.msg.tomp2p.TomP2PMessageModule.*;
import static io.bitsquare.network.Node.*;
import static java.lang.String.format;
public class BitsquareAppMain extends BitsquareExecutable {
@ -39,22 +40,33 @@ public class BitsquareAppMain extends BitsquareExecutable {
@Override
protected void customizeOptionParsing(OptionParser parser) {
parser.accepts(USER_DATA_DIR_KEY, "User data directory").withRequiredArg().defaultsTo(DEFAULT_USER_DATA_DIR);
parser.accepts(APP_NAME_KEY, "Application name").withRequiredArg().defaultsTo(DEFAULT_APP_NAME);
parser.accepts(APP_DATA_DIR_KEY, "Application data directory").withRequiredArg()
.defaultsTo(DEFAULT_APP_DATA_DIR);
parser.accepts(NAME_KEY, "Name of this node").withRequiredArg();
parser.accepts(PORT_KEY, "Port to listen on").withRequiredArg().ofType(int.class).defaultsTo(Node.DEFAULT_PORT);
parser.accepts(USE_MANUAL_PORT_FORWARDING_KEY, "Use manual port forwarding")
.withRequiredArg().ofType(boolean.class).defaultsTo(false);
parser.accepts(BitcoinNetwork.KEY).withRequiredArg().ofType(BitcoinNetwork.class)
.withValuesConvertedBy(new EnumValueConverter(BitcoinNetwork.class))
.defaultsTo(BitcoinNetwork.DEFAULT);
parser.accepts(BOOTSTRAP_NODE_NAME_KEY).withRequiredArg().defaultsTo(BootstrapNodes.DEFAULT.getName());
parser.accepts(BOOTSTRAP_NODE_IP_KEY).withRequiredArg().defaultsTo(BootstrapNodes.DEFAULT.getIp());
parser.accepts(BOOTSTRAP_NODE_PORT_KEY).withRequiredArg().ofType(int.class)
.defaultsTo(BootstrapNodes.DEFAULT.getPort());
parser.accepts(NETWORK_INTERFACE_KEY, "Network interface").withRequiredArg();
parser.accepts(USER_DATA_DIR_KEY, description("User data directory", DEFAULT_USER_DATA_DIR))
.withRequiredArg();
parser.accepts(APP_NAME_KEY, description("Application name", DEFAULT_APP_NAME))
.withRequiredArg();
parser.accepts(APP_DATA_DIR_KEY, description("Application data directory", DEFAULT_APP_DATA_DIR))
.withRequiredArg();
parser.accepts(NAME_KEY, description("Name of this node", null))
.withRequiredArg();
parser.accepts(PORT_KEY, description("Port to listen on", Node.DEFAULT_PORT))
.withRequiredArg()
.ofType(int.class);
parser.accepts(USE_MANUAL_PORT_FORWARDING_KEY, description("Use manual port forwarding", false))
.withRequiredArg()
.ofType(boolean.class);
parser.accepts(BitcoinNetwork.KEY, description("", BitcoinNetwork.DEFAULT))
.withRequiredArg()
.ofType(BitcoinNetwork.class)
.withValuesConvertedBy(new EnumValueConverter(BitcoinNetwork.class));
parser.accepts(BOOTSTRAP_NODE_NAME_KEY, description("", BootstrapNodes.DEFAULT.getName()))
.withRequiredArg();
parser.accepts(BOOTSTRAP_NODE_IP_KEY, description("", BootstrapNodes.DEFAULT.getIp()))
.withRequiredArg();
parser.accepts(BOOTSTRAP_NODE_PORT_KEY, description("", BootstrapNodes.DEFAULT.getPort()))
.withRequiredArg()
.ofType(int.class);
parser.accepts(NETWORK_INTERFACE_KEY, description("Network interface", null))
.withRequiredArg();
}
@Override