From 18b9a83c6c8c439098e8920a2ffe6c79a564d40c Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Wed, 4 May 2022 16:29:28 +0100
Subject: [PATCH 01/16] get and create cell sets

---
 src/api.v2/controllers/cellSetsController.js  | 45 ++++++++++
 src/api.v2/helpers/s3/S3Client.js             | 18 ++++
 src/api.v2/helpers/s3/bucketNames.js          |  1 +
 src/api.v2/helpers/s3/getObject.js            | 29 ++++++
 src/api.v2/helpers/s3/patchCellSetsObject.js  | 35 ++++++++
 src/api.v2/helpers/s3/putObject.js            | 22 +++++
 src/api.v2/routes/cellSets.js                 | 17 ++++
 src/specs/api.v2.yaml                         | 89 +++++++++++++++++++
 .../models/cell-sets-bodies/CellSets.v2.yaml  | 23 +++++
 .../v1Compatibility/formatExperimentId.js     |  8 ++
 10 files changed, 287 insertions(+)
 create mode 100644 src/api.v2/controllers/cellSetsController.js
 create mode 100644 src/api.v2/helpers/s3/S3Client.js
 create mode 100644 src/api.v2/helpers/s3/getObject.js
 create mode 100644 src/api.v2/helpers/s3/patchCellSetsObject.js
 create mode 100644 src/api.v2/helpers/s3/putObject.js
 create mode 100644 src/api.v2/routes/cellSets.js
 create mode 100644 src/specs/models/cell-sets-bodies/CellSets.v2.yaml
 create mode 100644 src/utils/v1Compatibility/formatExperimentId.js

diff --git a/src/api.v2/controllers/cellSetsController.js b/src/api.v2/controllers/cellSetsController.js
new file mode 100644
index 000000000..4a2e197b8
--- /dev/null
+++ b/src/api.v2/controllers/cellSetsController.js
@@ -0,0 +1,45 @@
+const getLogger = require('../../utils/getLogger');
+
+const getS3Object = require('../helpers/s3/getObject');
+const bucketNames = require('../helpers/s3/bucketNames');
+const formatExperimentId = require('../../utils/v1Compatibility/formatExperimentId');
+
+const { OK } = require('../../utils/responses');
+
+const patchCellSetsObject = require('../helpers/s3/patchCellSetsObject');
+
+const logger = getLogger('[CellSetsController] - ');
+
+const getCellSets = async (req, res) => {
+  let { experimentId } = req.params;
+
+  experimentId = experimentId.replace(/-/g, '');
+
+  logger.log(`Getting cell sets for experiment ${experimentId}`);
+
+  const cellSets = await getS3Object({
+    Bucket: bucketNames.CELL_SETS,
+    Key: formatExperimentId(experimentId),
+  });
+
+  logger.log(`Finished getting cell sets for experiment ${experimentId}`);
+
+  res.json(cellSets);
+};
+
+const patchCellSets = async (req, res) => {
+  const { experimentId } = req.params;
+  const patch = req.body;
+
+  logger.log(`Patching cell sets for ${experimentId}`);
+  await patchCellSetsObject(formatExperimentId(experimentId), patch);
+
+  logger.log(`Finished patching cell sets for experiment ${experimentId}`);
+
+  res.json(OK());
+};
+
+module.exports = {
+  getCellSets,
+  patchCellSets,
+};
diff --git a/src/api.v2/helpers/s3/S3Client.js b/src/api.v2/helpers/s3/S3Client.js
new file mode 100644
index 000000000..6f7f9b540
--- /dev/null
+++ b/src/api.v2/helpers/s3/S3Client.js
@@ -0,0 +1,18 @@
+const AWS = require('../../../utils/requireAWS');
+const config = require('../../../config');
+
+// Wanted to make this a wrapper class that extends S3,
+// but it's not advisable to do so:
+// https://github.com/aws/aws-sdk-js/issues/2006
+const getS3Client = (options) => {
+  const S3Config = {
+    apiVersion: '2006-03-01',
+    signatureVersion: 'v4',
+    region: config.awsRegion,
+    ...options,
+  };
+
+  return new AWS.S3(S3Config);
+};
+
+module.exports = getS3Client;
diff --git a/src/api.v2/helpers/s3/bucketNames.js b/src/api.v2/helpers/s3/bucketNames.js
index 3a0cdca5e..93250b727 100644
--- a/src/api.v2/helpers/s3/bucketNames.js
+++ b/src/api.v2/helpers/s3/bucketNames.js
@@ -2,6 +2,7 @@ const config = require('../../../config');
 
 const bucketNames = {
   SAMPLE_FILES: `biomage-originals-${config.clusterEnv}`,
+  CELL_SETS: `cell-sets-${config.clusterEnv}`,
 };
 
 module.exports = bucketNames;
diff --git a/src/api.v2/helpers/s3/getObject.js b/src/api.v2/helpers/s3/getObject.js
new file mode 100644
index 000000000..206ed3fc8
--- /dev/null
+++ b/src/api.v2/helpers/s3/getObject.js
@@ -0,0 +1,29 @@
+const getS3Client = require('./S3Client');
+const NotFoundError = require('../../../utils/responses/NotFoundError');
+
+const getObject = async (params) => {
+  if (!params.Bucket) throw new Error('Bucket is required');
+  if (!params.Key) throw new Error('Key is required');
+
+  const s3 = getS3Client();
+
+  try {
+    const outputObject = await s3.getObject(params).promise();
+
+    const data = JSON.parse(outputObject.Body.toString());
+
+    return data;
+  } catch (e) {
+    if (e.code === 'NoSuchKey') {
+      throw new NotFoundError(`Couldn't find object with key: ${params.Key}`);
+    }
+
+    if (e.code === 'NoSuchBucket') {
+      throw new NotFoundError(`Couldn't find bucket with key: ${params.Bucket}`);
+    }
+
+    throw e;
+  }
+};
+
+module.exports = getObject;
diff --git a/src/api.v2/helpers/s3/patchCellSetsObject.js b/src/api.v2/helpers/s3/patchCellSetsObject.js
new file mode 100644
index 000000000..28082669a
--- /dev/null
+++ b/src/api.v2/helpers/s3/patchCellSetsObject.js
@@ -0,0 +1,35 @@
+const jsonMerger = require('json-merger');
+
+const bucketNames = require('./bucketNames');
+const getObject = require('./getObject');
+const putObject = require('./putObject');
+
+const patchCellSetsObject = async (experimentId, patch) => {
+  const initialCellSet = await getObject({
+    Bucket: bucketNames.CELL_SETS,
+    Key: experimentId,
+  });
+
+  const { cellSets: cellSetsList } = initialCellSet;
+
+  /**
+   * The $remove operation will replace the element in the array with an
+   * undefined value. We will therefore remove this from the array.
+   *
+   * We use the $remove operation in the worker to update cell clusters,
+   * and we may end up using it in other places in the future.
+   */
+  const patchedArray = jsonMerger.mergeObjects(
+    [cellSetsList, patch],
+  ).filter((x) => x !== undefined);
+
+  const patchedCellSet = JSON.stringify({ cellSets: patchedArray });
+
+  await putObject({
+    Bucket: bucketNames.CELL_SETS,
+    Key: experimentId,
+    Body: patchedCellSet,
+  });
+};
+
+module.exports = patchCellSetsObject;
diff --git a/src/api.v2/helpers/s3/putObject.js b/src/api.v2/helpers/s3/putObject.js
new file mode 100644
index 000000000..f7b3f5433
--- /dev/null
+++ b/src/api.v2/helpers/s3/putObject.js
@@ -0,0 +1,22 @@
+const NotFoundError = require('../../../utils/responses/NotFoundError');
+const getS3Client = require('./S3Client');
+
+const pubObject = async (params) => {
+  if (!params.Bucket) throw new Error('Bucket is required');
+  if (!params.Key) throw new Error('Key is required');
+  if (!params.Body) throw new Error('Body is required');
+
+  const s3 = getS3Client();
+
+  try {
+    await s3.putObject(params).promise();
+  } catch (e) {
+    if (e.code === 'NoSuchBucket') {
+      throw new NotFoundError(`Couldn't find bucket with key: ${params.Bucket}`);
+    }
+
+    throw e;
+  }
+};
+
+module.exports = pubObject;
diff --git a/src/api.v2/routes/cellSets.js b/src/api.v2/routes/cellSets.js
new file mode 100644
index 000000000..ef062e0b0
--- /dev/null
+++ b/src/api.v2/routes/cellSets.js
@@ -0,0 +1,17 @@
+const {
+  getCellSets,
+  patchCellSets,
+} = require('../controllers/cellSetsController');
+
+const { expressAuthorizationMiddleware } = require('../middlewares/authMiddlewares');
+
+module.exports = {
+  'cellSets#getCellSets': [
+    expressAuthorizationMiddleware,
+    (req, res, next) => getCellSets(req, res).catch(next),
+  ],
+  'cellSets#patchCellSets': [
+    expressAuthorizationMiddleware,
+    (req, res, next) => patchCellSets(req, res).catch(next),
+  ],
+};
diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml
index a2679e795..9d59e9ecc 100644
--- a/src/specs/api.v2.yaml
+++ b/src/specs/api.v2.yaml
@@ -58,6 +58,93 @@ paths:
                   - env
                   - clusterEnv
       description: Returns a status on the health of the API.
