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

Keep track of baseUrl root path and handle path-absolute resources. #851

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aivins
Copy link

@aivins aivins commented Aug 21, 2014

This is related to issue #620.

API endpoint returns a path-absolute self link, such as /api/v1/person/5.

baseUrl setting includes part of this path, such as http://some.host:8000/api/v1/.
baseUrlRoot will be set to http://some.host:8000.

If setSelfLinkAbsoluteUrl is not true, then a second check will happen to see if the self link begins with a slash. If so it is treated as path-relative and appended to baseUrlRoot.

This covers my own issues with self links cross domain as well as the path-absolute issues touched upon in #620.

@pauldijou pauldijou added this to the 1.5.0 milestone Sep 1, 2014
@sevein
Copy link

sevein commented Oct 10, 2014

@aivins, would that work when the self link is relative to the API root? e.g. /person/5 instead of /api/v1/person/5 where baseUrl is /api. I've found that python-eve is moving into that direction (nicolaiarocci/eve@69b8141), which makes sense to me. See also #665.

alexjpaz added a commit to alexjpaz-archive/pazfit2-ui-web that referenced this pull request Nov 21, 2014
@ConcurrentHashMap ConcurrentHashMap removed this from the 1.5.0 milestone Feb 19, 2016
@daviesgeek
Copy link
Collaborator

I think this should be good; I need to read it again and test it more thoroughly. A couple items tho:

  • Can you rebase on master please?
  • And…can you please add some tests?

@bostrom
Copy link
Collaborator

bostrom commented Oct 14, 2016

Any chance of this getting merged anytime soon? I'm seeing quite a few open issues relating to this.

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

Successfully merging this pull request may close these issues.

7 participants