api: test existence of pid's

* Adds functionality to test pids for iexistence.
* Test during acq_accounts creation and update existence of pids.
* Test during acq_order_lines creation and update existence of pids.
* Test during acq_orders creation and update existence of pids.
* Test during holdings creation and update existence of pids.
* Test during items creation and update existence of pids.
* Test during loans creation and update existence of pids.
* closes #850

Co-Authored-by: Peter Weber <[email protected]>

data/locations.json

@@ -143,10 +143,10 @@
     "code": "HOG-PUBL",
     "name": "Public Section",
     "is_pickup": true,
+    "pickup_name": "Public Section",
     "library": {
       "$ref": "https://ils.rero.ch/api/libraries/5"
-    },
-    "pickup_name": "Public Section"
+    }
   },
   {
     "pid": "18",
@@ -351,4 +351,4 @@
     "pickup_name": "Jedi Archives",
     "is_online": false
   }
-]
+]

rero_ils/modules/acq_accounts/api.py

@@ -56,6 +56,17 @@ class AcqAccount(IlsRecord):
     minter = acq_account_id_minter
     fetcher = acq_account_id_fetcher
     provider = AcqAccountProvider
+    pids_exist_test = {
+        'required': {
+            'lib': 'library',
+            'budg': 'budget'
+        },
+        'not_required': {
+            'org': 'organisation',
+            'patr': 'patron'
+        },
+        'raise_on_erro': True
+    }
 
     @classmethod
     def create(cls, data, id_=None, delete_pid=False,
@@ -66,28 +77,30 @@ def create(cls, data, id_=None, delete_pid=False,
             data, id_, delete_pid, dbcommit, reindex, **kwargs)
         return record
 
+    def update(self, data, dbcommit=False, reindex=False):
+        """Update acq account record."""
+        self._acq_account_build_org_ref(data)
+        super(AcqAccount, self).update(data, dbcommit, reindex)
+        return self
+
     @classmethod
     def _acq_account_build_org_ref(cls, data):
         """Build $ref for the organisation of the acq account."""
-        library_pid = data.get('library', {}).get('pid')
-        if not library_pid:
-            library_pid = data.get('library').get(
-                '$ref').split('libraries/')[1]
-            org_pid = Library.get_record_by_pid(library_pid).organisation_pid
-        base_url = current_app.config.get('RERO_ILS_APP_BASE_URL')
-        url_api = '{base_url}/api/{doc_type}/{pid}'
-        org_ref = {
-            '$ref': url_api.format(
-                base_url=base_url,
-                doc_type='organisations',
-                pid=org_pid or cls.organisation_pid)
-        }
+        library = data.get('library', {})
+        library_pid = library.get('pid') or \
+            library.get('$ref').split('libraries/')[1]
+        org_ref = {'$ref': '{base_url}/api/{doc_type}/{pid}'.format(
+            base_url=current_app.config.get('RERO_ILS_APP_BASE_URL'),
+            doc_type='organisations',
+            pid=Library.get_record_by_pid(library_pid).organisation_pid
+        )}
         data['organisation'] = org_ref
+        return data
 
     @property
     def library_pid(self):
         """Shortcut for acq account library pid."""
-        return self.replace_refs().get('library').get('pid')
+        return self.replace_refs()['library']['pid']
 
     def get_number_of_acq_order_lines(self):
         """Get number of acquisition order lines linked to this account."""

rero_ils/modules/acq_order_lines/api.py

@@ -17,6 +17,7 @@
 
 """API for manipulating Acquisition Order Line."""
 
+from copy import deepcopy
 from functools import partial
 
 from flask import current_app
@@ -81,6 +82,17 @@ class AcqOrderLine(IlsRecord):
     minter = acq_order_line_id_minter
     fetcher = acq_order_line_id_fetcher
     provider = AcqOrderLineProvider
+    pids_exist_test = {
+        'required': {
+            'doc': 'document',
+            'acac': 'acq_account'
+        },
+        'not_required': {
+            'org': 'organisation',
+            'acor': 'acq_order'
+        },
+        'raise_on_erro': True
+    }
 
     @classmethod
     def create(cls, data, id_=None, delete_pid=False,
@@ -94,29 +106,25 @@ def create(cls, data, id_=None, delete_pid=False,
 
     def update(self, data, dbcommit=True, reindex=True):
         """Update Acquisition Order Line record."""
-        self._build_total_amount_for_order_line(data)
-        super(AcqOrderLine, self).update(data, dbcommit, reindex)
+        new_data = deepcopy(dict(self))
+        new_data.update(data)
+        self._acq_order_line_build_org_ref(new_data)
+        self._build_total_amount_for_order_line(new_data)
+        super(AcqOrderLine, self).update(new_data, dbcommit, reindex)
         return self
 
     @classmethod
     def _acq_order_line_build_org_ref(cls, data):
         """Build $ref for the organisation of the acquisition order."""
         from ..acq_orders.api import AcqOrder
-
-        order_pid = data.get('acq_order', {}).get('pid')
-        if not order_pid:
-            order_pid = data.get('acq_order').get(
-                '$ref').split('acq_orders/')[1]
-
-        org_pid = AcqOrder.get_record_by_pid(order_pid).organisation_pid
-        base_url = current_app.config.get('RERO_ILS_APP_BASE_URL')
-        url_api = '{base_url}/api/{doc_type}/{pid}'
-        org_ref = {
-            '$ref': url_api.format(
-                base_url=base_url,
-                doc_type='organisations',
-                pid=org_pid or cls.organisation_pid)
-        }
+        order = data.get('acq_order', {})
+        order_pid = order.get('pid') or \
+            order.get('$ref').split('acq_orders/')[1]
+        org_ref = {'$ref': '{base_url}/api/{doc_type}/{pid}'.format(
+            base_url=current_app.config.get('RERO_ILS_APP_BASE_URL'),
+            doc_type='organisations',
+            pid=AcqOrder.get_record_by_pid(order_pid).organisation_pid
+        )}
         data['organisation'] = org_ref
 
     @classmethod
@@ -141,8 +149,7 @@ def organisation_pid(self):
     def library_pid(self):
         """Shortcut for acquisition order library pid."""
         from ..acq_orders.api import AcqOrder
-        order = AcqOrder.get_record_by_pid(self.order_pid)
-        return order.library_pid
+        return AcqOrder.get_record_by_pid(self.order_pid).library_pid
 
     def get_organisation(self):
         """Shortcut to the organisation of the acquisition order."""

rero_ils/modules/acq_orders/api.py

@@ -56,6 +56,16 @@ class AcqOrder(IlsRecord):
     minter = acq_order_id_minter
     fetcher = acq_order_id_fetcher
     provider = AcqOrderProvider
+    pids_exist_test = {
+        'required': {
+            'lib': 'library',
+            'vndr': 'vendor'
+        },
+        'not_required': {
+            'org': 'organisation'
+        },
+        'raise_on_erro': True
+    }
 
     @classmethod
     def create(cls, data, id_=None, delete_pid=False,
@@ -66,22 +76,23 @@ def create(cls, data, id_=None, delete_pid=False,
             data, id_, delete_pid, dbcommit, reindex, **kwargs)
         return record
 
+    def update(self, data, dbcommit=False, reindex=False):
+        """Update acq order record."""
+        self._acq_order_build_org_ref(data)
+        super(AcqOrder, self).update(data, dbcommit, reindex)
+        return self
+
     @classmethod
     def _acq_order_build_org_ref(cls, data):
         """Build $ref for the organisation of the acquisition order."""
-        library_pid = data.get('library', {}).get('pid')
-        if not library_pid:
-            library_pid = data.get('library').get(
-                '$ref').split('libraries/')[1]
-        org_pid = Library.get_record_by_pid(library_pid).organisation_pid
-        base_url = current_app.config.get('RERO_ILS_APP_BASE_URL')
-        url_api = '{base_url}/api/{doc_type}/{pid}'
-        org_ref = {
-            '$ref': url_api.format(
-                base_url=base_url,
-                doc_type='organisations',
-                pid=org_pid or cls.organisation_pid)
-        }
+        library = data.get('library', {})
+        library_pid = library.get('pid') or \
+            library.get('$ref').split('libraries/')[1]
+        org_ref = {'$ref': '{base_url}/api/{doc_type}/{pid}'.format(
+            base_url=current_app.config.get('RERO_ILS_APP_BASE_URL'),
+            doc_type='organisations',
+            pid=Library.get_record_by_pid(library_pid).organisation_pid
+        )}
         data['organisation'] = org_ref
 
     @property
@@ -92,7 +103,7 @@ def organisation_pid(self):
     @property
     def library_pid(self):
         """Shortcut for acquisition order library pid."""
-        return self.replace_refs().get('library').get('pid')
+        return self.replace_refs()['library']['pid']
 
     def get_number_of_acq_order_lines(self):
         """Get number of acquisition order lines."""

rero_ils/modules/api.py

@@ -53,6 +53,15 @@ class PidMissing(Exception):
     class PidChange(Exception):
         """IlsRecord pid change."""
 
+    class PidAlradyUsed(Exception):
+        """IlsRecord pid already used."""
+
+    class PidDoesNotExist(Exception):
+        """Pid does not exist."""
+
+    class DataMissing(Exception):
+        """Data missing in record."""
+
 
 class IlsRecordsSearch(RecordsSearch):
     """Search Class for ils."""
@@ -199,6 +208,7 @@ class IlsRecord(Record):
     provider = None
     object_type = 'rec'
     indexer = IlsRecordIndexer
+    pids_exist_test = None
 
     def validate(self, **kwargs):
         """Validate record against schema.
@@ -222,6 +232,7 @@ def create(cls, data, id_=None, delete_pid=False,
                dbcommit=False, reindex=False, **kwargs):
         """Create a new ils record."""
         assert cls.minter
+        assert cls.provider
         if '$schema' not in data:
             type = cls.provider.pid_type
             schemas = current_app.config.get('RECORDS_JSON_SCHEMA')
@@ -237,11 +248,32 @@ def create(cls, data, id_=None, delete_pid=False,
                 }
                 data['$schema'] = '{base_url}{schema_endpoint}{schema}'\
                     .format(**data_schema)
-        if delete_pid and data.get('pid'):
-            del data['pid']
+        if delete_pid:
+            if data.get('pid'):
+                del data['pid']
+        else:
+            if data.get('pid'):
+                test_rec = cls.get_record_by_pid(data.get('pid'))
+                if test_rec is not None:
+                    raise IlsRecordError.PidAlradyUsed(
+                        '{pid_type} {pid} {uuid}'.format(
+                            pid_type=cls.provider.pid_type,
+                            pid=test_rec.pid,
+                            uuid=test_rec.id
+                        )
+                    )
         if not id_:
             id_ = uuid4()
         cls.minter(id_, data)
+        if cls.pids_exist_test:
+            from .utils import pids_exists_in_data
+            pids_exists_in_data(
+                info=cls.provider.pid_type,
+                data=data,
+                required=cls.pids_exist_test.get('required', {}),
+                not_required=cls.pids_exist_test.get('not_required', {}),
+                raise_on_error=cls.pids_exist_test.get('raise_on_error', False)
+            )
         record = super(IlsRecord, cls).create(data=data, id_=id_, **kwargs)
         if dbcommit:
             record.dbcommit(reindex)
@@ -260,6 +292,7 @@ def get_record_by_pid(cls, pid, with_deleted=False):
                 persistent_identifier.object_uuid,
                 with_deleted=with_deleted
             )
+        # TODO: is it better to raise a error or to return None?
         except NoResultFound:
             return None
         except PIDDoesNotExistError:
@@ -335,12 +368,24 @@ def update(self, data, dbcommit=False, reindex=False):
             db_record = self.get_record_by_id(self.id)
             if pid != db_record.pid:
                 raise IlsRecordError.PidChange(
-                    'changed pid from {old_pid} to {new_pid}'.format(
-                        old_pid=self.pid,
+                    '{class_n} changed pid from {old_pid} to {new_pid}'.format(
+                        class_n=self.__class__.__name__,
+                        old_pid=db_record.pid,
                         new_pid=pid
                     )
                 )
-
+        if self.pids_exist_test:
+            from .utils import pids_exists_in_data
+            data_to_test = deepcopy(dict(self))
+            data_to_test.update(data)
+            pids_exists_in_data(
+                info=self.provider.pid_type,
+                data=data_to_test,
+                required=self.pids_exist_test.get('required', {}),
+                not_required=self.pids_exist_test.get('not_required', {}),
+                raise_on_error=self.pids_exist_test.get('raise_on_error',
+                                                        False)
+            )
         super(IlsRecord, self).update(data)
         if dbcommit:
             self = super(IlsRecord, self).commit()
@@ -430,4 +475,8 @@ def organisation_pid(self):
         """Get organisation pid for circulation policy."""
         if self.get('organisation'):
             return self.replace_refs()['organisation']['pid']
-        return None
+        # return None
+        raise IlsRecordError.PidDoesNotExist(
+            self.provider.pid_type,
+            'organisation_pid:organisation'
+        )

rero_ils/modules/budgets/api.py

@@ -54,6 +54,12 @@ class Budget(IlsRecord):
     minter = budget_id_minter
     fetcher = budget_id_fetcher
     provider = BudgetProvider
+    pids_exist_test = {
+        'required': {
+            'org': 'organisation'
+        },
+        'raise_on_erro': True
+    }
 
     def get_number_of_acq_accounts(self):
         """Get number of acq accounts."""

rero_ils/modules/circ_policies/api.py

@@ -58,6 +58,18 @@ class CircPolicy(IlsRecord):
     minter = circ_policy_id_minter
     fetcher = circ_policy_id_fetcher
     provider = CircPolicyProvider
+    pids_exist_test = {
+        'required': {
+            'org': 'organisation',
+            'budg': 'budget'
+        },
+        'not_required': {
+            'lib': 'library',
+            'ptty': 'patron_type',
+            'itty': 'item_type'
+        },
+        'raise_on_erro': True
+    }
 
     def extended_validation(self, **kwargs):
         """Validate record against schema.

rero_ils/modules/holdings/api.py

@@ -81,6 +81,14 @@ class Holding(IlsRecord):
     provider = HoldingProvider
     # model_cls = HoldingMetadata
     indexer = HoldingsIndexer
+    pids_exist_test = {
+        'required': {
+            'doc': 'document',
+            'loc': 'location',
+            'itty': 'item_type'
+        },
+        'raise_on_erro': True
+    }
 
     def delete_from_index(self):
         """Delete record from index."""
@@ -112,8 +120,7 @@ def library_pid(self):
     @property
     def organisation_pid(self):
         """Get organisation pid for holding."""
-        location = Location.get_record_by_pid(self.location_pid)
-        return location.organisation_pid
+        return Location.get_record_by_pid(self.location_pid).organisation_pid
 
     @property
     def available(self):