Repository: Duke-GCB/DukeDSClient

Issue:
Make use of DukeDS user filtering improvements
DukeDS API has been updated to allow filtering users by email and/or username: https://github.com/Duke-Translational-Bioinformatics/duke-data-service/issues/1103

This will speed up how long it takes to list users.
NOTE: This is new feature currently deployed to DukeDS dev API.

Discussion:

Patch:
diff --git a/ddsc/core/ddsapi.py b/ddsc/core/ddsapi.py
index c026b27..71710f4 100644
--- a/ddsc/core/ddsapi.py
+++ b/ddsc/core/ddsapi.py
@@ -609,12 +609,21 @@ class DataServiceApi(object):
         }
         return self._get_collection('/users', data)
 
-    def get_all_users(self):
+    def get_users(self, full_name=None, email=None, username=None):
         """
-        Send GET request to /users for all users.
+        Send GET request to /users for users with optional full_name, email, and/or username filtering.
+        :param full_name: str name of the user we are searching for
+        :param email: str: optional email to filter by
+        :param username: str: optional username to filter by
         :return: requests.Response containing the successful result
         """
         data = {}
+        if full_name:
+            data['full_name_contains'] = full_name
+        if email:
+            data['email'] = email
+        if username:
+            data['username'] = username
         return self._get_collection('/users', data)
 
     def get_user_by_id(self, id):
@@ -947,6 +956,26 @@ class DataServiceApi(object):
         """
         return self._get_collection("/auth_providers", {})
 
+    def get_auth_provider(self, auth_provider_id):
+        """
+        Get auth provider details.
+        :param auth_provider_id: str: uuid of the auth provider
+        :return: requests.Response containing the successful result
+        """
+        return self._get_single_item('/auth_providers/{}/'.format(auth_provider_id), {})
+
+    def get_auth_provider_affiliates(self, auth_provider_id, full_name_contains):
+        """
+        List affiliates for a specific auth provider.
+        :param auth_provider_id: str: uuid of the auth provider to list affiliates of
+        :param full_name_contains: str: filters affiliates for this name
+        :return: requests.Response containing the successful result
+        """
+        data = {
+            'full_name_contains': full_name_contains
+        }
+        return self._get_collection("/auth_providers/{}/affiliates/".format(auth_provider_id), data)
+
     def auth_provider_add_user(self, auth_provider_id, username):
         """
         Transform an institutional affiliates UID, such as a Duke NetID, to a DDS specific user identity;


Test patch:
diff --git a/ddsc/core/remotestore.py b/ddsc/core/remotestore.py
index 178169d..35454bc 100644
--- a/ddsc/core/remotestore.py
+++ b/ddsc/core/remotestore.py
@@ -122,7 +122,7 @@ class RemoteStore(object):
         :param username: str username we are looking for
         :return: RemoteUser: user we found
         """
-        matches = [user for user in self.fetch_all_users() if user.username == username]
+        matches = self.fetch_users(username=username)
         if not matches:
             raise NotFoundError('Username not found: {}.'.format(username))
         if len(matches) > 1:
@@ -180,9 +180,9 @@ class RemoteStore(object):
         :param email: str email we are looking for
         :return: RemoteUser user we found
         """
-        matches = [user for user in self.fetch_all_users() if user.email == email]
+        matches = self.fetch_users(email=email)
         if not matches:
-            raise ValueError('Email not found: {}.'.format(email))
+            raise NotFoundError('Email not found: {}.'.format(email))
         if len(matches) > 1:
             raise ValueError('Multiple users with same email found: {}.'.format(email))
         return matches[0]
@@ -195,13 +195,15 @@ class RemoteStore(object):
         response = self.data_service.get_current_user().json()
         return RemoteUser(response)
 
