diff --git a/addons/base/views.py b/addons/base/views.py index 53186ea110a5..d56e209156b2 100644 --- a/addons/base/views.py +++ b/addons/base/views.py @@ -302,40 +302,25 @@ def get_authenticated_resource(resource_id): return resource -def get_file_version_from_wb(waterbutler_data: dict) -> FileVersion: - path = waterbutler_data.get('path') - if waterbutler_data['provider'] != 'osfstorage' or not path: - return - - path = waterbutler_data.get('path') - file_id = path.strip('/') - version = waterbutler_data.get('version') - if version: - try: - return FileVersion.objects.filter( - basefilenode___id=file_id, - identifier=int(version) - ).select_related('region').get() - except FileVersion.DoesNotExist: - raise HTTPError(http_status.HTTP_400_BAD_REQUEST) - else: - try: - return BaseFileNode.active.get( - _id=file_id, - ).versions.order_by( - '-created' - ).first() - except BaseFileNode.DoesNotExist: - raise HTTPError(http_status.HTTP_400_BAD_REQUEST) - +def _get_osfstorage_file_version(file_node: OsfStorageFileNode, version_string: str = None) -> FileVersion: + if not (file_node and file_node.is_file): + return None -def get_file_node_from_wb(waterbutler_data: dict) -> FileVersion: - path = waterbutler_data.get('path') - file_id = path.strip('/') try: - return BaseFileNode.active.get(_id=file_id) - except BaseFileNode.DoesNotExist: - raise HTTPError(http_status.HTTP_400_BAD_REQUEST) + return FileVersion.objects.select_related('region').get( + basefilenode=file_node, + identifier=version_string or str(file_node.versions.count()) + ) + except FileVersion.DoesNotExist: + raise HTTPError(http_status.HTTP_400_BAD_REQUEST, 'Requested File Version unavailable') + + +def _get_osfstorage_file_node(file_path: str) -> OsfStorageFileNode: + if not file_path: + return None + + file_id = file_path.strip('/') + return OsfStorageFileNode.load(file_id) def authenticate_user_if_needed(auth, waterbutler_data, resource): @@ -406,7 +391,11 @@ def get_auth(auth, **kwargs): provider = None # Get the file version from Waterbutler data, which is used for file-specific actions - fileversion = get_file_version_from_wb(waterbutler_data) + file_node = None + fileversion = None + if waterbutler_data['provider'] == 'osfstorage': + file_node = _get_osfstorage_file_node(waterbutler_data.get('path')) + fileversion = _get_osfstorage_file_version(file_node, waterbutler_data.get('version')) # Fetch Waterbutler credentials and settings for the resource credentials, waterbutler_settings = get_waterbutler_data( @@ -419,10 +408,8 @@ def get_auth(auth, **kwargs): if fileversion: # Trigger any file-specific signals based on the action taken (e.g., file viewed, downloaded) if action == 'render': - file_node = get_file_node_from_wb(waterbutler_data) file_signals.file_viewed.send(auth=auth, fileversion=fileversion, file_node=file_node) elif action == 'download': - file_node = get_file_node_from_wb(waterbutler_data) file_signals.file_downloaded.send(auth=auth, fileversion=fileversion, file_node=file_node) # Construct the response payload including the JWT diff --git a/api/base/views.py b/api/base/views.py index 1e2ae0e63a46..d3272db575e0 100644 --- a/api/base/views.py +++ b/api/base/views.py @@ -1,3 +1,4 @@ +import waffle from builtins import str from collections import defaultdict @@ -40,6 +41,7 @@ from api.nodes.permissions import ExcludeWithdrawals from api.users.serializers import UserSerializer from framework.auth.oauth_scopes import CoreScopes +from osf import features from osf.models import Contributor, MaintenanceState, BaseFileNode from osf.utils.permissions import API_CONTRIBUTOR_PERMISSIONS, READ, WRITE, ADMIN from waffle.models import Flag, Switch, Sample @@ -675,12 +677,16 @@ def bulk_get_file_nodes_from_wb_resp(self, files_list): # stuff list into QuerySet return BaseFileNode.objects.filter(id__in=[item.id for item in file_objs]) - def get_file_node_from_wb_resp(self, item, auth): + def get_file_node_from_wb_resp(self, item): """Takes file data from wb response, touches/updates metadata for it, and returns file object""" attrs = item['attributes'] - provider_name = GravyValetAddonAppConfig(self, attrs['provider'], self.request).legacy_config.name + if waffle.flag_is_active(self.request, features.ENABLE_GV): + provider_determinent = GravyValetAddonAppConfig(self, attrs['provider'], self.request).legacy_config.name + else: + provider_determinent = attrs['provider'] + file_node = BaseFileNode.resolve_class( - provider_name, + provider_determinent, BaseFileNode.FOLDER if attrs['kind'] == 'folder' else BaseFileNode.FILE, ).get_or_create(self.get_node(check_object_permissions=False), attrs['path']) diff --git a/api_tests/providers/test_files_with_gv.py b/api_tests/providers/test_files_with_gv.py index 9f8b7212195c..72f0a989e639 100644 --- a/api_tests/providers/test_files_with_gv.py +++ b/api_tests/providers/test_files_with_gv.py @@ -53,7 +53,29 @@ def addon_files_url(self, node, provider_gv_id): } ) - def test_must_have_auth(self, app, addon_files_url): + @responses.activate + def test_must_have_auth(self, app, file, node, addon_files_url): + from api_tests.draft_nodes.views.test_draft_node_files_lists import prepare_mock_wb_response + + prepare_mock_wb_response( + path=file.path + '/', + node=node, + provider='1', + files=[ + { + 'name': file.name, + 'path': file.path, + 'materialized': file.materialized_path, + 'kind': 'file', + 'modified': file.modified.isoformat(), + 'extra': { + 'extra': 'readAllAboutIt' + }, + 'provider': '1' + }, + ] + ) + res = app.get(addon_files_url, expect_errors=True) assert res.status_code == 401 @@ -66,6 +88,7 @@ def test_must_be_contributor(self, app, addon_files_url): assert res.status_code == 403 def test_get_file_provider(self, app, user, addon_files_url, file, provider_gv_id): + res = app.get( addon_files_url, auth=user.auth @@ -141,8 +164,9 @@ def test_must_be_contributor(self, app, file_url): @responses.activate def test_get_file_provider(self, app, user, file_url, file, provider_gv_id, node): from api_tests.draft_nodes.views.test_draft_node_files_lists import prepare_mock_wb_response + prepare_mock_wb_response( - path=file.path, + path=file.path + '/', node=node, provider='1', files=[