Repository: inveniosoftware/invenio-records (language: python)

Issue:
systemfields: related object fields issues 
Currently, we have the following type of system fields:

ModelField: 
- Purpose: Provides access to a model-level property (e.g. ``record.created`` -> ``record.model.created``) - i.e. created is stored on the same database table as the record json.
- Dumps/loads: ES dumper automatically dumps/loads the values and recreatest the SQLAlchemy model object.

PIDField:
- Purpose: Provides access to a related PersistentIdentifier object (an SQLAlchemy model object stored in a **different** database table), and stores it in the JSON. Ensures efficient handling of related objects
- Dumps/loads: No special handling, because the PersistentIdentifier object is denormalised into the record's JSON.

RelationsField:
- Purpose: Provides access to related Record objects. Stores a list of IDs in the record.
- Dumps/loads: Deserialize certain fields from the Related object into the dump, so it can be recreated.

# Properties/differences

- Type of related objects: A Record (via PIDs/PKs), An SQLAlchemy model (via primary key)
- Place the relation is stored: JSON, model field
- Denormalization: yes/no - Do we denormalize the related object into the record.
- Dumping/loading: Do we dump/load the entire object, only some fields.
- Caching: Do we cache the related object 

### New case

Currently, in Invenio-Vocabularies, a VocabularyRecord related to a VocabularyType via foreign key relationship at the database level. It doesn't currently fit in any of the above categories.

Like PIDField we provide access to a related SQLAlchemy object (not a record), but we don't denormalize the VocabularyType object into the JSON instead the vocabulary type id is stored on the database table.

Like ReleationsField we dump/load the vocabulary type.

So the relationship is somewhat sitting between RelationsFIeld and PIDField.




- VocabularyType
- PIDField

Both the ``RelationsField`` and the ``PIDField`` (invenio-records-resources).




## Describe the solution you'd like
A clear and concise description of what you want to happen.

## Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

## Additional context
Add any other context or screenshots about the feature request here.



Discussion / hints:

Fix patch:
diff --git a/invenio_records/systemfields/__init__.py b/invenio_records/systemfields/__init__.py
index 7ea12bf0..3a623512 100644
--- a/invenio_records/systemfields/__init__.py
+++ b/invenio_records/systemfields/__init__.py
@@ -74,6 +74,7 @@ class MyRecord(Record, SystemFieldsMixin):
 from .constant import ConstantField
 from .dict import DictField
 from .model import ModelField
