Skip to content

Commit

Permalink
common: update ByteSizeParser to follow builder pattern
Browse files Browse the repository at this point in the history
Motivation:

ByteSizeParser supports a fluent interface for configuration, ostensibly
following the builder pattern.  However, the Builder class never
actually builds the desired object; instead, parsing is done through the
builder.

This behaviour is potentially confusing for people familiar with the
builder pattern.

Modification:

Add a build method to the ByteSizeParser.Builder class that returns a
ByteSizeParser object.  Parsing is now done through this returned
object.

As the ByteSizeParser object is now immutable, may be safely used as a
constant (static final).  This allows the separation of the parser
definition from the use of the parser, potentially making code easier to
read.

The regular expression FSM is delayed until the first use.  This is to
avoid a (very minor) performance regression on start-up.  The FSM is
then reused, providing a (very minor) performance improvement if the
parser is used more than once.

Result:

No user or admin observable changes.  Slightly easier to understand
code.

Target: master
Requires-notes: no
Requires-book: no
Patch: https://rb.dcache.org/r/13121/
Acked-by: Lea Morschel
Acked-by: Albert Rossi
  • Loading branch information
paulmillar committed Aug 14, 2021
1 parent 6809690 commit 5bf281a
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING

public final class ArgumentHandler {

private static final ByteSizeParser SIZE_PARSER = ByteSizeParser.using(isoPrefix()).build();

public static long parseByteQuantity(String arg)
{
String s = arg.endsWith("B") ? arg.substring(0, arg.length()-1) : arg;
return checkNonNegative(ByteSizeParser.using(isoPrefix()).parse(s));
return checkNonNegative(SIZE_PARSER.parse(s));
}

public static long checkNonNegative(long size)
Expand Down
82 changes: 54 additions & 28 deletions modules/common/src/main/java/org/dcache/util/ByteSizeParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public static class Builder
{
private final Representation representation;
private NumericalInput input = NumericalInput.FLOATING_POINT;
private Coercion coersion = Coercion.ROUND;
private Coercion coercion = Coercion.ROUND;
private Whitespace whitespace = Whitespace.OPTIONAL;
private UnitPresence unitPresence = UnitPresence.OPTIONAL;
private ByteUnit defaultUnits = ByteUnit.BYTES;
Expand All @@ -135,7 +135,7 @@ private Builder(Representation representation)

public Builder withCoersion(Coercion coersion)
{
this.coersion = requireNonNull(coersion);
this.coercion = requireNonNull(coersion);
return this;
}

Expand Down Expand Up @@ -167,37 +167,21 @@ public Builder withDefaultUnits(ByteUnit units)
return this;
}

public long parse(String value, ByteUnit targetUnits) throws NumberFormatException
public ByteSizeParser build()
{
String whitespaceAndUnit = whitespace.regularExpression + "(?<unit>\\p{Alpha}+)";
String whitespaceAndUnitWithPresence = unitPresence == UnitPresence.REQUIRED
? whitespaceAndUnit
: ("(?:" + whitespaceAndUnit + ")?");

Pattern pattern = Pattern.compile("(?<number>" + input.regularExpression + ")"
+ whitespaceAndUnitWithPresence);
Matcher m = pattern.matcher(value);
if (!m.matches()) {
throw new NumberFormatException("Bad input \"" + value + "\" does not match " + pattern);
}

ByteUnit givenUnits = Optional.ofNullable(m.group("unit"))
.map(s -> representation.parse(s).orElseThrow(() -> new NumberFormatException("Unknown unit \"" + s + "\"")))
.orElse(defaultUnits);

return input.convert(m.group("number"), givenUnits, targetUnits, coersion);
return new ByteSizeParser(this);
}

public long parse(String value) throws NumberFormatException
{
return parse(value, ByteUnit.BYTES);
}
}

private ByteSizeParser()
{
// prevent instantiation
}
private final Whitespace whitespace;
private final UnitPresence unitPresence;
private final NumericalInput input;
private final Representation representation;
private final ByteUnit defaultUnits;
private final Coercion coercion;

private Pattern pattern;

/**
* Create a new Builder that parses the input with the specified
Expand All @@ -211,4 +195,46 @@ public static Builder using(Representation representation)
{
return new Builder(representation);
}

private ByteSizeParser(Builder builder)
{
whitespace = builder.whitespace;
unitPresence = builder.unitPresence;
input = builder.input;
representation = builder.representation;
defaultUnits = builder.defaultUnits;
coercion = builder.coercion;
}

private synchronized Pattern pattern()
{
if (pattern == null) {
String whitespaceAndUnit = whitespace.regularExpression + "(?<unit>\\p{Alpha}+)";
String whitespaceAndUnitWithPresence = unitPresence == UnitPresence.REQUIRED
? whitespaceAndUnit
: ("(?:" + whitespaceAndUnit + ")?");
pattern = Pattern.compile("(?<number>" + input.regularExpression + ")"
+ whitespaceAndUnitWithPresence);
}
return pattern;
}

public long parse(String value, ByteUnit targetUnits) throws NumberFormatException
{
Matcher m = pattern().matcher(value);
if (!m.matches()) {
throw new NumberFormatException("Bad input \"" + value + "\" does not match " + pattern);
}

ByteUnit givenUnits = Optional.ofNullable(m.group("unit"))
.map(s -> representation.parse(s).orElseThrow(() -> new NumberFormatException("Unknown unit \"" + s + "\"")))
.orElse(defaultUnits);

return input.convert(m.group("number"), givenUnits, targetUnits, coercion);
}

public long parse(String value) throws NumberFormatException
{
return parse(value, ByteUnit.BYTES);
}
}
Loading

0 comments on commit 5bf281a

Please sign in to comment.