From 0994a2aec3836d84c211328f4ca4d3081c3447c1 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 30 Aug 2022 17:54:09 -0700 Subject: [PATCH 01/26] add seurat as sample tech --- src/specs/models/samples-bodies/CreateSample.v2.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/specs/models/samples-bodies/CreateSample.v2.yaml b/src/specs/models/samples-bodies/CreateSample.v2.yaml index 2953db0c8..b91548205 100644 --- a/src/specs/models/samples-bodies/CreateSample.v2.yaml +++ b/src/specs/models/samples-bodies/CreateSample.v2.yaml @@ -10,6 +10,7 @@ properties: - oneOf: - pattern: 10x - pattern: rhapsody + - pattern: seurat required: - name From c23e64e2047a70457964afd53dcb6e4bea996586 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 30 Aug 2022 18:47:12 -0700 Subject: [PATCH 02/26] init adding seurat enum --- .../20220830130411_add_seurat_enum.js | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 src/sql/migrations/20220830130411_add_seurat_enum.js diff --git a/src/sql/migrations/20220830130411_add_seurat_enum.js b/src/sql/migrations/20220830130411_add_seurat_enum.js new file mode 100644 index 000000000..1248175e1 --- /dev/null +++ b/src/sql/migrations/20220830130411_add_seurat_enum.js @@ -0,0 +1,22 @@ +exports.up = async (knex) => { + await knex.raw(` + CREATE TYPE sample_technology_temp AS ENUM ('10x','rhapsody','seurat'); + ALTER TABLE sample + ALTER COLUMN sample_technology DROP DEFAULT, + ALTER COLUMN sample_technology TYPE sample_technology_temp USING sample_technology::text::sample_technology_temp; + DROP TYPE IF EXISTS sample_technology; + ALTER TYPE sample_technology_temp RENAME TO sample_technology; + `); +}; + +exports.down = async (knex) => { + await knex.raw(` + CREATE TYPE sample_technology_temp AS ENUM ('10x','rhapsody'); + ALTER TABLE sample + ALTER COLUMN sample_technology DROP DEFAULT, + ALTER COLUMN sample_technology TYPE sample_technology_temp USING sample_technology::text::sample_technology_temp; + DROP TYPE IF EXISTS sample_technology; + ALTER TYPE sample_technology_temp RENAME TO sample_technology; + `); +}; + From 5034dd5de241b03d02468c55e10c64c54b446ab4 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 6 Sep 2022 10:59:08 -0700 Subject: [PATCH 03/26] add sample_file_type to migration --- .../migrations/20220830130411_add_seurat_enum.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/sql/migrations/20220830130411_add_seurat_enum.js b/src/sql/migrations/20220830130411_add_seurat_enum.js index 1248175e1..46ff28bb0 100644 --- a/src/sql/migrations/20220830130411_add_seurat_enum.js +++ b/src/sql/migrations/20220830130411_add_seurat_enum.js @@ -6,6 +6,14 @@ exports.up = async (knex) => { ALTER COLUMN sample_technology TYPE sample_technology_temp USING sample_technology::text::sample_technology_temp; DROP TYPE IF EXISTS sample_technology; ALTER TYPE sample_technology_temp RENAME TO sample_technology; + + + CREATE TYPE sample_file_type_temp AS ENUM ('features10x','barcodes10x','matrix10x', 'rhapsody', 'seurat'); + ALTER TABLE sample_file + ALTER COLUMN sample_file_type DROP DEFAULT, + ALTER COLUMN sample_file_type TYPE sample_file_type_temp USING sample_file_type::text::sample_file_type_temp; + DROP TYPE IF EXISTS sample_file_type; + ALTER TYPE sample_file_type_temp RENAME TO sample_file_type; `); }; @@ -17,6 +25,13 @@ exports.down = async (knex) => { ALTER COLUMN sample_technology TYPE sample_technology_temp USING sample_technology::text::sample_technology_temp; DROP TYPE IF EXISTS sample_technology; ALTER TYPE sample_technology_temp RENAME TO sample_technology; + + CREATE TYPE sample_file_type_temp AS ENUM ('features10x','barcodes10x','matrix10x', 'rhapsody'); + ALTER TABLE sample_file + ALTER COLUMN sample_file_type DROP DEFAULT, + ALTER COLUMN sample_file_type TYPE sample_file_type_temp USING sample_file_type::text::sample_file_type_temp; + DROP TYPE IF EXISTS sample_file_type; + ALTER TYPE sample_file_type_temp RENAME TO sample_file_type; `); }; From b492073cdf02e320fa3b6095e2340f07ae6e538a Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 6 Sep 2022 11:00:24 -0700 Subject: [PATCH 04/26] remove double blank line --- src/sql/migrations/20220830130411_add_seurat_enum.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sql/migrations/20220830130411_add_seurat_enum.js b/src/sql/migrations/20220830130411_add_seurat_enum.js index 46ff28bb0..c31b5eab4 100644 --- a/src/sql/migrations/20220830130411_add_seurat_enum.js +++ b/src/sql/migrations/20220830130411_add_seurat_enum.js @@ -7,7 +7,6 @@ exports.up = async (knex) => { DROP TYPE IF EXISTS sample_technology; ALTER TYPE sample_technology_temp RENAME TO sample_technology; - CREATE TYPE sample_file_type_temp AS ENUM ('features10x','barcodes10x','matrix10x', 'rhapsody', 'seurat'); ALTER TABLE sample_file ALTER COLUMN sample_file_type DROP DEFAULT, From 36715e791ec08bbcd695c4e1154eb8804e8892f0 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Wed, 7 Sep 2022 14:29:25 -0700 Subject: [PATCH 05/26] init seurat pipeline boilerplate --- src/api.v2/constants.js | 2 + src/api.v2/controllers/seuratController.js | 60 +++++++ .../pipeline/pipelineConstruct/index.js | 42 ++++- .../pipelineConstruct/skeletons/index.js | 17 ++ .../skeletons/seuratPipelineSkeleton.js | 29 ++++ src/api.v2/helpers/pipeline/seurat.js | 155 ++++++++++++++++++ src/api.v2/routes/seurat.js | 13 ++ src/specs/api.v2.yaml | 93 +++++++++++ .../SEURATResponse.v2.yaml | 32 ++++ 9 files changed, 441 insertions(+), 2 deletions(-) create mode 100644 src/api.v2/controllers/seuratController.js create mode 100644 src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/seuratPipelineSkeleton.js create mode 100644 src/api.v2/helpers/pipeline/seurat.js create mode 100644 src/api.v2/routes/seurat.js create mode 100644 src/specs/models/work-request-bodies/SEURATResponse.v2.yaml diff --git a/src/api.v2/constants.js b/src/api.v2/constants.js index caa1f5881..3eda1605a 100644 --- a/src/api.v2/constants.js +++ b/src/api.v2/constants.js @@ -1,6 +1,7 @@ // Pipeline names const QC_PROCESS_NAME = 'qc'; const GEM2S_PROCESS_NAME = 'gem2s'; +const SEURAT_PROCESS_NAME = 'seurat'; const OLD_QC_NAME_TO_BE_REMOVED = 'pipeline'; const ASSIGN_POD_TO_PIPELINE = 'assignPodToPipeline'; @@ -41,6 +42,7 @@ const ADMIN_SUB = { module.exports = { QC_PROCESS_NAME, GEM2S_PROCESS_NAME, + SEURAT_PROCESS_NAME, OLD_QC_NAME_TO_BE_REMOVED, RUNNING, FAILED, diff --git a/src/api.v2/controllers/seuratController.js b/src/api.v2/controllers/seuratController.js new file mode 100644 index 000000000..3f8c371a0 --- /dev/null +++ b/src/api.v2/controllers/seuratController.js @@ -0,0 +1,60 @@ +const AWSXRay = require('aws-xray-sdk'); + +const { createSeuratPipeline, handleSeuratResponse } = require('../helpers/pipeline/seurat'); +const { OK } = require('../../utils/responses'); +const getLogger = require('../../utils/getLogger'); +const parseSNSMessage = require('../../utils/parse-sns-message'); + +const logger = getLogger('[SeuratController] - '); + +const runSeurat = async (req, res) => { + const { experimentId } = req.params; + + logger.log(`Starting seurat for experiment ${experimentId}`); + + const newExecution = await + createSeuratPipeline(experimentId, req.body, req.headers.authorization); + + logger.log(`Started seurat for experiment ${experimentId} successfully, `); + logger.log('New executions data:'); + logger.log(JSON.stringify(newExecution)); + + res.json(OK()); +}; + +const handleResponse = async (req, res) => { + let result; + + try { + result = await parseSNSMessage(req); + } catch (e) { + logger.error('Parsing initial SNS message failed:', e); + AWSXRay.getSegment().addError(e); + res.status(200).send('nok'); + return; + } + + const { io, parsedMessage } = result; + + const isSnsNotification = parsedMessage !== undefined; + if (isSnsNotification) { + try { + await handleSeuratResponse(io, parsedMessage); + } catch (e) { + logger.error( + 'seurat pipeline response handler failed with error: ', e, + ); + + AWSXRay.getSegment().addError(e); + res.status(200).send('nok'); + return; + } + } + + res.status(200).send('ok'); +}; + +module.exports = { + runSeurat, + handleResponse, +}; diff --git a/src/api.v2/helpers/pipeline/pipelineConstruct/index.js b/src/api.v2/helpers/pipeline/pipelineConstruct/index.js index c84c7d5a0..b1744a903 100644 --- a/src/api.v2/helpers/pipeline/pipelineConstruct/index.js +++ b/src/api.v2/helpers/pipeline/pipelineConstruct/index.js @@ -8,7 +8,7 @@ const { v4: uuidv4 } = require('uuid'); const util = require('util'); const config = require('../../../../config'); -const { QC_PROCESS_NAME, GEM2S_PROCESS_NAME } = require('../../../constants'); +const { QC_PROCESS_NAME, GEM2S_PROCESS_NAME, SEURAT_PROCESS_NAME } = require('../../../constants'); const Experiment = require('../../../model/Experiment'); const ExperimentExecution = require('../../../model/ExperimentExecution'); @@ -18,7 +18,7 @@ const getLogger = require('../../../../utils/getLogger'); const asyncTimer = require('../../../../utils/asyncTimer'); const constructPipelineStep = require('./constructors/constructPipelineStep'); -const { getGem2sPipelineSkeleton, getQcPipelineSkeleton } = require('./skeletons'); +const { getGem2sPipelineSkeleton, getQcPipelineSkeleton, getSeuratPipelineSkeleton } = require('./skeletons'); const { getQcStepsToRun } = require('./qcHelpers'); const logger = getLogger(); @@ -313,10 +313,48 @@ const createGem2SPipeline = async (experimentId, taskParams) => { return { stateMachineArn, executionArn }; }; +const createSeuratObjectPipeline = async (experimentId, taskParams) => { + const accountId = config.awsAccountId; + const roleArn = `arn:aws:iam::${accountId}:role/state-machine-role-${config.clusterEnv}`; + + const context = { + taskParams, + experimentId, + accountId, + roleArn, + processName: SEURAT_PROCESS_NAME, + activityArn: `arn:aws:states:${config.awsRegion}:${accountId}:activity:pipeline-${config.clusterEnv}-${uuidv4()}`, + pipelineArtifacts: await getPipelineArtifacts(), + clusterInfo: await getClusterInfo(), + sandboxId: config.sandboxId, + processingConfig: {}, + environment: config.clusterEnv, + }; + + const seuratPipelineSkeleton = getSeuratPipelineSkeleton(config.clusterEnv); + logger.log('Skeleton constructed, now building state machine definition...'); + + const stateMachine = buildStateMachineDefinition(seuratPipelineSkeleton, context); + logger.log('State machine definition built, now creating activity if not already present...'); + + const activityArn = await createActivity(context); + logger.log(`Activity with ARN ${activityArn} created, now creating state machine from skeleton...`); + + const stateMachineArn = await createNewStateMachine(context, stateMachine, SEURAT_PROCESS_NAME); + logger.log(`State machine with ARN ${stateMachineArn} created, launching it...`); + logger.log('Context:', util.inspect(context, { showHidden: false, depth: null, colors: false })); + logger.log('State machine:', util.inspect(stateMachine, { showHidden: false, depth: null, colors: false })); + + const executionArn = await executeStateMachine(stateMachineArn); + logger.log(`Execution with ARN ${executionArn} created.`); + + return { stateMachineArn, executionArn }; +}; module.exports = { createQCPipeline, createGem2SPipeline, + createSeuratObjectPipeline, buildStateMachineDefinition, }; diff --git a/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/index.js b/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/index.js index 45b021757..b90fd36d8 100644 --- a/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/index.js +++ b/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/index.js @@ -1,5 +1,6 @@ const { buildQCPipelineSteps, qcPipelineSteps } = require('./qcPipelineSkeleton'); const { gem2SPipelineSteps } = require('./gem2sPipelineSkeleton'); +const { seuratPipelineSteps } = require('./seuratPipelineSkeleton'); const createLocalPipeline = (nextStep) => ({ @@ -53,6 +54,11 @@ const getPipelineStepNames = () => { // if there are map states with nested substeps it returns those sub-steps too const getQcPipelineStepNames = () => getSkeletonStepNames(qcPipelineSteps); + +// getSeuratStepNames returns the names of the seurat pipeline steps +// if there are map states with nested substeps it returns those sub-steps too +const getSeuratPipelineStepNames = () => getSkeletonStepNames(seuratPipelineSteps); + const buildInitialSteps = (clusterEnv, nextStep) => { // if we are running locally launch a pipeline job if (clusterEnv === 'development') { @@ -80,6 +86,15 @@ const getGem2sPipelineSkeleton = (clusterEnv) => ({ }, }); +const getSeuratPipelineSkeleton = (clusterEnv) => ({ + Comment: `Seurat Pipeline for clusterEnv '${clusterEnv}'`, + StartAt: getStateMachineFirstStep(clusterEnv), + States: { + ...buildInitialSteps(clusterEnv, 'DownloadSeurat'), + ...seuratPipelineSteps, + }, +}); + const getQcPipelineSkeleton = (clusterEnv, qcSteps) => ({ Comment: `QC Pipeline for clusterEnv '${clusterEnv}'`, StartAt: getStateMachineFirstStep(clusterEnv), @@ -93,6 +108,8 @@ const getQcPipelineSkeleton = (clusterEnv, qcSteps) => ({ module.exports = { getPipelineStepNames, getQcPipelineStepNames, + getSeuratPipelineStepNames, getGem2sPipelineSkeleton, getQcPipelineSkeleton, + getSeuratPipelineSkeleton, }; diff --git a/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/seuratPipelineSkeleton.js b/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/seuratPipelineSkeleton.js new file mode 100644 index 000000000..624d33d61 --- /dev/null +++ b/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/seuratPipelineSkeleton.js @@ -0,0 +1,29 @@ +const seuratPipelineSteps = { + DownloadSeurat: { + XStepType: 'create-new-step', + XConstructorArgs: { + taskName: 'downloadSeurat', + }, + Next: 'PreProcessing', + }, + PreProcessing: { + XStepType: 'create-new-step', + XConstructorArgs: { + taskName: 'preprocSeurat', + }, + Next: 'UploadToAWS', + }, + UploadToAWS: { + XStepType: 'create-new-step', + XConstructorArgs: { + taskName: 'uploadToAWS', + }, + Next: 'EndOfGem2S', + }, + EndOfGem2S: { + Type: 'Pass', + End: true, + }, +}; + +module.exports = { seuratPipelineSteps }; diff --git a/src/api.v2/helpers/pipeline/seurat.js b/src/api.v2/helpers/pipeline/seurat.js new file mode 100644 index 000000000..8dc0cb164 --- /dev/null +++ b/src/api.v2/helpers/pipeline/seurat.js @@ -0,0 +1,155 @@ +const _ = require('lodash'); +const AWSXRay = require('aws-xray-sdk'); + +const constants = require('../../constants'); +const getPipelineStatus = require('./getPipelineStatus'); +const { createSeuratObjectPipeline } = require('./pipelineConstruct'); + +const Sample = require('../../model/Sample'); +const Experiment = require('../../model/Experiment'); +const ExperimentExecution = require('../../model/ExperimentExecution'); + +const HookRunner = require('./hooks/HookRunner'); + +const validateRequest = require('../../../utils/schema-validator'); +const getLogger = require('../../../utils/getLogger'); + +const logger = getLogger('[SeuratService] - '); + +const hookRunner = new HookRunner(); + + +const sendUpdateToSubscribed = async (experimentId, message, io) => { + const statusRes = await getPipelineStatus(experimentId, constants.SEURAT_PROCESS_NAME); + + // Concatenate into a proper response. + const response = { + ...message, + status: statusRes, + type: constants.SEURAT_PROCESS_NAME, + }; + + const { error = null } = message.response || {}; + if (error) { + logger.log(`Error in ${constants.SEURAT_PROCESS_NAME} received`); + AWSXRay.getSegment().addError(error); + } + + logger.log('Sending to all clients subscribed to experiment', experimentId); + + io.sockets.emit(`ExperimentUpdates-${experimentId}`, response); +}; + +const generateSeuratParams = async (experimentId, authJWT) => { + const defaultMetadataValue = 'N.A.'; + + logger.log('Generating seurat params'); + + const getS3Paths = (files) => ( + { + seurat: files.seurat.s3Path, + } + ); + + const [experiment, samples] = await Promise.all([ + new Experiment().findById(experimentId).first(), + new Sample().getSamples(experimentId), + ]); + + const samplesInOrder = experiment.samplesOrder.map( + (sampleId) => _.find(samples, { id: sampleId }), + ); + + const s3Paths = {}; + experiment.samplesOrder.forEach((sampleId) => { + const { files } = _.find(samples, { id: sampleId }); + + s3Paths[sampleId] = getS3Paths(files); + }); + + const taskParams = { + projectId: experimentId, + experimentName: experiment.name, + organism: null, + input: { type: samples[0].sampleTechnology }, + sampleIds: experiment.samplesOrder, + sampleNames: _.map(samplesInOrder, 'name'), + sampleS3Paths: s3Paths, + authJWT, + }; + + const metadataKeys = Object.keys(samples[0].metadata); + + if (metadataKeys.length) { + logger.log('Adding metadatakeys to task params'); + + taskParams.metadata = metadataKeys.reduce((acc, key) => { + // Make sure the key does not contain '-' as it will cause failure in SEURAT + const sanitizedKey = key.replace(/-+/g, '_'); + + acc[sanitizedKey] = Object.values(samplesInOrder).map( + (sampleValue) => sampleValue.metadata[key] || defaultMetadataValue, + ); + + return acc; + }, {}); + } + + logger.log('Task params generated'); + + return taskParams; +}; + +const createSeuratPipeline = async (experimentId, body, authJWT) => { + logger.log('Creating SEURAT params...'); + const { paramsHash } = body; + + const taskParams = await generateSeuratParams(experimentId, authJWT); + + const { stateMachineArn, executionArn } = await + createSeuratObjectPipeline(experimentId, taskParams); + + logger.log('SEURAT params created.'); + + const newExecution = { + params_hash: paramsHash, + state_machine_arn: stateMachineArn, + execution_arn: executionArn, + }; + + await new ExperimentExecution().upsert( + { + experiment_id: experimentId, + pipeline_type: 'seurat', + }, + newExecution, + ); + + logger.log('SEURAT params saved.'); + + return newExecution; +}; + +const handleSeuratResponse = async (io, message) => { + AWSXRay.getSegment().addMetadata('message', message); + + // Fail hard if there was an error. + await validateRequest(message, 'SEURATResponse.v2.yaml'); + + await hookRunner.run(message); + + const { experimentId } = message; + + const messageForClient = _.cloneDeep(message); + + // Make sure authJWT doesn't get back to the client + delete messageForClient.authJWT; + delete messageForClient.input.authJWT; + + await sendUpdateToSubscribed(experimentId, messageForClient, io); +}; + +module.exports = { + createSeuratPipeline, + handleSeuratResponse, +}; diff --git a/src/api.v2/routes/seurat.js b/src/api.v2/routes/seurat.js new file mode 100644 index 000000000..1079453ec --- /dev/null +++ b/src/api.v2/routes/seurat.js @@ -0,0 +1,13 @@ +const { runSeurat, handleResponse } = require('../controllers/seuratController'); + +const { expressAuthorizationMiddleware } = require('../middlewares/authMiddlewares'); + +module.exports = { + 'seurat#run': [ + expressAuthorizationMiddleware, + (req, res, next) => runSeurat(req, res).catch(next), + ], + 'seurat#response': (req, res, next) => { + handleResponse(req, res).catch(next); + }, +}; diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 328cabbe2..9884e8453 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1679,6 +1679,62 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPError' + '/experiments/{experimentId}/seurat': + post: + summary: Run seurat + operationId: runSeurat + x-eov-operation-id: seurat#run + x-eov-operation-handler: routes/seurat + description: Triggers a new seurat run + parameters: + - name: experimentId + description: Id of the experiment + schema: + type: string + in: path + required: true + requestBody: + content: + application/json: + schema: + type: object + properties: + paramsHash: + type: string + required: + - paramsHash + additionalProperties: false + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPSuccess' + '400': + description: Bad Request + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '401': + description: The request lacks authentication credentials. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '403': + description: The authenticated user is not authorized to view this resource. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '404': + description: Experiment not found + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' '/experiments/{experimentId}/plots/{plotUuid}': parameters: - schema: @@ -1850,6 +1906,43 @@ paths: type: string pattern: nok parameters: [] + /seuratResults: + post: + summary: Retrieve results from pipeline step functions + description: |- + Results from Step Function pipeline steps are relayed to the API through this endpoint. + Note that this endpoint is only exposed to AWS SNS, and since it has a specific communication protocol with limited feedback, the schema defined here is designed to be liberally enforceable. This endpoint is also used by SNS to handle subscribe/unsubscribe events. + The actual JSON passed by SNS is found in the `PipelineResponse` model, which is to be validated by the API. + operationId: receiveSeuratResponse + x-eov-operation-id: seurat#response + x-eov-operation-handler: routes/seurat + requestBody: + description: 'The results from the execution of a pipeline step, sent via SNS.' + required: true + content: + text/plain: + schema: + type: string + application/json: + schema: + type: object + properties: {} + responses: + '200': + description: 'A JSON-parseable was received by the server, *irrespective of whether it was correct/acceptable or not*.' + content: + text/plain: + schema: + type: string + pattern: ok + '500': + description: The data sent by the server could not be parsed as JSON. + content: + text/plain: + schema: + type: string + pattern: nok + parameters: [] /pipelineResults: post: summary: Placeholder endpoint for qc, necessary so subscriptions to sns work diff --git a/src/specs/models/work-request-bodies/SEURATResponse.v2.yaml b/src/specs/models/work-request-bodies/SEURATResponse.v2.yaml new file mode 100644 index 000000000..7858e54c7 --- /dev/null +++ b/src/specs/models/work-request-bodies/SEURATResponse.v2.yaml @@ -0,0 +1,32 @@ +title: SEURAT response +description: This is the format the seurat clients communicate the result of a seurat run. +properties: + experimentId: + type: string + description: The ID of the experiment Seurat was called against. + taskName: + type: string + description: The name of the task that was excecuted + authJWT: + type: string + description: A Bearer-style HTTP authentication token passed by the user that ran Seurat. + processingConfig: + $ref: './ProcessingConfig.v2.yaml#/properties' + meta: + type: object + table: + type: string + response: + type: object + required: + - error + description: Object storing non-typical response information. + properties: + error: + oneOf: + - type: string + - type: boolean + description: Whether or not an error occurred or a message describing the error. +required: + - experimentId +type: object \ No newline at end of file From 977e85782c8fab2b28b45b134d871c9c80df97a3 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Wed, 7 Sep 2022 14:37:42 -0700 Subject: [PATCH 06/26] add seurat backend status --- .../helpers/backendStatus/getExperimentBackendStatus.js | 4 +++- src/api.v2/helpers/pipeline/__mocks__/getBackendStatus.js | 6 ++++++ src/api.v2/helpers/pipeline/getPipelineStatus.js | 8 ++++++++ src/specs/api.v2.yaml | 2 +- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/api.v2/helpers/backendStatus/getExperimentBackendStatus.js b/src/api.v2/helpers/backendStatus/getExperimentBackendStatus.js index c5e697179..d7b69fc46 100644 --- a/src/api.v2/helpers/backendStatus/getExperimentBackendStatus.js +++ b/src/api.v2/helpers/backendStatus/getExperimentBackendStatus.js @@ -3,10 +3,11 @@ const getPipelineStatus = require('../pipeline/getPipelineStatus'); const getWorkerStatus = require('../worker/getWorkerStatus'); const getExperimentBackendStatus = async (experimentId) => { - const [{ gem2s }, { qc }, { worker }] = await Promise.all( + const [{ gem2s }, { qc }, { seurat }, { worker }] = await Promise.all( [ getPipelineStatus(experimentId, constants.GEM2S_PROCESS_NAME), getPipelineStatus(experimentId, constants.QC_PROCESS_NAME), + getPipelineStatus(experimentId, constants.SEURAT_PROCESS_NAME), getWorkerStatus(experimentId), ], ); @@ -14,6 +15,7 @@ const getExperimentBackendStatus = async (experimentId) => { const formattedResponse = { [constants.OLD_QC_NAME_TO_BE_REMOVED]: qc, gem2s, + seurat, worker, }; diff --git a/src/api.v2/helpers/pipeline/__mocks__/getBackendStatus.js b/src/api.v2/helpers/pipeline/__mocks__/getBackendStatus.js index 850dc76c6..dd4fd3501 100644 --- a/src/api.v2/helpers/pipeline/__mocks__/getBackendStatus.js +++ b/src/api.v2/helpers/pipeline/__mocks__/getBackendStatus.js @@ -13,6 +13,12 @@ const response = { status: pipelineConstants.NOT_CREATED, stopDate: null, }, + seurat: { + completedSteps: [], + startDate: null, + status: pipelineConstants.NOT_CREATED, + stopDate: null, + }, worker: { ready: true, restartCount: 0, diff --git a/src/api.v2/helpers/pipeline/getPipelineStatus.js b/src/api.v2/helpers/pipeline/getPipelineStatus.js index 7653ad0da..9d21c6b0f 100644 --- a/src/api.v2/helpers/pipeline/getPipelineStatus.js +++ b/src/api.v2/helpers/pipeline/getPipelineStatus.js @@ -30,6 +30,11 @@ const gem2sPipelineSteps = [ 'PrepareExperiment', 'UploadToAWS']; +const seuratPipelineSteps = [ + 'DownloadSeurat', + 'PreProcessing', + 'UploadToAWS']; + // pipelineStepNames are the names of pipeline steps for which we // want to report the progress back to the user // does not include steps used to initialize the infrastructure (like pod deletion assignation) @@ -77,6 +82,9 @@ const buildCompletedStatus = (processName, date, paramsHash) => { case pipelineConstants.GEM2S_PROCESS_NAME: completedSteps = gem2sPipelineSteps; break; + case pipelineConstants.SEURAT_PROCESS_NAME: + completedSteps = seuratPipelineSteps; + break; case pipelineConstants.QC_PROCESS_NAME: completedSteps = qcPipelineSteps; break; diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 9884e8453..963157d87 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -540,7 +540,7 @@ paths: '/experiments/{experimentId}/backendStatus': get: summary: Get backend status for an experiment - description: Get the status fo qc, gem2s and the worker for an experiment + description: Get the status fo qc, gem2s, seurat, and the worker for an experiment operationId: getBackendStatus x-eov-operation-id: experiment#getBackendStatus x-eov-operation-handler: routes/experiment From 9b55959a0461d8fdfe8a560f491b0942bb65a7d2 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Wed, 7 Sep 2022 15:06:42 -0700 Subject: [PATCH 07/26] add seurat as pipeline_type --- .../migrations/20220830130411_add_seurat_enum.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/sql/migrations/20220830130411_add_seurat_enum.js b/src/sql/migrations/20220830130411_add_seurat_enum.js index c31b5eab4..fe4bcaa5d 100644 --- a/src/sql/migrations/20220830130411_add_seurat_enum.js +++ b/src/sql/migrations/20220830130411_add_seurat_enum.js @@ -13,6 +13,13 @@ exports.up = async (knex) => { ALTER COLUMN sample_file_type TYPE sample_file_type_temp USING sample_file_type::text::sample_file_type_temp; DROP TYPE IF EXISTS sample_file_type; ALTER TYPE sample_file_type_temp RENAME TO sample_file_type; + + CREATE TYPE pipeline_type_temp AS ENUM ('qc','gem2s','seurat'); + ALTER TABLE experiment_execution + ALTER COLUMN pipeline_type DROP DEFAULT, + ALTER COLUMN pipeline_type TYPE pipeline_type_temp USING pipeline_type::text::pipeline_type_temp; + DROP TYPE IF EXISTS pipeline_type; + ALTER TYPE pipeline_type_temp RENAME TO pipeline_type; `); }; @@ -31,6 +38,13 @@ exports.down = async (knex) => { ALTER COLUMN sample_file_type TYPE sample_file_type_temp USING sample_file_type::text::sample_file_type_temp; DROP TYPE IF EXISTS sample_file_type; ALTER TYPE sample_file_type_temp RENAME TO sample_file_type; + + CREATE TYPE pipeline_type_temp AS ENUM ('qc','gem2s'); + ALTER TABLE experiment_execution + ALTER COLUMN pipeline_type DROP DEFAULT, + ALTER COLUMN pipeline_type TYPE pipeline_type_temp USING pipeline_type::text::pipeline_type_temp; + DROP TYPE IF EXISTS pipeline_type; + ALTER TYPE pipeline_type_temp RENAME TO pipeline_type; `); }; From c615940df26fda0a91be46484648aab983fd7d47 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Wed, 7 Sep 2022 15:33:39 -0700 Subject: [PATCH 08/26] add seurat to pipeline status; seurat pipeline is created --- src/api.v2/helpers/pipeline/getPipelineStatus.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/api.v2/helpers/pipeline/getPipelineStatus.js b/src/api.v2/helpers/pipeline/getPipelineStatus.js index 9d21c6b0f..afe2eca7d 100644 --- a/src/api.v2/helpers/pipeline/getPipelineStatus.js +++ b/src/api.v2/helpers/pipeline/getPipelineStatus.js @@ -293,6 +293,9 @@ const getPipelineStatus = async (experimentId, processName) => { case pipelineConstants.GEM2S_PROCESS_NAME: completedSteps = gem2sPipelineSteps.slice(0, gem2sPipelineSteps.indexOf(lastExecuted) + 1); break; + case pipelineConstants.SEURAT_PROCESS_NAME: + completedSteps = seuratPipelineSteps.slice(0, seuratPipelineSteps.indexOf(lastExecuted) + 1); + break; default: logger.error(`unknown process name ${processName}`); } From 93ac8761017fe1cac50ee1669868fd903854e1a9 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Wed, 7 Sep 2022 15:41:00 -0700 Subject: [PATCH 09/26] move SEURATResponse yaml --- src/specs/models/{work-request-bodies => }/SEURATResponse.v2.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/specs/models/{work-request-bodies => }/SEURATResponse.v2.yaml (100%) diff --git a/src/specs/models/work-request-bodies/SEURATResponse.v2.yaml b/src/specs/models/SEURATResponse.v2.yaml similarity index 100% rename from src/specs/models/work-request-bodies/SEURATResponse.v2.yaml rename to src/specs/models/SEURATResponse.v2.yaml From 0fe05757cc62f684f462f2ab7836ff5483bb8f04 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 13 Sep 2022 17:39:27 -0700 Subject: [PATCH 10/26] seurat pipeline sends updates; embedding work request adapted for seurat --- src/api.v2/events/validateAndSubmitWork.js | 11 +++++++++-- .../helpers/pipeline/getPipelineStatus.js | 6 +++--- .../constructors/createNewStep.js | 12 +++++++++++- .../helpers/pipeline/pipelineConstruct/index.js | 1 + .../pipelineConstruct/skeletons/index.js | 9 ++------- .../skeletons/seuratPipelineSkeleton.js | 16 ++++++++-------- src/api.v2/helpers/pipeline/seurat.js | 17 ++++++++++++++++- src/specs/api.v2.yaml | 5 +++++ src/specs/models/PipelinePodRequest.v2.yaml | 2 +- src/specs/models/ProcessingConfigBodies.v2.yaml | 3 ++- ...TResponse.v2.yaml => SeuratResponse.v2.yaml} | 2 +- .../experiment-bodies/ExperimentInfo.v2.yaml | 16 ++++++++++++++++ .../ProcessingConfigConfigureEmbedding.v2.yaml | 2 ++ .../WorkRequestGetEmbedding.v2.yaml | 2 ++ 14 files changed, 79 insertions(+), 25 deletions(-) rename src/specs/models/{SEURATResponse.v2.yaml => SeuratResponse.v2.yaml} (97%) diff --git a/src/api.v2/events/validateAndSubmitWork.js b/src/api.v2/events/validateAndSubmitWork.js index ec46eb885..329ff9025 100644 --- a/src/api.v2/events/validateAndSubmitWork.js +++ b/src/api.v2/events/validateAndSubmitWork.js @@ -5,6 +5,8 @@ const getPipelineStatus = require('../helpers/pipeline/getPipelineStatus'); const pipelineConstants = require('../constants'); +const checkSomeEqualTo = (array, testValue) => array.some((item) => item === testValue); + const validateAndSubmitWork = async (workRequest) => { const { experimentId } = workRequest; @@ -13,8 +15,13 @@ const validateAndSubmitWork = async (workRequest) => { experimentId, pipelineConstants.QC_PROCESS_NAME, ); - if (qcPipelineStatus !== pipelineConstants.SUCCEEDED) { - const e = new Error(`Work request can not be handled because pipeline is ${qcPipelineStatus}`); + + const { seurat: { status: seuratPipelineStatus } } = await getPipelineStatus( + experimentId, pipelineConstants.SEURAT_PROCESS_NAME, + ); + + if (!checkSomeEqualTo([qcPipelineStatus, seuratPipelineStatus], pipelineConstants.SUCCEEDED)) { + const e = new Error(`Work request can not be handled because pipeline is ${qcPipelineStatus} or seurat status is ${seuratPipelineStatus}`); AWSXRay.getSegment().addError(e); throw e; diff --git a/src/api.v2/helpers/pipeline/getPipelineStatus.js b/src/api.v2/helpers/pipeline/getPipelineStatus.js index afe2eca7d..a716ba8d1 100644 --- a/src/api.v2/helpers/pipeline/getPipelineStatus.js +++ b/src/api.v2/helpers/pipeline/getPipelineStatus.js @@ -32,8 +32,8 @@ const gem2sPipelineSteps = [ const seuratPipelineSteps = [ 'DownloadSeurat', - 'PreProcessing', - 'UploadToAWS']; + 'ProcessSeurat', + 'UploadSeuratToAWS']; // pipelineStepNames are the names of pipeline steps for which we // want to report the progress back to the user @@ -282,10 +282,10 @@ const getPipelineStatus = async (experimentId, processName) => { } const events = await getExecutionHistory(stepFunctions, executionArn); - error = checkError(events); const executedSteps = getStepsFromExecutionHistory(events); const lastExecuted = executedSteps[executedSteps.length - 1]; + switch (processName) { case pipelineConstants.QC_PROCESS_NAME: completedSteps = qcPipelineSteps.slice(0, qcPipelineSteps.indexOf(lastExecuted) + 1); diff --git a/src/api.v2/helpers/pipeline/pipelineConstruct/constructors/createNewStep.js b/src/api.v2/helpers/pipeline/pipelineConstruct/constructors/createNewStep.js index 6a1185357..20f38af9b 100644 --- a/src/api.v2/helpers/pipeline/pipelineConstruct/constructors/createNewStep.js +++ b/src/api.v2/helpers/pipeline/pipelineConstruct/constructors/createNewStep.js @@ -1,5 +1,5 @@ const config = require('../../../../../config'); -const { QC_PROCESS_NAME, GEM2S_PROCESS_NAME } = require('../../../../constants'); +const { QC_PROCESS_NAME, GEM2S_PROCESS_NAME, SEURAT_PROCESS_NAME } = require('../../../../constants'); const createTask = (taskName, context) => { const { @@ -41,6 +41,14 @@ const getGem2SParams = (task, context) => { }; }; +const getSeuratParams = (task, context) => { + const { taskParams } = context; + return { + ...task, + ...taskParams, + }; +}; + const buildParams = (task, context, stepArgs) => { let processParams; @@ -49,6 +57,8 @@ const buildParams = (task, context, stepArgs) => { processParams = getQCParams(task, context, stepArgs); } else if (task.processName === GEM2S_PROCESS_NAME) { processParams = getGem2SParams(task, context); + } else if (task.processName === SEURAT_PROCESS_NAME) { + processParams = getSeuratParams(task, context); } return { diff --git a/src/api.v2/helpers/pipeline/pipelineConstruct/index.js b/src/api.v2/helpers/pipeline/pipelineConstruct/index.js index b1744a903..3669cc7c7 100644 --- a/src/api.v2/helpers/pipeline/pipelineConstruct/index.js +++ b/src/api.v2/helpers/pipeline/pipelineConstruct/index.js @@ -313,6 +313,7 @@ const createGem2SPipeline = async (experimentId, taskParams) => { return { stateMachineArn, executionArn }; }; + const createSeuratObjectPipeline = async (experimentId, taskParams) => { const accountId = config.awsAccountId; const roleArn = `arn:aws:iam::${accountId}:role/state-machine-role-${config.clusterEnv}`; diff --git a/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/index.js b/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/index.js index b90fd36d8..e46e1767f 100644 --- a/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/index.js +++ b/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/index.js @@ -46,19 +46,15 @@ const getSkeletonStepNames = (skeleton) => { const getPipelineStepNames = () => { const gem2sStepNames = getSkeletonStepNames(gem2SPipelineSteps); const qcStepNames = getSkeletonStepNames(qcPipelineSteps); + const seuratStepNames = getSkeletonStepNames(seuratPipelineSteps); - return gem2sStepNames.concat(qcStepNames); + return gem2sStepNames.concat(qcStepNames).concat(seuratStepNames); }; // getPipelineStepNames returns the names of the QC pipeline steps // if there are map states with nested substeps it returns those sub-steps too const getQcPipelineStepNames = () => getSkeletonStepNames(qcPipelineSteps); - -// getSeuratStepNames returns the names of the seurat pipeline steps -// if there are map states with nested substeps it returns those sub-steps too -const getSeuratPipelineStepNames = () => getSkeletonStepNames(seuratPipelineSteps); - const buildInitialSteps = (clusterEnv, nextStep) => { // if we are running locally launch a pipeline job if (clusterEnv === 'development') { @@ -108,7 +104,6 @@ const getQcPipelineSkeleton = (clusterEnv, qcSteps) => ({ module.exports = { getPipelineStepNames, getQcPipelineStepNames, - getSeuratPipelineStepNames, getGem2sPipelineSkeleton, getQcPipelineSkeleton, getSeuratPipelineSkeleton, diff --git a/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/seuratPipelineSkeleton.js b/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/seuratPipelineSkeleton.js index 624d33d61..be2c03da2 100644 --- a/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/seuratPipelineSkeleton.js +++ b/src/api.v2/helpers/pipeline/pipelineConstruct/skeletons/seuratPipelineSkeleton.js @@ -4,23 +4,23 @@ const seuratPipelineSteps = { XConstructorArgs: { taskName: 'downloadSeurat', }, - Next: 'PreProcessing', + Next: 'ProcessSeurat', }, - PreProcessing: { + ProcessSeurat: { XStepType: 'create-new-step', XConstructorArgs: { - taskName: 'preprocSeurat', + taskName: 'processSeurat', }, - Next: 'UploadToAWS', + Next: 'UploadSeuratToAWS', }, - UploadToAWS: { + UploadSeuratToAWS: { XStepType: 'create-new-step', XConstructorArgs: { - taskName: 'uploadToAWS', + taskName: 'uploadSeuratToAWS', }, - Next: 'EndOfGem2S', + Next: 'EndOfSeurat', }, - EndOfGem2S: { + EndOfSeurat: { Type: 'Pass', End: true, }, diff --git a/src/api.v2/helpers/pipeline/seurat.js b/src/api.v2/helpers/pipeline/seurat.js index 8dc0cb164..7739d0879 100644 --- a/src/api.v2/helpers/pipeline/seurat.js +++ b/src/api.v2/helpers/pipeline/seurat.js @@ -9,6 +9,7 @@ const Sample = require('../../model/Sample'); const Experiment = require('../../model/Experiment'); const ExperimentExecution = require('../../model/ExperimentExecution'); +const sendNotification = require('./hooks/sendNotification'); const HookRunner = require('./hooks/HookRunner'); const validateRequest = require('../../../utils/schema-validator'); @@ -18,6 +19,20 @@ const logger = getLogger('[SeuratService] - '); const hookRunner = new HookRunner(); +const updateProcessingConfig = async (payload) => { + const { experimentId, item } = payload; + + await new Experiment().updateById(experimentId, { processing_config: item.processingConfig }); + + logger.log(`Experiment: ${experimentId}. Saved processing config received from seurat`); +}; + +hookRunner.register('uploadSeuratToAWS', [ + updateProcessingConfig, +]); + + +hookRunner.registerAll([sendNotification]); const sendUpdateToSubscribed = async (experimentId, message, io) => { const statusRes = await getPipelineStatus(experimentId, constants.SEURAT_PROCESS_NAME); @@ -134,7 +149,7 @@ const handleSeuratResponse = async (io, message) => { AWSXRay.getSegment().addMetadata('message', message); // Fail hard if there was an error. - await validateRequest(message, 'SEURATResponse.v2.yaml'); + await validateRequest(message, 'SeuratResponse.v2.yaml'); await hookRunner.run(message); diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 963157d87..711d2662f 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1623,6 +1623,7 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPError' + '/experiments/{experimentId}/gem2s': post: summary: Run gem2s @@ -1679,6 +1680,7 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPError' + '/experiments/{experimentId}/seurat': post: summary: Run seurat @@ -1735,6 +1737,7 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPError' + '/experiments/{experimentId}/plots/{plotUuid}': parameters: - schema: @@ -1906,6 +1909,7 @@ paths: type: string pattern: nok parameters: [] + /seuratResults: post: summary: Retrieve results from pipeline step functions @@ -1943,6 +1947,7 @@ paths: type: string pattern: nok parameters: [] + /pipelineResults: post: summary: Placeholder endpoint for qc, necessary so subscriptions to sns work diff --git a/src/specs/models/PipelinePodRequest.v2.yaml b/src/specs/models/PipelinePodRequest.v2.yaml index 84877b130..5526aa0c5 100644 --- a/src/specs/models/PipelinePodRequest.v2.yaml +++ b/src/specs/models/PipelinePodRequest.v2.yaml @@ -19,7 +19,7 @@ properties: description: The activity ID associated to the pipeline state machine. processName: type: string - description: The process name of the pipeline (qc / gem2s). + description: The process name of the pipeline (qc / gem2s / seurat). required: - sandboxId - activityId diff --git a/src/specs/models/ProcessingConfigBodies.v2.yaml b/src/specs/models/ProcessingConfigBodies.v2.yaml index 525fc7313..2135821e1 100644 --- a/src/specs/models/ProcessingConfigBodies.v2.yaml +++ b/src/specs/models/ProcessingConfigBodies.v2.yaml @@ -11,4 +11,5 @@ properties: - $ref: ./processing-config-bodies/ProcessingConfigDoubletScores.v2.yaml - $ref: ./processing-config-bodies/ProcessingConfigDataIntegration.v2.yaml - $ref: ./processing-config-bodies/ProcessingConfigConfigureEmbedding.v2.yaml - - $ref: ./processing-config-bodies/ProcessingConfigMeta.v2.yaml \ No newline at end of file + - $ref: ./processing-config-bodies/ProcessingConfigMeta.v2.yaml + - $ref: ./processing-config-bodies/ProcessingConfigSeurat.v2.yaml \ No newline at end of file diff --git a/src/specs/models/SEURATResponse.v2.yaml b/src/specs/models/SeuratResponse.v2.yaml similarity index 97% rename from src/specs/models/SEURATResponse.v2.yaml rename to src/specs/models/SeuratResponse.v2.yaml index 7858e54c7..84cd8eebf 100644 --- a/src/specs/models/SEURATResponse.v2.yaml +++ b/src/specs/models/SeuratResponse.v2.yaml @@ -1,4 +1,4 @@ -title: SEURAT response +title: Seurat response description: This is the format the seurat clients communicate the result of a seurat run. properties: experimentId: diff --git a/src/specs/models/experiment-bodies/ExperimentInfo.v2.yaml b/src/specs/models/experiment-bodies/ExperimentInfo.v2.yaml index 10ff82f88..68fb23c99 100644 --- a/src/specs/models/experiment-bodies/ExperimentInfo.v2.yaml +++ b/src/specs/models/experiment-bodies/ExperimentInfo.v2.yaml @@ -41,6 +41,22 @@ properties: - paramsHash - stateMachineArn - executionArn + seurat: + # Couldn't import PipelineExecution.v2.yaml twice for some reason + nullable: true + type: object + properties: + paramsHash: + nullable: true + type: string + stateMachineArn: + type: string + executionArn: + type: string + required: + - paramsHash + - stateMachineArn + - executionArn qc: nullable: true type: object diff --git a/src/specs/models/processing-config-bodies/ProcessingConfigConfigureEmbedding.v2.yaml b/src/specs/models/processing-config-bodies/ProcessingConfigConfigureEmbedding.v2.yaml index 2d3ba5315..595eb75e8 100644 --- a/src/specs/models/processing-config-bodies/ProcessingConfigConfigureEmbedding.v2.yaml +++ b/src/specs/models/processing-config-bodies/ProcessingConfigConfigureEmbedding.v2.yaml @@ -10,6 +10,8 @@ properties: method: type: string minLength: 1 + useSaved: + type: boolean methodSettings: type: object properties: diff --git a/src/specs/models/work-request-bodies/WorkRequestGetEmbedding.v2.yaml b/src/specs/models/work-request-bodies/WorkRequestGetEmbedding.v2.yaml index 74bac09b5..f02aea9d8 100644 --- a/src/specs/models/work-request-bodies/WorkRequestGetEmbedding.v2.yaml +++ b/src/specs/models/work-request-bodies/WorkRequestGetEmbedding.v2.yaml @@ -2,6 +2,8 @@ title: Get embedding description: Work request body for the Get Embedding task. allOf: - properties: + useSaved: + type: boolean name: type: string example: GetEmbedding From cacc5777150b18e727dd0e367045c717ee963f89 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Fri, 23 Sep 2022 11:06:23 -0700 Subject: [PATCH 11/26] fix tests --- src/api.v2/helpers/pipeline/__mocks__/getPipelineStatus.js | 6 ++++++ tests/api.v2/events/validateAndSubmitWork.test.js | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/api.v2/helpers/pipeline/__mocks__/getPipelineStatus.js b/src/api.v2/helpers/pipeline/__mocks__/getPipelineStatus.js index 5d1ee81d5..f47b8d4b4 100644 --- a/src/api.v2/helpers/pipeline/__mocks__/getPipelineStatus.js +++ b/src/api.v2/helpers/pipeline/__mocks__/getPipelineStatus.js @@ -7,6 +7,12 @@ const responseTemplates = { status: pipelineConstants.SUCCEEDED, stopDate: null, }, + seurat: { + completedSteps: [], + startDate: null, + status: pipelineConstants.NOT_CREATED, + stopDate: null, + }, qc: { completedSteps: [], startDate: null, diff --git a/tests/api.v2/events/validateAndSubmitWork.test.js b/tests/api.v2/events/validateAndSubmitWork.test.js index 29e827ecf..91a11f80c 100644 --- a/tests/api.v2/events/validateAndSubmitWork.test.js +++ b/tests/api.v2/events/validateAndSubmitWork.test.js @@ -135,7 +135,7 @@ describe('handleWorkRequest', () => { socketId: '6789', experimentId: 'my-experiment', timeout: '2099-01-01T00:00:00Z', - body: { name: 'GetEmbedding', type: 'umap', config: { distanceMetric: 'euclidean' } }, + body: { name: 'GetEmbedding', type: 'umap', config: { minimumDistance: 0, distanceMetric: 'euclidean' } }, }; getPipelineStatus.mockImplementationOnce(() => ({ From 663fcbb52fc015bb5963768259f84de5f089203c Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Mon, 26 Sep 2022 15:46:12 -0700 Subject: [PATCH 12/26] multipart upload works for large seurat objects --- .../controllers/sampleFileController.js | 19 +++-- src/api.v2/helpers/s3/signedUrl.js | 81 +++++++++++++++++-- src/api.v2/routes/sampleFile.js | 7 +- src/specs/api.v2.yaml | 62 +++++++++++++- .../20220830130411_add_seurat_enum.js | 4 + 5 files changed, 161 insertions(+), 12 deletions(-) diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index 2720d9de8..b33fe896b 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -3,7 +3,7 @@ const sqlClient = require('../../sql/sqlClient'); const Sample = require('../model/Sample'); const SampleFile = require('../model/SampleFile'); -const { getSampleFileUploadUrl, getSampleFileDownloadUrl } = require('../helpers/s3/signedUrl'); +const { getSampleFileUploadUrls, getSampleFileDownloadUrl, completeMultiPartUpload } = require('../helpers/s3/signedUrl'); const { OK } = require('../../utils/responses'); const getLogger = require('../../utils/getLogger'); @@ -24,17 +24,17 @@ const createFile = async (req, res) => { upload_status: 'uploading', }; - let signedUrl; + let signedUrls; await sqlClient.get().transaction(async (trx) => { await new SampleFile(trx).create(newSampleFile); await new Sample(trx).setNewFile(sampleId, sampleFileId, sampleFileType); - signedUrl = getSampleFileUploadUrl(sampleFileId, metadata); + signedUrls = await getSampleFileUploadUrls(sampleFileId, metadata, size); }); logger.log(`Finished creating sample file for experiment ${experimentId}, sample ${sampleId}, sampleFileType ${sampleFileType}`); - res.json(signedUrl); + res.json(signedUrls); }; const patchFile = async (req, res) => { @@ -50,6 +50,15 @@ const patchFile = async (req, res) => { res.json(OK()); }; +const completeMultipart = async (req, res) => { + const { + body: { parts, uploadId, sampleFileId }, + } = req; + + completeMultiPartUpload(sampleFileId, parts, uploadId); + res.json(OK()); +}; + const getS3DownloadUrl = async (req, res) => { const { experimentId, sampleId, sampleFileType } = req.params; @@ -63,5 +72,5 @@ const getS3DownloadUrl = async (req, res) => { }; module.exports = { - createFile, patchFile, getS3DownloadUrl, + createFile, patchFile, getS3DownloadUrl, completeMultipart, }; diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index ce215d0d7..959527b9f 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -22,7 +22,73 @@ const getSignedUrl = (operation, params) => { return s3.getSignedUrl(operation, params); }; -const getSampleFileUploadUrl = (sampleFileId, metadata) => { +const FILE_CHUNK_SIZE = 10000000; + +const getMultipartSignedUrls = async (operation, params, size) => { + if (!params.Bucket) throw new Error('Bucket is required'); + if (!params.Key) throw new Error('Key is required'); + + const S3Config = { + apiVersion: '2006-03-01', + signatureVersion: 'v4', + region: config.awsRegion, + }; + + const s3 = new AWS.S3(S3Config); + + const { UploadId } = await s3.createMultipartUpload(params).promise(); + + const baseParams = { + ...params, + UploadId, + }; + + const promises = []; + + // TODO: based on size + const parts = Math.ceil(size / FILE_CHUNK_SIZE); + + + for (let i = 0; i < parts; i += 1) { + promises.push( + s3.getSignedUrlPromise('uploadPart', { + ...baseParams, + PartNumber: i + 1, + }), + ); + } + + const signedUrls = await Promise.all(promises); + + return { + signedUrls, + UploadId, + }; +}; + + +const completeMultiPartUpload = async (sampleFileId, parts, uploadId) => { + const params = { + Bucket: bucketNames.SAMPLE_FILES, + Key: `${sampleFileId}`, + UploadId: uploadId, + MultipartUpload: { Parts: parts }, + }; + + + const S3Config = { + apiVersion: '2006-03-01', + signatureVersion: 'v4', + region: config.awsRegion, + }; + + const s3 = new AWS.S3(S3Config); + + + await s3.completeMultipartUpload(params).promise(); +}; + +const getSampleFileUploadUrls = async (sampleFileId, metadata, size) => { const params = { Bucket: bucketNames.SAMPLE_FILES, Key: `${sampleFileId}`, @@ -36,9 +102,8 @@ const getSampleFileUploadUrl = (sampleFileId, metadata) => { }; } - const signedUrl = getSignedUrl('putObject', params); - - return signedUrl; + const signedUrls = await getMultipartSignedUrls('putObject', params, size); + return signedUrls; }; const fileNameToReturn = { @@ -67,4 +132,10 @@ const getSampleFileDownloadUrl = async (experimentId, sampleId, fileType) => { return signedUrl; }; -module.exports = { getSampleFileUploadUrl, getSampleFileDownloadUrl, getSignedUrl }; +module.exports = { + getSampleFileUploadUrls, + getSampleFileDownloadUrl, + getSignedUrl, + getMultipartSignedUrls, + completeMultiPartUpload, +}; diff --git a/src/api.v2/routes/sampleFile.js b/src/api.v2/routes/sampleFile.js index 5cd731bea..201d4540f 100644 --- a/src/api.v2/routes/sampleFile.js +++ b/src/api.v2/routes/sampleFile.js @@ -1,4 +1,6 @@ -const { createFile, patchFile, getS3DownloadUrl } = require('../controllers/sampleFileController'); +const { + createFile, patchFile, getS3DownloadUrl, completeMultipart, +} = require('../controllers/sampleFileController'); const { expressAuthorizationMiddleware } = require('../middlewares/authMiddlewares'); @@ -11,6 +13,9 @@ module.exports = { expressAuthorizationMiddleware, (req, res, next) => patchFile(req, res).catch(next), ], + 'sampleFile#completeMultipart': [ + (req, res, next) => completeMultipart(req, res).catch(next), + ], 'sampleFile#downloadUrl': [ expressAuthorizationMiddleware, (req, res, next) => getS3DownloadUrl(req, res).catch(next), diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 711d2662f..b05a24fb1 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1237,7 +1237,7 @@ paths: content: application/json: schema: - type: string + type: object '400': description: Bad Request content: @@ -1335,6 +1335,66 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPError' + /completeMultipartUpload: + post: + summary: Complete a multipart upload + description: Complete a multipart upload of a sample file + operationId: completeMultipart + x-eov-operation-id: sampleFile#completeMultipart + x-eov-operation-handler: routes/sampleFile + requestBody: + content: + application/json: + schema: + description: information here + type: object + properties: + sampleFileId: + type: string + uploadId: + type: string + parts: + type: array + items: + type: object + additionalProperties: false + responses: + '200': + description: Success + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPSuccess' + '400': + description: Bad Request + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '401': + description: The request lacks authentication credentials. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '403': + description: Forbidden request for this user. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '404': + description: Not found error. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' + '424': + description: Terms not accepted error. + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPError' '/experiments/{experimentId}/clone': post: summary: Clone an experiment diff --git a/src/sql/migrations/20220830130411_add_seurat_enum.js b/src/sql/migrations/20220830130411_add_seurat_enum.js index fe4bcaa5d..c20da9665 100644 --- a/src/sql/migrations/20220830130411_add_seurat_enum.js +++ b/src/sql/migrations/20220830130411_add_seurat_enum.js @@ -20,6 +20,8 @@ exports.up = async (knex) => { ALTER COLUMN pipeline_type TYPE pipeline_type_temp USING pipeline_type::text::pipeline_type_temp; DROP TYPE IF EXISTS pipeline_type; ALTER TYPE pipeline_type_temp RENAME TO pipeline_type; + + ALTER TABLE sample_file ALTER COLUMN size TYPE BIGINT; `); }; @@ -45,6 +47,8 @@ exports.down = async (knex) => { ALTER COLUMN pipeline_type TYPE pipeline_type_temp USING pipeline_type::text::pipeline_type_temp; DROP TYPE IF EXISTS pipeline_type; ALTER TYPE pipeline_type_temp RENAME TO pipeline_type; + + ALTER TABLE sample_file ALTER COLUMN size TYPE INT; `); }; From 5ab2edcb56fc916dfa8c5ef41b1710d235ac9bbf Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Mon, 31 Oct 2022 12:37:28 -0700 Subject: [PATCH 13/26] comment out tests to check staging --- .../controllers/sampleFileController.test.js | 6 +- .../s3/__snapshots__/signedUrl.test.js.snap | 57 ------------------- tests/api.v2/helpers/s3/signedUrl.test.js | 52 +++++++++-------- 3 files changed, 32 insertions(+), 83 deletions(-) diff --git a/tests/api.v2/controllers/sampleFileController.test.js b/tests/api.v2/controllers/sampleFileController.test.js index 1ff068319..a1f86dbfe 100644 --- a/tests/api.v2/controllers/sampleFileController.test.js +++ b/tests/api.v2/controllers/sampleFileController.test.js @@ -23,12 +23,12 @@ const mockRes = { }; describe('sampleFileController', () => { - const mockSignedUrl = 'signedUrl'; + const mockSignedUrls = ['signedUrl']; beforeEach(async () => { jest.clearAllMocks(); - signedUrl.getSampleFileUploadUrl.mockReturnValue(mockSignedUrl); + signedUrl.getSampleFileUploadUrls.mockReturnValue(mockSignedUrls); }); it('createFile works correctly', async () => { @@ -61,7 +61,7 @@ describe('sampleFileController', () => { expect(sampleInstance.setNewFile).toHaveBeenCalledWith('sampleId', 'sampleFileId', 'features10x'); // Response is generated signed url - expect(mockRes.json).toHaveBeenCalledWith(mockSignedUrl); + expect(mockRes.json).toHaveBeenCalledWith(mockSignedUrls); }); it('createFile errors out if the transaction failed', async () => { diff --git a/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap b/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap index 7a7d4ca3d..114d45359 100644 --- a/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap +++ b/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap @@ -20,60 +20,3 @@ exports[`getSampleFileDownloadUrl works correctly 1`] = ` ], } `; - -exports[`getSampleFileUploadUrl works correctly with metadata cellrangerVersion 1`] = ` -[MockFunction] { - "calls": Array [ - Array [ - "putObject", - Object { - "Bucket": "biomage-originals-test-000000000000", - "Expires": 3600, - "Key": "mockSampleFileId", - }, - ], - Array [ - "putObject", - Object { - "Bucket": "biomage-originals-test-000000000000", - "Expires": 3600, - "Key": "mockSampleFileId", - "Metadata": Object { - "cellranger_version": "v2", - }, - }, - ], - ], - "results": Array [ - Object { - "type": "return", - "value": "signedUrl", - }, - Object { - "type": "return", - "value": "signedUrl", - }, - ], -} -`; - -exports[`getSampleFileUploadUrl works correctly without metadata 1`] = ` -[MockFunction] { - "calls": Array [ - Array [ - "putObject", - Object { - "Bucket": "biomage-originals-test-000000000000", - "Expires": 3600, - "Key": "mockSampleFileId", - }, - ], - ], - "results": Array [ - Object { - "type": "return", - "value": "signedUrl", - }, - ], -} -`; diff --git a/tests/api.v2/helpers/s3/signedUrl.test.js b/tests/api.v2/helpers/s3/signedUrl.test.js index 85ffab34f..310cd8792 100644 --- a/tests/api.v2/helpers/s3/signedUrl.test.js +++ b/tests/api.v2/helpers/s3/signedUrl.test.js @@ -5,7 +5,7 @@ const signedUrl = require('../../../../src/api.v2/helpers/s3/signedUrl'); const AWS = require('../../../../src/utils/requireAWS'); const { NotFoundError } = require('../../../../src/utils/responses'); -const { getSignedUrl, getSampleFileUploadUrl, getSampleFileDownloadUrl } = signedUrl; +const { getSignedUrl, getSampleFileDownloadUrl } = signedUrl; const sampleFileInstance = new SampleFile(); jest.mock('../../../../src/api.v2/model/SampleFile'); @@ -69,36 +69,42 @@ describe('getSignedUrl', () => { }); }); -describe('getSampleFileUploadUrl', () => { - const mockSampleFileId = 'mockSampleFileId'; +// describe('getSampleFileUploadUrls', () => { +// const mockSampleFileId = 'mockSampleFileId'; - const signedUrlResponse = 'signedUrl'; +// const signedUrlResponse = ['signedUrl']; - const signedUrlSpy = jest.fn(); +// const createMultipartUploadSpy = jest.fn(); +// createMultipartUploadSpy.promise = jest.fn(); +// const getSignedUrlPromiseSpy = jest.fn(); - beforeEach(() => { - signedUrlSpy.mockReturnValueOnce(signedUrlResponse); +// beforeEach(() => { +// createMultipartUploadSpy.promise.mockResolvedValue({ UploadId: 'uploadId' }); +// getSignedUrlPromiseSpy.mockResolvedValue('signedUrl'); - AWS.S3.mockReset(); - AWS.S3.mockImplementation(() => ({ - getSignedUrl: signedUrlSpy, - })); - }); +// AWS.S3.mockReset(); +// AWS.S3.mockImplementation(() => ({ +// createMultipartUpload: createMultipartUploadSpy, +// getSignedUrlPromise: getSignedUrlPromiseSpy, +// })); +// }); - it('works correctly without metadata', () => { - const response = getSampleFileUploadUrl(mockSampleFileId, {}); +// it('works correctly without metadata', async () => { +// const response = await getSampleFileUploadUrls(mockSampleFileId, {}, 1); - expect(response).toEqual(signedUrlResponse); - expect(signedUrlSpy).toMatchSnapshot(); - }); +// expect(response).toEqual(signedUrlResponse); +// expect(createMultipartUploadSpy).toMatchSnapshot(); +// expect(getSignedUrlPromiseSpy).toMatchSnapshot(); +// }); - it('works correctly with metadata cellrangerVersion', () => { - const response = getSampleFileUploadUrl(mockSampleFileId, { cellrangerVersion: 'v2' }); +// it('works correctly with metadata cellrangerVersion', () => { +// const response = getSampleFileUploadUrls(mockSampleFileId, { cellrangerVersion: 'v2' }, 1); - expect(response).toEqual(signedUrlResponse); - expect(signedUrlSpy).toMatchSnapshot(); - }); -}); +// expect(response).toEqual(signedUrlResponse); +// expect(createMultipartUploadSpy).toMatchSnapshot(); +// expect(getSignedUrlPromiseSpy).toMatchSnapshot(); +// }); +// }); describe('getSampleFileDownloadUrl', () => { From 72c1d841ca487e33c22a36204f585608c06df7d1 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Mon, 31 Oct 2022 15:41:21 -0700 Subject: [PATCH 14/26] try fix out of memory in api --- src/api.v2/controllers/sampleFileController.js | 1 + src/api.v2/helpers/s3/signedUrl.js | 9 ++++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index b33fe896b..498d59054 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -30,6 +30,7 @@ const createFile = async (req, res) => { await new SampleFile(trx).create(newSampleFile); await new Sample(trx).setNewFile(sampleId, sampleFileId, sampleFileType); + logger.log(`Getting multipart upload urls for ${experimentId}, sample ${sampleId}, sampleFileType ${sampleFileType}`); signedUrls = await getSampleFileUploadUrls(sampleFileId, metadata, size); }); diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index 959527b9f..088d8fe07 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -43,23 +43,22 @@ const getMultipartSignedUrls = async (operation, params, size) => { UploadId, }; - const promises = []; + const signedUrls = []; // TODO: based on size const parts = Math.ceil(size / FILE_CHUNK_SIZE); for (let i = 0; i < parts; i += 1) { - promises.push( - s3.getSignedUrlPromise('uploadPart', { + signedUrls.push( + // eslint-disable-next-line no-await-in-loop + await s3.getSignedUrlPromise('uploadPart', { ...baseParams, PartNumber: i + 1, }), ); } - const signedUrls = await Promise.all(promises); - return { signedUrls, UploadId, From c756fb4f4391017bc21aed802ed8f0b1850817e5 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 1 Nov 2022 11:49:27 -0700 Subject: [PATCH 15/26] add logging --- src/api.v2/controllers/sampleFileController.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index 498d59054..89066d46e 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -56,7 +56,14 @@ const completeMultipart = async (req, res) => { body: { parts, uploadId, sampleFileId }, } = req; + console.log('parts!!!!'); + console.log(parts); + + logger.log(`completing multipart upload for sampleFileId ${sampleFileId}, uploadId ${uploadId}`); + completeMultiPartUpload(sampleFileId, parts, uploadId); + + logger.log(`completed multipart upload for sampleFileId ${sampleFileId}, uploadId ${uploadId}`); res.json(OK()); }; From 7e6c4089f9f6acf6da03e5c07112886333025841 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 1 Nov 2022 14:41:51 -0700 Subject: [PATCH 16/26] add logging --- .../controllers/sampleFileController.js | 3 +++ src/api.v2/helpers/s3/signedUrl.js | 25 ++++++++++--------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index 89066d46e..2f015beb3 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -34,6 +34,9 @@ const createFile = async (req, res) => { signedUrls = await getSampleFileUploadUrls(sampleFileId, metadata, size); }); + console.log('signedUrls!!!'); + console.log(signedUrls); + logger.log(`Finished creating sample file for experiment ${experimentId}, sample ${sampleId}, sampleFileType ${sampleFileType}`); res.json(signedUrls); }; diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index 088d8fe07..00f4ec8fa 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -24,7 +24,7 @@ const getSignedUrl = (operation, params) => { const FILE_CHUNK_SIZE = 10000000; -const getMultipartSignedUrls = async (operation, params, size) => { +const getMultipartSignedUrls = async (params, size) => { if (!params.Bucket) throw new Error('Bucket is required'); if (!params.Key) throw new Error('Key is required'); @@ -43,22 +43,20 @@ const getMultipartSignedUrls = async (operation, params, size) => { UploadId, }; - const signedUrls = []; - - // TODO: based on size + const promises = []; const parts = Math.ceil(size / FILE_CHUNK_SIZE); - for (let i = 0; i < parts; i += 1) { - signedUrls.push( - // eslint-disable-next-line no-await-in-loop - await s3.getSignedUrlPromise('uploadPart', { + promises.push( + s3.getSignedUrlPromise('uploadPart', { ...baseParams, PartNumber: i + 1, }), ); } + const signedUrls = await Promise.all(promises); + return { signedUrls, UploadId, @@ -74,6 +72,8 @@ const completeMultiPartUpload = async (sampleFileId, parts, uploadId) => { MultipartUpload: { Parts: parts }, }; + console.log('completeMultiPartUpload params!!!!'); + console.log(params); const S3Config = { apiVersion: '2006-03-01', @@ -82,15 +82,16 @@ const completeMultiPartUpload = async (sampleFileId, parts, uploadId) => { }; const s3 = new AWS.S3(S3Config); + const res = await s3.completeMultipartUpload(params).promise(); - - await s3.completeMultipartUpload(params).promise(); + console.log('completeMultiPartUpload result!!'); + console.log(res); }; const getSampleFileUploadUrls = async (sampleFileId, metadata, size) => { const params = { Bucket: bucketNames.SAMPLE_FILES, - Key: `${sampleFileId}`, + Key: sampleFileId, // 1 hour timeout of upload link Expires: 3600, }; @@ -101,7 +102,7 @@ const getSampleFileUploadUrls = async (sampleFileId, metadata, size) => { }; } - const signedUrls = await getMultipartSignedUrls('putObject', params, size); + const signedUrls = await getMultipartSignedUrls(params, size); return signedUrls; }; From c54e0db4d37be8b0ba02067533f6c7205054d128 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Wed, 2 Nov 2022 13:10:46 -0700 Subject: [PATCH 17/26] add seurat tests --- tests/api.v2/routes/seurat.test.js | 118 +++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 tests/api.v2/routes/seurat.test.js diff --git a/tests/api.v2/routes/seurat.test.js b/tests/api.v2/routes/seurat.test.js new file mode 100644 index 000000000..a5e2994b1 --- /dev/null +++ b/tests/api.v2/routes/seurat.test.js @@ -0,0 +1,118 @@ +// @ts-nocheck +const express = require('express'); +const request = require('supertest'); +const expressLoader = require('../../../src/loaders/express'); + +const { OK } = require('../../../src/utils/responses'); + +const seuratController = require('../../../src/api.v2/controllers/seuratController'); + +jest.mock('../../../src/api.v2/controllers/seuratController', () => ({ + runSeurat: jest.fn(), + handleResponse: jest.fn(), +})); + +jest.mock('../../../src/api.v2/middlewares/authMiddlewares'); + +describe('tests for seurat route', () => { + let app = null; + + beforeEach(async () => { + const mockApp = await expressLoader(express()); + app = mockApp.app; + }); + + afterEach(() => { + /** + * Most important since b'coz of caching, the mocked implementations sometimes does not reset + */ + jest.resetModules(); + jest.restoreAllMocks(); + }); + + it('Creating a new seurat run results in a successful response', async (done) => { + seuratController.runSeurat.mockImplementationOnce((req, res) => { + res.json(OK()); + return Promise.resolve(); + }); + + const experimentId = 'experiment-id'; + + const mockReq = { paramsHash: 'mockParamsHash' }; + + request(app) + .post(`/v2/experiments/${experimentId}/seurat`) + .send(mockReq) + .expect(200) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); + + it('Creating a new seurat run with an invalid body fails', async (done) => { + seuratController.runSeurat.mockImplementationOnce((req, res) => { + res.json(OK()); + return Promise.resolve(); + }); + + const experimentId = 'experiment-id'; + + const mockReqBody = { paramsHashInvalidKey: 'mockParamsHash' }; + + request(app) + .post(`/v2/experiments/${experimentId}/seurat`) + .send(mockReqBody) + .expect(400) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); + + it('Sending a seuratResult results in a successful response', async (done) => { + seuratController.handleResponse.mockImplementationOnce((req, res) => { + res.json(OK()); + return Promise.resolve(); + }); + + const mockBody = { + Type: 'Notification', + MessageId: 'ce0a05bf-c500-4dc7-8d0d-2ba974bf2831', + TopicArn: 'arn:aws:sns:eu-west-1:000000000000:work-results-development-default-v2', + Message: '{a: "the actual message"}', + Timestamp: '2022-05-10T17:03:16.542Z', + SignatureVersion: '1', + Signature: 'EXAMPLEpH+..', + SigningCertURL: 'https://sns.us-east-1.amazonaws.com/SimpleNotificationService-0000000000000000000000.pem', + UnsubscribeURL: 'http://localhost:4566/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:000000000000:work-results-development-default-v2:18c4de8d-e180-485a-9b79-de0b173b7db8', + MessageAttributes: { + type: { + Type: 'String', + Value: 'SeuratResponse', + }, + }, + }; + + request(app) + .post('/v2/seuratResults') + .send(mockBody) + .expect(200) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); +}); From e0dc827f20b72a7497affcdbcb3f5ab8257e5252 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Fri, 4 Nov 2022 16:26:00 -0700 Subject: [PATCH 18/26] remove debug messages --- src/api.v2/controllers/sampleFileController.js | 9 ++------- src/api.v2/helpers/s3/signedUrl.js | 7 +------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index 2f015beb3..981bc95d2 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -34,8 +34,6 @@ const createFile = async (req, res) => { signedUrls = await getSampleFileUploadUrls(sampleFileId, metadata, size); }); - console.log('signedUrls!!!'); - console.log(signedUrls); logger.log(`Finished creating sample file for experiment ${experimentId}, sample ${sampleId}, sampleFileType ${sampleFileType}`); res.json(signedUrls); @@ -59,14 +57,11 @@ const completeMultipart = async (req, res) => { body: { parts, uploadId, sampleFileId }, } = req; - console.log('parts!!!!'); - console.log(parts); - - logger.log(`completing multipart upload for sampleFileId ${sampleFileId}, uploadId ${uploadId}`); + logger.log(`completing multipart upload for sampleFileId ${sampleFileId}`); completeMultiPartUpload(sampleFileId, parts, uploadId); - logger.log(`completed multipart upload for sampleFileId ${sampleFileId}, uploadId ${uploadId}`); + logger.log(`completed multipart upload for sampleFileId ${sampleFileId}`); res.json(OK()); }; diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index 00f4ec8fa..7c0f26046 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -72,8 +72,6 @@ const completeMultiPartUpload = async (sampleFileId, parts, uploadId) => { MultipartUpload: { Parts: parts }, }; - console.log('completeMultiPartUpload params!!!!'); - console.log(params); const S3Config = { apiVersion: '2006-03-01', @@ -82,10 +80,7 @@ const completeMultiPartUpload = async (sampleFileId, parts, uploadId) => { }; const s3 = new AWS.S3(S3Config); - const res = await s3.completeMultipartUpload(params).promise(); - - console.log('completeMultiPartUpload result!!'); - console.log(res); + await s3.completeMultipartUpload(params).promise(); }; const getSampleFileUploadUrls = async (sampleFileId, metadata, size) => { From 940f02106924a300ec986ca048cf2b7101929200 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Mon, 7 Nov 2022 16:49:05 -0800 Subject: [PATCH 19/26] improve naming --- src/api.v2/controllers/sampleFileController.js | 6 +++--- src/api.v2/helpers/s3/signedUrl.js | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index 981bc95d2..086f6cbd6 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -24,19 +24,19 @@ const createFile = async (req, res) => { upload_status: 'uploading', }; - let signedUrls; + let uploadUrlParams; await sqlClient.get().transaction(async (trx) => { await new SampleFile(trx).create(newSampleFile); await new Sample(trx).setNewFile(sampleId, sampleFileId, sampleFileType); logger.log(`Getting multipart upload urls for ${experimentId}, sample ${sampleId}, sampleFileType ${sampleFileType}`); - signedUrls = await getSampleFileUploadUrls(sampleFileId, metadata, size); + uploadUrlParams = await getSampleFileUploadUrls(sampleFileId, metadata, size); }); logger.log(`Finished creating sample file for experiment ${experimentId}, sample ${sampleId}, sampleFileType ${sampleFileType}`); - res.json(signedUrls); + res.json(uploadUrlParams); }; const patchFile = async (req, res) => { diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index 7c0f26046..9552d14c6 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -24,7 +24,7 @@ const getSignedUrl = (operation, params) => { const FILE_CHUNK_SIZE = 10000000; -const getMultipartSignedUrls = async (params, size) => { +const createMultipartUpload = async (params, size) => { if (!params.Bucket) throw new Error('Bucket is required'); if (!params.Key) throw new Error('Key is required'); @@ -59,7 +59,7 @@ const getMultipartSignedUrls = async (params, size) => { return { signedUrls, - UploadId, + uploadId: UploadId, }; }; @@ -97,8 +97,7 @@ const getSampleFileUploadUrls = async (sampleFileId, metadata, size) => { }; } - const signedUrls = await getMultipartSignedUrls(params, size); - return signedUrls; + return await createMultipartUpload(params, size); }; const fileNameToReturn = { @@ -131,6 +130,6 @@ module.exports = { getSampleFileUploadUrls, getSampleFileDownloadUrl, getSignedUrl, - getMultipartSignedUrls, + createMultipartUpload, completeMultiPartUpload, }; From 9dcdef0d7fced45a2b8ff7e386fff857d8b9a984 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Mon, 14 Nov 2022 15:49:12 -0800 Subject: [PATCH 20/26] adding tests --- .../pipelineConstruct.test.js.snap | 138 +++++++++++++++ .../pipeline/getPipelineStatus.test.js | 30 +++- .../pipeline/pipelineConstruct.test.js | 105 +++++++++++- .../s3/__snapshots__/signedUrl.test.js.snap | 159 ++++++++++++++++++ tests/api.v2/helpers/s3/signedUrl.test.js | 57 +++---- 5 files changed, 455 insertions(+), 34 deletions(-) diff --git a/tests/api.v2/helpers/pipeline/__snapshots__/pipelineConstruct.test.js.snap b/tests/api.v2/helpers/pipeline/__snapshots__/pipelineConstruct.test.js.snap index 110e436b2..9f7c451c0 100644 --- a/tests/api.v2/helpers/pipeline/__snapshots__/pipelineConstruct.test.js.snap +++ b/tests/api.v2/helpers/pipeline/__snapshots__/pipelineConstruct.test.js.snap @@ -141,6 +141,69 @@ Array [ ] `; +exports[`test for pipeline services Create Seurat pipeline works 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + Object { + "name": "biomage-test", + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "name": "biomage-test", + }, + }, + ], +} +`; + +exports[`test for pipeline services Create Seurat pipeline works 2`] = ` +Array [ + Object { + "type": "return", + "value": Object { + "loggingConfiguration": Object { + "level": "OFF", + }, + "name": "biomage-seurat-test-b2bacc54e31beb2bda7b4bd9b1ec33ec84eae60f", + "roleArn": "arn:aws:iam::000000000000:role/state-machine-role-test", + "tags": Array [ + Object { + "key": "experimentId", + "value": "testExperimentId", + }, + Object { + "key": "clusterEnv", + "value": "test", + }, + Object { + "key": "sandboxId", + "value": "default", + }, + ], + "type": "STANDARD", + }, + }, +] +`; + +exports[`test for pipeline services Create Seurat pipeline works 3`] = ` +Array [ + Object { + "type": "return", + "value": Object { + "input": "{\\"retries\\":[\\"retry\\"]}", + "stateMachineArn": "test-machine", + "traceHeader": undefined, + }, + }, +] +`; + exports[`test for pipeline services Gem2s Pipeline is updated instead of created if an error is thrown. 1`] = ` [MockFunction] { "calls": Array [ @@ -576,3 +639,78 @@ Array [ }, ] `; + +exports[`test for pipeline services Seurat Pipeline is updated instead of created if an error is thrown. 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + Object { + "name": "biomage-test", + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "name": "biomage-test", + }, + }, + ], +} +`; + +exports[`test for pipeline services Seurat Pipeline is updated instead of created if an error is thrown. 2`] = ` +Array [ + Object { + "type": "return", + "value": Object { + "loggingConfiguration": Object { + "level": "OFF", + }, + "name": "biomage-seurat-test-b2bacc54e31beb2bda7b4bd9b1ec33ec84eae60f", + "roleArn": "arn:aws:iam::000000000000:role/state-machine-role-test", + "tags": Array [ + Object { + "key": "experimentId", + "value": "testExperimentId", + }, + Object { + "key": "clusterEnv", + "value": "test", + }, + Object { + "key": "sandboxId", + "value": "default", + }, + ], + "type": "STANDARD", + }, + }, +] +`; + +exports[`test for pipeline services Seurat Pipeline is updated instead of created if an error is thrown. 3`] = ` +Array [ + Object { + "type": "return", + "value": Object { + "roleArn": "arn:aws:iam::000000000000:role/state-machine-role-test", + "stateMachineArn": "arn:aws:states:eu-west-1:000000000000:stateMachine:biomage-seurat-test-b2bacc54e31beb2bda7b4bd9b1ec33ec84eae60f", + }, + }, +] +`; + +exports[`test for pipeline services Seurat Pipeline is updated instead of created if an error is thrown. 4`] = ` +Array [ + Object { + "type": "return", + "value": Object { + "input": "{\\"retries\\":[\\"retry\\"]}", + "stateMachineArn": "arn:aws:states:eu-west-1:000000000000:stateMachine:biomage-seurat-test-b2bacc54e31beb2bda7b4bd9b1ec33ec84eae60f", + "traceHeader": undefined, + }, + }, +] +`; diff --git a/tests/api.v2/helpers/pipeline/getPipelineStatus.test.js b/tests/api.v2/helpers/pipeline/getPipelineStatus.test.js index 2b107e839..7f0e5573b 100644 --- a/tests/api.v2/helpers/pipeline/getPipelineStatus.test.js +++ b/tests/api.v2/helpers/pipeline/getPipelineStatus.test.js @@ -15,7 +15,7 @@ jest.mock('../../../../src/api.v2/model/ExperimentExecution'); jest.useFakeTimers('modern').setSystemTime(new Date(pipelineConstants.EXPIRED_EXECUTION_DATE).getTime()); const { - GEM2S_PROCESS_NAME, QC_PROCESS_NAME, + GEM2S_PROCESS_NAME, QC_PROCESS_NAME, SEURAT_PROCESS_NAME, } = constants; // these are constants used to indicate to a mocked component whether they should return a @@ -55,6 +55,13 @@ const mockRunResponse = [ paramsHash, lastStatusResponse: statusResponseSql, }, + { + pipelineType: SEURAT_PROCESS_NAME, + stateMachineArn: SUCCEEDED_ID, + executionArn: SUCCEEDED_ID, + paramsHash, + lastStatusResponse: statusResponseSql, + }, { pipelineType: QC_PROCESS_NAME, stateMachineArn: SUCCEEDED_ID, @@ -72,6 +79,13 @@ const mockExecutionNotExistResponse = [ paramsHash, lastStatusResponse: statusResponseSql, }, + { + pipelineType: SEURAT_PROCESS_NAME, + stateMachineArn: '', + executionArn: EXECUTION_DOES_NOT_EXIST_ID, + paramsHash, + lastStatusResponse: statusResponseSql, + }, { pipelineType: QC_PROCESS_NAME, stateMachineArn: '', @@ -89,6 +103,13 @@ const mockExecutionNotExistNullSqlResponse = [ paramsHash, lastStatusResponse: null, }, + { + pipelineType: SEURAT_PROCESS_NAME, + stateMachineArn: '', + executionArn: EXECUTION_DOES_NOT_EXIST_ID, + paramsHash, + lastStatusResponse: null, + }, { pipelineType: QC_PROCESS_NAME, stateMachineArn: '', @@ -106,6 +127,13 @@ const mockRandomExceptionResponse = [ paramsHash, lastStatusResponse: statusResponseSql, }, + { + pipelineType: SEURAT_PROCESS_NAME, + stateMachineArn: '', + executionArn: RANDOM_EXCEPTION, + paramsHash, + lastStatusResponse: statusResponseSql, + }, { pipelineType: QC_PROCESS_NAME, stateMachineArn: '', diff --git a/tests/api.v2/helpers/pipeline/pipelineConstruct.test.js b/tests/api.v2/helpers/pipeline/pipelineConstruct.test.js index 440572d43..69552cba1 100644 --- a/tests/api.v2/helpers/pipeline/pipelineConstruct.test.js +++ b/tests/api.v2/helpers/pipeline/pipelineConstruct.test.js @@ -11,7 +11,7 @@ const experimentExecutionInstance = new ExperimentExecution(); const mockStepNames = getQcPipelineStepNames(); -const { createQCPipeline, createGem2SPipeline } = jest.requireActual('../../../../src/api.v2/helpers/pipeline/pipelineConstruct'); +const { createQCPipeline, createGem2SPipeline, createSeuratObjectPipeline } = jest.requireActual('../../../../src/api.v2/helpers/pipeline/pipelineConstruct'); jest.mock('crypto', () => ({ ...jest.requireActual('crypto'), @@ -84,7 +84,16 @@ describe('test for pipeline services', () => { }, ]; - const taskParams = { + const gem2sTaskParams = { + projectId: 'test-project', + experimentName: 'valerio-massala', + organism: null, + input: { type: '10x' }, + sampleIds: ['3af6b6bb-a1aa-4375-9c2c-c112bada56ca'], + sampleNames: ['sample-1'], + }; + + const seuratTaskParams = { projectId: 'test-project', experimentName: 'valerio-massala', organism: null, @@ -269,7 +278,47 @@ describe('test for pipeline services', () => { callback(null, { executionArn: 'test-machine' }); }); - await createGem2SPipeline('testExperimentId', taskParams); + await createGem2SPipeline('testExperimentId', gem2sTaskParams); + expect(describeClusterSpy).toMatchSnapshot(); + + expect(createStateMachineSpy.mock.results).toMatchSnapshot(); + + expect(createActivitySpy).toHaveBeenCalled(); + expect(startExecutionSpy).toHaveBeenCalled(); + expect(startExecutionSpy.mock.results).toMatchSnapshot(); + }); + + it('Create Seurat pipeline works', async () => { + AWSMock.setSDKInstance(AWS); + + const describeClusterSpy = jest.fn((x) => x); + AWSMock.mock('EKS', 'describeCluster', (params, callback) => { + describeClusterSpy(params); + callback(null, mockCluster); + }); + + const createStateMachineSpy = jest.fn( + (stateMachineObject) => _.omit(stateMachineObject, ['definition', 'image']), + ); + + AWSMock.mock('StepFunctions', 'createStateMachine', (params, callback) => { + createStateMachineSpy(params); + callback(null, { stateMachineArn: 'test-machine' }); + }); + + const createActivitySpy = jest.fn((x) => x); + AWSMock.mock('StepFunctions', 'createActivity', (params, callback) => { + createActivitySpy(params); + callback(null, { activityArn: 'test-actvitiy' }); + }); + + const startExecutionSpy = jest.fn((x) => x); + AWSMock.mock('StepFunctions', 'startExecution', (params, callback) => { + startExecutionSpy(params); + callback(null, { executionArn: 'test-machine' }); + }); + + await createSeuratObjectPipeline('testExperimentId', seuratTaskParams); expect(describeClusterSpy).toMatchSnapshot(); expect(createStateMachineSpy.mock.results).toMatchSnapshot(); @@ -315,7 +364,55 @@ describe('test for pipeline services', () => { createGem2SPipeline.waitForDefinitionToPropagate = () => true; - await createGem2SPipeline('testExperimentId', taskParams); + await createGem2SPipeline('testExperimentId', gem2sTaskParams); + + expect(describeClusterSpy).toMatchSnapshot(); + expect(createStateMachineSpy.mock.results).toMatchSnapshot(); + + expect(updateStateMachineSpy).toHaveBeenCalled(); + expect(updateStateMachineSpy.mock.results).toMatchSnapshot(); + + expect(createActivitySpy).toHaveBeenCalled(); + expect(startExecutionSpy).toHaveBeenCalled(); + expect(startExecutionSpy.mock.results).toMatchSnapshot(); + }); + + it('Seurat Pipeline is updated instead of created if an error is thrown.', async () => { + AWSMock.setSDKInstance(AWS); + + const describeClusterSpy = jest.fn((x) => x); + AWSMock.mock('EKS', 'describeCluster', (params, callback) => { + describeClusterSpy(params); + callback(null, mockCluster); + }); + + const createStateMachineSpy = jest.fn((stateMachineObject) => _.omit(stateMachineObject, 'definition')); + AWSMock.mock('StepFunctions', 'createStateMachine', (params, callback) => { + createStateMachineSpy(params); + callback({ code: 'StateMachineAlreadyExists' }, null); + }); + + const updateStateMachineSpy = jest.fn((stateMachineObject) => _.omit(stateMachineObject, 'definition')); + AWSMock.mock('StepFunctions', 'updateStateMachine', (params, callback) => { + updateStateMachineSpy(params); + callback(null, { stateMachineArn: 'test-machine' }); + }); + + const createActivitySpy = jest.fn((x) => x); + AWSMock.mock('StepFunctions', 'createActivity', (params, callback) => { + createActivitySpy(params); + callback(null, { activityArn: 'test-actvitiy' }); + }); + + const startExecutionSpy = jest.fn((x) => x); + AWSMock.mock('StepFunctions', 'startExecution', (params, callback) => { + startExecutionSpy(params); + callback(null, { executionArn: 'test-execution' }); + }); + + createSeuratObjectPipeline.waitForDefinitionToPropagate = () => true; + + await createSeuratObjectPipeline('testExperimentId', seuratTaskParams); expect(describeClusterSpy).toMatchSnapshot(); expect(createStateMachineSpy.mock.results).toMatchSnapshot(); diff --git a/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap b/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap index 114d45359..1507a347f 100644 --- a/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap +++ b/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap @@ -20,3 +20,162 @@ exports[`getSampleFileDownloadUrl works correctly 1`] = ` ], } `; + +exports[`getSampleFileUploadUrls works correctly with metadata cellrangerVersion 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + Object { + "Bucket": "biomage-originals-test-000000000000", + "Expires": 3600, + "Key": "mockSampleFileId", + }, + ], + Array [ + Object { + "Bucket": "biomage-originals-test-000000000000", + "Expires": 3600, + "Key": "mockSampleFileId", + "Metadata": Object { + "cellranger_version": "v2", + }, + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "promise": [MockFunction] { + "calls": Array [ + Array [], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "UploadId": "uploadId", + }, + }, + ], + }, + }, + }, + Object { + "type": "return", + "value": Object { + "promise": [MockFunction] { + "calls": Array [ + Array [], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "UploadId": "uploadId", + }, + }, + ], + }, + }, + }, + ], +} +`; + +exports[`getSampleFileUploadUrls works correctly with metadata cellrangerVersion 2`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "uploadPart", + Object { + "Bucket": "biomage-originals-test-000000000000", + "Expires": 3600, + "Key": "mockSampleFileId", + "PartNumber": 1, + "UploadId": "uploadId", + }, + ], + Array [ + "uploadPart", + Object { + "Bucket": "biomage-originals-test-000000000000", + "Expires": 3600, + "Key": "mockSampleFileId", + "Metadata": Object { + "cellranger_version": "v2", + }, + "PartNumber": 1, + "UploadId": "uploadId", + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": "signedUrl", + }, + Object { + "type": "return", + "value": "signedUrl", + }, + ], +} +`; + +exports[`getSampleFileUploadUrls works correctly without metadata 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + Object { + "Bucket": "biomage-originals-test-000000000000", + "Expires": 3600, + "Key": "mockSampleFileId", + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "promise": [MockFunction] { + "calls": Array [ + Array [], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "UploadId": "uploadId", + }, + }, + ], + }, + }, + }, + ], +} +`; + +exports[`getSampleFileUploadUrls works correctly without metadata 2`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "uploadPart", + Object { + "Bucket": "biomage-originals-test-000000000000", + "Expires": 3600, + "Key": "mockSampleFileId", + "PartNumber": 1, + "UploadId": "uploadId", + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": "signedUrl", + }, + ], +} +`; diff --git a/tests/api.v2/helpers/s3/signedUrl.test.js b/tests/api.v2/helpers/s3/signedUrl.test.js index 310cd8792..0e2c0fa8b 100644 --- a/tests/api.v2/helpers/s3/signedUrl.test.js +++ b/tests/api.v2/helpers/s3/signedUrl.test.js @@ -5,7 +5,7 @@ const signedUrl = require('../../../../src/api.v2/helpers/s3/signedUrl'); const AWS = require('../../../../src/utils/requireAWS'); const { NotFoundError } = require('../../../../src/utils/responses'); -const { getSignedUrl, getSampleFileDownloadUrl } = signedUrl; +const { getSignedUrl, getSampleFileDownloadUrl, getSampleFileUploadUrls } = signedUrl; const sampleFileInstance = new SampleFile(); jest.mock('../../../../src/api.v2/model/SampleFile'); @@ -69,42 +69,41 @@ describe('getSignedUrl', () => { }); }); -// describe('getSampleFileUploadUrls', () => { -// const mockSampleFileId = 'mockSampleFileId'; +describe('getSampleFileUploadUrls', () => { + const mockSampleFileId = 'mockSampleFileId'; -// const signedUrlResponse = ['signedUrl']; + const signedUrlResponse = { signedUrls: ['signedUrl'], uploadId: 'uploadId' }; -// const createMultipartUploadSpy = jest.fn(); -// createMultipartUploadSpy.promise = jest.fn(); -// const getSignedUrlPromiseSpy = jest.fn(); + const createMultipartUploadSpy = jest.fn(); + const getSignedUrlPromiseSpy = jest.fn(); -// beforeEach(() => { -// createMultipartUploadSpy.promise.mockResolvedValue({ UploadId: 'uploadId' }); -// getSignedUrlPromiseSpy.mockResolvedValue('signedUrl'); + beforeEach(() => { + createMultipartUploadSpy.mockReturnValue({ promise: jest.fn().mockReturnValue({ UploadId: 'uploadId' }) }); + getSignedUrlPromiseSpy.mockReturnValue('signedUrl'); -// AWS.S3.mockReset(); -// AWS.S3.mockImplementation(() => ({ -// createMultipartUpload: createMultipartUploadSpy, -// getSignedUrlPromise: getSignedUrlPromiseSpy, -// })); -// }); + AWS.S3.mockReset(); + AWS.S3.mockImplementation(() => ({ + createMultipartUpload: createMultipartUploadSpy, + getSignedUrlPromise: getSignedUrlPromiseSpy, + })); + }); -// it('works correctly without metadata', async () => { -// const response = await getSampleFileUploadUrls(mockSampleFileId, {}, 1); + it('works correctly without metadata', async () => { + const response = await getSampleFileUploadUrls(mockSampleFileId, {}, 1); -// expect(response).toEqual(signedUrlResponse); -// expect(createMultipartUploadSpy).toMatchSnapshot(); -// expect(getSignedUrlPromiseSpy).toMatchSnapshot(); -// }); + expect(response).toEqual(signedUrlResponse); + expect(createMultipartUploadSpy).toMatchSnapshot(); + expect(getSignedUrlPromiseSpy).toMatchSnapshot(); + }); -// it('works correctly with metadata cellrangerVersion', () => { -// const response = getSampleFileUploadUrls(mockSampleFileId, { cellrangerVersion: 'v2' }, 1); + it('works correctly with metadata cellrangerVersion', async () => { + const response = await getSampleFileUploadUrls(mockSampleFileId, { cellrangerVersion: 'v2' }, 1); -// expect(response).toEqual(signedUrlResponse); -// expect(createMultipartUploadSpy).toMatchSnapshot(); -// expect(getSignedUrlPromiseSpy).toMatchSnapshot(); -// }); -// }); + expect(response).toEqual(signedUrlResponse); + expect(createMultipartUploadSpy).toMatchSnapshot(); + expect(getSignedUrlPromiseSpy).toMatchSnapshot(); + }); +}); describe('getSampleFileDownloadUrl', () => { From 64050269dd8a14250867917f68270849c69cd0a1 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Mon, 14 Nov 2022 16:15:04 -0800 Subject: [PATCH 21/26] add sampleFile tests for completeMultipartUpload --- src/specs/api.v2.yaml | 4 +++ tests/api.v2/routes/sampleFile.test.js | 49 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index b05a24fb1..aed004af2 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1358,6 +1358,10 @@ paths: items: type: object additionalProperties: false + required: + - sampleFileId + - uploadId + - parts responses: '200': description: Success diff --git a/tests/api.v2/routes/sampleFile.test.js b/tests/api.v2/routes/sampleFile.test.js index a912f5965..7c388103c 100644 --- a/tests/api.v2/routes/sampleFile.test.js +++ b/tests/api.v2/routes/sampleFile.test.js @@ -12,6 +12,7 @@ jest.mock('../../../src/api.v2/controllers/sampleFileController', () => ({ createFile: jest.fn(), patchFile: jest.fn(), getS3DownloadUrl: jest.fn(), + completeMultipart: jest.fn(), })); jest.mock('../../../src/api.v2/middlewares/authMiddlewares'); @@ -167,4 +168,52 @@ describe('tests for experiment route', () => { return done(); }); }); + + it('Completing a multipart upload results in a successful response', async (done) => { + sampleFileController.completeMultipart.mockImplementationOnce((req, res) => { + res.json(OK()); + return Promise.resolve(); + }); + + const completeMultipartBody = { + parts: [], uploadId: 'uploadId', sampleFileId: 'sampleFileId', + }; + + request(app) + .post('/v2/completeMultipartUpload') + .send(completeMultipartBody) + .expect(200) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); + it('Completing a multipart upload fails if body is invalid', async (done) => { + sampleFileController.completeMultipart.mockImplementationOnce((req, res) => { + res.json(OK()); + return Promise.resolve(); + }); + + const completeMultipartBody = { + parts: [], uploadId: 'uploadId', + }; + + request(app) + .post('/v2/completeMultipartUpload') + .send(completeMultipartBody) + .expect(400) + .end((err) => { + if (err) { + return done(err); + } + // there is no point testing for the values of the response body + // - if something is wrong, the schema validator will catch it + return done(); + }); + }); }); + From c623e30b43d010858a5c121ba7c16d9a0d6b413c Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Mon, 14 Nov 2022 16:20:32 -0800 Subject: [PATCH 22/26] add seuratController tests --- .../controllers/seuratController.test.js | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 tests/api.v2/controllers/seuratController.test.js diff --git a/tests/api.v2/controllers/seuratController.test.js b/tests/api.v2/controllers/seuratController.test.js new file mode 100644 index 000000000..9033acb3d --- /dev/null +++ b/tests/api.v2/controllers/seuratController.test.js @@ -0,0 +1,124 @@ +// @ts-nocheck +const seuratController = require('../../../src/api.v2/controllers/seuratController'); + +const { OK } = require('../../../src/utils/responses'); + +const seurat = require('../../../src/api.v2/helpers/pipeline/seurat'); +const parseSNSMessage = require('../../../src/utils/parse-sns-message'); + +jest.mock('../../../src/api.v2/helpers/pipeline/seurat'); +jest.mock('../../../src/utils/parse-sns-message'); + +const mockJsonSend = jest.fn(); +const mockRes = { + json: jest.fn(), + status: jest.fn(() => ({ send: mockJsonSend })), +}; + +describe('seuratController', () => { + beforeEach(async () => { + jest.clearAllMocks(); + }); + + it('runSeurat works correctly', async () => { + const experimentId = 'experimentId'; + const newExecution = 'mockNewExecution'; + + seurat.createSeuratPipeline.mockReturnValue(newExecution); + + const mockReq = { + params: { experimentId }, + headers: { authorization: 'mockAuthorization' }, + body: { paramsHash: 'mockParamsHash' }, + }; + + await seuratController.runSeurat(mockReq, mockRes); + + expect(seurat.createSeuratPipeline).toHaveBeenCalledWith( + experimentId, mockReq.body, mockReq.headers.authorization, + ); + + // Response is ok + expect(mockRes.json).toHaveBeenCalledWith(OK()); + }); + + it('handleResponse works correctly', async () => { + const experimentId = 'experimentId'; + + const io = 'mockIo'; + const parsedMessage = 'mockParsedMessage'; + + parseSNSMessage.mockReturnValue({ io, parsedMessage }); + + const mockReq = { params: { experimentId } }; + + await seuratController.handleResponse(mockReq, mockRes); + + expect(parseSNSMessage).toHaveBeenCalledWith(mockReq); + expect(seurat.handleSeuratResponse).toHaveBeenCalledWith(io, parsedMessage); + + // Response is ok + expect(mockRes.status).toHaveBeenCalledWith(200); + expect(mockJsonSend).toHaveBeenCalledWith('ok'); + }); + + it('handleResponse returns nok when parseSNSMessage fails', async () => { + const experimentId = 'experimentId'; + + parseSNSMessage.mockImplementationOnce(() => Promise.reject(new Error('Invalid sns message'))); + + const mockReq = { params: { experimentId } }; + + await seuratController.handleResponse(mockReq, mockRes); + + expect(parseSNSMessage).toHaveBeenCalledWith(mockReq); + + expect(seurat.handleSeuratResponse).not.toHaveBeenCalled(); + + // Response is nok + expect(mockRes.status).toHaveBeenCalledWith(200); + expect(mockJsonSend).toHaveBeenCalledWith('nok'); + }); + + it('handleResponse returns nok when seuratResponse fails', async () => { + const experimentId = 'experimentId'; + + const io = 'mockIo'; + const parsedMessage = 'mockParsedMessage'; + + parseSNSMessage.mockReturnValue({ io, parsedMessage }); + + seurat.handleSeuratResponse.mockImplementationOnce(() => Promise.reject(new Error('Some error with seuratResponse'))); + + const mockReq = { params: { experimentId } }; + + await seuratController.handleResponse(mockReq, mockRes); + + expect(parseSNSMessage).toHaveBeenCalledWith(mockReq); + expect(seurat.handleSeuratResponse).toHaveBeenCalledWith(io, parsedMessage); + + // Response is nok + expect(mockRes.status).toHaveBeenCalledWith(200); + expect(mockJsonSend).toHaveBeenCalledWith('nok'); + }); + + it('handleResponse ignores message if it isnt an sns notification', async () => { + const experimentId = 'experimentId'; + + const io = 'mockIo'; + const parsedMessage = undefined; + + parseSNSMessage.mockReturnValue({ io, parsedMessage }); + + const mockReq = { params: { experimentId } }; + + await seuratController.handleResponse(mockReq, mockRes); + + expect(parseSNSMessage).toHaveBeenCalledWith(mockReq); + expect(seurat.handleSeuratResponse).not.toHaveBeenCalled(); + + // Response is ok + expect(mockRes.status).toHaveBeenCalledWith(200); + expect(mockJsonSend).toHaveBeenCalledWith('ok'); + }); +}); From d9d0584eb20ff83ea321564b2a18c3a6c7111079 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 15 Nov 2022 09:16:47 -0800 Subject: [PATCH 23/26] test seurat pipeline helpers --- .../__snapshots__/seurat.test.js.snap | 67 +++++++++ tests/api.v2/helpers/pipeline/seurat.test.js | 134 ++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 tests/api.v2/helpers/pipeline/__snapshots__/seurat.test.js.snap create mode 100644 tests/api.v2/helpers/pipeline/seurat.test.js diff --git a/tests/api.v2/helpers/pipeline/__snapshots__/seurat.test.js.snap b/tests/api.v2/helpers/pipeline/__snapshots__/seurat.test.js.snap new file mode 100644 index 000000000..a45021466 --- /dev/null +++ b/tests/api.v2/helpers/pipeline/__snapshots__/seurat.test.js.snap @@ -0,0 +1,67 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`createSeuratObjectPipeline works correctly 1`] = ` +Array [ + Object { + "experiment_id": "mockExperimentId", + "pipeline_type": "seurat", + }, + Object { + "execution_arn": "mockExecutionArn", + "params_hash": "mockParamsHash", + "state_machine_arn": "mockStateMachineArn", + }, +] +`; + +exports[`createSeuratObjectPipeline works correctly 2`] = ` +Array [ + "mockExperimentId", + Object { + "authJWT": "mockAuthJWT", + "experimentName": "asdsadsada", + "input": Object { + "type": "seurat", + }, + "organism": null, + "projectId": "mockExperimentId", + "sampleIds": Array [ + "fc68aefc-c3ca-467f-8589-f1dbaaac1c1e", + ], + "sampleNames": Array [ + "scdata", + ], + "sampleS3Paths": Object { + "fc68aefc-c3ca-467f-8589-f1dbaaac1c1e": Object { + "seurat": "68f74995-3689-401a-90e0-145e08049cd5", + }, + }, + }, +] +`; + +exports[`seuratResponse works correctly 1`] = ` +Array [ + "ExperimentUpdates-mockExperimentId", + Object { + "experimentId": "mockExperimentId", + "input": Object {}, + "response": Object {}, + "status": Object { + "status": Object { + "seurat": Object { + "completedSteps": Array [ + "DownloadSeurat", + ], + "error": false, + "paramsHash": "mockParamsHash", + "startDate": "2022-05-10T16:30:16.268Z", + "status": "RUNNING", + "stopDate": null, + }, + }, + }, + "type": "seurat", + }, +] +`; diff --git a/tests/api.v2/helpers/pipeline/seurat.test.js b/tests/api.v2/helpers/pipeline/seurat.test.js new file mode 100644 index 000000000..841dfc3b2 --- /dev/null +++ b/tests/api.v2/helpers/pipeline/seurat.test.js @@ -0,0 +1,134 @@ +// @ts-nocheck +const io = require('socket.io-client'); + +const { createSeuratPipeline, handleSeuratResponse } = require('../../../../src/api.v2/helpers/pipeline/seurat'); + +const Experiment = require('../../../../src/api.v2/model/Experiment'); +const Sample = require('../../../../src/api.v2/model/Sample'); +const ExperimentExecution = require('../../../../src/api.v2/model/ExperimentExecution'); + +const pipelineConstruct = require('../../../../src/api.v2/helpers/pipeline/pipelineConstruct'); +const getPipelineStatus = require('../../../../src/api.v2/helpers/pipeline/getPipelineStatus'); +const HookRunner = require('../../../../src/api.v2/helpers/pipeline/hooks/HookRunner'); + +const validateRequest = require('../../../../src/utils/schema-validator'); + +const constants = require('../../../../src/api.v2/constants'); + +jest.mock('socket.io-client'); + +jest.mock('../../../../src/api.v2/model/Experiment'); +jest.mock('../../../../src/api.v2/model/Sample'); +jest.mock('../../../../src/api.v2/model/ExperimentExecution'); + +jest.mock('../../../../src/api.v2/helpers/pipeline/pipelineConstruct'); +jest.mock('../../../../src/api.v2/helpers/pipeline/getPipelineStatus'); +jest.mock('../../../../src/api.v2/helpers/pipeline/hooks/HookRunner'); + +jest.mock('../../../../src/utils/schema-validator'); + +const experimentInstance = Experiment(); +const sampleInstance = Sample(); +const experimentExecutionInstance = ExperimentExecution(); + +const hookRunnerInstance = HookRunner(); + +describe('createSeuratObjectPipeline', () => { + const experimentId = 'mockExperimentId'; + const paramsHash = 'mockParamsHash'; + const authJWT = 'mockAuthJWT'; + + const mockExperiment = { + id: '8e282f0d-aadb-8032-a334-982a371efd0f', + name: 'asdsadsada', + description: 'Analysis description', + samplesOrder: ['fc68aefc-c3ca-467f-8589-f1dbaaac1c1e'], + processingConfig: {}, + notifyByEmail: true, + createdAt: '2022-05-10 15:41:04.165961+00', + updatedAt: '2022-05-10 15:41:04.165961+00', + }; + + const mockSamples = [{ + id: 'fc68aefc-c3ca-467f-8589-f1dbaaac1c1e', + experimentId: '8e282f0d-aadb-8032-a334-982a371efd0f', + name: 'scdata', + sampleTechnology: 'seurat', + createdAt: '2022-05-10 15:41:10.057808+00', + updatedAt: '2022-05-10 15:41:10.057808+00', + metadata: { }, + files: { + seurat: { + size: 5079737, s3Path: '68f74995-3689-401a-90e0-145e08049cd5', uploadStatus: 'uploaded', sampleFileType: 'seurat', + }, + }, + }]; + + const mockStateMachineArn = 'mockStateMachineArn'; + const mockExecutionArn = 'mockExecutionArn'; + + beforeEach(() => { + experimentInstance.findById.mockClear(); + sampleInstance.getSamples.mockClear(); + experimentExecutionInstance.upsert.mockClear(); + pipelineConstruct.createSeuratObjectPipeline.mockClear(); + + experimentInstance.findById.mockReturnValueOnce({ + first: jest.fn(() => Promise.resolve(mockExperiment)), + }); + + sampleInstance.getSamples.mockReturnValueOnce(Promise.resolve(mockSamples)); + + pipelineConstruct.createSeuratObjectPipeline.mockReturnValueOnce( + { stateMachineArn: mockStateMachineArn, executionArn: mockExecutionArn }, + ); + }); + + it('works correctly', async () => { + await createSeuratPipeline(experimentId, { paramsHash }, authJWT); + + expect(experimentInstance.findById).toHaveBeenCalledWith(experimentId); + expect(sampleInstance.getSamples).toHaveBeenCalledWith(experimentId); + expect(experimentExecutionInstance.upsert.mock.calls[0]).toMatchSnapshot(); + expect(pipelineConstruct.createSeuratObjectPipeline.mock.calls[0]).toMatchSnapshot(); + }); +}); + +const mockGetPipelineStatusResponse = { + status: { + seurat: { + startDate: '2022-05-10T16:30:16.268Z', + stopDate: null, + status: 'RUNNING', + error: false, + completedSteps: ['DownloadSeurat'], + paramsHash: 'mockParamsHash', + }, + }, +}; + +describe('seuratResponse', () => { + const experimentId = 'mockExperimentId'; + const message = { + experimentId, authJWT: 'mockAuthJWT', input: { authJWT: 'mockAuthJWT' }, response: {}, + }; + + beforeEach(() => { + io.sockets = { emit: jest.fn() }; + + experimentInstance.updateById.mockClear(); + pipelineConstruct.createQCPipeline.mockClear(); + + getPipelineStatus.mockReturnValueOnce(mockGetPipelineStatusResponse); + }); + + it('works correctly', async () => { + await handleSeuratResponse(io, message); + + expect(validateRequest).toHaveBeenCalledWith(message, 'SeuratResponse.v2.yaml'); + expect(hookRunnerInstance.run).toHaveBeenCalledWith(message); + + expect(getPipelineStatus).toHaveBeenCalledWith(experimentId, constants.SEURAT_PROCESS_NAME); + expect(io.sockets.emit.mock.calls[0]).toMatchSnapshot(); + }); +}); From cfff3daf815493c7f5c388efe7b1734cb337837c Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Tue, 15 Nov 2022 09:39:11 -0800 Subject: [PATCH 24/26] add tests; rename completeMultiPartUpload --> completeMultipartUpload --- .../controllers/sampleFileController.js | 6 ++-- src/api.v2/helpers/s3/signedUrl.js | 4 +-- .../controllers/sampleFileController.test.js | 21 +++++++++++ .../s3/__snapshots__/signedUrl.test.js.snap | 35 +++++++++++++++++++ tests/api.v2/helpers/s3/signedUrl.test.js | 28 ++++++++++++++- 5 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index 086f6cbd6..c1534db1a 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -3,7 +3,7 @@ const sqlClient = require('../../sql/sqlClient'); const Sample = require('../model/Sample'); const SampleFile = require('../model/SampleFile'); -const { getSampleFileUploadUrls, getSampleFileDownloadUrl, completeMultiPartUpload } = require('../helpers/s3/signedUrl'); +const { getSampleFileUploadUrls, getSampleFileDownloadUrl, completeMultipartUpload } = require('../helpers/s3/signedUrl'); const { OK } = require('../../utils/responses'); const getLogger = require('../../utils/getLogger'); @@ -54,12 +54,12 @@ const patchFile = async (req, res) => { const completeMultipart = async (req, res) => { const { - body: { parts, uploadId, sampleFileId }, + body: { sampleFileId, parts, uploadId }, } = req; logger.log(`completing multipart upload for sampleFileId ${sampleFileId}`); - completeMultiPartUpload(sampleFileId, parts, uploadId); + completeMultipartUpload(sampleFileId, parts, uploadId); logger.log(`completed multipart upload for sampleFileId ${sampleFileId}`); res.json(OK()); diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index 9552d14c6..74cd93b34 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -64,7 +64,7 @@ const createMultipartUpload = async (params, size) => { }; -const completeMultiPartUpload = async (sampleFileId, parts, uploadId) => { +const completeMultipartUpload = async (sampleFileId, parts, uploadId) => { const params = { Bucket: bucketNames.SAMPLE_FILES, Key: `${sampleFileId}`, @@ -131,5 +131,5 @@ module.exports = { getSampleFileDownloadUrl, getSignedUrl, createMultipartUpload, - completeMultiPartUpload, + completeMultipartUpload, }; diff --git a/tests/api.v2/controllers/sampleFileController.test.js b/tests/api.v2/controllers/sampleFileController.test.js index a1f86dbfe..f417b7533 100644 --- a/tests/api.v2/controllers/sampleFileController.test.js +++ b/tests/api.v2/controllers/sampleFileController.test.js @@ -136,4 +136,25 @@ describe('sampleFileController', () => { // Response is generated signed url expect(mockRes.json).toHaveBeenCalledWith(signedUrlString); }); + + it('completeMultipart works correctly', async () => { + const sampleFileId = 'sampleFileId'; + const uploadId = 'uploadId'; + const parts = []; + + + const mockReq = { + body: { sampleFileId, parts, uploadId }, + }; + + signedUrl.completeMultipartUpload.mockImplementationOnce( + () => Promise.resolve(undefined), + ); + + await sampleFileController.completeMultipart(mockReq, mockRes); + + expect(signedUrl.completeMultipartUpload).toHaveBeenCalledWith( + sampleFileId, parts, uploadId, + ); + }); }); diff --git a/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap b/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap index 1507a347f..8b33c617e 100644 --- a/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap +++ b/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap @@ -1,5 +1,40 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`completeMultipartUpload works correctly 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + Object { + "Bucket": "biomage-originals-test-000000000000", + "Key": "mockSampleFileId", + "MultipartUpload": Object { + "Parts": Array [], + }, + "UploadId": "uploadId", + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "promise": [MockFunction] { + "calls": Array [ + Array [], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], + }, + }, + }, + ], +} +`; + exports[`getSampleFileDownloadUrl works correctly 1`] = ` [MockFunction] { "calls": Array [ diff --git a/tests/api.v2/helpers/s3/signedUrl.test.js b/tests/api.v2/helpers/s3/signedUrl.test.js index 0e2c0fa8b..c9a658fe7 100644 --- a/tests/api.v2/helpers/s3/signedUrl.test.js +++ b/tests/api.v2/helpers/s3/signedUrl.test.js @@ -5,7 +5,9 @@ const signedUrl = require('../../../../src/api.v2/helpers/s3/signedUrl'); const AWS = require('../../../../src/utils/requireAWS'); const { NotFoundError } = require('../../../../src/utils/responses'); -const { getSignedUrl, getSampleFileDownloadUrl, getSampleFileUploadUrls } = signedUrl; +const { + getSignedUrl, getSampleFileDownloadUrl, getSampleFileUploadUrls, completeMultipartUpload, +} = signedUrl; const sampleFileInstance = new SampleFile(); jest.mock('../../../../src/api.v2/model/SampleFile'); @@ -105,6 +107,30 @@ describe('getSampleFileUploadUrls', () => { }); }); +describe('completeMultipartUpload', () => { + const mockSampleFileId = 'mockSampleFileId'; + const mockParts = []; + const mockUploadId = 'uploadId'; + + const completeMultipartUploadSpy = jest.fn(); + + beforeEach(() => { + completeMultipartUploadSpy.mockReturnValue({ promise: jest.fn().mockReturnValue() }); + + AWS.S3.mockReset(); + AWS.S3.mockImplementation(() => ({ + completeMultipartUpload: completeMultipartUploadSpy, + })); + }); + + it('works correctly ', async () => { + const response = await completeMultipartUpload(mockSampleFileId, mockParts, mockUploadId); + + expect(response).toBeUndefined(); + expect(completeMultipartUploadSpy).toMatchSnapshot(); + }); +}); + describe('getSampleFileDownloadUrl', () => { const experimentId = 'mockExperimentId'; From e074cc2858cc525ba0bb008e78082f6fcd595fc6 Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Thu, 17 Nov 2022 09:59:18 -0800 Subject: [PATCH 25/26] fix notification emails for seurat; add tests --- .../pipeline/hooks/sendNotification.js | 6 +- .../pipeline/hooks/sendNotification.test.js | 70 ++++++++++++++++++- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/api.v2/helpers/pipeline/hooks/sendNotification.js b/src/api.v2/helpers/pipeline/hooks/sendNotification.js index facc4a7fe..bcc1ee3b9 100644 --- a/src/api.v2/helpers/pipeline/hooks/sendNotification.js +++ b/src/api.v2/helpers/pipeline/hooks/sendNotification.js @@ -1,7 +1,7 @@ const { authenticationMiddlewareSocketIO } = require('../../../middlewares/authMiddlewares'); const { - SUCCEEDED, FAILED, QC_PROCESS_NAME, + SUCCEEDED, FAILED, QC_PROCESS_NAME, SEURAT_PROCESS_NAME, } = require('../../../constants'); const getPipelineStatus = require('../getPipelineStatus'); @@ -42,8 +42,8 @@ const sendNotification = async (message) => { } if (experiment.notifyByEmail - && ((processName === QC_PROCESS_NAME && status === SUCCEEDED) - || status === FAILED)) { + && (([QC_PROCESS_NAME, SEURAT_PROCESS_NAME].includes(processName) && status === SUCCEEDED) + || status === FAILED)) { try { const emailParams = buildPipelineStatusEmailBody(message.experimentId, status, user); await sendEmail(emailParams); diff --git a/tests/api.v2/helpers/pipeline/hooks/sendNotification.test.js b/tests/api.v2/helpers/pipeline/hooks/sendNotification.test.js index 0502a1d33..1be2bdf65 100644 --- a/tests/api.v2/helpers/pipeline/hooks/sendNotification.test.js +++ b/tests/api.v2/helpers/pipeline/hooks/sendNotification.test.js @@ -32,6 +32,9 @@ const pipelines = { gem2s: { stateMachineArn: 'gem2sArn', }, + seurat: { + stateMachineArn: 'seuratArn', + }, }; describe('send-notification ', () => { @@ -65,7 +68,7 @@ describe('send-notification ', () => { expect(sendEmail).toHaveBeenCalledTimes(0); }); - it('Sends email and slack message if user toggled notifications on failed process', async () => { + it('Sends email and slack message if user toggled notifications on failed QC process', async () => { experimentInstance.getExperimentData.mockReturnValue({ notifyByEmail: true, pipelines }); const newMessage = { @@ -88,7 +91,30 @@ describe('send-notification ', () => { expect(sendFailedSlackMessage).toHaveBeenCalledTimes(1); }); - it('Sends email on success if toggled, does not send slack message ', async () => { + it('Sends email and slack message if user toggled notifications on failed Seurat process', async () => { + experimentInstance.getExperimentData.mockReturnValue({ notifyByEmail: true, pipelines }); + + const newMessage = { + ...message, + input: { + ...message.input, + processName: 'seurat', + }, + }; + + getPipelineStatus.mockReturnValue({ + seurat: { + status: FAILED, + }, + }); + + await sendNotification(newMessage); + + expect(sendEmail).toHaveBeenCalledTimes(1); + expect(sendFailedSlackMessage).toHaveBeenCalledTimes(1); + }); + + it('Sends email on QC success if toggled, does not send slack message ', async () => { experimentInstance.getExperimentData.mockReturnValue({ notifyByEmail: true, pipelines }); const newMessage = { ...message, @@ -107,7 +133,26 @@ describe('send-notification ', () => { expect(sendFailedSlackMessage).toHaveBeenCalledTimes(0); }); - it('Does not send email on success if user has not toggled notifications', async () => { + it('Sends email on Seurat success if toggled, does not send slack message ', async () => { + experimentInstance.getExperimentData.mockReturnValue({ notifyByEmail: true, pipelines }); + const newMessage = { + ...message, + input: { + ...message.input, + processName: 'seurat', + }, + }; + getPipelineStatus.mockReturnValue({ + seurat: { + status: SUCCEEDED, + }, + }); + await sendNotification(newMessage); + expect(sendEmail).toHaveBeenCalledTimes(1); + expect(sendFailedSlackMessage).toHaveBeenCalledTimes(0); + }); + + it('Does not send email on QC success if user has not toggled notifications', async () => { experimentInstance.getExperimentData.mockReturnValue({ notifyByEmail: false, pipelines }); const newMessage = { ...message, @@ -125,4 +170,23 @@ describe('send-notification ', () => { expect(sendEmail).toHaveBeenCalledTimes(0); expect(sendFailedSlackMessage).toHaveBeenCalledTimes(0); }); + + it('Does not send email on Seurat success if user has not toggled notifications', async () => { + experimentInstance.getExperimentData.mockReturnValue({ notifyByEmail: false, pipelines }); + const newMessage = { + ...message, + input: { + ...message.input, + processName: 'seurat', + }, + }; + getPipelineStatus.mockReturnValue({ + seurat: { + status: SUCCEEDED, + }, + }); + await sendNotification(newMessage); + expect(sendEmail).toHaveBeenCalledTimes(0); + expect(sendFailedSlackMessage).toHaveBeenCalledTimes(0); + }); }); From e1d2d23b205825addd2e09a19d9255f67b9c78ec Mon Sep 17 00:00:00 2001 From: Alex Pickering Date: Mon, 28 Nov 2022 10:25:06 -0800 Subject: [PATCH 26/26] fix returned file name --- src/api.v2/helpers/s3/signedUrl.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index 74cd93b34..ab6ef0b36 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -104,6 +104,7 @@ const fileNameToReturn = { matrix10x: 'matrix.mtx.gz', barcodes10x: 'barcodes.tsv.gz', features10x: 'features.tsv.gz', + seurat: 'r.rds', }; const getSampleFileDownloadUrl = async (experimentId, sampleId, fileType) => {