Merge pull request piccolo-orm#127 from piccolo-orm/migration_integration_testing

Migration integration testing

Author: dantownsend

piccolo/apps/migrations/auto/serialisation.py

@@ -13,6 +13,7 @@
 from piccolo.columns.defaults.base import Default
 from piccolo.columns.reference import LazyTableReference
 from piccolo.table import Table
+from piccolo.utils.repr import repr_class_instance
 from .serialisation_legacy import deserialise_legacy_params
 
 ###############################################################################
@@ -43,13 +44,7 @@ def __eq__(self, other):
         return self.__hash__() == other.__hash__()
 
     def __repr__(self):
-        args = ", ".join(
-            [
-                f"{key}={value.__repr__()}"
-                for key, value in self.instance.__dict__.items()
-            ]
-        )
-        return f"{self.instance.__class__.__name__}({args})"
+        return repr_class_instance(self.instance)
 
 
 @dataclass
@@ -149,7 +144,7 @@ def __eq__(self, other):
         return self.__hash__() == other.__hash__()
 
     def __repr__(self):
-        return f"UUID({str(self.instance)})"
+        return f"UUID('{str(self.instance)}')"
 
 
 ###############################################################################
@@ -327,6 +322,14 @@ def serialise_params(params: t.Dict[str, t.Any]) -> SerialisedParams:
             )
             continue
 
