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

Partially resolving one of multiple query parameters results in malformed template. #79

Closed
wertzui opened this issue Jun 28, 2023 · 6 comments · Fixed by #80
Closed
Assignees
Labels
bug Something isn't working

Comments

@wertzui
Copy link

wertzui commented Jun 28, 2023

When resolving one of multiple query parameters, the result is missing {&

var template = new UriTemplate("https://example.org/{?foo,bar}", true);
template.AddParameter("foo", 42);
Console.WriteLine(template.Resolve());

Results in

https://example.org/?foo=42bar}

But should be

https://example.org/?foo=42{&bar}
@HowardvanRooijen
Copy link

Thanks very much for reporting this and for the repro. We'll take a look!

@mwadams mwadams self-assigned this Jun 28, 2023
@mwadams mwadams added the bug Something isn't working label Jun 28, 2023
@mwadams
Copy link
Contributor

mwadams commented Jun 28, 2023

Quick note verifying a repro.

I can verify that this is the current behaviour.

The reference implementation in Tavis.UriTemplates produces the same result with the equivalent code.

Tavis.UriTemplates.UriTemplate template2 = new("https://example.org/{?foo,bar}", true);
template2.SetParameter("foo", 42);
Console.WriteLine(template2.Resolve());
https://example.org/?foo=42bar}

There are no specs that validate this case.

@mwadams
Copy link
Contributor

mwadams commented Jun 28, 2023

It seems that there is no proper support for "new style" partial template resolution in Tavis.UriTemplates.

I have added some code which produces non-malformed URI templates, but it breaks the multivariable template up into components to deal with the possibility of missing items "in the middle" of the list.

@mwadams
Copy link
Contributor

mwadams commented Jun 28, 2023

This is resolved in 8f53924

@mwadams
Copy link
Contributor

mwadams commented Jun 28, 2023

Which I have accidentally pushed to main. Clearly the branch protection is not correct here!

I'll sort this out and issue a PR.

@mwadams mwadams linked a pull request Jun 28, 2023 that will close this issue
@mwadams
Copy link
Contributor

mwadams commented Jun 28, 2023

@wertzui Thanks for the bug report; can you have a look at the PR and validate that it "works for you".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants