From 35f72dcc83b8dc2b152d3478d852d8cda7f2b07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Thu, 25 Jul 2024 11:51:43 +0200 Subject: [PATCH 1/4] Revert "Removes FilteringGitHubRepositoryDataSource" This reverts commit 660831c68e966e4e4d917d3cc23612fcc8eb4657. --- ...ilteringGitHubRepositoryDataSource.test.ts | 125 ++++++++++++++++++ src/composition.ts | 18 ++- .../FilteringGitHubRepositoryDataSource.ts | 38 ++++++ src/features/projects/domain/index.ts | 1 + 4 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 __test__/projects/FilteringGitHubRepositoryDataSource.test.ts create mode 100644 src/features/projects/domain/FilteringGitHubRepositoryDataSource.ts diff --git a/__test__/projects/FilteringGitHubRepositoryDataSource.test.ts b/__test__/projects/FilteringGitHubRepositoryDataSource.test.ts new file mode 100644 index 00000000..1f408320 --- /dev/null +++ b/__test__/projects/FilteringGitHubRepositoryDataSource.test.ts @@ -0,0 +1,125 @@ +import { FilteringGitHubRepositoryDataSource } from "../../src/features/projects/domain" + +test("It returns all repositories when no hidden repositories are provided", async () => { + const sut = new FilteringGitHubRepositoryDataSource({ + hiddenRepositories: [], + dataSource: { + async getRepositories() { + return [{ + owner: "acme", + name: "foo-openapi", + defaultBranchRef: { + id: "12345678", + name: "main" + }, + branches: [], + tags: [] + }, { + owner: "acme", + name: "bar-openapi", + defaultBranchRef: { + id: "12345678", + name: "bar" + }, + branches: [], + tags: [] + }] + } + } + }) + const repositories = await sut.getRepositories() + expect(repositories.length).toEqual(2) +}) + +test("It removes hidden repository", async () => { + const sut = new FilteringGitHubRepositoryDataSource({ + hiddenRepositories: ["acme/foo-openapi"], + dataSource: { + async getRepositories() { + return [{ + owner: "acme", + name: "foo-openapi", + defaultBranchRef: { + id: "12345678", + name: "main" + }, + branches: [], + tags: [] + }, { + owner: "acme", + name: "bar-openapi", + defaultBranchRef: { + id: "12345678", + name: "bar" + }, + branches: [], + tags: [] + }] + } + } + }) + const repositories = await sut.getRepositories() + expect(repositories.length).toEqual(1) +}) + +test("It returns unmodified list when hidden repository was not found", async () => { + const sut = new FilteringGitHubRepositoryDataSource({ + hiddenRepositories: ["acme/baz-openapi"], + dataSource: { + async getRepositories() { + return [{ + owner: "acme", + name: "foo-openapi", + defaultBranchRef: { + id: "12345678", + name: "main" + }, + branches: [], + tags: [] + }, { + owner: "acme", + name: "bar-openapi", + defaultBranchRef: { + id: "12345678", + name: "bar" + }, + branches: [], + tags: [] + }] + } + } + }) + const repositories = await sut.getRepositories() + expect(repositories.length).toEqual(2) +}) + +test("It removes multiple hidden repositories", async () => { + const sut = new FilteringGitHubRepositoryDataSource({ + hiddenRepositories: ["acme/foo-openapi", "acme/bar-openapi"], + dataSource: { + async getRepositories() { + return [{ + owner: "acme", + name: "foo-openapi", + defaultBranchRef: { + id: "12345678", + name: "main" + }, + branches: [], + tags: [] + }, { + owner: "acme", + name: "bar-openapi", + defaultBranchRef: { + id: "12345678", + name: "bar" + }, + branches: [], + tags: [] + }] + } + } + }) + const repositories = await sut.getRepositories() + expect(repositories.length).toEqual(0) +}) diff --git a/src/composition.ts b/src/composition.ts index 69806563..a78d6d82 100644 --- a/src/composition.ts +++ b/src/composition.ts @@ -22,6 +22,7 @@ import { } from "@/features/projects/data" import { CachingProjectDataSource, + FilteringGitHubRepositoryDataSource, ProjectRepository } from "@/features/projects/domain" import { @@ -158,13 +159,16 @@ export const projectRepository = new ProjectRepository({ export const projectDataSource = new CachingProjectDataSource({ dataSource: new GitHubProjectDataSource({ - repositoryDataSource: new GitHubRepositoryDataSource({ - loginsDataSource: new GitHubLoginDataSource({ - graphQlClient: userGitHubClient - }), - graphQlClient: userGitHubClient, - repositoryNameSuffix: env.getOrThrow("REPOSITORY_NAME_SUFFIX"), - projectConfigurationFilename: env.getOrThrow("SHAPE_DOCS_PROJECT_CONFIGURATION_FILENAME") + repositoryDataSource: new FilteringGitHubRepositoryDataSource({ + hiddenRepositories: listFromCommaSeparatedString(env.getOrThrow("HIDDEN_REPOSITORIES")), + dataSource: new GitHubRepositoryDataSource({ + loginsDataSource: new GitHubLoginDataSource({ + graphQlClient: userGitHubClient + }), + graphQlClient: userGitHubClient, + repositoryNameSuffix: env.getOrThrow("REPOSITORY_NAME_SUFFIX"), + projectConfigurationFilename: env.getOrThrow("SHAPE_DOCS_PROJECT_CONFIGURATION_FILENAME") + }) }), repositoryNameSuffix: env.getOrThrow("REPOSITORY_NAME_SUFFIX") }), diff --git a/src/features/projects/domain/FilteringGitHubRepositoryDataSource.ts b/src/features/projects/domain/FilteringGitHubRepositoryDataSource.ts new file mode 100644 index 00000000..26ee3be3 --- /dev/null +++ b/src/features/projects/domain/FilteringGitHubRepositoryDataSource.ts @@ -0,0 +1,38 @@ +import IGitHubRepositoryDataSource, { + GitHubRepository +} from "./IGitHubRepositoryDataSource" + +export default class FilteringGitHubRepositoryDataSource implements IGitHubRepositoryDataSource { + private readonly dataSource: IGitHubRepositoryDataSource + private readonly hiddenRepositories: string[] + + constructor(config: { + dataSource: IGitHubRepositoryDataSource, + hiddenRepositories: string[] + }) { + this.dataSource = config.dataSource + this.hiddenRepositories = config.hiddenRepositories + } + + async getRepositories(): Promise { + const repositories = await this.dataSource.getRepositories() + // Split full repository names into owner and repository. + // shapehq/foo becomes { owner: "shapehq", "repository": "foo" } + const hiddenOwnerAndRepoNameList = this.hiddenRepositories.map(str => { + const index = str.indexOf("/") + if (index === -1) { + return { owner: str, repository: "" } + } + const owner = str.substring(0, index) + const repository = str.substring(index + 1) + return { owner, repository } + }) + // Only return repositories that are not on the hidden list. + return repositories.filter(repository => { + const hiddenMatch = hiddenOwnerAndRepoNameList.find(e => + e.owner == repository.owner && e.repository == repository.name + ) + return hiddenMatch === undefined + }) + } +} \ No newline at end of file diff --git a/src/features/projects/domain/index.ts b/src/features/projects/domain/index.ts index ad17f135..62596d40 100644 --- a/src/features/projects/domain/index.ts +++ b/src/features/projects/domain/index.ts @@ -1,4 +1,5 @@ export { default as CachingProjectDataSource } from "./CachingProjectDataSource" +export { default as FilteringGitHubRepositoryDataSource } from "./FilteringGitHubRepositoryDataSource" export { default as getSelection } from "./getSelection" export type { default as IGitHubRepositoryDataSource } from "./IGitHubRepositoryDataSource" export * from "./IGitHubRepositoryDataSource" From 62bb5320bd0e4eaa910a867b7ef80e5c9042cf14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Thu, 25 Jul 2024 11:51:50 +0200 Subject: [PATCH 2/4] Revert "Removes HIDDEN_REPOSITORIES" This reverts commit 9d934625c586d7e9565c143bf62f85f0a53ba088. --- .env.example | 1 + 1 file changed, 1 insertion(+) diff --git a/.env.example b/.env.example index 7327e16b..0b7926ac 100644 --- a/.env.example +++ b/.env.example @@ -11,6 +11,7 @@ POSTGRESQL_USER=dbuser POSTGRESQL_PASSWORD= POSTGRESQL_DB=shape-docs REPOSITORY_NAME_SUFFIX=-openapi +HIDDEN_REPOSITORIES= GITHUB_WEBHOOK_SECRET=preshared secret also put in app configuration in GitHub GITHUB_WEBHOK_REPOSITORY_ALLOWLIST= GITHUB_WEBHOK_REPOSITORY_DISALLOWLIST= From 7d2c38505e22ecf69e7ea99b62c988e68572b3c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Thu, 25 Jul 2024 12:19:51 +0200 Subject: [PATCH 3/4] Allows empty HIDDEN_REPOSITORIES --- src/composition.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/composition.ts b/src/composition.ts index a78d6d82..88c7316c 100644 --- a/src/composition.ts +++ b/src/composition.ts @@ -160,7 +160,7 @@ export const projectRepository = new ProjectRepository({ export const projectDataSource = new CachingProjectDataSource({ dataSource: new GitHubProjectDataSource({ repositoryDataSource: new FilteringGitHubRepositoryDataSource({ - hiddenRepositories: listFromCommaSeparatedString(env.getOrThrow("HIDDEN_REPOSITORIES")), + hiddenRepositories: listFromCommaSeparatedString(env.get("HIDDEN_REPOSITORIES")), dataSource: new GitHubRepositoryDataSource({ loginsDataSource: new GitHubLoginDataSource({ graphQlClient: userGitHubClient From 00910cc9b489b61fbb6950744650e2cdbd92f8e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Thu, 25 Jul 2024 13:23:43 +0200 Subject: [PATCH 4/4] Adds splitOwnerAndRepository --- .../utils/splitOwnerAndRepository.test.ts | 26 +++++++++++++++++++ src/common/utils/index.ts | 1 + src/common/utils/splitOwnerAndRepository.ts | 14 ++++++++++ .../FilteringGitHubRepositoryDataSource.ts | 16 +++--------- 4 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 __test__/utils/splitOwnerAndRepository.test.ts create mode 100644 src/common/utils/splitOwnerAndRepository.ts diff --git a/__test__/utils/splitOwnerAndRepository.test.ts b/__test__/utils/splitOwnerAndRepository.test.ts new file mode 100644 index 00000000..d4c162a6 --- /dev/null +++ b/__test__/utils/splitOwnerAndRepository.test.ts @@ -0,0 +1,26 @@ +import { splitOwnerAndRepository } from "@/common" + +test("It returns undefined when string includes no slash", async () => { + const result = splitOwnerAndRepository("foo") + expect(result).toBeUndefined() +}) + +test("It returns undefined when repository is empty", async () => { + const result = splitOwnerAndRepository("foo/") + expect(result).toBeUndefined() +}) + +test("It returns undefined when owner is empty", async () => { + const result = splitOwnerAndRepository("/foo") + expect(result).toBeUndefined() +}) + +test("It splits owner and repository", async () => { + const result = splitOwnerAndRepository("acme/foo") + expect(result).toEqual({ owner: "acme", repository: "foo" }) +}) + +test("It splits owner and repository for repository name containing a slash", async () => { + const result = splitOwnerAndRepository("acme/foo/bar") + expect(result).toEqual({ owner: "acme", repository: "foo/bar" }) +}) diff --git a/src/common/utils/index.ts b/src/common/utils/index.ts index 9d51c208..76bd7c88 100644 --- a/src/common/utils/index.ts +++ b/src/common/utils/index.ts @@ -6,3 +6,4 @@ export { default as env } from "./env" export { default as getBaseFilename } from "./getBaseFilename" export { default as isMac } from "./isMac" export { default as useKeyboardShortcut } from "./useKeyboardShortcut" +export { default as splitOwnerAndRepository } from "./splitOwnerAndRepository" diff --git a/src/common/utils/splitOwnerAndRepository.ts b/src/common/utils/splitOwnerAndRepository.ts new file mode 100644 index 00000000..3a4f80f4 --- /dev/null +++ b/src/common/utils/splitOwnerAndRepository.ts @@ -0,0 +1,14 @@ +// Split full repository names into owner and repository. +// shapehq/foo becomes { owner: "shapehq", "repository": "foo" } +export default (str: string) => { + const index = str.indexOf("/") + if (index === -1) { + return undefined + } + const owner = str.substring(0, index) + const repository = str.substring(index + 1) + if (owner.length == 0 || repository.length == 0) { + return undefined + } + return { owner, repository } +} diff --git a/src/features/projects/domain/FilteringGitHubRepositoryDataSource.ts b/src/features/projects/domain/FilteringGitHubRepositoryDataSource.ts index 26ee3be3..e4018b3b 100644 --- a/src/features/projects/domain/FilteringGitHubRepositoryDataSource.ts +++ b/src/features/projects/domain/FilteringGitHubRepositoryDataSource.ts @@ -1,6 +1,7 @@ import IGitHubRepositoryDataSource, { GitHubRepository } from "./IGitHubRepositoryDataSource" +import { splitOwnerAndRepository } from "@/common" export default class FilteringGitHubRepositoryDataSource implements IGitHubRepositoryDataSource { private readonly dataSource: IGitHubRepositoryDataSource @@ -16,18 +17,9 @@ export default class FilteringGitHubRepositoryDataSource implements IGitHubRepos async getRepositories(): Promise { const repositories = await this.dataSource.getRepositories() - // Split full repository names into owner and repository. - // shapehq/foo becomes { owner: "shapehq", "repository": "foo" } - const hiddenOwnerAndRepoNameList = this.hiddenRepositories.map(str => { - const index = str.indexOf("/") - if (index === -1) { - return { owner: str, repository: "" } - } - const owner = str.substring(0, index) - const repository = str.substring(index + 1) - return { owner, repository } - }) - // Only return repositories that are not on the hidden list. + const hiddenOwnerAndRepoNameList = this.hiddenRepositories + .map(splitOwnerAndRepository) + .filter(e => e !== undefined) return repositories.filter(repository => { const hiddenMatch = hiddenOwnerAndRepoNameList.find(e => e.owner == repository.owner && e.repository == repository.name