From a89362c1ad5711311f29cd62c9e82404ee178330 Mon Sep 17 00:00:00 2001
From: Bruce Martin <bruce@chanzuckerberg.com>
Date: Mon, 22 Mar 2021 14:54:12 -0700
Subject: [PATCH] update gene set name validation to match latest requirements
 (#2117)

---
 local_server/data_common/data_adaptor.py  | 17 ++++---
 local_server/test/unit/common/test_api.py | 54 +++++++++++++++++++++--
 2 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/local_server/data_common/data_adaptor.py b/local_server/data_common/data_adaptor.py
index b4f7a226..3c5c03cb 100644
--- a/local_server/data_common/data_adaptor.py
+++ b/local_server/data_common/data_adaptor.py
@@ -276,11 +276,11 @@ class DataAdaptor(metaclass=ABCMeta):
 
         Rules:
         0. all geneset names must be unique.
-        1. All geneset names must be legal, meaning:
+        1. All geneset names must be comprised of legal characters, meaning:
             * no leading or trailing white space
             * no multi-space runs
-            * character set matches: [A-Z][a-z][0-9][ .()-]
-           Generates hard error.
+            * no tab, vertical tab, newline or return
+           Where "space" means ASCII 32. Generates hard error.
         2. Gene symbols must be part of the current var_index.  If symbol not in var_index,
            will generate a warning and the symbol removed.
         3. Duplicate gene symbols are silently de-duped.
@@ -299,19 +299,18 @@ class DataAdaptor(metaclass=ABCMeta):
             raise KeyError("All geneset names must be unique.")
 
         # 1. check gene set character set and format
-        legal_name = re.compile(r"^(\w|[ .()-])+$")
+        illegal_name = re.compile(r"^\s|  |[\v\t\r\n]|\s$")
         for name in geneset_names:
             if type(name) != str or len(name) == 0:
                 raise KeyError("Geneset names must be non-null string.")
-            if name[0] in " \t\n\r" or name[-1] in " \t\n\r" or not legal_name.match(name) or "  " in name:
+            if illegal_name.search(name):
                 messagefn(
                     "Error: "
-                    f"Geneset name {name} is not valid. Only alphanumeric and limited special characters (-_.) "
-                    "and space are allowed. Leading, trailing, and multiple spaces within a name are not allowed."
+                    f"Geneset name {name} "
+                    "is not valid. Leading, trailing, and multiple spaces within a name are not allowed."
                 )
                 raise KeyError(
-                    "Geneset name is not valid, only alphanumeric and limited special characters (-_.) "
-                    "and space are allowed. Leading, trailing, and multiple spaces within a name are not allowed."
+                    "Geneset name is not valid. Leading, trailing, and multiple spaces within a name are not allowed."
                 )
 
         # 2. & 3. check for duplicate gene symbols, and those not present in the dataset. They will
diff --git a/local_server/test/unit/common/test_api.py b/local_server/test/unit/common/test_api.py
index c3a37313..95536b05 100644
--- a/local_server/test/unit/common/test_api.py
+++ b/local_server/test/unit/common/test_api.py
@@ -568,8 +568,26 @@ summary test,,PIGU,\r
         result = self.session.put(url, json=test1)
         self.assertEqual(result.status_code, HTTPStatus.NOT_FOUND)
 
-        test2 = {"tid": 4, "genesets": [{"geneset_name": "foobar", "genes": []}]}
-        test2_response = {"tid": 4, "genesets": [{"geneset_name": "foobar", "geneset_description": "", "genes": []}]}
+        test2 = {
+            "tid": 4,
+            "genesets": [
+                {"geneset_name": "foobar", "genes": []},
+                {"geneset_name": "contains a space", "genes": []},
+                {"geneset_name": "contains_weird_characters: #$%^&*()_+=-!@<>,./?';:\"[]{}|\\", "genes": []},
+            ],
+        }
+        test2_response = {
+            "tid": 4,
+            "genesets": [
+                {"geneset_name": "foobar", "geneset_description": "", "genes": []},
+                {"geneset_name": "contains a space", "geneset_description": "", "genes": []},
+                {
+                    "geneset_name": "contains_weird_characters: #$%^&*()_+=-!@<>,./?';:\"[]{}|\\",
+                    "geneset_description": "",
+                    "genes": [],
+                },
+            ],
+        }
         result = self.session.put(url, json=test2)
         self.assertEqual(result.status_code, HTTPStatus.OK)
         result = self.session.get(url, headers={"Accept": "application/json"})
@@ -629,7 +647,37 @@ summary test,,PIGU,\r
 
         # illegal geneset_name
         test_case(
-            {"tid": tid + 1, "genesets": [{"geneset_name": "&quot;", "genes": []}]},
+            {"tid": tid + 1, "genesets": [{"geneset_name": " foo", "genes": []}]},
+            HTTPStatus.BAD_REQUEST,
+            original_data,
+        )
+        test_case(
+            {"tid": tid + 1, "genesets": [{"geneset_name": "foo ", "genes": []}]},
+            HTTPStatus.BAD_REQUEST,
+            original_data,
+        )
+        test_case(
+            {"tid": tid + 1, "genesets": [{"geneset_name": "f  oo", "genes": []}]},
+            HTTPStatus.BAD_REQUEST,
+            original_data,
+        )
+        test_case(
+            {"tid": tid + 1, "genesets": [{"geneset_name": "f\too", "genes": []}]},
+            HTTPStatus.BAD_REQUEST,
+            original_data,
+        )
+        test_case(
+            {"tid": tid + 1, "genesets": [{"geneset_name": "f\roo", "genes": []}]},
+            HTTPStatus.BAD_REQUEST,
+            original_data,
+        )
+        test_case(
+            {"tid": tid + 1, "genesets": [{"geneset_name": "f\noo", "genes": []}]},
+            HTTPStatus.BAD_REQUEST,
+            original_data,
+        )
+        test_case(
+            {"tid": tid + 1, "genesets": [{"geneset_name": "f\voo", "genes": []}]},
             HTTPStatus.BAD_REQUEST,
             original_data,
         )