+
+  '/experiments/{experimentId}/cellSets':
+    parameters:
+      - name: experimentId
+        in: path
+        description: ID of experiment to find cell sets of.
+        required: true
+        schema:
+          type: string
+    get:
+      tags:
+        - experiments
+      summary: Get cell sets for experiment
+      description: Returns a hirearchical view of cell sets in the experiment.
+      operationId: getExperimentCellSets
+      x-eov-operation-id: cellSets#getCellSets
+      x-eov-operation-handler: routes/cellSets
+      responses:
+        '200':
+          description: 'Request successful, hierarchy returned below.'
+          content:
+            application/json:
+              schema:
+                type: object
+                properties:
+                  cellSets:
+                    type: array
+                    items:
+                      $ref: '#/components/schemas/CellSets'
+        '401':
+          description: The request lacks authentication credentials.
+          content:
+            application/json:
+              schema:
+                $ref: ./models/HTTPError.v1.yaml
+        '403':
+          description: The authenticated user is not authorized to view this resource.
+          content:
+            application/json:
+              schema:
+                $ref: ./models/HTTPError.v1.yaml
+        '404':
+          description: Experiment not found.
+          content:
+            application/json:
+              schema:
+                $ref: ./models/HTTPError.v1.yaml
+    patch:
+      summary: ''
+      operationId: patchExperimentCellSets
+      x-eov-operation-id: cellSets#patchCellSets
+      x-eov-operation-handler: routes/cellSets
+      responses:
+        '200':
+          description: Update to object in response successful.
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  allOf:
+                    - $ref: '#/components/schemas/CellSets'
+        '401':
+          description: The request lacks authentication credentials.
+          content:
+            application/json:
+              schema:
+                $ref: ./models/HTTPError.v1.yaml
+        '403':
+          description: The authenticated user is not authorized to view this resource.
+          content:
+            application/json:
+              schema:
+                $ref: ./models/HTTPError.v1.yaml
+      description: Performs a partial update on the experiment's cell sets.
+      requestBody:
+        content:
+          application/boschni-json-merger+json:
+            schema:
+              oneOf:
+                - type: array
+                  items: {}
+                - type: object
+        description: The patch in the format declared by boschni/json-merger. Note the explicit naming of the content subtype (boschni-json-merger+json) which must be used to explicitly acknowledge the format.
+
+
+
   '/experiments/{experimentId}/processingConfig':
     get:
       summary: Get processing configuration for an experiment
@@ -904,6 +991,8 @@ components:
       $ref: './models/samples-bodies/CreateSampleFile.v2.yaml'
     PatchSampleFile:
       $ref: './models/samples-bodies/PatchSampleFile.v2.yaml'
+    CellSets:
+      $ref: ./models/cell-sets-bodies/CellSets.v2.yaml
     HTTPSuccess:
       $ref: './models/HTTPSuccess.v1.yaml'
     HTTPError:
diff --git a/src/specs/models/cell-sets-bodies/CellSets.v2.yaml b/src/specs/models/cell-sets-bodies/CellSets.v2.yaml
new file mode 100644
index 000000000..4c3686fe3
--- /dev/null
+++ b/src/specs/models/cell-sets-bodies/CellSets.v2.yaml
@@ -0,0 +1,23 @@
+title: Cell Sets Object
+type: object
+description: Schema for CellSets object
+properties:
+  key:
+    type: string
+    format: int32
+  name:
+    type: string
+  rootNode:
+    type: boolean
+  children:
+    oneOf:
+      - minItems: 0
+        maxItems: 0
+        items: {}
+      - minItems: 1
+        items:
+          $ref: ./CellSets.v2.yaml
+    type: array
+required:
+  - key
+  - name
diff --git a/src/utils/v1Compatibility/formatExperimentId.js b/src/utils/v1Compatibility/formatExperimentId.js
new file mode 100644
index 000000000..4b1becb9b
--- /dev/null
+++ b/src/utils/v1Compatibility/formatExperimentId.js
@@ -0,0 +1,8 @@
+// Existing experimentId in S3 (v1) are MD5 hashes not UUIDs.
+// They have the same number of alhpanum characters as UUIDs but no dashes
+// To maintain compatibility with v1, we remove the dashes from UUIDs.
+// This function should be removed once we have migrated to v2.
+
+const formatExperimentId = (experimentId) => experimentId.replace(/-/g, '');
+
+module.exports = formatExperimentId;

From ad050b8d9041b6c2fd15e0ad87455ae286c3294e Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Thu, 5 May 2022 12:06:20 +0100
Subject: [PATCH 02/16] add get object test

---
 src/api.v2/helpers/s3/getObject.js        |  4 +-
 tests/api.v2/helpers/s3/getObject.test.js | 85 +++++++++++++++++++++++
 2 files changed, 86 insertions(+), 3 deletions(-)
 create mode 100644 tests/api.v2/helpers/s3/getObject.test.js

