Merge pull request #188 from biomage-ltd/reorder-samples

[BIOMAGE-1137] - Order samples as defined in project

src/api/general-services/pipeline-manage/index.js

@@ -9,7 +9,6 @@ const AWS = require('../../../utils/requireAWS');
 const config = require('../../../config');
 const logger = require('../../../utils/logging');
 const ExperimentService = require('../../route-services/experiment');
-const SamplesService = require('../../route-services/samples');
 
 const { qcPipelineSkeleton } = require('./skeletons/qc-pipeline-skeleton');
 const { gem2sPipelineSkeleton } = require('./skeletons/gem2s-pipeline-skeleton');
@@ -19,7 +18,6 @@ const asyncTimer = require('../../../utils/asyncTimer');
 const { QC_PROCESS_NAME, GEM2S_PROCESS_NAME } = require('./constants');
 
 const experimentService = new ExperimentService();
-const samplesService = new SamplesService();
 
 const getPipelineArtifacts = async () => {
   const response = await fetch(
@@ -157,11 +155,10 @@ const createQCPipeline = async (experimentId, processingConfigUpdates) => {
   const accountId = await config.awsAccountIdPromise;
   const roleArn = `arn:aws:iam::${accountId}:role/state-machine-role-${config.clusterEnv}`;
   logger.log(`Fetching processing settings for ${experimentId}`);
-  const { processingConfig } = await experimentService.getProcessingConfig(experimentId);
-
-  const { samples } = await samplesService.getSamplesByExperimentId(experimentId);
-
-  const sampleIds = Object.keys(samples);
+  const {
+    processingConfig,
+    sampleIds,
+  } = await experimentService.getAttributesToCreateQCPipeline(experimentId);
 
   if (processingConfigUpdates) {
     processingConfigUpdates.forEach(({ name, body }) => {

src/api/route-services/experiment.js

@@ -18,7 +18,9 @@ const {
 } = require('./experimentHelpers');
 
 const {
-  createDynamoDbInstance, convertToJsObject, convertToDynamoDbRecord,
+  createDynamoDbInstance,
+  convertToJsObject,
+  convertToDynamoDbRecord,
   convertToDynamoUpdateParams,
 } = require('../../utils/dynamoDb');
 
@@ -34,7 +36,7 @@ class ExperimentService {
 
   async getExperimentData(experimentId) {
     const data = await getExperimentAttributes(this.experimentsTableName, experimentId,
-      ['projectId', 'meta', 'experimentId', 'experimentName']);
+      ['projectId', 'meta', 'experimentId', 'experimentName', 'sampleIds']);
     return data;
   }
 
@@ -72,7 +74,7 @@ class ExperimentService {
 
     const marshalledData = convertToDynamoDbRecord({
       ':experimentName': body.name,
-      ':createdAt': body.createdAt,
+      ':createdDate': body.createdDate,
       ':lastViewed': body.lastViewed,
       ':projectId': body.projectUuid,
       ':description': body.description,
@@ -81,18 +83,20 @@ class ExperimentService {
       ':rbac_can_write': documentClient.createSet(rbacCanWrite),
       ':meta': {},
       ':processingConfig': {},
+      ':sampleIds': body.sampleIds,
     });
 
     const params = {
       TableName: this.experimentsTableName,
       Key: key,
       UpdateExpression: `SET experimentName = :experimentName,
-                          createdAt = :createdAt,
+                          createdDate = :createdDate,
                           lastViewed = :lastViewed,
                           projectId = :projectId,
                           description = :description,
                           meta = :meta,
                           processingConfig = :processingConfig,
+                          sampleIds = :sampleIds,
                           rbac_can_write = :rbac_can_write`,
       ExpressionAttributeValues: marshalledData,
       ConditionExpression: 'attribute_not_exists(#experimentId)',
@@ -181,16 +185,21 @@ class ExperimentService {
       [constants.QC_PROCESS_NAME]: {
         stateMachineArn: '',
         executionArn: '',
-        ...data.meta.pipeline,
+        ...data.meta[constants.OLD_QC_NAME_TO_BE_REMOVED],
       },
       [constants.GEM2S_PROCESS_NAME]: {
         stateMachineArn: '',
         executionArn: '',
-        ...data.meta.gem2s,
+        ...data.meta[constants.GEM2S_PROCESS_NAME],
       },
     };
   }
 
+  async getAttributesToCreateQCPipeline(experimentId) {
+    const data = await getExperimentAttributes(this.experimentsTableName, experimentId, ['processingConfig', 'sampleIds']);
+    return data;
+  }
+
   async getCellSets(experimentId) {
     const s3 = new AWS.S3();
 

src/api/route-services/experimentHelpers.js

@@ -66,10 +66,11 @@ const getShallowAttrsUpdateParams = (body) => {
   const dataToUpdate = {
     experimentName: body.name || body.experimentName,
     apiVersion: body.apiVersion,
-    createdAt: body.createdAt,
+    createdDate: body.createdDate,
     lastViewed: body.lastViewed,
     projectId: body.projectUuid || body.projectId,
     description: body.description,
+    sampleIds: body.sampleIds,
   };
 
   const objectToMarshall = {};

src/api/route-services/gem2s.js

@@ -16,7 +16,7 @@ const { OK } = require('../../utils/responses');
 const logger = require('../../utils/logging');
 
 const ExperimentService = require('./experiment');
-const ProjectService = require('./projects');
+const ProjectsService = require('./projects');
 const SamplesService = require('./samples');
 
 const pipelineHook = new PipelineHook();
@@ -46,10 +46,12 @@ class Gem2sService {
     io.sockets.emit(`ExperimentUpdates-${experimentId}`, response);
   }
 
-  static async generateGem2sTaskParams(experimentId) {
+  static async generateGem2sParams(experimentId) {
     const experiment = await (new ExperimentService()).getExperimentData(experimentId);
     const { samples } = await (new SamplesService()).getSamplesByExperimentId(experimentId);
-    const { metadataKeys } = await (new ProjectService()).getProject(experiment.projectId);
+    const {
+      metadataKeys,
+    } = await new ProjectsService().getProject(experiment.projectId);
 
     const defaultMetadataValue = 'N.A.';
 
@@ -60,8 +62,8 @@ class Gem2sService {
       experimentName: experiment.experimentName,
       organism: experiment.meta.organism,
       input: { type: experiment.meta.type },
-      sampleIds: samplesEntries.map(([sampleId]) => sampleId),
-      sampleNames: samplesEntries.map(([, sample]) => sample.name),
+      sampleIds: experiment.sampleIds,
+      sampleNames: experiment.sampleIds.map((sampleId) => samples[sampleId].name),
     };
 
     if (metadataKeys.length) {
@@ -76,7 +78,18 @@ class Gem2sService {
       }, {});
     }
 
-    return taskParams;
+    // Different sample order should not change the hash.
+    const orderInvariantSampleIds = [...experiment.sampleIds].sort();
+
+    const hashParams = {
+      organism: experiment.meta.organism,
+      input: { type: experiment.meta.type },
+      sampleIds: orderInvariantSampleIds,
+      sampleNames: orderInvariantSampleIds.map((sampleId) => samples[sampleId].name),
+      metadata: taskParams.metadata,
+    };
+
+    return { taskParams, hashParams };
   }
 
   static async gem2sShouldRun(experimentId, paramsHash) {
@@ -100,11 +113,11 @@ class Gem2sService {
   }
 
   static async gem2sCreate(experimentId) {
-    const taskParams = await this.generateGem2sTaskParams(experimentId);
+    const { taskParams, hashParams } = await this.generateGem2sParams(experimentId);
 
     const paramsHash = crypto
       .createHash('sha1')
-      .update(JSON.stringify(taskParams))
+      .update(JSON.stringify(hashParams))
       .digest('hex');
 
     const shouldRun = await this.gem2sShouldRun(experimentId, paramsHash);

tests/api/general-services/pipeline-manage.test.js

@@ -8,8 +8,18 @@ jest.mock('crypto', () => ({
 }));
 jest.mock('../../../src/utils/asyncTimer');
 
-const MockProcessingConfig = {
+const MockExperimentData = {
   Item: {
+    sampleIds: {
+      L: [
+        {
+          S: 'oneSample',
+        },
+        {
+          S: 'otherSample',
+        },
+      ],
+    },
     processingConfig: {
       M: {
         doubletScores: {
@@ -49,29 +59,6 @@ const MockProcessingConfig = {
   },
 };
 
-const MockSamples = {
-  Item: {
-    samples: {
-      M: {
-        oneSample: {
-          M: {
-            uuid: {
-              S: 'oneSample',
-            },
-          },
-        },
-        otherSample: {
-          M: {
-            uuid: {
-              S: 'otherSample',
-            },
-          },
-        },
-      },
-    },
-  },
-};
-
 const { createQCPipeline } = jest.requireActual('../../../src/api/general-services/pipeline-manage');
 
 describe('test for pipeline services', () => {
@@ -138,15 +125,11 @@ describe('test for pipeline services', () => {
       callback(null, { executionArn: 'test-machine' });
     });
 
-    const getProcessingConfigSpy = jest.fn((x) => x);
-    const getSamplesSpy = jest.fn((x) => x);
+    const getExperimentDataSpy = jest.fn((x) => x);
     AWSMock.mock('DynamoDB', 'getItem', (params, callback) => {
       if (params.TableName.match('experiments')) {
-        getProcessingConfigSpy(params);
-        callback(null, MockProcessingConfig);
-      } else if (params.TableName.match('samples')) {
-        getSamplesSpy(params);
-        callback(null, MockSamples);
+        getExperimentDataSpy(params);
+        callback(null, MockExperimentData);
       }
     });
 
@@ -155,8 +138,7 @@ describe('test for pipeline services', () => {
 
     expect(createStateMachineSpy.mock.results).toMatchSnapshot();
 
-    expect(getProcessingConfigSpy).toHaveBeenCalled();
-    expect(getSamplesSpy).toHaveBeenCalled();
+    expect(getExperimentDataSpy).toHaveBeenCalled();
 
     expect(createActivitySpy).toHaveBeenCalled();
     expect(startExecutionSpy).toHaveBeenCalled();
@@ -204,15 +186,11 @@ describe('test for pipeline services', () => {
       callback(null, { executionArn: 'test-machine' });
     });
 
-    const getProcessingConfigSpy = jest.fn((x) => x);
-    const getSamplesSpy = jest.fn((x) => x);
+    const getExperimentDataSpy = jest.fn((x) => x);
     AWSMock.mock('DynamoDB', 'getItem', (params, callback) => {
       if (params.TableName.match('experiments')) {
-        getProcessingConfigSpy(params);
-        callback(null, MockProcessingConfig);
-      } else if (params.TableName.match('samples')) {
-        getSamplesSpy(params);
-        callback(null, MockSamples);
+        getExperimentDataSpy(params);
+        callback(null, MockExperimentData);
       }
     });
 

tests/api/general-services/pipeline-status.test.js

@@ -1,15 +1,11 @@
 const AWSMock = require('aws-sdk-mock');
 const AWS = require('../../../src/utils/requireAWS');
 const constants = require('../../../src/api/general-services/pipeline-manage/constants');
-const experimentHelpers = require('../../../src/api/route-services/experimentHelpers');
-
-const getExperimentAttributesSpy = jest.spyOn(experimentHelpers, 'getExperimentAttributes');
 
 const pipelineStatus = require('../../../src/api/general-services/pipeline-status');
-const ProjectService = require('../../../src/api/route-services/projects');
 
 const {
-  RUNNING, SUCCEEDED, NOT_CREATED, FAILED, TIMED_OUT, ABORTED, GEM2S_PROCESS_NAME, QC_PROCESS_NAME,
+  GEM2S_PROCESS_NAME, QC_PROCESS_NAME, OLD_QC_NAME_TO_BE_REMOVED,
 } = constants;
 
 describe('getStepsFromExecutionHistory', () => {
@@ -349,16 +345,33 @@ describe('getStepsFromExecutionHistory', () => {
 
 describe('pipelineStatus', () => {
   const mockNotRunResponse = {
-    meta: {
-      [GEM2S_PROCESS_NAME]: {
-        stateMachineArn: '',
-        executionArn: '',
-      },
-      [QC_PROCESS_NAME]: {
-        stateMachineArn: '',
-        executionArn: '',
+    Item: {
+      meta: {
+        M: {
+          [GEM2S_PROCESS_NAME]: {
+            M: {
+              stateMachineArn: {
+                S: '',
+              },
+              executionArn: {
+                S: '',
+              },
+            },
+          },
+          [OLD_QC_NAME_TO_BE_REMOVED]: {
+            M: {
+              stateMachineArn: {
+                S: '',
+              },
+              executionArn: {
+                S: '',
+              },
+            },
+          },
+        },
       },
     },
+
   };
 
   const mockDescribeExecution = jest.fn();
@@ -374,7 +387,6 @@ describe('pipelineStatus', () => {
   }));
 
   beforeEach(() => {
-    getExperimentAttributesSpy.mockClear();
     AWSMock.setSDKInstance(AWS);
 
     AWSMock.mock('StepFunctions', 'describeExecution', (params, callback) => {
@@ -385,13 +397,14 @@ describe('pipelineStatus', () => {
       callback(null, { events: [] });
     });
 
+    const getPipelineHandleSpy = jest.fn((x) => x);
     AWSMock.mock('DynamoDB', 'getItem', (params, callback) => {
-      callback(null, mockDynamoGetItem(params));
+      getPipelineHandleSpy(params);
+      callback(null, mockNotRunResponse);
     });
   });
 
   it('handles properly an empty dynamodb record', async () => {
-    getExperimentAttributesSpy.mockReturnValueOnce(mockNotRunResponse);
     const status = await pipelineStatus('1234', QC_PROCESS_NAME);
 
     expect(status).toEqual({

tests/api/route-services/experiment.test.js

@@ -39,7 +39,7 @@ describe('tests for the experiment service', () => {
         expect(fnSpy).toHaveBeenCalledWith({
           TableName: 'experiments-test',
           Key: { experimentId: { S: '12345' } },
-          ProjectionExpression: 'projectId,meta,experimentId,experimentName',
+          ProjectionExpression: 'projectId,meta,experimentId,experimentName,sampleIds',
         });
       })
       .then(() => done());