tidy share tests

admin_tests/nodes/test_views.py

@@ -22,6 +22,8 @@
     RemoveStuckRegistrationsView
 )
 from admin_tests.utilities import setup_log_view, setup_view
+from api.share.utils import shtrove_ingest_url
+from api_tests.share._utils import assert_ingest_request
 from website import settings
 from nose import tools as nt
 from django.utils import timezone
@@ -268,13 +270,9 @@ def test_reindex_node_share(self):
         with mock.patch('api.share.utils.settings.SHARE_ENABLED', True):
             with mock.patch('api.share.utils.settings.SHARE_API_TOKEN', 'mock-api-token'):
                 with responses.RequestsMock(assert_all_requests_are_fired=True) as rsps:
-                    rsps.add(responses.POST, 'https://share.osf.io/api/v2/normalizeddata/')
+                    rsps.add(responses.POST, shtrove_ingest_url())
                     view.post(self.request)
-                    data = json.loads(rsps.calls[-1].request.body.decode())
-
-                    share_graph = data['data']['attributes']['data']['@graph']
-                    identifier_node = next(n for n in share_graph if n['@type'] == 'workidentifier')
-                    assert identifier_node['creative_work']['@type'] == 'project'
+                    assert_ingest_request(rsps.calls[-1].request, self.node._id, token='mock-api-token')
                     nt.assert_equal(AdminLogEntry.objects.count(), count + 1)
 
     def test_reindex_registration_share(self):

admin_tests/preprints/test_views.py

@@ -1,7 +1,5 @@
 import pytest
 import mock
-import json
-import responses
 
 from django.test import RequestFactory
 from django.urls import reverse
@@ -24,6 +22,7 @@
 from osf.models.spam import SpamStatus
 from osf.utils.workflows import DefaultStates, RequestTypes
 
+from api_tests.share._utils import expect_ingest_request
 from admin_tests.utilities import setup_view, setup_log_view
 
 from admin.preprints import views
