Introduce path encoding (fixes #1561)

[aaronjwhiteside]

HttpRequestBuilder.java

• replaced armeria QueryParamsBuilder with apache http URIBuilder.
• minor generics cleanup
• introduced support for fragments.
• URIBuilder now handles path segment encoding, query parameters and any fragment.
• care was taken to ensure that path's prefixed with / are accepted, but a warning is printed letting people know they should remove the prefixing /

encoding.feature

• added more demo/test cases for path encoding

HttpUtils.java

• minor generics cleanup
• final is redundant on static methods

karate-demo/pom.xml

• scope = import only works in the dependencyManagement section, this fixes the maven warning.

Request.java

• minor generics cleanup
• use computeIfAbsent()

HttpClientFactory.java

• public static final are redundant on interface fields
• also use method reference

Description

Thanks for contributing this Pull Request. Make sure that you submit this Pull Request against the develop branch of this repository, add a brief description, and tag the relevant issue(s) and PR(s) below.

• Relevant Issues : Path encoding issue after upgrade from 0.9.6 to 1.0.1 #1561
• Relevant PRs : (optional)
• Type of change :
  • New feature
  • Bug fix for existing feature
  • Code quality improvement
  • Addition or Improvement of tests
  • Addition or Improvement of documentation

[ptrthomas]

thanks much ! saw you switched to the develop branch

also in case you use slack: https://twitter.com/KarateDSL/status/937271659785961477

[lodenrogue]

Single level of abstraction:

You could probably organize this like so (forgive any syntax errors. Wrote this in the browser):

try {
    URIBuilder builder = createBuilder(url, params);
    if (Objects.nonNull(paths)) {
        builder.setPathSegments(createSegments(builder, paths));
    }

    // Rest of the code
}

private List<String> createSegments(URIBuilder builder, List<String> paths) {
    paths.stream()
        .filter(path -> path.startsWith("/"))
        .forEach(this::warnPathSegment);
    
    return Stream.of(builder.getPathSegments(), paths)
        .flatMap(List::stream)
        .collect(Collectors.toList());
}

private void warnPathSegment(String path) {
    logger.warn("Path segment: '{}' starts with a '/', this is probably a mistake. The '/' character will be escaped and sent to the remote server as '%2F'. " +
                                "If you want to include multiple paths please separate them using commas. Ie. 'hello', 'world' instead of '/hello/world'.", path);
}

// Not sure the type of params
private URIBuilder createBuilder(String url, List<K, V> params) {
    URIBuilder builder = Objects.isNull(url) ? new URIBuilder() : new URIBuilder(url);
    if (Objects.nonNull(params)) {
        params.forEach((key, values) -> values.forEach(value -> builder.addParameter(key, value)));
    }
    return builder;
}

[aaronjwhiteside]

thats nice, I like that, thanks @lodenrogue

[ptrthomas]

closing because of inactivity, perhaps @lodenrogue would be interested to pick this up