From 3955f85298bf7e326eaefb6fc77f4ab2f9133be5 Mon Sep 17 00:00:00 2001 From: Keisuke Kumada Date: Fri, 31 Aug 2018 17:58:24 +0900 Subject: [PATCH] fix(1220): Use scmRepo instead of querying github (#102) --- index.js | 112 ++++++++++++++++++++++++++------------------- test/index.test.js | 67 +++++++++++++++++++-------- 2 files changed, 112 insertions(+), 67 deletions(-) diff --git a/index.js b/index.js index 70d1841..42f2df1 100644 --- a/index.js +++ b/index.js @@ -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 + }; } /** @@ -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', @@ -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({ @@ -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', @@ -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 { @@ -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', diff --git a/test/index.test.js b/test/index.test.js index d0caa75..b54c63e 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -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'); @@ -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); @@ -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, { @@ -1782,8 +1810,8 @@ 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', @@ -1791,14 +1819,13 @@ jobs: 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',