Merge pull request #78 from betatim/folder_upload

[MRG] Recursively upload files

osfclient/__main__.py

@@ -69,12 +69,15 @@ def _add_subparser(name, description, aliases=[]):
     list_parser = _add_subparser('list', list.__doc__, aliases=['ls'])
     list_parser.set_defaults(func=list_)
 
-    # Upload a single file
+    # Upload a single file or a directory tree
     upload_parser = _add_subparser('upload', upload.__doc__)
     upload_parser.set_defaults(func=upload)
     upload_parser.add_argument('-f', '--force',
                                help='Force overwriting of remote file',
                                action='store_true')
+    upload_parser.add_argument('-r', '--recursive',
+                               help='Recursively upload entire directories.',
+                               action='store_true')
     upload_parser.add_argument('source', help='Local file')
     upload_parser.add_argument('destination', help='Remote file path')
 

osfclient/cli.py

@@ -236,19 +236,54 @@ def upload(args):
     used.
 
     If the project is private you need to specify a username.
+
+    To upload a whole directory (and all its sub-directories) use the `-r`
+    command-line option. If your source directory name ends in a / then
+    files will be created directly in the remote directory. If it does not
+    end in a slash an extra sub-directory with the name of the local directory
+    will be created.
+
+    To place contents of local directory `foo` in remote directory `bar/foo`:
+    $ osf upload -r foo bar
+    To place contents of local directory `foo` in remote directory `bar`:
+    $ osf upload -r foo/ bar
     """
     osf = _setup_osf(args)
     if osf.username is None or osf.password is None:
         sys.exit('To upload a file you need to provide a username and'
                  ' password.')
 
     project = osf.project(args.project)
-
     storage, remote_path = split_storage(args.destination)
 
     store = project.storage(storage)
-    with open(args.source, 'rb') as fp:
-        store.create_file(remote_path, fp, update=args.force)
+    if args.recursive:
+        if not os.path.isdir(args.source):
+            raise RuntimeError("Expected source ({}) to be a directory when "
+                               "using recursive mode.".format(args.source))
+
+        # local name of the directory that is being uploaded
+        _, dir_name = os.path.split(args.source)
+
+        for root, _, files in os.walk(args.source):
+            # these are extra subdirectories we have walked into since the root
+            # directory, have to clean off leading slashes from their name
+            # for path.join() to work later on
+            subdir_path = root.replace(args.source, '')
+            if subdir_path.startswith('/'):
+                subdir_path = subdir_path[1:]
+
+            for fname in files:
+                local_path = os.path.join(root, fname)
+                with open(local_path, 'rb') as fp:
+                    # build the remote path + fname
+                    name = os.path.join(remote_path, dir_name, subdir_path,
+                                        fname)
+                    store.create_file(name, fp, update=args.force)
+
+    else:
+        with open(args.source, 'rb') as fp:
+            store.create_file(remote_path, fp, update=args.force)
 
 
 @might_need_auth

osfclient/tests/mocks.py

@@ -45,9 +45,10 @@ def MockProject(name):
 
 def MockArgs(username=None, password=None, output=None, project=None,
              source=None, destination=None, local=None, remote=None,
-             target=None, force=False):
+             target=None, force=False, recursive=False):
     args = MagicMock(spec=['username', 'password', 'output', 'project',
-                           'source', 'destination', 'target', 'force'])
+                           'source', 'destination', 'target', 'force',
+                           'recursive'])
     args._username_mock = PropertyMock(return_value=username)
     type(args).username = args._username_mock
     args._password_mock = PropertyMock(return_value=password)
@@ -74,6 +75,9 @@ def MockArgs(username=None, password=None, output=None, project=None,
     args._force_mock = PropertyMock(return_value=force)
     type(args).force = args._force_mock
 
+    args._recursive_mock = PropertyMock(return_value=recursive)
+    type(args).recursive = args._recursive_mock
+
     return args
 
 

osfclient/tests/test_uploading.py

@@ -1,5 +1,6 @@
 """Test `osf upload` command"""
 
+import mock
 from mock import call
 from mock import patch
 from mock import mock_open
@@ -8,11 +9,9 @@
 
 from osfclient import OSF
 from osfclient.cli import upload
-from osfclient.models import Project
 
 from osfclient.tests.mocks import MockArgs
 from osfclient.tests.mocks import MockProject
-from osfclient.tests.mocks import MockStorage
 
 
 def test_anonymous_doesnt_work():
@@ -26,8 +25,7 @@ def test_anonymous_doesnt_work():
 
 
 @patch.object(OSF, 'project', return_value=MockProject('1234'))
-@patch.object(Project, 'storage', return_value=MockStorage('osfstorage'))
-def test_select_project(Project_storage, OSF_project):
+def test_select_project(OSF_project):
     args = MockArgs(username='joe@example.com',
                     project='1234',
                     source='foo/bar.txt',
@@ -56,3 +54,118 @@ def simple_getenv(key):
     # we should call the create_file method on the return
     # value of _storage_mock
     assert fake_project._storage_mock.return_value.mock_calls == expected
+
+
+@patch.object(OSF, 'project', return_value=MockProject('1234'))
+def test_recursive_requires_directory(OSF_project):
+    # test that we check if source is a directory when using recursive mode
+    args = MockArgs(username='joe@example.com',
+                    project='1234',
+                    source='foo/bar.txt',
+                    recursive=True,
+                    destination='bar/bar/foo.txt')
+
+    def simple_getenv(key):
+        if key == 'OSF_PASSWORD':
+            return 'secret'
+
+    with pytest.raises(RuntimeError) as e:
+        with patch('osfclient.cli.os.getenv', side_effect=simple_getenv):
+            with patch('osfclient.cli.os.path.isdir', return_value=False):
+                upload(args)
+
+    assert 'recursive' in str(e.value)
+    assert 'Expected source (foo/bar.txt)' in str(e.value)
+
+
+@patch.object(OSF, 'project', return_value=MockProject('1234'))
+def test_recursive_upload(OSF_project):
+    # test that we check if source is a directory when using recursive mode
+    args = MockArgs(username='joe@example.com',
+                    project='1234',
+                    source='foobar/',
+                    recursive=True,
+                    destination='BAR/')
+
+    def simple_getenv(key):
+        if key == 'OSF_PASSWORD':
+            return 'secret'
+
+    fake_open = mock_open()
+    fake_storage = OSF_project.return_value.storage.return_value
+
+    # it is important we use foobar/ and not foobar to match with args.source
+    # to mimick the behaviour of os.walk()
+    dir_contents = [('foobar/', None, ['bar.txt', 'abc.txt']),
+                    ('foobar/baz', None, ['bar.txt', 'abc.txt'])
+                    ]
+
+    with patch('osfclient.cli.open', fake_open):
+        with patch('os.walk', return_value=iter(dir_contents)):
+            with patch('osfclient.cli.os.getenv', side_effect=simple_getenv):
+                with patch('osfclient.cli.os.path.isdir', return_value=True):
+                    upload(args)
+
+    assert call('foobar/bar.txt', 'rb') in fake_open.mock_calls
+    assert call('foobar/abc.txt', 'rb') in fake_open.mock_calls
+    assert call('foobar/baz/bar.txt', 'rb') in fake_open.mock_calls
+    assert call('foobar/baz/abc.txt', 'rb') in fake_open.mock_calls
+    # two directories with two files each -> four calls plus all the
+    # context manager __enter__ and __exit__ calls
+    assert len(fake_open.mock_calls) == 4 + 4*2
+
+    fake_storage.assert_has_calls([
+        call.create_file('BAR/bar.txt', mock.ANY, update=False),
+        call.create_file('BAR/abc.txt', mock.ANY, update=False),
+        call.create_file('BAR/baz/bar.txt', mock.ANY, update=False),
+        call.create_file('BAR/baz/abc.txt', mock.ANY, update=False)
+        ])
+    # two directories with two files each -> four calls
+    assert len(fake_storage.mock_calls) == 4
+
+
+@patch.object(OSF, 'project', return_value=MockProject('1234'))
+def test_recursive_upload_with_subdir(OSF_project):
+    # test that an extra level of subdirectory is created on the remote side
+    # this is because args.source does not end in a /
+    args = MockArgs(username='joe@example.com',
+                    project='1234',
+                    source='foobar',
+                    recursive=True,
+                    destination='BAR/')
+
+    def simple_getenv(key):
+        if key == 'OSF_PASSWORD':
+            return 'secret'
+
+    fake_open = mock_open()
+    fake_storage = OSF_project.return_value.storage.return_value
+
+    # it is important we use foobar and not foobar/ to match with args.source
+    # to mimick the behaviour of os.walk()
+    dir_contents = [('foobar', None, ['bar.txt', 'abc.txt']),
+                    ('foobar/baz', None, ['bar.txt', 'abc.txt'])
+                    ]
+
+    with patch('osfclient.cli.open', fake_open):
+        with patch('os.walk', return_value=iter(dir_contents)):
+            with patch('osfclient.cli.os.getenv', side_effect=simple_getenv):
+                with patch('osfclient.cli.os.path.isdir', return_value=True):
+                    upload(args)
+
+    assert call('foobar/bar.txt', 'rb') in fake_open.mock_calls
+    assert call('foobar/abc.txt', 'rb') in fake_open.mock_calls
+    assert call('foobar/baz/bar.txt', 'rb') in fake_open.mock_calls
+    assert call('foobar/baz/abc.txt', 'rb') in fake_open.mock_calls
+    # two directories with two files each -> four calls plus all the
+    # context manager __enter__ and __exit__ calls
+    assert len(fake_open.mock_calls) == 4 + 4*2
+
+    fake_storage.assert_has_calls([
+        call.create_file('BAR/foobar/bar.txt', mock.ANY, update=False),
+        call.create_file('BAR/foobar/abc.txt', mock.ANY, update=False),
+        call.create_file('BAR/foobar/baz/bar.txt', mock.ANY, update=False),
+        call.create_file('BAR/foobar/baz/abc.txt', mock.ANY, update=False)
+        ])
+    # two directories with two files each -> four calls
+    assert len(fake_storage.mock_calls) == 4