don't allocate handle pids for external files

b2share/modules/deposit/api.py

@@ -439,8 +439,10 @@ def create_file_pids(record_metadata):
     from flask import current_app
     throw_on_failure = current_app.config.get(
         'CFG_FAIL_ON_MISSING_FILE_PID', False)
+    external_pids = record_metadata['_deposit'].get('external_pids', [])
+    external_keys = { x.get('key') for x in external_pids }
     for f in record_metadata.get('_files'):
-        if f.get('ePIC_PID'):
+        if f.get('ePIC_PID') or f.get('key') in external_keys:
             continue
         file_url = url_for('invenio_files_rest.object_api',
                            bucket_id=f.get('bucket'), key=f.get('key'),

b2share/modules/records/serializers/schemas/json.py

@@ -84,11 +84,12 @@ def filter_internal(self, data):
             if user_has_permission:
                 data['files'] = data['metadata']['_files']
                 if external_pids and bucket:
+                    external_dict = {x['key']: x['ePIC_PID']
+                                     for x in external_pids}
                     for _file in data['files']:
-                        if 'external_pids' in data['metadata'] and \
-                                any(d['key'] == _file['key']
-                                    for d in external_pids):
+                        if _file['key'] in external_dict:
                             _file['b2safe'] = True
+                            _file['ePIC_PID'] = external_dict[_file['key']]
             del data['metadata']['_files']
         if '_pid' in data['metadata']:
             # move PIDs to metadata top level

tests/b2share_unit_tests/records/test_records_api.py

@@ -202,3 +202,39 @@ def test_record_delete_version(app, test_records, test_users):
 
         # TODO: test in ES that the previous version is reindexed
         # and can be found
+
+
+def test_record_publish_adds_no_handles_for_external_files(app,
+                            records_data_with_external_pids,
+                            test_records_data):
+    """Test that no handle PIDs are created for external files."""
+    for metadata in test_records_data:
+        with app.app_context():
+            app.config.update({'FAKE_EPIC_PID': True})
+
+            external_pids = records_data_with_external_pids['external_pids']
+            external_dict = {x['key']: x['ePIC_PID'] for x in external_pids}
+            data = deepcopy(metadata)
+            data['external_pids'] = deepcopy(external_pids)
+
+            record_uuid = uuid.uuid4()
+            b2share_deposit_uuid_minter(record_uuid, data=data)
+
+            deposit = Deposit.create(data, id_=record_uuid)
+            ObjectVersion.create(deposit.files.bucket, 'real_file_1.txt',
+                             stream=BytesIO(b'mycontent'))
+            ObjectVersion.create(deposit.files.bucket, 'real_file_2.txt',
+                             stream=BytesIO(b'mycontent'))
+            deposit.submit()
+            deposit.publish()
+            deposit.commit()
+
+            _, record = deposit.fetch_published()
+
+            # external files don't get a handle PID, they already have one
+            # which is stored in record['_deposit']['external_pids']
+            for f in record.files:
+                if f['key'] in external_dict:
+                    assert f.get('ePIC_PID') is None
+                else:
+                    assert '0000' in f['ePIC_PID'] # is a new fake PID

tests/b2share_unit_tests/records/test_records_rest.py

@@ -520,3 +520,48 @@ def test_access(user=None, deleted=True):
         assert res['hits']['total'] == 0
         res = current_search_client.search(index='deposits')
         assert res['hits']['total'] == 0
+
+
+
+def test_record_publish_with_external_pids(app, login_user,
+                                           records_data_with_external_pids): #test_users, test_communities
+    """Test record external files and handle allocation."""
+    uploaded_files = {
+        'myfile1.dat': b'contents1',
+        'myfile2.dat': b'contents2'
+    }
+
+    with app.app_context():
+        app.config.update({'FAKE_EPIC_PID': True})
+
+        creator = create_user('creator')
+        external_pids = records_data_with_external_pids['external_pids']
+        record_data = generate_record_data(external_pids=external_pids)
+        _, record_pid, record = create_record(
+            record_data, creator, files=uploaded_files
+        )
+
+        with app.test_client() as client:
+            login_user(creator, client)
+            headers = [('Accept', 'application/json')]
+            request_res = client.get(
+                url_for('b2share_records_rest.b2rec_item',
+                        pid_value=record_pid.pid_value),
+                headers=headers)
+
+            assert request_res.status_code == 200
+
+            record = json.loads(request_res.get_data(as_text=True))
+            assert len(record['files']) == len(external_pids) + len(uploaded_files)
+
+            for f in record['files']:
+                assert f['ePIC_PID']
+                if f['key'] in uploaded_files:
+                    # uploaded (internal) file
+                    assert '0000' in f['ePIC_PID'] # freshly allocated fake pid
+                else:
+                    # external file
+                    assert f['b2safe']
+                    x_pid = [rec for rec in external_pids
+                             if rec['key'] == f['key']][0]
+                    assert f['ePIC_PID'] == x_pid['ePIC_PID']