Skip to content

Links are relative to request context #527

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

Closed
cheinema opened this issue Dec 22, 2016 · 14 comments
Closed

Links are relative to request context #527

cheinema opened this issue Dec 22, 2016 · 14 comments
Assignees
Milestone

Comments

@cheinema
Copy link

Since 0.22.0.RELEASE, links are built relative to the request context. The following test case in ControllerLinkBuildUnitTest runs with success against the previous release, but fails with the current.

@Test
public void createsLinkRelativeToContextRoot() {

    request.setContextPath("/ctx");
    request.setServletPath("/foo");
    request.setRequestURI("/ctx/foo");

    assertThat(linkTo(PersonControllerImpl.class).withSelfRel().getHref(), endsWith("/ctx/people"));
}
java.lang.AssertionError: 
Expected: a string ending with "/ctx/people"
     but: was "http://localhost/ctx/foo/people"
Expected :a string ending with "/ctx/people"

This would be a regression to our application due to the invalid links.

@cheinema
Copy link
Author

I suspect that this behavior was introduced with #519.

@odrotbohm
Copy link
Member

Thanks for giving this a spin so quickly. Indeed our migration to the Spring APIs introduced that glitch. I have a fix ready to be polished here. We should be able to craft a release quickly.

@odrotbohm odrotbohm added this to the 0.23 milestone Dec 22, 2016
@odrotbohm odrotbohm self-assigned this Dec 22, 2016
odrotbohm added a commit that referenced this issue Dec 22, 2016
…ilder API.

Our migration to the header handling for #509 (47cefe) unfortunately moved from starting with the servlet mapping for URI creation to using the full URI from the start.

This is now fixed by switching back by creating a UriComponentsBuilder from the current servlet mapping.

Related ticket: #509.
@odrotbohm
Copy link
Member

0.23.0.BUILD-SNAPSHOT binaries available. Care to give them a spin before I start the machinery?

@feffef
Copy link

feffef commented Dec 22, 2016

I am happy to try that out, we also just noticed a similar regression after upgrading to 0.22.0 (as we've been eagerly awaiting the fix for #525 / #511)

@odrotbohm
Copy link
Member

Yeah, please go ahead. Sorry for the trouble. I'm pretty puzzled to see that going out unnoticed :(.

The more important parts of #525 / #511 were fixed in Spring Framework 4.3.5. So you should see very decent improvements by just moving to the latter. 4.3.5 has not really been a required release API wise, I just wanted to wait for it so that we can ship a dependency to it to make sure people get the changes that actually fix the reported issues against Spring HATEOAS.

@feffef
Copy link

feffef commented Dec 22, 2016

I can confirm that all links in our project that are generated with ControllerLinkBuilder are construced correctly again (i.e. not relative to the path of the incoming request) when using 0.23.0.BUILD-SNAPSHOT.

@cheinema
Copy link
Author

fb0ba25 LGTM

@otrosien
Copy link

grml - mea culpa (47cefe6)..

@odrotbohm
Copy link
Member

Alright. Looks like the issue hasn't appeared downstream as none of the projects (Spring Data) actually use the ControllerLinkBuilder.

@otrosien — Nevermind. We should've had tests for that in the first place.

About to push the buttons…

@odrotbohm
Copy link
Member

0.23 is live at the Spring repositories. Maven Central sync seems to have general hickups currently.

@feffef
Copy link

feffef commented Dec 22, 2016

Thanks a lot for fixing this so quickly! Everything is working fine with 0.23 - I only had to add the https://repo.spring.io/libs-release repository, as the release indeed doesn't yet seem to be available on maven central.

@otrosien
Copy link

@olivergierke - also the git tag has not been pushed back to github yet.

@odrotbohm
Copy link
Member

The artifacts are now available via Maven Central. However, the search index doesn't seem to list them yet.

@odrotbohm
Copy link
Member

Should be up now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants