diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 93860aa..c3498a9 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -1,6 +1,7 @@ name: Unit Tests on: + pull_request: push: branches-ignore: - '[0-9]*' diff --git a/docs/cli-usage.rst b/docs/cli-usage.rst index 77fdf85..cbd2a16 100644 --- a/docs/cli-usage.rst +++ b/docs/cli-usage.rst @@ -42,6 +42,16 @@ Below some examples on how to use the command-line program: If you're using python 3+, you can also use the aliases `ls` in place of `list`, and `rm` in place of `remove`. +Remote paths can start with a provider identifier, such as +``s3compatsigv4/folder/file.txt``. Providers are discovered from the project's +storage API. If the first path component names a provider the server offers +but the project has not connected, the command fails instead of writing into +``osfstorage``. Addons in the ``other`` category, such as ``binderhub``, do +not hold files and are not treated as providers. If it matches no provider +at all, the path refers to the default ``osfstorage``. +To access a folder whose name matches a provider, explicitly prefix the path +with ``osfstorage/``. + If the project is private you will need to provide authentication details. You can set the ``OSF_TOKEN`` environment diff --git a/osfclient/api.py b/osfclient/api.py index 529b177..6e72bbc 100644 --- a/osfclient/api.py +++ b/osfclient/api.py @@ -1,4 +1,5 @@ from .exceptions import OSFException +from .models import Addon from .models import OSFCore from .models import Project @@ -46,6 +47,16 @@ async def project(self, project_id): } }, self.session) + @property + async def addons(self): + """Iterate over all addons available on the server.""" + url = self._build_url('addons') + while url: + response = self._json(await self._get(url), 200) + for addon in response['data']: + yield Addon(addon, self.session) + url = response['links']['next'] + @property def token(self): if 'Authorization' not in self.session.headers: diff --git a/osfclient/cli.py b/osfclient/cli.py index 96dedcc..85dc60d 100644 --- a/osfclient/cli.py +++ b/osfclient/cli.py @@ -184,8 +184,9 @@ async def fetch(args): """Fetch an individual file from a project. The first part of the remote path is interpreted as the name of the - storage provider. If there is no match the default (osfstorage) is - used. + connected storage provider. A provider that exists on the server but + is not connected to the project is an error. If there is no match, + the default (osfstorage) is used. The local path defaults to the name of the remote file. @@ -195,7 +196,9 @@ async def fetch(args): If args.force is False but args.update is True, overwrite an existing local file only if local and remote files differ. """ - storage, remote_path = split_storage(args.remote) + osf = _setup_osf(args) + project = await osf.project(args.project) + store, remote_path = await split_storage(args.remote, osf, project) local_path = args.local if local_path is None: @@ -209,10 +212,6 @@ async def fetch(args): if directory: makedirs(directory, exist_ok=True) - osf = _setup_osf(args) - project = await osf.project(args.project) - - store = await project.storage(storage) # only fetching one file so we are done file_ = await find_by_path(store, remote_path) if file_ is None or is_folder(file_): @@ -279,8 +278,9 @@ async def upload(args): """Upload a new file to an existing project. The first part of the remote path is interpreted as the name of the - storage provider. If there is no match the default (osfstorage) is - used. + connected storage provider. A provider that exists on the server but + is not connected to the project is an error. If there is no match, + the default (osfstorage) is used. If the project is private you need to specify a username or token. @@ -300,9 +300,8 @@ async def upload(args): sys.exit('To upload a file you need to provide a token.') project = await osf.project(args.project) - storage, remote_path = split_storage(args.destination) + store, remote_path = await split_storage(args.destination, osf, project) - store = await project.storage(storage) if args.recursive: if not os.path.isdir(args.source): raise RuntimeError("Expected source ({}) to be a directory when " @@ -333,8 +332,9 @@ async def makefolder(args): """Create a new folder in an existing project. The first part of the remote path is interpreted as the name of the - storage provider. If there is no match the default (osfstorage) is - used. + connected storage provider. A provider that exists on the server but + is not connected to the project is an error. If there is no match, + the default (osfstorage) is used. """ osf = _setup_osf(args) if not osf.has_auth: @@ -342,9 +342,8 @@ async def makefolder(args): project = await osf.project(args.project) - storage, remote_path = split_storage(args.target) + store, remote_path = await split_storage(args.target, osf, project) - store = await project.storage(storage) f = await find_ancestral_folder(store, remote_path) if f is None: parent = store @@ -362,8 +361,9 @@ async def remove(args): """Remove a file from the project's storage. The first part of the remote path is interpreted as the name of the - storage provider. If there is no match the default (osfstorage) is - used. + connected storage provider. A provider that exists on the server but + is not connected to the project is an error. If there is no match, + the default (osfstorage) is used. """ osf = _setup_osf(args) if not osf.has_auth: @@ -371,9 +371,8 @@ async def remove(args): project = await osf.project(args.project) - storage, remote_path = split_storage(args.target) + store, remote_path = await split_storage(args.target, osf, project) - store = await project.storage(storage) f = await find_by_path(store, remote_path) if f is None: sys.exit('No files found to remove.') @@ -385,8 +384,9 @@ async def move(args): """Move a file to specified location on the project's storage. The first part of the paths is interpreted as the name of the - storage provider. If there is no match the default (osfstorage) is - used. + connected storage provider. A provider that exists on the server but + is not connected to the project is an error. If there is no match, + the default (osfstorage) is used. """ osf = _setup_osf(args) if not osf.has_auth: @@ -394,7 +394,9 @@ async def move(args): project = await osf.project(args.project) - target_storage, target_path = split_storage(args.target, normalize=False) + target_store, target_path = await split_storage( + args.target, osf, project, normalize=False) + target_storage = target_store.provider if target_path.endswith('/'): target_folder_path = target_path[:-1] @@ -409,16 +411,14 @@ async def move(args): else: target_folder_path = None target_filename = target_path - target_store = await project.storage(target_storage) if target_folder_path is None: target_folder = target_store else: target_folder = await _ensure_folder(target_store, target_folder_path) # Move a file - storage, remote_path = split_storage(args.source) + store, remote_path = await split_storage(args.source, osf, project) - store = await project.storage(storage) f = await find_by_path(store, remote_path) if f is None: sys.exit('No files found to move.') diff --git a/osfclient/models/__init__.py b/osfclient/models/__init__.py index 35c3271..3bcafe6 100644 --- a/osfclient/models/__init__.py +++ b/osfclient/models/__init__.py @@ -3,6 +3,7 @@ Users should not have to instantiate classes from here, instead they should use `osfclient.OSF()` to access the OSF. """ +from .addon import Addon from .core import OSFCore from .file import File from .file import Folder diff --git a/osfclient/models/addon.py b/osfclient/models/addon.py new file mode 100644 index 0000000..8b754be --- /dev/null +++ b/osfclient/models/addon.py @@ -0,0 +1,11 @@ +from .core import OSFCore + + +class Addon(OSFCore): + def _update_attributes(self, addon): + self.id = self._get_attribute(addon, 'id') + self.name = self._get_attribute(addon, 'attributes', 'name') + self.categories = self._get_attribute(addon, 'attributes', 'categories') + + def __str__(self): + return ''.format(self.id) diff --git a/osfclient/models/project.py b/osfclient/models/project.py index 6764349..3ab56b7 100644 --- a/osfclient/models/project.py +++ b/osfclient/models/project.py @@ -38,12 +38,9 @@ def __str__(self): async def storage(self, provider='osfstorage'): """Return storage `provider`.""" - stores = self._json(await self._get(self._storages_url), 200) - stores = stores['data'] - for store in stores: - provides = self._get_attribute(store, 'attributes', 'provider') - if provides == provider: - return Storage(store, self.session) + async for store in self.storages: + if store.provider == provider: + return store raise RuntimeError("Project has no storage " "provider '{}'".format(provider)) @@ -51,7 +48,9 @@ async def storage(self, provider='osfstorage'): @property async def storages(self): """Iterate over all storages for this projects.""" - stores = self._json(await self._get(self._storages_url), 200) - stores = stores['data'] - for store in stores: - yield Storage(store, self.session) + url = self._storages_url + while url: + response = self._json(await self._get(url), 200) + for store in response['data']: + yield Storage(store, self.session) + url = response['links']['next'] diff --git a/osfclient/tests/fake_responses.py b/osfclient/tests/fake_responses.py index 873d4a1..a6a0c79 100644 --- a/osfclient/tests/fake_responses.py +++ b/osfclient/tests/fake_responses.py @@ -53,6 +53,26 @@ def storage_node(project_id, storages=['osfstorage']): 'n_storages': len(used_storages)}) +# Use this to fake a response when asking for the server's addons +# e.g. osf.addons +def addons(ids, categories=['storage']): + return { + 'data': [{ + 'id': addon_id, + 'type': 'addon', + 'attributes': {'name': addon_id, 'categories': categories}, + 'links': {}, + } for addon_id in ids], + 'links': { + 'first': None, + 'last': None, + 'prev': None, + 'next': None, + 'meta': {'total': len(ids), 'per_page': 1000}, + }, + } + + def _folder(osf_id, name, storage='osfstorage'): template = """{ "relationships": { diff --git a/osfclient/tests/mocks.py b/osfclient/tests/mocks.py index e335188..913c8e5 100644 --- a/osfclient/tests/mocks.py +++ b/osfclient/tests/mocks.py @@ -1,4 +1,5 @@ import asyncio +from types import SimpleNamespace from mock import MagicMock, PropertyMock, AsyncMock from ..utils import norm_remote_path import copy @@ -58,7 +59,7 @@ def MockStorage(name): MockFolder('/a',folders=a_folders), MockFolder('/b',folders=b_folders), MockFolder('/c',folders=c_folders)] - mock = MagicMock(name='Storage-%s' % name, + mock = MagicMock(name='Storage-%s' % name, provider=name, folders=AsyncIterator(folders), children=AsyncIterator(folders)) mock.create_file = MagicMock(return_value=FutureWrapper()) @@ -99,16 +100,26 @@ def MockStream(path, mode, size=1024): def MockProject(name): - mock = MagicMock(name='Project-%s' % name, - storages=AsyncIterator([MockStorage('osfstorage'), MockStorage('gh')])) + default_store = MockStorage('osfstorage') + mock = MagicMock(name='Project-%s' % name, id=name, + storages=AsyncIterator([default_store, MockStorage('gh')])) storage = MagicMock(name='Project-%s-storage' % name, - return_value=FutureMockStorage('osfstorage')) + return_value=FutureWrapper(default_store)) type(mock).storage = storage mock._storage_mock = storage return mock +def MockAddon(addon_id, categories=['storage']): + return SimpleNamespace(id=addon_id, name=addon_id, categories=categories) + + +def MockAddons(addons): + """Stand-in for the `OSF.addons` property.""" + return PropertyMock(return_value=AsyncIterator(addons)) + + def MockArgs(output=None, project=None, source=None, destination=None, local=None, remote=None, target=None, force=False, update=False, recursive=False, diff --git a/osfclient/tests/test_api.py b/osfclient/tests/test_api.py index 185f753..9670f5a 100644 --- a/osfclient/tests/test_api.py +++ b/osfclient/tests/test_api.py @@ -7,6 +7,7 @@ from osfclient.models import OSFCore from osfclient.models import Project +from osfclient.tests import fake_responses from osfclient.tests.mocks import FakeResponse @@ -34,3 +35,34 @@ def test_endpoint(session_set_endpoint): osf = OSF(base_url='https://api.test.osf.io/v2/') session_set_endpoint.assert_called_with('https://api.test.osf.io/v2/') + + +@pytest.mark.asyncio +@patch.object(OSFCore, '_get') +async def test_addons(OSFCore_get): + osf = OSF(base_url='https://api.test.osf.io/v2/') + first_url = 'https://api.test.osf.io/v2/addons/' + next_url = first_url + '?page=2' + first = fake_responses.addons(['s3', 'github']) + first['links']['next'] = next_url + second = fake_responses.addons(['binderhub'], categories=['other']) + OSFCore_get.side_effect = [FakeResponse(200, first), + FakeResponse(200, second)] + + addons = [addon async for addon in osf.addons] + + assert [addon.id for addon in addons] == ['s3', 'github', 'binderhub'] + assert [addon.categories for addon in addons] == [ + ['storage'], ['storage'], ['other']] + assert all(addon.session is osf.session for addon in addons) + assert OSFCore_get.call_args_list == [call(first_url), call(next_url)] + + +@pytest.mark.asyncio +@patch.object(OSFCore, '_get', return_value=FakeResponse(403, {})) +async def test_addons_propagates_api_error(OSFCore_get): + osf = OSF() + + with pytest.raises(RuntimeError, match='403'): + async for _ in osf.addons: + pass diff --git a/osfclient/tests/test_connected_providers.py b/osfclient/tests/test_connected_providers.py new file mode 100644 index 0000000..e096386 --- /dev/null +++ b/osfclient/tests/test_connected_providers.py @@ -0,0 +1,151 @@ +"""Test file transfers using providers discovered from the project.""" + +import pytest +from mock import patch + +from osfclient import OSF +from osfclient.cli import fetch, upload, makefolder, remove, move +from osfclient.tests.mocks import ( + AsyncIterator, MockAddon, MockAddons, MockArgs, MockProject, MockStorage, + MockStream, + is_folder_mock, mock_async_open, +) +from osfclient.utils import find_by_path + + +@pytest.mark.asyncio +@pytest.mark.parametrize('provider', [ + 's3compatsigv4', 'dropboxbusiness', 'onedrive', 'future-provider', +]) +@pytest.mark.parametrize('leading_slash', ['', '/']) +async def test_upload_connected_provider(provider, leading_slash, monkeypatch): + monkeypatch.delenv('KNOWN_PROVIDERS', raising=False) + monkeypatch.setenv('OSF_TOKEN', 'secret') + project = MockProject('1234') + store = MockStorage(provider) + project.storages = AsyncIterator([MockStorage('osfstorage'), store]) + prefix = leading_slash + provider + '/' + args = MockArgs(project='1234', source='local.txt', + destination=prefix + 'folder/remote.txt') + stream = MockStream('local.txt', 'rb') + open_mock = mock_async_open(stream) + + with patch.object(OSF, 'project', return_value=project): + with patch('osfclient.cli.aiofiles.open', open_mock): + await upload(args) + + open_mock.assert_called_once_with('local.txt', 'rb') + store.create_file.assert_called_once_with( + 'folder/remote.txt', stream, force=False, update=False) + + +@pytest.mark.asyncio +@pytest.mark.parametrize('provider', [ + 's3compatsigv4', 'dropboxbusiness', 'onedrive', 'future-provider', +]) +@pytest.mark.parametrize('leading_slash', ['', '/']) +async def test_fetch_connected_provider(provider, leading_slash, monkeypatch, + tmp_path): + monkeypatch.delenv('KNOWN_PROVIDERS', raising=False) + monkeypatch.setenv('OSF_TOKEN', 'secret') + project = MockProject('1234') + store = MockStorage(provider) + project.storages = AsyncIterator([MockStorage('osfstorage'), store]) + prefix = leading_slash + provider + '/' + local = str(tmp_path / 'download.txt') + args = MockArgs(project='1234', remote=prefix + 'a/a/a', local=local) + stream = MockStream(local, 'wb') + open_mock = mock_async_open(stream) + + with patch.object(OSF, 'project', return_value=project): + with patch('osfclient.cli.aiofiles.open', open_mock): + with patch('osfclient.cli.is_folder', side_effect=is_folder_mock): + await fetch(args) + + open_mock.assert_called_once_with(local, 'wb') + file_ = await find_by_path(store, 'a/a/a') + file_.write_to.assert_called_once_with(stream) + + +@pytest.mark.asyncio +@pytest.mark.parametrize('command', [makefolder, remove]) +async def test_modify_connected_provider(command, monkeypatch): + monkeypatch.setenv('OSF_TOKEN', 'secret') + project = MockProject('1234') + default_store = MockStorage('osfstorage') + store = MockStorage('future-provider') + project.storages = AsyncIterator([default_store, store]) + path = 'new-folder' if command is makefolder else 'a/a/a' + args = MockArgs(project='1234', target='future-provider/' + path) + + with patch.object(OSF, 'project', return_value=project): + await command(args) + + if command is makefolder: + store.create_folder.assert_called_once_with('new-folder') + default_store.create_folder.assert_not_called() + else: + file_ = await find_by_path(store, 'a/a/a') + file_.remove.assert_called_once_with() + default_file = await find_by_path(default_store, 'a/a/a') + default_file.remove.assert_not_called() + + +@pytest.mark.asyncio +@pytest.mark.parametrize('target_path, folder_path, filename', [ + ('future-target/', None, None), + ('/future-target/', None, None), + ('/future-target/c/', 'c', None), + ('future-target/renamed.txt', None, 'renamed.txt'), +]) +async def test_move_between_connected_providers(target_path, folder_path, + filename, monkeypatch): + monkeypatch.setenv('OSF_TOKEN', 'secret') + project = MockProject('1234') + source = MockStorage('future-source') + target = MockStorage('future-target') + project.storages = AsyncIterator([source, target]) + args = MockArgs(project='1234', source='/future-source/a/a/a', + target=target_path) + + with patch.object(OSF, 'project', return_value=project): + with patch('osfclient.cli.is_folder', side_effect=is_folder_mock): + await move(args) + + folder = target if folder_path is None else await find_by_path( + target, folder_path) + file_ = await find_by_path(source, 'a/a/a') + file_.move_to.assert_called_once_with( + 'future-target', folder, to_filename=filename, force=False) + + +@pytest.mark.asyncio +@pytest.mark.parametrize('command, path_args', [ + (upload, {'source': 'local.txt', 'destination': None}), + (fetch, {'remote': None}), + (makefolder, {'target': None}), + (remove, {'target': None}), + (move, {'source': None, 'target': 'osfstorage/c/'}), +]) +@patch.object(OSF, 'addons', new=MockAddons([ + MockAddon('binderhub', categories=['other']), MockAddon('s3compatsigv4')])) +async def test_unconnected_provider_is_rejected(command, path_args, + monkeypatch): + monkeypatch.setenv('OSF_TOKEN', 'secret') + project = MockProject('1234') + default_store = MockStorage('osfstorage') + project.storages = AsyncIterator([default_store]) + path_args = {key: 's3compatsigv4/folder/file.txt' if value is None else value + for key, value in path_args.items()} + args = MockArgs(project='1234', **path_args) + + with patch.object(OSF, 'project', return_value=project): + with pytest.raises(RuntimeError, match=( + "Storage provider 's3compatsigv4' is not connected to " + "project '1234'")): + await command(args) + + default_store.create_file.assert_not_called() + default_store.create_folder.assert_not_called() + file_ = await find_by_path(default_store, 'folder/file.txt') + assert file_ is None diff --git a/osfclient/tests/test_mkdir.py b/osfclient/tests/test_mkdir.py index 85638be..91ef85d 100644 --- a/osfclient/tests/test_mkdir.py +++ b/osfclient/tests/test_mkdir.py @@ -8,6 +8,7 @@ from osfclient import OSF from osfclient.cli import makefolder +from osfclient.tests.mocks import MockAddon, MockAddons from osfclient.tests.mocks import MockArgs from osfclient.tests.mocks import MockProject @@ -114,6 +115,7 @@ def simple_getenv(key, default=None): @pytest.mark.asyncio +@patch.object(OSF, 'addons', new=MockAddons([MockAddon('github')])) @patch.object(OSF, 'project', return_value=MockProject('1234')) async def test_wrong_storage_name(OSF_project): args = MockArgs(project='1234', target='DOESNTEXIST/a/a/a') diff --git a/osfclient/tests/test_moving.py b/osfclient/tests/test_moving.py index f2f8dbd..cafd689 100644 --- a/osfclient/tests/test_moving.py +++ b/osfclient/tests/test_moving.py @@ -8,6 +8,7 @@ from osfclient import OSF from osfclient.cli import move +from osfclient.tests.mocks import MockAddon, MockAddons from osfclient.tests.mocks import MockArgs from osfclient.tests.mocks import MockProject @@ -328,6 +329,7 @@ def simple_getenv(key, default=None): @pytest.mark.asyncio +@patch.object(OSF, 'addons', new=MockAddons([MockAddon('github')])) @patch.object(OSF, 'project', return_value=MockProject('1234')) async def test_wrong_storage_name(OSF_project): args = MockArgs(project='1234', diff --git a/osfclient/tests/test_project.py b/osfclient/tests/test_project.py index ca8ed24..09c9527 100644 --- a/osfclient/tests/test_project.py +++ b/osfclient/tests/test_project.py @@ -1,9 +1,11 @@ from mock import patch import pytest +from osfclient import OSF from osfclient.models import OSFCore from osfclient.models import Project from osfclient.models import Storage +from osfclient.utils import split_storage from osfclient.tests import fake_responses from osfclient.tests.mocks import FakeResponse @@ -93,3 +95,56 @@ async def test_pass_down_session_to_storages(OSFCore_get): async for store in project.storages: assert store.session == project.session + + +@pytest.mark.asyncio +@patch.object(OSFCore, '_get') +async def test_resolve_storage_on_later_page(OSFCore_get): + project = Project({}) + project._storages_url = 'https://api.osf.io/v2/nodes/f3szh/files/' + next_url = project._storages_url + '?page=2' + first = fake_responses.storage_node('f3szh', ['osfstorage']) + first['links']['next'] = next_url + second = fake_responses.storage_node('f3szh', ['new-provider']) + second['data'][0]['attributes']['name'] = 'A display name' + OSFCore_get.side_effect = [FakeResponse(200, first), FakeResponse(200, second)] + + store, path = await split_storage('/new-provider/folder/file.txt', OSF(), project) + + assert store.provider == 'new-provider' + assert store.name == 'A display name' + assert store.session is project.session + assert path == 'folder/file.txt' + assert [c.args[0] for c in OSFCore_get.call_args_list] == [ + project._storages_url, next_url] + + +@pytest.mark.asyncio +@patch.object(OSFCore, '_get') +async def test_resolve_storage_propagates_api_error(OSFCore_get): + project = Project({}) + project._storages_url = 'https://api.osf.io/v2/nodes/f3szh/files/' + first = fake_responses.storage_node('f3szh', ['osfstorage']) + first['links']['next'] = project._storages_url + '?page=2' + OSFCore_get.side_effect = [FakeResponse(200, first), FakeResponse(403, {})] + + with pytest.raises(RuntimeError, match='403'): + await split_storage('new-provider/file.txt', OSF(), project) + + +@pytest.mark.asyncio +@pytest.mark.parametrize('missing_field', ['links', 'next']) +@patch.object(OSFCore, '_get') +async def test_resolve_storage_rejects_missing_pagination(OSFCore_get, + missing_field): + project = Project({}) + project._storages_url = 'https://api.osf.io/v2/nodes/f3szh/files/' + response = fake_responses.storage_node('f3szh', ['osfstorage']) + if missing_field == 'links': + del response['links'] + else: + del response['links']['next'] + OSFCore_get.return_value = FakeResponse(200, response) + + with pytest.raises(KeyError, match=missing_field): + await split_storage('folder/file.txt', OSF(), project) diff --git a/osfclient/tests/test_removing.py b/osfclient/tests/test_removing.py index 40e9547..0b181a9 100644 --- a/osfclient/tests/test_removing.py +++ b/osfclient/tests/test_removing.py @@ -8,6 +8,7 @@ from osfclient import OSF from osfclient.cli import remove +from osfclient.tests.mocks import MockAddon, MockAddons from osfclient.tests.mocks import MockArgs from osfclient.tests.mocks import MockProject @@ -52,6 +53,7 @@ def simple_getenv(key, default=None): @pytest.mark.asyncio +@patch.object(OSF, 'addons', new=MockAddons([MockAddon('github')])) @patch.object(OSF, 'project', return_value=MockProject('1234')) async def test_wrong_storage_name(OSF_project): args = MockArgs(project='1234', target='DOESNTEXIST/a/a/a') diff --git a/osfclient/tests/test_uploading.py b/osfclient/tests/test_uploading.py index fa958e3..88be834 100644 --- a/osfclient/tests/test_uploading.py +++ b/osfclient/tests/test_uploading.py @@ -10,6 +10,7 @@ from osfclient import OSF from osfclient.cli import upload +from osfclient.tests.mocks import MockAddon, MockAddons from osfclient.tests.mocks import MockArgs from osfclient.tests.mocks import MockProject from osfclient.tests.mocks import mock_async_open, MockStream @@ -31,6 +32,7 @@ def simple_getenv(key, default=None): @pytest.mark.asyncio +@patch.object(OSF, 'addons', new=MockAddons([MockAddon('github')])) @patch.object(OSF, 'project', return_value=MockProject('1234')) async def test_select_project(OSF_project): args = MockArgs(project='1234', @@ -53,9 +55,6 @@ def simple_getenv(key, default=None): # the mock project created by calling OSF().project() fake_project = OSF_project.return_value - expected = [call('osfstorage')] - assert fake_project._storage_mock.mock_calls == expected - # assert fake_project.mock_calls == expected expected = [call.create_file('bar/bar/foo.txt', fake_stream, force=False, update=False)] @@ -66,6 +65,7 @@ def simple_getenv(key, default=None): @pytest.mark.asyncio +@patch.object(OSF, 'addons', new=MockAddons([MockAddon('github')])) @patch.object(OSF, 'project', return_value=MockProject('1234')) async def test_recursive_requires_directory(OSF_project): # test that we check if source is a directory when using recursive mode @@ -89,6 +89,7 @@ def simple_getenv(key, default=None): @pytest.mark.asyncio +@patch.object(OSF, 'addons', new=MockAddons([MockAddon('github')])) @patch.object(OSF, 'project', return_value=MockProject('1234')) async def test_recursive_upload(OSF_project): # test that we check if source is a directory when using recursive mode @@ -135,6 +136,7 @@ def simple_getenv(key, default=None): @pytest.mark.asyncio +@patch.object(OSF, 'addons', new=MockAddons([MockAddon('github')])) @patch.object(OSF, 'project', return_value=MockProject('1234')) async def test_recursive_upload_with_subdir(OSF_project): # test that an extra level of subdirectory is created on the remote side diff --git a/osfclient/tests/test_utils.py b/osfclient/tests/test_utils.py index 3a0f9bc..07e20eb 100644 --- a/osfclient/tests/test_utils.py +++ b/osfclient/tests/test_utils.py @@ -1,3 +1,5 @@ +from types import SimpleNamespace + import pytest from mock import call, patch, Mock @@ -5,102 +7,83 @@ from osfclient.utils import norm_remote_path from osfclient.utils import makedirs from osfclient.utils import split_storage -from osfclient.tests.mocks import MockStream - - -def test_default_storage(): - def simple_getenv(key): - return None - - with patch('osfclient.cli.os.getenv', - side_effect=simple_getenv) as mock_getenv: - store, path = split_storage('foo/bar/baz') - assert store == 'osfstorage' - assert path == 'foo/bar/baz' - - store, path = split_storage('/foo/bar/baz') - assert store == 'osfstorage' - assert path == 'foo/bar/baz' - - -def test_split_storage(): - def simple_getenv(key): - return None - - with patch('osfclient.cli.os.getenv', - side_effect=simple_getenv) as mock_getenv: - store, path = split_storage('osfstorage/foo/bar/baz') - assert store == 'osfstorage' - assert path == 'foo/bar/baz' - - store, path = split_storage('github/foo/bar/baz') - assert store == 'github' - assert path == 'foo/bar/baz' - - store, path = split_storage('/github/foo/bar/baz') - assert store == 'github' - assert path == 'foo/bar/baz' +from osfclient.tests.mocks import AsyncIterator, MockAddon, MockStream - store, path = split_storage('figshare/foo/bar/baz') - assert store == 'figshare' - assert path == 'foo/bar/baz' - - store, path = split_storage('/figshare/foo/bar/baz') - assert store == 'figshare' - assert path == 'foo/bar/baz' - - store, path = split_storage('googledrive/foo/bar/baz') - assert store == 'googledrive' - assert path == 'foo/bar/baz' - - store, path = split_storage('/googledrive/foo/bar/baz') - assert store == 'googledrive' - assert path == 'foo/bar/baz' - - store, path = split_storage('osfstorage/foo/bar/baz/') - assert store == 'osfstorage' - assert path == 'foo/bar/baz' - - store, path = split_storage('osfstorage/foo/bar/baz/', normalize=False) - assert store == 'osfstorage' - assert path == 'foo/bar/baz/' +@pytest.mark.asyncio +@pytest.mark.parametrize('remote, normalize, provider, path', [ + ('folder/file.txt', True, 'osfstorage', 'folder/file.txt'), + ('/folder/file.txt', True, 'osfstorage', 'folder/file.txt'), + ('custom/files/data.txt', True, 'custom', 'files/data.txt'), + ('/custom/files/data.txt', True, 'custom', 'files/data.txt'), + ('./custom/files/data.txt', True, 'custom', 'files/data.txt'), + ('custom/files/../data.txt', True, 'custom', 'data.txt'), + ('/custom//data.txt', True, 'custom', 'data.txt'), + ('custom/files/', True, 'custom', 'files'), + ('/custom/files/', False, 'custom', 'files/'), + ('custom/', True, 'custom', ''), + ('custom', True, 'osfstorage', 'custom'), + ('/custom/', False, 'custom', ''), + ('osfstorage/custom/file.txt', True, 'osfstorage', 'custom/file.txt'), + ('github/file.txt', True, 'osfstorage', 'github/file.txt'), + ('custom-other/file.txt', True, 'osfstorage', 'custom-other/file.txt'), + ('unconnected', True, 'osfstorage', 'unconnected'), + ('osfstorage/unconnected/file.txt', True, 'osfstorage', + 'unconnected/file.txt'), + ('binderhub/notebook.ipynb', True, 'osfstorage', 'binderhub/notebook.ipynb'), +]) +async def test_split_storage(remote, normalize, provider, path, monkeypatch): + # Obsolete environment overrides must not hide connected providers. + monkeypatch.setenv('KNOWN_PROVIDERS', 'github') + stores = [SimpleNamespace(provider=name, name='Display name') + for name in ['osfstorage', 'custom']] + project = SimpleNamespace(id='1234', storages=AsyncIterator(stores)) + osf = SimpleNamespace(addons=AsyncIterator([ + MockAddon('custom'), MockAddon('unconnected'), + MockAddon('binderhub', categories=['other'])])) + store, actual_path = await split_storage(remote, osf, project, + normalize=normalize) + assert store is stores[['osfstorage', 'custom'].index(provider)] + assert actual_path == path -def test_custom_split_storage(): - def simple_getenv(key): - if key == 'KNOWN_PROVIDERS': - return 'osfstorage,s3,github' - return None - with patch('osfclient.cli.os.getenv', - side_effect=simple_getenv) as mock_getenv: - store, path = split_storage('osfstorage/foo/bar/baz') - assert store == 'osfstorage' - assert path == 'foo/bar/baz' +@pytest.mark.asyncio +async def test_split_storage_connected_provider_skips_addons(): + stores = [SimpleNamespace(provider=name) for name in ['osfstorage', 'custom']] + project = SimpleNamespace(id='1234', storages=AsyncIterator(stores)) + osf = SimpleNamespace(addons=AsyncIterator([MockAddon('custom')])) - store, path = split_storage('github/foo/bar/baz') - assert store == 'github' - assert path == 'foo/bar/baz' + store, path = await split_storage('custom/file.txt', osf, project) - store, path = split_storage('/github/foo/bar/baz') - assert store == 'github' - assert path == 'foo/bar/baz' + assert store is stores[1] + assert path == 'file.txt' + assert not osf.addons.__aiter__.called - store, path = split_storage('figshare/foo/bar/baz') - assert store == 'osfstorage' - assert path == 'figshare/foo/bar/baz' - store, path = split_storage('/figshare/foo/bar/baz') - assert store == 'osfstorage' - assert path == 'figshare/foo/bar/baz' +@pytest.mark.asyncio +@pytest.mark.parametrize('remote, normalize', [ + ('unconnected/file.txt', True), + ('/unconnected/file.txt', True), + ('unconnected/', True), + ('/unconnected/', False), +]) +async def test_split_storage_unconnected_provider(remote, normalize): + stores = [SimpleNamespace(provider='osfstorage')] + project = SimpleNamespace(id='1234', storages=AsyncIterator(stores)) + osf = SimpleNamespace(addons=AsyncIterator([ + MockAddon('custom'), MockAddon('unconnected', categories=['citations'])])) + + with pytest.raises(RuntimeError, match=( + "Storage provider 'unconnected' is not connected to project '1234'")): + await split_storage(remote, osf, project, normalize=normalize) - store, path = split_storage('s3/foo/bar/baz') - assert store == 's3' - assert path == 'foo/bar/baz' - store, path = split_storage('/s3/foo/bar/baz') - assert store == 's3' - assert path == 'foo/bar/baz' +@pytest.mark.asyncio +async def test_split_storage_missing_default(): + project = SimpleNamespace(id='1234', storages=AsyncIterator([])) + osf = SimpleNamespace(addons=AsyncIterator([])) + with pytest.raises(RuntimeError, match="no storage provider 'osfstorage'"): + await split_storage('folder/file.txt', osf, project) def test_norm_remote_path(): diff --git a/osfclient/utils.py b/osfclient/utils.py index ba195c7..4d8e5ff 100644 --- a/osfclient/utils.py +++ b/osfclient/utils.py @@ -9,15 +9,6 @@ import aiofiles -KNOWN_PROVIDERS = [ - 'osfstorage', 'github', 'figshare', 'googledrive', - 'azureblobstorage', 'bitbucket', 'box', 'dataverse', 'dropbox', - 'gitlab', 'iqbrims', 'nextcloud', 'nextcloudinstitutions', - 'ociinstitutions', 'owncloud', 'onedrivebusiness', 's3', 's3compat', - 's3compatb3', 's3compatinstitutions', 'swift', 'weko' -] - - def norm_remote_path(path: str) -> str: """Normalize `path`. @@ -30,29 +21,40 @@ def norm_remote_path(path: str) -> str: return path -def split_storage(path, default='osfstorage', normalize=True): - """Extract storage name from file path. +async def split_storage(path, osf, project, default='osfstorage', + normalize=True): + """Resolve a remote path against the project's connected storages. - If a path begins with a known storage provider the name is removed - from the path. Otherwise the `default` storage provider is returned - and the path is not modified. + A connected provider at the start of the path selects that storage. + A provider the server offers but the project has not connected is an + error rather than a folder in the default storage. Addons in the + ``other`` category never hold files, so their names stay usable as + folder names. Otherwise the entire path belongs to the default storage. + Explicitly prefix with ``osfstorage/`` to access a folder named after + a provider. """ + is_directory = path.endswith('/') if normalize: path = norm_remote_path(path) - env_known_providers = os.getenv('KNOWN_PROVIDERS') - if env_known_providers is not None: - known_providers = env_known_providers.split(',') - else: - known_providers = KNOWN_PROVIDERS - - for provider in known_providers: - if path.startswith(provider + '/'): - if six.PY3: - return path.split('/', maxsplit=1) - else: - return path.split('/', 1) - - return (default, path) + path = path.lstrip('/') + provider, separator, remote_path = path.partition('/') + has_provider = bool(separator) or is_directory + default_store = None + async for store in project.storages: + if has_provider and store.provider == provider: + return store, remote_path + if store.provider == default: + default_store = store + + if default_store is None: + raise RuntimeError("Project has no storage provider '{}'".format(default)) + if has_provider: + async for addon in osf.addons: + if addon.id == provider and 'other' not in addon.categories: + raise RuntimeError( + "Storage provider '{}' is not connected to project '{}'" + .format(provider, project.id)) + return default_store, path def makedirs(path, mode=511, exist_ok=False):