+        # Plain class type
+        if inspect.isclass(value) and not issubclass(value, Enum):
+            params[key] = SerialisedCallable(callable_=value)
+            extra_imports.append(
+                Import(module=value.__module__, target=value.__name__)
+            )
+            continue
+
         # All other types can remain as is.
 
     return SerialisedParams(

piccolo/apps/migrations/commands/base.py

@@ -6,6 +6,7 @@
 import typing as t
 
 from piccolo.conf.apps import (
+    AppConfig,
     MigrationModule,
     Finder,
 )
@@ -73,7 +74,7 @@ def get_migration_ids(
 
     async def get_migration_managers(
         self,
-        app_name: str,
+        app_config: AppConfig,
         max_migration_id: t.Optional[str] = None,
         offset: int = 0,
     ) -> t.List[MigrationManager]:
@@ -88,8 +89,6 @@ async def get_migration_managers(
         """
         migration_managers: t.List[MigrationManager] = []
 
-        app_config = self.get_app_config(app_name=app_name)
-
         migrations_folder = app_config.migrations_folder_path
 
         migration_modules: t.Dict[
@@ -129,8 +128,11 @@ async def get_table_from_snaphot(
         This will generate a SchemaSnapshot up to the given migration ID, and
         will return a DiffableTable class from that snapshot.
         """
+        app_config = self.get_app_config(app_name=app_name)
         migration_managers = await self.get_migration_managers(
-            app_name=app_name, max_migration_id=max_migration_id, offset=offset
+            app_config=app_config,
+            max_migration_id=max_migration_id,
+            offset=offset,
         )
         schema_snapshot = SchemaSnapshot(managers=migration_managers)
 

piccolo/apps/migrations/commands/forwards.py

@@ -12,15 +12,17 @@
 
 
 class ForwardsMigrationManager(BaseMigrationManager):
-    def __init__(self, app_name: str, migration_id: str, fake: bool = False):
+    def __init__(
+        self, app_name: str, migration_id: str = "all", fake: bool = False
+    ):
         self.app_name = app_name
         self.migration_id = migration_id
         self.fake = fake
         super().__init__()
 
     async def run_migrations(self, app_config: AppConfig) -> MigrationResult:
         already_ran = await Migration.get_migrations_which_ran(
-            app_name=self.app_name
+            app_name=app_config.app_name
         )
 
         migration_modules: t.Dict[
@@ -67,7 +69,7 @@ async def run_migrations(self, app_config: AppConfig) -> MigrationResult:
                 print(f"-> Ran {_id}")
 
             await Migration.insert().add(
-                Migration(name=_id, app_name=self.app_name)
+                Migration(name=_id, app_name=app_config.app_name)
             ).run()
 
         return MigrationResult(success=True, message="Ran successfully")

piccolo/apps/migrations/commands/new.py

@@ -1,4 +1,5 @@
 from __future__ import annotations
+from dataclasses import dataclass
 import datetime
 from itertools import chain
 import os
@@ -52,11 +53,22 @@ def _create_migrations_folder(migrations_path: str) -> bool:
         return True
 
 
-async def _create_new_migration(app_config: AppConfig, auto=False) -> None:
+@dataclass
+class NewMigrationMeta:
+    migration_id: str
+    migration_filename: str
+    migration_path: str
+
+
+def _generate_migration_meta(app_config: AppConfig) -> NewMigrationMeta:
     """
-    Creates a new migration file on disk.
+    Generates the migration ID and filename.
     """
-    _id = datetime.datetime.now().strftime("%Y-%m-%dT%H:%M:%S")
+    # The microseconds originally weren't part of the ID, but there was a
+    # chance that the IDs would clash if the migrations are generated
+    # programatically in quick succession (e.g. in a unit test), so they had
+    # to be added. The trade off is a longer ID.
+    _id = datetime.datetime.now().strftime("%Y-%m-%dT%H:%M:%S:%f")
 
     # Originally we just used the _id as the filename, but colons aren't
     # supported in Windows, so we need to sanitize it. We don't want to
@@ -66,6 +78,23 @@ async def _create_new_migration(app_config: AppConfig, auto=False) -> None:
 
     path = os.path.join(app_config.migrations_folder_path, f"{filename}.py")
 
+    return NewMigrationMeta(
+        migration_id=_id, migration_filename=filename, migration_path=path
+    )
+
+
+class NoChanges(Exception):
+    pass
+
+
+async def _create_new_migration(
+    app_config: AppConfig, auto=False
+) -> NewMigrationMeta:
+    """
+    Creates a new migration file on disk.
+    """
+    meta = _generate_migration_meta(app_config=app_config)
+
     if auto:
         alter_statements = await AutoMigrationManager().get_alter_statements(
             app_config=app_config
@@ -83,28 +112,31 @@ async def _create_new_migration(app_config: AppConfig, auto=False) -> None:
         )
 
         if sum([len(i.statements) for i in alter_statements]) == 0:
-            print("No changes detected - exiting.")
-            sys.exit(0)
+            raise NoChanges()
 
         file_contents = render_template(
-            migration_id=_id,
+            migration_id=meta.migration_id,
             auto=True,
             alter_statements=_alter_statements,
             extra_imports=extra_imports,
             extra_definitions=extra_definitions,
             app_name=app_config.app_name,
         )
     else:
-        file_contents = render_template(migration_id=_id, auto=False)
+        file_contents = render_template(
+            migration_id=meta.migration_id, auto=False
+        )
 
     # Beautify the file contents a bit.
     file_contents = black.format_str(
         file_contents, mode=black.FileMode(line_length=82)
     )
 
-    with open(path, "w") as f:
+    with open(meta.migration_path, "w") as f:
         f.write(file_contents)
 
+    return meta
+
 
 ###############################################################################
 
@@ -117,7 +149,7 @@ async def get_alter_statements(
         Works out which alter statements are required.
         """
         migration_managers = await self.get_migration_managers(
-            app_name=app_config.app_name
+            app_config=app_config
         )
 
         schema_snapshot = SchemaSnapshot(migration_managers)
@@ -164,4 +196,8 @@ async def new(app_name: str, auto: bool = False):
     app_config = Finder().get_app_config(app_name=app_name)
 
     _create_migrations_folder(app_config.migrations_folder_path)
-    await _create_new_migration(app_config=app_config, auto=auto)
+    try:
+        await _create_new_migration(app_config=app_config, auto=auto)
+    except NoChanges:
+        print("No changes detected - exiting.")
+        sys.exit(0)

piccolo/columns/base.py

@@ -7,6 +7,7 @@
 from enum import Enum
 import inspect
 import typing as t
+import uuid
 
 from piccolo.columns.operators.comparison import (
     ComparisonOperator,
@@ -27,6 +28,7 @@
 from piccolo.columns.combination import Where
 from piccolo.columns.choices import Choice
 from piccolo.columns.defaults.base import Default
+from piccolo.columns.defaults.interval import IntervalCustom
 from piccolo.columns.reference import LazyTableReference
 from piccolo.columns.indexes import IndexMethod
 from piccolo.querystring import QueryString
@@ -342,6 +344,7 @@ def _validate_default(
         self,
         default: t.Any,
         allowed_types: t.Iterable[t.Union[None, t.Type[t.Any]]],
+        allow_recursion: bool = True,
     ) -> bool:
         """
         Make sure that the default value is of the allowed types.
@@ -358,18 +361,23 @@ def _validate_default(
             self._validated = True
             return True
         elif callable(default):
-            self._validated = True
-            return True
+            # We need to prevent recursion, otherwise a function which returns
+            # a function would be an infinite loop.
+            if allow_recursion and self._validate_default(
+                default(), allowed_types=allowed_types, allow_recursion=False
+            ):
+                self._validated = True
+                return True
         elif (
             isinstance(default, Enum) and type(default.value) in allowed_types
         ):
             self._validated = True
             return True
-        else:
-            raise ValueError(
-                f"The default {default} isn't one of the permitted types - "
-                f"{allowed_types}"
-            )
+
+        raise ValueError(
+            f"The default {default} isn't one of the permitted types - "
+            f"{allowed_types}"
+        )
 
     def _validate_choices(
         self, choices: t.Type[Enum], allowed_type: t.Type[t.Any]
@@ -533,9 +541,18 @@ def get_sql_value(self, value: t.Any) -> t.Any:
         elif isinstance(value, bool):
             output = str(value).lower()
         elif isinstance(value, datetime.datetime):
-            output = f"'{value.isoformat().replace('T', '')}'"
+            output = f"'{value.isoformat().replace('T', ' ')}'"
+        elif isinstance(value, datetime.date):
+            output = f"'{value.isoformat()}'"
+        elif isinstance(value, datetime.time):
+            output = f"'{value.isoformat()}'"
+        elif isinstance(value, datetime.timedelta):
+            interval = IntervalCustom.from_timedelta(value)
+            output = getattr(interval, self._meta.engine_type)
         elif isinstance(value, bytes):
             output = f"'{value.hex()}'"
+        elif isinstance(value, uuid.UUID):
+            output = f"'{value}'"
         elif isinstance(value, list):
             # Convert to the array syntax.
             output = (

piccolo/columns/column_types.py

@@ -303,11 +303,23 @@ class Band(Table):
     value_type = uuid.UUID
 
     def __init__(self, default: UUIDArg = UUID4(), **kwargs) -> None:
+        if default is UUID4:
+            # In case the class is passed in, instead of an instance.
+            default = UUID4()
+
         self._validate_default(default, UUIDArg.__args__)  # type: ignore
 
         if default == uuid.uuid4:
             default = UUID4()
 
+        if isinstance(default, str):
+            try:
+                default = uuid.UUID(default)
+            except ValueError:
+                raise ValueError(
+                    "The default is a string, but not a valid uuid."
+                )
+
         self.default = default
         kwargs.update({"default": default})
         super().__init__(**kwargs)
@@ -486,7 +498,7 @@ def column_type(self):
 
 class Serial(Column):
     """
-    An alias to an autoincremenring integer column in Postgres.
+    An alias to an autoincrementing integer column in Postgres.
     """
 
     def __init__(self, **kwargs) -> None:
@@ -656,6 +668,9 @@ def __init__(self, default: DateArg = DateNow(), **kwargs) -> None:
         if isinstance(default, date):
             default = DateCustom.from_date(default)
 
+        if default == date.today:
+            default = DateNow()
+
         self.default = default
         kwargs.update({"default": default})
         super().__init__(**kwargs)

piccolo/columns/defaults/base.py

@@ -2,6 +2,8 @@
 from abc import abstractmethod, abstractproperty, ABC
 import typing as t
 
+from piccolo.utils.repr import repr_class_instance
+
 
 class Default(ABC):
     @abstractproperty
@@ -50,11 +52,11 @@ def get_sqlite_interval_string(self, attributes: t.List[str]) -> str:
 
         return ", ".join(interval_components)
 
+    def __repr__(self):
+        return repr_class_instance(self)
+
     def __str__(self):
-        args_str = ", ".join(
-            [f"{key}={value}" for key, value in self.__dict__.items()]
-        )
-        return f"{self.__class__.__name__}({args_str})"
+        return self.__repr__()
 
     def __eq__(self, other):
         return self.__hash__() == other.__hash__()

piccolo/columns/defaults/date.py

@@ -50,11 +50,11 @@ def __init__(
 
     @property
     def postgres(self):
-        return self.date.isostring()
+        return f"'{self.date.isoformat()}'"
 
     @property
     def sqlite(self):
-        return self.date.isostring()
+        return f"'{self.date.isoformat()}'"
 
     def python(self):
         return self.date

piccolo/columns/defaults/time.py

@@ -57,11 +57,11 @@ def __init__(self, hour: int, minute: int, second: int):
 
     @property
     def postgres(self):
-        return self.time.isostring()
+        return f"'{self.time.isoformat()}'"
 
     @property
     def sqlite(self):
-        return self.time.isostring()
+        return f"'{self.time.isoformat()}'"
 
     def python(self):
         return self.time