-    def fetch_all_users(self):
+    def fetch_users(self, email=None, username=None):
         """
-        Retrieves all users from data service.
+        Retrieves users with optional email and/or username filtering from data service.
+        :param email: str: optional email to filter by
+        :param username: str: optional username to filter by
         :return: [RemoteUser] list of all users we downloaded
         """
         users = []
-        result = self.data_service.get_all_users()
+        result = self.data_service.get_users(email=email, username=username)
         user_list_json = result.json()
         for user_json in user_list_json['results']:
             users.append(RemoteUser(user_json))
diff --git a/ddsc/core/tests/test_ddsapi.py b/ddsc/core/tests/test_ddsapi.py
index 08810ce..2f4a471 100644
--- a/ddsc/core/tests/test_ddsapi.py
+++ b/ddsc/core/tests/test_ddsapi.py
@@ -5,7 +5,7 @@ from ddsc.core.ddsapi import MultiJSONResponse, DataServiceApi, DataServiceAuth,
 from ddsc.core.ddsapi import MissingInitialSetupError, SoftwareAgentNotFoundError, AuthTokenCreationError, \
     UnexpectedPagingReceivedError, DataServiceError, DSResourceNotConsistentError, \
     retry_until_resource_is_consistent, retry_when_service_down
-from mock import MagicMock, Mock, patch
+from mock import MagicMock, Mock, patch, ANY
 
 
 def fake_response_with_pages(status_code, json_return_value, num_pages=1):
@@ -234,6 +234,41 @@ class TestDataServiceApi(TestCase):
         self.assertEqual(json_results, result.json())
         self.assertEqual('something.com/v1/auth_providers', mock_requests.get.call_args_list[0][0][0])
 
+    def test_get_auth_provider(self):
+        mock_requests = MagicMock()
+        mock_requests.get.side_effect = [
+            fake_response(status_code=200, json_return_value={"ok": True})
+        ]
+        api = DataServiceApi(auth=self.create_mock_auth(config_page_size=100), url="something.com/v1",
+                             http=mock_requests)
+        result = api.get_auth_provider('provider_id_123')
+        self.assertEqual(200, result.status_code)
+        mock_requests.get.assert_called_with('something.com/v1/auth_providers/provider_id_123/',
+                                             headers=ANY, params=ANY)
+
+    def test_get_auth_provider_affiliates(self):
+        user = {
+            "id": "abc4e9-9987-47eb-bb4e-19f0203efbf6",
+            "username": "joe",
+        }
+        json_results = {
+            "results": [
+                user
+            ]
+        }
+        mock_requests = MagicMock()
+        mock_requests.get.side_effect = [
+            fake_response_with_pages(status_code=200, json_return_value=json_results, num_pages=1)
+        ]
+        api = DataServiceApi(auth=self.create_mock_auth(config_page_size=100), url="something.com/v1",
+                             http=mock_requests)
+        result = api.get_auth_provider_affiliates('provider_id_123', 'Joe Smith')
+        self.assertEqual(200, result.status_code)
+        self.assertEqual(json_results, result.json())
+        mock_requests.get.assert_called_with('something.com/v1/auth_providers/provider_id_123/affiliates/',
+                                             headers=ANY,
+                                             params={'full_name_contains': 'Joe Smith', 'page': 1, 'per_page': 100})
+
     def test_auth_provider_add_user(self):
         user = {
             "id": "abc4e9-9987-47eb-bb4e-19f0203efbf6",
@@ -560,6 +595,52 @@ class TestDataServiceApi(TestCase):
         self.assertEqual(results[0]['user']['id'], '8593aeac-9999-11e8-9eb6-529269fb1459')
         self.assertEqual(results[0]['auth_role']['id'], 'project_admin')
 
+    def test_get_users_no_filtering(self):
+        mock_requests = MagicMock()
+        page1 = {
+            "results": [
+                {
+                    "id": "8593aeac-9999-11e8-9eb6-529269fb1459"
+                }
+            ]
+        }
+        mock_requests.get.side_effect = [
+            fake_response_with_pages(status_code=200, json_return_value=page1, num_pages=1),
+        ]
+        api = DataServiceApi(auth=self.create_mock_auth(config_page_size=100), url="something.com/v1",
+                             http=mock_requests)
+        response = api.get_users()
+
+        mock_requests.get.assert_called_with('something.com/v1/users', headers=ANY, params={'page': 1, 'per_page': 100})
+
+        results = response.json()['results']
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0]['id'], '8593aeac-9999-11e8-9eb6-529269fb1459')
+
+    def test_get_users_with_filtering(self):
+        mock_requests = MagicMock()
+        page1 = {
+            "results": [
+                {
+                    "id": "8593aeac-9999-11e8-9eb6-529269fb1459"
+                }
+            ]
+        }
+        mock_requests.get.side_effect = [
+            fake_response_with_pages(status_code=200, json_return_value=page1, num_pages=1),
+        ]
+        api = DataServiceApi(auth=self.create_mock_auth(config_page_size=100), url="something.com/v1",
+                             http=mock_requests)
+        response = api.get_users(full_name='Joe Bob', email='joe@joe.com', username='joe')
+
+        mock_requests.get.assert_called_with('something.com/v1/users', headers=ANY,
+                                             params={'full_name_contains': 'Joe Bob', 'email': 'joe@joe.com', 'username': 'joe',
+                                                     'page': 1, 'per_page': 100})
+
+        results = response.json()['results']
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0]['id'], '8593aeac-9999-11e8-9eb6-529269fb1459')
+
 
 class TestDataServiceAuth(TestCase):
     @patch('ddsc.core.ddsapi.get_user_agent_str')
