Merge pull request #1902 from OpenEnergyPlatform/release-v1.0.5-fixed

Release v1.0.5 fixed

Author: jh-RLI

.bumpversion.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 1.0.4
+current_version = 1.0.5
 
 [bumpversion:file:VERSION]
 

.gitignore

@@ -79,6 +79,7 @@ venv*/
 0_env/
 /envs
 /node_env
+/oep-django-5
 
 .DS_Store
 
@@ -98,3 +99,7 @@ ontologies/
 scripts/
 
 site/
+
+# Jenna fuseki graph db installation
+/apache-jena-fuseki*/
+/apache-jena-fuseki*.tar.gz*

.pre-commit-config.yaml

@@ -1,4 +1,4 @@
-exclude: ^.*(.min.js|.min.css|.html|.js|.js.map)$
+exclude: ^.*(.min.js|.min.css|.html|.js|.js.map|docs/*|mkdocs.yml)$
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.4.0

.prettierignore

@@ -1,3 +1,5 @@
 # Ignore all HTML files, because they contain
 # django markup that will not be broken by prettier
 **/*.html
+
+docs/*

CITATION.cff

@@ -28,7 +28,7 @@ authors:
 title: "Open Energy Family - Open Energy Platform (OEP)"
 type: software
 license: AGPL-3.0-or-later
-version: 1.0.4
+version: 1.0.5
 doi:
-date-released: 2024-09-25
+date-released: 2024-11-19
 url: "https://github.com/OpenEnergyPlatform/oeplatform/"

RELEASE_PROCEDURE.md

@@ -52,7 +52,7 @@ Before see How to [Contribute](https://github.com/OpenEnergyPlatform/oeplatform/
    - `versioneer` automatically updates the version number based on the tag
    - this is now the official tagged commit
    - Push the tag upstream: `git push upstream --tags`
-   - Alternatievely: tag on github platform while creating release
+   - Alternatively: tag on github platform while creating release
 1. Make a new release on Github
    - https://github.com/OpenEnergyPlatform/oeplatform/releases/new
    - make sure that you choose the tag name defined above

VERSION

@@ -1 +1 @@
-1.0.4
+1.0.5

api/actions.py

@@ -857,11 +857,15 @@ def table_create(schema, table, column_definitions, constraints_definitions):
 
         # check for duplicate column names
         if col.name in columns_by_name:
-            raise APIError("Duplicate column name: %s" % col.name)
+            error = APIError("Duplicate column name: %s" % col.name)
+            logger.error(error)
+            raise error
         columns_by_name[col.name] = col
         if col.primary_key:
             if primary_key_col_names:
-                raise APIError("Multiple definitions of primary key")
+                error = APIError("Multiple definitions of primary key")
+                logger.error(error)
+                raise error
             primary_key_col_names = [col.name]
 
     constraints = []
@@ -887,7 +891,7 @@ def table_create(schema, table, column_definitions, constraints_definitions):
                 # to column level PK, both must be the same (#1110)
                 if set(ccolumns) == set(primary_key_col_names):
                     continue
-                raise APIError("Multiple definitions of primary key")
+                raise APIError("Multiple definitions of primary key.")
             primary_key_col_names = ccolumns
 
             const = sa.schema.PrimaryKeyConstraint(*ccolumns, **kwargs)

api/tests/test_regression/test_issue_1885_table_creation_fail_behavior.py

@@ -0,0 +1,43 @@
+from api import actions
+from api.actions import has_table
+from api.tests import APITestCase
+
+
+class TestTableNameUnique(APITestCase):
+    schema_sandbox = "sandbox"
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        actions.perform_sql(f"DROP SCHEMA IF EXISTS {cls.schema_sandbox} CASCADE")
+        actions.perform_sql(f"CREATE SCHEMA {cls.schema_sandbox}")
+        actions.perform_sql(f"DROP SCHEMA IF EXISTS _{cls.schema_sandbox} CASCADE")
+        actions.perform_sql(f"CREATE SCHEMA _{cls.schema_sandbox}")
+
+    def test_tables_should_not_exists_on_error(self):
+        test_duplicate_column_table_name = "table_column_duplicate"
+        # create table with duplicated column names will should an error
+        duplicate_field_error_data_struct = {
+            "columns": [
+                {"name": "id", "data_type": "bigint"},
+                {"name": "id", "data_type": "bigint"},
+            ]
+        }
+        # create table in default (test) schema (django_db)
+        self.assertRaises(
+            AssertionError,
+            self.create_table,
+            table=test_duplicate_column_table_name,
+            structure=duplicate_field_error_data_struct,
+            schema=self.schema_sandbox,
+        )
+
+        # also check: table should not have been created in oedb
+        self.assertFalse(
+            has_table(
+                {
+                    "table": test_duplicate_column_table_name,
+                    "schema": self.schema_sandbox,
+                }
+            )
+        )

api/views.py

@@ -14,6 +14,7 @@
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.contrib.postgres.search import TrigramSimilarity
 from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
+from django.db import DatabaseError, transaction
 from django.db.models import Q
 from django.db.utils import IntegrityError
 from django.http import (
@@ -482,6 +483,100 @@ def validate_column_names(self, column_definitions):
             if len(colname) > MAX_COL_NAME_LENGTH:
                 raise APIError(f"Column name is too long! {err_msg}")
 
+    def oep_create_table_transaction(
+        self,
+        django_schema_object,
+        schema,
+        table,
+        column_definitions,
+        constraint_definitions,
+    ):
+        """
+        This method handles atomic table creation transactions on the OEP. It
+        attempts to create first the django table objects and stored it in
+        dataedit_tables table. Then it attempts to create the OEDB table.
+        If there is an error raised during the first two steps the function
+        will cleanup any table object or table artifacts created during the
+        process. The order of execution matters, it should always first
+        create the django table object.
+
+        Params:
+            django_schema_object: The schema object stored in the django
+                database
+            schema:
+            table
+            column_definitions
+            constraint_definitions
+
+        returns:
+            table_object: The django table objects that was created
+        """
+
+        try:
+            with transaction.atomic():
+                # First create the table object in the django database.
+                table_object = self._create_table_object(django_schema_object, table)
+            # Then attempt to create the OEDB table to check
+            # if creation will succeed - action includes checks
+            # and will raise api errors
+            actions.table_create(
+                schema, table, column_definitions, constraint_definitions
+            )
+        except DatabaseError as e:
+            # remove any oedb table artifacts left after table creation
+            # transaction failed
+            self.__remove_oedb_table_on_exception_raised_during_creation_transaction(
+                table, schema
+            )
+
+            # also remove any django table object
+            # find the created django table object
+            object_to_delete = DBTable.objects.filter(
+                name=table, schema=django_schema_object
+            )
+            # delete it if it exists
+            if object_to_delete.exists():
+                object_to_delete.delete()
+
+            raise APIError(
+                message="Error during table creation transaction. All table fragments"
+                f"have been removed. For further details see: {e}"
+            )
+
+        # for now only return the django table object
+        # TODO: Check if is necessary to return the response dict returned by the oedb
+        # table creation function
+        return table_object
+
+    def __remove_oedb_table_on_exception_raised_during_creation_transaction(
+        self, table, schema
+    ):
+        """
+        This private method handles removing a table form the OEDB only for the case
+        where an error was raised during table creation. It specifically will delete
+        the OEDB table created by the user and also the edit_ meta(revision) table
+        that is automatically created in the background.
+        """
+        # find the created oedb table
+        if actions.has_table({"table": table, "schema": schema}):
+            # get table and schema names, also for meta(revision) tables
+            schema, table = actions.get_table_name(schema, table)
+            meta_schema = actions.get_meta_schema_name(schema)
+
+            # drop the revision table with edit_ prefix
+            edit_table = actions.get_edit_table_name(schema, table)
+            actions._get_engine().execute(
+                'DROP TABLE "{schema}"."{table}" CASCADE;'.format(
+                    schema=meta_schema, table=edit_table
+                )
+            )
+            # drop the data table
+            actions._get_engine().execute(
+                'DROP TABLE "{schema}"."{table}" CASCADE;'.format(
+                    schema=schema, table=table
+                )
+            )
+
     @load_cursor()
     def __create_table(
         self,
@@ -510,11 +605,13 @@ def __create_table(
             raise embargo_error
 
         if embargo_payload_check:
-            table_object = self._create_table_object(schema_object, table)
-            actions.table_create(
-                schema, table, column_definitions, constraint_definitions
+            table_object = self.oep_create_table_transaction(
+                django_schema_object=schema_object,
+                table=table,
+                schema=schema,
+                column_definitions=column_definitions,
+                constraint_definitions=constraint_definitions,
             )
-
             self._apply_embargo(table_object, embargo_data)
 
             if metadata:
@@ -534,9 +631,12 @@ def __create_table(
                 )
 
         else:
-            table_object = self._create_table_object(schema_object, table)
-            actions.table_create(
-                schema, table, column_definitions, constraint_definitions
+            table_object = self.oep_create_table_transaction(
+                django_schema_object=schema_object,
+                table=table,
+                schema=schema,
+                column_definitions=column_definitions,
+                constraint_definitions=constraint_definitions,
             )
             self._assign_table_holder(request.user, schema, table)