Repository: virtool/virtool
Issue: Validate name field in sample creation requests

Currently possible to create samples with no name as long as subtraction host and read files are specified.

Changed files:
virtool/sample.py, virtool/handlers/samples.py, tests/handlers/test_samples.py

diff --git a/tests/handlers/test_samples.py b/tests/handlers/test_samples.py
index b28ad3303..33f9a02bd 100644
--- a/tests/handlers/test_samples.py
+++ b/tests/handlers/test_samples.py
@@ -240,7 +240,8 @@ class TestCreate:
             "sample_all_read": True,
             "sample_all_write": True,
             "sample_group_read": True,
-            "sample_group_write": True
+            "sample_group_write": True,
+            "sample_unique_names": True
         })
 
         m_reserve = make_mocked_coro()
@@ -327,6 +328,8 @@ class TestCreate:
     async def test_name_exists(self, spawn_client, static_time, resp_is):
         client = await spawn_client(authorize=True, permissions=["create_sample"])
 
+        client.app["settings"]["sample_unique_names"] = True
+
         await client.db.samples.insert_one({
             "_id": "foobar",
             "name": "Foobar",
@@ -342,7 +345,7 @@ class TestCreate:
             "subtraction": "apple"
         })
 
-        assert await resp_is.conflict(resp, "Sample name 'Foobar' already exists")
+        assert await resp_is.conflict(resp, "Sample name is already in use")
 
     async def test_force_choice(self, spawn_client, static_time, resp_is):
         """
@@ -353,6 +356,7 @@ class TestCreate:
         client = await spawn_client(authorize=True, permissions=["create_sample"])
 
         client.app["settings"]["sample_group"] = "force_choice"
+        client.app["settings"]["sample_unique_names"] = True
 
         resp = await client.post("/api/samples", {
             "name": "Foobar",
@@ -366,6 +370,7 @@ class TestCreate:
         client = await spawn_client(authorize=True, permissions=["create_sample"])
 
         client.app["settings"]["sample_group"] = "force_choice"
+        client.app["settings"]["sample_unique_names"] = True
 
         resp = await client.post("/api/samples", {
             "name": "Foobar",
@@ -380,6 +385,8 @@ class TestCreate:
     async def test_subtraction_dne(self, in_db, spawn_client, resp_is):
         client = await spawn_client(authorize=True, permissions=["create_sample"])
 
+        client.app["settings"]["sample_unique_names"] = True
+
         resp = await client.post("/api/samples", {
             "name": "Foobar",
             "files": ["test.fq"],
@@ -402,6 +409,8 @@ class TestCreate:
         """
         client = await spawn_client(authorize=True, permissions=["create_sample"])
 
+        client.app["settings"]["sample_unique_names"] = True
+
         await client.db.subtraction.insert_one({
             "_id": "apple",
             "is_host": True
diff --git a/virtool/handlers/samples.py b/virtool/handlers/samples.py
index f932e25e0..89bb36625 100644
--- a/virtool/handlers/samples.py
+++ b/virtool/handlers/samples.py
@@ -86,7 +86,7 @@ async def get(req):
 
 @protected("create_sample")
 @validation({
-    "name": {"type": "string", "required": True},
+    "name": {"type": "string", "minlength": 1, "required": True},
     "host": {"type": "string"},
     "isolate": {"type": "string"},
     "group": {"type": "string"},
@@ -97,6 +97,11 @@ async def get(req):
 async def create(req):
     db, data = await unpack_request(req)
 
+    message = await virtool.sample.check_name(db, req.app["settings"], data["name"])
+
+    if message:
+        return conflict(message)
+
     if req.app["settings"].get("sample_group") == "force_choice":
         try:
             if not await db.groups.count({"_id": data["group"]}):
@@ -177,7 +182,7 @@ async def create(req):
 
 
 @validation({
-    "name": {"type": "string"},
+    "name": {"type": "string", "minlength": 1},
     "host": {"type": "string"},
     "isolate": {"type": "string"},
     "locale": {"type": "string"}
@@ -189,7 +194,14 @@ async def edit(req):
     """
     db, data = req.app["db"], req["data"]
 
-    document = await db.samples.find_one_and_update({"_id": req.match_info["sample_id"]}, {
+    sample_id = req.match_info["sample_id"]
+
+    message = await virtool.sample.check_name(db, req.app["settings"], data["name"], sample_id=sample_id)
+
+    if message:
+        return conflict(message)
+
+    document = await db.samples.find_one_and_update({"_id": sample_id}, {
         "$set": data
     }, return_document=ReturnDocument.AFTER, projection=virtool.sample.LIST_PROJECTION)
 
diff --git a/virtool/sample.py b/virtool/sample.py
index 5c53f0531..9e5a4660d 100644
--- a/virtool/sample.py
+++ b/virtool/sample.py
@@ -83,27 +83,22 @@ def calculate_algorithm_tags(analyses):
     }
 
 
-async def recalculate_algorithm_tags(db, sample_id):
-    """
-    Recalculate and apply algorithm tags (eg. "ip", True) for a given sample. Finds the associated analyses and calls
-    :func:`calculate_algorithm_tags`, then applies the update to the sample document.
+async def check_name(db, settings, name, sample_id=None):
 
-    :param db: the application database client
-    :type db: :class:`~motor.motor_asyncio.AsyncIOMotorClient`
+    if settings["sample_unique_names"]:
+        query = {
+            "name": name
+        }
 
-    :param sample_id: the id of the sample to recalculate tags for
-    :type sample_id: str
+        if sample_id:
+            query["_id"] = {
+                "$ne": sample_id
+            }
 
-    """
-    analyses = await db.analyses.find({"sample.id": sample_id}, ["ready", "algorithm"]).to_list(None)
+        if await db.samples.count(query):
+            return "Sample name is already in use"
 
-    update = calculate_algorithm_tags(analyses)
-
-    document = await db.samples.find_one_and_update({"_id": sample_id}, {
-        "$set": update
-    }, return_document=pymongo.ReturnDocument.AFTER, projection=LIST_PROJECTION)
-
-    return document
+    return None
 
 
 async def get_sample_owner(db, sample_id):
@@ -127,6 +122,29 @@ async def get_sample_owner(db, sample_id):
     return None
 
 
+async def recalculate_algorithm_tags(db, sample_id):
+    """
+    Recalculate and apply algorithm tags (eg. "ip", True) for a given sample. Finds the associated analyses and calls
+    :func:`calculate_algorithm_tags`, then applies the update to the sample document.
+
+    :param db: the application database client
+    :type db: :class:`~motor.motor_asyncio.AsyncIOMotorClient`
+
+    :param sample_id: the id of the sample to recalculate tags for
+    :type sample_id: str
+
+    """
+    analyses = await db.analyses.find({"sample.id": sample_id}, ["ready", "algorithm"]).to_list(None)
+
+    update = calculate_algorithm_tags(analyses)
+
+    document = await db.samples.find_one_and_update({"_id": sample_id}, {
+        "$set": update
+    }, return_document=pymongo.ReturnDocument.AFTER, projection=LIST_PROJECTION)
+
+    return document
+
+
 async def remove_samples(db, settings, id_list):
     """
     Complete removes the samples identified by the document ids in ``id_list``. In order, it: