From 9a45ea93ccc496a9ebec99a0c9cd32b5e861be1c Mon Sep 17 00:00:00 2001 From: bgvozdev <20631664+bgvozdev@users.noreply.github.com> Date: Mon, 4 Dec 2023 16:49:48 +1100 Subject: [PATCH] NONE: remove FF (#2569) Co-authored-by: Gary Xue <105693507+gxueatlassian@users.noreply.github.com> --- src/config/feature-flags.ts | 2 - src/sync/branch.test.ts | 2 + src/sync/build.test.ts | 7 + src/sync/code-scanning-alerts.test.ts | 9 + src/sync/commits.test.ts | 2 + src/sync/dependabot-alerts.test.ts | 10 +- src/sync/deployment.test.ts | 5 + src/sync/installation.test.ts | 320 ++++++++++++------------ src/sync/pull-requests.test.ts | 2 + src/sync/scheduler.test.ts | 12 +- src/sync/scheduler.ts | 12 +- src/sync/secret-scanning-alerts.test.ts | 9 +- 12 files changed, 207 insertions(+), 185 deletions(-) diff --git a/src/config/feature-flags.ts b/src/config/feature-flags.ts index b8720f5103..aa7d0a574a 100644 --- a/src/config/feature-flags.ts +++ b/src/config/feature-flags.ts @@ -30,7 +30,6 @@ export enum BooleanFlags { export enum StringFlags { BLOCKED_INSTALLATIONS = "blocked-installations", LOG_LEVEL = "log-level", - HEADERS_TO_ENCRYPT = "headers-to-encrypt", SEND_ALL = "send-all" } @@ -40,7 +39,6 @@ export enum NumberFlags { PREEMPTIVE_RATE_LIMIT_THRESHOLD = "preemptive-rate-limit-threshold", NUMBER_OF_BUILD_PAGES_TO_FETCH_IN_PARALLEL = "number-of-build-to-fetch-in-parallel", BACKFILL_PAGE_SIZE = "backfill-page-size", - BACKFILL_MAX_SUBTASKS = "backfill-max-subtasks", INSTALLATION_TOKEN_CACHE_MAX_SIZE = "installation-token-cache-max-size" } diff --git a/src/sync/branch.test.ts b/src/sync/branch.test.ts index 31a9533bd9..ab71b15d2e 100644 --- a/src/sync/branch.test.ts +++ b/src/sync/branch.test.ts @@ -133,6 +133,7 @@ describe("sync/branches", () => { .create(); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); }); @@ -293,6 +294,7 @@ describe("sync/branches", () => { gitHubServerApp = builderResult.gitHubServerApp!; gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); }); const makeExpectedResponse = async (branchName: string) => { diff --git a/src/sync/build.test.ts b/src/sync/build.test.ts index 16be636043..98671ca123 100644 --- a/src/sync/build.test.ts +++ b/src/sync/build.test.ts @@ -81,6 +81,7 @@ describe("sync/builds", () => { it("should sync builds to Jira when build message contains issue key", async () => { const data: BackfillMessagePayload = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); @@ -137,6 +138,7 @@ describe("sync/builds", () => { it("should not explode when returned payload doesn't have head_commit", async () => { const data: BackfillMessagePayload = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); @@ -202,6 +204,7 @@ describe("sync/builds", () => { expect.anything(), expect.anything() ).mockResolvedValue(NUMBER_OF_PARALLEL_FETCHES); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); const data: BackfillMessagePayload = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; @@ -224,6 +227,7 @@ describe("sync/builds", () => { const data: BackfillMessagePayload = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubNock .get(`/repos/integrations/test-repo-name/actions/runs?per_page=20&page=6`) @@ -236,6 +240,7 @@ describe("sync/builds", () => { it("should sync multiple builds to Jira when they contain issue keys", async () => { const data: BackfillMessagePayload = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); @@ -306,6 +311,7 @@ describe("sync/builds", () => { it("should not call Jira if no issue keys are present", async () => { const data: BackfillMessagePayload = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); @@ -329,6 +335,7 @@ describe("sync/builds", () => { it("should not call Jira if no data is returned", async () => { const data: BackfillMessagePayload = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubNock .get(`/repos/integrations/test-repo-name/actions/runs?per_page=20&page=21`) diff --git a/src/sync/code-scanning-alerts.test.ts b/src/sync/code-scanning-alerts.test.ts index d22f197e84..d70a712c7e 100644 --- a/src/sync/code-scanning-alerts.test.ts +++ b/src/sync/code-scanning-alerts.test.ts @@ -51,6 +51,7 @@ describe("sync/code-scanning-alerts", () => { .get("/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(200, codeScanningAlerts); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); jiraNock .post("/rest/security/1.0/bulk", expectedResponseCloudServer(subscription)) .reply(200); @@ -73,6 +74,7 @@ describe("sync/code-scanning-alerts", () => { .get("/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(200, []); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -86,6 +88,7 @@ describe("sync/code-scanning-alerts", () => { .get("/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(403, { message: "Code scanning is not enabled for this repository" }); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -99,6 +102,7 @@ describe("sync/code-scanning-alerts", () => { .get("/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(403, { message: "Advanced Security must be enabled for this repository to use code scanning" }); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -112,6 +116,7 @@ describe("sync/code-scanning-alerts", () => { .get("/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(404, { message: "Ano analysis found" }); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -125,6 +130,7 @@ describe("sync/code-scanning-alerts", () => { .get("/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(404); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -138,6 +144,7 @@ describe("sync/code-scanning-alerts", () => { .get("/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(451); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -191,6 +198,7 @@ describe("sync/code-scanning-alerts", () => { } }; gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); gheNock .get("/api/v3/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(200, codeScanningAlerts); @@ -216,6 +224,7 @@ describe("sync/code-scanning-alerts", () => { } }; gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); gheNock .get("/api/v3/repos/integrations/test-repo-name/code-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(200, []); diff --git a/src/sync/commits.test.ts b/src/sync/commits.test.ts index f6909047fb..89d0c5e4e8 100644 --- a/src/sync/commits.test.ts +++ b/src/sync/commits.test.ts @@ -89,6 +89,7 @@ describe("sync/commits", () => { .create(); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); }); const verifyMessageSent = async (data: BackfillMessagePayload, delaySec ?: number) => { @@ -260,6 +261,7 @@ describe("sync/commits", () => { gitHubServerApp = builderResult.gitHubServerApp!; gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); }); const makeExpectedJiraResponse = async (commits) => ({ diff --git a/src/sync/dependabot-alerts.test.ts b/src/sync/dependabot-alerts.test.ts index 7e802abc41..c29a5ebdb0 100644 --- a/src/sync/dependabot-alerts.test.ts +++ b/src/sync/dependabot-alerts.test.ts @@ -53,6 +53,7 @@ describe("sync/dependabot-alerts", () => { const data = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; nockDependabotAlertsRequest(dependabotAlerts); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); jiraNock .post( "/rest/security/1.0/bulk", @@ -79,6 +80,7 @@ describe("sync/dependabot-alerts", () => { "data": [] }); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -92,6 +94,7 @@ describe("sync/dependabot-alerts", () => { .get("/repos/integrations/test-repo-name/dependabot/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(403, { message: "Dependabot alerts are disabled for this repository" }); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -105,6 +108,7 @@ describe("sync/dependabot-alerts", () => { .get("/repos/integrations/test-repo-name/dependabot/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(403, { message: "Dependabot alerts are not available for archived repositories" }); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -118,6 +122,7 @@ describe("sync/dependabot-alerts", () => { .get("/repos/integrations/test-repo-name/dependabot/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(404); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -131,6 +136,7 @@ describe("sync/dependabot-alerts", () => { .get("/repos/integrations/test-repo-name/dependabot/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(451); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); // No Jira Nock await expect(processInstallation(mockBackfillQueueSendMessage)(data, sentry, getLogger("test"))).toResolve(); @@ -188,6 +194,7 @@ describe("sync/dependabot-alerts", () => { } }; gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); nockDependabotAlertsRequest(dependabotAlerts); jiraNock .post( @@ -215,6 +222,7 @@ describe("sync/dependabot-alerts", () => { } }; gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); nockDependabotAlertsRequest({ "data": [] }); @@ -357,4 +365,4 @@ const expectedResponseGHEServer = (subscription: Subscription) => ({ "workspaceId": subscription.id }, "operationType": "BACKFILL" -}); \ No newline at end of file +}); diff --git a/src/sync/deployment.test.ts b/src/sync/deployment.test.ts index 4517e4a6f5..608165edf2 100644 --- a/src/sync/deployment.test.ts +++ b/src/sync/deployment.test.ts @@ -320,6 +320,7 @@ describe("sync/deployments", () => { githubUserTokenNock(installationId); githubUserTokenNock(installationId); githubUserTokenNock(installationId); + githubUserTokenNock(installationId); createGitHubNock(deploymentNodesFixture); const lastEdges = deploymentNodesFixture.data.repository.deployments.edges; @@ -458,6 +459,7 @@ describe("sync/deployments", () => { githubUserTokenNock(installationId); githubUserTokenNock(installationId); githubUserTokenNock(installationId); + githubUserTokenNock(installationId); githubNock.get(`/repos/test-repo-owner/test-repo-name/commits/${commitId}`) .reply(200, { commit: { @@ -620,6 +622,7 @@ describe("sync/deployments", () => { const lastEdges = deploymentNodesFixture.data.repository.deployments.edges; createGitHubNock({ data: { repository: { deployments: { edges: [] } } } }, lastEdges[lastEdges.length -1].cursor); + githubUserTokenNock(installationId); githubUserTokenNock(installationId); githubNock.get(`/repos/test-repo-owner/test-repo-name/commits/51e16759cdac67b0d2a94e0674c9603b75a840f6`) .reply(200, { @@ -650,6 +653,7 @@ describe("sync/deployments", () => { it("should not call Jira if no data is returned", async () => { const data = { installationId, jiraHost }; + githubUserTokenNock(installationId); createGitHubNock({ data: { repository: { @@ -732,6 +736,7 @@ describe("sync/deployments", () => { gheUserTokenNock(installationId); gheUserTokenNock(installationId); gheUserTokenNock(installationId); + gheUserTokenNock(installationId); createGitHubServerNock(deploymentNodesFixture); const lastEdges = deploymentNodesFixture.data.repository.deployments.edges; diff --git a/src/sync/installation.test.ts b/src/sync/installation.test.ts index 14f2dee59c..a1b86fae1a 100644 --- a/src/sync/installation.test.ts +++ b/src/sync/installation.test.ts @@ -105,12 +105,6 @@ describe("sync/installation", () => { } }; - when(numberFlag).calledWith( - NumberFlags.BACKFILL_MAX_SUBTASKS, - 0, - jiraHost - ).mockResolvedValue(0); - when(numberFlag).calledWith( NumberFlags.BACKFILL_PAGE_SIZE, expect.anything(), @@ -123,7 +117,6 @@ describe("sync/installation", () => { const sentry: Hub = { setUser: jest.fn() } as Hub; describe("isRetryableWithSmallerRequest()", () => { - it("should return false for error without isRetryable flag", () => { const err = { errors: [ @@ -202,128 +195,134 @@ describe("sync/installation", () => { expect((await Subscription.findByPk(subscription?.id))?.syncStatus).toEqual(SyncStatus.COMPLETE); }); - it("should update cursor and continue sync", async () => { - const sendSqsMessage = jest.fn(); - githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); - githubNock - .post("/graphql", branchesNoLastCursor()) - .query(true) - .reply(200, branchNodesFixture); - jiraNock.post("/rest/devinfo/0.10/bulk").reply(200); - - dedupCallThrough = true; - await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); + describe("with scheduler", () => { + beforeEach(async () => { + configureRateLimit(10, 10); // no subtasks by default, please + }); - expect(sendSqsMessage).toBeCalledTimes(1); - await repoSyncState.reload(); - expect(repoSyncState.branchCursor).toEqual("MQ"); - expect(repoSyncState.branchStatus).toEqual("pending"); - }); + it("should update cursor and continue sync", async () => { + const sendSqsMessage = jest.fn(); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubNock + .post("/graphql", branchesNoLastCursor()) + .query(true) + .reply(200, branchNodesFixture); + jiraNock.post("/rest/devinfo/0.10/bulk").reply(200); - it("should mark task as finished and continue sync", async () => { - const sendSqsMessage = jest.fn(); - githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); - const fixture = cloneDeep(branchNodesFixture); - fixture.data.repository.refs.edges = []; - githubNock - .post("/graphql", branchesNoLastCursor()) - .query(true) - .reply(200, fixture); - - dedupCallThrough = true; - await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); + dedupCallThrough = true; + await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); - expect(sendSqsMessage).toBeCalledTimes(1); - await repoSyncState.reload(); - expect(repoSyncState.branchStatus).toEqual("complete"); - }); + expect(sendSqsMessage).toBeCalledTimes(1); + await repoSyncState.reload(); + expect(repoSyncState.branchCursor).toEqual("MQ"); + expect(repoSyncState.branchStatus).toEqual("pending"); + }); - it("should use page size from FF", async () => { - when(numberFlag).calledWith( - NumberFlags.BACKFILL_PAGE_SIZE, - expect.anything(), - expect.anything() - ).mockResolvedValue(90); + it("should mark task as finished and continue sync", async () => { + const sendSqsMessage = jest.fn(); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + const fixture = cloneDeep(branchNodesFixture); + fixture.data.repository.refs.edges = []; + githubNock + .post("/graphql", branchesNoLastCursor()) + .query(true) + .reply(200, fixture); - const sendSqsMessage = jest.fn(); - githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); - const query = branchesNoLastCursor(); - query.variables.per_page = 90; - githubNock - .post("/graphql", query) - .query(true) - .reply(200, branchNodesFixture); - jiraNock.post("/rest/devinfo/0.10/bulk").reply(200); - - dedupCallThrough = true; - await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); + dedupCallThrough = true; + await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); - expect(sendSqsMessage).toBeCalledTimes(1); - }); + expect(sendSqsMessage).toBeCalledTimes(1); + await repoSyncState.reload(); + expect(repoSyncState.branchStatus).toEqual("complete"); + }); - it("should not allow page sizes larger than 100", async () => { - when(numberFlag).calledWith( - NumberFlags.BACKFILL_PAGE_SIZE, - expect.anything(), - expect.anything() - ).mockResolvedValue(120); + it("should use page size from FF", async () => { + when(numberFlag).calledWith( + NumberFlags.BACKFILL_PAGE_SIZE, + expect.anything(), + expect.anything() + ).mockResolvedValue(90); - const sendSqsMessage = jest.fn(); - githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); - const query = branchesNoLastCursor(); - query.variables.per_page = 100; - githubNock - .post("/graphql", query) - .query(true) - .reply(200, branchNodesFixture); - jiraNock.post("/rest/devinfo/0.10/bulk").reply(200); - - dedupCallThrough = true; - await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); + const sendSqsMessage = jest.fn(); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + const query = branchesNoLastCursor(); + query.variables.per_page = 90; + githubNock + .post("/graphql", query) + .query(true) + .reply(200, branchNodesFixture); + jiraNock.post("/rest/devinfo/0.10/bulk").reply(200); - expect(sendSqsMessage).toBeCalledTimes(1); - }); + dedupCallThrough = true; + await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); - it("should rethrow GitHubClient error", async () => { - const sendSqsMessage = jest.fn(); - githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); - githubNock - .post("/graphql") - .query(true) - .reply(404, {}); + expect(sendSqsMessage).toBeCalledTimes(1); + }); - let err: TaskError; - try { - await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); - await mockedExecuteWithDeduplication.mock.calls[0][1](); - } catch (caught) { - err = caught; - } - expect(err!).toBeInstanceOf(TaskError); - expect(err!.task.task).toEqual("branch"); - expect(err!.cause).toBeInstanceOf(GithubClientNotFoundError); - }); + it("should not allow page sizes larger than 100", async () => { + when(numberFlag).calledWith( + NumberFlags.BACKFILL_PAGE_SIZE, + expect.anything(), + expect.anything() + ).mockResolvedValue(120); - it("should rethrow Jira error", async () => { - const sendSqsMessage = jest.fn(); - githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); - githubNock - .post("/graphql", branchesNoLastCursor()) - .query(true) - .reply(200, branchNodesFixture); - jiraNock.post("/rest/devinfo/0.10/bulk").reply(500); - - let err: TaskError; - try { + const sendSqsMessage = jest.fn(); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + const query = branchesNoLastCursor(); + query.variables.per_page = 100; + githubNock + .post("/graphql", query) + .query(true) + .reply(200, branchNodesFixture); + jiraNock.post("/rest/devinfo/0.10/bulk").reply(200); + + dedupCallThrough = true; await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); - await mockedExecuteWithDeduplication.mock.calls[0][1](); - } catch (caught) { - err = caught; - } - expect(err!).toBeInstanceOf(TaskError); - expect(err!.task.task).toEqual("branch"); - expect(err!.cause).toBeInstanceOf(JiraClientError); + expect(sendSqsMessage).toBeCalledTimes(1); + }); + + it("should rethrow GitHubClient error", async () => { + const sendSqsMessage = jest.fn(); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubNock + .post("/graphql") + .query(true) + .reply(404, {}); + + let err: TaskError; + try { + await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); + await mockedExecuteWithDeduplication.mock.calls[0][1](); + } catch (caught) { + err = caught; + } + expect(err!).toBeInstanceOf(TaskError); + expect(err!.task.task).toEqual("branch"); + expect(err!.cause).toBeInstanceOf(GithubClientNotFoundError); + }); + + it("should rethrow Jira error", async () => { + const sendSqsMessage = jest.fn(); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubNock + .post("/graphql", branchesNoLastCursor()) + .query(true) + .reply(200, branchNodesFixture); + jiraNock.post("/rest/devinfo/0.10/bulk").reply(500); + + let err: TaskError; + try { + await processInstallation(sendSqsMessage)(MESSAGE_PAYLOAD, sentry, TEST_LOGGER); + await mockedExecuteWithDeduplication.mock.calls[0][1](); + + } catch (caught) { + err = caught; + } + expect(err!).toBeInstanceOf(TaskError); + expect(err!.task.task).toEqual("branch"); + expect(err!.cause).toBeInstanceOf(JiraClientError); + }); }); }); @@ -350,27 +349,52 @@ describe("sync/installation", () => { describe("markCurrentTaskAsFailedAndContinue", () => { const mockError = new Error("Oh noes, An error occurred"); - it("does nothing when there's no subscription", async () => { - const sendMessageMock = jest.fn(); - await markCurrentTaskAsFailedAndContinue({ - ...MESSAGE_PAYLOAD, - installationId: (MESSAGE_PAYLOAD.installationId as number) + 1 - }, TASK, false, sendMessageMock, getLogger("test"), mockError); - - const refreshedRepoSyncState = await RepoSyncState.findByPk(repoSyncState.id); - const refreshedSubscription = await Subscription.findByPk(subscription?.id); - expect(repoSyncState.get({ plain: true })).toStrictEqual(refreshedRepoSyncState?.get({ plain: true })); - expect(refreshedSubscription?.get({ plain: true })).toStrictEqual(subscription?.get({ plain: true })); - expect(sendMessageMock).toBeCalledTimes(0); - }); - it("updates status in RepoSyncState table", async () => { - await markCurrentTaskAsFailedAndContinue(MESSAGE_PAYLOAD, TASK, false, jest.fn(), getLogger("test"), mockError); + describe("common", () => { + it("does nothing when there's no subscription", async () => { + const sendMessageMock = jest.fn(); + await markCurrentTaskAsFailedAndContinue({ + ...MESSAGE_PAYLOAD, + installationId: (MESSAGE_PAYLOAD.installationId as number) + 1 + }, TASK, false, sendMessageMock, getLogger("test"), mockError); + + const refreshedRepoSyncState = await RepoSyncState.findByPk(repoSyncState.id); + const refreshedSubscription = await Subscription.findByPk(subscription?.id); + expect(repoSyncState.get({ plain: true })).toStrictEqual(refreshedRepoSyncState?.get({ plain: true })); + expect(refreshedSubscription?.get({ plain: true })).toStrictEqual(subscription?.get({ plain: true })); + expect(sendMessageMock).toBeCalledTimes(0); + }); + + it("updates status in RepoSyncState table", async () => { + await markCurrentTaskAsFailedAndContinue(MESSAGE_PAYLOAD, TASK, false, jest.fn(), getLogger("test"), mockError); - const refreshedRepoSyncState = await RepoSyncState.findByPk(repoSyncState.id); - const refreshedSubscription = await Subscription.findByPk(subscription?.id); - expect(refreshedRepoSyncState?.branchStatus).toEqual("failed"); - expect(refreshedSubscription?.get({ plain: true })).toStrictEqual(subscription?.get({ plain: true })); + const refreshedRepoSyncState = await RepoSyncState.findByPk(repoSyncState.id); + const refreshedSubscription = await Subscription.findByPk(subscription?.id); + expect(refreshedRepoSyncState?.branchStatus).toEqual("failed"); + expect(refreshedSubscription?.get({ plain: true })).toStrictEqual(subscription?.get({ plain: true })); + }); + + it("does not update cursor in RepoSyncState table", async () => { + await markCurrentTaskAsFailedAndContinue(MESSAGE_PAYLOAD, TASK, false, jest.fn(), getLogger("test"), mockError); + + const refreshedRepoSyncState = await RepoSyncState.findByPk(repoSyncState.id); + expect(refreshedRepoSyncState?.branchCursor).toEqual(repoSyncState.branchCursor); + }); + + it("schedules next message", async () => { + const sendMessageMock = jest.fn(); + await markCurrentTaskAsFailedAndContinue(MESSAGE_PAYLOAD, TASK, false, sendMessageMock, getLogger("test"), mockError); + + expect(sendMessageMock).toBeCalledTimes(1); + }); + + it("sets up sync warning on permission error", async () => { + const sendMessageMock = jest.fn(); + await markCurrentTaskAsFailedAndContinue(MESSAGE_PAYLOAD, TASK, true, sendMessageMock, getLogger("test"), mockError); + + const refreshedSubscription = await Subscription.findByPk(subscription?.id); + expect(refreshedSubscription?.syncWarning).toEqual("Invalid permissions for branch task"); + }); }); // TODO: bgvozdev to finish off before enabling the FF for everyone @@ -401,12 +425,6 @@ describe("sync/installation", () => { } await RepoSyncState.bulkCreate(newRepoSyncStatesData); - when(numberFlag).calledWith( - NumberFlags.BACKFILL_MAX_SUBTASKS, - 0, - jiraHost - ).mockResolvedValue(100); - // That would give 2 tasks: one main and one subtask configureRateLimit(1000, 1000); }); @@ -485,28 +503,6 @@ describe("sync/installation", () => { expect(updatedRows.length).toEqual(1); }); }); - - it("does not update cursor in RepoSyncState table", async () => { - await markCurrentTaskAsFailedAndContinue(MESSAGE_PAYLOAD, TASK, false, jest.fn(), getLogger("test"), mockError); - - const refreshedRepoSyncState = await RepoSyncState.findByPk(repoSyncState.id); - expect(refreshedRepoSyncState?.branchCursor).toEqual(repoSyncState.branchCursor); - }); - - it("schedules next message", async () => { - const sendMessageMock = jest.fn(); - await markCurrentTaskAsFailedAndContinue(MESSAGE_PAYLOAD, TASK, false, sendMessageMock, getLogger("test"), mockError); - - expect(sendMessageMock).toBeCalledTimes(1); - }); - - it("sets up sync warning on permission error", async () => { - const sendMessageMock = jest.fn(); - await markCurrentTaskAsFailedAndContinue(MESSAGE_PAYLOAD, TASK, true, sendMessageMock, getLogger("test"), mockError); - - const refreshedSubscription = await Subscription.findByPk(subscription?.id); - expect(refreshedSubscription?.syncWarning).toEqual("Invalid permissions for branch task"); - }); }); describe("updateTaskStatusAndContinue", () => { diff --git a/src/sync/pull-requests.test.ts b/src/sync/pull-requests.test.ts index 37727019a0..dd561fc2c5 100644 --- a/src/sync/pull-requests.test.ts +++ b/src/sync/pull-requests.test.ts @@ -114,6 +114,7 @@ describe("sync/pull-request", () => { ])("PR Title: %p, PR Head Ref: %p", (title, head) => { it("should sync to Jira when Pull Request Nodes have jira references", async () => { + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubNock .post("/graphql") @@ -200,6 +201,7 @@ describe("sync/pull-request", () => { }); it("should not sync if nodes are empty", async () => { + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); githubNock .post("/graphql") diff --git a/src/sync/scheduler.test.ts b/src/sync/scheduler.test.ts index a1104099aa..b12aba6d5f 100644 --- a/src/sync/scheduler.test.ts +++ b/src/sync/scheduler.test.ts @@ -4,7 +4,7 @@ import { Subscription } from "models/subscription"; import { getLogger } from "config/logger"; import { RepoSyncState } from "models/reposyncstate"; import { when } from "jest-when"; -import { booleanFlag, BooleanFlags, numberFlag, NumberFlags } from "config/feature-flags"; +import { booleanFlag, BooleanFlags } from "config/feature-flags"; jest.mock("config/feature-flags"); @@ -27,8 +27,6 @@ describe("scheduler", () => { newRepoSyncStatesData.push(newRepoSyncState); } await RepoSyncState.bulkCreate(newRepoSyncStatesData); - - when(numberFlag).calledWith(NumberFlags.BACKFILL_MAX_SUBTASKS, 0, expect.anything()).mockResolvedValue(100); }); const configureRateLimit = (coreQuotaRemainig: number, graphQlQuotaRemaining: number) => { @@ -94,14 +92,6 @@ describe("scheduler", () => { expect(tasks.otherTasks.length).toEqual(100); }); - it("should only return mask task when number of subtasks is set to 0 in FF", async () => { - when(numberFlag).calledWith(NumberFlags.BACKFILL_MAX_SUBTASKS, 0, expect.anything()).mockResolvedValue(0); - - const tasks = await getNextTasks(subscription, [], getLogger("test")); - expect(tasks.mainTask).toBeDefined(); - expect(tasks.otherTasks.length).toStrictEqual(0); - }); - it("does not blow up when quota is higher than available number of tasks", async () => { configureRateLimit(100000, 100000); diff --git a/src/sync/scheduler.ts b/src/sync/scheduler.ts index 7c3e281d21..2c3c53858d 100644 --- a/src/sync/scheduler.ts +++ b/src/sync/scheduler.ts @@ -4,7 +4,7 @@ import { RepoSyncState } from "models/reposyncstate"; import { getTargetTasks } from "~/src/sync/installation"; import { createInstallationClient } from "utils/get-github-client-config"; import Logger from "bunyan"; -import { booleanFlag, BooleanFlags, numberFlag, NumberFlags } from "config/feature-flags"; +import { booleanFlag, BooleanFlags } from "config/feature-flags"; import { Op } from "sequelize"; import { without } from "lodash"; @@ -16,14 +16,10 @@ const RATE_LIMIT_QUOTA_PER_TASK_RESERVE = 500; // Coefficient to determine pool size of the selection for the subtasks const SUBTASKS_POOL_COEF = 10; +const MAX_SUBTASKS = 100; + const estimateNumberOfSubtasks = async (subscription: Subscription, logger: Logger) => { try { - const maxNumberOfSubtasks = await numberFlag(NumberFlags.BACKFILL_MAX_SUBTASKS, 0, subscription.jiraHost); - if (!maxNumberOfSubtasks) { - logger.info({ nSubTasks: 0 }, `Using subtasks: 0`); - return 0; - } - const metrics = { trigger: "ratelimit_check_backfill" }; @@ -35,7 +31,7 @@ const estimateNumberOfSubtasks = async (subscription: Subscription, logger: Logg const availQuotaForSubtasks = Math.max(0, availQuota - RATE_LIMIT_QUOTA_PER_TASK_RESERVE); const allowedSubtasks = Math.floor(availQuotaForSubtasks / RATE_LIMIT_QUOTA_PER_TASK_RESERVE); - const nSubTasks = Math.min(allowedSubtasks, maxNumberOfSubtasks); + const nSubTasks = Math.min(allowedSubtasks, MAX_SUBTASKS); logger.info({ nSubTasks, rateLimitData }, `Using subtasks: ${nSubTasks}`); return nSubTasks; diff --git a/src/sync/secret-scanning-alerts.test.ts b/src/sync/secret-scanning-alerts.test.ts index 8ef3f2eec2..7ef38d8dca 100644 --- a/src/sync/secret-scanning-alerts.test.ts +++ b/src/sync/secret-scanning-alerts.test.ts @@ -51,6 +51,7 @@ describe("sync/secret-scanning-alerts", () => { .get("/repos/integrations/test-repo-name/secret-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(200, secretScanningAlerts); githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); jiraNock .post("/rest/security/1.0/bulk", expectedResponseCloudServer(subscription)) .reply(200); @@ -68,6 +69,7 @@ describe("sync/secret-scanning-alerts", () => { }); it("should not call Jira if no secret scanning alerts are found", async () => { + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); when(booleanFlag).calledWith(BooleanFlags.ENABLE_GITHUB_SECURITY_IN_JIRA, expect.anything()).mockResolvedValue(true); const data = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; githubNock @@ -81,6 +83,7 @@ describe("sync/secret-scanning-alerts", () => { }); it("should handle secret scanning disabled error", async () => { + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); when(booleanFlag).calledWith(BooleanFlags.ENABLE_GITHUB_SECURITY_IN_JIRA, expect.anything()).mockResolvedValue(true); const data = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; githubNock @@ -94,6 +97,7 @@ describe("sync/secret-scanning-alerts", () => { }); it("should handle 404 error", async () => { + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); when(booleanFlag).calledWith(BooleanFlags.ENABLE_GITHUB_SECURITY_IN_JIRA, expect.anything()).mockResolvedValue(true); const data = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; githubNock @@ -106,6 +110,7 @@ describe("sync/secret-scanning-alerts", () => { await verifyMessageSent(data); }); it("should handle 451 error", async () => { + githubUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); when(booleanFlag).calledWith(BooleanFlags.ENABLE_GITHUB_SECURITY_IN_JIRA, expect.anything()).mockResolvedValue(true); const data = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost }; githubNock @@ -164,6 +169,7 @@ describe("sync/secret-scanning-alerts", () => { } }; gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); + gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); gheNock .get("/api/v3/repos/integrations/test-repo-name/secret-scanning/alerts?per_page=20&page=1&sort=created&direction=desc") .reply(200, secretScanningAlerts); @@ -177,6 +183,7 @@ describe("sync/secret-scanning-alerts", () => { it("should not call Jira if no secret scanning alerts are found", async () => { when(booleanFlag).calledWith(BooleanFlags.ENABLE_GITHUB_SECURITY_IN_JIRA, expect.anything()).mockResolvedValue(true); + gheUserTokenNock(DatabaseStateCreator.GITHUB_INSTALLATION_ID); const data = { installationId: DatabaseStateCreator.GITHUB_INSTALLATION_ID, jiraHost, @@ -265,4 +272,4 @@ const expectedResponseGHEServer = (subscription: Subscription) => ({ "workspaceId": subscription.id }, "operationType": "BACKFILL" -}); \ No newline at end of file +});