From 1fb61ab7c45850278d845d3389d69dcb8a343485 Mon Sep 17 00:00:00 2001 From: Aditya <38064122+bettercallav@users.noreply.github.com> Date: Fri, 13 Oct 2023 15:34:21 +1100 Subject: [PATCH 1/5] feat: get rate limit of GitHub user. feat: Promise based follower and following resolve if withing rate limit fix: hopefully improve search time --- .../providers/github/GitHubProvider.ts | 265 +++++++++++++----- 1 file changed, 190 insertions(+), 75 deletions(-) diff --git a/src/identities/providers/github/GitHubProvider.ts b/src/identities/providers/github/GitHubProvider.ts index 9c2098c71..3345aa834 100644 --- a/src/identities/providers/github/GitHubProvider.ts +++ b/src/identities/providers/github/GitHubProvider.ts @@ -266,6 +266,48 @@ class GitHubProvider extends Provider { }; } + public async fetchAllFollowing( + authIdentityId: IdentityId, + token: ProviderToken, + ): Promise { + let following = []; + let page = 1; + while (true) { + const request = this.createRequest( + `${this.apiUrl}/user/following?per_page=100&page=${page}`, + { method: 'GET' }, + token, + ); + const response = await fetch(request); + const data = await response.json(); + following = following.concat(data); + if (data.length < 100) break; + page++; + } + return following; + } + + public async fetchAllFollowers( + authIdentityId: IdentityId, + token: ProviderToken, + ): Promise { + let followers = []; + let page = 1; + while (true) { + const request = this.createRequest( + `${this.apiUrl}/user/followers?per_page=100&page=${page}`, + { method: 'GET' }, + token, + ); + const response = await fetch(request); + const data = await response.json(); + followers = followers.concat(data); + if (data.length < 100) break; + page++; + } + return followers; + } + /** * Gets connected IdentityData from following and follower connections. */ @@ -280,40 +322,62 @@ class GitHubProvider extends Provider { ); } providerToken = await this.checkToken(providerToken, authIdentityId); - let pageNum = 1; - while (true) { - const request = this.createRequest( - `${this.apiUrl}/user/following?per_page=100&page=${pageNum}`, + + const getRateLimit = async (token: ProviderToken) => { + const rateLimitRequest = this.createRequest( + `${this.apiUrl}/rate_limit`, { method: 'GET', }, + token, + ); + const rateLimitResponse = await fetch(rateLimitRequest); + return await rateLimitResponse.json(); + }; + + const initialRateLimit = await getRateLimit(providerToken); + let rateLimit = initialRateLimit.resources.core.remaining; + + const getUserInfo = async (token: ProviderToken) => { + const userInfoRequest = this.createRequest( + `${this.apiUrl}/user`, + { method: 'GET' }, + token, + ); + const userInfoResponse = await fetch(userInfoRequest); + return await userInfoResponse.json(); + }; + + const userInfo = await getUserInfo(providerToken); + rateLimit--; + + const totalConnections = userInfo.followers + userInfo.following; + + const canUsePromiseAll = rateLimit > totalConnections; + + if (canUsePromiseAll) { + const allFollowing = await this.fetchAllFollowing( + authIdentityId, providerToken, ); - const response = await fetch(request); - if (!response.ok) { - if (response.status === 401) { - throw new identitiesErrors.ErrorProviderUnauthenticated( - `Invalid access token`, - ); - } - throw new identitiesErrors.ErrorProviderCall( - `Provider responded with ${response.status} ${response.statusText}`, - ); - } - let data; - try { - data = await response.json(); - } catch (e) { - throw new identitiesErrors.ErrorProviderCall( - `Provider response body is not valid JSON`, - { cause: e }, - ); - } - for (const item of data) { - const identityData = await this.getIdentityData( - authIdentityId, - item.login, - ); + const allFollowers = await this.fetchAllFollowers( + authIdentityId, + providerToken, + ); + + const followingPromises = allFollowing.map((user) => + this.getIdentityData(authIdentityId, user.login), + ); + const followerPromises = allFollowers.map((user) => + this.getIdentityData(authIdentityId, user.login), + ); + + const allIdentityData = await Promise.all([ + ...followingPromises, + ...followerPromises, + ]); + + for (const identityData of allIdentityData) { if ( identityData && identitiesUtils.matchIdentityData(identityData, searchTerms) @@ -321,57 +385,108 @@ class GitHubProvider extends Provider { yield identityData; } } - if (data.length === 0) { - break; - } else { - pageNum = pageNum + 1; - } - } - pageNum = 1; - while (true) { - const request = this.createRequest( - `${this.apiUrl}/user/followers?per_page=100&page=${pageNum}`, - { - method: 'GET', - }, - providerToken, - ); - const response = await fetch(request); - if (!response.ok) { - if (response.status === 401) { - throw new identitiesErrors.ErrorProviderUnauthenticated( - `Invalid access token`, - ); + } else { + let pageNum = 1; + while (true) { + if (rateLimit <= 0) { + break; } - throw new identitiesErrors.ErrorProviderCall( - `Provider responded with ${response.status} ${response.statusText}`, - ); - } - let data; - try { - data = await response.json(); - } catch (e) { - throw new identitiesErrors.ErrorProviderCall( - `Provider response body is not valid JSON`, - { cause: e }, + const request = this.createRequest( + `${this.apiUrl}/user/following?per_page=100&page=${pageNum}`, + { + method: 'GET', + }, + providerToken, ); + const response = await fetch(request); + if (!response.ok) { + if (response.status === 401) { + throw new identitiesErrors.ErrorProviderUnauthenticated( + `Invalid access token`, + ); + } + throw new identitiesErrors.ErrorProviderCall( + `Provider responded with ${response.status} ${response.statusText}`, + ); + } + let data; + try { + data = await response.json(); + } catch (e) { + throw new identitiesErrors.ErrorProviderCall( + `Provider response body is not valid JSON`, + { cause: e }, + ); + } + for (const item of data) { + const identityData = await this.getIdentityData( + authIdentityId, + item.login, + ); + if ( + identityData && + identitiesUtils.matchIdentityData(identityData, searchTerms) + ) { + yield identityData; + } + } + rateLimit--; + if (data.length === 0) { + break; + } else { + pageNum = pageNum + 1; + } } - for (const item of data) { - const identityData = await this.getIdentityData( - authIdentityId, - item.login, + pageNum = 1; + while (true) { + if (rateLimit <= 0) { + break; + } + const request = this.createRequest( + `${this.apiUrl}/user/followers?per_page=100&page=${pageNum}`, + { + method: 'GET', + }, + providerToken, ); - if ( - identityData && - identitiesUtils.matchIdentityData(identityData, searchTerms) - ) { - yield identityData; + const response = await fetch(request); + if (!response.ok) { + if (response.status === 401) { + throw new identitiesErrors.ErrorProviderUnauthenticated( + `Invalid access token`, + ); + } + throw new identitiesErrors.ErrorProviderCall( + `Provider responded with ${response.status} ${response.statusText}`, + ); + } + let data; + try { + data = await response.json(); + } catch (e) { + throw new identitiesErrors.ErrorProviderCall( + `Provider response body is not valid JSON`, + { cause: e }, + ); + } + for (const item of data) { + const identityData = await this.getIdentityData( + authIdentityId, + item.login, + ); + if ( + identityData && + identitiesUtils.matchIdentityData(identityData, searchTerms) + ) { + yield identityData; + } + } + rateLimit--; + if (data.length === 0) { + break; + } else { + pageNum = pageNum + 1; } - } - if (data.length === 0) { - break; - } else { - pageNum = pageNum + 1; } } } From a0cf85bb1ac42033529d84e099a0facb39b58ce7 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Sat, 14 Oct 2023 00:07:33 +1100 Subject: [PATCH 2/5] feat: GIthubProvider now uses GraphQL to speed up `getConnectedIdentityDatas` --- .../providers/github/GitHubProvider.ts | 210 ++++-------------- 1 file changed, 39 insertions(+), 171 deletions(-) diff --git a/src/identities/providers/github/GitHubProvider.ts b/src/identities/providers/github/GitHubProvider.ts index 3345aa834..44671ad81 100644 --- a/src/identities/providers/github/GitHubProvider.ts +++ b/src/identities/providers/github/GitHubProvider.ts @@ -266,48 +266,6 @@ class GitHubProvider extends Provider { }; } - public async fetchAllFollowing( - authIdentityId: IdentityId, - token: ProviderToken, - ): Promise { - let following = []; - let page = 1; - while (true) { - const request = this.createRequest( - `${this.apiUrl}/user/following?per_page=100&page=${page}`, - { method: 'GET' }, - token, - ); - const response = await fetch(request); - const data = await response.json(); - following = following.concat(data); - if (data.length < 100) break; - page++; - } - return following; - } - - public async fetchAllFollowers( - authIdentityId: IdentityId, - token: ProviderToken, - ): Promise { - let followers = []; - let page = 1; - while (true) { - const request = this.createRequest( - `${this.apiUrl}/user/followers?per_page=100&page=${page}`, - { method: 'GET' }, - token, - ); - const response = await fetch(request); - const data = await response.json(); - followers = followers.concat(data); - if (data.length < 100) break; - page++; - } - return followers; - } - /** * Gets connected IdentityData from following and follower connections. */ @@ -323,78 +281,17 @@ class GitHubProvider extends Provider { } providerToken = await this.checkToken(providerToken, authIdentityId); - const getRateLimit = async (token: ProviderToken) => { - const rateLimitRequest = this.createRequest( - `${this.apiUrl}/rate_limit`, - { - method: 'GET', - }, - token, - ); - const rateLimitResponse = await fetch(rateLimitRequest); - return await rateLimitResponse.json(); - }; - - const initialRateLimit = await getRateLimit(providerToken); - let rateLimit = initialRateLimit.resources.core.remaining; - - const getUserInfo = async (token: ProviderToken) => { - const userInfoRequest = this.createRequest( - `${this.apiUrl}/user`, - { method: 'GET' }, - token, - ); - const userInfoResponse = await fetch(userInfoRequest); - return await userInfoResponse.json(); - }; - - const userInfo = await getUserInfo(providerToken); - rateLimit--; - - const totalConnections = userInfo.followers + userInfo.following; - - const canUsePromiseAll = rateLimit > totalConnections; - - if (canUsePromiseAll) { - const allFollowing = await this.fetchAllFollowing( - authIdentityId, - providerToken, - ); - const allFollowers = await this.fetchAllFollowers( - authIdentityId, - providerToken, - ); - - const followingPromises = allFollowing.map((user) => - this.getIdentityData(authIdentityId, user.login), - ); - const followerPromises = allFollowers.map((user) => - this.getIdentityData(authIdentityId, user.login), - ); - - const allIdentityData = await Promise.all([ - ...followingPromises, - ...followerPromises, - ]); - - for (const identityData of allIdentityData) { - if ( - identityData && - identitiesUtils.matchIdentityData(identityData, searchTerms) - ) { - yield identityData; - } - } - } else { - let pageNum = 1; + const foundIdentityIds: Set = new Set(); + for (const identityGroup of ['followers', 'following'] as const) { + let cursor: string | undefined; while (true) { - if (rateLimit <= 0) { - break; - } const request = this.createRequest( - `${this.apiUrl}/user/following?per_page=100&page=${pageNum}`, + `${this.apiUrl}/graphql`, { - method: 'GET', + method: 'POST', + body: JSON.stringify({ + query: this.getConnectedIdentityDatasRequestBody(authIdentityId, identityGroup, cursor) + }) }, providerToken, ); @@ -418,77 +315,48 @@ class GitHubProvider extends Provider { { cause: e }, ); } - for (const item of data) { - const identityData = await this.getIdentityData( - authIdentityId, - item.login, - ); - if ( - identityData && - identitiesUtils.matchIdentityData(identityData, searchTerms) - ) { + const foundIdentityData: any[] = data?.data?.user?.[identityGroup]?.nodes ?? []; + for (const identityData of foundIdentityData) { + identityData.providerId = this.id; + if (!foundIdentityIds.has(identityData.identityId) && identitiesUtils.matchIdentityData(identityData, searchTerms)) { + foundIdentityIds.add(identityData.identityId); yield identityData; } } - rateLimit--; - if (data.length === 0) { + if (foundIdentityData.length === 0) { break; } else { - pageNum = pageNum + 1; + const endCursor: string | undefined = data?.data?.user?.followers?.pageInfo.endCursor; + if (endCursor == null) break; + cursor = endCursor; } } - pageNum = 1; - while (true) { - if (rateLimit <= 0) { - break; - } - const request = this.createRequest( - `${this.apiUrl}/user/followers?per_page=100&page=${pageNum}`, - { - method: 'GET', - }, - providerToken, - ); - const response = await fetch(request); - if (!response.ok) { - if (response.status === 401) { - throw new identitiesErrors.ErrorProviderUnauthenticated( - `Invalid access token`, - ); + } + } + + protected getConnectedIdentityDatasRequestBody( + authIdentityId: IdentityId, + identityGroup: 'following' | 'followers', + cursor?: string + ): string { + const query = `query { + user(login: "${authIdentityId}") { + ${identityGroup}(first: 100${cursor == null ? '' : `, after: "${cursor}"`}) { + nodes { + identityId: login + name + email + url } - throw new identitiesErrors.ErrorProviderCall( - `Provider responded with ${response.status} ${response.statusText}`, - ); - } - let data; - try { - data = await response.json(); - } catch (e) { - throw new identitiesErrors.ErrorProviderCall( - `Provider response body is not valid JSON`, - { cause: e }, - ); - } - for (const item of data) { - const identityData = await this.getIdentityData( - authIdentityId, - item.login, - ); - if ( - identityData && - identitiesUtils.matchIdentityData(identityData, searchTerms) - ) { - yield identityData; + pageInfo { + endCursor + startCursor } - } - rateLimit--; - if (data.length === 0) { - break; - } else { - pageNum = pageNum + 1; + totalCount } } - } + }`; + return query; } /** From e318a6cbba2e55acbd3d2fc433595e5c3ffe0d3d Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Sat, 14 Oct 2023 00:25:09 +1100 Subject: [PATCH 3/5] fix: added error handling for GraphQL queries in`GithubProvider.getConnectedIdentityDatas` --- src/identities/providers/github/GitHubProvider.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/identities/providers/github/GitHubProvider.ts b/src/identities/providers/github/GitHubProvider.ts index 44671ad81..ccab8f63b 100644 --- a/src/identities/providers/github/GitHubProvider.ts +++ b/src/identities/providers/github/GitHubProvider.ts @@ -315,6 +315,15 @@ class GitHubProvider extends Provider { { cause: e }, ); } + const error = data?.errors?.at?.(0); + if (error != null) { + throw new identitiesErrors.ErrorProviderCall( + `Provider response body contains an error: ${error.message}`, + { + data: error + } + ); + } const foundIdentityData: any[] = data?.data?.user?.[identityGroup]?.nodes ?? []; for (const identityData of foundIdentityData) { identityData.providerId = this.id; From 6137948803a92c4c0b1a8213e5decf0aef990e3c Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Sat, 14 Oct 2023 12:56:33 +1100 Subject: [PATCH 4/5] fix: moved `JSON.stringify` to `GitHubProvider.getConnectedIdentityDatasRequestBody` --- src/identities/providers/github/GitHubProvider.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/identities/providers/github/GitHubProvider.ts b/src/identities/providers/github/GitHubProvider.ts index ccab8f63b..4e1ec59b5 100644 --- a/src/identities/providers/github/GitHubProvider.ts +++ b/src/identities/providers/github/GitHubProvider.ts @@ -282,16 +282,14 @@ class GitHubProvider extends Provider { providerToken = await this.checkToken(providerToken, authIdentityId); const foundIdentityIds: Set = new Set(); - for (const identityGroup of ['followers', 'following'] as const) { + for (const identityGroup of ['following', 'followers'] as const) { let cursor: string | undefined; while (true) { const request = this.createRequest( `${this.apiUrl}/graphql`, { method: 'POST', - body: JSON.stringify({ - query: this.getConnectedIdentityDatasRequestBody(authIdentityId, identityGroup, cursor) - }) + body: this.getConnectedIdentityDatasRequestBody(authIdentityId, identityGroup, cursor) }, providerToken, ); @@ -365,7 +363,7 @@ class GitHubProvider extends Provider { } } }`; - return query; + return JSON.stringify({ query }); } /** From 0a5899a32d09120fc55b60a0550006cd32c6c7c5 Mon Sep 17 00:00:00 2001 From: Amy Yan Date: Thu, 19 Oct 2023 17:45:44 +1100 Subject: [PATCH 5/5] fix: `GitHubProvider.getConnectedIdentityDatas` would not correctly paginate [ci-skip] --- .../providers/github/GitHubProvider.ts | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/identities/providers/github/GitHubProvider.ts b/src/identities/providers/github/GitHubProvider.ts index 4e1ec59b5..4a4e93b9f 100644 --- a/src/identities/providers/github/GitHubProvider.ts +++ b/src/identities/providers/github/GitHubProvider.ts @@ -280,7 +280,6 @@ class GitHubProvider extends Provider { ); } providerToken = await this.checkToken(providerToken, authIdentityId); - const foundIdentityIds: Set = new Set(); for (const identityGroup of ['following', 'followers'] as const) { let cursor: string | undefined; @@ -289,7 +288,7 @@ class GitHubProvider extends Provider { `${this.apiUrl}/graphql`, { method: 'POST', - body: this.getConnectedIdentityDatasRequestBody(authIdentityId, identityGroup, cursor) + body: this.getConnectedIdentityDatasGraphQLBody(authIdentityId, identityGroup, cursor) }, providerToken, ); @@ -322,7 +321,10 @@ class GitHubProvider extends Provider { } ); } - const foundIdentityData: any[] = data?.data?.user?.[identityGroup]?.nodes ?? []; + // FollowerConnection and FollowingConnection always exists on User + const foundIdentityGroupData = data.data.user[identityGroup]; + // Array always exists on FollowerConnection and FollowingConnection + const foundIdentityData: IdentityData[] = foundIdentityGroupData.nodes; for (const identityData of foundIdentityData) { identityData.providerId = this.id; if (!foundIdentityIds.has(identityData.identityId) && identitiesUtils.matchIdentityData(identityData, searchTerms)) { @@ -333,7 +335,8 @@ class GitHubProvider extends Provider { if (foundIdentityData.length === 0) { break; } else { - const endCursor: string | undefined = data?.data?.user?.followers?.pageInfo.endCursor; + // endCursor may be nullish if this is the last page + const endCursor: string | null = foundIdentityGroupData.pageInfo.endCursor; if (endCursor == null) break; cursor = endCursor; } @@ -341,7 +344,22 @@ class GitHubProvider extends Provider { } } - protected getConnectedIdentityDatasRequestBody( + /** + * Returns a string suitable for use as the request body to the GitHub GraphQL endpoint. + * This is used to construct a query that returns either the `followers` or the `following` of a user. + * + * Schemas Used: + * - https://docs.github.com/en/graphql/reference/queries#user + * - https://docs.github.com/en/graphql/reference/objects#user + * - https://docs.github.com/en/graphql/reference/objects#followerconnection + * + * @param authIdentityId - The GitHub authentication token to use when getting user data + * @param identityGroup - Specify whether the GraphQL query requests the `followers` or the `following` of a user + * @param cursor - cursor for pagination, + * this can be retrieved from `.data.user[identityGroup].pageinfo.endCursor` + * of the JSON body on a response for a request made with the return value of this method as the body. + */ + protected getConnectedIdentityDatasGraphQLBody( authIdentityId: IdentityId, identityGroup: 'following' | 'followers', cursor?: string