Skip to content

Commit

Permalink
fix(1220): Use scmRepo instead of querying github (#102)
Browse files Browse the repository at this point in the history
  • Loading branch information
kumada626 authored and catto committed Aug 31, 2018
1 parent d9794f2 commit 3955f85
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 67 deletions.
112 changes: 65 additions & 47 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,25 +140,33 @@ class GithubScm extends Scm {
async lookupScmUri(config) {
const [scmHost, scmId, scmBranch] = config.scmUri.split(':');

try {
const repo = await this.breaker.runCommand({
action: 'getById',
token: config.token,
params: { id: scmId }
});
let repoFullName;

const [repoOwner, repoName] = repo.data.full_name.split('/');
if (config.scmRepo) {
repoFullName = config.scmRepo.name;
} else {
try {
const repo = await this.breaker.runCommand({
action: 'getById',
token: config.token,
params: { id: scmId }
});

return {
branch: scmBranch,
host: scmHost,
repo: repoName,
owner: repoOwner
};
} catch (err) {
winston.error(err);
throw err;
repoFullName = repo.data.full_name;
} catch (err) {
winston.error(err);
throw err;
}
}

const [repoOwner, repoName] = repoFullName.split('/');

return {
branch: scmBranch,
host: scmHost,
repo: repoName,
owner: repoOwner
};
}

/**
Expand Down Expand Up @@ -466,13 +474,17 @@ class GithubScm extends Scm {
* @return {Promise} Resolves to the owner's repository permissions
*/
async _getPermissions(config) {
let scmInfo;
const lookupConfig = {
scmUri: config.scmUri,
token: config.token
};

if (config.scmRepo) {
lookupConfig.scmRepo = config.scmRepo;
}

try {
scmInfo = await this.lookupScmUri({
scmUri: config.scmUri,
token: config.token
});
const scmInfo = await this.lookupScmUri(lookupConfig);

const repo = await this.breaker.runCommand({
action: 'get',
Expand Down Expand Up @@ -513,10 +525,16 @@ class GithubScm extends Scm {
return this._getPrInfo(config).then(pr => pr.sha);
}

const scmInfo = await this.lookupScmUri({
const lookupConfig = {
scmUri: config.scmUri,
token: config.token
});
};

if (config.scmRepo) {
lookupConfig.scmRepo = config.scmRepo;
}

const scmInfo = await this.lookupScmUri(lookupConfig);

try {
const branch = await this.breaker.runCommand({
Expand Down Expand Up @@ -594,17 +612,17 @@ class GithubScm extends Scm {
* @return {Promise} Resolves to string containing contents of file
*/
async _getFile(config) {
let scmInfo = {};
const lookupConfig = {
scmUri: config.scmUri,
token: config.token
};

if (!config.scmInfo) {
scmInfo = await this.lookupScmUri({
scmUri: config.scmUri,
token: config.token
});
} else {
scmInfo = config.scmInfo;
if (config.scmRepo) {
lookupConfig.scmRepo = config.scmRepo;
}

const scmInfo = await this.lookupScmUri(lookupConfig);

try {
const file = await this.breaker.runCommand({
action: 'getContent',
Expand Down Expand Up @@ -758,17 +776,17 @@ class GithubScm extends Scm {
* @return {Promise} Resolves to decorated url object
*/
async _decorateUrl(config) {
let scmInfo = {};
const lookupConfig = {
scmUri: config.scmUri,
token: config.token
};

if (!config.scmInfo) {
scmInfo = await this.lookupScmUri({
scmUri: config.scmUri,
token: config.token
});
} else {
scmInfo = config.scmInfo;
if (config.scmRepo) {
lookupConfig.scmRepo = config.scmRepo;
}

const scmInfo = await this.lookupScmUri(lookupConfig);

const baseUrl = `${scmInfo.host}/${scmInfo.owner}/${scmInfo.repo}`;

return {
Expand Down Expand Up @@ -982,17 +1000,17 @@ class GithubScm extends Scm {
* @return {Promise}
*/
async _getPrInfo(config) {
let scmInfo = {};
const lookupConfig = {
scmUri: config.scmUri,
token: config.token
};

if (!config.scmInfo) {
scmInfo = await this.lookupScmUri({
scmUri: config.scmUri,
token: config.token
});
} else {
scmInfo = config.scmInfo;
if (config.scmRepo) {
lookupConfig.scmRepo = config.scmRepo;
}

const scmInfo = await this.lookupScmUri(lookupConfig);

try {
const pullRequestInfo = await this.breaker.runCommand({
action: 'get',
Expand Down
67 changes: 47 additions & 20 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,35 @@ describe('index', function () {
});
});

it('promises to get permissions without querying github when scmRepo is passed', () => {
const configWithScmRepo = Object.assign({}, config);

configWithScmRepo.scmRepo = {
branch: 'branch',
url: 'https://github.com/screwdriver-cd/models/tree/branch',
name: 'screwdriver-cd/models'
};

githubMock.repos.get.yieldsAsync(null, { data: repo });

return scm.getPermissions(configWithScmRepo)
.then((data) => {
assert.deepEqual(data, repo.permissions);

assert.notCalled(githubMock.repos.getById);

assert.calledWith(githubMock.repos.get, {
owner: 'screwdriver-cd',
repo: 'models'
});

assert.calledWith(githubMock.authenticate, {
type: 'oauth',
token: config.token
});
});
});

it('returns an error when github command fails', () => {
const err = new Error('githubError');

Expand Down Expand Up @@ -779,18 +808,18 @@ jobs:
});
});

it('promises to get content without lookupScmUri when a ref is passed', () => {
const configWithScmInfo = Object.assign({}, config);
it('promises to get content without querying github' +
'when a ref and scmRepo is passed', () => {
const configWithScmRepo = Object.assign({}, config);

githubMock.repos.getContent.yieldsAsync(null, { data: returnData });
configWithScmInfo.scmInfo = {
owner: 'screwdriver-cd',
repo: 'models',
configWithScmRepo.scmRepo = {
branch: 'branch',
host: 'host'
url: 'https://github.com/screwdriver-cd/models/tree/branch',
name: 'screwdriver-cd/models'
};

return scm.getFile(configWithScmInfo)
return scm.getFile(configWithScmRepo)
.then((data) => {
assert.deepEqual(data, expectedYaml);

Expand Down Expand Up @@ -1415,18 +1444,17 @@ jobs:
});
});

it('decorates a scm uri with scmInfo', () => {
it('decorates a scm uri without querying github when scmRepo is passed', () => {
const scmUri = 'github.com:102498:boat';
const scmInfo = {
const scmRepo = {
branch: 'boat',
host: 'github.com',
owner: 'iAm',
repo: 'theCaptain'
url: 'https://github.com/iAm/theCaptain/tree/boat',
name: 'iAm/theCaptain'
};

return scm.decorateUrl({
scmUri,
scmInfo,
scmRepo,
token: 'mytokenfortesting'
}).then((data) => {
assert.deepEqual(data, {
Expand Down Expand Up @@ -1782,23 +1810,22 @@ jobs:
});
});

it('returns a pull request with the given prNum and scmInfo', () => {
const configWithScmInfo = Object.assign({}, config);
it('returns a pull request with the given prNum and scmRepo', () => {
const configWithScmRepo = Object.assign({}, config);

githubMock.pullRequests.get.yieldsAsync(null,
{ data: { html_url: 'https://github.com/repoOwner/repoName/pull/1',
number: 1,
head: { sha } } }
);

configWithScmInfo.scmInfo = {
configWithScmRepo.scmRepo = {
branch: 'branch',
host: 'github.com',
owner: 'repoOwner',
repo: 'repoName'
url: 'https://github.com/repoOwner/repoName/tree/branch',
name: 'repoOwner/repoName'
};

return scm._getPrInfo(configWithScmInfo).then((data) => {
return scm._getPrInfo(configWithScmRepo).then((data) => {
assert.deepEqual(data,
{
name: 'PR-1',
Expand Down

0 comments on commit 3955f85

Please sign in to comment.