Skip to content

Commit

Permalink
Merge pull request #597 from opentable/registryUIBug
Browse files Browse the repository at this point in the history
[OC-120] Registry webUI: preview route/view bug
  • Loading branch information
nickbalestra authored Aug 12, 2017
2 parents 3846910 + 890fcf3 commit 8f825fa
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 5 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ src/components/base-component-handlebars/_package
src/components/base-component-jade/_package

test/fixtures/test.tar.gz
test/fixtures/targz-test
test/fixtures/targz-test
5 changes: 5 additions & 0 deletions src/registry/domain/options-sanitiser.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ module.exports = function(input) {
options.prefix = '/';
}

const hasTrailingPrefix = new RegExp(options.prefix + '$');
if (!options.baseUrl.match(hasTrailingPrefix)) {
options.baseUrl = options.baseUrl.replace(/\/$/, '') + options.prefix;
}

if (!options.tempDir) {
options.tempDir = settings.registry.defaultTempPath;
}
Expand Down
59 changes: 55 additions & 4 deletions test/unit/registry-domain-options-sanitiser.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('registry : domain : options-sanitiser', () => {
const sanitise = require('../../src/registry/domain/options-sanitiser');

describe('when options is empty', () => {
const options = {};
const options = { baseUrl: 'http://my-registry.com' };
const defaults = {
prefix: '/',
tempDir: './temp/',
Expand All @@ -25,7 +25,7 @@ describe('registry : domain : options-sanitiser', () => {
});

describe('when verbosity is provided', () => {
const options = { verbosity: 3 };
const options = { baseUrl: 'http://my-registry.com', verbosity: 3 };

it('should leave value untouched', () => {
expect(sanitise(options).verbosity).to.equal(3);
Expand All @@ -35,6 +35,7 @@ describe('registry : domain : options-sanitiser', () => {
describe('customHeadersToSkipOnWeakVersion', () => {
describe('when it contains valid elements', () => {
const options = {
baseUrl: 'http://my-registry.com',
customHeadersToSkipOnWeakVersion: ['header1', 'HeAdEr-TwO', 'HEADER3']
};

Expand All @@ -50,7 +51,10 @@ describe('registry : domain : options-sanitiser', () => {

describe('fallbackRegistryUrl', () => {
describe("when fallbackRegistryUrl doesn't contain / at the end of url", () => {
const options = { fallbackRegistryUrl: 'http://test-url.com' };
const options = {
fallbackRegistryUrl: 'http://test-url.com',
baseUrl: 'http://my-registry.com'
};

it('should add `/` at the end of url', () => {
expect(sanitise(options).fallbackRegistryUrl).to.be.eql(
Expand All @@ -60,7 +64,10 @@ describe('registry : domain : options-sanitiser', () => {
});

describe('when fallbackRegistryUrl contains `/` at the end of url', () => {
const options = { fallbackRegistryUrl: 'http://test-url.com/' };
const options = {
fallbackRegistryUrl: 'http://test-url.com/',
baseUrl: 'http://my-registry.com'
};

it('should not modify fallbackRegistryUrl url', () => {
expect(sanitise(options).fallbackRegistryUrl).to.be.eql(
Expand All @@ -69,4 +76,48 @@ describe('registry : domain : options-sanitiser', () => {
});
});
});

describe('prefix and baseUrl sanitization', () => {
const prefixAndBaseUrlScenarios = [
{
options: { baseUrl: 'http://my-registry.com' },
expected: { baseUrl: 'http://my-registry.com/', prefix: '/' }
},
{
options: { baseUrl: 'http://my-registry.com/' },
expected: { baseUrl: 'http://my-registry.com/', prefix: '/' }
},
{
options: { prefix: '/', baseUrl: 'http://my-registry.com' },
expected: { baseUrl: 'http://my-registry.com/', prefix: '/' }
},
{
options: { prefix: '/', baseUrl: 'http://my-registry.com/' },
expected: { baseUrl: 'http://my-registry.com/', prefix: '/' }
},
{
options: { prefix: '/-/', baseUrl: 'http://my-registry.com' },
expected: { baseUrl: 'http://my-registry.com/-/', prefix: '/-/' }
},
{
options: { prefix: '/-/', baseUrl: 'http://my-registry.com/' },
expected: { baseUrl: 'http://my-registry.com/-/', prefix: '/-/' }
},
{
options: { prefix: '/-/', baseUrl: 'http://my-registry.com/-/' },
expected: { baseUrl: 'http://my-registry.com/-/', prefix: '/-/' }
}
];

it('should support various scenarios correctly', () => {
prefixAndBaseUrlScenarios.forEach(scenario => {
expect(sanitise(scenario.options).prefix).to.equal(
scenario.expected.prefix
);
expect(sanitise(scenario.options).baseUrl).to.equal(
scenario.expected.baseUrl
);
});
});
});
});

0 comments on commit 8f825fa

Please sign in to comment.