@@ -334,22 +333,17 @@ def test_change_preprint_provider_subjects_change_permissions(self, plain_view,
 @pytest.mark.enable_implicit_clean
 class TestPreprintReindex:
 
-    def test_reindex_preprint_share(self, preprint, req):
+    def test_reindex_preprint_share(self, preprint, req, mock_share_requests):
         preprint.provider.access_token = 'totally real access token I bought from a guy wearing a trenchcoat in the summer'
         preprint.provider.save()
 
         count = AdminLogEntry.objects.count()
         view = views.PreprintReindexShare()
         view = setup_log_view(view, req, guid=preprint._id)
 
-        with mock.patch('api.share.utils.settings.SHARE_ENABLED', True):
-            with mock.patch('api.share.utils.settings.SHARE_API_TOKEN', 'mock-api-token'):
-                with responses.RequestsMock(assert_all_requests_are_fired=True) as rsps:
-                    rsps.add(responses.POST, 'https://share.osf.io/api/v2/normalizeddata/')
-                    view.post(req)
-                    data = json.loads(rsps.calls[0].request.body.decode())
-                    assert data['data']['type'] == 'NormalizedData'
-                    assert AdminLogEntry.objects.count() == count + 1
+        with expect_ingest_request(mock_share_requests, preprint._id, token=preprint.provider.access_token):
+            view.post(req)
+        assert AdminLogEntry.objects.count() == count + 1
 
     @mock.patch('website.search.search.update_preprint')
     def test_reindex_preprint_elastic(self, mock_update_search, preprint, req):

admin_tests/registration_providers/test_views.py

@@ -144,9 +144,9 @@ def view(self, req, provider):
         view = views.ShareSourceRegistrationProvider()
         return setup_view(view, req, registration_provider_id=provider.id)
 
-    def test_share_source(self, mock_share, view, provider, req):
-        mock_share.reset()
-        mock_share.add(
+    def test_share_source(self, mock_share_responses, view, provider, req):
+        mock_share_responses.reset()
+        mock_share_responses.add(
             responses.POST,
             f'{settings.SHARE_URL}api/v2/sources/',
             json.dumps(
@@ -172,9 +172,9 @@ def test_share_source(self, mock_share, view, provider, req):
         assert provider.access_token == 'test access token'
 
     @mock.patch.object(settings, 'SHARE_PROVIDER_PREPEND', 'testenv')
-    def test_share_source_prefix(self, mock_share, view, provider, req):
-        mock_share.reset()
-        mock_share.add(
+    def test_share_source_prefix(self, mock_share_responses, view, provider, req):
+        mock_share_responses.reset()
+        mock_share_responses.add(
             responses.POST,
             f'{settings.SHARE_URL}api/v2/sources/',
             json.dumps(

api/share/utils.py

@@ -49,21 +49,11 @@ def update_share(resource):
     if not _osfguid_value:
         logger.warning(f'update_share skipping resource that has no guids: {resource}')
         return
-    enqueue_task(task__update_share.s(_osfguid_value))
+    _enqueue_update_share(_osfguid_value)
 
 
-def do_update_share(osfguid: str):
-    logger.debug('%s.do_update_share("%s")', __name__, osfguid)
-    _guid_instance = apps.get_model('osf.Guid').load(osfguid)
-    if _guid_instance is None:
-        raise ValueError(f'unknown osfguid "{osfguid}"')
-    _resource = _guid_instance.referent
-    _response = (
-        pls_delete_trove_indexcard(_resource)
-        if _should_delete_indexcard(_resource)
-        else pls_send_trove_indexcard(_resource)
-    )
-    return _response
+def _enqueue_update_share(osfguid_value: str):
+    enqueue_task(task__update_share.s(osfguid_value))
 
 
 @celery_app.task(bind=True, max_retries=4, acks_late=True)
@@ -74,7 +64,7 @@ def task__update_share(self, guid: str):
     :param guid:
     :return:
     """
-    resp = do_update_share(guid)
+    resp = _do_update_share(guid)
     try:
         resp.raise_for_status()
     except Exception as e:
@@ -123,6 +113,20 @@ def pls_delete_trove_indexcard(osf_item):
     )
 
 
+def _do_update_share(osfguid: str):
+    logger.warning('%s._do_update_share("%s")', __name__, osfguid)
+    _guid_instance = apps.get_model('osf.Guid').load(osfguid)
+    if _guid_instance is None:
+        raise ValueError(f'unknown osfguid "{osfguid}"')
+    _resource = _guid_instance.referent
+    _response = (
+        pls_delete_trove_indexcard(_resource)
+        if _should_delete_indexcard(_resource)
+        else pls_send_trove_indexcard(_resource)
+    )
+    return _response
+
+
 def _shtrove_record_identifier(osf_item):
     return osf_item.guids.values_list('_id', flat=True).first()
 

api_tests/share/_utils.py

@@ -7,11 +7,11 @@
 
 
 @contextlib.contextmanager
-def expect_ingest_request(mock_share, osfguid, *, token=None, delete=False, count=1):
-    mock_share._calls.reset()
+def expect_ingest_request(mock_share_responses, osfguid, *, token=None, delete=False, count=1):
+    mock_share_responses._calls.reset()
     yield
-    assert len(mock_share.calls) == count
-    for _call in mock_share.calls:
+    assert len(mock_share_responses.calls) == count
+    for _call in mock_share_responses.calls:
         assert_ingest_request(_call.request, osfguid, token=token, delete=delete)
 
 

api_tests/share/test_share_node.py

@@ -98,15 +98,15 @@ def registration_outcome(self, registration):
         )
         return o
 
-    def test_update_node_share(self, mock_share, node, user):
-        with expect_ingest_request(mock_share, node._id):
+    def test_update_node_share(self, mock_share_responses, node, user):
+        with expect_ingest_request(mock_share_responses, node._id):
             on_node_updated(node._id, user._id, False, {'is_public'})
 
-    def test_update_registration_share(self, mock_share, registration, user):
-        with expect_ingest_request(mock_share, registration._id):
+    def test_update_registration_share(self, mock_share_responses, registration, user):
+        with expect_ingest_request(mock_share_responses, registration._id):
             on_node_updated(registration._id, user._id, False, {'is_public'})
 
-    def test_update_share_correctly_for_projects(self, mock_share, node, user):
+    def test_update_share_correctly_for_projects(self, mock_share_responses, node, user):
         cases = [{
             'is_deleted': False,
             'attrs': {'is_public': True, 'is_deleted': False, 'spam_status': SpamStatus.HAM}
@@ -121,14 +121,14 @@ def test_update_share_correctly_for_projects(self, mock_share, node, user):
             'attrs': {'is_public': True, 'is_deleted': False, 'spam_status': SpamStatus.SPAM}
         }]
 
-        mock_share._calls.reset()  # reset after factory calls
+        mock_share_responses._calls.reset()  # reset after factory calls
         for i, case in enumerate(cases):
             for attr, value in case['attrs'].items():
                 setattr(node, attr, value)
-            with expect_ingest_request(mock_share, node._id, delete=case['is_deleted']):
+            with expect_ingest_request(mock_share_responses, node._id, delete=case['is_deleted']):
                 node.save()
 
-    def test_update_share_correctly_for_registrations(self, mock_share, registration, user):
+    def test_update_share_correctly_for_registrations(self, mock_share_responses, registration, user):
         cases = [{
             'is_deleted': True,
             'attrs': {'is_public': False, 'is_deleted': False}
@@ -140,42 +140,42 @@ def test_update_share_correctly_for_registrations(self, mock_share, registration
             'attrs': {'is_public': True, 'is_deleted': False}
         }]
 
-        mock_share._calls.reset()  # reset after factory calls
+        mock_share_responses._calls.reset()  # reset after factory calls
         for i, case in enumerate(cases):
             for attr, value in case['attrs'].items():
                 setattr(registration, attr, value)
-            with expect_ingest_request(mock_share, registration._id, delete=case['is_deleted']):
+            with expect_ingest_request(mock_share_responses, registration._id, delete=case['is_deleted']):
                 registration.save()
             assert registration.is_registration
 
-    def test_update_share_correctly_for_projects_with_qa_tags(self, mock_share, node, user):
-        with expect_ingest_request(mock_share, node._id, delete=True):
+    def test_update_share_correctly_for_projects_with_qa_tags(self, mock_share_responses, node, user):
+        with expect_ingest_request(mock_share_responses, node._id, delete=True):
             node.add_tag(settings.DO_NOT_INDEX_LIST['tags'][0], auth=Auth(user))
-        with expect_ingest_request(mock_share, node._id, delete=False):
+        with expect_ingest_request(mock_share_responses, node._id, delete=False):
             node.remove_tag(settings.DO_NOT_INDEX_LIST['tags'][0], auth=Auth(user), save=True)
 
-    def test_update_share_correctly_for_registrations_with_qa_tags(self, mock_share, registration, user):
-        with expect_ingest_request(mock_share, registration._id, delete=True):
+    def test_update_share_correctly_for_registrations_with_qa_tags(self, mock_share_responses, registration, user):
+        with expect_ingest_request(mock_share_responses, registration._id, delete=True):
             registration.add_tag(settings.DO_NOT_INDEX_LIST['tags'][0], auth=Auth(user))
-        with expect_ingest_request(mock_share, registration._id):
+        with expect_ingest_request(mock_share_responses, registration._id):
             registration.remove_tag(settings.DO_NOT_INDEX_LIST['tags'][0], auth=Auth(user), save=True)
 
-    def test_update_share_correctly_for_projects_with_qa_titles(self, mock_share, node, user):
+    def test_update_share_correctly_for_projects_with_qa_titles(self, mock_share_responses, node, user):
         node.title = settings.DO_NOT_INDEX_LIST['titles'][0] + ' arbitary text for test title.'
         node.save()
-        with expect_ingest_request(mock_share, node._id, delete=True):
+        with expect_ingest_request(mock_share_responses, node._id, delete=True):
             on_node_updated(node._id, user._id, False, {'is_public'})
         node.title = 'Not a qa title'
-        with expect_ingest_request(mock_share, node._id):
+        with expect_ingest_request(mock_share_responses, node._id):
             node.save()
         assert node.title not in settings.DO_NOT_INDEX_LIST['titles']
 
-    def test_update_share_correctly_for_registrations_with_qa_titles(self, mock_share, registration, user):
+    def test_update_share_correctly_for_registrations_with_qa_titles(self, mock_share_responses, registration, user):
         registration.title = settings.DO_NOT_INDEX_LIST['titles'][0] + ' arbitary text for test title.'
-        with expect_ingest_request(mock_share, registration._id, delete=True):
+        with expect_ingest_request(mock_share_responses, registration._id, delete=True):
             registration.save()
         registration.title = 'Not a qa title'
-        with expect_ingest_request(mock_share, registration._id):
+        with expect_ingest_request(mock_share_responses, registration._id):
             registration.save()
         assert registration.title not in settings.DO_NOT_INDEX_LIST['titles']
 
@@ -184,21 +184,21 @@ def test_skips_no_settings(self, node, user):
         on_node_updated(node._id, user._id, False, {'is_public'})
         assert len(responses.calls) == 0
 
-    def test_call_async_update_on_500_retry(self, mock_share, node, user):
+    def test_call_async_update_on_500_retry(self, mock_share_responses, node, user):
         """This is meant to simulate a temporary outage, so the retry mechanism should kick in and complete it."""
-        mock_share.replace(responses.POST, shtrove_ingest_url(), status=500)
-        mock_share.add(responses.POST, shtrove_ingest_url(), status=200)
-        with expect_ingest_request(mock_share, node._id, count=2):
+        mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=500)
+        mock_share_responses.add(responses.POST, shtrove_ingest_url(), status=200)
+        with expect_ingest_request(mock_share_responses, node._id, count=2):
             on_node_updated(node._id, user._id, False, {'is_public'})
 
-    def test_call_async_update_on_500_failure(self, mock_share, node, user):
+    def test_call_async_update_on_500_failure(self, mock_share_responses, node, user):
         """This is meant to simulate a total outage, so the retry mechanism should try X number of times and quit."""
-        mock_share.assert_all_requests_are_fired = False  # allows it to retry indefinitely
-        mock_share.replace(responses.POST, shtrove_ingest_url(), status=500)
-        with expect_ingest_request(mock_share, node._id, count=5):  # tries five times
+        mock_share_responses.assert_all_requests_are_fired = False  # allows it to retry indefinitely
+        mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=500)
+        with expect_ingest_request(mock_share_responses, node._id, count=5):  # tries five times
             on_node_updated(node._id, user._id, False, {'is_public'})
 
-    def test_no_call_async_update_on_400_failure(self, mock_share, node, user):
-        mock_share.replace(responses.POST, shtrove_ingest_url(), status=400)
-        with expect_ingest_request(mock_share, node._id):
+    def test_no_call_async_update_on_400_failure(self, mock_share_responses, node, user):
+        mock_share_responses.replace(responses.POST, shtrove_ingest_url(), status=400)
+        with expect_ingest_request(mock_share_responses, node._id):
             on_node_updated(node._id, user._id, False, {'is_public'})