modified entotyview creation of strings and list cols

Author: andrewelamb

synapseclient/extensions/curator/file_based_metadata_task.py

@@ -24,7 +24,7 @@
 from synapseclient.operations import FileOptions, get
 
 TYPE_DICT = {
-    "string": ColumnType.STRING,
+    "string": ColumnType.MEDIUMTEXT,
     "number": ColumnType.DOUBLE,
     "integer": ColumnType.INTEGER,
     "boolean": ColumnType.BOOLEAN,
@@ -36,6 +36,9 @@
     "boolean": ColumnType.BOOLEAN_LIST,
 }
 
+MAX_LIST_STRING_ITEM_SIZE = 100
+MAX_LIST_LENGTH = 50
+
 
 def create_json_schema_entity_view(
     syn: Synapse,
@@ -199,48 +202,78 @@ def _create_columns_from_json_schema(json_schema: dict[str, Any]) -> list[Column
         raise ValueError(
             "The 'properties' field in the JSON Schema must be a dictionary."
         )
-    columns = []
-    for name, prop_schema in properties.items():
-        column_type = _get_column_type_from_js_property(prop_schema)
-        maximum_size = None
-        if column_type == "STRING":
-            maximum_size = 100
-        if column_type in LIST_TYPE_DICT.values():
-            maximum_size = 5
-
-        column = Column(
-            name=name,
-            column_type=column_type,
-            maximum_size=maximum_size,
-            default_value=None,
-        )
-        columns.append(column)
+    columns = [
+        _create_synapse_column_from_js_property(prop_schema, name)
+        for name, prop_schema in properties.items()
+    ]
     return columns
 
 
+def _create_synapse_column_from_js_property(
+    js_property: dict[str, Any], name: str
+) -> Column:
+    """
+    Creates a Synapse Column based on a JSON Schema property.
+
+    Args:
+        js_property: A JSON Schema property in dict form.
+        name: The name of the column.
+
+    Returns:
+        A Synapse Column based on the JSON Schema property.
+    """
+    column_type = _get_column_type_from_js_property(js_property)
+    maximum_size = None
+    maximum_list_length = None
+    if column_type in LIST_TYPE_DICT.values():
+        maximum_list_length = MAX_LIST_LENGTH
+        if column_type == ColumnType.STRING_LIST:
+            maximum_size = MAX_LIST_STRING_ITEM_SIZE
+
+    return Column(
+        name=name,
+        column_type=column_type,
+        maximum_size=maximum_size,
+        maximum_list_length=maximum_list_length,
+    )
+
+
 def _get_column_type_from_js_property(js_property: dict[str, Any]) -> ColumnType:
     """
     Gets the Synapse column type from a JSON Schema property.
     The JSON Schema should be valid but that should not be assumed.
-    If the type can not be determined ColumnType.STRING will be returned.
+    If the type can not be determined ColumnType.MEDIUMTEXT will be returned.
 
     Args:
         js_property: A JSON Schema property in dict form.
 
     Returns:
         A Synapse ColumnType based on the JSON Schema type
     """
-    # Enums are always strings in Synapse tables
+    # Enums are set as MediumText columns
     if "enum" in js_property:
-        return ColumnType.STRING
+        return ColumnType.MEDIUMTEXT
     if "type" in js_property:
-        if js_property["type"] == "array":
+        js_type = js_property["type"]
+        # Synapse columns cannot be more than one type
+        # If the JSONSchema type is a list of types, check if it's a nullable single type
+        if isinstance(js_type, list):
+            types = [t for t in js_type if t != "null"]
+            if len(types) == 1:
+                js_type = types[0]
+            else:
+                return ColumnType.MEDIUMTEXT
+        if js_type == "array":
             return _get_list_column_type_from_js_property(js_property)
-        return TYPE_DICT.get(js_property["type"], ColumnType.STRING)
+        # If there is only one JSONSChema type, return the corresponding Synapse column type,
+        #  defaulting to MediumText if there is no match
+        return TYPE_DICT.get(js_type, ColumnType.MEDIUMTEXT)
     # A oneOf list usually indicates that the type could be one or more different things
+    # Curator extension does not create the types of JSON Schemas where this is the case
+    #  but if it is present we will attempt to determine the type based on the items in the oneOf list.
     if "oneOf" in js_property and isinstance(js_property["oneOf"], list):
         return _get_column_type_from_js_one_of_list(js_property["oneOf"])
-    return ColumnType.STRING
+    return ColumnType.MEDIUMTEXT
 
 
 def _get_column_type_from_js_one_of_list(js_one_of_list: list[Any]) -> ColumnType:
@@ -258,15 +291,15 @@ def _get_column_type_from_js_one_of_list(js_one_of_list: list[Any]) -> ColumnTyp
     items = [item for item in js_one_of_list if isinstance(item, dict)]
     # Enums are always strings in Synapse tables
     if [item for item in items if "enum" in item]:
-        return ColumnType.STRING
+        return ColumnType.MEDIUMTEXT
     # For Synapse ColumnType we can ignore null types in JSON Schemas
     type_items = [item for item in items if "type" in item if item["type"] != "null"]
     if len(type_items) == 1:
         type_item = type_items[0]
         if type_item["type"] == "array":
             return _get_list_column_type_from_js_property(type_item)
-        return TYPE_DICT.get(type_item["type"], ColumnType.STRING)
-    return ColumnType.STRING
+        return TYPE_DICT.get(type_item["type"], ColumnType.MEDIUMTEXT)
+    return ColumnType.MEDIUMTEXT
 
 
 def _get_list_column_type_from_js_property(js_property: dict[str, Any]) -> ColumnType:

tests/unit/synapseclient/extensions/test_file_based_metadata_task.py

@@ -0,0 +1,117 @@
+"""Unit tests for file_based_metadata_task.py"""
+from typing import Any
+
+import pytest
+
+from synapseclient.extensions.curator.file_based_metadata_task import (
+    MAX_LIST_LENGTH,
+    MAX_LIST_STRING_ITEM_SIZE,
+    _create_columns_from_json_schema,
+    _create_synapse_column_from_js_property,
+    _get_column_type_from_js_property,
+)
+from synapseclient.models import Column, ColumnType
+
+
+def test_create_columns_from_json_schema():
+    schema = {
+        "properties": {
+            "string_col": {"type": "string"},
+            "int_col": {"type": "integer"},
+            "bool_col": {"type": "boolean"},
+        }
+    }
+    expected = [
+        Column(name="string_col", column_type=ColumnType.MEDIUMTEXT),
+        Column(name="int_col", column_type=ColumnType.INTEGER),
+        Column(name="bool_col", column_type=ColumnType.BOOLEAN),
+    ]
+    assert _create_columns_from_json_schema(schema) == expected
+
+
+@pytest.mark.parametrize(
+    "schema",
+    [{}, {"properties": []}],
+    ids=["empty schema", "properties is not a d ict"],
+)
+def test_create_columns_from_json_schema_exceptions(schema: dict[str, Any]):
+    with pytest.raises(ValueError):
+        _create_columns_from_json_schema(schema)
+
+
+@pytest.mark.parametrize(
+    "prop, name, expected_type, expected_max_size, expected_max_list_length",
+    [
+        (
+            {"type": "array", "items": {"type": "string"}},
+            "string_list_col",
+            ColumnType.STRING_LIST,
+            MAX_LIST_STRING_ITEM_SIZE,
+            MAX_LIST_LENGTH,
+        ),
+        (
+            {"type": "array", "items": {"type": "integer"}},
+            "int_list_col",
+            ColumnType.INTEGER_LIST,
+            None,
+            MAX_LIST_LENGTH,
+        ),
+        (
+            {"type": "array", "items": {"type": "boolean"}},
+            "bool_list_col",
+            ColumnType.BOOLEAN_LIST,
+            None,
+            MAX_LIST_LENGTH,
+        ),
+        (
+            {"type": "string"},
+            "string_col",
+            ColumnType.MEDIUMTEXT,
+            None,
+            None,
+        ),
+    ],
+    ids=["string_list", "integer_list", "boolean_list", "string"],
+)
+def test_create_synapse_column_from_js_property(
+    prop, name, expected_type, expected_max_size, expected_max_list_length
+):
+    result = _create_synapse_column_from_js_property(prop, name)
+    assert isinstance(result, Column)
+    assert result.name == name
+    assert result.column_type == expected_type
+    assert result.maximum_size == expected_max_size
+    assert result.maximum_list_length == expected_max_list_length
+
+
+@pytest.mark.parametrize(
+    "prop, expected",
+    [
+        ({"enum": ["a", "b", "c"]}, ColumnType.MEDIUMTEXT),
+        ({"type": "string"}, ColumnType.MEDIUMTEXT),
+        ({"type": "integer"}, ColumnType.INTEGER),
+        ({"type": "number"}, ColumnType.DOUBLE),
+        ({"type": "boolean"}, ColumnType.BOOLEAN),
+        ({"type": ["integer", "null"]}, ColumnType.INTEGER),
+        ({"type": ["integer", "string"]}, ColumnType.MEDIUMTEXT),
+        ({"type": "array", "items": {"type": "integer"}}, ColumnType.INTEGER_LIST),
+        ({"oneOf": [{"type": "integer"}, {"type": "null"}]}, ColumnType.INTEGER),
+        ({"type": "unknown"}, ColumnType.MEDIUMTEXT),
+        ({}, ColumnType.MEDIUMTEXT),
+    ],
+    ids=[
+        "enum_property",
+        "type_string",
+        "type_integer",
+        "type_number",
+        "type_boolean",
+        "type_list_nullable",
+        "type_list_multiple_types",
+        "type_array",
+        "one_of_list",
+        "unknown_type",
+        "empty_property",
+    ],
+)
+def test_get_column_type_from_js_property(prop, expected):
+    assert _get_column_type_from_js_property(prop) == expected