From e0970ac49b00098f996c2f76befd1015c94d0a31 Mon Sep 17 00:00:00 2001 From: stefanbabukov Date: Wed, 27 Sep 2023 12:47:08 +0100 Subject: [PATCH 1/8] cell level metadata added Signed-off-by: stefanbabukov --- src/api.v2/controllers/cellLevelController.js | 42 +++++++++++++++ .../controllers/experimentController.js | 2 - .../controllers/sampleFileController.js | 5 +- src/api.v2/helpers/s3/signedUrl.js | 8 +-- src/api.v2/model/CellLevel.js | 46 ++++++++++++++++ src/api.v2/model/CellLevelToExperiment.js | 16 ++++++ src/api.v2/model/Experiment.js | 29 +++++++---- src/api.v2/model/tableNames.js | 2 + src/api.v2/routes/cellLevel.js | 12 +++++ src/specs/api.v2.yaml | 52 +++++++++++++++++++ .../controllers/sampleFileController.test.js | 2 +- .../handleWorkRequest.test.js.snap | 30 +++++++++++ .../s3/__snapshots__/signedUrl.test.js.snap | 8 +-- tests/api.v2/helpers/s3/signedUrl.test.js | 9 ++-- tests/api.v2/model/Experiment.test.js | 2 +- 15 files changed, 237 insertions(+), 28 deletions(-) create mode 100644 src/api.v2/controllers/cellLevelController.js create mode 100644 src/api.v2/model/CellLevel.js create mode 100644 src/api.v2/model/CellLevelToExperiment.js create mode 100644 src/api.v2/routes/cellLevel.js diff --git a/src/api.v2/controllers/cellLevelController.js b/src/api.v2/controllers/cellLevelController.js new file mode 100644 index 000000000..d323a1bc3 --- /dev/null +++ b/src/api.v2/controllers/cellLevelController.js @@ -0,0 +1,42 @@ +const { v4: uuidv4 } = require('uuid'); +const bucketNames = require('../../config/bucketNames'); +const sqlClient = require('../../sql/sqlClient'); +const CellLevel = require('../model/CellLevel'); +const { getSignedUrl } = require('../helpers/s3/signedUrl'); + + +const uploadCellLevelMetadata = async (req, res) => { + const { experimentId } = req.params; + const { name, createdAt } = req.body; + + const cellLevelKey = uuidv4(); + const bucketName = bucketNames.CELL_METADATA; + const newCellLevelFile = { + id: cellLevelKey, + created_at: createdAt, + name, + upload_status: 'uploaded', + }; + const cellLevelToExperimentMap = { + experiment_id: experimentId, + cell_metadata_file_id: cellLevelKey, + }; + + let uploadUrl; + await sqlClient.get().transaction(async (trx) => { + await new CellLevel(trx).create(newCellLevelFile); + await new CellLevelToExperiment(trx).create(cellLevelToExperimentMap); + + uploadUrl = getSignedUrl('putObject', + { + Bucket: bucketName, + Key: cellLevelKey, + }); + }); + + res.json(uploadUrl); +}; + +module.exports = { + uploadCellLevelMetadata, +}; diff --git a/src/api.v2/controllers/experimentController.js b/src/api.v2/controllers/experimentController.js index 0138712ab..2b56e9c48 100644 --- a/src/api.v2/controllers/experimentController.js +++ b/src/api.v2/controllers/experimentController.js @@ -69,7 +69,6 @@ const getExampleExperiments = async (req, res) => { const getExperiment = async (req, res) => { const { params: { experimentId } } = req; logger.log(`Getting experiment ${experimentId}`); - const data = await new Experiment().getExperimentData(experimentId); logger.log(`Finished getting experiment ${experimentId}`); @@ -146,7 +145,6 @@ const getProcessingConfig = async (req, res) => { logger.log('Getting processing config for experiment ', experimentId); const result = await new Experiment().getProcessingConfig(experimentId); - logger.log('Finished getting processing config for experiment ', experimentId); res.json(result); }; diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index c1534db1a..853bc0684 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -2,8 +2,9 @@ const sqlClient = require('../../sql/sqlClient'); const Sample = require('../model/Sample'); const SampleFile = require('../model/SampleFile'); +const bucketNames = require('../../config/bucketNames'); -const { getSampleFileUploadUrls, getSampleFileDownloadUrl, completeMultipartUpload } = require('../helpers/s3/signedUrl'); +const { getFileUploadUrls, getSampleFileDownloadUrl, completeMultipartUpload } = require('../helpers/s3/signedUrl'); const { OK } = require('../../utils/responses'); const getLogger = require('../../utils/getLogger'); @@ -31,7 +32,7 @@ const createFile = async (req, res) => { await new Sample(trx).setNewFile(sampleId, sampleFileId, sampleFileType); logger.log(`Getting multipart upload urls for ${experimentId}, sample ${sampleId}, sampleFileType ${sampleFileType}`); - uploadUrlParams = await getSampleFileUploadUrls(sampleFileId, metadata, size); + uploadUrlParams = await getFileUploadUrls(sampleFileId, metadata, size, bucketNames.SAMPLE_FILES); }); diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index 0ebb4b318..49626368c 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -83,10 +83,10 @@ const completeMultipartUpload = async (sampleFileId, parts, uploadId) => { await s3.completeMultipartUpload(params).promise(); }; -const getSampleFileUploadUrls = async (sampleFileId, metadata, size) => { +const getFileUploadUrls = async (key, metadata, size, bucketName) => { const params = { - Bucket: bucketNames.SAMPLE_FILES, - Key: sampleFileId, + Bucket: bucketName, + Key: key, // 1 hour timeout of upload link Expires: 3600, }; @@ -130,7 +130,7 @@ const getSampleFileDownloadUrl = async (experimentId, sampleId, fileType) => { }; module.exports = { - getSampleFileUploadUrls, + getFileUploadUrls, getSampleFileDownloadUrl, getSignedUrl, createMultipartUpload, diff --git a/src/api.v2/model/CellLevel.js b/src/api.v2/model/CellLevel.js new file mode 100644 index 000000000..f8f019ba0 --- /dev/null +++ b/src/api.v2/model/CellLevel.js @@ -0,0 +1,46 @@ +const _ = require('lodash'); + +const BasicModel = require('./BasicModel'); +const sqlClient = require('../../sql/sqlClient'); +const tableNames = require('./tableNames'); +const getLogger = require('../../utils/getLogger'); + +const logger = getLogger(); +const fields = [ + 'id', + 'name', + 'upload_status', + 'created_at', +]; + +class CellLevel extends BasicModel { + constructor(sql = sqlClient.get()) { + super(sql, tableNames.CELL_LEVEL, fields); + } + + async getMetadataByExperimentId(experimentId) { + try { + const result = await this.sql + .select(fields) + .from(tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP) + .leftJoin( + tableNames.CELL_LEVEL, + `${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.cell_metadata_file_id`, + `${tableNames.CELL_LEVEL}.id`, + ) // Join with cell_metadata_file table + .where(`${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.experiment_id`, experimentId) + .first(); + + if (_.isEmpty(result)) { + logger.log(`No cell level metadata for experiment ${experimentId}`); + } + console.log('RESULT IS ', result); + return result; + } catch (error) { + console.error('Error fetching cell metadata file by experiment ID:', error); + throw error; + } + } +} + +module.exports = CellLevel; diff --git a/src/api.v2/model/CellLevelToExperiment.js b/src/api.v2/model/CellLevelToExperiment.js new file mode 100644 index 000000000..c05b60911 --- /dev/null +++ b/src/api.v2/model/CellLevelToExperiment.js @@ -0,0 +1,16 @@ +const BasicModel = require('./BasicModel'); +const sqlClient = require('../../sql/sqlClient'); +const tableNames = require('./tableNames'); + +const fields = [ + 'experiment_id', + 'cell_metadata_file_id', +]; + +class CellLevelToExperiment extends BasicModel { + constructor(sql = sqlClient.get()) { + super(sql, tableNames.CELL_LEVEL, fields); + } +} + +module.exports = CellLevelToExperiment; diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 10ae40f49..7e4379371 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -1,10 +1,11 @@ +/* eslint-disable no-param-reassign */ const _ = require('lodash'); const { v4: uuidv4 } = require('uuid'); const BasicModel = require('./BasicModel'); const sqlClient = require('../../sql/sqlClient'); const { collapseKeyIntoArray, replaceNullsWithObject } = require('../../sql/helpers'); - +const CellLevel = require('./CellLevel'); const { NotFoundError, BadRequestError } = require('../../utils/responses'); const tableNames = require('./tableNames'); const config = require('../../config'); @@ -52,13 +53,6 @@ class Experiment extends BasicModel { ...aliasedExperimentFields, 'm.key', 'p.parent_experiment_id', - /* - The parent_experiment_id could be null in cases where the - parent experiment has been deleted. - The existence of a row with the experiment_id in the experiment_parent - table after doing a leftJoin, - indicates that it's a subsetted experiment. - */ this.sql.raw('CASE WHEN p.experiment_id IS NOT NULL THEN true ELSE false END as is_subsetted'), ]) .from(tableNames.USER_ACCESS) @@ -68,7 +62,7 @@ class Experiment extends BasicModel { .leftJoin(`${tableNames.EXPERIMENT_PARENT} as p`, 'e.id', 'p.experiment_id') .as('mainQuery'); - const result = await collapseKeyIntoArray( + const experiments = await collapseKeyIntoArray( mainQuery, [...fields, 'parent_experiment_id', 'is_subsetted'], 'key', @@ -76,9 +70,24 @@ class Experiment extends BasicModel { this.sql, ); - return result; + // Assuming the getMetadataByExperimentId function is async and returns the metadata + const cellLevel = new CellLevel(); + + await Promise.all(experiments.map(async (experiment) => { + try { + const result = await cellLevel.getMetadataByExperimentId(experiment.id); + experiment.cellLevelMetadata = result || {}; + } catch (error) { + console.error('Error fetching metadata for experiment:', experiment.id, error); + experiment.cellLevelMetadata = {}; + } + })); + + console.log('RESULT IS ', experiments); + return experiments; } + async getExampleExperiments() { const fields = [ 'id', diff --git a/src/api.v2/model/tableNames.js b/src/api.v2/model/tableNames.js index f611a8486..87009ff08 100644 --- a/src/api.v2/model/tableNames.js +++ b/src/api.v2/model/tableNames.js @@ -10,6 +10,8 @@ const tableNames = { PLOT: 'plot', SAMPLE_IN_METADATA_TRACK_MAP: 'sample_in_metadata_track_map', SAMPLE_TO_SAMPLE_FILE_MAP: 'sample_to_sample_file_map', + CELL_LEVEL: 'cell_metadata_file', + CELL_LEVEL_TO_EXPERIMENT_MAP: 'cell_metadata_file_to_experiment', }; module.exports = tableNames; diff --git a/src/api.v2/routes/cellLevel.js b/src/api.v2/routes/cellLevel.js new file mode 100644 index 000000000..8e0b4c957 --- /dev/null +++ b/src/api.v2/routes/cellLevel.js @@ -0,0 +1,12 @@ +const { + uploadCellLevelMetadata, +} = require('../controllers/cellLevelController'); + +const { expressAuthorizationMiddleware } = require('../middlewares/authMiddlewares'); + +module.exports = { + 'cellLevel#uploadCellLevelMetadata': [ + expressAuthorizationMiddleware, + (req, res, next) => uploadCellLevelMetadata(req, res).catch(next), + ], +}; diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml index 503aea84a..ba0a44c3e 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1039,6 +1039,58 @@ paths: application/json: schema: $ref: "#/components/schemas/HTTPError" + "/experiments/{experimentId}/cellLevel": + post: + summary: Upload cell level metadata file + operationId: createCellLevelMetadataFromFile + x-eov-operation-id: cellLevel#uploadCellLevelMetadata + x-eov-operation-handler: routes/cellLevel + parameters: + - in: path + name: experimentId + required: true + schema: + type: string + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + name: + type: string + 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" "/experiments/{experimentId}/metadataTracks/{metadataTrackKey}": post: summary: Creates a new metadata track diff --git a/tests/api.v2/controllers/sampleFileController.test.js b/tests/api.v2/controllers/sampleFileController.test.js index 4729698f1..4b1b08015 100644 --- a/tests/api.v2/controllers/sampleFileController.test.js +++ b/tests/api.v2/controllers/sampleFileController.test.js @@ -28,7 +28,7 @@ describe('sampleFileController', () => { beforeEach(async () => { jest.clearAllMocks(); - signedUrl.getSampleFileUploadUrls.mockReturnValue(mockSignedUrls); + signedUrl.getFileUploadUrls.mockReturnValue(mockSignedUrls); }); it('createFile works correctly', async () => { diff --git a/tests/api.v2/events/__snapshots__/handleWorkRequest.test.js.snap b/tests/api.v2/events/__snapshots__/handleWorkRequest.test.js.snap index abd782f73..af4df6888 100644 --- a/tests/api.v2/events/__snapshots__/handleWorkRequest.test.js.snap +++ b/tests/api.v2/events/__snapshots__/handleWorkRequest.test.js.snap @@ -30,6 +30,36 @@ Array [ ] `; +exports[`Handle work socket callback If the worker fails, the API emits a work-response with error 3`] = ` +Array [ + "WorkResponse-etag-of-the-work-request", + Object { + "request": Object { + "Authorization": "Bearer some-token", + "ETag": "etag-of-the-work-request", + "experimentId": "experi-mentid-11111111-1111111-11111", + "uuid": "someuuid-asd-asd-asdddasa", + }, + "response": Object { + "cacheable": false, + "error": "fail", + }, + "results": Array [], + }, +] +`; + +exports[`Handle work socket callback If the worker fails, the API emits a work-response with error 4`] = ` +Array [ + Object { + "Authorization": "Bearer some-token", + "ETag": "etag-of-the-work-request", + "experimentId": "experi-mentid-11111111-1111111-11111", + "uuid": "someuuid-asd-asd-asdddasa", + }, +] +`; + exports[`Handle work socket callback data without authorization also throws an error 1`] = ` Array [ "WorkResponse-etag-of-the-work-request", 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 8b33c617e..4ce666db2 100644 --- a/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap +++ b/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap @@ -56,7 +56,7 @@ exports[`getSampleFileDownloadUrl works correctly 1`] = ` } `; -exports[`getSampleFileUploadUrls works correctly with metadata cellrangerVersion 1`] = ` +exports[`getFileUploadUrls works correctly with metadata cellrangerVersion 1`] = ` [MockFunction] { "calls": Array [ Array [ @@ -118,7 +118,7 @@ exports[`getSampleFileUploadUrls works correctly with metadata cellrangerVersion } `; -exports[`getSampleFileUploadUrls works correctly with metadata cellrangerVersion 2`] = ` +exports[`getFileUploadUrls works correctly with metadata cellrangerVersion 2`] = ` [MockFunction] { "calls": Array [ Array [ @@ -158,7 +158,7 @@ exports[`getSampleFileUploadUrls works correctly with metadata cellrangerVersion } `; -exports[`getSampleFileUploadUrls works correctly without metadata 1`] = ` +exports[`getFileUploadUrls works correctly without metadata 1`] = ` [MockFunction] { "calls": Array [ Array [ @@ -192,7 +192,7 @@ exports[`getSampleFileUploadUrls works correctly without metadata 1`] = ` } `; -exports[`getSampleFileUploadUrls works correctly without metadata 2`] = ` +exports[`getFileUploadUrls works correctly without metadata 2`] = ` [MockFunction] { "calls": Array [ Array [ diff --git a/tests/api.v2/helpers/s3/signedUrl.test.js b/tests/api.v2/helpers/s3/signedUrl.test.js index 47fa7a7de..910af33d4 100644 --- a/tests/api.v2/helpers/s3/signedUrl.test.js +++ b/tests/api.v2/helpers/s3/signedUrl.test.js @@ -1,12 +1,13 @@ // @ts-nocheck const SampleFile = require('../../../../src/api.v2/model/SampleFile'); const signedUrl = require('../../../../src/api.v2/helpers/s3/signedUrl'); +const bucketNames = require('../../../../src/config/bucketNames'); const AWS = require('../../../../src/utils/requireAWS'); const { NotFoundError } = require('../../../../src/utils/responses'); const { - getSignedUrl, getSampleFileDownloadUrl, getSampleFileUploadUrls, completeMultipartUpload, + getSignedUrl, getSampleFileDownloadUrl, getFileUploadUrls, completeMultipartUpload, } = signedUrl; const sampleFileInstance = new SampleFile(); @@ -67,7 +68,7 @@ describe('getSignedUrl', () => { }); }); -describe('getSampleFileUploadUrls', () => { +describe('getFileUploadUrls', () => { const mockSampleFileId = 'mockSampleFileId'; const signedUrlResponse = { signedUrls: ['signedUrl'], uploadId: 'uploadId' }; @@ -87,7 +88,7 @@ describe('getSampleFileUploadUrls', () => { }); it('works correctly without metadata', async () => { - const response = await getSampleFileUploadUrls(mockSampleFileId, {}, 1); + const response = await getFileUploadUrls(mockSampleFileId, {}, 1, bucketNames.SAMPLE_FILES); expect(response).toEqual(signedUrlResponse); expect(createMultipartUploadSpy).toMatchSnapshot(); @@ -95,7 +96,7 @@ describe('getSampleFileUploadUrls', () => { }); it('works correctly with metadata cellrangerVersion', async () => { - const response = await getSampleFileUploadUrls(mockSampleFileId, { cellrangerVersion: 'v2' }, 1); + const response = await getFileUploadUrls(mockSampleFileId, { cellrangerVersion: 'v2' }, 1, bucketNames.SAMPLE_FILES); expect(response).toEqual(signedUrlResponse); expect(createMultipartUploadSpy).toMatchSnapshot(); diff --git a/tests/api.v2/model/Experiment.test.js b/tests/api.v2/model/Experiment.test.js index 1fa02cec5..dff50fed1 100644 --- a/tests/api.v2/model/Experiment.test.js +++ b/tests/api.v2/model/Experiment.test.js @@ -44,7 +44,7 @@ describe('model/Experiment', () => { }); it('getAllExperiments works correctly', async () => { - const queryResult = 'result'; + const queryResult = ['result']; helpers.collapseKeyIntoArray.mockReturnValueOnce( Promise.resolve(queryResult), ); From 3abf9d4b9e4b04f0e5b4f5c289d53f25c4bd08ad Mon Sep 17 00:00:00 2001 From: stefanbabukov Date: Wed, 27 Sep 2023 16:23:08 +0100 Subject: [PATCH 2/8] fix Signed-off-by: stefanbabukov --- src/api.v2/controllers/cellLevelController.js | 3 ++- src/api.v2/model/CellLevel.js | 6 ------ src/api.v2/model/CellLevelToExperiment.js | 2 +- src/api.v2/model/Experiment.js | 10 ++-------- tests/api.v2/model/Experiment.test.js | 2 +- 5 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/api.v2/controllers/cellLevelController.js b/src/api.v2/controllers/cellLevelController.js index d323a1bc3..08f8122a6 100644 --- a/src/api.v2/controllers/cellLevelController.js +++ b/src/api.v2/controllers/cellLevelController.js @@ -2,6 +2,7 @@ const { v4: uuidv4 } = require('uuid'); const bucketNames = require('../../config/bucketNames'); const sqlClient = require('../../sql/sqlClient'); const CellLevel = require('../model/CellLevel'); +const CellLevelToExperiment = require('../model/CellLevelToExperiment'); const { getSignedUrl } = require('../helpers/s3/signedUrl'); @@ -27,7 +28,7 @@ const uploadCellLevelMetadata = async (req, res) => { await new CellLevel(trx).create(newCellLevelFile); await new CellLevelToExperiment(trx).create(cellLevelToExperimentMap); - uploadUrl = getSignedUrl('putObject', + uploadUrl = await getSignedUrl('putObject', { Bucket: bucketName, Key: cellLevelKey, diff --git a/src/api.v2/model/CellLevel.js b/src/api.v2/model/CellLevel.js index f8f019ba0..d3ab6635d 100644 --- a/src/api.v2/model/CellLevel.js +++ b/src/api.v2/model/CellLevel.js @@ -1,5 +1,3 @@ -const _ = require('lodash'); - const BasicModel = require('./BasicModel'); const sqlClient = require('../../sql/sqlClient'); const tableNames = require('./tableNames'); @@ -31,10 +29,6 @@ class CellLevel extends BasicModel { .where(`${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.experiment_id`, experimentId) .first(); - if (_.isEmpty(result)) { - logger.log(`No cell level metadata for experiment ${experimentId}`); - } - console.log('RESULT IS ', result); return result; } catch (error) { console.error('Error fetching cell metadata file by experiment ID:', error); diff --git a/src/api.v2/model/CellLevelToExperiment.js b/src/api.v2/model/CellLevelToExperiment.js index c05b60911..fb5d1af28 100644 --- a/src/api.v2/model/CellLevelToExperiment.js +++ b/src/api.v2/model/CellLevelToExperiment.js @@ -9,7 +9,7 @@ const fields = [ class CellLevelToExperiment extends BasicModel { constructor(sql = sqlClient.get()) { - super(sql, tableNames.CELL_LEVEL, fields); + super(sql, tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP, fields); } } diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 7e4379371..f5ff2474a 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -74,16 +74,10 @@ class Experiment extends BasicModel { const cellLevel = new CellLevel(); await Promise.all(experiments.map(async (experiment) => { - try { - const result = await cellLevel.getMetadataByExperimentId(experiment.id); - experiment.cellLevelMetadata = result || {}; - } catch (error) { - console.error('Error fetching metadata for experiment:', experiment.id, error); - experiment.cellLevelMetadata = {}; - } + const result = await cellLevel.getMetadataByExperimentId(experiment.id); + experiment.cellLevelMetadata = result; })); - console.log('RESULT IS ', experiments); return experiments; } diff --git a/tests/api.v2/model/Experiment.test.js b/tests/api.v2/model/Experiment.test.js index dff50fed1..6d9d18dfb 100644 --- a/tests/api.v2/model/Experiment.test.js +++ b/tests/api.v2/model/Experiment.test.js @@ -44,7 +44,7 @@ describe('model/Experiment', () => { }); it('getAllExperiments works correctly', async () => { - const queryResult = ['result']; + const queryResult = [{ id: 'some-id-experiment' }]; helpers.collapseKeyIntoArray.mockReturnValueOnce( Promise.resolve(queryResult), ); From e9d16bbfe96ebe9e1c4ab2142baed02c6815c111 Mon Sep 17 00:00:00 2001 From: stefanbabukov Date: Wed, 27 Sep 2023 16:39:57 +0100 Subject: [PATCH 3/8] update snapshots Signed-off-by: stefanbabukov --- .../handleWorkRequest.test.js.snap | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/tests/api.v2/events/__snapshots__/handleWorkRequest.test.js.snap b/tests/api.v2/events/__snapshots__/handleWorkRequest.test.js.snap index af4df6888..abd782f73 100644 --- a/tests/api.v2/events/__snapshots__/handleWorkRequest.test.js.snap +++ b/tests/api.v2/events/__snapshots__/handleWorkRequest.test.js.snap @@ -30,36 +30,6 @@ Array [ ] `; -exports[`Handle work socket callback If the worker fails, the API emits a work-response with error 3`] = ` -Array [ - "WorkResponse-etag-of-the-work-request", - Object { - "request": Object { - "Authorization": "Bearer some-token", - "ETag": "etag-of-the-work-request", - "experimentId": "experi-mentid-11111111-1111111-11111", - "uuid": "someuuid-asd-asd-asdddasa", - }, - "response": Object { - "cacheable": false, - "error": "fail", - }, - "results": Array [], - }, -] -`; - -exports[`Handle work socket callback If the worker fails, the API emits a work-response with error 4`] = ` -Array [ - Object { - "Authorization": "Bearer some-token", - "ETag": "etag-of-the-work-request", - "experimentId": "experi-mentid-11111111-1111111-11111", - "uuid": "someuuid-asd-asd-asdddasa", - }, -] -`; - exports[`Handle work socket callback data without authorization also throws an error 1`] = ` Array [ "WorkResponse-etag-of-the-work-request", From 65293d2e81e90a0df37e1d14fbae0a6b898438b4 Mon Sep 17 00:00:00 2001 From: stefanbabukov Date: Fri, 29 Sep 2023 12:41:23 +0100 Subject: [PATCH 4/8] comment fixes Signed-off-by: stefanbabukov --- src/api.v2/controllers/cellLevelController.js | 5 ++-- src/api.v2/model/CellLevel.js | 29 +++++++------------ src/api.v2/model/Experiment.js | 7 +++++ 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/api.v2/controllers/cellLevelController.js b/src/api.v2/controllers/cellLevelController.js index 08f8122a6..702e3f326 100644 --- a/src/api.v2/controllers/cellLevelController.js +++ b/src/api.v2/controllers/cellLevelController.js @@ -8,15 +8,14 @@ const { getSignedUrl } = require('../helpers/s3/signedUrl'); const uploadCellLevelMetadata = async (req, res) => { const { experimentId } = req.params; - const { name, createdAt } = req.body; + const { name } = req.body; const cellLevelKey = uuidv4(); const bucketName = bucketNames.CELL_METADATA; const newCellLevelFile = { id: cellLevelKey, - created_at: createdAt, name, - upload_status: 'uploaded', + upload_status: 'uploading', }; const cellLevelToExperimentMap = { experiment_id: experimentId, diff --git a/src/api.v2/model/CellLevel.js b/src/api.v2/model/CellLevel.js index d3ab6635d..47ea86cfa 100644 --- a/src/api.v2/model/CellLevel.js +++ b/src/api.v2/model/CellLevel.js @@ -1,9 +1,7 @@ const BasicModel = require('./BasicModel'); const sqlClient = require('../../sql/sqlClient'); const tableNames = require('./tableNames'); -const getLogger = require('../../utils/getLogger'); -const logger = getLogger(); const fields = [ 'id', 'name', @@ -17,23 +15,18 @@ class CellLevel extends BasicModel { } async getMetadataByExperimentId(experimentId) { - try { - const result = await this.sql - .select(fields) - .from(tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP) - .leftJoin( - tableNames.CELL_LEVEL, - `${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.cell_metadata_file_id`, - `${tableNames.CELL_LEVEL}.id`, - ) // Join with cell_metadata_file table - .where(`${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.experiment_id`, experimentId) - .first(); + const result = await this.sql + .select(fields) + .from(tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP) + .leftJoin( + tableNames.CELL_LEVEL, + `${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.cell_metadata_file_id`, + `${tableNames.CELL_LEVEL}.id`, + ) // Join with cell_metadata_file table + .where(`${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.experiment_id`, experimentId) + .first(); - return result; - } catch (error) { - console.error('Error fetching cell metadata file by experiment ID:', error); - throw error; - } + return result; } } diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index f5ff2474a..a28eab250 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -53,6 +53,13 @@ class Experiment extends BasicModel { ...aliasedExperimentFields, 'm.key', 'p.parent_experiment_id', + /* + The parent_experiment_id could be null in cases where the + parent experiment has been deleted. + The existence of a row with the experiment_id in the experiment_parent + table after doing a leftJoin, + indicates that it's a subsetted experiment. + */ this.sql.raw('CASE WHEN p.experiment_id IS NOT NULL THEN true ELSE false END as is_subsetted'), ]) .from(tableNames.USER_ACCESS) From dd5de06883d951115ac3ae6f86f913c5fd3d39f1 Mon Sep 17 00:00:00 2001 From: stefanbabukov Date: Fri, 29 Sep 2023 12:50:53 +0100 Subject: [PATCH 5/8] added a test Signed-off-by: stefanbabukov --- tests/api.v2/model/CellLevel.test.js | 36 ++++++++++++++++ .../__snapshots__/CellLevel.test.js.snap | 41 +++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 tests/api.v2/model/CellLevel.test.js create mode 100644 tests/api.v2/model/__snapshots__/CellLevel.test.js.snap diff --git a/tests/api.v2/model/CellLevel.test.js b/tests/api.v2/model/CellLevel.test.js new file mode 100644 index 000000000..ebece1b4e --- /dev/null +++ b/tests/api.v2/model/CellLevel.test.js @@ -0,0 +1,36 @@ +// @ts-nocheck +const { mockSqlClient } = require('../mocks/getMockSqlClient')(); +const CellLevel = require('../../../src/api.v2/model/CellLevel'); + +jest.mock('../../../src/sql/sqlClient', () => ({ + get: jest.fn(() => mockSqlClient), +})); + +describe('model/CellLevel', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('getMetadataByExperimentId retrieves data correctly', async () => { + const mockExperimentId = 'mockExperimentId'; + const mockResult = { + id: 'mockId', + name: 'mockName', + upload_status: 'mockStatus', + created_at: 'mockDate', + }; + + mockSqlClient.first.mockReturnValueOnce(Promise.resolve(mockResult)); + + const cellLevel = new CellLevel(); + const result = await cellLevel.getMetadataByExperimentId(mockExperimentId); + + expect(result).toEqual(mockResult); + + + expect(mockSqlClient.select.mock.calls).toMatchSnapshot(); + expect(mockSqlClient.from.mock.calls).toMatchSnapshot(); + expect(mockSqlClient.leftJoin.mock.calls).toMatchSnapshot(); + expect(mockSqlClient.where.mock.calls).toMatchSnapshot(); + }); +}); diff --git a/tests/api.v2/model/__snapshots__/CellLevel.test.js.snap b/tests/api.v2/model/__snapshots__/CellLevel.test.js.snap new file mode 100644 index 000000000..7128772a0 --- /dev/null +++ b/tests/api.v2/model/__snapshots__/CellLevel.test.js.snap @@ -0,0 +1,41 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`model/CellLevel getMetadataByExperimentId retrieves data correctly 1`] = ` +Array [ + Array [ + Array [ + "id", + "name", + "upload_status", + "created_at", + ], + ], +] +`; + +exports[`model/CellLevel getMetadataByExperimentId retrieves data correctly 2`] = ` +Array [ + Array [ + "cell_metadata_file_to_experiment", + ], +] +`; + +exports[`model/CellLevel getMetadataByExperimentId retrieves data correctly 3`] = ` +Array [ + Array [ + "cell_metadata_file", + "cell_metadata_file_to_experiment.cell_metadata_file_id", + "cell_metadata_file.id", + ], +] +`; + +exports[`model/CellLevel getMetadataByExperimentId retrieves data correctly 4`] = ` +Array [ + Array [ + "cell_metadata_file_to_experiment.experiment_id", + "mockExperimentId", + ], +] +`; From a5c5721c945ccf976f79e2bbce6bcd22e8d7ce15 Mon Sep 17 00:00:00 2001 From: stefanbabukov Date: Mon, 2 Oct 2023 15:04:43 +0100 Subject: [PATCH 6/8] using multipart upload for the cell level metadata Signed-off-by: stefanbabukov --- src/api.v2/controllers/cellLevelController.js | 28 +++++--- src/api.v2/controllers/s3Upload.js | 30 ++++++++ .../controllers/sampleFileController.js | 17 +---- src/api.v2/helpers/s3/signedUrl.js | 7 +- src/api.v2/model/CellLevel.js | 9 ++- src/api.v2/model/Experiment.js | 19 ++++-- src/api.v2/routes/cellLevel.js | 6 +- src/api.v2/routes/s3Upload.js | 7 ++ src/api.v2/routes/sampleFile.js | 5 +- src/specs/api.v2.yaml | 68 +++++++++++++++++-- .../experiment-bodies/ExperimentInfo.v2.yaml | 2 + .../experiment-bodies/ExperimentsList.v2.yaml | 2 + .../experiment-bodies/PatchExperiment.v2.yaml | 2 + .../controllers/sampleFileController.test.js | 10 ++- .../s3/__snapshots__/signedUrl.test.js.snap | 44 ++++++------ tests/api.v2/helpers/s3/signedUrl.test.js | 2 +- tests/api.v2/model/CellLevel.test.js | 6 +- .../__snapshots__/CellLevel.test.js.snap | 10 +-- tests/api.v2/routes/sampleFile.test.js | 24 ------- 19 files changed, 186 insertions(+), 112 deletions(-) create mode 100644 src/api.v2/controllers/s3Upload.js create mode 100644 src/api.v2/routes/s3Upload.js diff --git a/src/api.v2/controllers/cellLevelController.js b/src/api.v2/controllers/cellLevelController.js index 702e3f326..e75d3d2dc 100644 --- a/src/api.v2/controllers/cellLevelController.js +++ b/src/api.v2/controllers/cellLevelController.js @@ -1,14 +1,15 @@ const { v4: uuidv4 } = require('uuid'); +const _ = require('lodash'); const bucketNames = require('../../config/bucketNames'); const sqlClient = require('../../sql/sqlClient'); const CellLevel = require('../model/CellLevel'); const CellLevelToExperiment = require('../model/CellLevelToExperiment'); -const { getSignedUrl } = require('../helpers/s3/signedUrl'); - +const { getFileUploadUrls } = require('../helpers/s3/signedUrl'); +const OK = require('../../utils/responses/OK'); const uploadCellLevelMetadata = async (req, res) => { const { experimentId } = req.params; - const { name } = req.body; + const { name, size } = req.body; const cellLevelKey = uuidv4(); const bucketName = bucketNames.CELL_METADATA; @@ -22,21 +23,26 @@ const uploadCellLevelMetadata = async (req, res) => { cell_metadata_file_id: cellLevelKey, }; - let uploadUrl; + let uploadUrlParams; await sqlClient.get().transaction(async (trx) => { await new CellLevel(trx).create(newCellLevelFile); await new CellLevelToExperiment(trx).create(cellLevelToExperimentMap); - - uploadUrl = await getSignedUrl('putObject', - { - Bucket: bucketName, - Key: cellLevelKey, - }); + uploadUrlParams = await getFileUploadUrls(cellLevelKey, {}, size, bucketName); + uploadUrlParams = { ...uploadUrlParams, fileId: cellLevelKey }; }); - res.json(uploadUrl); + res.json({ data: uploadUrlParams }); }; + +const updateCellLevelMetadata = async (req, res) => { + const { params: { experimentId }, body } = req; + const snakeCasedKeysToPatch = _.mapKeys(body, (_value, key) => _.snakeCase(key)); + const { cellMetadataFileId } = await new CellLevelToExperiment().find({ experiment_id: experimentId }).first(); + await new CellLevel().updateById(cellMetadataFileId, snakeCasedKeysToPatch); + res.json(OK()); +}; module.exports = { uploadCellLevelMetadata, + updateCellLevelMetadata, }; diff --git a/src/api.v2/controllers/s3Upload.js b/src/api.v2/controllers/s3Upload.js new file mode 100644 index 000000000..fbf507794 --- /dev/null +++ b/src/api.v2/controllers/s3Upload.js @@ -0,0 +1,30 @@ +const { completeMultipartUpload } = require('../helpers/s3/signedUrl'); + +const bucketNames = require('../../config/bucketNames'); +const getLogger = require('../../utils/getLogger'); +const { OK, NotFoundError } = require('../../utils/responses'); + +const logger = getLogger(); +const completeMultipartUploads = async (req, res) => { + const { + fileId, uploadId, parts, type, + } = req.body; + let bucketName; + if (type === 'sample') { + bucketName = bucketNames.SAMPLE_FILES; + } else if (type === 'cellLevel') { + bucketName = bucketNames.CELL_METADATA; + } else { + throw new NotFoundError('Invalid bucket specified'); + } + logger.log(`completing multipart upload for file ${fileId}`); + + await completeMultipartUpload(fileId, parts, uploadId, bucketName); + + logger.log(`completed multipart upload for file ${fileId}`); + res.json(OK()); +}; + +module.exports = { + completeMultipartUploads, +}; diff --git a/src/api.v2/controllers/sampleFileController.js b/src/api.v2/controllers/sampleFileController.js index 853bc0684..0d372e751 100644 --- a/src/api.v2/controllers/sampleFileController.js +++ b/src/api.v2/controllers/sampleFileController.js @@ -4,7 +4,7 @@ const Sample = require('../model/Sample'); const SampleFile = require('../model/SampleFile'); const bucketNames = require('../../config/bucketNames'); -const { getFileUploadUrls, getSampleFileDownloadUrl, completeMultipartUpload } = require('../helpers/s3/signedUrl'); +const { getFileUploadUrls, getSampleFileDownloadUrl } = require('../helpers/s3/signedUrl'); const { OK } = require('../../utils/responses'); const getLogger = require('../../utils/getLogger'); @@ -53,19 +53,6 @@ const patchFile = async (req, res) => { res.json(OK()); }; -const completeMultipart = async (req, res) => { - const { - body: { sampleFileId, parts, uploadId }, - } = req; - - logger.log(`completing multipart upload for sampleFileId ${sampleFileId}`); - - completeMultipartUpload(sampleFileId, parts, uploadId); - - logger.log(`completed multipart upload for sampleFileId ${sampleFileId}`); - res.json(OK()); -}; - const getS3DownloadUrl = async (req, res) => { const { experimentId, sampleId, sampleFileType } = req.params; @@ -79,5 +66,5 @@ const getS3DownloadUrl = async (req, res) => { }; module.exports = { - createFile, patchFile, getS3DownloadUrl, completeMultipart, + createFile, patchFile, getS3DownloadUrl, }; diff --git a/src/api.v2/helpers/s3/signedUrl.js b/src/api.v2/helpers/s3/signedUrl.js index 49626368c..bcaecf1aa 100644 --- a/src/api.v2/helpers/s3/signedUrl.js +++ b/src/api.v2/helpers/s3/signedUrl.js @@ -64,15 +64,14 @@ const createMultipartUpload = async (params, size) => { }; -const completeMultipartUpload = async (sampleFileId, parts, uploadId) => { +const completeMultipartUpload = async (key, parts, uploadId, bucketName) => { const params = { - Bucket: bucketNames.SAMPLE_FILES, - Key: `${sampleFileId}`, + Bucket: bucketName, + Key: key, UploadId: uploadId, MultipartUpload: { Parts: parts }, }; - const S3Config = { apiVersion: '2006-03-01', signatureVersion: 'v4', diff --git a/src/api.v2/model/CellLevel.js b/src/api.v2/model/CellLevel.js index 47ea86cfa..98d532700 100644 --- a/src/api.v2/model/CellLevel.js +++ b/src/api.v2/model/CellLevel.js @@ -14,17 +14,16 @@ class CellLevel extends BasicModel { super(sql, tableNames.CELL_LEVEL, fields); } - async getMetadataByExperimentId(experimentId) { + async getMetadataByExperimentIds(experimentIds) { const result = await this.sql - .select(fields) + .select([...fields, `${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.experiment_id`]) .from(tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP) .leftJoin( tableNames.CELL_LEVEL, `${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.cell_metadata_file_id`, `${tableNames.CELL_LEVEL}.id`, - ) // Join with cell_metadata_file table - .where(`${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.experiment_id`, experimentId) - .first(); + ) + .whereIn(`${tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP}.experiment_id`, experimentIds); return result; } diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index a28eab250..feef9cafd 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -79,12 +79,19 @@ class Experiment extends BasicModel { // Assuming the getMetadataByExperimentId function is async and returns the metadata const cellLevel = new CellLevel(); - - await Promise.all(experiments.map(async (experiment) => { - const result = await cellLevel.getMetadataByExperimentId(experiment.id); - experiment.cellLevelMetadata = result; - })); - + const experimentIds = experiments.map((experiment) => experiment.id); + const cellLevelResults = await cellLevel.getMetadataByExperimentIds(experimentIds); + + if (cellLevelResults.length) { + cellLevelResults.forEach( + (cellLevelMetaResult) => { + const experimentIndx = experiments.findIndex( + (experiment) => experiment.id === cellLevelMetaResult.experimentId, + ); + experiments[experimentIndx].cellLevelMetadata = cellLevelMetaResult; + }, + ); + } return experiments; } diff --git a/src/api.v2/routes/cellLevel.js b/src/api.v2/routes/cellLevel.js index 8e0b4c957..5cd1edae0 100644 --- a/src/api.v2/routes/cellLevel.js +++ b/src/api.v2/routes/cellLevel.js @@ -1,5 +1,5 @@ const { - uploadCellLevelMetadata, + uploadCellLevelMetadata, updateCellLevelMetadata, } = require('../controllers/cellLevelController'); const { expressAuthorizationMiddleware } = require('../middlewares/authMiddlewares'); @@ -9,4 +9,8 @@ module.exports = { expressAuthorizationMiddleware, (req, res, next) => uploadCellLevelMetadata(req, res).catch(next), ], + 'cellLevel#updateCellLevelMetadata': [ + expressAuthorizationMiddleware, + (req, res, next) => updateCellLevelMetadata(req, res).catch(next), + ], }; diff --git a/src/api.v2/routes/s3Upload.js b/src/api.v2/routes/s3Upload.js new file mode 100644 index 000000000..d70f60e99 --- /dev/null +++ b/src/api.v2/routes/s3Upload.js @@ -0,0 +1,7 @@ +const { completeMultipartUploads } = require('../controllers/s3Upload'); + +module.exports = { + 's3Upload#completeMultipartUpload': [ + (req, res, next) => completeMultipartUploads(req, res).catch(next), + ], +}; diff --git a/src/api.v2/routes/sampleFile.js b/src/api.v2/routes/sampleFile.js index 201d4540f..552b67141 100644 --- a/src/api.v2/routes/sampleFile.js +++ b/src/api.v2/routes/sampleFile.js @@ -1,5 +1,5 @@ const { - createFile, patchFile, getS3DownloadUrl, completeMultipart, + createFile, patchFile, getS3DownloadUrl, } = require('../controllers/sampleFileController'); const { expressAuthorizationMiddleware } = require('../middlewares/authMiddlewares'); @@ -13,9 +13,6 @@ 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 ba0a44c3e..ba38ea4d0 100644 --- a/src/specs/api.v2.yaml +++ b/src/specs/api.v2.yaml @@ -1060,6 +1060,8 @@ paths: properties: name: type: string + size: + type: integer responses: "200": description: Success @@ -1091,6 +1093,61 @@ paths: application/json: schema: $ref: "#/components/schemas/HTTPError" + patch: + summary: Patch the cell level metadata of an experiment + description: Update the cell level metadata of an experiment + operationId: patchCellLevelMetadata + x-eov-operation-id: cellLevel#updateCellLevelMetadata + x-eov-operation-handler: routes/cellLevel + parameters: + - name: experimentId + schema: + type: string + in: path + required: true + description: ID of the experiment + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/PatchSampleFile" + responses: + "200": + description: patch cell level metadata + 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}/metadataTracks/{metadataTrackKey}": post: summary: Creates a new metadata track @@ -1485,8 +1542,8 @@ paths: 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 + x-eov-operation-id: s3Upload#completeMultipartUpload + x-eov-operation-handler: routes/s3Upload requestBody: content: application/json: @@ -1494,7 +1551,7 @@ paths: description: information here type: object properties: - sampleFileId: + fileId: type: string uploadId: type: string @@ -1502,11 +1559,14 @@ paths: type: array items: type: object + type: + type: string additionalProperties: false required: - - sampleFileId + - fileId - uploadId - parts + - type responses: "200": description: Success diff --git a/src/specs/models/experiment-bodies/ExperimentInfo.v2.yaml b/src/specs/models/experiment-bodies/ExperimentInfo.v2.yaml index a60239e9d..68b240658 100644 --- a/src/specs/models/experiment-bodies/ExperimentInfo.v2.yaml +++ b/src/specs/models/experiment-bodies/ExperimentInfo.v2.yaml @@ -7,6 +7,8 @@ properties: type: string description: type: string + cellLevelMetadata: + type: object samplesOrder: type: array items: diff --git a/src/specs/models/experiment-bodies/ExperimentsList.v2.yaml b/src/specs/models/experiment-bodies/ExperimentsList.v2.yaml index d3e220520..35336a1e9 100644 --- a/src/specs/models/experiment-bodies/ExperimentsList.v2.yaml +++ b/src/specs/models/experiment-bodies/ExperimentsList.v2.yaml @@ -9,6 +9,8 @@ items: type: string description: type: string + cellLevelMetadata: + type: object notifyByEmail: type: boolean pipelineVersion: diff --git a/src/specs/models/experiment-bodies/PatchExperiment.v2.yaml b/src/specs/models/experiment-bodies/PatchExperiment.v2.yaml index a99124eb1..9ba19e36f 100644 --- a/src/specs/models/experiment-bodies/PatchExperiment.v2.yaml +++ b/src/specs/models/experiment-bodies/PatchExperiment.v2.yaml @@ -8,5 +8,7 @@ properties: type: string notifyByEmail: type: boolean + cellLevelMetadata: + type: object additionalProperties: false diff --git a/tests/api.v2/controllers/sampleFileController.test.js b/tests/api.v2/controllers/sampleFileController.test.js index 4b1b08015..aa96899b5 100644 --- a/tests/api.v2/controllers/sampleFileController.test.js +++ b/tests/api.v2/controllers/sampleFileController.test.js @@ -2,11 +2,13 @@ const Sample = require('../../../src/api.v2/model/Sample'); const SampleFile = require('../../../src/api.v2/model/SampleFile'); const { mockSqlClient, mockTrx } = require('../mocks/getMockSqlClient')(); +const bucketNames = require('../../../src/config/bucketNames'); const sampleInstance = new Sample(); const sampleFileInstance = new SampleFile(); const sampleFileController = require('../../../src/api.v2/controllers/sampleFileController'); +const s3UploadController = require('../../../src/api.v2/controllers/s3Upload'); const { OK } = require('../../../src/utils/responses'); const signedUrl = require('../../../src/api.v2/helpers/s3/signedUrl'); @@ -143,17 +145,19 @@ describe('sampleFileController', () => { const mockReq = { - body: { sampleFileId, parts, uploadId }, + body: { + fileId: sampleFileId, parts, uploadId, type: 'sample', + }, }; signedUrl.completeMultipartUpload.mockImplementationOnce( () => Promise.resolve(undefined), ); - await sampleFileController.completeMultipart(mockReq, mockRes); + await s3UploadController.completeMultipartUploads(mockReq, mockRes); expect(signedUrl.completeMultipartUpload).toHaveBeenCalledWith( - sampleFileId, parts, uploadId, + sampleFileId, parts, uploadId, bucketNames.SAMPLE_FILES, ); }); }); 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 4ce666db2..38032893d 100644 --- a/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap +++ b/tests/api.v2/helpers/s3/__snapshots__/signedUrl.test.js.snap @@ -5,7 +5,7 @@ exports[`completeMultipartUpload works correctly 1`] = ` "calls": Array [ Array [ Object { - "Bucket": "biomage-originals-test-000000000000", + "Bucket": "some-bucket", "Key": "mockSampleFileId", "MultipartUpload": Object { "Parts": Array [], @@ -35,27 +35,6 @@ exports[`completeMultipartUpload works correctly 1`] = ` } `; -exports[`getSampleFileDownloadUrl works correctly 1`] = ` -[MockFunction] { - "calls": Array [ - Array [ - "getObject", - Object { - "Bucket": "biomage-originals-test-000000000000", - "Key": "123", - "ResponseContentDisposition": "attachment; filename=\\"features.tsv.gz\\"", - }, - ], - ], - "results": Array [ - Object { - "type": "return", - "value": "signedUrl", - }, - ], -} -`; - exports[`getFileUploadUrls works correctly with metadata cellrangerVersion 1`] = ` [MockFunction] { "calls": Array [ @@ -214,3 +193,24 @@ exports[`getFileUploadUrls works correctly without metadata 2`] = ` ], } `; + +exports[`getSampleFileDownloadUrl works correctly 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "getObject", + Object { + "Bucket": "biomage-originals-test-000000000000", + "Key": "123", + "ResponseContentDisposition": "attachment; filename=\\"features.tsv.gz\\"", + }, + ], + ], + "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 910af33d4..aef38f59c 100644 --- a/tests/api.v2/helpers/s3/signedUrl.test.js +++ b/tests/api.v2/helpers/s3/signedUrl.test.js @@ -121,7 +121,7 @@ describe('completeMultipartUpload', () => { }); it('works correctly ', async () => { - const response = await completeMultipartUpload(mockSampleFileId, mockParts, mockUploadId); + const response = await completeMultipartUpload(mockSampleFileId, mockParts, mockUploadId, 'some-bucket'); expect(response).toBeUndefined(); expect(completeMultipartUploadSpy).toMatchSnapshot(); diff --git a/tests/api.v2/model/CellLevel.test.js b/tests/api.v2/model/CellLevel.test.js index ebece1b4e..5db81ebdb 100644 --- a/tests/api.v2/model/CellLevel.test.js +++ b/tests/api.v2/model/CellLevel.test.js @@ -20,12 +20,10 @@ describe('model/CellLevel', () => { created_at: 'mockDate', }; - mockSqlClient.first.mockReturnValueOnce(Promise.resolve(mockResult)); + mockSqlClient.mockReturnValueOnce(Promise.resolve([mockResult])); const cellLevel = new CellLevel(); - const result = await cellLevel.getMetadataByExperimentId(mockExperimentId); - - expect(result).toEqual(mockResult); + await cellLevel.getMetadataByExperimentIds([mockExperimentId]); expect(mockSqlClient.select.mock.calls).toMatchSnapshot(); diff --git a/tests/api.v2/model/__snapshots__/CellLevel.test.js.snap b/tests/api.v2/model/__snapshots__/CellLevel.test.js.snap index 7128772a0..89532130d 100644 --- a/tests/api.v2/model/__snapshots__/CellLevel.test.js.snap +++ b/tests/api.v2/model/__snapshots__/CellLevel.test.js.snap @@ -8,6 +8,7 @@ Array [ "name", "upload_status", "created_at", + "cell_metadata_file_to_experiment.experiment_id", ], ], ] @@ -31,11 +32,4 @@ Array [ ] `; -exports[`model/CellLevel getMetadataByExperimentId retrieves data correctly 4`] = ` -Array [ - Array [ - "cell_metadata_file_to_experiment.experiment_id", - "mockExperimentId", - ], -] -`; +exports[`model/CellLevel getMetadataByExperimentId retrieves data correctly 4`] = `Array []`; diff --git a/tests/api.v2/routes/sampleFile.test.js b/tests/api.v2/routes/sampleFile.test.js index 7c388103c..75dcafecb 100644 --- a/tests/api.v2/routes/sampleFile.test.js +++ b/tests/api.v2/routes/sampleFile.test.js @@ -168,30 +168,6 @@ 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()); From 000b8cb881e34140d5fe7d7ce34029d6d25c1070 Mon Sep 17 00:00:00 2001 From: stefanbabukov Date: Tue, 3 Oct 2023 15:54:34 +0100 Subject: [PATCH 7/8] fix Signed-off-by: stefanbabukov --- src/api.v2/model/Experiment.js | 20 +++++++++----------- src/api.v2/model/__mocks__/CellLevel.js | 10 ++++++++++ tests/api.v2/model/Experiment.test.js | 2 +- 3 files changed, 20 insertions(+), 12 deletions(-) create mode 100644 src/api.v2/model/__mocks__/CellLevel.js diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index feef9cafd..801f454d1 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -77,21 +77,19 @@ class Experiment extends BasicModel { this.sql, ); - // Assuming the getMetadataByExperimentId function is async and returns the metadata const cellLevel = new CellLevel(); const experimentIds = experiments.map((experiment) => experiment.id); const cellLevelResults = await cellLevel.getMetadataByExperimentIds(experimentIds); - if (cellLevelResults.length) { - cellLevelResults.forEach( - (cellLevelMetaResult) => { - const experimentIndx = experiments.findIndex( - (experiment) => experiment.id === cellLevelMetaResult.experimentId, - ); - experiments[experimentIndx].cellLevelMetadata = cellLevelMetaResult; - }, - ); - } + cellLevelResults.forEach( + (cellLevelMetaResult) => { + const experimentIndx = experiments.findIndex( + (experiment) => experiment.id === cellLevelMetaResult.experimentId, + ); + experiments[experimentIndx].cellLevelMetadata = cellLevelMetaResult; + }, + ); + return experiments; } diff --git a/src/api.v2/model/__mocks__/CellLevel.js b/src/api.v2/model/__mocks__/CellLevel.js new file mode 100644 index 000000000..c9e4fdcbc --- /dev/null +++ b/src/api.v2/model/__mocks__/CellLevel.js @@ -0,0 +1,10 @@ +const BasicModel = require('./BasicModel')(); + +const stub = { + getMetadataByExperimentIds: () => ([]), + ...BasicModel, +}; + +const CellLevel = jest.fn().mockImplementation(() => stub); + +module.exports = CellLevel; diff --git a/tests/api.v2/model/Experiment.test.js b/tests/api.v2/model/Experiment.test.js index 6d9d18dfb..9f4469aac 100644 --- a/tests/api.v2/model/Experiment.test.js +++ b/tests/api.v2/model/Experiment.test.js @@ -29,7 +29,7 @@ jest.mock('../../../src/sql/helpers', () => ({ '{}'::jsonb ) as pipelines,}),`), })); - +jest.mock('../../../src/api.v2/model/CellLevel'); const Experiment = require('../../../src/api.v2/model/Experiment'); const constants = require('../../../src/utils/constants'); const tableNames = require('../../../src/api.v2/model/tableNames'); From c586e599e5a84f5dfa2ff57f5696f00c57e28c41 Mon Sep 17 00:00:00 2001 From: stefanbabukov Date: Wed, 4 Oct 2023 19:51:08 +0100 Subject: [PATCH 8/8] changed cellLevel to cellLevelMeta Signed-off-by: stefanbabukov --- ...ntroller.js => cellLevelMetaController.js} | 26 +++++++++---------- .../model/{CellLevel.js => CellLevelMeta.js} | 4 +-- ...riment.js => CellLevelMetaToExperiment.js} | 4 +-- src/api.v2/model/Experiment.js | 8 +++--- .../{CellLevel.js => CellLevelMeta.js} | 4 +-- src/api.v2/routes/cellLevel.js | 2 +- ...ellLevel.test.js => CellLevelMeta.test.js} | 2 +- tests/api.v2/model/Experiment.test.js | 2 +- ...est.js.snap => CellLevelMeta.test.js.snap} | 0 9 files changed, 26 insertions(+), 26 deletions(-) rename src/api.v2/controllers/{cellLevelController.js => cellLevelMetaController.js} (53%) rename src/api.v2/model/{CellLevel.js => CellLevelMeta.js} (91%) rename src/api.v2/model/{CellLevelToExperiment.js => CellLevelMetaToExperiment.js} (76%) rename src/api.v2/model/__mocks__/{CellLevel.js => CellLevelMeta.js} (56%) rename tests/api.v2/model/{CellLevel.test.js => CellLevelMeta.test.js} (93%) rename tests/api.v2/model/__snapshots__/{CellLevel.test.js.snap => CellLevelMeta.test.js.snap} (100%) diff --git a/src/api.v2/controllers/cellLevelController.js b/src/api.v2/controllers/cellLevelMetaController.js similarity index 53% rename from src/api.v2/controllers/cellLevelController.js rename to src/api.v2/controllers/cellLevelMetaController.js index e75d3d2dc..78075004d 100644 --- a/src/api.v2/controllers/cellLevelController.js +++ b/src/api.v2/controllers/cellLevelMetaController.js @@ -2,8 +2,8 @@ const { v4: uuidv4 } = require('uuid'); const _ = require('lodash'); const bucketNames = require('../../config/bucketNames'); const sqlClient = require('../../sql/sqlClient'); -const CellLevel = require('../model/CellLevel'); -const CellLevelToExperiment = require('../model/CellLevelToExperiment'); +const CellLevelMeta = require('../model/CellLevelMeta'); +const CellLevelMetaToExperiment = require('../model/CellLevelMetaToExperiment'); const { getFileUploadUrls } = require('../helpers/s3/signedUrl'); const OK = require('../../utils/responses/OK'); @@ -11,24 +11,24 @@ const uploadCellLevelMetadata = async (req, res) => { const { experimentId } = req.params; const { name, size } = req.body; - const cellLevelKey = uuidv4(); + const cellLevelMetaKey = uuidv4(); const bucketName = bucketNames.CELL_METADATA; - const newCellLevelFile = { - id: cellLevelKey, + const newCellLevelMetaFile = { + id: cellLevelMetaKey, name, upload_status: 'uploading', }; - const cellLevelToExperimentMap = { + const cellLevelMetaToExperimentMap = { experiment_id: experimentId, - cell_metadata_file_id: cellLevelKey, + cell_metadata_file_id: cellLevelMetaKey, }; let uploadUrlParams; await sqlClient.get().transaction(async (trx) => { - await new CellLevel(trx).create(newCellLevelFile); - await new CellLevelToExperiment(trx).create(cellLevelToExperimentMap); - uploadUrlParams = await getFileUploadUrls(cellLevelKey, {}, size, bucketName); - uploadUrlParams = { ...uploadUrlParams, fileId: cellLevelKey }; + await new CellLevelMeta(trx).create(newCellLevelMetaFile); + await new CellLevelMetaToExperiment(trx).create(cellLevelMetaToExperimentMap); + uploadUrlParams = await getFileUploadUrls(cellLevelMetaKey, {}, size, bucketName); + uploadUrlParams = { ...uploadUrlParams, fileId: cellLevelMetaKey }; }); res.json({ data: uploadUrlParams }); @@ -38,8 +38,8 @@ const uploadCellLevelMetadata = async (req, res) => { const updateCellLevelMetadata = async (req, res) => { const { params: { experimentId }, body } = req; const snakeCasedKeysToPatch = _.mapKeys(body, (_value, key) => _.snakeCase(key)); - const { cellMetadataFileId } = await new CellLevelToExperiment().find({ experiment_id: experimentId }).first(); - await new CellLevel().updateById(cellMetadataFileId, snakeCasedKeysToPatch); + const { cellMetadataFileId } = await new CellLevelMetaToExperiment().find({ experiment_id: experimentId }).first(); + await new CellLevelMeta().updateById(cellMetadataFileId, snakeCasedKeysToPatch); res.json(OK()); }; module.exports = { diff --git a/src/api.v2/model/CellLevel.js b/src/api.v2/model/CellLevelMeta.js similarity index 91% rename from src/api.v2/model/CellLevel.js rename to src/api.v2/model/CellLevelMeta.js index 98d532700..8dd340a92 100644 --- a/src/api.v2/model/CellLevel.js +++ b/src/api.v2/model/CellLevelMeta.js @@ -9,7 +9,7 @@ const fields = [ 'created_at', ]; -class CellLevel extends BasicModel { +class CellLevelMeta extends BasicModel { constructor(sql = sqlClient.get()) { super(sql, tableNames.CELL_LEVEL, fields); } @@ -29,4 +29,4 @@ class CellLevel extends BasicModel { } } -module.exports = CellLevel; +module.exports = CellLevelMeta; diff --git a/src/api.v2/model/CellLevelToExperiment.js b/src/api.v2/model/CellLevelMetaToExperiment.js similarity index 76% rename from src/api.v2/model/CellLevelToExperiment.js rename to src/api.v2/model/CellLevelMetaToExperiment.js index fb5d1af28..6446655cc 100644 --- a/src/api.v2/model/CellLevelToExperiment.js +++ b/src/api.v2/model/CellLevelMetaToExperiment.js @@ -7,10 +7,10 @@ const fields = [ 'cell_metadata_file_id', ]; -class CellLevelToExperiment extends BasicModel { +class CellLevelMetaToExperiment extends BasicModel { constructor(sql = sqlClient.get()) { super(sql, tableNames.CELL_LEVEL_TO_EXPERIMENT_MAP, fields); } } -module.exports = CellLevelToExperiment; +module.exports = CellLevelMetaToExperiment; diff --git a/src/api.v2/model/Experiment.js b/src/api.v2/model/Experiment.js index 801f454d1..05b837440 100644 --- a/src/api.v2/model/Experiment.js +++ b/src/api.v2/model/Experiment.js @@ -5,7 +5,7 @@ const { v4: uuidv4 } = require('uuid'); const BasicModel = require('./BasicModel'); const sqlClient = require('../../sql/sqlClient'); const { collapseKeyIntoArray, replaceNullsWithObject } = require('../../sql/helpers'); -const CellLevel = require('./CellLevel'); +const CellLevelMeta = require('./CellLevelMeta'); const { NotFoundError, BadRequestError } = require('../../utils/responses'); const tableNames = require('./tableNames'); const config = require('../../config'); @@ -77,11 +77,11 @@ class Experiment extends BasicModel { this.sql, ); - const cellLevel = new CellLevel(); + const cellLevelMeta = new CellLevelMeta(); const experimentIds = experiments.map((experiment) => experiment.id); - const cellLevelResults = await cellLevel.getMetadataByExperimentIds(experimentIds); + const cellLevelMetaResults = await cellLevelMeta.getMetadataByExperimentIds(experimentIds); - cellLevelResults.forEach( + cellLevelMetaResults.forEach( (cellLevelMetaResult) => { const experimentIndx = experiments.findIndex( (experiment) => experiment.id === cellLevelMetaResult.experimentId, diff --git a/src/api.v2/model/__mocks__/CellLevel.js b/src/api.v2/model/__mocks__/CellLevelMeta.js similarity index 56% rename from src/api.v2/model/__mocks__/CellLevel.js rename to src/api.v2/model/__mocks__/CellLevelMeta.js index c9e4fdcbc..fb85863ac 100644 --- a/src/api.v2/model/__mocks__/CellLevel.js +++ b/src/api.v2/model/__mocks__/CellLevelMeta.js @@ -5,6 +5,6 @@ const stub = { ...BasicModel, }; -const CellLevel = jest.fn().mockImplementation(() => stub); +const CellLevelMeta = jest.fn().mockImplementation(() => stub); -module.exports = CellLevel; +module.exports = CellLevelMeta; diff --git a/src/api.v2/routes/cellLevel.js b/src/api.v2/routes/cellLevel.js index 5cd1edae0..cfda68e2a 100644 --- a/src/api.v2/routes/cellLevel.js +++ b/src/api.v2/routes/cellLevel.js @@ -1,6 +1,6 @@ const { uploadCellLevelMetadata, updateCellLevelMetadata, -} = require('../controllers/cellLevelController'); +} = require('../controllers/cellLevelMetaController'); const { expressAuthorizationMiddleware } = require('../middlewares/authMiddlewares'); diff --git a/tests/api.v2/model/CellLevel.test.js b/tests/api.v2/model/CellLevelMeta.test.js similarity index 93% rename from tests/api.v2/model/CellLevel.test.js rename to tests/api.v2/model/CellLevelMeta.test.js index 5db81ebdb..0ba7bb860 100644 --- a/tests/api.v2/model/CellLevel.test.js +++ b/tests/api.v2/model/CellLevelMeta.test.js @@ -1,6 +1,6 @@ // @ts-nocheck const { mockSqlClient } = require('../mocks/getMockSqlClient')(); -const CellLevel = require('../../../src/api.v2/model/CellLevel'); +const CellLevel = require('../../../src/api.v2/model/CellLevelMeta'); jest.mock('../../../src/sql/sqlClient', () => ({ get: jest.fn(() => mockSqlClient), diff --git a/tests/api.v2/model/Experiment.test.js b/tests/api.v2/model/Experiment.test.js index 9f4469aac..37e64a171 100644 --- a/tests/api.v2/model/Experiment.test.js +++ b/tests/api.v2/model/Experiment.test.js @@ -29,7 +29,7 @@ jest.mock('../../../src/sql/helpers', () => ({ '{}'::jsonb ) as pipelines,}),`), })); -jest.mock('../../../src/api.v2/model/CellLevel'); +jest.mock('../../../src/api.v2/model/CellLevelMeta'); const Experiment = require('../../../src/api.v2/model/Experiment'); const constants = require('../../../src/utils/constants'); const tableNames = require('../../../src/api.v2/model/tableNames'); diff --git a/tests/api.v2/model/__snapshots__/CellLevel.test.js.snap b/tests/api.v2/model/__snapshots__/CellLevelMeta.test.js.snap similarity index 100% rename from tests/api.v2/model/__snapshots__/CellLevel.test.js.snap rename to tests/api.v2/model/__snapshots__/CellLevelMeta.test.js.snap