From 5172860219daac78c03ea935a001d945d363860d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20tom=20W=C3=B6rden?= <h.tomwoerden@indiscale.com>
Date: Mon, 4 Mar 2024 21:40:32 +0100
Subject: [PATCH] MAINT: comments and sanity checks

---
 .../table_json_conversion/table_generator.py  | 54 +++++++++++--------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/src/caosadvancedtools/table_json_conversion/table_generator.py b/src/caosadvancedtools/table_json_conversion/table_generator.py
index 913ddd7d..2eb48c39 100644
--- a/src/caosadvancedtools/table_json_conversion/table_generator.py
+++ b/src/caosadvancedtools/table_json_conversion/table_generator.py
@@ -139,6 +139,9 @@ class TableTemplateGenerator(ABC):
         ----------
             array_paths: list
                 a list of path along the way to the current object, where the json contains arrays
+            schema: dict
+                part of the json schema; it must be the level that contains the type definition
+                (e.g. 'type' or 'oneOf' key)
 
         Returns
         -------
@@ -161,20 +164,26 @@ class TableTemplateGenerator(ABC):
         if 'type' in schema and schema['type'] == 'array':
             if ('type' in schema['items'] and schema['items']['type'] == 'object'
                     and len(path) > 1):  # list of references; special treatment
-                # we add a new sheet
+                # we add a new sheet with columns generated from the subtree of the schema
                 sheetname = ".".join(path)
+                if sheetname in sheets:
+                    raise ValueError(f"The shema would lead to two sheets with the same name which"
+                                     f" is forbidden:{sheetname}")
                 sheets[sheetname] = self._treat_schema_element(
                     schema['items'], sheets, path, foreign_keys, len(path),
                     array_paths=array_paths+[path]  # since this level is an array, we extend the list
                 )
+                # and add the foreign keys that are necessary up to this point
                 for p in array_paths:
                     keys = self._get_foreign_keys(foreign_keys, p)
                     for k in keys:
-                        sheets[sheetname].update({k: (ColumnType.FOREIGN, f"see sheet '{path[0]}'", p+[k])})
+                        if k in sheets[sheetname]:
+                            raise ValueError(f"The shema would lead to two columns with the same "
+                                             f"name which is forbidden:{k}")
+                        sheets[sheetname][k] = (ColumnType.FOREIGN, f"see sheet '{path[0]}'", p+[k])
                 # columns are added to the new sheet, thus we do not return columns
                 return {}
-            else:
-                # it is a list of primitive types -> semi colon separated list
+            else:  # it is a list of primitive types -> semi colon separated list
                 schema = schema['items']
                 ctype = ColumnType.LIST
 
@@ -183,31 +192,34 @@ class TableTemplateGenerator(ABC):
                 if 'type' in el:
                     schema = el
 
-        if "properties" in schema:
-            # recurse for each property
+        if "properties" in schema:  # recurse for each property
             cols = {}
             for pname in schema["properties"].keys():
-                cols.update(self._treat_schema_element(
+                col_defs = self._treat_schema_element(
                     schema["properties"][pname], sheets, path+[pname], foreign_keys,
-                    level_in_sheet_name, array_paths=array_paths))
+                    level_in_sheet_name, array_paths=array_paths)
+                for k in col_defs.keys():
+                    if k in cols:
+                        raise ValueError(f"The shema would lead to two columns with the same "
+                                         f"name which is forbidden:{k}")
+                cols.update(col_defs)
             return cols
-        else:
+        else:  # those are the leaves
             description = schema['description'] if 'description' in schema else None
-
-            # those are the leaves
-            if 'type' not in schema:
-                if 'enum' in schema:
-                    return {".".join(path[level_in_sheet_name:]): (ctype, description, path)}
-                if 'anyOf' in schema:
-                    for d in schema['anyOf']:
-                        # currently the only case where this occurs is date formats
-                        assert d['type'] == 'string'
-                        assert d['format'] == 'date' or d['format'] == 'date-time'
-                    return {".".join(path[level_in_sheet_name:]): (ctype, description, path)}
+            # definition of a single column
+            default_return = {".".join(path[level_in_sheet_name:]): (ctype, description, path)}
+            if 'type' not in schema and 'enum' in schema:
+                return default_return
+            elif 'type' not in schema and 'anyOf' in schema:
+                for d in schema['anyOf']:
+                    # currently the only case where this occurs is date formats
+                    assert d['type'] == 'string'
+                    assert d['format'] == 'date' or d['format'] == 'date-time'
+                return default_return
             elif schema["type"] in ['string', 'number', 'integer', 'boolean']:
                 if 'format' in schema and schema['format'] == 'data-url':
                     return {}  # file; ignore for now
-                return {".".join(path[level_in_sheet_name:]): (ctype, description, path)}
+                return default_return
             else:
                 raise ValueError("Inappropriate JSON schema: The following part should define an"
                                  f" object with properties or a primitive type:\n{schema}\n")
-- 
GitLab