Merge pull request #36 from ESSS/fb-EDEN-2586-fix-support-for-nested-…

…polymorphic

Fix issues with nested polymorphic for sqlalchemy 1.3 and 1.4

.github/workflows/main.yml

@@ -12,17 +12,6 @@ jobs:
       matrix:
         python: ["3.6", "3.7", "3.8", "3.9", "3.10"]
         os: [ubuntu-20.04, windows-latest]
-        include:
-          - python: "3.6"
-            tox_env: "py36"
-          - python: "3.7"
-            tox_env: "py37"
-          - python: "3.8"
-            tox_env: "py38"
-          - python: "3.9"
-            tox_env: "py39"
-          - python: "3.10"
-            tox_env: "py310"
         exclude:
           - python: "3.6"
             os: "windows-latest"
@@ -36,10 +25,9 @@ jobs:
       - name: Install tox
         run: |
           python -m pip install --upgrade pip
-          pip install tox
-      - name: Test
-        run: |
-          tox -e ${{ matrix.tox_env }}
+          python -m pip install tox tox-gh-actions
+      - name: Test with tox
+        run: tox
 
   deploy:
 
@@ -54,7 +42,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v1
         with:
-          python-version: "3.7"
+          python-version: "3.10"
       - name: Install wheel
         run: |
           python -m pip install --upgrade pip

environment.devenv.yml

@@ -1,15 +1,24 @@
-name: serialchemy
+{% set PY = os.environ['CONDA_PY'] | int %}
+name: serialchemy-py{{ PY }}
 
 dependencies:
   - black>=19.3b0
   - mypy
   - pre_commit
   - pytest
+  - pytest-cov
   - pytest-datadir
   - pytest-mock>=1.10
   - pytest-regressions>=1.0.0
-  - python>=3.6
-  - sqlalchemy>=1.2
+  - pytest-freezegun ==0.4.2
+  {% if PY==36 %}
+  - python ==3.6.11
+  {% elif PY==310 %}
+  - python ==3.10.4  # [win]
+  - python ==3.10.2  # [linux]
+  {% endif %}
+  - sqlalchemy ==1.3.15  # [PY==36]
+  - sqlalchemy ==1.4.44  # [PY==310]
   - sqlalchemy-utc>=0.10
   - sqlalchemy-utils>=0.33
   - tox

setup.py

@@ -16,6 +16,7 @@
         "pytest",
         "pytest-cov",
         "pytest-regressions",
+        "pytest-freezegun",
         "pre-commit",
         "tox",
         "sqlalchemy_utils",

src/serialchemy/_tests/sample_model.py

@@ -23,6 +23,10 @@ class Company(Base):
     name = Column(String)
     location = Column(String)
     employees = relationship("Employee", lazy='dynamic')
+    master_engeneer_id = Column(Integer, ForeignKey('SpecialistEngineer.esp_id'))
+    master_engeneer = relationship('SpecialistEngineer', foreign_keys=[master_engeneer_id])
+    master_manager_id = Column(Integer, ForeignKey('Manager.id'))
+    master_manager = relationship('Manager', foreign_keys=[master_manager_id])
 
 
 class Department(Base):
@@ -75,7 +79,7 @@ class Employee(Base):
 
     __tablename__ = 'Employee'
 
-    id = Column(Integer, primary_key=True)
+    id = Column('id', Integer, primary_key=True)
     firstname = Column(String)
     lastname = Column(String)
     email = Column(String)
@@ -117,7 +121,7 @@ class Engineer(Employee):
 
     __tablename__ = 'Engineer'
 
-    id = Column(Integer, ForeignKey('Employee.id'), primary_key=True)
+    id = Column('eng_id',Integer, ForeignKey('Employee.id'), primary_key=True)
     engineer_name = Column(String(30))
 
     __mapper_args__ = {'polymorphic_identity': 'Engineer', 'polymorphic_on': Employee.role}
@@ -127,7 +131,7 @@ class SpecialistEngineer(Engineer):
 
     __tablename__ = 'SpecialistEngineer'
 
