Path pattern builder

[noodlze]

Motivation:
Addresses #659.

• Add PathPattern#of accepting PathPattern... pathPatterns

PathPattern.of(PatternPattern.of("json"), PathPattern.startsWith("/foo/bar"));

• Add PathPatternBuilder to provide a way to combination many patterns into one.
  The following initial options have been added:

startsWith(String dirPath)

contains(String dirPath)

endsWith(String filename) 
//or
extension(String extension)

Modifications:

• PathPatternOption, PathPathOptions -> define and manage the options available.
• PathPatternBuilder -> responsible for composing one pathPattern from the added options.
  • Can only use one of the options endsWith or extension ; currently, option added after will override previous

Result:

PathPattern.of(PatternPattern.of("json"), PathPattern.startsWith("/foo/bar"));

PathPattern.builder().startsWith("/foo/bar").build(); // "/foo/bar/**"

PathPattern.builder()
                    .startsWith("/foo/bar")
                    .endsWith("foo.txt")
                     .build(); // "/foo/bar/**/foo.txt"
                           
 PathPattern.builder()
                      .startsWith("/foo")
                      .contains("/bar/")
                      .extension("json")
                      .build() // "/foo/**/bar/**/*.json"

 PathPattern.builder()
                     .startsWith("/foo")
                     .endsWith("qux.json")
                     .extension("json")
                     .build() // "/foo/**/*.json"

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

Patch coverage: 85.93% and project coverage change: -0.05 ⚠️

Comparison is base (bda3bec) 65.71% compared to head (53ebe9f) 65.67%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #809      +/-   ##
============================================
- Coverage     65.71%   65.67%   -0.05%     
- Complexity     3289     3337      +48     
============================================
  Files           350      356       +6     
  Lines         13731    13911     +180     
  Branches       1488     1510      +22     
============================================
+ Hits           9024     9136     +112     
- Misses         3863     3921      +58     
- Partials        844      854      +10     

Impacted Files	Coverage Δ
.../com/linecorp/centraldogma/common/PathPattern.java	62.50% <40.00%> (-37.50%)	⬇️
...necorp/centraldogma/common/PathPatternBuilder.java	92.30% <92.30%> (ø)
...necorp/centraldogma/common/DefaultPathPattern.java	97.95% <100.00%> (+0.52%)	⬆️

... and 17 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[noodlze]

We may allow multiple contains unlike other methods such as startWith() and enddWith().

• startsWith() supports only one path. We can simply set the value as a string field.
  endsWith() and extension() are mutually exclusive. We can make the methods override the same field.

public final class PathPatternBulider {

    @Nullable
    private String startPattern;
    @Nullable
    private String endPattern;
    @Nullable
    private List<String> innerPatterns;

    public PathPatternBuilder startsWith(String dirPath) {
        // validation and normalize ...
        this.startPattern = dirPath;
    }


    public PathPatternBuilder extension(String extension) {
        // validation and normalize ...
        this.endPattern = normalize(extension);
    }
}

As a result of moving the pattern build order logic from PathPatternOption to PathPatternBuilder , precedence info stored in PathPatternOption is no longer needed.
Also, it's harder to determine whether endsWith/extension option has been added more than once with this new structure as adding either will simply override any existing value stored in endPattern. It's easy to do so for startPattern since there is only one option (startsWith). So in the end, for consistency, I found it simpler to just allow adding same option twice to the builder; when we add the same option, it will just override existing.

[ikhoon]

Overall looks good. Left minor comments.

[ikhoon]

Nice work! 🚀:tada:

[trustin]

Suggested change

	return new DefaultPathPattern(ImmutableList.copyOf(pathPatterns));
	return new DefaultPathPattern(ImmutableSet.copyOf(pathPatterns));

[noodlze]

Previously, a new constructor for DefaultPathPattern(List<PathPattern> verifiedPatterns) was introduced that would skip the validatePathPattern step.
I changed the argument type from List<PathPattern> -> varargs PathPattern... in commit 0d85d78.

[trustin]

Can't we just use PathPattern here?

[trustin]

I'm not sure if this class is needed at all. Why do we need to create a PathPattern lazily? Can't we just create it as early as possible?

[noodlze]

I have merged PathPatternOptions and PathPatternOption with PathPatternBuilder

[trustin]

I think this can be quite confusing to users. Should we always override, even for contains()?

[noodlze]

Hi @trustin ,
Your point about differing behavior for contains(combining allowed) vs other options (combining not allowed , will override) being confusing is valid.
I also understand @ikhoon point's that, when considered in isolation, contains can be combined (ref #809 (comment) ).
Personally, I am on the fence, with no strong opinion.
Perhaps @minwoox , and @jrhee17 could share their opinion :)