Repository: Duke-GCB/DukeDSClient

Issue:
DDSClient is creating multiple projects with the same name
Email report from David Corcoran:

DDS client is currently creating multiple projects with the same name.  Can you please make it so that there is an error when this is attempted? 
 
I also got the occasional error (see attached).  It happened for a handful of the projects.
 
![dds_error](https://user-images.githubusercontent.com/2752599/27604649-e98c3cf8-5b47-11e7-886c-c69607426f58.png)


Discussion:

Patch:
diff --git a/ddsc/core/ddsapi.py b/ddsc/core/ddsapi.py
index b02dc6b..1633145 100644
--- a/ddsc/core/ddsapi.py
+++ b/ddsc/core/ddsapi.py
@@ -238,41 +238,40 @@ class DataServiceApi(object):
         resp = self.http.get(url, headers=headers, params=data_str)
         return self._check_err(resp, url_suffix, data, allow_pagination=False)
 
-    def _get_single_page(self, url_suffix, data, content_type, page_num):
+    def _get_single_page(self, url_suffix, data, page_num):
         """
         Send GET request to API at url_suffix with post_data adding page and per_page parameters to
         retrieve a single page. Always requests with per_page=DEFAULT_RESULTS_PER_PAGE.
         :param url_suffix: str URL path we are sending a GET to
         :param data: object data we are sending
-        :param content_type: str from ContentType that determines how we format the data
         :param page_num: int: page number to fetch
         :return: requests.Response containing the result
         """
         data_with_per_page = dict(data)
         data_with_per_page['page'] = page_num
         data_with_per_page['per_page'] = DEFAULT_RESULTS_PER_PAGE
-        (url, data_str, headers) = self._url_parts(url_suffix, data_with_per_page, content_type=content_type)
+        (url, data_str, headers) = self._url_parts(url_suffix, data_with_per_page,
+                                                   content_type=ContentType.form)
         resp = self.http.get(url, headers=headers, params=data_str)
         return self._check_err(resp, url_suffix, data, allow_pagination=True)
 
-    def _get_collection(self, url_suffix, data, content_type=ContentType.json):
+    def _get_collection(self, url_suffix, data):
         """
         Performs GET for all pages based on x-total-pages in first response headers.
         Merges the json() 'results' arrays.
         If x-total-pages is missing or 1 just returns the response without fetching multiple pages.
         :param url_suffix: str URL path we are sending a GET to
         :param data: object data we are sending
-        :param content_type: str from ContentType that determines how we format the data
         :return: requests.Response containing the result
         """
-        response = self._get_single_page(url_suffix, data, content_type, page_num=1)
+        response = self._get_single_page(url_suffix, data, page_num=1)
         total_pages_str = response.headers.get('x-total-pages')
         if total_pages_str:
             total_pages = int(total_pages_str)
             if total_pages > 1:
                 multi_response = MultiJSONResponse(base_response=response, merge_array_field_name="results")
                 for page in range(2, total_pages + 1):
-                    additional_response = self._get_single_page(url_suffix, data, content_type, page_num=page)
+                    additional_response = self._get_single_page(url_suffix, data, page_num=page)
                     multi_response.add_response(additional_response)
                 return multi_response
         return response
@@ -393,7 +392,7 @@ class DataServiceApi(object):
         if name_contains is not None:
             data['name_contains'] = name_contains
         url_prefix = "/{}/{}/children".format(parent_name, parent_id)
-        return self._get_collection(url_prefix, data, content_type=ContentType.form)
+        return self._get_collection(url_prefix, data)
 
     def create_upload(self, project_id, filename, content_type, size,
                       hash_value, hash_alg):
@@ -525,7 +524,7 @@ class DataServiceApi(object):
         data = {
             "full_name_contains": full_name,
         }
-        return self._get_collection('/users', data, content_type=ContentType.form)
+        return self._get_collection('/users', data)
 
     def get_all_users(self):
         """
@@ -533,7 +532,7 @@ class DataServiceApi(object):
         :return: requests.Response containing the successful result
         """
         data = {}
-        return self._get_collection('/users', data, content_type=ContentType.form)
+        return self._get_collection('/users', data)
 
     def get_user_by_id(self, id):
         """
@@ -620,7 +619,7 @@ class DataServiceApi(object):
         :param context: str which roles do we want 'project' or 'system'
         :return: requests.Response containing the successful result
         """
-        return self._get_collection("/auth_roles", {"context": context}, content_type=ContentType.form)
+        return self._get_collection("/auth_roles", {"context": context})
 
     def get_project_transfers(self, project_id):
         """


Test patch:
diff --git a/ddsc/core/tests/test_ddsapi.py b/ddsc/core/tests/test_ddsapi.py
index 1715d3a..87ec866 100644
--- a/ddsc/core/tests/test_ddsapi.py
+++ b/ddsc/core/tests/test_ddsapi.py
@@ -1,7 +1,7 @@
 from __future__ import absolute_import
 from unittest import TestCase
 import requests
-from ddsc.core.ddsapi import MultiJSONResponse, DataServiceApi, ContentType, UNEXPECTED_PAGING_DATA_RECEIVED
+from ddsc.core.ddsapi import MultiJSONResponse, DataServiceApi, UNEXPECTED_PAGING_DATA_RECEIVED
 from mock import MagicMock
 
 
@@ -55,7 +55,7 @@ class TestDataServiceApi(TestCase):
                                      json_return_value={"results": [1, 2, 3]},
                                      num_pages=1)]
         api = DataServiceApi(auth=None, url="something.com/v1/", http=mock_requests)
-        response = api._get_collection(url_suffix="users", data={}, content_type=ContentType.json)
+        response = api._get_collection(url_suffix="users", data={})
         self.assertEqual([1, 2, 3], response.json()["results"])
         call_args_list = mock_requests.get.call_args_list
         self.assertEqual(1, len(call_args_list))
@@ -64,9 +64,10 @@ class TestDataServiceApi(TestCase):
         first_param = call_args[0][0]
         self.assertEqual('something.com/v1/users', first_param)
         dict_param = call_args[1]
-        self.assertEqual('application/json', dict_param['headers']['Content-Type'])
+        self.assertEqual('application/x-www-form-urlencoded', dict_param['headers']['Content-Type'])
         self.assertIn("DukeDSClient/", dict_param['headers']['User-Agent'])
-        self.assertIn('"per_page": 100', dict_param['params'])
+        self.assertEqual(100, dict_param['params']['per_page'])
+        self.assertEqual(1, dict_param['params']['page'])
 
     def test_get_collection_two_pages(self):
         mock_requests = MagicMock()
@@ -79,7 +80,7 @@ class TestDataServiceApi(TestCase):
                                      num_pages=2)
         ]
         api = DataServiceApi(auth=None, url="something.com/v1/", http=mock_requests)
-        response = api._get_collection(url_suffix="projects", data={}, content_type=ContentType.json)
+        response = api._get_collection(url_suffix="projects", data={})
         self.assertEqual([1, 2, 3, 4, 5], response.json()["results"])
         call_args_list = mock_requests.get.call_args_list
         self.assertEqual(2, len(call_args_list))
@@ -88,18 +89,19 @@ class TestDataServiceApi(TestCase):
         first_param = call_args[0][0]
         self.assertEqual('something.com/v1/projects', first_param)
         dict_param = call_args[1]
-        self.assertEqual('application/json', dict_param['headers']['Content-Type'])
+        self.assertEqual('application/x-www-form-urlencoded', dict_param['headers']['Content-Type'])
         self.assertIn("DukeDSClient/", dict_param['headers']['User-Agent'])
-        self.assertIn('"per_page": 100', dict_param['params'])
+        self.assertEqual(100, dict_param['params']['per_page'])
+        self.assertEqual(1, dict_param['params']['page'])
         # Check second request
         call_args = call_args_list[1]
         first_param = call_args[0][0]
         self.assertEqual('something.com/v1/projects', first_param)
         dict_param = call_args[1]
-        self.assertEqual('application/json', dict_param['headers']['Content-Type'])
+        self.assertEqual('application/x-www-form-urlencoded', dict_param['headers']['Content-Type'])
         self.assertIn("DukeDSClient/", dict_param['headers']['User-Agent'])
-        self.assertIn('"per_page": 100', dict_param['params'])
-        self.assertIn('"page": 2', dict_param['params'])
+        self.assertEqual(100, dict_param['params']['per_page'])
+        self.assertEqual(2, dict_param['params']['page'])
 
     def test_get_collection_three_pages(self):
         mock_requests = MagicMock()
@@ -115,7 +117,7 @@ class TestDataServiceApi(TestCase):
                                      num_pages=3)
         ]
         api = DataServiceApi(auth=None, url="something.com/v1/", http=mock_requests)
-        response = api._get_collection(url_suffix="uploads", data={}, content_type=ContentType.json)
+        response = api._get_collection(url_suffix="uploads", data={})
         self.assertEqual([1, 2, 3, 4, 5, 6, 7], response.json()["results"])
         call_args_list = mock_requests.get.call_args_list
         self.assertEqual(3, len(call_args_list))
@@ -124,27 +126,29 @@ class TestDataServiceApi(TestCase):
         first_param = call_args[0][0]
         self.assertEqual('something.com/v1/uploads', first_param)
         dict_param = call_args[1]
-        self.assertEqual('application/json', dict_param['headers']['Content-Type'])
+        self.assertEqual('application/x-www-form-urlencoded', dict_param['headers']['Content-Type'])
         self.assertIn("DukeDSClient/", dict_param['headers']['User-Agent'])
-        self.assertIn('"per_page": 100', dict_param['params'])
+        self.assertEqual(100, dict_param['params']['per_page'])
+        self.assertEqual(1, dict_param['params']['page'])
         # Check second request
         call_args = call_args_list[1]
         first_param = call_args[0][0]
         self.assertEqual('something.com/v1/uploads', first_param)
         dict_param = call_args[1]
-        self.assertEqual('application/json', dict_param['headers']['Content-Type'])
+        self.assertEqual('application/x-www-form-urlencoded', dict_param['headers']['Content-Type'])
         self.assertIn("DukeDSClient/", dict_param['headers']['User-Agent'])
-        self.assertIn('"per_page": 100', dict_param['params'])
-        self.assertIn('"page": 2', dict_param['params'])
+        self.assertEqual(100, dict_param['params']['per_page'])
+        self.assertEqual(2, dict_param['params']['page'])
+
         # Check third request
         call_args = call_args_list[2]
         first_param = call_args[0][0]
         self.assertEqual('something.com/v1/uploads', first_param)
         dict_param = call_args[1]
-        self.assertEqual('application/json', dict_param['headers']['Content-Type'])
+        self.assertEqual('application/x-www-form-urlencoded', dict_param['headers']['Content-Type'])
         self.assertIn("DukeDSClient/", dict_param['headers']['User-Agent'])
-        self.assertIn('"per_page": 100', dict_param['params'])
-        self.assertIn('"page": 3', dict_param['params'])
+        self.assertEqual(100, dict_param['params']['per_page'])
+        self.assertEqual(3, dict_param['params']['page'])
 
     def test_put_raises_error_on_paging_response(self):
         mock_requests = MagicMock()
@@ -192,7 +196,7 @@ class TestDataServiceApi(TestCase):
             fake_response_with_pages(status_code=200, json_return_value={"ok": True}, num_pages=3)
         ]
         api = DataServiceApi(auth=None, url="something.com/v1/", http=mock_requests)
-        resp = api._get_single_page(url_suffix='stuff', data={}, content_type=ContentType.json, page_num=1)
+        resp = api._get_single_page(url_suffix='stuff', data={}, page_num=1)
         self.assertEqual(True, resp.json()['ok'])
 
     def test_get_auth_providers(self):
@@ -346,3 +350,37 @@ class TestDataServiceApi(TestCase):
         api = DataServiceApi(auth=None, url="something.com/v1/", http=None)
         self.assertIsNotNone(api.http)
         self.assertEqual(type(api.http), requests.sessions.Session)
+
+    def test_get_projects(self):
+        page1 = {
+            "results": [
+                {
+                    "id": "1234"
+                }
+            ]
+        }
+        page2 = {
+            "results": [
+                {
+                    "id": "1235"
+                }
+            ]
+        }
+        mock_requests = MagicMock()
+        mock_requests.get.side_effect = [
+            fake_response_with_pages(status_code=200, json_return_value=page1, num_pages=2),
+            fake_response_with_pages(status_code=200, json_return_value=page2, num_pages=2),
+        ]
+        api = DataServiceApi(auth=None, url="something.com/v1", http=mock_requests)
+        resp = api.get_projects()
+        self.assertEqual(2, len(resp.json()['results']))
+        self.assertEqual("1234", resp.json()['results'][0]['id'])
+        self.assertEqual("1235", resp.json()['results'][1]['id'])
+        self.assertEqual(2, mock_requests.get.call_count)
+        first_call_second_arg = mock_requests.get.call_args_list[0][1]
+        self.assertEqual('application/x-www-form-urlencoded', first_call_second_arg['headers']['Content-Type'])
+        self.assertEqual(100, first_call_second_arg['params']['per_page'])
+        self.assertEqual(1, first_call_second_arg['params']['page'])
+        second_call_second_arg = mock_requests.get.call_args_list[0][1]
+        self.assertEqual(100, second_call_second_arg['params']['per_page'])
+        self.assertEqual(1, second_call_second_arg['params']['page'])