diff --git a/ddsc/core/tests/test_localstore.py b/ddsc/core/tests/test_localstore.py
index 0f6386b..f7ef640 100644
--- a/ddsc/core/tests/test_localstore.py
+++ b/ddsc/core/tests/test_localstore.py
@@ -1,6 +1,6 @@
 import shutil
 import tarfile
-from unittest import TestCase
+from unittest import TestCase, skip
 from ddsc.core.localstore import LocalFile, LocalFolder, LocalProject
 from mock import patch
 
@@ -99,6 +99,7 @@ class TestProjectContent(TestCase):
         content.add_path('/tmp/DukeDsClientTestFolder/scripts')
         self.assertEqual('project: [folder:scripts [file:makemoney.sh]]', str(content))
 
+    @skip(reason="Fragile test breaks due to item sorting differences on travis")
     def test_nested_folder_str(self):
         content = LocalProject(False, file_exclude_regex=INCLUDE_ALL)
         content.add_path('/tmp/DukeDsClientTestFolder/results')
@@ -109,6 +110,7 @@ class TestProjectContent(TestCase):
                           'folder:subresults2 []'
                           ']]'), str(content))
 
+    @skip(reason="Fragile test breaks due to item sorting differences on travis")
     def test_big_folder_str(self):
         content = LocalProject(False, file_exclude_regex=INCLUDE_ALL)
         content.add_path('/tmp/DukeDsClientTestFolder')
diff --git a/ddsc/core/tests/test_remotestore.py b/ddsc/core/tests/test_remotestore.py
index d894537..e84512c 100644
--- a/ddsc/core/tests/test_remotestore.py
+++ b/ddsc/core/tests/test_remotestore.py
@@ -9,7 +9,7 @@ from ddsc.core.remotestore import RemoteAuthRole
 from ddsc.core.remotestore import RemoteProjectChildren
 from ddsc.core.remotestore import RemoteAuthProvider
 from ddsc.core.remotestore import ProjectNameOrId
-from ddsc.core.remotestore import ProjectFile, RemoteFileUrl
+from ddsc.core.remotestore import ProjectFile, RemoteFileUrl, NotFoundError
 
 
 class TestProjectFolderFile(TestCase):
