diff --git a/app-e2e/src/Test/E2E/GitHubIssue.purs b/app-e2e/src/Test/E2E/GitHubIssue.purs index e94585b1..bbcf791a 100644 --- a/app-e2e/src/Test/E2E/GitHubIssue.purs +++ b/app-e2e/src/Test/E2E/GitHubIssue.purs @@ -40,6 +40,19 @@ spec = do assertNoComment "Job completed successfully" requests assertOpen requests + Spec.it "links to existing logs without polling a duplicate job" do + let event = mkPublishEvent Fixtures.effectPublishData + _ <- runWorkflow event + WireMock.clearGithubRequests + + requests <- runWorkflow event + assertComment "Duplicate job" requests + assertComment "Logs:" requests + assertNoComment "Job started" requests + assertNoComment "Job completed successfully" requests + assertNoComment "Job failed" requests + assertOpen requests + Spec.it "calls Teams API to verify trustee membership for authenticated operation" do requests <- runWorkflow $ mkAuthenticatedEvent packagingTeamUser Fixtures.trusteeAuthenticatedData assertComment "Job started" requests diff --git a/app/src/App/GitHubIssue.purs b/app/src/App/GitHubIssue.purs index 516d9a5c..65fdbdd3 100644 --- a/app/src/App/GitHubIssue.purs +++ b/app/src/App/GitHubIssue.purs @@ -114,22 +114,39 @@ runGitHubIssue env = do submitResult <- Run.liftAff $ submitJob (registryApiUrl <> endpoint) jsonBody case submitResult of Left err -> Except.throw $ "Failed to submit job: " <> err - Right { jobId } -> do + Right (DuplicateJob { jobId }) -> do let jobIdStr = unwrap jobId + let logsUrl = registryApiUrl <> "/v1/jobs/" <> jobIdStr + Log.debug $ "Duplicate job: " <> jobIdStr + Run.liftAff $ void $ Octokit.request env.octokit $ Octokit.createCommentRequest + { address: Constants.registry + , issue: env.issue + , body: "Duplicate job: `" <> jobIdStr <> "`\nLogs: " <> logsUrl + } + pure false + Right (CreatedJob { jobId }) -> do + let jobIdStr = unwrap jobId + let logsUrl = registryApiUrl <> "/v1/jobs/" <> jobIdStr Log.debug $ "Job created: " <> jobIdStr -- Post initial comment with job ID Run.liftAff $ void $ Octokit.request env.octokit $ Octokit.createCommentRequest { address: Constants.registry , issue: env.issue - , body: "Job started: `" <> jobIdStr <> "`\nLogs: " <> registryApiUrl <> "/v1/jobs/" <> jobIdStr + , body: "Job started: `" <> jobIdStr <> "`\nLogs: " <> logsUrl } -- Poll for completion, posting logs as comments pollAndReport env.octokit env.issue env.pollConfig registryApiUrl jobId +-- | The V1 response body is unchanged; the HTTP status distinguishes whether +-- | the server created a job (201) or returned an existing duplicate (200). +data JobSubmissionResult + = CreatedJob V1.JobCreatedResponse + | DuplicateJob V1.JobCreatedResponse + -- | Submit a job to the registry API -submitJob :: String -> String -> Aff (Either String V1.JobCreatedResponse) +submitJob :: String -> String -> Aff (Either String JobSubmissionResult) submitJob url body = do result <- Aff.attempt $ Fetch.fetch url { method: POST @@ -140,10 +157,12 @@ submitJob url body = do Left err -> pure $ Left $ "Network error: " <> Aff.message err Right response -> do responseBody <- response.text - if response.status >= 200 && response.status < 300 then + if response.status == 200 || response.status == 201 then case JSON.parse responseBody >>= \json -> lmap CJ.DecodeError.print (CJ.decode V1.jobCreatedResponseCodec json) of Left err -> pure $ Left $ "Failed to parse response: " <> err - Right r -> pure $ Right r + Right jobResponse + | response.status == 200 -> pure $ Right $ DuplicateJob jobResponse + | otherwise -> pure $ Right $ CreatedJob jobResponse else pure $ Left $ "HTTP " <> show response.status <> ": " <> responseBody diff --git a/app/src/App/Server/Env.purs b/app/src/App/Server/Env.purs index 260c2a98..5c0ddb39 100644 --- a/app/src/App/Server/Env.purs +++ b/app/src/App/Server/Env.purs @@ -8,6 +8,7 @@ import Data.String as String import Effect.Aff as Aff import HTTPurple (JsonDecoder(..), JsonEncoder(..), Request, Response) import HTTPurple as HTTPurple +import HTTPurple.Status as Status import Node.Path as Path import Registry.API.V1 (JobId, Route) import Registry.App.API (COMPILER_CACHE, PURS_GRAPH_CACHE, _compilerCache, _pursGraphCache) @@ -134,6 +135,9 @@ jsonEncoder codec = JsonEncoder (stringifyJson codec) jsonOk :: forall m a. MonadAff m => CJ.Codec a -> a -> m Response jsonOk codec datum = HTTPurple.ok' HTTPurple.jsonHeaders $ HTTPurple.toJson (jsonEncoder codec) datum +jsonCreated :: forall m a. MonadAff m => CJ.Codec a -> a -> m Response +jsonCreated codec datum = HTTPurple.response' Status.created HTTPurple.jsonHeaders $ HTTPurple.toJson (jsonEncoder codec) datum + runEffects :: forall a. ServerEnv -> Run ServerEffects a -> Aff (Either Aff.Error a) runEffects env operation = Aff.attempt do today <- nowUTC diff --git a/app/src/App/Server/Router.purs b/app/src/App/Server/Router.purs index 9fc60bf8..090e348b 100644 --- a/app/src/App/Server/Router.purs +++ b/app/src/App/Server/Router.purs @@ -19,7 +19,7 @@ import Registry.App.Auth as Auth import Registry.App.Effect.Db as Db import Registry.App.Effect.Env as Env import Registry.App.Effect.Log as Log -import Registry.App.Server.Env (ServerEffects, ServerEnv, jsonDecoder, jsonOk, runEffects) +import Registry.App.Server.Env (ServerEffects, ServerEnv, jsonCreated, jsonDecoder, jsonOk, runEffects) import Registry.Operation (PackageSetOperation(..)) import Registry.Operation as Operation import Run (Run) @@ -80,14 +80,13 @@ router { route, method, body } = HTTPurple.usingCont case route, method of publish <- HTTPurple.fromJson (jsonDecoder Operation.publishCodec) body lift $ Log.info $ "Received Publish request: " <> printJson Operation.publishCodec publish - jobId <- lift (Db.selectPublishJob publish.name publish.version) >>= case _ of + lift (Db.selectPublishJob publish.name publish.version) >>= case _ of Just job -> do lift $ Log.warn $ "Duplicate publish job insertion, returning existing one: " <> unwrap job.jobId - pure job.jobId + jsonOk V1.jobCreatedResponseCodec { jobId: job.jobId } Nothing -> do - lift $ Db.insertPublishJob { payload: publish } - - jsonOk V1.jobCreatedResponseCodec { jobId } + jobId <- lift $ Db.insertPublishJob { payload: publish } + jsonCreated V1.jobCreatedResponseCodec { jobId } Unpublish, Post -> do auth <- HTTPurple.fromJson (jsonDecoder Operation.authenticatedCodec) body @@ -95,18 +94,17 @@ router { route, method, body } = HTTPurple.usingCont case route, method of Operation.Unpublish payload -> do lift $ Log.info $ "Received Unpublish request: " <> printJson Operation.unpublishCodec payload - jobId <- lift (Db.selectUnpublishJob payload.name payload.version) >>= case _ of + lift (Db.selectUnpublishJob payload.name payload.version) >>= case _ of Just job -> do lift $ Log.warn $ "Duplicate unpublish job insertion, returning existing one: " <> unwrap job.jobId - pure job.jobId + jsonOk V1.jobCreatedResponseCodec { jobId: job.jobId } Nothing -> do - lift $ Db.insertUnpublishJob + jobId <- lift $ Db.insertUnpublishJob { payload: payload , rawPayload: auth.rawPayload , signature: auth.signature } - - jsonOk V1.jobCreatedResponseCodec { jobId } + jsonCreated V1.jobCreatedResponseCodec { jobId } _ -> HTTPurple.badRequest "Expected unpublish operation." @@ -116,18 +114,17 @@ router { route, method, body } = HTTPurple.usingCont case route, method of Operation.Transfer payload -> do lift $ Log.info $ "Received Transfer request: " <> printJson Operation.transferCodec payload - jobId <- lift (Db.selectTransferJob payload.name) >>= case _ of + lift (Db.selectTransferJob payload.name) >>= case _ of Just job -> do lift $ Log.warn $ "Duplicate transfer job insertion, returning existing one: " <> unwrap job.jobId - pure job.jobId + jsonOk V1.jobCreatedResponseCodec { jobId: job.jobId } Nothing -> do - lift $ Db.insertTransferJob + jobId <- lift $ Db.insertTransferJob { payload: payload , rawPayload: auth.rawPayload , signature: auth.signature } - - jsonOk V1.jobCreatedResponseCodec { jobId } + jsonCreated V1.jobCreatedResponseCodec { jobId } _ -> HTTPurple.badRequest "Expected transfer operation." @@ -190,18 +187,17 @@ router { route, method, body } = HTTPurple.usingCont case route, method of lift $ Log.info "Package set authentication successful." -- Check for duplicate pending job with the same payload - jobId <- lift (Db.selectPackageSetJobByPayload request.payload) >>= case _ of + lift (Db.selectPackageSetJobByPayload request.payload) >>= case _ of Just job -> do lift $ Log.warn $ "Duplicate package set job insertion, returning existing one: " <> unwrap job.jobId - pure job.jobId + jsonOk V1.jobCreatedResponseCodec { jobId: job.jobId } Nothing -> do - lift $ Db.insertPackageSetJob + jobId <- lift $ Db.insertPackageSetJob { payload: request.payload , rawPayload: request.rawPayload , signature: request.signature } - - jsonOk V1.jobCreatedResponseCodec { jobId } + jsonCreated V1.jobCreatedResponseCodec { jobId } Status, Get -> HTTPurple.emptyResponse Status.ok