From 286d508d4f8d13af72d566612c103288d874d05a Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Fri, 21 Aug 2020 09:46:33 +0100 Subject: [PATCH] Fix GitHub branch limit (#4510) * Ensure we fetch all repo's and branches when deploying github apps * Ensure we only fetch branches once - SUSE/stratos vs suse/stratos - ensure branches are treated as 'local' lists * Increate page size for github commits and gitlab requests --- .../actions/deploy-applications.actions.ts | 1 + .../deploy-application-step2.component.ts | 2 +- .../scm/github-pagination.helper.ts | 152 ++++++++++++++++++ .../shared/data-services/scm/github-scm.ts | 39 ++++- .../shared/data-services/scm/gitlab-scm.ts | 24 ++- .../entity-pagination-request-pipeline.ts | 2 - .../src/helpers/paginated-request-helpers.ts | 49 ++++-- src/frontend/packages/store/src/public-api.ts | 11 +- 8 files changed, 252 insertions(+), 28 deletions(-) create mode 100644 src/frontend/packages/cloud-foundry/src/shared/data-services/scm/github-pagination.helper.ts diff --git a/src/frontend/packages/cloud-foundry/src/actions/deploy-applications.actions.ts b/src/frontend/packages/cloud-foundry/src/actions/deploy-applications.actions.ts index 9604b767da..17c35c2e7f 100644 --- a/src/frontend/packages/cloud-foundry/src/actions/deploy-applications.actions.ts +++ b/src/frontend/packages/cloud-foundry/src/actions/deploy-applications.actions.ts @@ -86,6 +86,7 @@ export class FetchBranchesForProject implements PaginatedAction { type = FETCH_BRANCHES_FOR_PROJECT; entityType = gitBranchesEntityType; paginationKey: string; + flattenPagination = true; static createPaginationKey = (scm: GitSCM, projectName: string) => scm.getType() + ':' + projectName; } diff --git a/src/frontend/packages/cloud-foundry/src/features/applications/deploy-application/deploy-application-step2/deploy-application-step2.component.ts b/src/frontend/packages/cloud-foundry/src/features/applications/deploy-application/deploy-application-step2/deploy-application-step2.component.ts index da3babd5ca..59f9703e0a 100644 --- a/src/frontend/packages/cloud-foundry/src/features/applications/deploy-application/deploy-application-step2/deploy-application-step2.component.ts +++ b/src/frontend/packages/cloud-foundry/src/features/applications/deploy-application/deploy-application-step2/deploy-application-step2.component.ts @@ -247,7 +247,7 @@ export class DeployApplicationStep2Component .pipe( // Wait for a new project name change filter(state => state && !state.checking && !state.error && state.exists), - distinctUntilChanged((x, y) => x.name === y.name), + distinctUntilChanged((x, y) => x.name.toLowerCase() === y.name.toLowerCase()), // Convert project name into branches pagination observable switchMap(state => cfEntityCatalog.gitBranch.store.getPaginationService(null, null, { diff --git a/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/github-pagination.helper.ts b/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/github-pagination.helper.ts new file mode 100644 index 0000000000..1bf9de0e5e --- /dev/null +++ b/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/github-pagination.helper.ts @@ -0,0 +1,152 @@ +import { HttpClient, HttpResponse } from '@angular/common/http'; +import { PaginationFlattener } from '@stratosui/store'; +import { Observable } from 'rxjs'; + +// --------------- Note ---------------- +// There are two types of pagination github uses +// 1) Pagination info is in the body of the response (GithubPaginationResponse / GithubFlattenerPaginationConfig) +// - Array is in the body of the response +// - Things like total number of results in the body of the response +// 2) Pagination info is in the response header (GithubPaginationArrayResponse / GithubFlattenerForArrayPaginationConfig) +// - Array is the body of the response +// - Thinks like total number of results are in a `link` property in the response header + + +/** + * Body of a github pagination response (pagination info inside body) + */ +type GithubPaginationResponse = { + incomplete_results: boolean, + items: T[], + total_count: number +} + +/** + * Body of a github pagination response (pagination info is in header) + */ +type GithubPaginationArrayResponse = T[]; + +export const GITHUB_PER_PAGE_PARAM = 'per_page'; +export const GITHUB_PER_PAGE_PARAM_VALUE = 100; +const GITHUB_MAX_PAGES = 5; +const GITHUB_PAGE_PARAM = 'page'; +const GITHUB_LINK_PAGE_REGEX = /page=([\d]*)/ + +/** + * Config used with `flattenPagination`. To use when the pagination info is in the body + */ +export class GithubFlattenerPaginationConfig implements PaginationFlattener> { + constructor( + private httpClient: HttpClient, + public url: string, + ) { } + + getTotalPages = (res: GithubPaginationResponse): number => { + const total = Math.floor(this.getTotalResults(res) / GITHUB_PER_PAGE_PARAM_VALUE) + 1; + if (total > GITHUB_MAX_PAGES) { + console.warn(`Not fetching all github entities (too many pages: ${total})`) + return GITHUB_MAX_PAGES; + } + return total; + } + getTotalResults = (res: GithubPaginationResponse): number => { + return res.total_count; + }; + mergePages = (response: any[]): T[] => { + return response.reduce((all, res) => { + return all.concat(...res.items); + }, [] as T[]) + }; + fetch = (...args: any[]): Observable> => { + return this.httpClient.get>( + this.url, + { + params: { + ...args[0] + }, + } + ); + }; + buildFetchParams = (i: number): any[] => { + const requestOption = { + [GITHUB_PAGE_PARAM]: i.toString(), + [GITHUB_PER_PAGE_PARAM]: GITHUB_PER_PAGE_PARAM_VALUE.toString() + }; + return [requestOption]; + }; + clearResults = (res: GithubPaginationResponse, allResults: number) => { + throw new Error('Not Implemented') + }; +} + +/** + * Config used with `flattenPagination`. To use when the pagination info in the response header + */ +export class GithubFlattenerForArrayPaginationConfig implements PaginationFlattener, HttpResponse>> { + constructor( + private httpClient: HttpClient, + public url: string, + ) { } + + getTotalPages = (res: HttpResponse>): number => { + // Link: ; rel="next", + // ; rel="last" + + const link = res.headers.get('link'); + if (!link) { + // There's no `link` if there's only one page..... + return 1; + } + const parts = link.split(','); + if (!parts.length) { + throw new Error('Unable to depagination github request (no commas in `link`)'); + } + const last = parts.find(part => part.endsWith('rel="last"')) + if (!last) { + throw new Error('Unable to depagination github request (no `last` in `link`)'); + } + const trimmedLast = last.trim(); + const lastUrl = trimmedLast.slice(1, trimmedLast.indexOf('>')); + const lastPageNumber = GITHUB_LINK_PAGE_REGEX.exec(lastUrl); + if (lastPageNumber.length < 2) { + throw new Error(`Unable to depagination github request (could not find page number in ${lastUrl})`) + } + + const total = parseInt(lastPageNumber[1], 10); + if (total > GITHUB_MAX_PAGES) { + console.warn(`Not fetching all github entities (too many pages: ${total})`) + return GITHUB_MAX_PAGES; + } + return total; + } + getTotalResults = (res: HttpResponse>): number => { + return this.getTotalPages(res) * GITHUB_PER_PAGE_PARAM_VALUE; + }; + mergePages = (response: any[]): GithubPaginationArrayResponse => { + return response.reduce((all, res) => { + return all.concat(...res.body); + }, [] as GithubPaginationArrayResponse) + }; + fetch = (...args: any[]): Observable>> => { + return this.httpClient.get>( + this.url, + { + params: { + ...args[0] + }, + // Required to ensure we can access the https response header + observe: 'response' + } + ); + }; + buildFetchParams = (i: number): any[] => { + const requestOption = { + [GITHUB_PAGE_PARAM]: i.toString(), + [GITHUB_PER_PAGE_PARAM]: GITHUB_PER_PAGE_PARAM_VALUE.toString() + }; + return [requestOption]; + }; + clearResults = (res: HttpResponse>, allResults: number) => { + throw new Error('Not Implemented') + }; +} \ No newline at end of file diff --git a/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/github-scm.ts b/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/github-scm.ts index 5c6180fb38..915eae9576 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/github-scm.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/github-scm.ts @@ -1,9 +1,16 @@ import { HttpClient } from '@angular/common/http'; +import { flattenPagination } from '@stratosui/store'; import { Observable } from 'rxjs'; -import { filter, map } from 'rxjs/operators'; +import { map } from 'rxjs/operators'; import { getGitHubAPIURL } from '../../../../../core/src/core/github.helpers'; import { GitBranch, GitCommit, GitRepo } from '../../../store/types/git.types'; +import { + GITHUB_PER_PAGE_PARAM, + GITHUB_PER_PAGE_PARAM_VALUE, + GithubFlattenerForArrayPaginationConfig, + GithubFlattenerPaginationConfig, +} from './github-pagination.helper'; import { GitSCM, SCMIcon } from './scm'; import { GitSCMType } from './scm.service'; @@ -37,7 +44,14 @@ export class GitHubSCM implements GitSCM { } getBranches(httpClient: HttpClient, projectName: string): Observable { - return httpClient.get(`${this.gitHubURL}/repos/${projectName}/branches`) as Observable; + const url = `${this.gitHubURL}/repos/${projectName}/branches`; + const config = new GithubFlattenerForArrayPaginationConfig(httpClient, url) + const firstRequest = config.fetch(...config.buildFetchParams(1)) + return flattenPagination( + null, + firstRequest, + config + ) } getCommit(httpClient: HttpClient, projectName: string, commitSha: string): Observable { @@ -49,7 +63,12 @@ export class GitHubSCM implements GitSCM { } getCommits(httpClient: HttpClient, projectName: string, ref: string): Observable { - return httpClient.get(`${this.gitHubURL}/repos/${projectName}/commits?sha=${ref}`) as Observable; + return httpClient.get( + `${this.gitHubURL}/repos/${projectName}/commits?sha=${ref}`, { + params: { + [GITHUB_PER_PAGE_PARAM]: GITHUB_PER_PAGE_PARAM_VALUE.toString() + } + }); } getCloneURL(projectName: string): string { @@ -70,12 +89,18 @@ export class GitHubSCM implements GitSCM { if (prjParts.length > 1) { url = `${this.gitHubURL}/search/repositories?q=${prjParts[1]}+in:name+fork:true+user:${prjParts[0]}`; } - return httpClient.get(url).pipe( - filter((repos: any) => !!repos.items), + + const config = new GithubFlattenerPaginationConfig(httpClient, url) + const firstRequest = config.fetch(...config.buildFetchParams(1)) + return flattenPagination( + null, + firstRequest, + config + ).pipe( map(repos => { - return repos.items.map(item => item.full_name); + return repos.map(item => item.full_name); }) - ); + ) } public convertCommit(projectName: string, commit: any): GitCommit { diff --git a/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/gitlab-scm.ts b/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/gitlab-scm.ts index 943a674f9f..badb54157d 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/gitlab-scm.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/data-services/scm/gitlab-scm.ts @@ -8,6 +8,8 @@ import { GitSCM, SCMIcon } from './scm'; import { GitSCMType } from './scm.service'; const gitLabAPIUrl = 'https://gitlab.com/api/v4'; +const GITLAB_PER_PAGE_PARAM = 'per_page'; +const GITLAB_PER_PAGE_PARAM_VALUE = 100; export class GitLabSCM implements GitSCM { @@ -58,7 +60,13 @@ export class GitLabSCM implements GitSCM { getBranches(httpClient: HttpClient, projectName: string): Observable { const prjNameEncoded = encodeURIComponent(projectName); - return httpClient.get(`${gitLabAPIUrl}/projects/${prjNameEncoded}/repository/branches`).pipe( + return httpClient.get( + `${gitLabAPIUrl}/projects/${prjNameEncoded}/repository/branches`, { + params: { + [GITLAB_PER_PAGE_PARAM]: GITLAB_PER_PAGE_PARAM_VALUE.toString() + } + } + ).pipe( map((data: any) => { const branches = []; data.forEach(b => { @@ -86,7 +94,13 @@ export class GitLabSCM implements GitSCM { getCommits(httpClient: HttpClient, projectName: string, commitSha: string): Observable { const prjNameEncoded = encodeURIComponent(projectName); - return httpClient.get(`${gitLabAPIUrl}/projects/${prjNameEncoded}/repository/commits?ref_name=${commitSha}`).pipe( + return httpClient.get( + `${gitLabAPIUrl}/projects/${prjNameEncoded}/repository/commits?ref_name=${commitSha}`, { + params: { + [GITLAB_PER_PAGE_PARAM]: GITLAB_PER_PAGE_PARAM_VALUE.toString() + } + } + ).pipe( map((data: any) => { const commits = []; data.forEach(c => commits.push(this.convertCommit(projectName, c))); @@ -112,7 +126,11 @@ export class GitLabSCM implements GitSCM { const obs$ = prjParts.length > 1 ? this.getMatchingUserGroupRepositories(httpClient, prjParts) : - httpClient.get(`${gitLabAPIUrl}/projects?search=${projectName}`); + httpClient.get(`${gitLabAPIUrl}/projects?search=${projectName}`, { + params: { + [GITLAB_PER_PAGE_PARAM]: GITLAB_PER_PAGE_PARAM_VALUE.toString() + } + }); return obs$.pipe( map((repos: any[]) => repos.map(item => item.path_with_namespace)), diff --git a/src/frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts b/src/frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts index fa82840e99..353886937b 100644 --- a/src/frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts +++ b/src/frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts @@ -10,7 +10,6 @@ import { StratosCatalogEntity, } from '../entity-catalog/entity-catalog-entity/entity-catalog-entity'; import { IStratosEntityDefinition } from '../entity-catalog/entity-catalog.types'; -import { PaginationFlattenerConfig } from '../helpers/paginated-request-helpers'; import { selectPaginationState } from '../selectors/pagination.selectors'; import { PaginatedAction, PaginationEntityState } from '../types/pagination.types'; import { @@ -69,7 +68,6 @@ function getRequestObservable( } export interface PaginatedRequestPipelineConfig extends BasePipelineConfig { action: PaginatedAction; - pageFlattenerConfig: PaginationFlattenerConfig; } export const basePaginatedRequestPipeline: EntityRequestPipeline = ( store: Store, diff --git a/src/frontend/packages/store/src/helpers/paginated-request-helpers.ts b/src/frontend/packages/store/src/helpers/paginated-request-helpers.ts index 2a55648abc..b769333738 100644 --- a/src/frontend/packages/store/src/helpers/paginated-request-helpers.ts +++ b/src/frontend/packages/store/src/helpers/paginated-request-helpers.ts @@ -1,6 +1,6 @@ -import { HttpClient, HttpRequest } from '@angular/common/http'; -import { forkJoin, Observable, of as observableOf } from 'rxjs'; -import { first, map, mergeMap } from 'rxjs/operators'; +import { HttpClient, HttpRequest, HttpResponse } from '@angular/common/http'; +import { forkJoin, Observable, of as observableOf, of } from 'rxjs'; +import { first, map, mergeMap, switchMap } from 'rxjs/operators'; import { UpdatePaginationMaxedState } from '../actions/pagination.actions'; import { ActionDispatcher } from '../entity-request-pipeline/entity-request-pipeline.types'; @@ -9,15 +9,10 @@ import { ActionDispatcher } from '../entity-request-pipeline/entity-request-pipe // TODO: See #4208. This should be replaced with // src/frontend/packages/store/src/entity-request-pipeline/pagination-request-base-handlers/pagination-iterator.pipe.ts -export interface PaginationFlattenerConfig extends Pick< - PaginationFlattener, - 'getTotalPages' | 'getTotalResults' | 'mergePages' | 'clearResults' - > { } - export interface PaginationFlattener { getTotalPages: (res: C) => number; getTotalResults: (res: C) => number; - mergePages: (res: T[]) => T; + mergePages: (res: C[]) => T; fetch: (...args) => Observable; buildFetchParams: (i: number) => any[]; clearResults: (res: C, allResults: number) => Observable; @@ -66,6 +61,39 @@ export class BaseHttpFetcher { } } +/** + * T is what we get back per request, C is what we return in the observable + */ +export interface IteratePaginationConfig { + getFirst: (url: string) => Observable>; + getNext: (response: HttpResponse) => Observable>; + getResult: (response: HttpResponse) => C[]; +} +/** + * T is what we get back per request, C is what we return in the observable + */ +export function iteratePagination( + results: C[] = [], + request: Observable>, + iterator: IteratePaginationConfig +): Observable { + return request.pipe( + first(), + switchMap(response => { + const nextRequest = iterator.getNext(response); + results.push(...iterator.getResult(response)); + if (!nextRequest) { + return of(results); + } + return iteratePagination( + results, + nextRequest, + iterator + ); + }), + ); +} + export function flattenPagination( actionDispatcher: ActionDispatcher, firstRequest: Observable, @@ -90,7 +118,6 @@ export function flattenPagination( return forkJoin([flattener.clearResults(firstResData, allResults)]); } } - // Discover the endpoint with the most pages. This is the amount of request we will need to make to fetch all pages from all // Make those requests const maxRequests = flattener.getTotalPages(firstResData); const requests = []; @@ -102,7 +129,7 @@ export function flattenPagination( } return forkJoin(requests); }), - map((responses: T[]) => { + map((responses: C[]) => { // Merge all responses into the first page return flattener.mergePages(responses); }), diff --git a/src/frontend/packages/store/src/public-api.ts b/src/frontend/packages/store/src/public-api.ts index 1b1787baef..8320216886 100644 --- a/src/frontend/packages/store/src/public-api.ts +++ b/src/frontend/packages/store/src/public-api.ts @@ -2,8 +2,11 @@ * Public API Surface of store */ - // Helpers - export * from './helpers/store-helpers'; - // App State - export { AppState } from './app-state'; \ No newline at end of file +// Helpers +export * from './helpers/store-helpers'; + +// App State +export { AppState } from './app-state'; + +export { flattenPagination, PaginationFlattener } from './helpers/paginated-request-helpers'; \ No newline at end of file