@@ -515,6 +515,88 @@ class TestRemoteStore(TestCase):
         remote_store = RemoteStore(config=mock_config)
         self.assertTrue(mock_data_service_api.called)
 
+    @patch("ddsc.core.remotestore.DataServiceApi")
+    @patch("ddsc.core.remotestore.RemoteUser")
+    def test_fetch_users_no_filter(self, mock_remote_user, mock_data_service_api):
+        user_dict = {
+            'id': '123',
+        }
+        users_resp = Mock()
+        users_resp.json.return_value = {
+            'results': [
+                user_dict,
+            ]
+        }
+        mock_data_service_api.return_value.get_users.return_value = users_resp
+
+        remote_store = RemoteStore(config=MagicMock())
+        users = remote_store.fetch_users()
+
+        mock_data_service_api.return_value.get_users.assert_called_with(email=None, username=None)
+        self.assertEqual(len(users), 1)
+        self.assertEqual(users[0], mock_remote_user.return_value)
+        mock_remote_user.assert_called_with(user_dict)
+
+    @patch("ddsc.core.remotestore.DataServiceApi")
+    @patch("ddsc.core.remotestore.RemoteUser")
+    def test_fetch_users_with_filter(self, mock_remote_user, mock_data_service_api):
+        user_dict = {
+            'id': '123',
+        }
+        users_resp = Mock()
+        users_resp.json.return_value = {
+            'results': [
+                user_dict,
+            ]
+        }
+        mock_data_service_api.return_value.get_users.return_value = users_resp
+
+        remote_store = RemoteStore(config=MagicMock())
+        users = remote_store.fetch_users(email='joe@joe.com', username='joe')
+
+        mock_data_service_api.return_value.get_users.assert_called_with(email='joe@joe.com', username='joe')
+        self.assertEqual(len(users), 1)
+        self.assertEqual(users[0], mock_remote_user.return_value)
+        mock_remote_user.assert_called_with(user_dict)
+
+    def test_lookup_user_by_username(self):
+        mock_user = Mock()
+        mock_user2 = Mock()
+        remote_store = RemoteStore(config=MagicMock())
+        remote_store.fetch_users = Mock()
+
+        remote_store.fetch_users.return_value = []
+        with self.assertRaises(NotFoundError) as raised_exception:
+            remote_store.lookup_user_by_username('joe')
+        self.assertEqual(str(raised_exception.exception), 'Username not found: joe.')
+
+        remote_store.fetch_users.return_value = [mock_user]
+        self.assertEqual(remote_store.lookup_user_by_username('joe'), mock_user)
+
+        remote_store.fetch_users.return_value = [mock_user, mock_user2]
+        with self.assertRaises(ValueError) as raised_exception:
+            remote_store.lookup_user_by_username('joe')
+        self.assertEqual(str(raised_exception.exception), 'Multiple users with same username found: joe.')
+
+    def test_lookup_user_by_email(self):
+        mock_user = Mock()
+        mock_user2 = Mock()
+        remote_store = RemoteStore(config=MagicMock())
+        remote_store.fetch_users = Mock()
+
+        remote_store.fetch_users.return_value = []
+        with self.assertRaises(NotFoundError) as raised_exception:
+            remote_store.lookup_user_by_email('joe@joe.com')
+        self.assertEqual(str(raised_exception.exception), 'Email not found: joe@joe.com.')
+
+        remote_store.fetch_users.return_value = [mock_user]
+        self.assertEqual(remote_store.lookup_user_by_email('joe@joe.com'), mock_user)
+
+        remote_store.fetch_users.return_value = [mock_user, mock_user2]
+        with self.assertRaises(ValueError) as raised_exception:
+            remote_store.lookup_user_by_email('joe@joe.com')
+        self.assertEqual(str(raised_exception.exception), 'Multiple users with same email found: joe@joe.com.')
+
 
 class TestRemoteProjectChildren(TestCase):
     def test_simple_case(self):