diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-api.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-api.ts index cc44b73b86..3283ed75c9 100644 --- a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-api.ts +++ b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-api.ts @@ -51,6 +51,23 @@ export class GithubApi { return `${pathname}${joiner}${search}`; } + protected getPaginated(pathname: string, baseParams: RequestParams = {}, currentPage: number = 0): Promise { + const perPage = 100; + const params = { + ...baseParams, + page: currentPage, + per_page: perPage, + }; + + return this.get(pathname, params).then(items => { + if (items.length < perPage) { + return items; + } + + return this.getPaginated(pathname, baseParams, currentPage + 1).then(moreItems => [...items, ...moreItems]); + }); + } + protected request(method: string, path: string, data: any = null): Promise { return new Promise((resolve, reject) => { const options = { diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts index a93ea73da8..84b926a593 100644 --- a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts +++ b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts @@ -22,30 +22,11 @@ export class GithubPullRequests extends GithubApi { } public fetchAll(state: PullRequestState = 'all'): Promise { - process.stdout.write(`Fetching ${state} pull requests...`); - return this.fetchUntilDone(state, 0); - } + console.log(`Fetching ${state} pull requests...`); - // Methods - Protected - protected fetchUntilDone(state: PullRequestState, currentPage: number): Promise { - process.stdout.write('.'); - - const perPage = 100; const pathname = `/repos/${this.repoSlug}/pulls`; - const params = { - page: currentPage, - per_page: perPage, - state, - }; + const params = {state}; - return this.get(pathname, params).then(pullRequests => { - if (pullRequests.length < perPage) { - console.log('done'); - return pullRequests; - } - - return this.fetchUntilDone(state, currentPage + 1). - then(morePullRequests => [...pullRequests, ...morePullRequests]); - }); + return this.getPaginated(pathname, params); } } diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-api.spec.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-api.spec.ts index 557bf5cdba..5cf50d9e18 100644 --- a/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-api.spec.ts +++ b/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-api.spec.ts @@ -148,6 +148,81 @@ describe('GithubApi', () => { }); + describe('getPaginated()', () => { + let deferreds: {resolve: Function, reject: Function}[]; + + beforeEach(() => { + deferreds = []; + spyOn(api, 'get').and.callFake(() => new Promise((resolve, reject) => deferreds.push({resolve, reject}))); + }); + + + it('should return a promise', () => { + expect((api as any).getPaginated()).toEqual(jasmine.any(Promise)); + }); + + + it('should call \'get()\' with the correct pathname and params', () => { + (api as any).getPaginated('/foo/bar'); + (api as any).getPaginated('/foo/bar', {baz: 'qux'}); + + expect(api.get).toHaveBeenCalledWith('/foo/bar', {page: 0, per_page: 100}); + expect(api.get).toHaveBeenCalledWith('/foo/bar', {baz: 'qux', page: 0, per_page: 100}); + }); + + + it('should reject if the request fails', done => { + (api as any).getPaginated('/foo/bar').catch(err => { + expect(err).toBe('Test'); + done(); + }); + + deferreds[0].reject('Test'); + }); + + + it('should resolve with the returned items', done => { + const items = [{id: 1}, {id: 2}]; + + (api as any).getPaginated('/foo/bar').then((data: any) => { + expect(data).toEqual(items); + done(); + }); + + deferreds[0].resolve(items); + }); + + + it('should iteratively call \'get()\' to fetch all items', done => { + // Create an array or 250 objects. + const allItems = '.'.repeat(250).split('').map((_, i) => ({id: i})); + const apiGetSpy = api.get as jasmine.Spy; + + (api as any).getPaginated('/foo/bar', {baz: 'qux'}).then((data: any) => { + const paramsForPage = (page: number) => ({baz: 'qux', page, per_page: 100}); + + expect(apiGetSpy).toHaveBeenCalledTimes(3); + expect(apiGetSpy.calls.argsFor(0)).toEqual(['/foo/bar', paramsForPage(0)]); + expect(apiGetSpy.calls.argsFor(1)).toEqual(['/foo/bar', paramsForPage(1)]); + expect(apiGetSpy.calls.argsFor(2)).toEqual(['/foo/bar', paramsForPage(2)]); + + expect(data).toEqual(allItems); + + done(); + }); + + deferreds[0].resolve(allItems.slice(0, 100)); + setTimeout(() => { + deferreds[1].resolve(allItems.slice(100, 200)); + setTimeout(() => { + deferreds[2].resolve(allItems.slice(200)); + }, 0); + }, 0); + }); + + }); + + describe('request()', () => { let httpsRequestSpy: jasmine.Spy; let latestRequest: ClientRequest; diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts index 3c3b13c83b..2f35abc4b0 100644 --- a/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts +++ b/aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts @@ -78,89 +78,38 @@ describe('GithubPullRequests', () => { describe('fetchAll()', () => { let prs: GithubPullRequests; - let deferreds: {resolve: Function, reject: Function}[]; + let prsGetPaginatedSpy: jasmine.Spy; beforeEach(() => { prs = new GithubPullRequests('foo/bar', '12345'); - deferreds = []; - - spyOn(process.stdout, 'write'); - spyOn(prs, 'get').and.callFake(() => new Promise((resolve, reject) => deferreds.push({resolve, reject}))); + prsGetPaginatedSpy = spyOn(prs as any, 'getPaginated'); + spyOn(console, 'log'); }); - it('should return a promise', () => { - expect(prs.fetchAll()).toEqual(jasmine.any(Promise)); - }); + it('should call \'getPaginated()\' with the correct pathname and params', () => { + const expectedPathname = '/repos/foo/bar/pulls'; - - it('should call \'get()\' with the correct pathname and params', () => { + prs.fetchAll('all'); + prs.fetchAll('closed'); prs.fetchAll('open'); - expect(prs.get).toHaveBeenCalledWith('/repos/foo/bar/pulls', { - page: 0, - per_page: 100, - state: 'open', - }); + expect(prsGetPaginatedSpy).toHaveBeenCalledTimes(3); + expect(prsGetPaginatedSpy.calls.argsFor(0)).toEqual([expectedPathname, {state: 'all'}]); + expect(prsGetPaginatedSpy.calls.argsFor(1)).toEqual([expectedPathname, {state: 'closed'}]); + expect(prsGetPaginatedSpy.calls.argsFor(2)).toEqual([expectedPathname, {state: 'open'}]); }); it('should default to \'all\' if no state is specified', () => { prs.fetchAll(); - - expect(prs.get).toHaveBeenCalledWith('/repos/foo/bar/pulls', { - page: 0, - per_page: 100, - state: 'all', - }); + expect(prsGetPaginatedSpy).toHaveBeenCalledWith('/repos/foo/bar/pulls', {state: 'all'}); }); - it('should reject if the request fails', done => { - prs.fetchAll().catch(err => { - expect(err).toBe('Test'); - done(); - }); - - deferreds[0].reject('Test'); - }); - - - it('should resolve with the returned pull requests', done => { - const pullRequests = [{id: 1}, {id: 2}]; - - prs.fetchAll().then(data => { - expect(data).toEqual(pullRequests); - done(); - }); - - deferreds[0].resolve(pullRequests); - }); - - - it('should iteratively call \'get()\' to fetch all pull requests', done => { - // Create an array or 250 objects. - const allPullRequests = '.'.repeat(250).split('').map((_, i) => ({id: i})); - const prsGetApy = prs.get as jasmine.Spy; - - prs.fetchAll().then(data => { - const paramsForPage = (page: number) => ({page, per_page: 100, state: 'all'}); - - expect(prsGetApy).toHaveBeenCalledTimes(3); - expect(prsGetApy.calls.argsFor(0)).toEqual(['/repos/foo/bar/pulls', paramsForPage(0)]); - expect(prsGetApy.calls.argsFor(1)).toEqual(['/repos/foo/bar/pulls', paramsForPage(1)]); - expect(prsGetApy.calls.argsFor(2)).toEqual(['/repos/foo/bar/pulls', paramsForPage(2)]); - - expect(data).toEqual(allPullRequests); - - done(); - }); - - deferreds[0].resolve(allPullRequests.slice(0, 100)); - setTimeout(() => { - deferreds[1].resolve(allPullRequests.slice(100, 200)); - setTimeout(() => deferreds[2].resolve(allPullRequests.slice(200)), 0); - }, 0); + it('should forward the value returned by \'getPaginated()\'', () => { + prsGetPaginatedSpy.and.returnValue('Test'); + expect(prs.fetchAll()).toBe('Test'); }); });