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.

Sign up for GitHub

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

Jump to bottom

Allow UriTemplate to be built with an empty template #32432

Closed
wants to merge 2 commits into from
Closed

Allow UriTemplate to be built with an empty template #32432

wants to merge 2 commits into from

Conversation

bsgrd
Copy link
Contributor

@bsgrd bsgrd commented Mar 13, 2024

@pivotal-cla
Copy link

@bsgrd Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@bsgrd bsgrd mentioned this pull request Mar 13, 2024
Open
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 13, 2024
@pivotal-cla
Copy link

@bsgrd Thank you for signing the Contributor License Agreement!

@jhoeller
Copy link
Contributor

@bsgrd has this been tried with Spring Cloud Gateway already? If this change is definitely sufficient for the purposes there, that would be great to know. Also, do you happen to have any unit tests for the empty template case, e.g. for UriTemplateTests?

Last but not least, it would be great to rebase this PR onto our 6.1.x branch since that is were it would get merged first.

@jhoeller jhoeller changed the title Changed assert int UriTemplate to Asser.notNull in stead of Assert.hasText Change assert in UriTemplate to notNull instead of hasText Mar 13, 2024
@jhoeller
Copy link
Contributor

jhoeller commented Mar 13, 2024 •
edited

Just added a few unit tests to UriTemplateTests locally, seems to work fine so far. No need to go to extra lengths on your side if you don't have any existing unit tests, I'd be happy to add mine then.

It would be great to know whether this works with Spring Cloud Gateway, in any case. And it would help to have the PR rebased onto 6.1.x, we could merge it as-is for tomorrow's 6.1.5 release then.

@jhoeller jhoeller self-assigned this Mar 13, 2024
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 13, 2024
@jhoeller jhoeller added this to the 6.1.5 milestone Mar 13, 2024
@bsgrd
Copy link
Contributor Author

bsgrd commented Mar 13, 2024

@jhoeller Thank you for your feedback.
I have added the unittest I made when making the changes. If you have tests that are more comprehensive, feel free to push 😊
Regarding the rebase, can I just change the target branch in the PR to 6.1.x?

@snicoll snicoll changed the base branch from main to 6.1.x March 13, 2024 13:49
@snicoll snicoll changed the base branch from 6.1.x to main March 13, 2024 13:49
@jhoeller
Copy link
Contributor

@bsgrd thanks that helps. As for the rebase, no worries, we'll handle that on our side.

@snicoll snicoll added for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x labels Mar 13, 2024
@snicoll snicoll self-assigned this Mar 13, 2024
@snicoll snicoll changed the title Change assert in UriTemplate to notNull instead of hasText Allow UriTemplate to be built with an empty template Mar 13, 2024
This was referenced Mar 13, 2024
Closed
Closed
@snicoll snicoll added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x labels Mar 13, 2024
snicoll pushed a commit that referenced this pull request Mar 13, 2024
@bsgrd @snicoll
Allow UriTemplate to be built with an empty template
1b25a15 
See gh-32432
snicoll added a commit that referenced this pull request Mar 13, 2024
@snicoll
Polish "Allow UriTemplate to be built with an empty template"
a34ceb4 
See gh-32432
@snicoll snicoll closed this in 2922a82 Mar 13, 2024
@snicoll
Copy link
Member

snicoll commented Mar 13, 2024

@bsgrd thanks very much for making your first contribution to Spring Framework.

jhoeller added a commit that referenced this pull request Mar 13, 2024
@jhoeller
Additional unit tests for operations on empty UriTemplate
54a6d89 
See gh-32432
jhoeller added a commit that referenced this pull request Mar 13, 2024
@jhoeller
Additional unit tests for operations on empty UriTemplate
072ebb6 
See gh-32432

(cherry picked from commit 54a6d89)
jhoeller added a commit that referenced this pull request Mar 13, 2024
@jhoeller
Additional unit tests for operations on empty UriTemplate
274fba4 
See gh-32432

(cherry picked from commit 54a6d89)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@snicoll snicoll

@jhoeller jhoeller

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Milestone
6.1.5
Development

Successfully merging this pull request may close these issues.

Add support for empty path prefixes
5 participants
@bsgrd @pivotal-cla @jhoeller @snicoll @spring-projects-issues