diff --git a/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.test.ts b/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.test.ts index c384debb71..cc10e03b68 100644 --- a/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.test.ts +++ b/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.test.ts @@ -231,7 +231,7 @@ describe('MicrosoftGraphIncrementalEntityProvider', () => { mockClient, 'users', expect.objectContaining({ - query: expect.objectContaining({ top: 999 }), + query: expect.objectContaining({ top: 999 }), // USER_PAGE_SIZE }), ); // No users → advances straight to groups phase @@ -735,6 +735,62 @@ describe('MicrosoftGraphIncrementalEntityProvider', () => { ); }); + it('logs a warning and skips a group member when the transformer throws', async () => { + (mockClient.getOrganization as jest.Mock).mockResolvedValue({ + id: 'org-id', + displayName: 'My Org', + }); + mockRequestOnePage.mockResolvedValueOnce({ + items: [ + { id: 'grp-1', displayName: 'Engineering', mail: 'eng@example.com' }, + ], + nextLink: undefined, + }); + (mockClient.getGroupMembers as jest.Mock).mockReturnValue( + asyncYield( + { + '@odata.type': '#microsoft.graph.user', + id: 'u-bad', + // sparse — no userPrincipalName, transformer will throw + }, + { + '@odata.type': '#microsoft.graph.user', + id: 'u-good', + displayName: 'Alice', + userPrincipalName: 'alice@example.com', + }, + ), + ); + + const provider = new MicrosoftGraphIncrementalEntityProvider({ + id: 'default', + provider: baseProviderConfig as any, + logger, + userTransformer: async user => { + if (!user.userPrincipalName) throw new Error('Missing UPN'); + const { defaultUserTransformer } = await import( + '@backstage/plugin-catalog-backend-module-msgraph' + ); + return defaultUserTransformer(user); + }, + }); + + const result = await provider.next(makeContext(), groupsCursor); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('group member user u-bad failed to transform'), + expect.anything(), + ); + const groupEntity = result.entities!.find( + e => + e.entity.metadata.annotations?.[ + MICROSOFT_GRAPH_GROUP_ID_ANNOTATION + ] === 'grp-1', + ); + // Only the good member should appear + expect(groupEntity!.entity.spec?.members).toHaveLength(1); + }); + it('merges transformer-pre-populated members with fetched membership', async () => { (mockClient.getOrganization as jest.Mock).mockResolvedValue({ id: 'org-id', diff --git a/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.ts b/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.ts index 8b0835e08d..06e125789c 100644 --- a/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.ts +++ b/plugins/catalog-backend-module-msgraph-incremental/src/MicrosoftGraphIncrementalEntityProvider.ts @@ -47,7 +47,10 @@ import { import { LoggerService } from '@backstage/backend-plugin-api'; import { getUserPhotoGated, requestOnePage } from './clientHelpers'; -const PAGE_SIZE = 999; +const USER_PAGE_SIZE = 999; +// Groups phase fetches members for every group on the page, so a smaller page +// size keeps each burst within its time budget. +const GROUP_PAGE_SIZE = 100; /** * Backstage entity names must be ≤63 chars ([a-zA-Z0-9] separated by [-_.]). @@ -301,7 +304,7 @@ export class MicrosoftGraphIncrementalEntityProvider filter: provider.userFilter, expand: provider.userExpand, select: provider.userSelect, - top: PAGE_SIZE, + top: USER_PAGE_SIZE, }, queryMode: provider.queryMode, nextLink, @@ -376,7 +379,7 @@ export class MicrosoftGraphIncrementalEntityProvider search: provider.groupSearch, expand: provider.groupExpand, select: provider.groupSelect, - top: PAGE_SIZE, + top: GROUP_PAGE_SIZE, }, queryMode: provider.queryMode, nextLink, @@ -425,22 +428,31 @@ export class MicrosoftGraphIncrementalEntityProvider const childRefs: string[] = []; for await (const member of client.getGroupMembers(group.id!, { - top: PAGE_SIZE, + top: GROUP_PAGE_SIZE, })) { if (member['@odata.type'] === '#microsoft.graph.user') { - const userEntity = await userTransformer( - member as MicrosoftGraph.User, - ); - if (userEntity) { - userEntity.metadata.name = capEntityName( - userEntity.metadata.name, + try { + const userEntity = await userTransformer( + member as MicrosoftGraph.User, ); - userRefs.push(stringifyEntityRef(userEntity)); - } else { - this.options.logger.debug( + if (userEntity) { + userEntity.metadata.name = capEntityName( + userEntity.metadata.name, + ); + userRefs.push(stringifyEntityRef(userEntity)); + } else { + this.options.logger.debug( + `${this.getProviderName()}: group member user ${ + member.id + } could not be transformed (sparse object?), skipping`, + ); + } + } catch (e) { + this.options.logger.warn( `${this.getProviderName()}: group member user ${ member.id - } could not be transformed (sparse object?), skipping`, + } failed to transform, skipping`, + { error: e }, ); } } else if (member['@odata.type'] === '#microsoft.graph.group') { diff --git a/plugins/catalog-backend-module-msgraph-incremental/src/clientHelpers.test.ts b/plugins/catalog-backend-module-msgraph-incremental/src/clientHelpers.test.ts index fb8b28d8ec..93dd290f2a 100644 --- a/plugins/catalog-backend-module-msgraph-incremental/src/clientHelpers.test.ts +++ b/plugins/catalog-backend-module-msgraph-incremental/src/clientHelpers.test.ts @@ -110,6 +110,21 @@ describe('requestOnePage', () => { ); }); + it('sends $count=true in advanced mode even when no query is provided', async () => { + (client.requestApi as jest.Mock).mockResolvedValue( + makeResponse(200, { value: [] }), + ); + + await requestOnePage(client, 'users', { queryMode: 'advanced' }); + + expect(client.requestApi).toHaveBeenCalledWith( + 'users', + expect.objectContaining({ count: true }), + { ConsistencyLevel: 'eventual' }, + undefined, + ); + }); + it('does not set $count for basic mode', async () => { (client.requestApi as jest.Mock).mockResolvedValue( makeResponse(200, { value: [] }), diff --git a/plugins/catalog-backend-module-msgraph-incremental/src/clientHelpers.ts b/plugins/catalog-backend-module-msgraph-incremental/src/clientHelpers.ts index 7a0cce38c8..2397ba46b9 100644 --- a/plugins/catalog-backend-module-msgraph-incremental/src/clientHelpers.ts +++ b/plugins/catalog-backend-module-msgraph-incremental/src/clientHelpers.ts @@ -45,9 +45,7 @@ export async function requestOnePage( // Microsoft Graph requires $count=true whenever ConsistencyLevel: eventual is set, // including plain listing requests with no $filter or $search. const finalQuery = - appliedQueryMode === 'advanced' && query - ? { ...query, count: true } - : query; + appliedQueryMode === 'advanced' ? { ...(query ?? {}), count: true } : query; const headers: Record = appliedQueryMode === 'advanced' ? { ConsistencyLevel: 'eventual' } : {};