-    id = Column(Integer, ForeignKey('Engineer.id'), primary_key=True)
+    id = Column('esp_id', Integer, ForeignKey('Engineer.eng_id'), primary_key=True)
     specialization = Column(String(30))
 
     __mapper_args__ = {
@@ -139,7 +143,7 @@ class Manager(Employee):
 
     __tablename__ = 'Manager'
 
-    id = Column(Integer, ForeignKey('Employee.id'), primary_key=True)
+    id = Column('id', Integer, ForeignKey('Employee.id'), primary_key=True)
     manager_name = Column(String(30))
 
     __mapper_args__ = {

src/serialchemy/_tests/test_func/test_dump.yml

@@ -3,6 +3,8 @@ admission: '2000-01-01'
 company:
   id: 5
   location: Korhal
+  master_engeneer_id: null
+  master_manager_id: null
   name: Terrans
 contract_type: null
 created_at: '2000-01-02T00:00:00'

src/serialchemy/_tests/test_nested_fields.py

@@ -1,4 +1,5 @@
 import pytest
+from freezegun import freeze_time
 
 from serialchemy import ModelSerializer
 from serialchemy._tests.sample_model import (
@@ -29,6 +30,12 @@ class EmployeeSerializerNestedAttrsFields(ModelSerializer):
     company = NestedAttributesField(("name", "location"))
 
 
+class CompanySerializer(ModelSerializer):
+
+    master_engeneer = NestedModelField(SpecialistEngineer)
+    master_manager = NestedModelField(Manager)
+
+
 @pytest.fixture(autouse=True)
 def setup(db_session):
     company = Company(id=5, name='Terrans', location='Korhal')
@@ -48,6 +55,11 @@ def setup(db_session):
     db_session.add_all([company, emp1, emp2, emp3, emp4])
     db_session.commit()
 
+    company.master_engeneer = emp4
+    company.master_manager = emp1
+    db_session.commit()
+
+
 
 @pytest.mark.parametrize(
     "serializer_class",
@@ -102,3 +114,37 @@ def test_empty_nested(db_session):
     assert serialized["company"] is None
     model = serializer.load(serialized, session=db_session)
     assert model.company is None
+
+
+def test_dump_with_nested_polymorphic(db_session, data_regression):
+    serializer = CompanySerializer(Company)
+    serialized = serializer.dump(db_session.query(Company).first())
+    data_regression.check(serialized)
+
+
+@freeze_time("2021-06-15")
+def test_load_with_nested_polymorphic_with_different_table_pk_names(db_session, data_regression):
+    # SpecializedEngeneer and its base class Engeneer have different names for the primary key on the database table
+    serializer = CompanySerializer(Company)
+    serialized = {
+        'id': 5,
+        'master_engeneer': {
+            'id': 4
+        }
+    }
+    model = serializer.load(serialized, session=db_session)
+    data_regression.check(serializer.dump(model))
+
+
+def test_load_with_nested_polymorphic_same_table_pk_names(db_session, data_regression):
+    # Manager and its base class Empoyee have the same name for the primary key on the database table
+    serializer = CompanySerializer(Company)
+    serialized = {
+        'id': 5,
+        'master_manager': {
+            'id': 1
+        }
+    }
+    model = serializer.load(serialized, session=db_session)
+    data_regression.check(serializer.dump(model))
+

src/serialchemy/_tests/test_nested_fields/test_custom_serializer_EmployeeSerializerNestedModelFields.yml

@@ -10,6 +10,8 @@ admission: '2000-01-01'
 company:
   id: 5
   location: Korhal
+  master_engeneer_id: 4
+  master_manager_id: 1
   name: Terrans
 company_id: 5
 contract_type: null

src/serialchemy/_tests/test_nested_fields/test_dump_with_nested_polymorphic.yml

@@ -0,0 +1,32 @@
+id: 5
+location: Korhal
+master_engeneer:
+  address_id: null
+  admission: '2000-01-01'
+  company_id: null
+  contract_type: null
+  created_at: '2000-01-02T00:00:00'
+  email: null
+  engineer_name: null
+  firstname: Doran
+  id: 4
+  lastname: Routhe
+  password: null
+  role: Specialist Engineer
+  specialization: Mechanical
+master_engeneer_id: 4
+master_manager:
+  address_id: 1
+  admission: '2000-01-01'
+  company_id: 5
+  contract_type: null
+  created_at: '2000-01-02T00:00:00'
+  email: null
+  firstname: Jim
+  id: 1
+  lastname: Raynor
+  manager_name: null
+  password: null
+  role: Manager
+master_manager_id: 1
+name: Terrans

src/serialchemy/_tests/test_nested_fields/test_load_with_nested_polymorphic_same_table_pk_names.yml

@@ -0,0 +1,19 @@
+id: 5
+location: null
+master_engeneer: null
+master_engeneer_id: null
+master_manager:
+  address_id: 1
+  admission: '2000-01-01'
+  company_id: 5
+  contract_type: null
+  created_at: '2000-01-02T00:00:00'
+  email: null
+  firstname: Jim
+  id: 1
+  lastname: Raynor
+  manager_name: null
+  password: null
+  role: Manager
+master_manager_id: null
+name: null

src/serialchemy/_tests/test_nested_fields/test_load_with_nested_polymorphic_with_different_table_pk_names.yml

@@ -0,0 +1,20 @@
+id: 5
+location: null
+master_engeneer:
+  address_id: null
+  admission: '2000-01-01'
+  company_id: null
+  contract_type: null
+  created_at: '2000-01-02T00:00:00'
+  email: null
+  engineer_name: null
+  firstname: Doran
+  id: 4
+  lastname: Routhe
+  password: null
+  role: Specialist Engineer
+  specialization: Mechanical
+master_engeneer_id: null
+master_manager: null
+master_manager_id: null
+name: null

src/serialchemy/_tests/test_serialization/test_one2many_pk_field.yml

@@ -3,4 +3,6 @@ employees:
 - 2
 id: 5
 location: Korhal
+master_engeneer_id: null
+master_manager_id: null
 name: Terrans

src/serialchemy/nested_fields.py

@@ -160,21 +160,12 @@ def get_model_pk_attr_name(model_class):
 
     :return: str: the attribute name for the column with primary key
     """
-    import sys
-
-    # TODO EDEN-2586: Investigate SqlAlchemy inspect failing on Python 3.6
-    if sys.version_info[:2] == (3, 6):
-        primary_key_columns = list(
-            filter(lambda attr_col: attr_col[1].primary_key, model_class.__mapper__.columns.items())
-        )
-        primary_key_names = [pk[0] for pk in primary_key_columns]
-    else:
-        from sqlalchemy.inspection import inspect
-
-        primary_key_names = [pk for pk in inspect(model_class)._primary_key_propkeys]
-
+    primary_key_columns = list(
+        filter(lambda attr_col: attr_col[1].primary_key, model_class.__mapper__.columns.items())
+    )
+    primary_key_names = set(column[0] for column in primary_key_columns)
     if len(primary_key_names) == 1:
-        return primary_key_names[0]
+        return primary_key_names.pop()
     elif len(primary_key_names) < 1:
         raise RuntimeError(f"Couldn't find attribute for {model_class}")
     else:

tox.ini

@@ -1,12 +1,22 @@
 [tox]
-envlist = py36, py37, py38, py39, py310, linting, docs
+envlist = py{36,37,38,39,310}-sqla{13,14}, linting, docs
 isolated_build = true
 
+[gh-actions]
+python =
+    3.6: py36
+    3.8: py38
+    3.9: py39
+    3.10: py310
+
 [testenv]
 extras = testing
 commands =
     pytest --cov={envsitepackagesdir}/serialchemy --pyargs serialchemy
     codecov
+deps =
+    sqla13: sqlalchemy>=1.3,<1.4
+    sqla14: sqlalchemy>=1.4,<2
 
 [testenv:linting]
 skip_install = True