From 6ea35b00f29407372f57e950faa425340fc70451 Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Tue, 1 Jun 2021 17:54:38 -0300
Subject: [PATCH 1/9] Add deletion of files of deleted project

---
 src/api/route-services/projects.js |  4 +--
 src/api/route-services/samples.js  | 46 ++++++++++++++++++++++++++----
 src/api/routes/samples.js          |  1 +
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/src/api/route-services/projects.js b/src/api/route-services/projects.js
index 5b724b829..1c86d4c26 100644
--- a/src/api/route-services/projects.js
+++ b/src/api/route-services/projects.js
@@ -190,12 +190,12 @@ class ProjectsService {
     const dynamodb = createDynamoDbInstance();
 
     try {
-      const { experiments } = await this.getProject(projectUuid);
+      const { experiments, samples: sampleUuids } = await this.getProject(projectUuid);
 
       if (experiments.length > 0) {
         const deletePromises = experiments.reduce((acc, experimentId) => {
           acc.push(experimentService.deleteExperiment(experimentId));
-          acc.push(samplesService.deleteSamples(projectUuid, experimentId));
+          acc.push(samplesService.deleteSamples(projectUuid, experimentId, sampleUuids));
           return acc;
         }, []);
 
diff --git a/src/api/route-services/samples.js b/src/api/route-services/samples.js
index 3bf5a3038..09d95a3aa 100644
--- a/src/api/route-services/samples.js
+++ b/src/api/route-services/samples.js
@@ -1,15 +1,19 @@
+const _ = require('lodash');
+
 const { NotFoundError, OK } = require('../../utils/responses');
 
 const config = require('../../config');
 const {
   createDynamoDbInstance, convertToJsObject, convertToDynamoDbRecord,
 } = require('../../utils/dynamoDb');
-const logger = require('../../utils/logging');
+const AWS = require('../../utils/requireAWS');
 
+const logger = require('../../utils/logging');
 
 class SamplesService {
   constructor() {
     this.tableName = `samples-${config.clusterEnv}`;
+    this.sampleFilesBucketName = `biomage-originals-${config.clusterEnv}`;
   }
 
   async getSamples(projectUuid) {
@@ -92,14 +96,40 @@ class SamplesService {
     }
   }
 
-  async deleteSamples(projectUuid, experimentId) {
+  async deleteSamplesFromS3(projectUuid, sampleUuids) {
+    const s3 = new AWS.S3();
+
+    const sampleObjectsToDelete = sampleUuids.map((sampleUuid) => (
+      [
+        { Key: `${projectUuid}/${sampleUuid}/barcodes.tsv.gz` },
+        { Key: `${projectUuid}/${sampleUuid}/features.tsv.gz` },
+        { Key: `${projectUuid}/${sampleUuid}/matrix.mtx.gz` },
+      ]
+    ));
+
+    const s3Params = {
+      Bucket: this.sampleFilesBucketName,
+      Delete: {
+        Objects: _.flatten(sampleObjectsToDelete),
+        Quiet: false,
+      },
+    };
+
+    const result = await s3.deleteObjects(s3Params).promise();
+
+    if (result.Errors.length) {
+      throw Error(`Delete S3 object errors: ${JSON.stringify(result.Errors)}`);
+    }
+  }
+
+  async deleteSamples(projectUuid, experimentId, sampleUuids) {
     logger.log(`Deleting sample for project ${projectUuid} and expId ${experimentId}`);
 
     const marshalledKey = convertToDynamoDbRecord({
       experimentId,
     });
 
-    const params = {
+    const dynamodbParams = {
       TableName: this.tableName,
       Key: marshalledKey,
     };
@@ -107,14 +137,18 @@ class SamplesService {
     const dynamodb = createDynamoDbInstance();
 
     try {
-      await dynamodb.deleteItem(params).send();
-      return OK();
+      await dynamodb.deleteItem(dynamodbParams).send();
     } catch (e) {
       if (e.statusCode === 404) throw NotFoundError('Project not found');
       throw e;
     }
+
+    if (sampleUuids.length) {
+      this.deleteSamplesFromS3(projectUuid, sampleUuids);
+    }
+
+    return OK();
   }
 }
 
-
 module.exports = SamplesService;
diff --git a/src/api/routes/samples.js b/src/api/routes/samples.js
index 896cba653..e4ff27e96 100644
--- a/src/api/routes/samples.js
+++ b/src/api/routes/samples.js
@@ -15,6 +15,7 @@ module.exports = {
   },
   'samples#update': (req, res, next) => {
     const { body, params: { projectUuid } } = req;
+
     samplesService.updateSamples(projectUuid, body)
       .then((data) => res.json(data))
       .catch(next);

From 4dfd3bd0af68914c35dbcfaa0a8a71a40723d656 Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Thu, 3 Jun 2021 12:26:43 -0300
Subject: [PATCH 2/9] Add basic delete single sample operation (still needs
 updates)

---
 src/api/route-services/__mocks__/samples.js   |  2 +-
 src/api/route-services/projects.js            |  2 +-
 src/api/route-services/samples.js             | 26 ++++++++++++++++---
 src/api/route-services/utils.js               | 18 +++++++++++++
 src/api/routes/samples.js                     |  7 +++++
 src/specs/api.yaml                            |  4 ---
 .../samples-bodies/SamplesUpdateBody.v1.yaml  | 12 +++++++++
 7 files changed, 62 insertions(+), 9 deletions(-)
 create mode 100644 src/api/route-services/utils.js
 create mode 100644 src/specs/models/samples-bodies/SamplesUpdateBody.v1.yaml

diff --git a/src/api/route-services/__mocks__/samples.js b/src/api/route-services/__mocks__/samples.js
index 294c0636b..7781d8422 100644
--- a/src/api/route-services/__mocks__/samples.js
+++ b/src/api/route-services/__mocks__/samples.js
@@ -75,7 +75,7 @@ const mock = jest.fn().mockImplementation(() => ({
   getSamples: mockGetSamples,
   getSamplesByExperimentId: mockGetByExperimentId,
   updateSamples: mockUpdateSamples,
-  deleteSamples: mockDeleteSamples,
+  deleteSamplesEntry: mockDeleteSamples,
 }));
 
 module.exports = mock;
diff --git a/src/api/route-services/projects.js b/src/api/route-services/projects.js
index 1c86d4c26..61810a5c7 100644
--- a/src/api/route-services/projects.js
+++ b/src/api/route-services/projects.js
@@ -195,7 +195,7 @@ class ProjectsService {
       if (experiments.length > 0) {
         const deletePromises = experiments.reduce((acc, experimentId) => {
           acc.push(experimentService.deleteExperiment(experimentId));
-          acc.push(samplesService.deleteSamples(projectUuid, experimentId, sampleUuids));
+          acc.push(samplesService.deleteSamplesEntry(projectUuid, experimentId, sampleUuids));
           return acc;
         }, []);
 
diff --git a/src/api/route-services/samples.js b/src/api/route-services/samples.js
index 09d95a3aa..6aed5c34f 100644
--- a/src/api/route-services/samples.js
+++ b/src/api/route-services/samples.js
@@ -76,7 +76,6 @@ class SamplesService {
       ':projectUuid': projectUuid,
     });
 
-
     // Update samples
     const params = {
       TableName: this.tableName,
@@ -122,8 +121,29 @@ class SamplesService {
     }
   }
 
-  async deleteSamples(projectUuid, experimentId, sampleUuids) {
-    logger.log(`Deleting sample for project ${projectUuid} and expId ${experimentId}`);
+  async removeSamples(projectUuid, experimentId, sampleUuids) {
+    logger.log(`Removing samples in an entry for project ${projectUuid} and expId ${experimentId}`);
+
+    const marshalledKey = convertToDynamoDbRecord({
+      experimentId,
+    });
+
+    const removeSamplesExpression = sampleUuids.map(
+      (sampleUuid) => `samples.${sampleUuid}, samples.ids.${sampleUuid}`,
+    ).join(', ');
+
+    const params = {
+      TableName: this.tableName,
+      Key: marshalledKey,
+      UpdateExpression: `REMOVE ${removeSamplesExpression}`,
+      ReturnValues: 'ALL_NEW',
+    };
+
+    await createDynamoDbInstance().updateItem(params).promise();
+  }
+
+  async deleteSamplesEntry(projectUuid, experimentId, sampleUuids) {
+    logger.log(`Deleting samples entry for project ${projectUuid} and expId ${experimentId}`);
 
     const marshalledKey = convertToDynamoDbRecord({
       experimentId,
diff --git a/src/api/route-services/utils.js b/src/api/route-services/utils.js
new file mode 100644
index 000000000..2230406d9
--- /dev/null
+++ b/src/api/route-services/utils.js
@@ -0,0 +1,18 @@
+// Updates each sub attribute separately for
+// one particular attribute (of type object) of a dynamodb entry
+
+const removePropertiesFromObject = async (
+  entryKey, attributeName, propertyToRemove,
+  tableName, dynamodb,
+) => {
+  const params = {
+    TableName: tableName,
+    Key: entryKey,
+    UpdateExpression: `REMOVE ${attributeName}.${propertyToRemove}`,
+    ReturnValues: 'ALL_NEW',
+  };
+
+  await dynamodb.updateItem(params).promise();
+};
+
+module.exports = { removePropertiesFromObject };
diff --git a/src/api/routes/samples.js b/src/api/routes/samples.js
index e4ff27e96..830eac2f2 100644
--- a/src/api/routes/samples.js
+++ b/src/api/routes/samples.js
@@ -20,4 +20,11 @@ module.exports = {
       .then((data) => res.json(data))
       .catch(next);
   },
+  'samples#delete': (req, res, next) => {
+    const { body, params: { experimentId, projectUuid } } = req;
+
+    samplesService.deleteSamplesEntry(projectUuid, experimentId, body.samples.ids)
+      .then((data) => res.json(data))
+      .catch(next);
+  },
 };
diff --git a/src/specs/api.yaml b/src/specs/api.yaml
index c7b083dfb..df584a0d0 100644
--- a/src/specs/api.yaml
+++ b/src/specs/api.yaml
@@ -875,10 +875,6 @@ paths:
               schema:
                 type: object
                 properties:
-                  projectUuid:
-                    type: string
-                  experimentId:
-                    type: string
                   samples:
                     $ref: ./models/api-body-schemas/Samples.v1.yaml
   '/projects':
diff --git a/src/specs/models/samples-bodies/SamplesUpdateBody.v1.yaml b/src/specs/models/samples-bodies/SamplesUpdateBody.v1.yaml
new file mode 100644
index 000000000..e9452f31b
--- /dev/null
+++ b/src/specs/models/samples-bodies/SamplesUpdateBody.v1.yaml
@@ -0,0 +1,12 @@
+title: Update Samples Body
+description: An array specifying updates to perform to a samples entry
+type: object
+properties:
+  add:
+    description: Array samples to set to the entry
+    $ref: ../api-body-schemas/Samples.v1.yaml
+  remove:
+    description: An array of sample ids to delete from the entry
+    type: array
+    items:
+      type: string
\ No newline at end of file

From 9e4fc195db2f4dad559b7b09f1c3bede87ee7019 Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Fri, 4 Jun 2021 14:10:46 -0300
Subject: [PATCH 3/9] Remove samples unnecessary ids field

---
 .../general-services/pipeline-manage/index.js | 18 +++++-----
 src/api/route-services/samples.js             | 14 ++++++--
 src/api/routes/samples.js                     |  7 ----
 .../models/api-body-schemas/Samples.v1.yaml   | 36 ++-----------------
 .../models/samples-bodies/Sample.v1.yaml      | 28 +++++++++++++++
 5 files changed, 52 insertions(+), 51 deletions(-)
 create mode 100644 src/specs/models/samples-bodies/Sample.v1.yaml

diff --git a/src/api/general-services/pipeline-manage/index.js b/src/api/general-services/pipeline-manage/index.js
index d23a99845..04f8745a6 100644
--- a/src/api/general-services/pipeline-manage/index.js
+++ b/src/api/general-services/pipeline-manage/index.js
@@ -164,6 +164,8 @@ const createQCPipeline = async (experimentId, processingConfigUpdates) => {
 
   const { samples } = await samplesService.getSamplesByExperimentId(experimentId);
 
+  const sampleIds = Object.keys(samples);
+
   if (processingConfigUpdates) {
     processingConfigUpdates.forEach(({ name, body }) => {
       if (!processingConfig[name]) {
@@ -182,14 +184,14 @@ const createQCPipeline = async (experimentId, processingConfigUpdates) => {
   const mergedProcessingConfig = _.cloneDeepWith(processingConfig, (o) => {
     if (_.isObject(o) && !o.dataIntegration && !o.embeddingSettings && typeof o.enabled === 'boolean') {
       // Find which samples have sample-specific configurations.
-      const sampleConfigs = _.intersection(Object.keys(o), samples.ids);
+      const sampleConfigs = _.intersection(Object.keys(o), sampleIds);
 
       // Get an object that is only the "raw" configuration.
       const rawConfig = _.omit(o, sampleConfigs);
 
       const result = {};
 
-      samples.ids.forEach((sample) => {
+      sampleIds.forEach((sample) => {
         result[sample] = _.merge({}, rawConfig, o[sample]);
       });
 
@@ -219,7 +221,7 @@ const createQCPipeline = async (experimentId, processingConfigUpdates) => {
   logger.log(`State machine with ARN ${stateMachineArn} created, launching it...`);
 
   const execInput = {
-    samples: samples.ids.map((sampleUuid, index) => ({ sampleUuid, index })),
+    samples: sampleIds.map((sampleUuid, index) => ({ sampleUuid, index })),
   };
 
   const executionArn = await executeStateMachine(stateMachineArn, execInput);
@@ -237,21 +239,21 @@ const createGem2SPipeline = async (experimentId) => {
   const { metadataKeys } = await projectService.getProject(experiment.projectId);
 
   const defaultMetadataValue = 'N.A.';
+  const samplesEntries = Object.entries(samples);
 
   const taskParams = {
     projectId: experiment.projectId,
     experimentName: experiment.experimentName,
     organism: experiment.meta.organism,
     input: { type: experiment.meta.type },
-    sampleIds: samples.ids,
-    sampleNames: samples.ids.map((id) => samples[id].name),
+    sampleIds: samplesEntries.map(([sampleId]) => sampleId),
+    sampleNames: samplesEntries.map(([, sample]) => sample.name),
   };
 
   if (metadataKeys.length) {
     taskParams.metadata = metadataKeys.reduce((acc, key) => {
-      acc[key] = samples.ids.map(
-        (sampleUuid) => samples[sampleUuid].metadata[key] || defaultMetadataValue,
-      );
+      acc[key] = samplesEntries.map(([, sample]) => sample.metadata[key] || defaultMetadataValue);
+
       return acc;
     }, {});
   }
diff --git a/src/api/route-services/samples.js b/src/api/route-services/samples.js
index 6aed5c34f..f1e4cc85c 100644
--- a/src/api/route-services/samples.js
+++ b/src/api/route-services/samples.js
@@ -37,7 +37,14 @@ class SamplesService {
       throw new NotFoundError('Samples not found!');
     }
 
-    return items.map((item) => convertToJsObject(item));
+    return items.map((item) => {
+      const prettyItem = convertToJsObject(item);
+
+      // Remove ids property from old sample entries that still have it
+      delete prettyItem.samples.ids;
+
+      return prettyItem;
+    });
   }
 
 
@@ -58,9 +65,12 @@ class SamplesService {
 
     if (response.Item) {
       const prettyResponse = convertToJsObject(response.Item);
+      // Remove ids property from old sample entries that still have it
+      delete prettyResponse.samples.ids;
       return prettyResponse;
     }
 
+
     throw new NotFoundError('Samples not found');
   }
 
@@ -129,7 +139,7 @@ class SamplesService {
     });
 
     const removeSamplesExpression = sampleUuids.map(
-      (sampleUuid) => `samples.${sampleUuid}, samples.ids.${sampleUuid}`,
+      (sampleUuid) => `samples.${sampleUuid}`,
     ).join(', ');
 
     const params = {
diff --git a/src/api/routes/samples.js b/src/api/routes/samples.js
index 830eac2f2..e4ff27e96 100644
--- a/src/api/routes/samples.js
+++ b/src/api/routes/samples.js
@@ -20,11 +20,4 @@ module.exports = {
       .then((data) => res.json(data))
       .catch(next);
   },
-  'samples#delete': (req, res, next) => {
-    const { body, params: { experimentId, projectUuid } } = req;
-
-    samplesService.deleteSamplesEntry(projectUuid, experimentId, body.samples.ids)
-      .then((data) => res.json(data))
-      .catch(next);
-  },
 };
diff --git a/src/specs/models/api-body-schemas/Samples.v1.yaml b/src/specs/models/api-body-schemas/Samples.v1.yaml
index 50c8e88f9..84b3fa464 100644
--- a/src/specs/models/api-body-schemas/Samples.v1.yaml
+++ b/src/specs/models/api-body-schemas/Samples.v1.yaml
@@ -1,37 +1,5 @@
-title: Sample object
+title: Samples container object as it is stored
 description: Schema for a sample object
 type: object
-properties:
-  ids:
-    type: array
-    items:
-      type: string
 additionalProperties:
-  type: object
-  properties:
-    name:
-      type: string
-    projectUuid:
-      type: string
-    uuid:
-      type: string
-    species:
-      nullable: true
-      type: string
-    createdDate:
-      type: string
-    lastModified:
-      nullable: true
-      type: string
-    complete:
-      type: boolean
-    error:
-      type: boolean
-    fileNames:
-      type: array
-      items:
-        type: string
-    files:
-      type: object
-required:
-  - ids
\ No newline at end of file
+  $ref: ../samples-bodies/Sample.v1.yaml
\ No newline at end of file
diff --git a/src/specs/models/samples-bodies/Sample.v1.yaml b/src/specs/models/samples-bodies/Sample.v1.yaml
new file mode 100644
index 000000000..41feafcd7
--- /dev/null
+++ b/src/specs/models/samples-bodies/Sample.v1.yaml
@@ -0,0 +1,28 @@
+title: Sample
+description: A single sample
+type: object
+properties:
+  name:
+    type: string
+  projectUuid:
+    type: string
+  uuid:
+    type: string
+  species:
+    nullable: true
+    type: string
+  createdDate:
+    type: string
+  lastModified:
+    nullable: true
+    type: string
+  complete:
+    type: boolean
+  error:
+    type: boolean
+  fileNames:
+    type: array
+    items:
+      type: string
+  files:
+    type: object
\ No newline at end of file

From 334a2db1dec3c13142d9ecc9365323bbe8976858 Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Fri, 4 Jun 2021 17:08:10 -0300
Subject: [PATCH 4/9] Fixes and add validation

---
 src/api/route-services/samples.js             | 28 +++++++++----
 src/api/routes/samples.js                     |  7 ++++
 src/specs/api.yaml                            | 42 +++++++++++++++++--
 .../samples-bodies/SamplesRemoveBody.v1.yaml  | 11 +++++
 .../samples-bodies/SamplesUpdateBody.v1.yaml  | 12 ------
 5 files changed, 78 insertions(+), 22 deletions(-)
 create mode 100644 src/specs/models/samples-bodies/SamplesRemoveBody.v1.yaml
 delete mode 100644 src/specs/models/samples-bodies/SamplesUpdateBody.v1.yaml

diff --git a/src/api/route-services/samples.js b/src/api/route-services/samples.js
index f1e4cc85c..a483d5219 100644
--- a/src/api/route-services/samples.js
+++ b/src/api/route-services/samples.js
@@ -138,18 +138,28 @@ class SamplesService {
       experimentId,
     });
 
-    const removeSamplesExpression = sampleUuids.map(
-      (sampleUuid) => `samples.${sampleUuid}`,
-    ).join(', ');
+    const removeSamplesList = sampleUuids.map((sampleUuid, index) => `samples.#val${index}`);
+    const expressionAttributeNames = sampleUuids.reduce((acc, sampleId, index) => {
+      acc[`#val${index}`] = sampleId;
+      return acc;
+    }, {});
 
     const params = {
       TableName: this.tableName,
       Key: marshalledKey,
-      UpdateExpression: `REMOVE ${removeSamplesExpression}`,
+      UpdateExpression: `REMOVE ${removeSamplesList.join(', ')}`,
+      ExpressionAttributeNames: expressionAttributeNames,
       ReturnValues: 'ALL_NEW',
     };
 
-    await createDynamoDbInstance().updateItem(params).promise();
+    const promises = [
+      createDynamoDbInstance().updateItem(params).promise(),
+      this.deleteSamplesFromS3(projectUuid, sampleUuids),
+    ];
+
+    await Promise.all(promises);
+
+    return OK();
   }
 
   async deleteSamplesEntry(projectUuid, experimentId, sampleUuids) {
@@ -166,17 +176,21 @@ class SamplesService {
 
     const dynamodb = createDynamoDbInstance();
 
+    const promises = [];
+
     try {
-      await dynamodb.deleteItem(dynamodbParams).send();
+      promises.push(await dynamodb.deleteItem(dynamodbParams).send());
     } catch (e) {
       if (e.statusCode === 404) throw NotFoundError('Project not found');
       throw e;
     }
 
     if (sampleUuids.length) {
-      this.deleteSamplesFromS3(projectUuid, sampleUuids);
+      promises.push(await this.deleteSamplesFromS3(projectUuid, sampleUuids));
     }
 
+    await Promise.all(promises);
+
     return OK();
   }
 }
diff --git a/src/api/routes/samples.js b/src/api/routes/samples.js
index e4ff27e96..e55a1da1c 100644
--- a/src/api/routes/samples.js
+++ b/src/api/routes/samples.js
@@ -20,4 +20,11 @@ module.exports = {
       .then((data) => res.json(data))
       .catch(next);
   },
+  'samples#remove': (req, res, next) => {
+    const { body: { ids }, params: { projectUuid, experimentId } } = req;
+
+    samplesService.removeSamples(projectUuid, experimentId, ids)
+      .then((data) => res.json(data))
+      .catch(next);
+  },
 };
diff --git a/src/specs/api.yaml b/src/specs/api.yaml
index df584a0d0..898bb05b3 100644
--- a/src/specs/api.yaml
+++ b/src/specs/api.yaml
@@ -839,8 +839,8 @@ paths:
           schema:
             type: string
       put:
-        summary: Update sample
-        operationId: updateSample
+        summary: Update samples
+        operationId: updateSamples
         x-eov-operation-id: samples#update
         x-eov-operation-handler: routes/samples
         responses:
@@ -868,7 +868,7 @@ paths:
               application/json:
                 schema:
                   $ref: ./models/HTTPError.v1.yaml
-        description: Updates a sample
+        description: Updates a samples entry
         requestBody:
           content:
             application/json:
@@ -877,6 +877,42 @@ paths:
                 properties:
                   samples:
                     $ref: ./models/api-body-schemas/Samples.v1.yaml
+      delete:
+        summary: Remove samples
+        operationId: removeSamples
+        x-eov-operation-id: samples#remove
+        x-eov-operation-handler: routes/samples
+        responses:
+          '200':
+            description: OK
+            content:
+              application/json:
+                schema:
+                  $ref: ./models/HTTPSuccess.v1.yaml
+          '400':
+            description: Bad Request
+            content:
+              application/json:
+                schema:
+                  $ref: ./models/HTTPError.v1.yaml
+          '404':
+            description: Not Found
+            content:
+              application/json:
+                schema:
+                  $ref: ./models/HTTPError.v1.yaml
+          '415':
+            description: Unsupported Media Type
+            content:
+              application/json:
+                schema:
+                  $ref: ./models/HTTPError.v1.yaml
+        description: Removes samples from an entry
+        requestBody:
+          content:
+            application/json:
+              schema:
+                $ref: ./models/samples-bodies/SamplesRemoveBody.v1.yaml
   '/projects':
     get:
       summary: 'Get the list of projects'
diff --git a/src/specs/models/samples-bodies/SamplesRemoveBody.v1.yaml b/src/specs/models/samples-bodies/SamplesRemoveBody.v1.yaml
new file mode 100644
index 000000000..bc6d61729
--- /dev/null
+++ b/src/specs/models/samples-bodies/SamplesRemoveBody.v1.yaml
@@ -0,0 +1,11 @@
+title: Remove Samples Body
+description: A wrapper object for the array of sample ids to be removed
+type: object
+properties:
+  ids:
+    description: An array of sample ids specifying which samples to delete
+    type: array
+    items:
+      type: string
+required: 
+- ids
\ No newline at end of file
diff --git a/src/specs/models/samples-bodies/SamplesUpdateBody.v1.yaml b/src/specs/models/samples-bodies/SamplesUpdateBody.v1.yaml
deleted file mode 100644
index e9452f31b..000000000
--- a/src/specs/models/samples-bodies/SamplesUpdateBody.v1.yaml
+++ /dev/null
@@ -1,12 +0,0 @@
-title: Update Samples Body
-description: An array specifying updates to perform to a samples entry
-type: object
-properties:
-  add:
-    description: Array samples to set to the entry
-    $ref: ../api-body-schemas/Samples.v1.yaml
-  remove:
-    description: An array of sample ids to delete from the entry
-    type: array
-    items:
-      type: string
\ No newline at end of file

From bd2f38db666b6907166c12ca8c2983a2e1bc2bc0 Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Mon, 7 Jun 2021 11:23:15 -0300
Subject: [PATCH 5/9] Fix tests

---
 src/api/route-services/samples.js             |  2 +-
 .../general-services/pipeline-manage.test.js  | 92 ++++++++++---------
 tests/api/route-services/projects.test.js     |  3 +-
 tests/api/route-services/samples.test.js      | 23 ++++-
 tests/test-utils/mockAWSServices.js           | 13 +++
 5 files changed, 83 insertions(+), 50 deletions(-)

diff --git a/src/api/route-services/samples.js b/src/api/route-services/samples.js
index a483d5219..5190bf2d1 100644
--- a/src/api/route-services/samples.js
+++ b/src/api/route-services/samples.js
@@ -65,12 +65,12 @@ class SamplesService {
 
     if (response.Item) {
       const prettyResponse = convertToJsObject(response.Item);
+
       // Remove ids property from old sample entries that still have it
       delete prettyResponse.samples.ids;
       return prettyResponse;
     }
 
-
     throw new NotFoundError('Samples not found');
   }
 
diff --git a/tests/api/general-services/pipeline-manage.test.js b/tests/api/general-services/pipeline-manage.test.js
index e2490ea7f..63ad55546 100644
--- a/tests/api/general-services/pipeline-manage.test.js
+++ b/tests/api/general-services/pipeline-manage.test.js
@@ -8,38 +8,28 @@ jest.mock('crypto', () => ({
 }));
 jest.mock('../../../src/utils/asyncTimer');
 
-const { createQCPipeline } = jest.requireActual('../../../src/api/general-services/pipeline-manage');
-
-describe('test for pipeline services', () => {
-  afterEach(() => {
-    AWSMock.restore('EKS');
-    AWSMock.restore('StepFunctions');
-  });
-
-
-  const MockProcessingConfig = {
-    Item: {
-      processingConfig: {
-        M: {
-          doubletScores: {
-            M: {
-              enabled: {
-                BOOL: true,
-              },
-              filterSettings: {
-                M: {
-                  oneSetting: {
-                    N: 1,
-                  },
+const MockProcessingConfig = {
+  Item: {
+    processingConfig: {
+      M: {
+        doubletScores: {
+          M: {
+            enabled: {
+              BOOL: true,
+            },
+            filterSettings: {
+              M: {
+                oneSetting: {
+                  N: 1,
                 },
               },
-              oneSample: {
-                M: {
-                  filterSettings: {
-                    M: {
-                      oneSetting: {
-                        N: 1,
-                      },
+            },
+            oneSample: {
+              M: {
+                filterSettings: {
+                  M: {
+                    oneSetting: {
+                      N: 1,
                     },
                   },
                 },
@@ -49,23 +39,39 @@ describe('test for pipeline services', () => {
         },
       },
     },
-  };
+  },
+};
 
-  const MockSamples = {
-    Item: {
-      samples: {
-        M: {
-          ids: {
-            L: [
-              { S: 'oneSample' },
-              { S: 'otherSample' },
-            ],
+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', () => {
+  afterEach(() => {
+    AWSMock.restore('EKS');
+    AWSMock.restore('StepFunctions');
+  });
 
   const mockCluster = {
     cluster: {
@@ -196,10 +202,10 @@ describe('test for pipeline services', () => {
     AWSMock.mock('DynamoDB', 'getItem', (params, callback) => {
       if (params.TableName.match('experiments')) {
         getProcessingConfigSpy(params);
-        callback(null, _.cloneDeep(MockProcessingConfig));
+        callback(null, MockProcessingConfig);
       } else if (params.TableName.match('samples')) {
         getSamplesSpy(params);
-        callback(null, _.cloneDeep(MockSamples));
+        callback(null, MockSamples);
       }
     });
 
diff --git a/tests/api/route-services/projects.test.js b/tests/api/route-services/projects.test.js
index adc369752..ff0445b60 100644
--- a/tests/api/route-services/projects.test.js
+++ b/tests/api/route-services/projects.test.js
@@ -252,13 +252,14 @@ describe('tests for the projects service', () => {
 
   it('DeleteProject deletes project and samples properly', async (done) => {
     const experiments = ['project-1'];
+    const samples = [];
 
     const marshalledKey = AWS.DynamoDB.Converter.marshall({
       projectUuid: 'project-1',
     });
 
     const deleteSpy = mockDynamoDeleteItem();
-    const getSpy = mockDynamoGetItem({ projects: { experiments } });
+    const getSpy = mockDynamoGetItem({ projects: { experiments, samples } });
 
     (new ProjectsService()).deleteProject('project-1')
       .then((res) => {
diff --git a/tests/api/route-services/samples.test.js b/tests/api/route-services/samples.test.js
index 35affc2dd..7484396a2 100644
--- a/tests/api/route-services/samples.test.js
+++ b/tests/api/route-services/samples.test.js
@@ -8,6 +8,7 @@ const {
   mockDynamoQuery,
   mockDynamoUpdateItem,
   mockDynamoDeleteItem,
+  mockS3DeleteObjects,
 } = require('../../test-utils/mockAWSServices');
 
 describe('tests for the samples service', () => {
@@ -19,7 +20,6 @@ describe('tests for the samples service', () => {
   it('Get samples by projectUuid works', async (done) => {
     const jsData = {
       samples: {
-        ids: ['sample-1'],
         'sample-1': {
           name: 'sample-1',
         },
@@ -48,7 +48,6 @@ describe('tests for the samples service', () => {
   it('Get sample by experimentId works', async (done) => {
     const jsData = {
       samples: {
-        ids: ['sample-1', 'sample-2'],
         'sample-1': { name: 'sample-1' },
         'sample-2': { name: 'sample-2' },
       },
@@ -112,15 +111,29 @@ describe('tests for the samples service', () => {
       experimentId: 'experiment-1',
     });
 
-    const getFnSpy = mockDynamoDeleteItem();
+    const deleteDynamoFnSpy = mockDynamoDeleteItem();
+    const deleteS3FnSpy = mockS3DeleteObjects({ Errors: [] });
+
+    const s3DeleteParams = {
+      Bucket: 'biomage-originals-test',
+      Delete: {
+        Objects: [
+          { Key: 'project-1/sampleUuid-1/barcodes.tsv.gz' },
+          { Key: 'project-1/sampleUuid-1/features.tsv.gz' },
+          { Key: 'project-1/sampleUuid-1/matrix.mtx.gz' },
+        ],
+        Quiet: false,
+      },
+    };
 
-    (new SamplesService()).deleteSamples('project-1', 'experiment-1')
+    (new SamplesService()).deleteSamplesEntry('project-1', 'experiment-1', ['sampleUuid-1'])
       .then((data) => {
         expect(data).toEqual(OK());
-        expect(getFnSpy).toHaveBeenCalledWith({
+        expect(deleteDynamoFnSpy).toHaveBeenCalledWith({
           TableName: 'samples-test',
           Key: marshalledKey,
         });
+        expect(deleteS3FnSpy).toHaveBeenCalledWith(s3DeleteParams);
       })
       .then(() => done());
   });
diff --git a/tests/test-utils/mockAWSServices.js b/tests/test-utils/mockAWSServices.js
index a7b2e9af6..a849658ed 100644
--- a/tests/test-utils/mockAWSServices.js
+++ b/tests/test-utils/mockAWSServices.js
@@ -116,6 +116,18 @@ const mockS3PutObject = (payload = {}, error = null) => {
   return fnSpy;
 };
 
+const mockS3DeleteObjects = (payload = {}, error = null) => {
+  const fnSpy = jest.fn((x) => x);
+  AWSMock.setSDKInstance(AWS);
+
+  AWSMock.mock('S3', 'deleteObjects', (params, callback) => {
+    fnSpy(params);
+    callback(error, payload);
+  });
+
+  return fnSpy;
+};
+
 module.exports = {
   mockDynamoGetItem,
   mockDynamoBatchGetItem,
@@ -125,4 +137,5 @@ module.exports = {
   mockDynamoDeleteItem,
   mockS3GetObject,
   mockS3PutObject,
+  mockS3DeleteObjects,
 };

From 7a04d309372f1db298105ae32f89c0af8a1517ea Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Mon, 7 Jun 2021 13:49:35 -0300
Subject: [PATCH 6/9] Fix tests and cleanup code

---
 src/api/route-services/__mocks__/samples.js       | 1 -
 src/specs/models/api-body-schemas/Samples.v1.yaml | 2 +-
 tests/api/event-services/work-request.test.js     | 2 --
 tests/api/route-services/samples.test.js          | 1 -
 tests/api/routes/samples.test.js                  | 1 -
 5 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/api/route-services/__mocks__/samples.js b/src/api/route-services/__mocks__/samples.js
index 7781d8422..dcd44dd63 100644
--- a/src/api/route-services/__mocks__/samples.js
+++ b/src/api/route-services/__mocks__/samples.js
@@ -1,7 +1,6 @@
 const { OK } = require('../../../utils/responses');
 
 const mockSamples = {
-  ids: ['sample-1'],
   'sample-1': {
     name: 'sample-1',
     projectUuid: 'project-1',
diff --git a/src/specs/models/api-body-schemas/Samples.v1.yaml b/src/specs/models/api-body-schemas/Samples.v1.yaml
index 84b3fa464..64244d169 100644
--- a/src/specs/models/api-body-schemas/Samples.v1.yaml
+++ b/src/specs/models/api-body-schemas/Samples.v1.yaml
@@ -1,4 +1,4 @@
-title: Samples container object as it is stored
+title: Sample object
 description: Schema for a sample object
 type: object
 additionalProperties:
diff --git a/tests/api/event-services/work-request.test.js b/tests/api/event-services/work-request.test.js
index f1cca58d1..8dc873a93 100644
--- a/tests/api/event-services/work-request.test.js
+++ b/tests/api/event-services/work-request.test.js
@@ -155,8 +155,6 @@ describe('handleWorkRequest', () => {
     try {
       await handleWorkRequest(workRequest, socket);
     } catch (e) {
-      console.log(e.message);
-
       expect(e.message).toMatch(
         'Work request can not be handled because pipeline is RUNNING',
       );
diff --git a/tests/api/route-services/samples.test.js b/tests/api/route-services/samples.test.js
index 7484396a2..e75491055 100644
--- a/tests/api/route-services/samples.test.js
+++ b/tests/api/route-services/samples.test.js
@@ -76,7 +76,6 @@ describe('tests for the samples service', () => {
       projectUuid: 'project-1',
       experimentId: 'experiment-1',
       samples: {
-        ids: ['sample-1', 'sample-2'],
         'sample-1': { name: 'sample-1' },
         'sample-2': { name: 'sample-2' },
       },
diff --git a/tests/api/routes/samples.test.js b/tests/api/routes/samples.test.js
index ee8363565..54f2aa627 100644
--- a/tests/api/routes/samples.test.js
+++ b/tests/api/routes/samples.test.js
@@ -46,7 +46,6 @@ describe('tests for samples route', () => {
       projectUuid: 'project-uuid',
       experimentId: 'experiment-id',
       samples: {
-        ids: ['sample-1'],
         'sample-1': {
           name: 'sample-1',
         },

From 20f3d1a1bf4d28a274dfa8894f2a99848b76c709 Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Mon, 7 Jun 2021 14:46:23 -0300
Subject: [PATCH 7/9] Reorder

---
 src/specs/api.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/specs/api.yaml b/src/specs/api.yaml
index df31c93d6..244af64cd 100644
--- a/src/specs/api.yaml
+++ b/src/specs/api.yaml
@@ -806,6 +806,7 @@ paths:
           type: string
     get:
       summary: Get samples under a project
+      description: Get all samples under a project
       operationId: getProjectSamples
       x-eov-operation-id: samples#get
       x-eov-operation-handler: routes/samples
@@ -831,7 +832,6 @@ paths:
             application/json:
               schema:
                 $ref: ./models/HTTPError.v1.yaml
-      description: Get all samples under a project
   '/projects/{projectUuid}/{experimentId}/samples':
       parameters:
         - name: projectUuid

From cb334eeb3ebce7e2e3e34a7623d3a6e123302820 Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Tue, 8 Jun 2021 18:33:19 -0300
Subject: [PATCH 8/9] Get sample files instead of hardcoded files

---
 src/api/route-services/samples.js | 34 ++++++++++++++++++++-----------
 src/api/route-services/utils.js   | 16 ++++++++++++++-
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/src/api/route-services/samples.js b/src/api/route-services/samples.js
index 5190bf2d1..122a7a372 100644
--- a/src/api/route-services/samples.js
+++ b/src/api/route-services/samples.js
@@ -2,6 +2,8 @@ const _ = require('lodash');
 
 const { NotFoundError, OK } = require('../../utils/responses');
 
+const { undefinedIfNotFound } = require('./utils');
+
 const config = require('../../config');
 const {
   createDynamoDbInstance, convertToJsObject, convertToDynamoDbRecord,
@@ -105,16 +107,14 @@ class SamplesService {
     }
   }
 
-  async deleteSamplesFromS3(projectUuid, sampleUuids) {
+  async deleteSamplesFromS3(projectUuid, samplesToRemoveUuids, allSamples) {
     const s3 = new AWS.S3();
 
-    const sampleObjectsToDelete = sampleUuids.map((sampleUuid) => (
-      [
-        { Key: `${projectUuid}/${sampleUuid}/barcodes.tsv.gz` },
-        { Key: `${projectUuid}/${sampleUuid}/features.tsv.gz` },
-        { Key: `${projectUuid}/${sampleUuid}/matrix.mtx.gz` },
-      ]
-    ));
+    const sampleObjectsToDelete = samplesToRemoveUuids.map((sampleUuid) => {
+      const fileKeysToDelete = Object.keys(allSamples[sampleUuid].files);
+
+      return fileKeysToDelete.map((fileKey) => ({ Key: `${projectUuid}/${sampleUuid}/${fileKey}` }));
+    });
 
     const s3Params = {
       Bucket: this.sampleFilesBucketName,
@@ -138,7 +138,7 @@ class SamplesService {
       experimentId,
     });
 
-    const removeSamplesList = sampleUuids.map((sampleUuid, index) => `samples.#val${index}`);
+    const updateExpressionList = sampleUuids.map((sampleUuid, index) => `samples.#val${index}`);
     const expressionAttributeNames = sampleUuids.reduce((acc, sampleId, index) => {
       acc[`#val${index}`] = sampleId;
       return acc;
@@ -147,14 +147,20 @@ class SamplesService {
     const params = {
       TableName: this.tableName,
       Key: marshalledKey,
-      UpdateExpression: `REMOVE ${removeSamplesList.join(', ')}`,
+      UpdateExpression: `REMOVE ${updateExpressionList.join(', ')}`,
       ExpressionAttributeNames: expressionAttributeNames,
       ReturnValues: 'ALL_NEW',
     };
 
+    const a = await undefinedIfNotFound(
+      this.getSamplesByExperimentId(experimentId),
+    ) || {};
+
+    const { samples: allSamples = {} } = a;
+
     const promises = [
       createDynamoDbInstance().updateItem(params).promise(),
-      this.deleteSamplesFromS3(projectUuid, sampleUuids),
+      this.deleteSamplesFromS3(projectUuid, sampleUuids, allSamples),
     ];
 
     await Promise.all(promises);
@@ -165,6 +171,10 @@ class SamplesService {
   async deleteSamplesEntry(projectUuid, experimentId, sampleUuids) {
     logger.log(`Deleting samples entry for project ${projectUuid} and expId ${experimentId}`);
 
+    const { samples: allSamples = {} } = await undefinedIfNotFound(
+      this.getSamplesByExperimentId(experimentId),
+    ) || {};
+
     const marshalledKey = convertToDynamoDbRecord({
       experimentId,
     });
@@ -186,7 +196,7 @@ class SamplesService {
     }
 
     if (sampleUuids.length) {
-      promises.push(await this.deleteSamplesFromS3(projectUuid, sampleUuids));
+      promises.push(await this.deleteSamplesFromS3(projectUuid, sampleUuids, allSamples));
     }
 
     await Promise.all(promises);
diff --git a/src/api/route-services/utils.js b/src/api/route-services/utils.js
index 2230406d9..a57fd7040 100644
--- a/src/api/route-services/utils.js
+++ b/src/api/route-services/utils.js
@@ -1,6 +1,8 @@
 // Updates each sub attribute separately for
 // one particular attribute (of type object) of a dynamodb entry
 
+const { NotFoundError } = require('../../utils/responses');
+
 const removePropertiesFromObject = async (
   entryKey, attributeName, propertyToRemove,
   tableName, dynamodb,
@@ -15,4 +17,16 @@ const removePropertiesFromObject = async (
   await dynamodb.updateItem(params).promise();
 };
 
-module.exports = { removePropertiesFromObject };
+const undefinedIfNotFound = async (promise) => {
+  try {
+    return await promise;
+  } catch (e) {
+    if (e instanceof NotFoundError) {
+      return undefined;
+    }
+
+    throw e;
+  }
+};
+
+module.exports = { removePropertiesFromObject, undefinedIfNotFound };

From 778f78f9acbc13824df5e13020725d3fdbcffbdb Mon Sep 17 00:00:00 2001
From: Martin Fosco <martinfosco@Martins-MacBook-Pro.local>
Date: Tue, 8 Jun 2021 18:47:02 -0300
Subject: [PATCH 9/9] Fix tests

---
 tests/api/route-services/projects.test.js |  2 +-
 tests/api/route-services/samples.test.js  | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/tests/api/route-services/projects.test.js b/tests/api/route-services/projects.test.js
index ff0445b60..38baa9761 100644
--- a/tests/api/route-services/projects.test.js
+++ b/tests/api/route-services/projects.test.js
@@ -259,7 +259,7 @@ describe('tests for the projects service', () => {
     });
 
     const deleteSpy = mockDynamoDeleteItem();
-    const getSpy = mockDynamoGetItem({ projects: { experiments, samples } });
+    const getSpy = mockDynamoGetItem({ projects: { experiments, samples }, samples });
 
     (new ProjectsService()).deleteProject('project-1')
       .then((res) => {
diff --git a/tests/api/route-services/samples.test.js b/tests/api/route-services/samples.test.js
index e75491055..ae930fecc 100644
--- a/tests/api/route-services/samples.test.js
+++ b/tests/api/route-services/samples.test.js
@@ -111,6 +111,19 @@ describe('tests for the samples service', () => {
     });
 
     const deleteDynamoFnSpy = mockDynamoDeleteItem();
+
+    mockDynamoGetItem({
+      samples: {
+        'sampleUuid-1': {
+          files: {
+            'barcodes.tsv.gz': {},
+            'features.tsv.gz': {},
+            'matrix.mtx.gz': {},
+          },
+        },
+      },
+    });
+
     const deleteS3FnSpy = mockS3DeleteObjects({ Errors: [] });
 
     const s3DeleteParams = {
@@ -125,7 +138,7 @@ describe('tests for the samples service', () => {
       },
     };
 
-    (new SamplesService()).deleteSamplesEntry('project-1', 'experiment-1', ['sampleUuid-1'])
+    (new SamplesService()).deleteSamplesEntry('project-1', 'experiment-1', ['sampleUuid-1'], {})
       .then((data) => {
         expect(data).toEqual(OK());
         expect(deleteDynamoFnSpy).toHaveBeenCalledWith({