Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GH-1319] Add Support for Path Style Parameter Expansion #1537

Merged
merged 5 commits into from
Mar 25, 2022

Conversation

kdavisk6
Copy link
Member

@kdavisk6 kdavisk6 commented Nov 8, 2021

Fixes #1319

This change adds limited Path Style support to Feign URI template-style
templates. Variable expressions that start with a semi-colon ;
are now expanded in accordance to RFC 6570 Section 3.2.7
with the following modifications:

  • Maps and Lists are expanded by default.
  • Only Single variable templates are supported.

Examples:

{;who}             ;who=fred
{;half}            ;half=50%25
{;empty}           ;empty
{;list}            ;list=red;list=green;list=blue
{;keys}            ;semi=%3B;dot=.;comma=%2C

Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it probably would be nice to have a Feign client example using this param expansion

@kdavisk6
Copy link
Member Author

kdavisk6 commented Nov 8, 2021

Agreed. I'll update. I'm also going to export this expression as an Expander so it can be used directly be extending contracts.

@dpodder
Copy link

dpodder commented Jan 5, 2022

Hi @kdavisk6 - thank you for working on this support!

One project I'm working on has a use case where the server expects an optional Map of ;key=value pairs - such that the values are optional. E.g., assuming

/path/from{;map}/values

Here are some expected expansions:

  • Case 1 - map := {} (empty)

    /path/from/values
    
  • Case 2 - map := {"a": "1", "b": "", "c": "3"}

    /path/from;a=1;b=;c=3/values
    

    or:

    /path/from;a=1;b;c=3/values
    

Would these use cases work, or could the current solution be extended to support them?

@kdavisk6
Copy link
Member Author

@dpodder

We'll adhere to the URI template specification:
https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.7

the spec defines these use cases:

Example Template     Expansion

       {;who}             ;who=fred
       {;half}            ;half=50%25
       {;empty}           ;empty
       {;v,empty,who}     ;v=6;empty;who=fred
       {;v,bar,who}       ;v=6;who=fred
       {;x,y}             ;x=1024;y=768
       {;x,y,empty}       ;x=1024;y=768;empty
       {;x,y,undef}       ;x=1024;y=768
       {;hello:5}         ;hello=Hello
       {;list}            ;list=red,green,blue
       {;list*}           ;list=red;list=green;list=blue
       {;keys}            ;keys=semi,%3B,dot,.,comma,%2C
       {;keys*}           ;semi=%3B;dot=.;comma=%2C

In the spec ,empty would result in the name being included. To achieve the behavior you are looking for, the map would need to be undefined, which in our world would be null. There are more hints in the spec that explain this behavior:

` else if an explode modifier is given, then

      *  if named is true, then for each defined list member or array
         (name, value) pair with a defined value, do:

         +  if this is not the first defined member/value, append the
            sep string to the result string;

         +  if this is a list, append the varname to the result string
            using the same encoding process as for literals;

         +  if this is a pair, append the name to the result string
            using the same encoding process as for literals;

         +  if the member/value is empty, append the ifemp string to the
            result string; otherwise, append "=" and the member/value to
            the result string, after pct-encoding any member/value
            characters that are not in the allow set.

      *  else if named is false, then

         +  if this is a list, append each defined list member to the
            result string, after pct-encoding any characters that are
            not in the allow set, with the sep string appended to the
            result between each defined list member.

         +  if this is an array of (name, value) pairs, append each pair
            with a defined value to the result string as "name=value",
            after pct-encoding any characters that are not in the allow
            set, with the sep string appended to the result between each
            defined pair.

@kdavisk6 kdavisk6 force-pushed the GH-1319/reserved-characters branch from 8e925cc to c3cc97d Compare March 24, 2022 13:24
Fixes OpenFeign#1319

This change adds limited Path Style support to Feign URI template-style
templates.  Variable expressions that start with a semi-colon `;`
are now expanded in accordance to [RFC 6570 Section 3.2.7](https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.7)
with the following modifications:

* Maps and Lists are expanded by default.
* Only Single variable templates are supported.

Examples:
```
{;who}             ;who=fred
{;half}            ;half=50%25
{;empty}           ;empty
{;list}            ;list=red;list=green;list=blue
{;keys}            ;semi=%3B;dot=.;comma=%2C
```
@kdavisk6 kdavisk6 added the ready to merge Will be merged if no other member ask for changes label Mar 24, 2022
@kdavisk6 kdavisk6 merged commit e1a7028 into OpenFeign:master Mar 25, 2022
@kdavisk6 kdavisk6 deleted the GH-1319/reserved-characters branch March 25, 2022 13:38
@dpodder
Copy link

dpodder commented Mar 27, 2022

@kdavisk6 - thank you for the clarification and details pointing out the relevant details from the spec! It looks like the {;keys*} example supported by the spec is exactly the behavior we're looking for, and would expand correctly for our cases. (Edit to add:) which with the caveat mentioned in the readme, would need to be used as {;keys} for us, but no big deal.

velo pushed a commit that referenced this pull request Oct 7, 2024
* [GH-1319] Add Support for Path Style Parameter Expansion

Fixes #1319

This change adds limited Path Style support to Feign URI template-style
templates.  Variable expressions that start with a semi-colon `;`
are now expanded in accordance to [RFC 6570 Section 3.2.7](https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.7)
with the following modifications:

* Maps and Lists are expanded by default.
* Only Single variable templates are supported.

Examples:
```
{;who}             ;who=fred
{;half}            ;half=50%25
{;empty}           ;empty
{;list}            ;list=red;list=green;list=blue
{;keys}            ;semi=%3B;dot=.;comma=%2C
```

* Export Path Style Expression as an Expander for use with custom contracts

* Added example to ReadMe

* Additional Test Cases.
velo pushed a commit that referenced this pull request Oct 8, 2024
* [GH-1319] Add Support for Path Style Parameter Expansion

Fixes #1319

This change adds limited Path Style support to Feign URI template-style
templates.  Variable expressions that start with a semi-colon `;`
are now expanded in accordance to [RFC 6570 Section 3.2.7](https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.7)
with the following modifications:

* Maps and Lists are expanded by default.
* Only Single variable templates are supported.

Examples:
```
{;who}             ;who=fred
{;half}            ;half=50%25
{;empty}           ;empty
{;list}            ;list=red;list=green;list=blue
{;keys}            ;semi=%3B;dot=.;comma=%2C
```

* Export Path Style Expression as an Expander for use with custom contracts

* Added example to ReadMe

* Additional Test Cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Will be merged if no other member ask for changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support unencoded reserved characters in path segments
3 participants