diff --git a/addons/base/exceptions.py b/addons/base/exceptions.py index 551a9ffc7ca..cc3a70cdf3a 100644 --- a/addons/base/exceptions.py +++ b/addons/base/exceptions.py @@ -13,3 +13,9 @@ class InvalidAuthError(AddonError): class HookError(AddonError): pass + +class QueryError(AddonError): + pass + +class DoesNotExist(AddonError): + pass diff --git a/addons/base/views.py b/addons/base/views.py index e229cb50d7a..d3afdea8de0 100644 --- a/addons/base/views.py +++ b/addons/base/views.py @@ -19,6 +19,7 @@ from api.caching.tasks import update_storage_usage_with_size +from addons.base import exceptions as addon_errors from addons.base.models import BaseStorageAddon from addons.osfstorage.models import OsfStorageFile from addons.osfstorage.models import OsfStorageFileNode @@ -726,7 +727,29 @@ def addon_view_or_download_file(auth, path, provider, **kwargs): }) savepoint_id = transaction.savepoint() - file_node = BaseFileNode.resolve_class(provider, BaseFileNode.FILE).get_or_create(target, path) + + try: + file_node = BaseFileNode.resolve_class( + provider, BaseFileNode.FILE + ).get_or_create( + target, path, **extras + ) + except addon_errors.QueryError as e: + raise HTTPError( + http_status.HTTP_400_BAD_REQUEST, + data={ + 'message_short': 'Bad Request', + 'message_long': str(e) + } + ) + except addon_errors.DoesNotExist as e: + raise HTTPError( + http_status.HTTP_404_NOT_FOUND, + data={ + 'message_short': 'Not Found', + 'message_long': str(e) + } + ) # Note: Cookie is provided for authentication to waterbutler # it is overridden to force authentication as the current user diff --git a/addons/dataverse/models.py b/addons/dataverse/models.py index 8b7447c1f25..f586a99a958 100644 --- a/addons/dataverse/models.py +++ b/addons/dataverse/models.py @@ -1,8 +1,10 @@ # -*- coding: utf-8 -*- from rest_framework import status as http_status +from addons.base import exceptions as addon_errors from addons.base.models import (BaseOAuthNodeSettings, BaseOAuthUserSettings, BaseStorageAddon) +from django.contrib.contenttypes.models import ContentType from django.db import models from framework.auth.decorators import Auth from framework.exceptions import HTTPError @@ -15,6 +17,36 @@ class DataverseFileNode(BaseFileNode): _provider = 'dataverse' + @classmethod + def get_or_create(cls, target, path, **query_params): + '''Override get_or_create for Dataverse. + + Dataverse is weird and reuses paths, so we need to extract a "version" + query param to determine which file to get. We also don't want to "create" + here, as that might lead to integrity errors. + ''' + version = query_params.get('version', None) + if version not in {'latest', 'latest-published'}: + raise addon_errors.QueryError( + 'Dataverse requires a "version" query paramater. ' + 'Acceptable options are "latest" or "latest-published"' + ) + + content_type = ContentType.objects.get_for_model(target) + try: + obj = cls.objects.get( + target_object_id=target.id, + target_content_type=content_type, + _path='/' + path.lstrip('/'), + _history__0__extra__datasetVersion=version, + ) + except cls.DoesNotExist: + raise addon_errors.DoesNotExist( + f'Requested Dataverse file does not exist with version "{version}"' + ) + + return obj + class DataverseFolder(DataverseFileNode, Folder): pass diff --git a/addons/osfstorage/models.py b/addons/osfstorage/models.py index 3d24b2237da..aa9d8932317 100644 --- a/addons/osfstorage/models.py +++ b/addons/osfstorage/models.py @@ -85,7 +85,7 @@ def get(cls, _id, target): return cls.objects.get(_id=_id, target_object_id=target.id, target_content_type=ContentType.objects.get_for_model(target)) @classmethod - def get_or_create(cls, target, path): + def get_or_create(cls, target, path, **unused_query_params): """Override get or create for osfstorage Path is always the _id of the osfstorage filenode. Use load here as its way faster than find. diff --git a/api/base/views.py b/api/base/views.py index 2cba81f78c2..eddb7e2a4a6 100644 --- a/api/base/views.py +++ b/api/base/views.py @@ -638,11 +638,23 @@ def bulk_get_file_nodes_from_wb_resp(self, files_list): ) # mirrors BaseFileNode get_or_create + _path = '/' + attrs['path'].lstrip('/') + query = { + 'target_object_id': node.id, + 'target_content_type': content_type, + '_path': _path, + } + + # Dataverse provides us two sets of files with the same path, so we disambiguate the paths, this + # preserves legacy behavior by distingishing them by version (Draft/Published). + if attrs['provider'] == 'dataverse': + query.update({'_history__0__extra__datasetVersion': attrs['extra']['datasetVersion']}) + try: - file_obj = base_class.objects.get(target_object_id=node.id, target_content_type=content_type, _path='/' + attrs['path'].lstrip('/')) + file_obj = base_class.objects.get(**query) except base_class.DoesNotExist: # create method on BaseFileNode appends provider, bulk_create bypasses this step so it is added here - file_obj = base_class(target=node, _path='/' + attrs['path'].lstrip('/'), provider=base_class._provider) + file_obj = base_class(target=node, _path=_path, provider=base_class._provider) objs_to_create[base_class].append(file_obj) else: file_objs.append(file_obj) diff --git a/api/files/annotations.py b/api/files/annotations.py index ffd88e4a3a2..53b38bcf069 100644 --- a/api/files/annotations.py +++ b/api/files/annotations.py @@ -1,8 +1,9 @@ -from django.db.models import Max, Case, When +from django.db.models import BooleanField, Case, Exists, F, IntegerField, Max, OuterRef, Q, Subquery, Value, When from django.db.models.functions.base import Cast from django.contrib.postgres.fields.jsonb import KeyTextTransform from osf.utils.datetime_aware_jsonfield import DateTimeAwareJSONField from osf.utils.fields import NonNaiveDateTimeField +from osf.models import FileVersion # Get date modified for OSF and non-OSF providers @@ -28,3 +29,58 @@ ), output_field=NonNaiveDateTimeField(), ), ) + +def make_show_as_unviewed_annotations(user): + '''Returns the annotations required to set the current_user_has_viewed attribute. + + Usage: + OsfStorageFile.objects.annotate(**make_show_as_unviewed_annotations(request.user)) + + show_as_unviewed is only true if the user has not seen the latest version of a file + but has looked at it previously. Making this determination requires multiple annotations, + which is why this returns a dictionary that must be unpacked into kwargs. + ''' + if user.is_anonymous: + return {'show_as_unviewed': Value(False, output_field=BooleanField())} + + seen_versions = FileVersion.objects.annotate( + latest_version=Subquery( + FileVersion.objects.filter( + basefilenode=OuterRef('basefilenode'), + ).order_by('-created').values('id')[:1], + output_field=IntegerField(), + ), + ).filter(seen_by=user) + + has_seen_latest = Exists( + seen_versions.filter(basefilenode=OuterRef('id')).filter(id=F('latest_version')), + ) + has_previously_seen = Exists( + seen_versions.filter(basefilenode=OuterRef('id')).exclude(id=F('latest_version')), + ) + show_as_unviewed = Case( + When(Q(has_seen_latest=False) & Q(has_previously_seen=True), then=Value(True)), + default=Value(False), + output_field=BooleanField(), + ) + + return { + 'has_seen_latest': has_seen_latest, + 'has_previously_seen': has_previously_seen, + 'show_as_unviewed': show_as_unviewed, + } + +def check_show_as_unviewed(user, osf_file): + '''A separate function for assigning the show_as_unviewed value to a single instance. + + Our logic is not conducive to assigning annotations to a single file, so do it manually + in the DetailView case. + ''' + if user.is_anonymous: + return False + + latest_version = osf_file.versions.order_by('-created').first() + return ( + osf_file.versions.filter(seen_by=user).exists() + and not latest_version.seen_by.filter(id=user.id).exists() + ) diff --git a/api/files/serializers.py b/api/files/serializers.py index 8cb9a46085f..c007a37d9f1 100644 --- a/api/files/serializers.py +++ b/api/files/serializers.py @@ -266,9 +266,12 @@ class BaseFileSerializer(JSONAPISerializer): def absolute_url(self, obj): if obj.is_file: - return furl.furl(settings.DOMAIN).set( + url = furl.furl(settings.DOMAIN).set( path=(obj.target._id, 'files', obj.provider, obj.path.lstrip('/')), - ).url + ) + if obj.provider == 'dataverse': + url.add(query_params={'version': obj.history[-1]['extra']['datasetVersion']}) + return url.url def get_download_link(self, obj): if obj.is_file: @@ -317,6 +320,10 @@ def get_extra(self, obj): } if obj.provider == 'osfstorage' and obj.is_file: extras['downloads'] = obj.get_download_count() + + if obj.provider == 'dataverse': + extras.update(obj.history[-1]['extra']) + return extras def get_current_user_can_comment(self, obj): @@ -383,14 +390,14 @@ class FileSerializer(BaseFileSerializer): min_version='2.0', max_version='2.7', ) target = TargetField(link_type='related', meta={'type': 'get_target_type'}) - current_user_has_viewed = ser.SerializerMethodField(help_text='Whether the current user has already viewed the file') - def get_current_user_has_viewed(self, obj): - version = obj.versions.order_by('created').last() - if version and not self.context['request'].user.is_anonymous: # This is to ensure compatibility with tests, ugh. - return version.seen_by.filter(id=self.context['request'].user.id).exists() - else: - return True + # Assigned via annotation. See api/files/annotations for info + show_as_unviewed = ser.BooleanField( + read_only=True, + required=False, + default=False, + help_text='Whether to mark the file as unviewed for the current user', + ) def get_target_type(self, obj): if isinstance(obj, Preprint): diff --git a/api/files/views.py b/api/files/views.py index 28c03bec5d0..6cfffb3c192 100644 --- a/api/files/views.py +++ b/api/files/views.py @@ -24,6 +24,7 @@ from api.base import permissions as base_permissions from api.nodes.permissions import ContributorOrPublic from api.nodes.permissions import ReadOnlyIfRegistration +from api.files import annotations from api.files.permissions import IsPreprintFile from api.files.permissions import CheckedOutOrAdmin from api.files.permissions import FileMetadataRecordPermission @@ -110,6 +111,9 @@ def get_object(self): # We normally would pass this through `get_file` as an annotation, but the `select_for_update` feature prevents # grouping versions in an annotation if file.kind == 'file': + file.show_as_unviewed = annotations.check_show_as_unviewed( + user=self.request.user, osf_file=file, + ) if file.provider == 'osfstorage': file.date_modified = file.versions.aggregate(Max('created'))['created__max'] else: diff --git a/api/nodes/views.py b/api/nodes/views.py index dceb0a6a0de..cf83529ca66 100644 --- a/api/nodes/views.py +++ b/api/nodes/views.py @@ -2,7 +2,7 @@ from distutils.version import StrictVersion from django.apps import apps -from django.db.models import Q, Subquery, F, Max +from django.db.models import F, Max, Q, Subquery from django.utils import timezone from django.contrib.contenttypes.models import ContentType from rest_framework import generics, permissions as drf_permissions @@ -65,7 +65,7 @@ ) from api.draft_registrations.serializers import DraftRegistrationSerializer, DraftRegistrationDetailSerializer from api.files.serializers import FileSerializer, OsfStorageFileSerializer -from api.files.annotations import DATE_MODIFIED +from api.files import annotations as file_annotations from api.identifiers.serializers import NodeIdentifierSerializer from api.identifiers.views import IdentifierList from api.institutions.serializers import InstitutionSerializer @@ -150,7 +150,7 @@ File, Folder, ) -from addons.osfstorage.models import Region, OsfStorageFileNode +from addons.osfstorage.models import Region from osf.utils.permissions import ADMIN, WRITE_NODE from website import mails, settings @@ -1178,21 +1178,19 @@ def get_queryset(self): # query param info when used on a folder gives that folder's metadata instead of the metadata of it's children if 'info' in self.request.query_params and path.endswith('/'): resource = self.get_resource() - file_obj = self.get_file_object(resource, path, provider) - - if provider == 'osfstorage': - queryset = OsfStorageFileNode.objects.filter(id=file_obj.id) - else: - base_class = BaseFileNode.resolve_class(provider, BaseFileNode.FOLDER) - queryset = base_class.objects.filter( - target_object_id=resource.id, - target_content_type=ContentType.objects.get_for_model(resource), - _path=path, - ) + base_class = BaseFileNode.resolve_class(provider, BaseFileNode.FOLDER) + queryset = base_class.objects.filter( + target_object_id=resource.id, + target_content_type=ContentType.objects.get_for_model(resource), + _path=path, + ) else: queryset = self.get_queryset_from_request() - return queryset.annotate(date_modified=DATE_MODIFIED) + return queryset.annotate( + date_modified=file_annotations.DATE_MODIFIED, + **file_annotations.make_show_as_unviewed_annotations(self.request.user) + ) class NodeFileDetail(JSONAPIBaseView, generics.RetrieveAPIView, WaterButlerMixin, NodeMixin): @@ -1223,6 +1221,9 @@ def get_object(self): # We should not have gotten a folder here raise NotFound if fobj.kind == 'file': + fobj.show_as_unviewed = file_annotations.check_show_as_unviewed( + user=self.request.user, osf_file=fobj, + ) if fobj.provider == 'osfstorage': fobj.date_modified = fobj.versions.aggregate(Max('created'))['created__max'] else: diff --git a/api_tests/files/views/test_file_detail.py b/api_tests/files/views/test_file_detail.py index ce067065944..93b93163330 100644 --- a/api_tests/files/views/test_file_detail.py +++ b/api_tests/files/views/test_file_detail.py @@ -10,10 +10,11 @@ from addons.github.models import GithubFileNode from addons.osfstorage import settings as osfstorage_settings from addons.osfstorage.listeners import checkin_files_task +from addons.osfstorage.tests.factories import FileVersionFactory from api.base.settings.defaults import API_BASE from api_tests import utils as api_utils from framework.auth.core import Auth -from osf.models import NodeLog, Session, QuickFilesNode, Node +from osf.models import NodeLog, Session, QuickFilesNode, Node, FileVersionUserMetadata from osf.utils.permissions import WRITE, READ from osf.utils.workflows import DefaultStates from osf_tests.factories import ( @@ -918,3 +919,62 @@ def test_withdrawn_preprint_files(self, app, file_url, preprint, user, other_use # Admin contrib res = app.get(file_url, auth=user.auth, expect_errors=True) assert res.status_code == 403 + +@pytest.mark.django_db +class TestShowAsUnviewed: + + @pytest.fixture + def node(self, user): + return ProjectFactory(creator=user, is_public=True) + + @pytest.fixture + def test_file(self, user, node): + test_file = api_utils.create_test_file(node, user, create_guid=False) + test_file.add_version(FileVersionFactory()) + return test_file + + @pytest.fixture + def url(self, test_file): + return f'/{API_BASE}files/{test_file._id}/' + + def test_show_as_unviewed__previously_seen(self, app, user, test_file, url): + FileVersionUserMetadata.objects.create( + user=user, + file_version=test_file.versions.order_by('created').first() + ) + + res = app.get(url, auth=user.auth) + assert res.json['data']['attributes']['show_as_unviewed'] + + FileVersionUserMetadata.objects.create( + user=user, + file_version=test_file.versions.order_by('-created').first() + ) + + res = app.get(url, auth=user.auth) + assert not res.json['data']['attributes']['show_as_unviewed'] + + def test_show_as_unviewed__not_previously_seen(self, app, user, test_file, url): + res = app.get(url, auth=user.auth) + assert not res.json['data']['attributes']['show_as_unviewed'] + + def test_show_as_unviewed__different_user(self, app, user, test_file, url): + FileVersionUserMetadata.objects.create( + user=user, + file_version=test_file.versions.order_by('created').first() + ) + file_viewer = AuthUserFactory() + + res = app.get(url, auth=file_viewer.auth) + assert not res.json['data']['attributes']['show_as_unviewed'] + + def test_show_as_unviewed__anonymous_user(self, app, test_file, url): + res = app.get(url) + assert not res.json['data']['attributes']['show_as_unviewed'] + + def test_show_as_unviewed__no_versions(self, app, user, test_file, url): + # Most Non-OSFStorage providers don't have versions; make sure this still works + test_file.versions.all().delete() + + res = app.get(url, auth=user.auth) + assert not res.json['data']['attributes']['show_as_unviewed'] diff --git a/api_tests/files/views/test_file_list.py b/api_tests/files/views/test_file_list.py index fb219dbfd94..6c59a36a218 100644 --- a/api_tests/files/views/test_file_list.py +++ b/api_tests/files/views/test_file_list.py @@ -1,4 +1,5 @@ import pytest +import responses from api.base.settings.defaults import API_BASE from api_tests import utils as api_utils @@ -7,6 +8,9 @@ ProjectFactory, AuthUserFactory, ) +from addons.dataverse.tests.factories import DataverseAccountFactory +from api_tests.draft_nodes.views.test_draft_node_files_lists import prepare_mock_wb_response +from addons.dataverse.models import DataverseFile @pytest.fixture() @@ -17,6 +21,25 @@ def user(): @pytest.mark.django_db class TestNodeFileList: + @pytest.fixture() + def dataverse(self, user, node): + addon = node.add_addon('dataverse', auth=Auth(user)) + oauth_settings = DataverseAccountFactory() + oauth_settings.save() + user.add_addon('dataverse') + user.external_accounts.add(oauth_settings) + user.save() + addon.user_settings = user.get_addon('dataverse') + addon.external_account = oauth_settings + addon.dataset_doi = 'test dataset_doi' + addon.dataset = 'test dataset' + addon._dataset_id = 'test dataset_id' + addon.save() + addon.user_settings.oauth_grants[node._id] = { + oauth_settings._id: []} + addon.user_settings.save() + node.save() + @pytest.fixture() def node(self, user): return ProjectFactory(creator=user) @@ -26,6 +49,22 @@ def file(self, user, node): return api_utils.create_test_file( node, user, filename='file_one') + @pytest.fixture() + def dataverse_published_filenode(self, node): + return DataverseFile.objects.create( + target=node, + path='/testpath', + _history=[{'extra': {'datasetVersion': 'latest-published'}}], + ) + + @pytest.fixture() + def dataverse_draft_filenode(self, node): + return DataverseFile.objects.create( + target=node, + path='/testpath', + _history=[{'extra': {'datasetVersion': 'latest'}}], + ) + @pytest.fixture() def deleted_file(self, user, node): deleted_file = api_utils.create_test_file( @@ -42,6 +81,119 @@ def test_does_not_return_trashed_files( data = res.json.get('data') assert len(data) == 1 + @responses.activate + def test_disambiguate_dataverse_paths_initial(self, app, user, node, dataverse): + ''' + This test is for retrieving files from Dataverse initially, (Osf is contacting Dataverse after a update to their + Dataverse files) this test ensures both files are made into OSF filenodes and their `extra` info is passed along + to the front-end. + ''' + prepare_mock_wb_response( + path='/', + node=node, + provider='dataverse', + files=[ + { + 'name': 'testpath', + 'path': '/testpath', + 'materialized': '/testpath', + 'kind': 'file', + 'modified': 'Wed, 20 Jul 2011 22:04:50 +0000', + 'extra': { + 'datasetVersion': 'latest' + }, + 'provider': 'dataverse' + }, + { + 'name': 'testpath', + 'path': '/testpath', + 'materialized': '/testpath', + 'kind': 'file', + 'modified': 'Wed, 20 Jul 2011 22:04:50 +0000', + 'extra': { + 'datasetVersion': 'latest-published' + }, + 'provider': 'dataverse' + }, + ] + ) + res = app.get( + f'/{API_BASE}nodes/{node._id}/files/dataverse/?sort=date_modified', + auth=node.creator.auth + ) + data = res.json['data'] + assert len(data) == 2 + assert data[0]['attributes']['extra'] == { + 'datasetVersion': 'latest', + 'hashes': { + 'md5': None, + 'sha256': None + } + } + assert data[1]['attributes']['extra'] == { + 'datasetVersion': 'latest-published', + 'hashes': { + 'md5': None, + 'sha256': None + } + } + + @responses.activate + def test_disambiguate_dataverse_paths_retrieve(self, app, user, node, dataverse, dataverse_draft_filenode, dataverse_published_filenode): + ''' + This test is for retrieving files from Dataverse and disambiguating their corresponding OSF filenodes and + ensures their `extra` info is passed along to the front-end. Waterbulter must also be mocked here, otherwise OSF + will assume the files are gone. + ''' + prepare_mock_wb_response( + path='/', + node=node, + provider='dataverse', + files=[ + { + 'name': 'testpath', + 'path': '/testpath', + 'materialized': '/testpath', + 'kind': 'file', + + 'extra': { + 'datasetVersion': 'latest', + }, + 'provider': 'dataverse', + }, + { + 'name': 'testpath', + 'path': '/testpath', + 'materialized': '/testpath', + 'kind': 'file', + 'extra': { + 'datasetVersion': 'latest-published', + }, + 'provider': 'dataverse', + }, + ] + ) + res = app.get( + f'/{API_BASE}nodes/{node._id}/files/dataverse/?sort=date_modified', + auth=node.creator.auth + ) + data = res.json['data'] + assert len(data) == 2 + assert data[0]['attributes']['extra'] == { + 'datasetVersion': 'latest', + 'hashes': { + 'md5': None, + 'sha256': None + } + } + assert data[1]['attributes']['extra'] == { + 'datasetVersion': 'latest-published', + 'hashes': { + 'md5': None, + 'sha256': None + } + } + @pytest.mark.django_db class TestFileFiltering: diff --git a/api_tests/nodes/views/test_node_files_list.py b/api_tests/nodes/views/test_node_files_list.py index d9d4662c512..352787045b8 100644 --- a/api_tests/nodes/views/test_node_files_list.py +++ b/api_tests/nodes/views/test_node_files_list.py @@ -10,10 +10,12 @@ from addons.github.models import GithubFolder from addons.github.tests.factories import GitHubAccountFactory +from addons.osfstorage.tests.factories import FileVersionFactory from api.base.settings.defaults import API_BASE from api.base.utils import waterbutler_api_url_for from api_tests import utils as api_utils from tests.base import ApiTestCase +from osf.models.files import FileVersionUserMetadata from osf_tests.factories import ( ProjectFactory, AuthUserFactory, @@ -731,3 +733,56 @@ def test_can_view_if_public(self): def test_cannot_view_if_private(self): res = self.app.get(self.private_url, expect_errors=True) assert_equal(res.status_code, 401) + + +class TestShowAsUnviewed(ApiTestCase): + + def setUp(self): + super().setUp() + self.user = AuthUserFactory() + self.node = ProjectFactory(is_public=True, creator=self.user) + self.test_file = api_utils.create_test_file(self.node, self.user, create_guid=False) + self.test_file.add_version(FileVersionFactory()) + self.url = f'/{API_BASE}nodes/{self.node._id}/files/osfstorage/' + + def test_show_as_unviewed__previously_seen(self): + FileVersionUserMetadata.objects.create( + user=self.user, + file_version=self.test_file.versions.order_by('created').first() + ) + + res = self.app.get(self.url, auth=self.user.auth) + assert res.json['data'][0]['attributes']['show_as_unviewed'] + + FileVersionUserMetadata.objects.create( + user=self.user, + file_version=self.test_file.versions.order_by('-created').first() + ) + + res = self.app.get(self.url, auth=self.user.auth) + assert not res.json['data'][0]['attributes']['show_as_unviewed'] + + def test_show_as_unviewed__not_previously_seen(self): + res = self.app.get(self.url, auth=self.user.auth) + assert not res.json['data'][0]['attributes']['show_as_unviewed'] + + def test_show_as_unviewed__different_user(self): + FileVersionUserMetadata.objects.create( + user=self.user, + file_version=self.test_file.versions.order_by('created').first() + ) + file_viewer = AuthUserFactory() + + res = self.app.get(self.url, auth=file_viewer.auth) + assert not res.json['data'][0]['attributes']['show_as_unviewed'] + + def test_show_as_unviewed__anonymous_user(self): + res = self.app.get(self.url) + assert not res.json['data'][0]['attributes']['show_as_unviewed'] + + def test_show_as_unviewed__no_versions(self): + # Most Non-OSFStorage providers don't have versions; make sure this still works + self.test_file.versions.all().delete() + + res = self.app.get(self.url, auth=self.user.auth) + assert not res.json['data'][0]['attributes']['show_as_unviewed'] diff --git a/osf/apps.py b/osf/apps.py index ff34451084e..f32ed14ef46 100644 --- a/osf/apps.py +++ b/osf/apps.py @@ -1,7 +1,11 @@ import logging from django.apps import AppConfig as BaseAppConfig from django.db.models.signals import post_migrate -from osf.migrations import update_permission_groups, update_waffle_flags +from osf.migrations import ( + update_permission_groups, + update_waffle_flags, + create_cache_table +) logger = logging.getLogger(__file__) @@ -21,3 +25,7 @@ def ready(self): update_waffle_flags, dispatch_uid='osf.apps.update_waffle_flags' ) + post_migrate.connect( + create_cache_table, + dispatch_uid='osf.apps.create_cache_table' + ) diff --git a/osf/migrations/0137_transfer_preprint_service_permissions.py b/osf/migrations/0137_transfer_preprint_service_permissions.py index 300d88f42fd..bdcca0eb2c9 100644 --- a/osf/migrations/0137_transfer_preprint_service_permissions.py +++ b/osf/migrations/0137_transfer_preprint_service_permissions.py @@ -6,7 +6,6 @@ def unmigrate_preprint_service_permissions(state, schema): - Permission = state.get_model('auth', 'permission') # New permission groups @@ -19,8 +18,6 @@ def migrate_preprint_service_permissions(state, schema): """ Django permissions on the preprint model have new names. """ - # this is to make sure that the permissions created earlier exist! - Permission = state.get_model('auth', 'permission') Permission.objects.filter(codename='add_preprint').delete() diff --git a/osf/migrations/0156_create_cache_table.py b/osf/migrations/0156_create_cache_table.py index 8a4246592ff..e3cb3126d11 100644 --- a/osf/migrations/0156_create_cache_table.py +++ b/osf/migrations/0156_create_cache_table.py @@ -1,7 +1,6 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals from django.db import migrations -from django.conf import settings class Migration(migrations.Migration): @@ -9,15 +8,4 @@ class Migration(migrations.Migration): ('osf', '0155_merge_20190115_1437'), ] operations = [ - migrations.RunSQL([ - """ - CREATE TABLE "{}" ( - "cache_key" varchar(255) NOT NULL PRIMARY KEY, - "value" text NOT NULL, - "expires" timestamp with time zone NOT NULL - ); - """.format(settings.CACHES[settings.STORAGE_USAGE_CACHE_NAME]['LOCATION']) - ], [ - """DROP TABLE "{}"; """.format(settings.CACHES[settings.STORAGE_USAGE_CACHE_NAME]['LOCATION']) - ]) ] diff --git a/osf/migrations/__init__.py b/osf/migrations/__init__.py index e25d43954c7..9e3bf4a3f1e 100644 --- a/osf/migrations/__init__.py +++ b/osf/migrations/__init__.py @@ -3,6 +3,9 @@ import logging from django.db.utils import ProgrammingError from osf.management.commands.manage_switch_flags import manage_waffle +from django.core.management import call_command +from api.base import settings + logger = logging.getLogger(__file__) @@ -128,3 +131,8 @@ def update_waffle_flags(sender, verbosity=0, **kwargs): if 'pytest' not in sys.modules: manage_waffle() logger.info('Waffle flags have been synced') + + +def create_cache_table(sender, verbosity=0, **kwargs): + if getattr(sender, 'label', None) == 'osf': + call_command('createcachetable', tablename=settings.CACHES[settings.STORAGE_USAGE_CACHE_NAME]['LOCATION']) diff --git a/osf/models/files.py b/osf/models/files.py index 41d7f2a8185..59b8be9b818 100644 --- a/osf/models/files.py +++ b/osf/models/files.py @@ -191,10 +191,14 @@ def create(cls, **kwargs): return cls(**kwargs) @classmethod - def get_or_create(cls, target, path): + def get_or_create(cls, target, path, **unused_query_params): content_type = ContentType.objects.get_for_model(target) try: - obj = cls.objects.get(target_object_id=target.id, target_content_type=content_type, _path='/' + path.lstrip('/')) + obj = cls.objects.get( + target_object_id=target.id, + target_content_type=content_type, + _path='/' + path.lstrip('/'), + ) except cls.DoesNotExist: obj = cls(target_object_id=target.id, target_content_type=content_type, _path='/' + path.lstrip('/')) return obj diff --git a/tests/test_addons.py b/tests/test_addons.py index 8f482fe0368..3c427a0f516 100644 --- a/tests/test_addons.py +++ b/tests/test_addons.py @@ -224,62 +224,6 @@ def test_action_download_mfr_views_non_contrib(self): assert_equal(test_file.get_view_count(), 1) assert_equal(node.logs.count(), nlogs) # don't log views - def test_current_user_has_viewed_public(self): - node = ProjectFactory(is_public=True) - file = create_test_file(node, node.creator, create_guid=False) - django_app = JSONAPITestApp() - - file_viewer = AuthUserFactory() - - res = django_app.get(f'/{API_BASE}files/{file._id}/', auth=file_viewer.auth) - assert res.status_code == 200 - assert res.json['data']['attributes']['current_user_has_viewed'] is False - - # This mocks the Waterbutler callback endpoint (`get_auth` function in addons/base/views.py ) which indicates if - # a file has been view with the MFR. - self.app.get( - self.build_url( - nid=node._id, - provider='osfstorage', - path=file.path, - version=1 - ), - auth=file_viewer.auth - ) - - res = django_app.get(f'/{API_BASE}files/{file._id}/', auth=file_viewer.auth) - - version = file.versions.get() - assert version.seen_by.exists() - assert res.json['data']['attributes']['current_user_has_viewed'] - - def test_current_user_has_viewed_private(self): - node = ProjectFactory() - file = create_test_file(node, node.creator, create_guid=False) - django_app = JSONAPITestApp() - - res = django_app.get(f'/{API_BASE}files/{file._id}/', auth=node.creator.auth) - assert res.status_code == 200 - assert res.json['data']['attributes']['current_user_has_viewed'] is False - - # This mocks the Waterbutler callback endpoint (`get_auth` function in addons/base/views.py ) which indicates if - # a file has been view with the MFR. - self.app.get( - self.build_url( - nid=node._id, - provider='osfstorage', - path=file.path, - version=1 - ), - auth=node.creator.auth - ) - - res = django_app.get(f'/{API_BASE}files/{file._id}/', auth=node.creator.auth) - - version = file.versions.get() - assert version.seen_by.exists() - assert res.json['data']['attributes']['current_user_has_viewed'] - class TestAddonLogs(OsfTestCase):