Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

Commit

Permalink
fix: don't append leading '/' when building url
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexlafroscia
baljeet authored and alexlafroscia committed Jul 18, 2018
1 parent f71854f commit 3a25d71
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 21 deletions.
32 changes: 20 additions & 12 deletions addon/mixins/ajax-request.ts
View file
Expand Up @@ -102,6 +102,10 @@ function removeLeadingSlash(string: string) {
return string.substring(1);
}

function removeTrailingSlash(string: string) {
return string.slice(0, -1);
}

function stripSlashes(path: string) {
// make sure path starts with `/`
if (startsWithSlash(path)) {
Expand All @@ -110,7 +114,7 @@ function stripSlashes(path: string) {

// remove end `/`
if (endsWithSlash(path)) {
path = path.slice(0, -1);
path = removeTrailingSlash(path);
}
return path;
}
Expand Down Expand Up @@ -473,25 +477,29 @@ export default Mixin.create({

let host = options.host || get(this, 'host');
if (host) {
host = stripSlashes(host);
host = endsWithSlash(host) ? removeTrailingSlash(host) : host;
urlParts.push(host);
}
urlParts.push(host);

let namespace = options.namespace || get(this, 'namespace');
if (namespace) {
namespace = stripSlashes(namespace);
// If the URL has already been constructed (presumably, by Ember Data), then we should just leave it alone
const hasNamespaceRegex = new RegExp(`^(/)?${stripSlashes(namespace)}/`);
if (hasNamespaceRegex.test(url)) {
return url;
}
// If host is given then we need to strip leading slash too( as it will be added through join)
if (host) {
namespace = stripSlashes(namespace);
} else if (endsWithSlash(namespace)) {
namespace = removeTrailingSlash(namespace);
}
urlParts.push(namespace);
}

// If the URL has already been constructed (presumably, by Ember Data), then we should just leave it alone
const hasNamespaceRegex = new RegExp(`^(/)?${namespace}/`);
if (namespace && hasNamespaceRegex.test(url)) {
return url;
}

// *Only* remove a leading slash -- we need to maintain a trailing slash for
// *Only* remove a leading slash when there is host or namespace -- we need to maintain a trailing slash for
// APIs that differentiate between it being and not being present
if (startsWithSlash(url)) {
if (startsWithSlash(url) && urlParts.length !== 0) {
url = removeLeadingSlash(url);
}
urlParts.push(url);
Expand Down
70 changes: 62 additions & 8 deletions tests/unit/mixins/ajax-request-test.js
View file
Expand Up @@ -36,7 +36,7 @@ describe('Unit | Mixin | ajax request', function() {
describe('options method', function() {
it('sets raw data', function() {
const service = new AjaxRequest();
const url = 'test';
const url = '/test';
const type = 'GET';
const ajaxOptions = service.options(url, {
type,
Expand All @@ -57,7 +57,7 @@ describe('Unit | Mixin | ajax request', function() {

it('sets options correctly', function() {
const service = new AjaxRequest();
const url = 'test';
const url = '/test';
const type = 'POST';
const data = JSON.stringify({ key: 'value' });
const ajaxOptions = service.options(url, {
Expand Down Expand Up @@ -87,7 +87,7 @@ describe('Unit | Mixin | ajax request', function() {

it('does not override contentType when defined', function() {
const service = new AjaxRequest();
const url = 'test';
const url = '/test';
const type = 'POST';
const data = JSON.stringify({ key: 'value' });
const ajaxOptions = service.options(url, {
Expand All @@ -108,7 +108,7 @@ describe('Unit | Mixin | ajax request', function() {

it('can handle empty data', function() {
const service = new AjaxRequest();
const url = 'test';
const url = '/test';
const type = 'POST';
const ajaxOptions = service.options(url, { type });

Expand Down Expand Up @@ -198,6 +198,56 @@ describe('Unit | Mixin | ajax request', function() {

expect(ajaxoptions.url).to.equal('https://myurl.com/users/me');
});

it('is set on the namespace(namespace not starting with `/`)', function() {
const RequestWithHostAndNamespace = AjaxRequest.extend({
host: 'https://discuss.emberjs.com',
namespace: 'api/v1'
});
const service = new RequestWithHostAndNamespace();
const url = 'users/me';
const ajaxoptions = service.options(url);

expect(ajaxoptions.url).to.equal(
'https://discuss.emberjs.com/api/v1/users/me'
);
});

it('is set on the namespace(namespace starting with `/`)', function() {
const RequestWithHostAndNamespace = AjaxRequest.extend({
host: 'https://discuss.emberjs.com',
namespace: '/api/v1'
});
const service = new RequestWithHostAndNamespace();
const url = 'users/me';
const ajaxoptions = service.options(url);

expect(ajaxoptions.url).to.equal(
'https://discuss.emberjs.com/api/v1/users/me'
);
});

it('is set with the host address as `//` and url not starting with `/`', function() {
const RequestWithHostAndNamespace = AjaxRequest.extend({
host: '//'
});
const service = new RequestWithHostAndNamespace();
const url = 'users/me';
const ajaxoptions = service.options(url);

expect(ajaxoptions.url).to.equal('//users/me');
});

it('is set with the host address as `//` and url starting with `/`', function() {
const RequestWithHostAndNamespace = AjaxRequest.extend({
host: '//'
});
const service = new RequestWithHostAndNamespace();
const url = '/users/me';
const ajaxoptions = service.options(url);

expect(ajaxoptions.url).to.equal('//users/me');
});
});

describe('namespace', function() {
Expand All @@ -215,9 +265,12 @@ describe('Unit | Mixin | ajax request', function() {
it('can be set on a per-request basis', function() {
const service = new AjaxRequest();

expect(service.options('users/me', { namespace: 'api' }).url).to.equal(
expect(service.options('users/me', { namespace: '/api' }).url).to.equal(
'/api/users/me'
);
expect(service.options('users/me', { namespace: 'api' }).url).to.equal(
'api/users/me'
);
});

it('is set on the url (namespace not starting with `/`)', function() {
Expand All @@ -227,8 +280,8 @@ describe('Unit | Mixin | ajax request', function() {

const service = new RequestWithHost();

expect(service.options('/users/me').url).to.equal('/api/v1/users/me');
expect(service.options('users/me').url).to.equal('/api/v1/users/me');
expect(service.options('/users/me').url).to.equal('api/v1/users/me');
expect(service.options('users/me').url).to.equal('api/v1/users/me');
});
});

Expand Down Expand Up @@ -881,6 +934,7 @@ describe('Unit | Mixin | ajax request', function() {
it('correctly handles no namespace or host', function() {
const req = new AjaxRequest();
expect(req._buildURL('/baz')).to.equal('/baz');
expect(req._buildURL('baz')).to.equal('baz');
});

it('does not build the URL if the namespace is already present', function() {
Expand Down Expand Up @@ -910,7 +964,7 @@ describe('Unit | Mixin | ajax request', function() {

const req = new RequestWithNamespace();
expect(req._buildURL('/admin_users/post')).to.equal(
'/admin/admin_users/post'
'admin/admin_users/post'
);
});

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/services/ajax-test.js
View file
Expand Up @@ -42,6 +42,6 @@ describe('AJAX Service', function() {
})
});
service = CustomService.create();
return service.request('example.com');
return service.request('/example.com');
});
});

0 comments on commit 3a25d71

Please sign in to comment.