You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2020/12/04 14:35:25 UTC

[GitHub] [httpcomponents-core] larshelge opened a new pull request #234: Add appendPath method to URIBuilder

larshelge opened a new pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234


   This PR is adding a `appendPath` method to `URIBuilder`.
   
   The added benefit of this method is allowing a URI to built successively throughout an application. As an example, one method could set the base URL, pass the `URIBuilder` along to the next method which can set an API version, and a final method set the context path.
   
   This would be equivalent to Spring's (UriComponentsBuilder)[https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/util/UriComponentsBuilder.html#path-java.lang.String-]. When migrating a recent application from Spring to HTTP Components, an equivalent for this method was lacking.
    


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536766401



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       @ok2c Got it. I will update it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536762219



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       @larshelge Should not it be possible to re-use `#setPathSegments` here?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536766111



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       > Sorry, I did not fully understand - do you mean re-use `appendPathSegments(List)` ?
   
   @larshelge Yes, that is what I meant. I thought it would make it a bit neater. Not very important, though.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on pull request #234: Add appendPath method to URIBuilder

Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#issuecomment-738954581


   @larshelge Looks good to me. Could you please add `#appendPathSegments` method for the sake of completeness?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536785613



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,16 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {

Review comment:
       Updated it now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536763113



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       I fully agree. I thought about it but favored consistency with existing code, e.g. `setPathSegments(String...)` and `setPathSegments(List)`. I am happy to change it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536766401



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       @ok2c Got it. I will update it a bit later.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536766401



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       @ok2c Got it. I will update it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536765272



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       Sorry, I did not fully understand - do you mean re-use `appendPathSegments(List)` ?
   
   There is definitely room for method re-use here, but I favored consistency with existing code, e.g. `setPathSegments(String...)` could call `setPathSegments(List)`. I am happy to make changes.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536769359



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       @ok2c Made the changes now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c merged pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
ok2c merged pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536765272



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,18 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {
+        final List<String> segments = new ArrayList<>(getPathSegments());
+        segments.addAll(path != null ? splitPath(path) : new ArrayList<String>());
+        setPathSegments(segments);

Review comment:
       Sorry, I did not fully understand - do you mean re-use `appendPathSegments(List)` ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#issuecomment-739251244


   Absolutely @ok2c. Added `appendPathSegments(String...)` and `appendPathSegments(List)`  now.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] larshelge commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
larshelge commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536784990



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -541,6 +560,18 @@ public URIBuilder setPathSegments(final List<String> pathSegments) {
         return this;
     }
 
+    /**
+     * Appends segments to URI path. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPathSegments(final List<String> pathSegments) {
+        final List<String> segments = new ArrayList<>(getPathSegments());

Review comment:
       Agreed @ok2c. I wanted to avoid if-statements but we can favor less heap usage here.
   
   Note that the `new ArrayList<>(getPathSegments())` wrapper list is necessary as `getPathSegments` might return an immutable list which cannot be added to later.
   
   I have made the updates now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536771602



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -499,6 +499,16 @@ public URIBuilder setPath(final String path) {
         return this;
     }
 
+    /**
+     * Appends path to URI. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPath(final String path) {

Review comment:
       @larshelge One last thing. I suppose this should be a little more efficient by avoiding unnecessary operations and intermediate garbage.
   ```
       public URIBuilder appendPath(final String path) {
           if (path != null) {
                appendPathSegments(splitPath(path);
           }
           return this;
       }
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #234: Add appendPath and appendPathSegments methods to URIBuilder

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #234:
URL: https://github.com/apache/httpcomponents-core/pull/234#discussion_r536771774



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
##########
@@ -541,6 +560,18 @@ public URIBuilder setPathSegments(final List<String> pathSegments) {
         return this;
     }
 
+    /**
+     * Appends segments to URI path. The value is expected to be unescaped and may contain non ASCII characters.
+     *
+     * @return this.
+     */
+    public URIBuilder appendPathSegments(final List<String> pathSegments) {
+        final List<String> segments = new ArrayList<>(getPathSegments());

Review comment:
       @larshelge Same here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org