From 45d480cba2c47b69ae72da74e69e9e283a40e08d Mon Sep 17 00:00:00 2001 From: Daniel Gorelik Date: Thu, 13 Aug 2020 12:39:28 -0400 Subject: [PATCH] Revert "Add support for setting foreign keys" --- setup.py | 2 +- spanner_orm/admin/update.py | 14 +--- spanner_orm/foreign_key_relationship.py | 68 ------------------- spanner_orm/metadata.py | 15 ---- spanner_orm/model.py | 12 ---- spanner_orm/tests/migrations_emulator_test.py | 38 ++--------- .../create_foreign_key_test_model.py | 52 -------------- spanner_orm/tests/models.py | 15 ---- spanner_orm/tests/update_test.py | 21 ------ 9 files changed, 8 insertions(+), 229 deletions(-) delete mode 100644 spanner_orm/foreign_key_relationship.py delete mode 100644 spanner_orm/tests/migrations_for_emulator_test/create_foreign_key_test_model.py diff --git a/setup.py b/setup.py index 011756a..566b38b 100644 --- a/setup.py +++ b/setup.py @@ -25,7 +25,7 @@ include_package_data=True, python_requires='~=3.7', install_requires=['google-cloud-spanner >= 1.6, <2.0.0dev'], - tests_require=['absl-py', 'google-api-core', 'portpicker'], + tests_require=['absl-py', 'portpicker'], entry_points={ 'console_scripts': ['spanner-orm = spanner_orm.admin.scripts:main'] }) diff --git a/spanner_orm/admin/update.py b/spanner_orm/admin/update.py index a77a101..4277e10 100644 --- a/spanner_orm/admin/update.py +++ b/spanner_orm/admin/update.py @@ -49,23 +49,13 @@ def __init__(self, model_: Type[model.Model]): self._model = model_ def ddl(self) -> str: - key_fields = [ + fields = [ '{} {}'.format(name, field.ddl()) for name, field in self._model.fields.items() ] - key_fields_ddl = ', '.join(key_fields) - for relation in self._model.foreign_key_relations.values(): - for constraint in relation.constraints: - key_fields_ddl += ( - ', FOREIGN KEY ({referencing_column}) REFERENCES' - ' {referenced_table} ({referenced_column})').format( - referencing_column=constraint.referencing_column, - referenced_table=constraint.referenced_table_name, - referenced_column=constraint.referenced_column, - ) index_ddl = 'PRIMARY KEY ({})'.format(', '.join(self._model.primary_keys)) statement = 'CREATE TABLE {} ({}) {}'.format(self._model.table, - key_fields_ddl, index_ddl) + ', '.join(fields), index_ddl) if self._model.interleaved: statement += ', INTERLEAVE IN PARENT {parent} ON DELETE CASCADE'.format( diff --git a/spanner_orm/foreign_key_relationship.py b/spanner_orm/foreign_key_relationship.py deleted file mode 100644 index 6dc7cb4..0000000 --- a/spanner_orm/foreign_key_relationship.py +++ /dev/null @@ -1,68 +0,0 @@ -# python3 -# Copyright 2019 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Helps define a foreign key relationship between two models.""" - -from typing import List, Mapping - -import dataclasses -from spanner_orm import registry - - -@dataclasses.dataclass -class ForeignKeyRelationshipConstraint: - referencing_column: str - referenced_column: str - referenced_table_name: str - - -class ForeignKeyRelationship(object): - """Helps define a foreign key relationship between two models.""" - - def __init__(self, - referenced_table_name: str, - constraints: Mapping[str, str]): - """Creates a ForeignKeyRelationship. - - Args: - referenced_table_name: Name of the table which the foreign key references. - constraints: Dictionary where the keys are names of columns from the - referencing table and the values are the names of the columns in the - referenced table. - # TODO(dgorelik): Allow constraints to have custom names. - """ - self.origin = None - self.name = None - self._referenced_table_name = referenced_table_name - self._constraints = constraints - - @property - def constraints(self) -> List[ForeignKeyRelationshipConstraint]: - return self._parse_constraints() - - def _parse_constraints(self) -> List[ForeignKeyRelationshipConstraint]: - """Returns a list of Constraints for the relationship.""" - constraints = [] - referenced_table = registry.model_registry().get( - self._referenced_table_name) - for referencing_column, referenced_column in self._constraints.items(): - constraints.append( - ForeignKeyRelationshipConstraint( - referencing_column, - referenced_column, - referenced_table.table, - ) - ) - - return constraints diff --git a/spanner_orm/metadata.py b/spanner_orm/metadata.py index 251dfc2..8c7c5e2 100644 --- a/spanner_orm/metadata.py +++ b/spanner_orm/metadata.py @@ -32,7 +32,6 @@ from spanner_orm import error from spanner_orm import field -from spanner_orm import foreign_key_relationship from spanner_orm import index from spanner_orm import registry from spanner_orm import relationship @@ -45,11 +44,6 @@ def __init__(self, table: Optional[str] = None, fields: Optional[Dict[str, field.Field]] = None, relations: Optional[Dict[str, relationship.Relationship]] = None, - foreign_key_relations: Optional[ - Dict[ - str, - foreign_key_relationship.ForeignKeyRelationship] - ] = None, indexes: Optional[Dict[str, index.Index]] = None, interleaved: Optional[str] = None, model_class: Optional[Type[Any]] = None): @@ -61,7 +55,6 @@ def __init__(self, self.model_class = model_class self.primary_keys = [] self.relations = dict(relations or {}) - self.foreign_key_relations = dict(foreign_key_relations or {}) self.table = table or '' def finalize(self) -> None: @@ -108,14 +101,6 @@ def add_relation(self, name: str, new_relation.name = name self.relations[name] = new_relation - def add_foreign_key_relation( - self, - name: str, - new_relation: foreign_key_relationship.ForeignKeyRelationship, - ) -> None: - new_relation.name = name - self.foreign_key_relations[name] = new_relation - def add_index(self, name: str, new_index: index.Index) -> None: new_index.name = name self.indexes[name] = new_index diff --git a/spanner_orm/model.py b/spanner_orm/model.py index c21999d..06060cb 100644 --- a/spanner_orm/model.py +++ b/spanner_orm/model.py @@ -21,7 +21,6 @@ from spanner_orm import api from spanner_orm import condition from spanner_orm import error -from spanner_orm import foreign_key_relationship from spanner_orm import field from spanner_orm import index from spanner_orm import metadata @@ -59,11 +58,6 @@ def __new__(mcs, name: str, bases: Any, attrs: Dict[str, Any], **kwargs: Any): model_metadata.add_index(key, value) elif isinstance(value, relationship.Relationship): model_metadata.add_relation(key, value) - elif isinstance( - value, - foreign_key_relationship.ForeignKeyRelationship, - ): - model_metadata.add_foreign_key_relation(key, value) else: non_model_attrs[key] = value @@ -118,12 +112,6 @@ def primary_keys(cls) -> List[str]: def relations(cls) -> Dict[str, relationship.Relationship]: return cls.meta.relations - @property - def foreign_key_relations( - cls) -> Dict[str, foreign_key_relationship.ForeignKeyRelationship]: - return cls.meta.foreign_key_relations - - @property def fields(cls) -> Dict[str, field.Field]: return cls.meta.fields diff --git a/spanner_orm/tests/migrations_emulator_test.py b/spanner_orm/tests/migrations_emulator_test.py index 75d03ce..3e4f8a1 100644 --- a/spanner_orm/tests/migrations_emulator_test.py +++ b/spanner_orm/tests/migrations_emulator_test.py @@ -12,16 +12,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import datetime import logging import os import unittest -from spanner_orm.testlib.spanner_emulator import testlib as spanner_emulator_testlib -from spanner_orm.tests import models - -from google.api_core import exceptions as google_api_exceptions +import spanner_orm +from spanner_orm.tests import models +from spanner_orm.testlib.spanner_emulator import testlib as spanner_emulator_testlib class MigrationsEmulatorTest(spanner_emulator_testlib.TestCase): TEST_MIGRATIONS_DIR = os.path.join( @@ -34,39 +32,13 @@ def setUp(self): self.run_orm_migrations(self.TEST_MIGRATIONS_DIR) def test_basic(self): - models.SmallTestModel({'key': 'key', 'value_1': 'value'}).save() + test_model = models.SmallTestModel({'key': 'key', 'value_1': 'value'}) + test_model.save() self.assertEqual( [x.values for x in models.SmallTestModel.all()], [{'key': 'key', 'value_1': 'value', 'value_2': None}], ) - def test_error_with_missing_referencing_key(self): - with self.assertRaisesRegex( - google_api_exceptions.FailedPrecondition, - 'Cannot find referenced key', - ): - models.ForeignKeyTestModel({ - 'referencing_key_1': 'key', - 'referencing_key_2': 'key', - 'referencing_key_3': 42, - 'value': 'value' - }).save() - - def test_key(self): - models.SmallTestModel({'key': 'key', 'value_1': 'value'}).save() - models.UnittestModel( - {'string': 'string', - 'int_': 42, - 'float_': 4.2, - 'timestamp': datetime.datetime.now(tz=datetime.timezone.utc), - }).save() - models.ForeignKeyTestModel({ - 'referencing_key_1': 'key', - 'referencing_key_2': 'string', - 'referencing_key_3': 42, - 'value': 'value' - }).save() - if __name__ == '__main__': logging.basicConfig() unittest.main() diff --git a/spanner_orm/tests/migrations_for_emulator_test/create_foreign_key_test_model.py b/spanner_orm/tests/migrations_for_emulator_test/create_foreign_key_test_model.py deleted file mode 100644 index a88811d..0000000 --- a/spanner_orm/tests/migrations_for_emulator_test/create_foreign_key_test_model.py +++ /dev/null @@ -1,52 +0,0 @@ -# Lint as: python3 -# Copyright 2020 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Creates table with ForeignKeyTestModel. - -Migration ID: 'f735d6b706d3' -Created: 2020-07-10 16:24 -""" - -import spanner_orm -from spanner_orm import field -from spanner_orm import foreign_key_relationship - -migration_id = 'f735d6b706d4' -prev_migration_id = 'f735d6b706d3' - - -class OriginalForeignKeyTestModelTable(spanner_orm.model.Model): - """ORM Model with the original schema for the ForeignKeyTestModel table.""" - - __table__ = 'ForeignKeyTestModel' - referencing_key_1 = field.Field(field.String, primary_key=True) - referencing_key_2 = field.Field(field.String, primary_key=True) - referencing_key_3 = field.Field(field.Integer, primary_key=True) - value = field.Field(field.String) - foreign_key_1 = foreign_key_relationship.ForeignKeyRelationship( - 'SmallTestModel', {'referencing_key_1': 'key'}) - foreign_key_2 = foreign_key_relationship.ForeignKeyRelationship( - 'UnittestModel', - {'referencing_key_2': 'string', 'referencing_key_3': 'int_'}, - ) - - -def upgrade() -> spanner_orm.CreateTable: - """See ORM migrations interface.""" - return spanner_orm.CreateTable(OriginalForeignKeyTestModelTable) - - -def downgrade() -> spanner_orm.DropTable: - """See ORM migrations interface.""" - return spanner_orm.DropTable(OriginalForeignKeyTestModelTable.__table__) diff --git a/spanner_orm/tests/models.py b/spanner_orm/tests/models.py index e82c071..9aef1ed 100644 --- a/spanner_orm/tests/models.py +++ b/spanner_orm/tests/models.py @@ -15,7 +15,6 @@ """Models used by unit tests.""" from spanner_orm import field -from spanner_orm import foreign_key_relationship from spanner_orm import index from spanner_orm import model from spanner_orm import relationship @@ -62,20 +61,6 @@ class RelationshipTestModel(model.Model): parents = relationship.Relationship('spanner_orm.tests.models.SmallTestModel', {'parent_key': 'key'}) -class ForeignKeyTestModel(model.Model): - """Model class for testing foreign keys.""" - - __table__ = 'ForeignKeyTestModel' - referencing_key_1 = field.Field(field.String, primary_key=True) - referencing_key_2 = field.Field(field.String, primary_key=True) - referencing_key_3 = field.Field(field.Integer, primary_key=True) - value = field.Field(field.String) - foreign_key_1 = foreign_key_relationship.ForeignKeyRelationship( - 'SmallTestModel', {'referencing_key_1': 'key'}) - foreign_key_2 = foreign_key_relationship.ForeignKeyRelationship( - 'UnittestModel', - {'referencing_key_2': 'string', 'referencing_key_3': 'int_'}, - ) class InheritanceTestModel(SmallTestModel): """Model class used for testing model inheritance.""" diff --git a/spanner_orm/tests/update_test.py b/spanner_orm/tests/update_test.py index 9b84b31..633b0c3 100644 --- a/spanner_orm/tests/update_test.py +++ b/spanner_orm/tests/update_test.py @@ -86,27 +86,6 @@ def test_create_table_interleaved(self, get_model): 'INTERLEAVE IN PARENT SmallTestModel ON DELETE CASCADE') self.assertEqual(test_update.ddl(), test_model_ddl) - @mock.patch('spanner_orm.admin.metadata.SpannerMetadata.model') - def test_create_table_foreign_key(self, get_model): - self.maxDiff = 2000 - - get_model.return_value = None - new_model = models.ForeignKeyTestModel - test_update = update.CreateTable(new_model) - test_update.validate() - - test_model_ddl = ( - 'CREATE TABLE ForeignKeyTestModel (' - 'referencing_key_1 STRING(MAX) NOT NULL, ' - 'referencing_key_2 STRING(MAX) NOT NULL, ' - 'referencing_key_3 INT64 NOT NULL, ' - 'value STRING(MAX) NOT NULL, ' - 'FOREIGN KEY (referencing_key_1) REFERENCES SmallTestModel (key), ' - 'FOREIGN KEY (referencing_key_2) REFERENCES table (string), ' - 'FOREIGN KEY (referencing_key_3) REFERENCES table (int_)) ' - 'PRIMARY KEY (referencing_key_1, referencing_key_2, referencing_key_3)') - self.assertEqual(test_update.ddl(), test_model_ddl) - @mock.patch('spanner_orm.admin.metadata.SpannerMetadata.model') def test_create_table_error_on_existing_table(self, get_model): get_model.return_value = models.SmallTestModel