diff --git a/src/api.v2/helpers/s3/getObject.js b/src/api.v2/helpers/s3/getObject.js
index 206ed3fc8..1979afa79 100644
--- a/src/api.v2/helpers/s3/getObject.js
+++ b/src/api.v2/helpers/s3/getObject.js
@@ -9,9 +9,7 @@ const getObject = async (params) => {
 
   try {
     const outputObject = await s3.getObject(params).promise();
-
-    const data = JSON.parse(outputObject.Body.toString());
-
+    const data = outputObject.Body.toString();
     return data;
   } catch (e) {
     if (e.code === 'NoSuchKey') {
diff --git a/tests/api.v2/helpers/s3/getObject.test.js b/tests/api.v2/helpers/s3/getObject.test.js
new file mode 100644
index 000000000..84577d355
--- /dev/null
+++ b/tests/api.v2/helpers/s3/getObject.test.js
@@ -0,0 +1,85 @@
+// @ts-nocheck
+const getObject = require('../../../../src/api.v2/helpers/s3/getObject');
+const getS3Client = require('../../../../src/api.v2/helpers/s3/getS3Client');
+
+const NotFoundError = require('../../../../src/utils/responses/NotFoundError');
+
+jest.mock('../../../../src/api.v2/helpers/s3/S3Client', () => jest.fn(() => ({
+  getObject: jest.fn(() => (
+    {
+      promise: () => Promise.resolve({
+        Body: {
+          toString: () => 'some data',
+        },
+      }),
+    }
+  )),
+})));
+
+const mockBucketName = 'mock-bucket';
+const mockKeyName = 'mock-key';
+
+class MockS3Error extends Error {
+  constructor(message, code) {
+    super(message);
+    this.code = code;
+  }
+}
+
+const mockParam = {
+  Bucket: mockBucketName,
+  Key: mockKeyName,
+};
+
+describe('getObject', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('Throws an error if param is in complete', async () => {
+    await expect(getObject()).rejects.toThrow();
+
+    const noBucketParam = { ...mockParam };
+    delete noBucketParam.Bucket;
+
+    await expect(getObject(noBucketParam)).rejects.toThrow();
+
+    const noKeyParam = { ...mockParam };
+    delete noKeyParam.Key;
+
+    await expect(getObject(noKeyParam)).rejects.toThrow();
+  });
+
+  it('Returns data from S3', async () => {
+    const data = await getObject(mockParam);
+    expect(data).toEqual('some data');
+  });
+
+  it('Throws NotFoundError if bucket is not found', async () => {
+    getS3Client.mockImplementation(() => ({
+      getObject: jest.fn(() => { throw new MockS3Error('no bucket', 'NoSuchBucket'); }),
+    }));
+
+    const errorText = `Couldn't find bucket with key: ${mockBucketName}`;
+    await expect(getObject(mockParam)).rejects.toThrow(new NotFoundError(errorText));
+  });
+
+  it('Throws NotFoundError if key is not found', async () => {
+    getS3Client.mockImplementation(() => ({
+      getObject: jest.fn(() => { throw new MockS3Error('no object with key', 'NoSuchKey'); }),
+    }));
+
+    const errorText = `Couldn't find object with key: ${mockKeyName}`;
+    await expect(getObject(mockParam)).rejects.toThrow(new NotFoundError(errorText));
+  });
+
+  it('Throws a general error if the error is not handled manually', async () => {
+    const errMsg = 'key too long';
+
+    getS3Client.mockImplementation(() => ({
+      getObject: jest.fn(() => { throw new MockS3Error(errMsg, 'KeyTooLongError'); }),
+    }));
+
+    await expect(getObject(mockParam)).rejects.toThrow(errMsg);
+  });
+});

From 7e283885eddff81373cb9b4c72189df52f698e8a Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Thu, 5 May 2022 12:07:04 +0100
Subject: [PATCH 03/16] add put object test

---
 src/api.v2/helpers/s3/putObject.js        |  2 +-
 tests/api.v2/helpers/s3/putObject.test.js | 78 +++++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 tests/api.v2/helpers/s3/putObject.test.js

diff --git a/src/api.v2/helpers/s3/putObject.js b/src/api.v2/helpers/s3/putObject.js
index f7b3f5433..29ea2aa36 100644
--- a/src/api.v2/helpers/s3/putObject.js
+++ b/src/api.v2/helpers/s3/putObject.js
@@ -1,5 +1,5 @@
 const NotFoundError = require('../../../utils/responses/NotFoundError');
-const getS3Client = require('./S3Client');
+const getS3Client = require('./getS3Client');
 
 const pubObject = async (params) => {
   if (!params.Bucket) throw new Error('Bucket is required');
diff --git a/tests/api.v2/helpers/s3/putObject.test.js b/tests/api.v2/helpers/s3/putObject.test.js
new file mode 100644
index 000000000..132f3928b
--- /dev/null
+++ b/tests/api.v2/helpers/s3/putObject.test.js
@@ -0,0 +1,78 @@
+// @ts-nocheck
+const putObject = require('../../../../src/api.v2/helpers/s3/putObject');
+const getS3Client = require('../../../../src/api.v2/helpers/s3/getS3Client');
+
+const NotFoundError = require('../../../../src/utils/responses/NotFoundError');
+
+jest.mock('../../../../src/api.v2/helpers/s3/S3Client', () => jest.fn(() => ({
+  putObject: jest.fn(() => (
+    {
+      promise: () => Promise.resolve({}),
+    }
+  )),
+})));
+
+class MockS3Error extends Error {
+  constructor(message, code) {
+    super(message);
+    this.code = code;
+  }
+}
+
+const mockBucketName = 'mock-bucket';
+const mockKeyName = 'mock-key';
+const mockBody = 'mock-body';
+
+const mockParam = {
+  Bucket: mockBucketName,
+  Key: mockKeyName,
+  Body: mockBody,
+};
+
+describe('putObject', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('Throws an error if param is in complete', async () => {
+    await expect(putObject()).rejects.toThrow();
+
+    const noBucketParam = { ...mockParam };
+    delete noBucketParam.Bucket;
+
+    await expect(putObject(noBucketParam)).rejects.toThrow();
+
+    const noKeyParam = { ...mockParam };
+    delete noKeyParam.Bucket;
+
+    await expect(putObject(noKeyParam)).rejects.toThrow();
+
+    const noBodyParam = { ...mockParam };
+    delete noBodyParam.Bucket;
+
+    await expect(putObject(noKeyParam)).rejects.toThrow();
+  });
+
+  it('Does not return anything on success', async () => {
+    await expect(putObject(mockParam)).resolves.toBeUndefined();
+  });
+
+  it('Throws NotFoundError if bucket is not found', async () => {
+    getS3Client.mockImplementation(() => ({
+      putObject: jest.fn(() => { throw new MockS3Error('no bucket', 'NoSuchBucket'); }),
+    }));
+
+    const errorText = `Couldn't find bucket with key: ${mockBucketName}`;
+    await expect(putObject(mockParam)).rejects.toThrow(new NotFoundError(errorText));
+  });
+
+  it('Throws a general error if the error is not handled manually', async () => {
+    const errMsg = 'key too long';
+
+    getS3Client.mockImplementation(() => ({
+      putObject: jest.fn(() => { throw new MockS3Error(errMsg, 'KeyTooLongError'); }),
+    }));
+
+    await expect(putObject(mockParam)).rejects.toThrow(errMsg);
+  });
+});

From e08c068e98a2da76224da8a4370ad1bf0fd78a96 Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Thu, 5 May 2022 12:07:46 +0100
Subject: [PATCH 04/16] change s3 client name

---
 src/api.v2/helpers/s3/getObject.js                    | 2 +-
 src/api.v2/helpers/s3/{S3Client.js => getS3Client.js} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename src/api.v2/helpers/s3/{S3Client.js => getS3Client.js} (100%)

diff --git a/src/api.v2/helpers/s3/getObject.js b/src/api.v2/helpers/s3/getObject.js
index 1979afa79..a5ded1b3e 100644
--- a/src/api.v2/helpers/s3/getObject.js
+++ b/src/api.v2/helpers/s3/getObject.js
@@ -1,4 +1,4 @@
-const getS3Client = require('./S3Client');
+const getS3Client = require('./getS3Client');
 const NotFoundError = require('../../../utils/responses/NotFoundError');
 
 const getObject = async (params) => {
diff --git a/src/api.v2/helpers/s3/S3Client.js b/src/api.v2/helpers/s3/getS3Client.js
similarity index 100%
rename from src/api.v2/helpers/s3/S3Client.js
rename to src/api.v2/helpers/s3/getS3Client.js

From 4a8e230214baabfd7f93810d575f88eb74b4ca63 Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Thu, 5 May 2022 12:08:06 +0100
Subject: [PATCH 05/16] add getS3Client test

---
 .../s3/__snapshots__/getS3Client.test.js.snap | 18 +++++++++
 tests/api.v2/helpers/s3/getS3Client.test.js   | 39 +++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 tests/api.v2/helpers/s3/__snapshots__/getS3Client.test.js.snap
 create mode 100644 tests/api.v2/helpers/s3/getS3Client.test.js

diff --git a/tests/api.v2/helpers/s3/__snapshots__/getS3Client.test.js.snap b/tests/api.v2/helpers/s3/__snapshots__/getS3Client.test.js.snap
new file mode 100644
index 000000000..e84829656
--- /dev/null
+++ b/tests/api.v2/helpers/s3/__snapshots__/getS3Client.test.js.snap
@@ -0,0 +1,18 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`getS3Client Returns an S3 client with defau lt config values if not given any params 1`] = `
+Object {
+  "apiVersion": "2006-03-01",
+  "region": "eu-west-1",
+  "signatureVersion": "v4",
+}
+`;
+
+exports[`getS3Client Takes in params and return S3 client with those params 1`] = `
+Object {
+  "apiVersion": "2006-03-01",
+  "endpointUrl": "https://s3.biomage-cloud.com",
+  "region": "us-east-1",
+  "signatureVersion": "v4",
+}
+`;
diff --git a/tests/api.v2/helpers/s3/getS3Client.test.js b/tests/api.v2/helpers/s3/getS3Client.test.js
new file mode 100644
index 000000000..61c9bca2a
--- /dev/null
+++ b/tests/api.v2/helpers/s3/getS3Client.test.js
@@ -0,0 +1,39 @@
+const getS3Client = require('../../../../src/api.v2/helpers/s3/getS3Client');
+const AWS = require('../../../../src/utils/requireAWS');
+
+jest.mock('../../../../src/utils/requireAWS', () => ({
+  S3: jest.fn((params) => ({ ...params })),
+}));
+
+describe('getS3Client', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('Returns an S3 client with defau lt config values if not given any params', () => {
+    const s3Client = getS3Client();
+
+    expect(AWS.S3).toHaveBeenCalledTimes(1);
+
+    const configParams = AWS.S3.mock.calls[0][0];
+
+    expect(configParams).toMatchSnapshot();
+    expect(s3Client).not.toBeUndefined();
+  });
+
+  it('Takes in params and return S3 client with those params', () => {
+    const additionalParams = {
+      region: 'us-east-1',
+      endpointUrl: 'https://s3.biomage-cloud.com',
+    };
+
+    const s3Client = getS3Client(additionalParams);
+
+    expect(AWS.S3).toHaveBeenCalledTimes(1);
+
+    const configParams = AWS.S3.mock.calls[0][0];
+
+    expect(configParams).toMatchSnapshot();
+    expect(s3Client).not.toBeUndefined();
+  });
+});

From 463b481fdf884641043a4148e759f3ed822eadd2 Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Thu, 5 May 2022 12:51:07 +0100
Subject: [PATCH 06/16] add test for routes

---
 .../__mocks__/cellSetsController.js           |   7 +
 src/api.v2/controllers/cellSetsController.js  |   2 +-
 src/specs/api.v2.yaml                         |   6 +-
 tests/api.v2/routes/cellSets.test.js          | 155 ++++++++++++++++++
 4 files changed, 165 insertions(+), 5 deletions(-)
 create mode 100644 src/api.v2/controllers/__mocks__/cellSetsController.js
 create mode 100644 tests/api.v2/routes/cellSets.test.js

diff --git a/src/api.v2/controllers/__mocks__/cellSetsController.js b/src/api.v2/controllers/__mocks__/cellSetsController.js
new file mode 100644
index 000000000..81147f95f
--- /dev/null
+++ b/src/api.v2/controllers/__mocks__/cellSetsController.js
@@ -0,0 +1,7 @@
+const mockGetCellSets = jest.fn();
+const mockPatchCellSets = jest.fn();
+
+module.exports = {
+  getCellSets: mockGetCellSets,
+  patchCellSets: mockPatchCellSets,
+};
diff --git a/src/api.v2/controllers/cellSetsController.js b/src/api.v2/controllers/cellSetsController.js
index 4a2e197b8..e946c76e3 100644
--- a/src/api.v2/controllers/cellSetsController.js
+++ b/src/api.v2/controllers/cellSetsController.js
@@ -24,7 +24,7 @@ const getCellSets = async (req, res) => {
 
   logger.log(`Finished getting cell sets for experiment ${experimentId}`);
 
-  res.json(cellSets);
+  res.send(cellSets);
 };
 
 const patchCellSets = async (req, res) => {
diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml
index 9d59e9ecc..5e950c6d5 100644
--- a/src/specs/api.v2.yaml
+++ b/src/specs/api.v2.yaml
@@ -116,10 +116,8 @@ paths:
           content:
             application/json:
               schema:
-                type: array
-                items:
-                  allOf:
-                    - $ref: '#/components/schemas/CellSets'
+                type: object
+                properties: {}
         '401':
           description: The request lacks authentication credentials.
           content:
diff --git a/tests/api.v2/routes/cellSets.test.js b/tests/api.v2/routes/cellSets.test.js
new file mode 100644
index 000000000..d98f68aed
--- /dev/null
+++ b/tests/api.v2/routes/cellSets.test.js
@@ -0,0 +1,155 @@
+// @ts-nocheck
+const express = require('express');
+const request = require('supertest');
+const expressLoader = require('../../../src/loaders/express');
+
+const cellSetsController = require('../../../src/api.v2/controllers/cellSetsController');
+const { NotFoundError, OK } = require('../../../src/utils/responses');
+
+jest.mock('../../../src/api.v2/middlewares/authMiddlewares');
+jest.mock('../../../src/api.v2/controllers/cellSetsController');
+
+const endpoint = '/v2/experiments/mockExperimentId/cellSets';
+
+const mockPatch = {
+  key: '05e036a5-a2ae-4909-99e1-c3b927a584e3', name: 'New Cluster', color: '#3957ff', type: 'cellSets', cellIds: [438, 444, 713, 822, 192, 576, 675],
+};
+
+const mockCellSet = {
+  cellSets: [
+    {
+      key: 'louvain',
+      name: 'Louvain',
+      children: [{
+        key: 'louvain-0',
+        name: 'Louvain 0',
+        children: [1, 2, 4],
+      }],
+    },
+    {
+      key: 'scratchpad',
+      name: 'Scratchpad',
+      children: [],
+    },
+  ],
+};
+
+describe('Cell sets endpoint', () => {
+  let app;
+
+  beforeEach(async () => {
+    const mockApp = await expressLoader(express());
+    app = mockApp.app;
+  });
+
+  afterEach(() => {
+    jest.resetModules();
+    jest.restoreAllMocks();
+  });
+
+  it('Getting an existing cell set returns 200', (done) => {
+    cellSetsController.getCellSets.mockImplementationOnce((req, res) => {
+      res.json();
+      Promise.resolve();
+    });
+    request(app)
+      .get(endpoint)
+      .expect(200)
+      .end((err) => {
+        if (err) {
+          return done(err);
+        }
+        return done();
+      });
+  });
+
+  it('Getting a non-existing cell set returns 404', (done) => {
+    cellSetsController.getCellSets.mockImplementationOnce(() => {
+      throw new NotFoundError('Experiment not found');
+    });
+
+    request(app)
+      .get(endpoint)
+      .expect(404)
+      .end((err) => {
+        if (err) {
+          return done(err);
+        }
+        return done();
+      });
+  });
+
+  it('Patching cell sets with a valid body content type results in a successful response', (done) => {
+    cellSetsController.patchCellSets.mockImplementationOnce((req, res) => {
+      res.json();
+      Promise.resolve();
+    });
+
+    const createNewCellSetJsonMerger = [{
+      $match: {
+        query: '$[?(@.key == "scratchpad")]',
+        value: {
+          children: [{
+            $insert: {
+              index: '-',
+              value: mockPatch,
+            },
+          }],
+        },
+      },
+    }];
+
+    const validContentType = 'application/boschni-json-merger+json';
+
+    request(app)
+      .patch(endpoint)
+      .set('Content-type', validContentType)
+      .send(createNewCellSetJsonMerger)
+      .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('Patching cell sets with an invalid body content type results in a 415', (done) => {
+    cellSetsController.patchCellSets.mockImplementationOnce((req, res) => {
+      res.json();
+      Promise.resolve();
+    });
+
+    const createNewCellSetJsonMerger = [{
+      $match: {
+        query: '$[?(@.key == "scratchpad")]',
+        value: {
+          children: [{
+            $insert: {
+              index: '-',
+              value: mockPatch,
+            },
+          }],
+        },
+      },
+    }];
+
+    const invalidContentType = 'application/json';
+
+    request(app)
+      .patch(endpoint)
+      .set('Content-type', invalidContentType)
+      .send(createNewCellSetJsonMerger)
+      .expect(415)
+      .end((err) => {
+        if (err) {
+          return done(err);
+        }
+        // there is no point testing for the values of the response body
+        // - if something is wrong, the schema validator will catch it
+        return done();
+      });
+  });
+});

From 9c6580d03fd1c857f9da49e9ab5d397c18bf704c Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Thu, 5 May 2022 14:27:52 +0100
Subject: [PATCH 07/16] add cell set controller test

---
 src/api.v2/controllers/cellSetsController.js  |   2 +-
 .../controllers/cellSetsController.test.js    | 108 ++++++++++++++++++
 tests/api.v2/routes/cellSets.test.js          |  21 +---
 3 files changed, 110 insertions(+), 21 deletions(-)
 create mode 100644 tests/api.v2/controllers/cellSetsController.test.js

diff --git a/src/api.v2/controllers/cellSetsController.js b/src/api.v2/controllers/cellSetsController.js
index e946c76e3..4a2e197b8 100644
--- a/src/api.v2/controllers/cellSetsController.js
+++ b/src/api.v2/controllers/cellSetsController.js
@@ -24,7 +24,7 @@ const getCellSets = async (req, res) => {
 
   logger.log(`Finished getting cell sets for experiment ${experimentId}`);
 
-  res.send(cellSets);
+  res.json(cellSets);
 };
 
 const patchCellSets = async (req, res) => {
diff --git a/tests/api.v2/controllers/cellSetsController.test.js b/tests/api.v2/controllers/cellSetsController.test.js
new file mode 100644
index 000000000..999c90089
--- /dev/null
+++ b/tests/api.v2/controllers/cellSetsController.test.js
@@ -0,0 +1,108 @@
+// @ts-nocheck
+const cellSetsController = require('../../../src/api.v2/controllers/cellSetsController');
+const bucketNames = require('../../../src/api.v2/helpers/s3/bucketNames');
+
+const getS3Object = require('../../../src/api.v2/helpers/s3/getObject');
+const patchCellSetsObject = require('../../../src/api.v2/helpers/s3/patchCellSetsObject');
+const { OK } = require('../../../src/utils/responses');
+
+const formatExperimentId = require('../../../src/utils/v1Compatibility/formatExperimentId');
+
+jest.mock('../../../src/api.v2/helpers/s3/getObject');
+jest.mock('../../../src/api.v2/helpers/s3/patchCellSetsObject');
+
+const mockRes = {
+  json: jest.fn(),
+};
+
+const mockCellSets = {
+  cellSets:
+  [
+    {
+      key: 'louvain',
+      name: 'louvain clusters',
+      rootNode: true,
+      type: 'cellSets',
+      children: [
+        {
+          key: 'louvain-0',
+          name: 'Cluster 0',
+          rootNode: false,
+          type: 'cellSets',
+          color: '#77aadd',
+          cellIds: [0, 1, 2, 3],
+        },
+      ],
+    },
+  ],
+};
+
+const mockPatch = [
+  {
+    $match: {
+      query: '$[?(@.key == "scratchpad")]',
+      value: {
+        children: [
+          {
+            $insert:
+              {
+                index: '-',
+                value:
+                  {
+                    key: 'new-cluster-1',
+                    name: 'New Cluster 1',
+                    color: '#3957ff',
+                    type: 'cellSets',
+                    cellIds: [4, 5, 6],
+                  },
+              },
+          },
+        ],
+      },
+    },
+  },
+];
+
+const mockExperimentId = '1234-5678-9012';
+
+describe('cellSetsController', () => {
+  beforeEach(async () => {
+    jest.clearAllMocks();
+  });
+
+  it('getCellSets works correctly', async () => {
+    const mockReq = { params: { experimentId: mockExperimentId } };
+    getS3Object.mockImplementationOnce(
+      () => Promise.resolve(mockCellSets),
+    );
+
+    await cellSetsController.getCellSets(mockReq, mockRes);
+
+    expect(getS3Object).toHaveBeenCalledWith({
+      Bucket: bucketNames.CELL_SETS,
+      Key: formatExperimentId(mockExperimentId),
+    });
+
+    expect(mockRes.json).toHaveBeenCalledWith(mockCellSets);
+  });
+
+  it('patchCellSetsObject works correctly', async () => {
+    const mockReq = {
+      params: { experimentId: mockExperimentId },
+      body: mockPatch,
+    };
+
+    patchCellSetsObject.mockImplementationOnce(
+      () => Promise.resolve(null),
+    );
+
+    await cellSetsController.patchCellSets(mockReq, mockRes);
+
+    expect(patchCellSetsObject).toHaveBeenCalledWith(
+      formatExperimentId(mockExperimentId),
+      mockPatch,
+    );
+
+    expect(mockRes.json).toHaveBeenCalledWith(OK());
+  });
+});
diff --git a/tests/api.v2/routes/cellSets.test.js b/tests/api.v2/routes/cellSets.test.js
index d98f68aed..59143d41d 100644
--- a/tests/api.v2/routes/cellSets.test.js
+++ b/tests/api.v2/routes/cellSets.test.js
@@ -4,7 +4,7 @@ const request = require('supertest');
 const expressLoader = require('../../../src/loaders/express');
 
 const cellSetsController = require('../../../src/api.v2/controllers/cellSetsController');
-const { NotFoundError, OK } = require('../../../src/utils/responses');
+const { NotFoundError } = require('../../../src/utils/responses');
 
 jest.mock('../../../src/api.v2/middlewares/authMiddlewares');
 jest.mock('../../../src/api.v2/controllers/cellSetsController');
@@ -15,25 +15,6 @@ const mockPatch = {
   key: '05e036a5-a2ae-4909-99e1-c3b927a584e3', name: 'New Cluster', color: '#3957ff', type: 'cellSets', cellIds: [438, 444, 713, 822, 192, 576, 675],
 };
 
-const mockCellSet = {
-  cellSets: [
-    {
-      key: 'louvain',
-      name: 'Louvain',
-      children: [{
-        key: 'louvain-0',
-        name: 'Louvain 0',
-        children: [1, 2, 4],
-      }],
-    },
-    {
-      key: 'scratchpad',
-      name: 'Scratchpad',
-      children: [],
-    },
-  ],
-};
-
 describe('Cell sets endpoint', () => {
   let app;
 

From 72bcd51586274c3c96b0d4d15611b7dac51bedf5 Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Thu, 5 May 2022 14:46:01 +0100
Subject: [PATCH 08/16] add tests

---
 .../patchCellSetsObject.test.js.snap          |  9 +++
 tests/api.v2/helpers/s3/getObject.test.js     |  2 +-
 .../helpers/s3/patchCellSetsObject.test.js    | 72 +++++++++++++++++++
 tests/api.v2/helpers/s3/putObject.test.js     |  2 +-
 4 files changed, 83 insertions(+), 2 deletions(-)
 create mode 100644 tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
 create mode 100644 tests/api.v2/helpers/s3/patchCellSetsObject.test.js

diff --git a/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap b/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
new file mode 100644
index 000000000..d13063011
--- /dev/null
+++ b/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
@@ -0,0 +1,9 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`patchCellSetsObject Works correctly 1`] = `
+Object {
+  "Body": "{\\"cellSets\\":[{\\"key\\":\\"louvain\\",\\"name\\":\\"louvain clusters\\",\\"rootNode\\":true,\\"type\\":\\"cellSets\\",\\"children\\":[{\\"key\\":\\"louvain-0\\",\\"name\\":\\"Cluster 0\\",\\"rootNode\\":false,\\"type\\":\\"cellSets\\",\\"color\\":\\"#77aadd\\",\\"cellIds\\":[0,1,2,3]}]}]}",
+  "Bucket": "cell-sets-test",
+  "Key": "mock-experiment-id",
+}
+`;
diff --git a/tests/api.v2/helpers/s3/getObject.test.js b/tests/api.v2/helpers/s3/getObject.test.js
index 84577d355..d34cf5cae 100644
--- a/tests/api.v2/helpers/s3/getObject.test.js
+++ b/tests/api.v2/helpers/s3/getObject.test.js
@@ -4,7 +4,7 @@ const getS3Client = require('../../../../src/api.v2/helpers/s3/getS3Client');
 
 const NotFoundError = require('../../../../src/utils/responses/NotFoundError');
 
-jest.mock('../../../../src/api.v2/helpers/s3/S3Client', () => jest.fn(() => ({
+jest.mock('../../../../src/api.v2/helpers/s3/getS3Client', () => jest.fn(() => ({
   getObject: jest.fn(() => (
     {
       promise: () => Promise.resolve({
diff --git a/tests/api.v2/helpers/s3/patchCellSetsObject.test.js b/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
new file mode 100644
index 000000000..90fd8bf16
--- /dev/null
+++ b/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
@@ -0,0 +1,72 @@
+// @ts-nocheck
+const patchCellSetsObject = require('../../../../src/api.v2/helpers/s3/patchCellSetsObject');
+const getObject = require('../../../../src/api.v2/helpers/s3/getObject');
+const putObject = require('../../../../src/api.v2/helpers/s3/putObject');
+
+jest.mock('../../../../src/api.v2/helpers/s3/getObject');
+jest.mock('../../../../src/api.v2/helpers/s3/putObject');
+
+const mockExperimentId = 'mock-experiment-id';
+
+const mockCellSets = {
+  cellSets: [
+    {
+      key: 'louvain',
+      name: 'louvain clusters',
+      rootNode: true,
+      type: 'cellSets',
+      children: [
+        {
+          key: 'louvain-0',
+          name: 'Cluster 0',
+          rootNode: false,
+          type: 'cellSets',
+          color: '#77aadd',
+          cellIds: [0, 1, 2, 3],
+        },
+      ],
+    },
+  ],
+};
+
+const mockPatch = [
+  {
+    $match: {
+      query: '$[?(@.key == "scratchpad")]',
+      value: {
+        children: [
+          {
+            $insert:
+              {
+                index: '-',
+                value:
+                  {
+                    key: 'new-cluster-1',
+                    name: 'New Cluster 1',
+                    color: '#3957ff',
+                    type: 'cellSets',
+                    cellIds: [4, 5, 6],
+                  },
+              },
+          },
+        ],
+      },
+    },
+  },
+];
+
+describe('patchCellSetsObject', () => {
+  it('Works correctly', async () => {
+    getObject.mockReturnValueOnce(mockCellSets);
+
+    const result = await patchCellSetsObject(mockExperimentId, mockPatch);
+
+    // Put a modified object
+    const putParams = putObject.mock.calls[0][0];
+
+    expect(putParams).toMatchSnapshot();
+
+    // Does not return anything on success
+    expect(result).toBeUndefined();
+  });
+});
diff --git a/tests/api.v2/helpers/s3/putObject.test.js b/tests/api.v2/helpers/s3/putObject.test.js
index 132f3928b..f8ca6f48a 100644
--- a/tests/api.v2/helpers/s3/putObject.test.js
+++ b/tests/api.v2/helpers/s3/putObject.test.js
@@ -4,7 +4,7 @@ const getS3Client = require('../../../../src/api.v2/helpers/s3/getS3Client');
 
 const NotFoundError = require('../../../../src/utils/responses/NotFoundError');
 
-jest.mock('../../../../src/api.v2/helpers/s3/S3Client', () => jest.fn(() => ({
+jest.mock('../../../../src/api.v2/helpers/s3/getS3Client', () => jest.fn(() => ({
   putObject: jest.fn(() => (
     {
       promise: () => Promise.resolve({}),

From f5c9c71d14d2b02e31ae31c42195394046085aa9 Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Fri, 6 May 2022 10:12:38 +0100
Subject: [PATCH 09/16] add proper schema for validation

---
 src/specs/api.v2.yaml                         |  9 ++-----
 .../models/cell-sets-bodies/CellSet.v2.yaml   | 18 +++++++++++++
 .../cell-sets-bodies/CellSetClass.v2.yaml     | 18 +++++++++++++
 .../models/cell-sets-bodies/CellSets.v2.yaml  | 25 +++++--------------
 4 files changed, 44 insertions(+), 26 deletions(-)
 create mode 100644 src/specs/models/cell-sets-bodies/CellSet.v2.yaml
 create mode 100644 src/specs/models/cell-sets-bodies/CellSetClass.v2.yaml

diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml
index 5e950c6d5..762c8c214 100644
--- a/src/specs/api.v2.yaml
+++ b/src/specs/api.v2.yaml
@@ -81,12 +81,7 @@ paths:
           content:
             application/json:
               schema:
-                type: object
-                properties:
-                  cellSets:
-                    type: array
-                    items:
-                      $ref: '#/components/schemas/CellSets'
+                $ref: '#/components/schemas/CellSets'
         '401':
           description: The request lacks authentication credentials.
           content:
@@ -913,7 +908,7 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/HTTPError'
-                
+
   '/access/{experimentId}':
     get:
       summary: Get the users with access to an experiment
diff --git a/src/specs/models/cell-sets-bodies/CellSet.v2.yaml b/src/specs/models/cell-sets-bodies/CellSet.v2.yaml
new file mode 100644
index 000000000..ca97d4b53
--- /dev/null
+++ b/src/specs/models/cell-sets-bodies/CellSet.v2.yaml
@@ -0,0 +1,18 @@
+title: Cell Set
+description: An object representing a cell set (e.g. Cluster 1)
+type: object
+properties:
+  key:
+    type: string
+  name:
+    type: string
+  rootNode:
+    type: boolean
+  cellIds:
+    type: array
+    items:
+      type: integer
+required:
+  - key
+  - name
+  - cellIds
\ No newline at end of file
diff --git a/src/specs/models/cell-sets-bodies/CellSetClass.v2.yaml b/src/specs/models/cell-sets-bodies/CellSetClass.v2.yaml
new file mode 100644
index 000000000..dbd03b4ab
--- /dev/null
+++ b/src/specs/models/cell-sets-bodies/CellSetClass.v2.yaml
@@ -0,0 +1,18 @@
+title: Cell Set
+description: An object representing a cell set (e.g. Cluster 1)
+type: object
+properties:
+  key:
+    type: string
+  name:
+    type: string
+  rootNode:
+    type: boolean
+  children:
+    type: array
+    items:
+      $ref: ./CellSet.v2.yaml
+required:
+  - key
+  - name
+  - children
\ No newline at end of file
diff --git a/src/specs/models/cell-sets-bodies/CellSets.v2.yaml b/src/specs/models/cell-sets-bodies/CellSets.v2.yaml
index 4c3686fe3..636efb888 100644
--- a/src/specs/models/cell-sets-bodies/CellSets.v2.yaml
+++ b/src/specs/models/cell-sets-bodies/CellSets.v2.yaml
@@ -1,23 +1,10 @@
-title: Cell Sets Object
-type: object
+title: Cell Sets object
 description: Schema for CellSets object
+type: object
 properties:
-  key:
-    type: string
-    format: int32
-  name:
-    type: string
-  rootNode:
-    type: boolean
-  children:
-    oneOf:
-      - minItems: 0
-        maxItems: 0
-        items: {}
-      - minItems: 1
-        items:
-          $ref: ./CellSets.v2.yaml
+  cellSets:
     type: array
+    items:
+      $ref: ./CellSetClass.v2.yaml
 required:
-  - key
-  - name
+  - cellSets

From 45d907f8511a53e5c4bb5b64fde97fd42db401ba Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Fri, 6 May 2022 10:13:05 +0100
Subject: [PATCH 10/16] add tests for patchCellSets

---
 src/api.v2/helpers/s3/patchCellSetsObject.js  |  8 +++-
 .../patchCellSetsObject.test.js.snap          |  2 +-
 .../helpers/s3/patchCellSetsObject.test.js    | 39 +++++++++++++++++--
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/src/api.v2/helpers/s3/patchCellSetsObject.js b/src/api.v2/helpers/s3/patchCellSetsObject.js
index 28082669a..95907df42 100644
--- a/src/api.v2/helpers/s3/patchCellSetsObject.js
+++ b/src/api.v2/helpers/s3/patchCellSetsObject.js
@@ -4,6 +4,8 @@ const bucketNames = require('./bucketNames');
 const getObject = require('./getObject');
 const putObject = require('./putObject');
 
+const validateRequest = require('../../../utils/schema-validator');
+
 const patchCellSetsObject = async (experimentId, patch) => {
   const initialCellSet = await getObject({
     Bucket: bucketNames.CELL_SETS,
@@ -23,12 +25,14 @@ const patchCellSetsObject = async (experimentId, patch) => {
     [cellSetsList, patch],
   ).filter((x) => x !== undefined);
 
-  const patchedCellSet = JSON.stringify({ cellSets: patchedArray });
+  const patchedCellSet = { cellSets: patchedArray };
+
+  await validateRequest(patchedCellSet, 'cell-sets-bodies/CellSets.v2.yaml');
 
   await putObject({
     Bucket: bucketNames.CELL_SETS,
     Key: experimentId,
-    Body: patchedCellSet,
+    Body: JSON.stringify(patchedCellSet),
   });
 };
 
diff --git a/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap b/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
index d13063011..7820a60e3 100644
--- a/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
+++ b/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
@@ -2,7 +2,7 @@
 
 exports[`patchCellSetsObject Works correctly 1`] = `
 Object {
-  "Body": "{\\"cellSets\\":[{\\"key\\":\\"louvain\\",\\"name\\":\\"louvain clusters\\",\\"rootNode\\":true,\\"type\\":\\"cellSets\\",\\"children\\":[{\\"key\\":\\"louvain-0\\",\\"name\\":\\"Cluster 0\\",\\"rootNode\\":false,\\"type\\":\\"cellSets\\",\\"color\\":\\"#77aadd\\",\\"cellIds\\":[0,1,2,3]}]}]}",
+  "Body": "{\\"cellSets\\":[{\\"key\\":\\"louvain\\",\\"name\\":\\"louvain clusters\\",\\"rootNode\\":true,\\"type\\":\\"cellSets\\",\\"children\\":[{\\"key\\":\\"cluster-0\\",\\"name\\":\\"Cluster 0\\",\\"rootNode\\":false,\\"type\\":\\"cellSets\\",\\"color\\":\\"#77aadd\\",\\"cellIds\\":[0,1,2,3]},{\\"key\\":\\"new-cluster-1\\",\\"name\\":\\"New Cluster 1\\",\\"rootNode\\":false,\\"color\\":\\"#3957ff\\",\\"type\\":\\"cellSets\\",\\"cellIds\\":[4,5,6]}]}]}",
   "Bucket": "cell-sets-test",
   "Key": "mock-experiment-id",
 }
diff --git a/tests/api.v2/helpers/s3/patchCellSetsObject.test.js b/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
index 90fd8bf16..4df707578 100644
--- a/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
+++ b/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
@@ -32,7 +32,7 @@ const mockCellSets = {
 const mockPatch = [
   {
     $match: {
-      query: '$[?(@.key == "scratchpad")]',
+      query: '$[?(@.key == "louvain")]',
       value: {
         children: [
           {
@@ -43,6 +43,7 @@ const mockPatch = [
                   {
                     key: 'new-cluster-1',
                     name: 'New Cluster 1',
+                    rootNode: false,
                     color: '#3957ff',
                     type: 'cellSets',
                     cellIds: [4, 5, 6],
@@ -55,10 +56,10 @@ const mockPatch = [
   },
 ];
 
+getObject.mockReturnValue(mockCellSets);
+
 describe('patchCellSetsObject', () => {
   it('Works correctly', async () => {
-    getObject.mockReturnValueOnce(mockCellSets);
-
     const result = await patchCellSetsObject(mockExperimentId, mockPatch);
 
     // Put a modified object
@@ -69,4 +70,36 @@ describe('patchCellSetsObject', () => {
     // Does not return anything on success
     expect(result).toBeUndefined();
   });
+
+  it.only('Throws an error if the JSON merger result is not correct', async () => {
+    // Should fail validation because cellIds is not an array
+    const malformedPatch = [
+      {
+        $match: {
+          query: '$[?(@.key == "louvain")]',
+          value: {
+            children: [
+              {
+                $insert:
+              {
+                index: '-',
+                value:
+                  {
+                    key: 'singular-cluster',
+                    name: 'Singular cluster',
+                    rootNode: false,
+                    color: '#3957ff',
+                    type: 'cellSets',
+                    cellIds: 1,
+                  },
+              },
+              },
+            ],
+          },
+        },
+      },
+    ];
+
+    await expect(patchCellSetsObject(mockExperimentId, malformedPatch)).rejects.toThrow();
+  });
 });

From 0368903e58665dedb911be64502cc2226117783f Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Fri, 6 May 2022 15:09:19 +0100
Subject: [PATCH 11/16] address comments

---
 src/api.v2/helpers/s3/patchCellSetsObject.js       | 14 +++++++-------
 src/utils/schema-validator.js                      |  4 +++-
 src/utils/v1Compatibility/formatExperimentId.js    |  1 +
 .../__snapshots__/patchCellSetsObject.test.js.snap |  2 +-
 .../api.v2/helpers/s3/patchCellSetsObject.test.js  |  2 +-
 5 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/src/api.v2/helpers/s3/patchCellSetsObject.js b/src/api.v2/helpers/s3/patchCellSetsObject.js
index 95907df42..2cadaa168 100644
--- a/src/api.v2/helpers/s3/patchCellSetsObject.js
+++ b/src/api.v2/helpers/s3/patchCellSetsObject.js
@@ -7,12 +7,12 @@ const putObject = require('./putObject');
 const validateRequest = require('../../../utils/schema-validator');
 
 const patchCellSetsObject = async (experimentId, patch) => {
-  const initialCellSet = await getObject({
+  const currentCellSet = await getObject({
     Bucket: bucketNames.CELL_SETS,
     Key: experimentId,
   });
 
-  const { cellSets: cellSetsList } = initialCellSet;
+  const { cellSets: prePatchCellSets } = currentCellSet;
 
   /**
    * The $remove operation will replace the element in the array with an
@@ -21,18 +21,18 @@ const patchCellSetsObject = async (experimentId, patch) => {
    * We use the $remove operation in the worker to update cell clusters,
    * and we may end up using it in other places in the future.
    */
-  const patchedArray = jsonMerger.mergeObjects(
-    [cellSetsList, patch],
+  const patchedCellSetslist = jsonMerger.mergeObjects(
+    [prePatchCellSets, patch],
   ).filter((x) => x !== undefined);
 
-  const patchedCellSet = { cellSets: patchedArray };
+  const patchedCellSets = { cellSets: patchedCellSetslist };
 
-  await validateRequest(patchedCellSet, 'cell-sets-bodies/CellSets.v2.yaml');
+  await validateRequest(patchedCellSets, 'cell-sets-bodies/CellSets.v2.yaml');
 
   await putObject({
     Bucket: bucketNames.CELL_SETS,
     Key: experimentId,
-    Body: JSON.stringify(patchedCellSet),
+    Body: JSON.stringify(patchedCellSets),
   });
 };
 
diff --git a/src/utils/schema-validator.js b/src/utils/schema-validator.js
index ae03bf075..1fd4d6203 100644
--- a/src/utils/schema-validator.js
+++ b/src/utils/schema-validator.js
@@ -6,6 +6,8 @@ const Validator = require('swagger-model-validator');
 const yaml = require('js-yaml');
 const _ = require('lodash');
 
+const BadRequestError = require('./responses/BadRequestError');
+
 const getLogger = require('./getLogger');
 
 const logger = getLogger();
@@ -42,7 +44,7 @@ const validateRequest = async (request, schemaPath) => {
 
   if (!validation.valid) {
     logger.log(`Validation error for: ${request}`);
-    throw new Error(validation.errors[0]);
+    throw new BadRequestError(validation.errors[0]);
   }
 };
 
diff --git a/src/utils/v1Compatibility/formatExperimentId.js b/src/utils/v1Compatibility/formatExperimentId.js
index 4b1becb9b..a1c9a6e83 100644
--- a/src/utils/v1Compatibility/formatExperimentId.js
+++ b/src/utils/v1Compatibility/formatExperimentId.js
@@ -2,6 +2,7 @@
 // They have the same number of alhpanum characters as UUIDs but no dashes
 // To maintain compatibility with v1, we remove the dashes from UUIDs.
 // This function should be removed once we have migrated to v2.
+// TODO: migrate existing cellsets to the dashes uuidv4 format so this function is not necessary
 
 const formatExperimentId = (experimentId) => experimentId.replace(/-/g, '');
 
diff --git a/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap b/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
index 7820a60e3..a9f808097 100644
--- a/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
+++ b/tests/api.v2/helpers/s3/__snapshots__/patchCellSetsObject.test.js.snap
@@ -2,7 +2,7 @@
 
 exports[`patchCellSetsObject Works correctly 1`] = `
 Object {
-  "Body": "{\\"cellSets\\":[{\\"key\\":\\"louvain\\",\\"name\\":\\"louvain clusters\\",\\"rootNode\\":true,\\"type\\":\\"cellSets\\",\\"children\\":[{\\"key\\":\\"cluster-0\\",\\"name\\":\\"Cluster 0\\",\\"rootNode\\":false,\\"type\\":\\"cellSets\\",\\"color\\":\\"#77aadd\\",\\"cellIds\\":[0,1,2,3]},{\\"key\\":\\"new-cluster-1\\",\\"name\\":\\"New Cluster 1\\",\\"rootNode\\":false,\\"color\\":\\"#3957ff\\",\\"type\\":\\"cellSets\\",\\"cellIds\\":[4,5,6]}]}]}",
+  "Body": "{\\"cellSets\\":[{\\"key\\":\\"louvain\\",\\"name\\":\\"louvain clusters\\",\\"rootNode\\":true,\\"type\\":\\"cellSets\\",\\"children\\":[{\\"key\\":\\"louvain-0\\",\\"name\\":\\"Cluster 0\\",\\"rootNode\\":false,\\"type\\":\\"cellSets\\",\\"color\\":\\"#77aadd\\",\\"cellIds\\":[0,1,2,3]},{\\"key\\":\\"new-cluster-1\\",\\"name\\":\\"New Cluster 1\\",\\"rootNode\\":false,\\"color\\":\\"#3957ff\\",\\"type\\":\\"cellSets\\",\\"cellIds\\":[4,5,6]}]}]}",
   "Bucket": "cell-sets-test",
   "Key": "mock-experiment-id",
 }
diff --git a/tests/api.v2/helpers/s3/patchCellSetsObject.test.js b/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
index 4df707578..d7a7bab4e 100644
--- a/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
+++ b/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
@@ -71,7 +71,7 @@ describe('patchCellSetsObject', () => {
     expect(result).toBeUndefined();
   });
 
-  it.only('Throws an error if the JSON merger result is not correct', async () => {
+  it('Throws an error if the JSON merger result is not correct', async () => {
     // Should fail validation because cellIds is not an array
     const malformedPatch = [
       {

From 862f16261a9832eef8dc5576c373c1f4154d3b2c Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Mon, 9 May 2022 15:08:10 +0100
Subject: [PATCH 12/16] change json to send

---
 src/api.v2/controllers/cellSetsController.js        | 2 +-
 src/specs/api.v2.yaml                               | 2 --
 tests/api.v2/controllers/cellSetsController.test.js | 3 ++-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/api.v2/controllers/cellSetsController.js b/src/api.v2/controllers/cellSetsController.js
index 4a2e197b8..e946c76e3 100644
--- a/src/api.v2/controllers/cellSetsController.js
+++ b/src/api.v2/controllers/cellSetsController.js
@@ -24,7 +24,7 @@ const getCellSets = async (req, res) => {
 
   logger.log(`Finished getting cell sets for experiment ${experimentId}`);
 
-  res.json(cellSets);
+  res.send(cellSets);
 };
 
 const patchCellSets = async (req, res) => {
diff --git a/src/specs/api.v2.yaml b/src/specs/api.v2.yaml
index 762c8c214..2ea53d1ba 100644
--- a/src/specs/api.v2.yaml
+++ b/src/specs/api.v2.yaml
@@ -136,8 +136,6 @@ paths:
                 - type: object
         description: The patch in the format declared by boschni/json-merger. Note the explicit naming of the content subtype (boschni-json-merger+json) which must be used to explicitly acknowledge the format.
 
-
-
   '/experiments/{experimentId}/processingConfig':
     get:
       summary: Get processing configuration for an experiment
diff --git a/tests/api.v2/controllers/cellSetsController.test.js b/tests/api.v2/controllers/cellSetsController.test.js
index 999c90089..0bdeb6d6b 100644
--- a/tests/api.v2/controllers/cellSetsController.test.js
+++ b/tests/api.v2/controllers/cellSetsController.test.js
@@ -13,6 +13,7 @@ jest.mock('../../../src/api.v2/helpers/s3/patchCellSetsObject');
 
 const mockRes = {
   json: jest.fn(),
+  send: jest.fn(),
 };
 
 const mockCellSets = {
@@ -83,7 +84,7 @@ describe('cellSetsController', () => {
       Key: formatExperimentId(mockExperimentId),
     });
 
-    expect(mockRes.json).toHaveBeenCalledWith(mockCellSets);
+    expect(mockRes.send).toHaveBeenCalledWith(mockCellSets);
   });
 
   it('patchCellSetsObject works correctly', async () => {

From 0974a1e44ec34bdde47472787b50f6f6a7f3482b Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Tue, 10 May 2022 14:19:53 +0100
Subject: [PATCH 13/16] remove filter in patch cell sets object

---
 src/api.v2/helpers/s3/patchCellSetsObject.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/api.v2/helpers/s3/patchCellSetsObject.js b/src/api.v2/helpers/s3/patchCellSetsObject.js
index 2cadaa168..8ac4f8f18 100644
--- a/src/api.v2/helpers/s3/patchCellSetsObject.js
+++ b/src/api.v2/helpers/s3/patchCellSetsObject.js
@@ -23,7 +23,7 @@ const patchCellSetsObject = async (experimentId, patch) => {
    */
   const patchedCellSetslist = jsonMerger.mergeObjects(
     [prePatchCellSets, patch],
-  ).filter((x) => x !== undefined);
+  );
 
   const patchedCellSets = { cellSets: patchedCellSetslist };
 

From b683b338b06ea1ab5d0496c3bf1b0ebc8a1b33b2 Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Tue, 10 May 2022 14:26:23 +0100
Subject: [PATCH 14/16] change back codecov threshold to 0

---
 codecov.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/codecov.yaml b/codecov.yaml
index a1c074a06..58b6b2592 100644
--- a/codecov.yaml
+++ b/codecov.yaml
@@ -11,7 +11,7 @@ coverage:
       default:
         target: auto
         # don't allow new commits to decrease coverage
-        threshold: 1%
+        threshold: 0%
 
     patch:                    # measuring the coverage of new changes
       default:

From 9d3aec773a3db68732963d74af548296b0323841 Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Tue, 10 May 2022 14:58:30 +0100
Subject: [PATCH 15/16] parse JSON object

---
 src/api.v2/helpers/s3/patchCellSetsObject.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/api.v2/helpers/s3/patchCellSetsObject.js b/src/api.v2/helpers/s3/patchCellSetsObject.js
index 8ac4f8f18..6d31dcdb3 100644
--- a/src/api.v2/helpers/s3/patchCellSetsObject.js
+++ b/src/api.v2/helpers/s3/patchCellSetsObject.js
@@ -12,7 +12,7 @@ const patchCellSetsObject = async (experimentId, patch) => {
     Key: experimentId,
   });
 
-  const { cellSets: prePatchCellSets } = currentCellSet;
+  const { cellSets: prePatchCellSets } = JSON.parse(currentCellSet);
 
   /**
    * The $remove operation will replace the element in the array with an

From 4ae793b78af66cb755b7be849a2bddd2b5ed53bd Mon Sep 17 00:00:00 2001
From: Anugerah Erlaut <aerlaut@live.com>
Date: Tue, 10 May 2022 15:10:32 +0100
Subject: [PATCH 16/16] fix test

---
 tests/api.v2/helpers/s3/patchCellSetsObject.test.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/api.v2/helpers/s3/patchCellSetsObject.test.js b/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
index d7a7bab4e..7fe083b56 100644
--- a/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
+++ b/tests/api.v2/helpers/s3/patchCellSetsObject.test.js
@@ -8,7 +8,7 @@ jest.mock('../../../../src/api.v2/helpers/s3/putObject');
 
 const mockExperimentId = 'mock-experiment-id';
 
-const mockCellSets = {
+const mockCellSets = JSON.stringify({
   cellSets: [
     {
       key: 'louvain',
@@ -27,7 +27,7 @@ const mockCellSets = {
       ],
     },
   ],
-};
+});
 
 const mockPatch = [
   {