diff --git a/tests/core/test_utils.py b/tests/core/test_utils.py index 827774efe1..13595d46d5 100644 --- a/tests/core/test_utils.py +++ b/tests/core/test_utils.py @@ -4,6 +4,10 @@ import pytest from waterbutler.core import utils +from waterbutler.core.path import WaterButlerPath +from waterbutler.core.streams import EmptyStream + +from tests.utils import MockCoroutine, MockFileMetadata, MockProvider class TestAsyncRetry: @@ -159,3 +163,43 @@ def test_content_disposition(self, filename, expected): def test_disposition_encoding(self, filename, expected): encoded = utils.encode_for_disposition(filename) assert encoded == expected + + +class TestZipStreamGenerator: + + @pytest.mark.asyncio + async def test_passes_child_metadata_to_download(self): + provider = MockProvider() + root_path = WaterButlerPath('/folder/', folder=True) + file_meta = MockFileMetadata() + provider.metadata = MockCoroutine(return_value=[file_meta]) + provider.download = MockCoroutine(return_value=EmptyStream()) + + generator = utils.ZipStreamGenerator(provider, root_path, minimal=True) + zip_name, _ = await generator.__anext__() + + assert zip_name == 'Foo.name' + provider.download.assert_called_once() + assert provider.download.call_args.kwargs['metadata'] is file_meta + + +class TestZipStreamGeneratorPaginated: + + @pytest.mark.asyncio + async def test_passes_child_metadata_to_download(self): + provider = MockProvider() + root_path = WaterButlerPath('/folder/', folder=True) + file_meta = MockFileMetadata() + provider.download = MockCoroutine(return_value=EmptyStream()) + + async def iter_children_pages(path, **kwargs): + yield [file_meta] + + provider.iter_children_pages = iter_children_pages + + generator = utils.ZipStreamGeneratorPaginated(provider, root_path, minimal=True) + zip_name, _ = await generator.__anext__() + + assert zip_name == 'Foo.name' + provider.download.assert_called_once() + assert provider.download.call_args.kwargs['metadata'] is file_meta diff --git a/tests/providers/osfstorage/fixtures.py b/tests/providers/osfstorage/fixtures.py index ca9e76b1b0..eb82270525 100644 --- a/tests/providers/osfstorage/fixtures.py +++ b/tests/providers/osfstorage/fixtures.py @@ -95,6 +95,11 @@ def folder_children_metadata(): with open(os.path.join(os.path.dirname(__file__), 'fixtures/root_provider.json')) as fp: return json.load(fp)['folder_children_metadata'] +@pytest.fixture +def folder_children_metadata_minimal(): + with open(os.path.join(os.path.dirname(__file__), 'fixtures/root_provider.json')) as fp: + return json.load(fp)['folder_children_metadata_minimal'] + @pytest.fixture def download_response(): diff --git a/tests/providers/osfstorage/fixtures/root_provider.json b/tests/providers/osfstorage/fixtures/root_provider.json index 1907f27b4c..7c62c0875a 100644 --- a/tests/providers/osfstorage/fixtures/root_provider.json +++ b/tests/providers/osfstorage/fixtures/root_provider.json @@ -66,6 +66,32 @@ "latestVersionSeen": null } ], + "folder_children_metadata_minimal": [ + { + "id": "59c0054cb7d1c90114c456af", + "kind": "folder", + "name": "New Folder", + "path": "/59c0054cb7d1c90114c456af/" + }, + { + "id": "59a9b637b7d1c903ab5a8f58", + "kind": "file", + "name": "one", + "path": "/59a9b637b7d1c903ab5a8f58" + }, + { + "id": "59a9b628b7d1c903ab5a8f52", + "kind": "file", + "name": "doc.rst", + "path": "/59a9b628b7d1c903ab5a8f52" + }, + { + "id": "59a5adb9b7d1c901cd40f0e9", + "kind": "file", + "name": "video.mp4", + "path": "/59a5adb9b7d1c901cd40f0e9" + } + ], "file_metadata": { "checkout": null, "contentType": null, diff --git a/tests/providers/osfstorage/test_provider.py b/tests/providers/osfstorage/test_provider.py index c6b3f50dff..657e505def 100644 --- a/tests/providers/osfstorage/test_provider.py +++ b/tests/providers/osfstorage/test_provider.py @@ -24,7 +24,7 @@ file_lineage, file_metadata, file_metadata_object, file_path, folder_lineage, folder_metadata, - folder_children_metadata, folder_path, + folder_children_metadata, folder_children_metadata_minimal, folder_path, revisions_metadata, revision_metadata_object, download_response, download_path, upload_response, upload_path, root_path, @@ -70,13 +70,13 @@ class TestDownload: @pytest.mark.asyncio @pytest.mark.aiohttpretty - async def test_download_with_auth(self, provider_and_mock_one, download_response, - download_path, mock_time): + async def test_download(self, provider_and_mock_one, download_response, + download_path, mock_time): provider, inner_provider = provider_and_mock_one - uri, params = build_signed_url_with_auth(provider, 'GET', download_path.identifier, - 'download', version=None, mode=None) + uri, params = build_signed_url_without_auth(provider, 'GET', download_path.identifier, + 'download', version=None, mode=None) aiohttpretty.register_json_uri('GET', uri, body=download_response, params=params) @@ -92,29 +92,6 @@ async def test_download_with_auth(self, provider_and_mock_one, download_response inner_provider.download.assert_called_once_with(path=expected_path, display_name=expected_display_name) - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_download_without_auth(self, provider_and_mock_one, download_response, - download_path, mock_time): - provider, inner_provider = provider_and_mock_one - - provider.auth = {} - url, params = build_signed_url_without_auth(provider, 'GET', download_path.identifier, - 'download', version=None, mode=None) - aiohttpretty.register_json_uri('GET', url, params=params, body=download_response) - - await provider.download(download_path) - - assert provider.make_provider.called - assert inner_provider.download.called - assert aiohttpretty.has_call(method='GET', uri=url, params=params) - provider.make_provider.assert_called_once_with(download_response['settings']) - - expected_path = WaterButlerPath('/' + download_response['data']['path']) - expected_display_name = download_response['data']['name'] - inner_provider.download.assert_called_once_with(path=expected_path, - display_name=expected_display_name) - @pytest.mark.asyncio @pytest.mark.aiohttpretty async def test_download_without_id(self, provider_one, download_response, file_path, @@ -141,7 +118,7 @@ async def test_download_with_display_name(self, provider_and_mock_one, download_ provider, inner_provider = provider_and_mock_one - uri, params = build_signed_url_with_auth(provider, 'GET', download_path.identifier, + uri, params = build_signed_url_without_auth(provider, 'GET', download_path.identifier, 'download', version=None, mode=None) aiohttpretty.register_json_uri('GET', uri, body=download_response, params=params) @@ -157,6 +134,78 @@ async def test_download_with_display_name(self, provider_and_mock_one, download_ inner_provider.download.assert_called_once_with(path=expected_path, display_name=expected_name) + @pytest.mark.asyncio + @pytest.mark.aiohttpretty + async def test_download_with_embedded_storage_skips_osf( + self, provider_and_mock_one, download_response, download_path, mock_time): + + provider, inner_provider = provider_and_mock_one + uri, params = build_signed_url_without_auth( + provider, 'GET', download_path.identifier, 'download', version=None, mode=None, + ) + + metadata = OsfStorageFileMetadata({ + 'kind': 'file', + 'name': download_response['data']['name'], + 'path': f'/{download_path.identifier}', + 'storage': download_response, + }, str(download_path)) + + await provider.download(download_path, metadata=metadata) + + assert not aiohttpretty.has_call(method='GET', uri=uri, params=params) + provider.make_provider.assert_called_once_with(download_response['settings']) + expected_path = WaterButlerPath('/' + download_response['data']['path']) + inner_provider.download.assert_called_once_with( + path=expected_path, + display_name=download_response['data']['name'], + ) + + @pytest.mark.asyncio + @pytest.mark.aiohttpretty + async def test_download_without_storage_falls_back_to_osf( + self, provider_and_mock_one, download_response, download_path, mock_time): + + provider, inner_provider = provider_and_mock_one + uri, params = build_signed_url_without_auth( + provider, 'GET', download_path.identifier, 'download', version=None, mode=None, + ) + aiohttpretty.register_json_uri('GET', uri, body=download_response, params=params) + + metadata = OsfStorageFileMetadata({ + 'kind': 'file', + 'name': download_response['data']['name'], + 'path': f'/{download_path.identifier}', + }, str(download_path)) + + await provider.download(download_path, metadata=metadata) + + assert aiohttpretty.has_call(method='GET', uri=uri, params=params) + inner_provider.download.assert_called_once() + + @pytest.mark.asyncio + @pytest.mark.aiohttpretty + async def test_download_with_version_ignores_embedded_storage( + self, provider_and_mock_one, download_response, download_path, mock_time): + + provider, inner_provider = provider_and_mock_one + uri, params = build_signed_url_without_auth( + provider, 'GET', download_path.identifier, 'download', version=1, mode=None, + ) + aiohttpretty.register_json_uri('GET', uri, body=download_response, params=params) + + metadata = OsfStorageFileMetadata({ + 'kind': 'file', + 'name': download_response['data']['name'], + 'path': f'/{download_path.identifier}', + 'storage': download_response, + }, str(download_path)) + + await provider.download(download_path, metadata=metadata, version=1) + + assert aiohttpretty.has_call(method='GET', uri=uri, params=params) + inner_provider.download.assert_called_once() + class TestDelete: @@ -210,7 +259,7 @@ async def test_delete_folder_contents(self, provider_one, file_path, folder_path children_url, params = build_signed_url_without_auth(provider_one, 'GET', folder_path.identifier, 'children', - user_id=provider_one.auth['id']) + user_id=provider_one.auth['id'], minimal=False) aiohttpretty.register_json_uri('GET', children_url, params=params, status=200, body=folder_children_metadata) @@ -227,7 +276,7 @@ class TestMetadata: async def test_provider_metadata_empty(self, provider_one, folder_path, mock_time): url, params = build_signed_url_without_auth(provider_one, 'GET', folder_path.identifier, - 'children', user_id=provider_one.auth['id']) + 'children', user_id=provider_one.auth['id'], minimal=False) aiohttpretty.register_json_uri('GET', url, params=params, status_code=200, body=[]) res = await provider_one.metadata(folder_path) @@ -241,7 +290,7 @@ async def test_provider_metadata_folder(self, provider_one, folder_path, folder_children_metadata, mock_time): url, params = build_signed_url_without_auth(provider_one, 'GET', folder_path.identifier, - 'children', user_id=provider_one.auth['id']) + 'children', user_id=provider_one.auth['id'], minimal=False) aiohttpretty.register_json_uri('GET', url, params=params, status=200, body=folder_children_metadata) @@ -276,7 +325,7 @@ async def test_provider_metadata_without_id(self, provider_one, folder_path, folder_children_metadata, mock_time): url, params = build_signed_url_without_auth(provider_one, 'GET', folder_path.identifier, - 'children') + 'children', minimal=False) aiohttpretty.register_json_uri('GET', url, params=params, status=200, body=folder_children_metadata) folder_path._parts[-1]._id = None @@ -732,7 +781,7 @@ async def test_validate_path_404s(self, provider_one, file_lineage, mock_time): async def test_revalidate_path_new(self, provider_one, folder_path, folder_children_metadata, mock_time): url, params = build_signed_url_without_auth(provider_one, 'GET', folder_path.identifier, - 'children', user_id=provider_one.auth['id']) + 'children', user_id=provider_one.auth['id'], minimal=False) aiohttpretty.register_json_uri('GET', url, params=params, status=200, body=folder_children_metadata) @@ -745,7 +794,7 @@ async def test_revalidate_path_new(self, provider_one, folder_path, folder_child async def test_revalidate_path_existing(self, provider_one, folder_path, folder_children_metadata, mock_time): url, params = build_signed_url_without_auth(provider_one, 'GET', folder_path.identifier, - 'children', user_id=provider_one.auth['id']) + 'children', user_id=provider_one.auth['id'], minimal=False) aiohttpretty.register_json_uri('GET', url, params=params, status=200, body=folder_children_metadata) @@ -1253,3 +1302,38 @@ async def test_intra_copy_reject_by_quota(self, provider_one, auth, credentials, src_provider.can_intra_copy.assert_called_once_with(dst_provider, src_path) src_provider.intra_copy.assert_not_called() src_provider.download.assert_not_called() + +class TestMinimalMetadataDAZ: + + @pytest.mark.asyncio + async def test_zip_requests_minimal_metadata(self, provider_one, folder_path, monkeypatch): + captured = {} + + async def capture_zip(self, path, **kwargs): + captured.update(kwargs) + return object() + + monkeypatch.setattr( + 'waterbutler.core.provider.BaseProvider.zip', + capture_zip, + ) + + await provider_one.zip(folder_path) + + assert captured.get('minimal') is True + + @pytest.mark.asyncio + @pytest.mark.aiohttpretty + async def test_minimal_metadata(self, provider_one, folder_path, folder_children_metadata_minimal): + + url, params = build_signed_url_without_auth(provider_one, 'GET', folder_path.identifier, + 'children', user_id=provider_one.auth['id'], minimal=True) + aiohttpretty.register_json_uri('GET', url, params=params, status=200, + body=folder_children_metadata_minimal) + + metadata = await provider_one.metadata(folder_path, minimal=True) + + assert [type(metadata_item) for metadata_item in metadata] == [OsfStorageFolderMetadata, OsfStorageFileMetadata, OsfStorageFileMetadata, OsfStorageFileMetadata] + assert [metadata_item.name for metadata_item in metadata] == ['New Folder', 'one', 'doc.rst', 'video.mp4'] + assert [metadata_item.path for metadata_item in metadata] == ['/59c0054cb7d1c90114c456af/', '/59a9b637b7d1c903ab5a8f58', '/59a9b628b7d1c903ab5a8f52', '/59a5adb9b7d1c901cd40f0e9'] + assert [metadata_item.provider for metadata_item in metadata] == ['osfstorage', 'osfstorage', 'osfstorage', 'osfstorage'] diff --git a/tests/server/api/v1/test_metadata_mixin.py b/tests/server/api/v1/test_metadata_mixin.py index 48308e806e..8e7ed22719 100644 --- a/tests/server/api/v1/test_metadata_mixin.py +++ b/tests/server/api/v1/test_metadata_mixin.py @@ -233,7 +233,7 @@ async def test_download_folder_as_zip(self, http_request, mock_stream): await handler.download_folder_as_zip() assert handler._headers['Content-Type'] == 'application/zip' - expected = 'attachment; filename="test_file.zip"; filename*=UTF-8\'\'test_file.zip' + expected = 'attachment; filename="guid1-test_file.zip"; filename*=UTF-8\'\'guid1-test_file.zip' assert handler._headers['Content-Disposition'] == expected handler.write_stream.assert_called_once_with(mock_stream) @@ -249,7 +249,7 @@ async def test_download_folder_as_zip_root(self, http_request, mock_stream): await handler.download_folder_as_zip() assert handler._headers['Content-Type'] == 'application/zip' - expected = 'attachment; filename="MockProvider-archive.zip"; filename*=UTF-8\'\'MockProvider-archive.zip' + expected = 'attachment; filename="guid1-MockProvider-archive.zip"; filename*=UTF-8\'\'guid1-MockProvider-archive.zip' assert handler._headers['Content-Disposition'] == expected handler.write_stream.assert_called_once_with(mock_stream) diff --git a/waterbutler/core/cache.py b/waterbutler/core/cache.py new file mode 100644 index 0000000000..59e71cf87f --- /dev/null +++ b/waterbutler/core/cache.py @@ -0,0 +1,43 @@ +import asyncio + + +class CacheableMetadataProviderProxy: + """Wraps a provider so that folder ``metadata`` requests are cached and the + metadata of any child folders is prefetched concurrently. + + When a folder is listed, a metadata task is scheduled for each of its child + folders without awaiting them. Those tasks run on the event loop while files + are being downloaded, so the generator can issue all of its metadata requests + up front instead of fetching folder listings strictly one-at-a-time. + + Any attribute that is not overridden here (``download``, ``path_from_metadata``, + etc.) is delegated to the wrapped provider. + """ + + def __init__(self, provider): + self.provider = provider + self.__cache = dict[str, asyncio.Task]() + + def metadata_task(self, path, **kwargs): + key = path.identifier or str(path) + if key not in self.__cache: + self.__cache[key] = asyncio.create_task( + self.provider.metadata(path, **kwargs) + ) + return self.__cache[key] + + async def metadata(self, path, **kwargs): + if path.is_file: + return await self.provider.metadata(path, **kwargs) + + items = await self.metadata_task(path, **kwargs) + + for item in items: + if item.is_folder: + child = self.provider.path_from_metadata(path, item) + self.metadata_task(child, **kwargs) + + return items + + def __getattr__(self, name): + return getattr(self.provider, name) diff --git a/waterbutler/core/provider.py b/waterbutler/core/provider.py index 45788a698d..2c6afea9e3 100644 --- a/waterbutler/core/provider.py +++ b/waterbutler/core/provider.py @@ -18,7 +18,8 @@ from waterbutler import settings as wb_settings from waterbutler.core.metrics import MetricsRecord from waterbutler.core import metadata as wb_metadata -from waterbutler.core.utils import ZipStreamGenerator +from waterbutler.core.cache import CacheableMetadataProviderProxy +from waterbutler.core.utils import ZipStreamGenerator, ZipStreamGeneratorPaginated from waterbutler.core.utils import RequestHandlerContext @@ -678,18 +679,17 @@ async def revalidate_path(self, """ return base.child(path, folder=folder) - async def zip(self, path: wb_path.WaterButlerPath, **kwargs) -> asyncio.StreamReader: + async def zip(self, path: wb_path.WaterButlerPath, use_cache: bool = True, paginated: bool = False, **kwargs) -> asyncio.StreamReader: """Streams a Zip archive of the given folder :param path: ( :class:`.WaterButlerPath` ) The folder to compress + :param use_cache: ( :class:`bool` ) Whether to prefetch metadata requests for nested folders during zip, + does not work for paginated metadata requests + :param paginated: ( :class:`bool` ) Whether to use the paginated zip stream generator """ - - meta_data = await self.metadata(path) # type: ignore - if path.is_file: - meta_data = [meta_data] # type: ignore - path = path.parent - - return streams.ZipStreamReader(ZipStreamGenerator(self, path, *meta_data)) # type: ignore + provider = CacheableMetadataProviderProxy(self) if use_cache else self + generator = ZipStreamGeneratorPaginated if paginated else ZipStreamGenerator + return streams.ZipStreamReader(generator(provider, path, **kwargs)) # type: ignore def shares_storage_root(self, other: 'BaseProvider') -> bool: """Returns True if ``self`` and ``other`` both point to the same storage root. Used to @@ -763,6 +763,20 @@ async def metadata(self, path: wb_path.WaterButlerPath, **kwargs) \ """ raise NotImplementedError + async def iter_children_pages(self, path: wb_path.WaterButlerPath, **kwargs): + """Yield a folder's children metadata one page at a time. + + The default implementation yields a single page containing the entire folder + listing, since most providers return all children in one request. Providers + with server-side pagination should override this to lazily yield successive + pages, so callers (e.g. the zip stream generators) don't hold an entire large + folder listing in memory at once. + + :param path: ( :class:`.WaterButlerPath` ) The folder to list + :param kwargs: ( :class:`dict` ) Arguments to be parsed by child classes + """ + yield await self.metadata(path, **kwargs) + @abc.abstractmethod async def validate_v1_path(self, path: str, **kwargs) -> wb_path.WaterButlerPath: """API v1 requires that requests against folder endpoints always end with a slash, and diff --git a/waterbutler/core/utils.py b/waterbutler/core/utils.py index ff8a3e13f2..983e9b3b66 100644 --- a/waterbutler/core/utils.py +++ b/waterbutler/core/utils.py @@ -1,4 +1,5 @@ import re +import abc import json import pytz import asyncio @@ -192,25 +193,55 @@ def make_disposition(filename): encoded_filename) -class ZipStreamGenerator: - def __init__(self, provider, parent_path, *metadata_objs): +class BaseZipStreamGenerator(abc.ABC): + + def __init__(self, provider, root_path, **kwargs): self.provider = provider - self.parent_path = parent_path - self.remaining = [ - (parent_path, metadata) - for metadata in metadata_objs - ] + self.root_path = root_path + self.parent_path = root_path.parent if root_path.is_file else root_path + self.kwargs = kwargs + + self.remaining = [] + self.initialized = False async def __aiter__(self): return self + @abc.abstractmethod + async def _initialize(self): + raise NotImplementedError + + @abc.abstractmethod async def __anext__(self): + raise NotImplementedError + + +class ZipStreamGenerator(BaseZipStreamGenerator): + + async def _initialize(self): + if self.initialized: + return + + if self.root_path.is_file: + metadata = await self.provider.metadata(self.root_path, **self.kwargs) + self.remaining.append((self.root_path.parent, metadata)) + return + + items = await self.provider.metadata(self.root_path, **self.kwargs) + self.remaining.extend((self.root_path, item) for item in items) + + self.initialized = True + + async def __anext__(self): + if not self.initialized: + await self._initialize() + if not self.remaining: raise StopAsyncIteration current = self.remaining.pop(0) path = self.provider.path_from_metadata(*current) if path.is_dir: - items = await self.provider.metadata(path) + items = await self.provider.metadata(path, **self.kwargs) if items: self.remaining.extend([ (path, item) for item in items @@ -219,7 +250,69 @@ async def __anext__(self): else: return path.path.replace(self.parent_path.path, '', 1), EmptyStream() - return path.path.replace(self.parent_path.path, '', 1), await self.provider.download(path) + return ( + path.path.replace(self.parent_path.path, '', 1), + await self.provider.download(path, metadata=current[1]) + ) + + +class ZipStreamGeneratorPaginated(BaseZipStreamGenerator): + + async def _initialize(self): + if self.initialized: + return + + if self.root_path.is_file: + metadata = await self.provider.metadata( + self.root_path, + **self.kwargs, + ) + self.remaining.append((self.root_path.parent, metadata)) + return + + async for page in self.provider.iter_children_pages( + self.root_path, + **self.kwargs, + ): + self.remaining.extend( + (self.root_path, item) + for item in page + ) + + self.initialized = True + + async def __anext__(self): + if not self.initialized: + await self._initialize() + + if not self.remaining: + raise StopAsyncIteration + + current = self.remaining.pop(0) + path = self.provider.path_from_metadata(*current) + + if path.is_dir: + async for page in self.provider.iter_children_pages( + path, + **self.kwargs, + ): + if not page: + return ( + path.path.replace(self.parent_path.path, '', 1), + EmptyStream(), + ) + + self.remaining.extend( + (path, item) + for item in page + ) + + return await self.__anext__() + + return ( + path.path.replace(self.parent_path.path, '', 1), + await self.provider.download(path, metadata=current[1]), + ) class RequestHandlerContext: diff --git a/waterbutler/providers/osfstorage/provider.py b/waterbutler/providers/osfstorage/provider.py index 0b59f2accf..39dd7b774e 100644 --- a/waterbutler/providers/osfstorage/provider.py +++ b/waterbutler/providers/osfstorage/provider.py @@ -6,6 +6,8 @@ import logging from http import HTTPStatus +import sentry_sdk + from waterbutler.core import utils from waterbutler.core import signing from waterbutler.core import streams @@ -13,6 +15,7 @@ from waterbutler.core import exceptions from waterbutler.core.path import WaterButlerPath from waterbutler.core.metadata import BaseMetadata +from waterbutler.core import path as wb_path from waterbutler.providers.osfstorage import settings from waterbutler.providers.osfstorage.metadata import OsfStorageFileMetadata @@ -194,7 +197,7 @@ def build_signed_url(method, url, data=None, params=None, ttl=100, **kwargs): return url, data, params - async def download(self, path, version=None, revision=None, mode=None, **kwargs): + async def download(self, path, version=None, revision=None, mode=None, metadata=None, **kwargs): if not path.identifier: raise exceptions.NotFoundError(str(path)) @@ -208,28 +211,46 @@ async def download(self, path, version=None, revision=None, mode=None, **kwargs) # version could be 0 here version = revision - # Capture user_id for analytics if user is logged in - user_param = {} - if self.auth.get('id', None): - user_param = {'user': self.auth['id']} - - # osf storage metadata will return a virtual path within the provider - resp = await self.make_signed_request( - 'GET', - self.build_url(path.identifier, 'download', version=version, mode=mode), - expects=(200, ), - params=user_param, - throws=exceptions.DownloadError, - ) - data = await resp.json() + storage = None + use_embedded_storage = False + if metadata is not None: + storage = metadata.raw.get('storage', None) + use_embedded_storage = version is None and bool(storage and storage.get('data', None)) + if not use_embedded_storage: + with sentry_sdk.new_scope() as scope: + scope.set_context('osfstorage_data', { + 'node': self.nid, + 'path': str(path), + }) + sentry_sdk.capture_message( + 'osfstorage download fell back to OSF /download endpoint', + level='info', + ) + + # DAZ passes minimal child metadata with an embedded ``storage`` block so we can + # call the inner storage provider directly. When that payload is usable we skip + # the per-file OSF ``/download`` hop; otherwise we fall back to fetching it. + if use_embedded_storage: + data = storage + else: + # osf storage metadata will return a virtual path within the provider + resp = await self.make_signed_request( + 'GET', + self.build_url(path.identifier, 'download', version=version, mode=mode), + expects=(200, ), + throws=exceptions.DownloadError, + ) + data = await resp.json() provider_object = self.make_provider(data['settings']) - name = data['data'].pop('name') - data['data']['path'] = await provider_object.validate_path('/' + data['data']['path']) + file_data = dict(data['data']) + name = file_data.pop('name') + file_data['path'] = await provider_object.validate_path('/' + file_data['path']) download_kwargs = {} download_kwargs.update(kwargs) - download_kwargs.update(data['data']) + download_kwargs.update(file_data) download_kwargs['display_name'] = kwargs.get('display_name') or name + return await provider_object.download(**download_kwargs) async def upload(self, stream, path, **kwargs): @@ -315,13 +336,20 @@ async def delete(self, path, confirm_delete=0, **kwargs): expects=(200, ) )).release() + async def zip(self, path: wb_path.WaterButlerPath, **kwargs) -> asyncio.StreamReader: + """Streams a Zip archive of the given folder + + :param path: ( :class:`.WaterButlerPath` ) The folder to compress + """ + return await super().zip(path, **kwargs, minimal=True) + async def metadata(self, path, **kwargs): if path.identifier is None: raise exceptions.MetadataError(f'{str(path)} not found', code=404) if not path.is_dir: return await self._item_metadata(path) - return await self._children_metadata(path) + return await self._children_metadata(path, **kwargs) async def revisions(self, path, view_only=None, **kwargs): if path.identifier is None: @@ -514,9 +542,28 @@ async def copy(self, return meta_data, created + async def iter_children_pages(self, path, **kwargs): + limit = kwargs.pop('limit', None) + after = None + + while True: + page = await self.metadata(path, limit=limit, after=after, **kwargs) + yield page + + if not limit or len(page) < int(limit): + break + + after = page[-1].raw['id'] + # ========== private ========== async def _item_metadata(self, path, revision=None): + """Fetch metadata for a single file from the OSF. + + :param path: file path whose ``identifier`` is the OSF file node id + :param revision: optional file version identifier passed to OSF as ``revision`` + :return: :class:`OsfStorageFileMetadata` for the file + """ resp = await self.make_signed_request( 'GET', self.build_url(path.identifier, revision=revision), @@ -524,10 +571,35 @@ async def _item_metadata(self, path, revision=None): ) return OsfStorageFileMetadata((await resp.json()), str(path)) - async def _children_metadata(self, path): + async def _children_metadata(self, path, limit=None, after=None, **kwargs): + """Fetch folder children metadata from the OSF. + + :param path: folder path whose ``identifier`` is the OSF folder node id + :param limit: page size; when set, enables ORM pagination on OSF + :param after: cursor (child node pk) for the next ORM page + :return: list of :class:`OsfStorageFolderMetadata` and + :class:`OsfStorageFileMetadata` instances + """ + query = { + 'user_id': self.auth.get('id'), + 'minimal': kwargs.get('minimal', False), + } + + # By default OSF serves minimal children via raw SQL on the backend. The Django + # ORM implementation is used only when ``orm`` is explicitly requested or when + # ``limit`` is set (pagination requires ORM). The raw SQL path may be removed in + # the future in favor of ORM-only responses. + if limit is not None: + query['orm'] = True + query['limit'] = limit + if after is not None: + query['after'] = after + elif kwargs.get('orm') is not None: + query['orm'] = kwargs['orm'] + resp = await self.make_signed_request( 'GET', - self.build_url(path.identifier, 'children', user_id=self.auth.get('id')), + self.build_url(path.identifier, 'children', **query), expects=(200, ) ) resp_json = await resp.json() diff --git a/waterbutler/server/api/v1/provider/metadata.py b/waterbutler/server/api/v1/provider/metadata.py index add78e150a..4c376922eb 100644 --- a/waterbutler/server/api/v1/provider/metadata.py +++ b/waterbutler/server/api/v1/provider/metadata.py @@ -127,10 +127,10 @@ async def get_file_revisions(self): return self.write({'data': [r.json_api_serialized() for r in result]}) async def download_folder_as_zip(self): - zipfile_name = self.path.name or f'{self.provider.NAME}-archive' + zipfile_name = f'{self.path_kwargs.get("resource")}-{self.path.name}' if self.path.name else f'{self.path_kwargs.get("resource")}-{self.provider.NAME}-archive' self.set_header('Content-Type', 'application/zip') self.set_header('Content-Disposition', make_disposition(zipfile_name + '.zip')) - result = await self.provider.zip(self.path) + result = await self.provider.zip(self.path, **self.arguments) await self.write_stream(result)