+from .relatedmodelfield import RelatedModelField, RelatedModelFieldContext
 from .relations import PKRelation, RelationsField
 
 __all__ = (
@@ -81,6 +82,8 @@ class MyRecord(Record, SystemFieldsMixin):
     'DictField',
     'ModelField',
     'PKRelation',
+    'RelatedModelField',
+    'RelatedModelFieldContext',
     'RelationsField',
     'SystemField',
     'SystemFieldContext',
diff --git a/invenio_records/systemfields/relatedmodelfield.py b/invenio_records/systemfields/relatedmodelfield.py
new file mode 100644
index 00000000..4c7ccd46
--- /dev/null
+++ b/invenio_records/systemfields/relatedmodelfield.py
@@ -0,0 +1,139 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2020-2021 CERN.
+# Copyright (C) 2020 Northwestern University.
+#
+# Invenio-Records is free software; you can redistribute it and/or
+# modify it under the terms of the MIT License; see LICENSE file for more
+# details.
+
+"""Related model field.
+
+The related model field serializes/dumps a related object into the record JSON
+dictionary (basically denormalization). This way, the object can be recreated
+directly from the record JSON dictionary instead of querying the database.
+
+This is useful for instance during search queries to avoid hitting the
+database. In addition you can subclass this field to provide life-cycle hooks
+on the record. For instance a PIDField, could create the related persistent
+identifier.
+"""
+
+from invenio_db import db
+from sqlalchemy import inspect
+
+from .base import SystemField, SystemFieldContext
+
+
+class RelatedModelFieldContext(SystemFieldContext):
+    """Context for RelatedModelField.
+
+    This class implements the class-level methods available on a
+    RelatedModelField. I.e. when you access the field through the class, for
+    instance:
+
+    .. code-block:: python
+
+        Record.myattr.session_merge(record)
+    """
+
+    def session_merge(self, record):
+        """Merge the PID to the session if not persistent."""
+        obj = self.field.obj(record)
+        if not inspect(obj).persistent:
+            obj = db.session.merge(obj)
+            self.field._set_cache(record, obj)
+
+
+class RelatedModelField(SystemField):
+    """Related model system field."""
+
+    def __init__(self, model, key=None, required=False, load=None, dump=None,
+                 context_cls=None):
+        """Initialize the field.
+
+        :param model: Related SQLAlchemy model.
+        :param key: Name of key in the record to serialize the related object
+            under.
+        :param required: Flag to determine if a related object is required on
+            record commit time.
+        :param load: Callable to load the related object from a JSON object.
+        :param dump: Callable to dump the related object as a JSON object.
+        :param context_cls: The context class is used to provide additional
+            methods on the field itself.
+        """
+        self._model = model
+        self._required = required
+        self._load = load or model.load_obj
+        self._dump = dump or model.dump_obj
+        self._context_cls = context_cls or RelatedModelFieldContext
+        super().__init__(key=key)
+
+    #
+    # Life-cycle hooks
+    #
+    def pre_commit(self, record):
+        """Called before a record is committed."""
+        # Make sure we serialize/dump the related objet on record.commit() time
+        # as it might have changed.
+        related_obj = getattr(record, self.attr_name)
+        if related_obj is not None:
+            self.set_obj(record, related_obj)
+        elif self._required:
+            raise RuntimeError("You must provide a related object.")
+
+    #
+    # Helpers
+    #
+    def obj(self, record):
+        """Get the related object.
+
+        Uses a cached object if it exists.
+
+        IMPORTANT: By default, if the object is loaded from the record JSON
+        object instead of from the database mdoel, it is NOT added to the
+        database session. Thus, the related object will be in a transient state
+        instead of persistent state. This is useful for instance in search
+        queries to avoid hitting the database, however if you need to make
+        operations on it you should add it to the session using:
+
+        .. code-block::
+
+            Record.myattr.session_merge(record)
+        """
+        # Check cache
+        obj = self._get_cache(record)
+        if obj:
+            return obj
+
+        obj = self._load(self, record)
+        if obj:
+            # Cache object
+            self._set_cache(record, obj)
+            return obj
+        return None
+
+    def set_obj(self, record, obj):
+        """Set the object."""
+        assert isinstance(obj, self._model)
+
+        # Store data values on the attribute name (e.g. 'type') using dump
+        # method provided by either to the field or a function on the related
+        # object.
+        self._dump(self, record, obj)
+
+        # Cache object
+        self._set_cache(record, obj)
+
+    #
+    # Data descriptor methods (i.e. attribute access)
+    #
+    def __get__(self, record, owner=None):
+        """Get the persistent identifier."""
+        if record is None:
+            return self._context_cls(self, owner)
+        return self.obj(record)
+
+    def __set__(self, record, pid):
+        """Set persistent identifier on record."""
+        self.set_obj(record, pid)
diff --git a/invenio_records/version.py b/invenio_records/version.py
index 275d01de..b24dba3d 100644
--- a/invenio_records/version.py
+++ b/invenio_records/version.py
@@ -13,4 +13,4 @@
 """
 
 
-__version__ = '1.5.0a1'
+__version__ = '1.5.0a2'
diff --git a/setup.py b/setup.py
index f280b543..183d244f 100644
--- a/setup.py
+++ b/setup.py
@@ -17,8 +17,7 @@
 
 
 tests_require = [
-    'mock>=3.0.5',
-    'pytest-invenio>=1.4.0',
+    'pytest-invenio>=1.4.1',
 ]
 
 extras_require = {
@@ -53,7 +52,7 @@
 install_requires = [
     'arrow>=0.16.0',
     'invenio-base>=1.2.3',
-    'invenio-celery>=1.2.1',
+    'invenio-celery>=1.2.2',
     'invenio-i18n>=1.2.0',
     'jsonpatch>=1.26',
     'jsonref>=0.2',


Test patch:
diff --git a/run-tests.sh b/run-tests.sh
index d154b4b8..fc6a24a6 100755
--- a/run-tests.sh
+++ b/run-tests.sh
@@ -24,7 +24,7 @@ trap cleanup EXIT
 
 python -m check_manifest --ignore ".*-requirements.txt"
 python -m sphinx.cmd.build -qnNW docs docs/_build/html
-eval "$(docker-services-cli up --db ${DB:-} --env)"
+eval "$(docker-services-cli up --db ${DB:-postgresql} --env)"
 python -m pytest
 tests_exit_code=$?
 exit "$tests_exit_code"
diff --git a/tests/conftest.py b/tests/conftest.py
index d1a09d12..a5504916 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -18,6 +18,8 @@
 
 from invenio_records import InvenioRecords
 
+pytest_plugins = ("celery.contrib.pytest", )
+
 
 @compiles(DropTable, 'postgresql')
 def _compile_drop_table(element, compiler, **kwargs):
diff --git a/tests/test_admin.py b/tests/test_admin.py
index dadc1a7c..1bb314a0 100644
--- a/tests/test_admin.py
+++ b/tests/test_admin.py
@@ -10,10 +10,10 @@
 
 
 import uuid
+from unittest.mock import patch
 
 from flask import url_for
 from flask_admin import Admin, menu
-from mock import patch
 from sqlalchemy.exc import SQLAlchemyError
 
 from invenio_records.admin import record_adminview
diff --git a/tests/test_systemfields_relatedmodelfield.py b/tests/test_systemfields_relatedmodelfield.py
new file mode 100644
index 00000000..e8349f4c
--- /dev/null
+++ b/tests/test_systemfields_relatedmodelfield.py
@@ -0,0 +1,196 @@
+# -*- coding: utf-8 -*-
+#
+# This file is part of Invenio.
+# Copyright (C) 2021 CERN.
+#
+# Invenio is free software; you can redistribute it and/or modify it
+# under the terms of the MIT License; see LICENSE file for more details.
+
+"""Test for related model system fields."""
+
+from collections import namedtuple
+from copy import deepcopy
+
+import pytest
+
+from invenio_records.api import Record
+from invenio_records.systemfields import RelatedModelField, \
+    RelatedModelFieldContext, SystemFieldsMixin, relatedmodelfield
+
+
+#
+# Fixtures
+#
+@pytest.fixture()
+def patch_inspect(monkeypatch):
+    """Monkey patch sqlalchemy.inspect"""
+    # Used to avoid having to create DB tables
+    MockResult = namedtuple('MockResult', ['persistent'])
+
+    def mock_inspect(obj):
+        return MockResult(persistent=getattr(obj, 'persistent', False))
+
+    monkeypatch.setattr(
+        relatedmodelfield,
+        'inspect',
+        mock_inspect)
+
+    return mock_inspect
+
+
+@pytest.fixture()
+def patch_db(monkeypatch):
+    """Monkey patch session_merge"""
+    # Used to avoid having to create DB tables
+    class MockSession:
+        @staticmethod
+        def merge(obj):
+            obj = deepcopy(obj)
+            obj.persistent = True
+            return obj
+
+    class MockDb:
+        session = MockSession
+
+    monkeypatch.setattr(
+        relatedmodelfield,
+        'db',
+        MockDb
+    )
+
+
+@pytest.fixture()
+def MyModel():
+    """Create a model class."""
+    class MyModel:
+        def __init__(self, val):
+            self._val = val
+
+        @classmethod
+        def dump_obj(cls, field, record, obj):
+            field.set_dictkey(record, {'akey': obj._val})
+
+        @classmethod
+        def load_obj(cls, field, record):
+            data = field.get_dictkey(record)
+            if data:
+                return cls(data['akey'])
+            return None
+
+        def __eq__(self, o):
+            return self._val == o._val
+
+    return MyModel
+
+
+@pytest.fixture()
+def MyRecord(MyModel):
+    """A record with a related model system field."""
+    class MyRecord(Record, SystemFieldsMixin):
+        myobj = RelatedModelField(MyModel)
+
+    return MyRecord
+
+
+#
+# Tests
+#
+def test_field(MyRecord, MyModel):
+    """Test class behaviour."""
+    assert isinstance(MyRecord.myobj, RelatedModelFieldContext)
+
+
+def test_setting_related_obj(testapp, db, MyRecord, MyModel):
+    """Set the related object"""
+    # via attr access
+    record = MyRecord({})
+    record.myobj = MyModel('a')
+    assert record['myobj'] == {'akey': 'a'}
+    record.myobj = MyModel('b')
+    assert record['myobj'] == {'akey': 'b'}
+
+    # via __init__() with a myobj argument
+    record = MyRecord({}, myobj=MyModel('c'))
+    assert record['myobj'] == {'akey': 'c'}
+
+    # via create() with a myobj argument
+    record = MyRecord.create({}, myobj=MyModel('c'))
+    assert record['myobj'] == {'akey': 'c'}
+
+
+def test_change_key(testapp, db, MyModel):
+    """Change key where the record is stored."""
+    class Record1(Record, SystemFieldsMixin):
+        myobj = RelatedModelField(MyModel, key='custom')
+
+    assert Record1({}, myobj=MyModel('a'))['custom'] == {'akey': 'a'}
+
+
+def test_custom_dump_load(testapp, db, MyModel):
+    """Change custom dump/load function."""
+    # A custom dumper function
+    def my_dump(field, record, obj):
+        field.set_dictkey(record, {'custom': obj._val})
+
+    # A custom loader function
+    def my_load(field, record):
+        data = field.get_dictkey(record)
+        if data:
+            return MyModel(data['custom'])
+        return None
+
+    class Record1(Record, SystemFieldsMixin):
+        myobj = RelatedModelField(MyModel, load=my_load, dump=my_dump)
+
+    # Dumping
+    record = Record1({}, myobj=MyModel('a'))
+    assert record['myobj'] == {'custom': 'a'}
+
+    # Loading
+    record = Record1(dict(record))
+    assert record.myobj == MyModel('a')
+
+
+def test_serialize_on_commit(testapp, db, MyRecord, MyModel):
+    """Test serialize on commit."""
+    obj = MyModel('a')
+    record = MyRecord.create({}, myobj=obj)
+    assert record['myobj'] == {'akey': 'a'}
+
+    # Change related object outside the APIs
+    obj._val = 'b'
+    assert record['myobj'] == {'akey': 'a'}
+
+    # Now commit and check that latest value are serialized.
+    record.commit()
+    assert record['myobj'] == {'akey': 'b'}
+
+
+def test_required(testapp, db, MyModel, MyRecord):
+    """Test required property."""
+    class Record1(Record, SystemFieldsMixin):
+        myobj = RelatedModelField(MyModel, required=True)
+    # Test that an error is raised when required
+    pytest.raises(RuntimeError, Record1.create({}).commit)
+    # Test that no error is raised (MyRecord does not require the obj)
+    assert MyRecord.create({}).commit() == {}
+
+
+def test_field_context(MyModel):
+    """Test the field context."""
+    class MyFieldCtx(RelatedModelFieldContext):
+        def test(self):
+            return True
+
+    class Record1(Record, SystemFieldsMixin):
+        myobj = RelatedModelField(MyModel, context_cls=MyFieldCtx)
+
+    assert Record1.myobj.test()
+
+
+def test_ctx_session_merge(MyRecord, MyModel, patch_inspect, patch_db):
+    """Test the session_merge."""
+    record = MyRecord(dict(MyRecord({}, myobj=MyModel('a'))))
+    assert patch_inspect(record.myobj).persistent is False
+    MyRecord.myobj.session_merge(record)
+    assert patch_inspect(record.myobj).persistent is True