From b53b569b51a6b226322bd1da192df08d3c727e0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Fri, 12 Jan 2024 12:59:15 +0000 Subject: [PATCH 01/11] addded condition to prevent previous users and groups from being wiped if sync request fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../providers/GithubMultiOrgEntityProvider.ts | 29 ++++++++++-------- .../src/providers/GithubOrgEntityProvider.ts | 30 +++++++++++-------- .../src/processors/LdapOrgEntityProvider.ts | 23 +++++++------- .../MicrosoftGraphOrgEntityProvider.ts | 23 +++++++------- 4 files changed, 59 insertions(+), 46 deletions(-) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts index cabb9e21ce..858095e766 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts @@ -298,21 +298,24 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { } const allUsers = Array.from(allUsersMap.values()); + if (allUsers.length > 0 || allTeams.length > 0) { + const { markCommitComplete } = markReadComplete({ allUsers, allTeams }); - const { markCommitComplete } = markReadComplete({ allUsers, allTeams }); + await this.connection.applyMutation({ + type: 'full', + entities: [...allUsers, ...allTeams].map(entity => ({ + locationKey: `github-multi-org-provider:${this.options.id}`, + entity: withLocations( + `https://${this.options.gitHubConfig.host}`, + entity, + ), + })), + }); - await this.connection.applyMutation({ - type: 'full', - entities: [...allUsers, ...allTeams].map(entity => ({ - locationKey: `github-multi-org-provider:${this.options.id}`, - entity: withLocations( - `https://${this.options.gitHubConfig.host}`, - entity, - ), - })), - }); - - markCommitComplete(); + markCommitComplete(); + } else { + logger.info('No users or teams to process'); + } } private supportsEventTopics(): string[] { diff --git a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.ts index 0fe61aa229..a3ff980fd6 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.ts @@ -229,21 +229,25 @@ export class GithubOrgEntityProvider } } - const { markCommitComplete } = markReadComplete({ users, teams }); + if (users.length > 0 || teams.length > 0) { + const { markCommitComplete } = markReadComplete({ users, teams }); - await this.connection.applyMutation({ - type: 'full', - entities: [...users, ...teams].map(entity => ({ - locationKey: `github-org-provider:${this.options.id}`, - entity: withLocations( - `https://${this.options.gitHubConfig.host}`, - org, - entity, - ), - })), - }); + await this.connection.applyMutation({ + type: 'full', + entities: [...users, ...teams].map(entity => ({ + locationKey: `github-org-provider:${this.options.id}`, + entity: withLocations( + `https://${this.options.gitHubConfig.host}`, + org, + entity, + ), + })), + }); - markCommitComplete(); + markCommitComplete(); + } else { + logger.info('No users or teams to process'); + } } /** {@inheritdoc @backstage/plugin-events-node#EventSubscriber.onEvent} */ diff --git a/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts b/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts index d4f19d0522..ced28face8 100644 --- a/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts @@ -192,18 +192,21 @@ export class LdapOrgEntityProvider implements EntityProvider { logger, }, ); + if (users.length > 0 || groups.length > 0) { + const { markCommitComplete } = markReadComplete({ users, groups }); - const { markCommitComplete } = markReadComplete({ users, groups }); + await this.connection.applyMutation({ + type: 'full', + entities: [...users, ...groups].map(entity => ({ + locationKey: `ldap-org-provider:${this.options.id}`, + entity: withLocations(this.options.id, entity), + })), + }); - await this.connection.applyMutation({ - type: 'full', - entities: [...users, ...groups].map(entity => ({ - locationKey: `ldap-org-provider:${this.options.id}`, - entity: withLocations(this.options.id, entity), - })), - }); - - markCommitComplete(); + markCommitComplete(); + } else { + logger.info('No users or teams to process'); + } } private schedule(schedule: LdapOrgEntityProviderOptions['schedule']) { diff --git a/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts b/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts index 160696e2e7..3a3d5c29c4 100644 --- a/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts @@ -323,18 +323,21 @@ export class MicrosoftGraphOrgEntityProvider implements EntityProvider { logger: logger, }, ); + if (users.length > 0 || groups.length > 0) { + const { markCommitComplete } = markReadComplete({ users, groups }); - const { markCommitComplete } = markReadComplete({ users, groups }); + await this.connection.applyMutation({ + type: 'full', + entities: [...users, ...groups].map(entity => ({ + locationKey: `msgraph-org-provider:${this.options.id}`, + entity: withLocations(this.options.id, entity), + })), + }); - await this.connection.applyMutation({ - type: 'full', - entities: [...users, ...groups].map(entity => ({ - locationKey: `msgraph-org-provider:${this.options.id}`, - entity: withLocations(this.options.id, entity), - })), - }); - - markCommitComplete(); + markCommitComplete(); + } else { + logger.info('No users or teams to process'); + } } private schedule(taskRunner: TaskRunner) { From a950ed0c7d36524ae6c7a669c3b019d3d181ee63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Fri, 12 Jan 2024 13:12:12 +0000 Subject: [PATCH 02/11] added changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .changeset/thirty-dolls-admire.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/thirty-dolls-admire.md diff --git a/.changeset/thirty-dolls-admire.md b/.changeset/thirty-dolls-admire.md new file mode 100644 index 0000000000..2b39472f8d --- /dev/null +++ b/.changeset/thirty-dolls-admire.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend-module-github': minor +'@backstage/plugin-catalog-backend-module-ldap': minor +'@backstage/plugin-catalog-backend-module-msgraph': minor +--- + +Prevent Entity Providers from eliminating Users and Groups from the DB when the synchronisation fails From 1a37e86b9dfe33aa14308b0a685060ddc2e8c1c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Fri, 12 Jan 2024 13:14:39 +0000 Subject: [PATCH 03/11] added break line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../src/providers/GithubMultiOrgEntityProvider.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts index 858095e766..1465857092 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts @@ -298,6 +298,7 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { } const allUsers = Array.from(allUsersMap.values()); + if (allUsers.length > 0 || allTeams.length > 0) { const { markCommitComplete } = markReadComplete({ allUsers, allTeams }); From 7647d8d870a6486ef7567eee722edba82e4b6881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Tue, 16 Jan 2024 14:26:14 +0000 Subject: [PATCH 04/11] removed if statements checking length, removed try catch preventing error from breaking execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../providers/GithubMultiOrgEntityProvider.ts | 102 ++++++++---------- .../src/providers/GithubOrgEntityProvider.ts | 30 +++--- .../src/processors/LdapOrgEntityProvider.ts | 22 ++-- .../MicrosoftGraphOrgEntityProvider.ts | 22 ++-- 4 files changed, 78 insertions(+), 98 deletions(-) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts index 1465857092..8387a91e3a 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.ts @@ -248,75 +248,67 @@ export class GithubMultiOrgEntityProvider implements EntityProvider { : await this.getAllOrgs(this.options.gitHubConfig); for (const org of orgsToProcess) { - try { - const { headers, type: tokenType } = - await this.options.githubCredentialsProvider.getCredentials({ - url: `${this.options.githubUrl}/${org}`, - }); - const client = graphql.defaults({ - baseUrl: this.options.gitHubConfig.apiBaseUrl, - headers, + const { headers, type: tokenType } = + await this.options.githubCredentialsProvider.getCredentials({ + url: `${this.options.githubUrl}/${org}`, }); + const client = graphql.defaults({ + baseUrl: this.options.gitHubConfig.apiBaseUrl, + headers, + }); - logger.info(`Reading GitHub users and teams for org: ${org}`); + logger.info(`Reading GitHub users and teams for org: ${org}`); - const { users } = await getOrganizationUsers( - client, - org, - tokenType, - this.options.userTransformer, - ); + const { users } = await getOrganizationUsers( + client, + org, + tokenType, + this.options.userTransformer, + ); - const { teams } = await getOrganizationTeams( - client, - org, - this.defaultMultiOrgTeamTransformer.bind(this), - ); + const { teams } = await getOrganizationTeams( + client, + org, + this.defaultMultiOrgTeamTransformer.bind(this), + ); - // Grab current users from `allUsersMap` if they already exist in our - // pending users so we can append to their group membership relations - const pendingUsers = users.map(u => { - const userRef = stringifyEntityRef(u); - if (!allUsersMap.has(userRef)) { - allUsersMap.set(userRef, u); - } - - return allUsersMap.get(userRef); - }); - - if (areGroupEntities(teams)) { - buildOrgHierarchy(teams); - if (areUserEntities(pendingUsers)) { - assignGroupsToUsers(pendingUsers, teams); - } + // Grab current users from `allUsersMap` if they already exist in our + // pending users so we can append to their group membership relations + const pendingUsers = users.map(u => { + const userRef = stringifyEntityRef(u); + if (!allUsersMap.has(userRef)) { + allUsersMap.set(userRef, u); } - allTeams.push(...teams); - } catch (e) { - logger.error(`Failed to read GitHub org data for ${org}: ${e}`); + return allUsersMap.get(userRef); + }); + + if (areGroupEntities(teams)) { + buildOrgHierarchy(teams); + if (areUserEntities(pendingUsers)) { + assignGroupsToUsers(pendingUsers, teams); + } } + + allTeams.push(...teams); } const allUsers = Array.from(allUsersMap.values()); - if (allUsers.length > 0 || allTeams.length > 0) { - const { markCommitComplete } = markReadComplete({ allUsers, allTeams }); + const { markCommitComplete } = markReadComplete({ allUsers, allTeams }); - await this.connection.applyMutation({ - type: 'full', - entities: [...allUsers, ...allTeams].map(entity => ({ - locationKey: `github-multi-org-provider:${this.options.id}`, - entity: withLocations( - `https://${this.options.gitHubConfig.host}`, - entity, - ), - })), - }); + await this.connection.applyMutation({ + type: 'full', + entities: [...allUsers, ...allTeams].map(entity => ({ + locationKey: `github-multi-org-provider:${this.options.id}`, + entity: withLocations( + `https://${this.options.gitHubConfig.host}`, + entity, + ), + })), + }); - markCommitComplete(); - } else { - logger.info('No users or teams to process'); - } + markCommitComplete(); } private supportsEventTopics(): string[] { diff --git a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.ts index a3ff980fd6..0fe61aa229 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.ts @@ -229,25 +229,21 @@ export class GithubOrgEntityProvider } } - if (users.length > 0 || teams.length > 0) { - const { markCommitComplete } = markReadComplete({ users, teams }); + const { markCommitComplete } = markReadComplete({ users, teams }); - await this.connection.applyMutation({ - type: 'full', - entities: [...users, ...teams].map(entity => ({ - locationKey: `github-org-provider:${this.options.id}`, - entity: withLocations( - `https://${this.options.gitHubConfig.host}`, - org, - entity, - ), - })), - }); + await this.connection.applyMutation({ + type: 'full', + entities: [...users, ...teams].map(entity => ({ + locationKey: `github-org-provider:${this.options.id}`, + entity: withLocations( + `https://${this.options.gitHubConfig.host}`, + org, + entity, + ), + })), + }); - markCommitComplete(); - } else { - logger.info('No users or teams to process'); - } + markCommitComplete(); } /** {@inheritdoc @backstage/plugin-events-node#EventSubscriber.onEvent} */ diff --git a/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts b/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts index ced28face8..1a35852a8d 100644 --- a/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts @@ -192,21 +192,17 @@ export class LdapOrgEntityProvider implements EntityProvider { logger, }, ); - if (users.length > 0 || groups.length > 0) { - const { markCommitComplete } = markReadComplete({ users, groups }); + const { markCommitComplete } = markReadComplete({ users, groups }); - await this.connection.applyMutation({ - type: 'full', - entities: [...users, ...groups].map(entity => ({ - locationKey: `ldap-org-provider:${this.options.id}`, - entity: withLocations(this.options.id, entity), - })), - }); + await this.connection.applyMutation({ + type: 'full', + entities: [...users, ...groups].map(entity => ({ + locationKey: `ldap-org-provider:${this.options.id}`, + entity: withLocations(this.options.id, entity), + })), + }); - markCommitComplete(); - } else { - logger.info('No users or teams to process'); - } + markCommitComplete(); } private schedule(schedule: LdapOrgEntityProviderOptions['schedule']) { diff --git a/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts b/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts index 3a3d5c29c4..7c4c41996b 100644 --- a/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts @@ -323,21 +323,17 @@ export class MicrosoftGraphOrgEntityProvider implements EntityProvider { logger: logger, }, ); - if (users.length > 0 || groups.length > 0) { - const { markCommitComplete } = markReadComplete({ users, groups }); + const { markCommitComplete } = markReadComplete({ users, groups }); - await this.connection.applyMutation({ - type: 'full', - entities: [...users, ...groups].map(entity => ({ - locationKey: `msgraph-org-provider:${this.options.id}`, - entity: withLocations(this.options.id, entity), - })), - }); + await this.connection.applyMutation({ + type: 'full', + entities: [...users, ...groups].map(entity => ({ + locationKey: `msgraph-org-provider:${this.options.id}`, + entity: withLocations(this.options.id, entity), + })), + }); - markCommitComplete(); - } else { - logger.info('No users or teams to process'); - } + markCommitComplete(); } private schedule(taskRunner: TaskRunner) { From 6dcd52322735b89de3cd0d4f67bb44c094843988 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Tue, 16 Jan 2024 14:27:55 +0000 Subject: [PATCH 05/11] updated changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .changeset/thirty-dolls-admire.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/.changeset/thirty-dolls-admire.md b/.changeset/thirty-dolls-admire.md index 2b39472f8d..1011508a98 100644 --- a/.changeset/thirty-dolls-admire.md +++ b/.changeset/thirty-dolls-admire.md @@ -1,7 +1,5 @@ --- '@backstage/plugin-catalog-backend-module-github': minor -'@backstage/plugin-catalog-backend-module-ldap': minor -'@backstage/plugin-catalog-backend-module-msgraph': minor --- Prevent Entity Providers from eliminating Users and Groups from the DB when the synchronisation fails From eb896509a23d09f65e31def9a193ffbf5b690ae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Tue, 16 Jan 2024 14:28:54 +0000 Subject: [PATCH 06/11] added breaking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../src/processors/LdapOrgEntityProvider.ts | 1 + .../src/processors/MicrosoftGraphOrgEntityProvider.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts b/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts index 1a35852a8d..d4f19d0522 100644 --- a/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-ldap/src/processors/LdapOrgEntityProvider.ts @@ -192,6 +192,7 @@ export class LdapOrgEntityProvider implements EntityProvider { logger, }, ); + const { markCommitComplete } = markReadComplete({ users, groups }); await this.connection.applyMutation({ diff --git a/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts b/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts index 7c4c41996b..160696e2e7 100644 --- a/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts +++ b/plugins/catalog-backend-module-msgraph/src/processors/MicrosoftGraphOrgEntityProvider.ts @@ -323,6 +323,7 @@ export class MicrosoftGraphOrgEntityProvider implements EntityProvider { logger: logger, }, ); + const { markCommitComplete } = markReadComplete({ users, groups }); await this.connection.applyMutation({ From 1c39e619a78cf1c1ee84705bc034d6373425335f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Tue, 16 Jan 2024 16:08:49 +0000 Subject: [PATCH 07/11] added test to assure mutation is not applied when request fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../providers/GithubOrgEntityProvider.test.ts | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts index c054f26522..1c563b01b8 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts @@ -171,6 +171,53 @@ describe('GithubOrgEntityProvider', () => { type: 'full', }); }); + + // New test case for handling request failure + it('should not apply mutation if a request fails', async () => { + const mockClient = jest.fn(); + + // Simulate a request failure + mockClient.mockRejectedValue(new Error('Network error')); + + (graphql.defaults as jest.Mock).mockReturnValue(mockClient); + + const entityProviderConnection: EntityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + + const logger = getVoidLogger(); + + const gitHubConfig: GithubIntegrationConfig = { + host: 'https://github.com', + }; + + const mockGetCredentials = jest.fn().mockReturnValue({ + headers: { token: 'blah' }, + type: 'app', + }); + + const githubCredentialsProvider: GithubCredentialsProvider = { + getCredentials: mockGetCredentials, + }; + + const entityProvider = new GithubOrgEntityProvider({ + id: 'my-id', + githubCredentialsProvider, + orgUrl: 'https://github.com/backstage', + gitHubConfig, + logger, + }); + + entityProvider.connect(entityProviderConnection); + + try { + await entityProvider.read(); + } catch (e) { + // Failed successfuly! + } + expect(entityProviderConnection.applyMutation).not.toHaveBeenCalled(); + }); }); describe('withLocations', () => { From a16c772dd896f94d3645508e0d07334eadec0b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Tue, 16 Jan 2024 16:15:10 +0000 Subject: [PATCH 08/11] removed redundancies in the tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../providers/GithubOrgEntityProvider.test.ts | 127 +++++++----------- 1 file changed, 46 insertions(+), 81 deletions(-) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts index 1c563b01b8..5aa01c1acf 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts @@ -30,13 +30,51 @@ jest.mock('@octokit/graphql'); describe('GithubOrgEntityProvider', () => { describe('read', () => { + let mockClient; + let entityProviderConnection; + let entityProvider; + + const setupMocks = response => { + mockClient = jest.fn().mockImplementation(response); + (graphql.defaults as jest.Mock).mockReturnValue(mockClient); + }; + + beforeEach(() => { + entityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + + const logger = getVoidLogger(); + const gitHubConfig = { + host: 'https://github.com', + }; + + const mockGetCredentials = jest.fn().mockReturnValue({ + headers: { token: 'blah' }, + type: 'app', + }); + + const githubCredentialsProvider = { + getCredentials: mockGetCredentials, + }; + + entityProvider = new GithubOrgEntityProvider({ + id: 'my-id', + githubCredentialsProvider, + orgUrl: 'https://github.com/backstage', + gitHubConfig, + logger, + }); + + entityProvider.connect(entityProviderConnection); + }); + afterEach(() => jest.resetAllMocks()); it('should read org data and apply mutation', async () => { - const mockClient = jest.fn(); - - mockClient - .mockResolvedValueOnce({ + setupMocks(() => + Promise.resolve({ organization: { membersWithRole: { pageInfo: { hasNextPage: false }, @@ -50,10 +88,6 @@ describe('GithubOrgEntityProvider', () => { }, ], }, - }, - }) - .mockResolvedValueOnce({ - organization: { teams: { pageInfo: { hasNextPage: false }, nodes: [ @@ -76,38 +110,8 @@ describe('GithubOrgEntityProvider', () => { ], }, }, - }); - - (graphql.defaults as jest.Mock).mockReturnValue(mockClient); - - const entityProviderConnection: EntityProviderConnection = { - applyMutation: jest.fn(), - refresh: jest.fn(), - }; - - const logger = getVoidLogger(); - const gitHubConfig: GithubIntegrationConfig = { - host: 'https://github.com', - }; - - const mockGetCredentials = jest.fn().mockReturnValue({ - headers: { token: 'blah' }, - type: 'app', - }); - - const githubCredentialsProvider: GithubCredentialsProvider = { - getCredentials: mockGetCredentials, - }; - - const entityProvider = new GithubOrgEntityProvider({ - id: 'my-id', - githubCredentialsProvider, - orgUrl: 'https://github.com/backstage', - gitHubConfig, - logger, - }); - - entityProvider.connect(entityProviderConnection); + }), + ); await entityProvider.read(); @@ -172,50 +176,11 @@ describe('GithubOrgEntityProvider', () => { }); }); - // New test case for handling request failure it('should not apply mutation if a request fails', async () => { - const mockClient = jest.fn(); + setupMocks(() => Promise.reject(new Error('Network error'))); - // Simulate a request failure - mockClient.mockRejectedValue(new Error('Network error')); + await expect(entityProvider.read()).rejects.toThrow('Network error'); - (graphql.defaults as jest.Mock).mockReturnValue(mockClient); - - const entityProviderConnection: EntityProviderConnection = { - applyMutation: jest.fn(), - refresh: jest.fn(), - }; - - const logger = getVoidLogger(); - - const gitHubConfig: GithubIntegrationConfig = { - host: 'https://github.com', - }; - - const mockGetCredentials = jest.fn().mockReturnValue({ - headers: { token: 'blah' }, - type: 'app', - }); - - const githubCredentialsProvider: GithubCredentialsProvider = { - getCredentials: mockGetCredentials, - }; - - const entityProvider = new GithubOrgEntityProvider({ - id: 'my-id', - githubCredentialsProvider, - orgUrl: 'https://github.com/backstage', - gitHubConfig, - logger, - }); - - entityProvider.connect(entityProviderConnection); - - try { - await entityProvider.read(); - } catch (e) { - // Failed successfuly! - } expect(entityProviderConnection.applyMutation).not.toHaveBeenCalled(); }); }); From 7866861572955369ebbc1c04ebf4af30305392c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Thu, 18 Jan 2024 11:54:25 +0000 Subject: [PATCH 09/11] Added tests to GithubMultiOrgEntityProvider, reduced redundancies in the mocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../GithubMultiOrgEntityProvider.test.ts | 102 +++++++++--------- 1 file changed, 52 insertions(+), 50 deletions(-) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts index 504e5772dd..51ff514ce9 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts @@ -43,11 +43,50 @@ jest.mock('@backstage/integration', () => ({ describe('GithubMultiOrgEntityProvider', () => { describe('read', () => { + let mockClient; + let entityProviderConnection; + let logger; + let gitHubConfig; + let mockGetCredentials; + let entityProvider; + + beforeEach(() => { + mockClient = jest.fn(); + (graphql.defaults as jest.Mock).mockReturnValue(mockClient); + + entityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + + logger = getVoidLogger(); + + gitHubConfig = { host: 'github.com' }; + + mockGetCredentials = jest.fn().mockReturnValue({ + headers: { token: 'blah' }, + type: 'app', + }); + + const githubCredentialsProvider: GithubCredentialsProvider = { + getCredentials: mockGetCredentials, + }; + + entityProvider = new GithubMultiOrgEntityProvider({ + id: 'my-id', + gitHubConfig, + githubCredentialsProvider, + githubUrl: 'https://github.com', + logger, + orgs: ['orgA', 'orgB'], // only include for tests that require it + }); + + entityProvider.connect(entityProviderConnection); + }); + afterEach(() => jest.resetAllMocks()); it('should read specified orgs', async () => { - const mockClient = jest.fn(); - mockClient .mockResolvedValueOnce({ organization: { @@ -155,36 +194,6 @@ describe('GithubMultiOrgEntityProvider', () => { (graphql.defaults as jest.Mock).mockReturnValue(mockClient); - const entityProviderConnection: EntityProviderConnection = { - applyMutation: jest.fn(), - refresh: jest.fn(), - }; - - const logger = getVoidLogger(); - const gitHubConfig: GithubIntegrationConfig = { - host: 'github.com', - }; - - const mockGetCredentials = jest.fn().mockReturnValue({ - headers: { token: 'blah' }, - type: 'app', - }); - - const githubCredentialsProvider: GithubCredentialsProvider = { - getCredentials: mockGetCredentials, - }; - - const entityProvider = new GithubMultiOrgEntityProvider({ - id: 'my-id', - gitHubConfig, - githubCredentialsProvider, - githubUrl: 'https://github.com', - logger, - orgs: ['orgA', 'orgB'], - }); - - entityProvider.connect(entityProviderConnection); - await entityProvider.read(); expect(mockGetCredentials).toHaveBeenCalledWith({ @@ -353,8 +362,6 @@ describe('GithubMultiOrgEntityProvider', () => { }, ]); - const mockClient = jest.fn(); - mockClient .mockResolvedValueOnce({ organization: { @@ -462,26 +469,11 @@ describe('GithubMultiOrgEntityProvider', () => { (graphql.defaults as jest.Mock).mockReturnValue(mockClient); - const entityProviderConnection: EntityProviderConnection = { - applyMutation: jest.fn(), - refresh: jest.fn(), - }; - - const logger = getVoidLogger(); - const gitHubConfig: GithubIntegrationConfig = { - host: 'github.com', - }; - - const mockGetCredentials = jest.fn().mockReturnValue({ - headers: { token: 'blah' }, - type: 'app', - }); - const githubCredentialsProvider: GithubCredentialsProvider = { getCredentials: mockGetCredentials, }; - const entityProvider = new GithubMultiOrgEntityProvider({ + entityProvider = new GithubMultiOrgEntityProvider({ id: 'my-id', gitHubConfig, githubCredentialsProvider, @@ -642,6 +634,16 @@ describe('GithubMultiOrgEntityProvider', () => { type: 'full', }); }); + + it('should not call applyMutation if an error is thrown', async () => { + mockClient.mockImplementationOnce(() => { + throw new Error('Network Error'); + }); + + await expect(entityProvider.read()).rejects.toThrow('Network Error'); + + expect(entityProviderConnection.applyMutation).not.toHaveBeenCalled(); + }); }); describe('withLocations', () => { From 1d5f379be5b9d3991fad402579f3409c71e72149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Wed, 24 Jan 2024 15:23:53 +0000 Subject: [PATCH 10/11] Added TS definitions to tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../providers/GithubMultiOrgEntityProvider.test.ts | 13 +++++++------ .../src/providers/GithubOrgEntityProvider.test.ts | 6 +++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts index 51ff514ce9..9b0d9ab7c8 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts @@ -28,6 +28,7 @@ import { GithubMultiOrgEntityProvider, withLocations, } from './GithubMultiOrgEntityProvider'; +import { Logger } from 'winston'; jest.mock('@octokit/graphql'); @@ -43,12 +44,12 @@ jest.mock('@backstage/integration', () => ({ describe('GithubMultiOrgEntityProvider', () => { describe('read', () => { - let mockClient; - let entityProviderConnection; - let logger; - let gitHubConfig; - let mockGetCredentials; - let entityProvider; + let mockClient: jest.Mock; + let entityProviderConnection: EntityProviderConnection; + let logger: Logger; + let gitHubConfig: { host: string }; + let mockGetCredentials: jest.Mock; + let entityProvider: GithubMultiOrgEntityProvider; beforeEach(() => { mockClient = jest.fn(); diff --git a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts index 5aa01c1acf..486ff47f1c 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubOrgEntityProvider.test.ts @@ -31,10 +31,10 @@ jest.mock('@octokit/graphql'); describe('GithubOrgEntityProvider', () => { describe('read', () => { let mockClient; - let entityProviderConnection; - let entityProvider; + let entityProviderConnection: EntityProviderConnection; + let entityProvider: GithubOrgEntityProvider; - const setupMocks = response => { + const setupMocks = (response: ((...args: any) => any) | undefined) => { mockClient = jest.fn().mockImplementation(response); (graphql.defaults as jest.Mock).mockReturnValue(mockClient); }; From 4134450da3f8383ef9fae524d764c71390b538bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Costa?= Date: Wed, 24 Jan 2024 15:26:38 +0000 Subject: [PATCH 11/11] removed unecessary import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Costa --- .../src/providers/GithubMultiOrgEntityProvider.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts index 9b0d9ab7c8..78c12d92cf 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubMultiOrgEntityProvider.test.ts @@ -17,10 +17,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { GroupEntity, UserEntity } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; -import { - GithubCredentialsProvider, - GithubIntegrationConfig, -} from '@backstage/integration'; +import { GithubCredentialsProvider } from '@backstage/integration'; import { EntityProviderConnection } from '@backstage/plugin-catalog-node'; import { EventSubscriber } from '@backstage/plugin-events-node'; import { graphql } from '@octokit/graphql';