Skip to content

Commit

Permalink
Implement @Natim about expliciting the data parameter.
Browse files Browse the repository at this point in the history
  • Loading branch information
Rémy HUBSCHER committed May 10, 2017
1 parent 9ce74a6 commit 36f93ab
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 93 deletions.
65 changes: 46 additions & 19 deletions kinto_http/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ def server_info(self):

def create_bucket(self, bucket=None, data=None, permissions=None,
safe=True, if_not_exists=False):

if bucket is None and data:
bucket = data.get('id', None)

if if_not_exists:
return self._create_if_not_exists('bucket',
bucket=bucket,
Expand All @@ -227,8 +231,12 @@ def create_bucket(self, bucket=None, data=None, permissions=None,
headers=headers)
return resp

def update_bucket(self, bucket=None, data=None, permissions=None,
def update_bucket(self, data=None, bucket=None, permissions=None,
safe=True, if_match=None, method='put'):

if bucket is None and data:
bucket = data.get('id', None)

endpoint = self.get_endpoint('bucket', bucket=bucket)
headers = self._get_cache_headers(safe, data, if_match)

Expand Down Expand Up @@ -294,9 +302,15 @@ def get_groups(self, bucket=None, **kwargs):
endpoint = self.get_endpoint('groups', bucket=bucket)
return self._paginated(endpoint, **kwargs)

def create_group(self, group, bucket=None,
data=None, permissions=None,
def create_group(self, group=None, bucket=None, data=None, permissions=None,
safe=True, if_not_exists=False):

if group is None and data:
group = data.get('id', None)

if group is None:
raise KeyError('Please provide a group id')

if if_not_exists:
return self._create_if_not_exists('group',
group=group,
Expand Down Expand Up @@ -325,9 +339,15 @@ def create_group(self, group, bucket=None,

return resp

def update_group(self, group, data=None, bucket=None,
permissions=None, method='put',
safe=True, if_match=None):
def update_group(self, group=None, bucket=None, data=None, permissions=None,
method='put', safe=True, if_match=None):

if group is None and data:
group = data.get('id', None)

if group is None:
raise KeyError('Please provide a group id')

endpoint = self.get_endpoint('group',
bucket=bucket,
group=group)
Expand All @@ -345,9 +365,7 @@ def patch_group(self, *args, **kwargs):
return self.update_group(*args, **kwargs)

def get_group(self, group, bucket=None):
endpoint = self.get_endpoint('group',
bucket=bucket,
group=group)
endpoint = self.get_endpoint('group', bucket=bucket, group=group)

logger.info("Get group %r in bucket %r" % (group, bucket or self._bucket_name))

Expand Down Expand Up @@ -388,16 +406,20 @@ def get_collections(self, bucket=None, **kwargs):
endpoint = self.get_endpoint('collections', bucket=bucket)
return self._paginated(endpoint, **kwargs)

def create_collection(self, collection=None, bucket=None,
data=None, permissions=None, safe=True,
if_not_exists=False):
def create_collection(self, collection=None, bucket=None, data=None,
permissions=None, safe=True, if_not_exists=False):

if collection is None and data:
collection = data.get('id', None)

if if_not_exists:
return self._create_if_not_exists('collection',
collection=collection,
bucket=bucket,
data=data,
permissions=permissions,
safe=safe)

headers = DO_NOT_OVERWRITE if safe else None
endpoint = self.get_endpoint('collection',
bucket=bucket,
Expand All @@ -420,9 +442,13 @@ def create_collection(self, collection=None, bucket=None,

return resp

def update_collection(self, data=None, collection=None, bucket=None,
permissions=None, method='put',
def update_collection(self, collection=None, bucket=None,
data=None, permissions=None, method='put',
safe=True, if_match=None):

if collection is None and data:
collection = data.get('id', None)

endpoint = self.get_endpoint('collection',
bucket=bucket,
collection=collection)
Expand Down Expand Up @@ -481,8 +507,7 @@ def delete_collections(self, bucket=None, safe=True, if_match=None):

# Records

def get_records_timestamp(self, collection=None, bucket=None,
**kwargs):
def get_records_timestamp(self, collection=None, bucket=None, **kwargs):
endpoint = self.get_endpoint('records',
bucket=bucket,
collection=collection)
Expand Down Expand Up @@ -514,8 +539,10 @@ def get_record(self, id, collection=None, bucket=None):
resp, _ = self.session.request('get', endpoint)
return resp

def create_record(self, data, id=None, collection=None, permissions=None,
def create_record(self, id=None, collection=None, data=None, permissions=None,
bucket=None, safe=True, if_not_exists=False):

id = id or data.get('id', None)
if if_not_exists:
return self._create_if_not_exists('record',
data=data,
Expand All @@ -524,7 +551,7 @@ def create_record(self, data, id=None, collection=None, permissions=None,
permissions=permissions,
bucket=bucket,
safe=safe)
id = id or data.get('id', None) or str(uuid.uuid4())
id = id or str(uuid.uuid4())
# Make sure that no record already exists with this id.
headers = DO_NOT_OVERWRITE if safe else None

Expand All @@ -550,7 +577,7 @@ def create_record(self, data, id=None, collection=None, permissions=None,

return resp

def update_record(self, data, id=None, collection=None, permissions=None,
def update_record(self, id=None, collection=None, data=None, permissions=None,
bucket=None, safe=True, method='put',
if_match=None):
id = id or data.get('id')
Expand Down
34 changes: 17 additions & 17 deletions kinto_http/tests/functional.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ def test_bucket_retrieval(self):
def test_bucket_modification(self):
bucket = self.client.create_bucket('mozilla', data={'version': 1})
assert bucket['data']['version'] == 1
bucket = self.client.patch_bucket('mozilla', data={'author': 'you'})
bucket = self.client.patch_bucket(bucket='mozilla', data={'author': 'you'})
assert bucket['data']['version'] == 1
assert bucket['data']['author'] == 'you'
bucket = self.client.update_bucket('mozilla', data={'date': 'today'})
bucket = self.client.update_bucket(bucket='mozilla', data={'date': 'today'})
assert bucket['data']['date'] == 'today'
assert 'version' not in bucket['data']

Expand Down Expand Up @@ -115,7 +115,7 @@ def test_bucket_save(self):
def test_group_creation(self):
self.client.create_bucket('mozilla')
self.client.create_group(
'payments', bucket='mozilla',
group='payments', bucket='mozilla',
data={'members': ['blah', ]},
permissions={'write': ['blah', ]})
# Test retrieval of a group gets the permissions as well.
Expand All @@ -124,27 +124,27 @@ def test_group_creation(self):

def test_group_creation_if_not_exists(self):
self.client.create_bucket('mozilla')
self.client.create_group('payments', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group(group='payments', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group(
'payments', bucket='mozilla',
group='payments', bucket='mozilla',
data={'members': ['blah', ]},
permissions={'write': ['blah', ]},
if_not_exists=True)

def test_group_creation_if_bucket_does_not_exist(self):
with pytest.raises(KintoException):
self.client.create_group(
'payments', bucket='mozilla',
group='payments', bucket='mozilla',
data={'members': ['blah', ]})
self.client.create_group(
'payments', bucket='mozilla',
group='payments', bucket='mozilla',
data={'members': ['blah', ]},
if_not_exists=True)

def test_group_update(self):
self.client.create_bucket('mozilla')
group = self.client.create_group(
'payments', bucket='mozilla',
group='payments', bucket='mozilla',
data={'members': ['blah', ]},
if_not_exists=True)
assert group['data']['members'][0] == 'blah'
Expand All @@ -155,8 +155,8 @@ def test_group_update(self):

def test_group_list(self):
self.client.create_bucket('mozilla')
self.client.create_group('receipts', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group('assets', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group(group='receipts', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group(group='assets', bucket='mozilla', data={'members': ['blah', ]})
# The returned groups should be strings.
groups = self.client.get_groups('mozilla')
self.assertEquals(2, len(groups))
Expand All @@ -165,13 +165,13 @@ def test_group_list(self):

def test_group_deletion(self):
self.client.create_bucket('mozilla')
self.client.create_group('payments', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group(group='payments', bucket='mozilla', data={'members': ['blah', ]})
self.client.delete_group('payments', bucket='mozilla')
assert len(self.client.get_groups(bucket='mozilla')) == 0

def test_group_deletion_if_exists(self):
self.client.create_bucket('mozilla')
self.client.create_group('payments', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group(group='payments', bucket='mozilla', data={'members': ['blah', ]})
self.client.delete_group('payments', bucket='mozilla')
self.client.delete_group('payments', bucket='mozilla', if_exists=True)

Expand All @@ -183,8 +183,8 @@ def test_group_deletion_can_still_raise_errors(self):

def test_groups_deletion(self):
self.client.create_bucket('mozilla')
self.client.create_group('amo', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group('blocklist', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group(group='amo', bucket='mozilla', data={'members': ['blah', ]})
self.client.create_group(group='blocklist', bucket='mozilla', data={'members': ['blah', ]})
self.client.delete_groups(bucket='mozilla')
assert len(self.client.get_groups(bucket='mozilla')) == 0

Expand Down Expand Up @@ -391,8 +391,8 @@ def test_updating_data_on_a_group(self):
client = Client(server_url=self.server_url, auth=self.auth,
bucket='mozilla')
client.create_bucket()
client.create_group('payments', data={'members': []})
client.patch_group('payments', data={'secret': 'psssssst!'})
client.create_group(group='payments', data={'members': []})
client.patch_group(group='payments', data={'secret': 'psssssst!'})
group = client.get_group('payments')
assert group['data']['secret'] == 'psssssst!'

Expand All @@ -412,7 +412,7 @@ def test_collection_sharing(self):

self.client.create_bucket('bob-bucket')
self.client.create_collection(
'shared',
collection='shared',
bucket='bob-bucket',
permissions={'read': [alice_userid, ]})

Expand Down
41 changes: 18 additions & 23 deletions kinto_http/tests/test_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,14 @@ def setUp(self):
mock_response(self.session)

def test_put_is_issued_on_creation(self):
self.client.create_bucket('testbucket')
self.client.create_bucket(bucket='testbucket')
self.session.request.assert_called_with(
'put', '/buckets/testbucket', data=None, permissions=None,
headers=DO_NOT_OVERWRITE)

def test_put_is_issued_on_update(self):
self.client.update_bucket(
'testbucket',
bucket='testbucket',
data={'foo': 'bar', 'last_modified': '1234'},
permissions={'read': ['natim']})
self.session.request.assert_called_with(
Expand All @@ -226,9 +226,9 @@ def test_put_is_issued_on_update(self):
headers={'If-Match': '"1234"'})

def test_patch_is_issued_on_patch(self):
self.client.create_bucket('testbucket')
self.client.create_bucket(bucket='testbucket')
self.client.patch_bucket(
'testbucket',
bucket='testbucket',
data={'foo': 'bar'},
permissions={'read': ['natim']})
self.session.request.assert_called_with(
Expand All @@ -240,7 +240,7 @@ def test_patch_is_issued_on_patch(self):

def test_update_bucket_handles_if_match(self):
self.client.update_bucket(
'testbucket',
bucket='testbucket',
data={'foo': 'bar'},
if_match=1234)
self.session.request.assert_called_with(
Expand Down Expand Up @@ -343,7 +343,7 @@ def test_get_or_create_dont_raise_in_case_of_conflict(self):
(bucket_data, None)
]
returned_data = self.client.create_bucket(
"buck",
bucket="buck",
if_not_exists=True) # Should not raise.
assert returned_data == bucket_data

Expand All @@ -369,7 +369,7 @@ def test_collection_names_are_slugified(self):

def test_collection_creation_issues_an_http_put(self):
self.client.create_collection(
'mycollection',
collection='mycollection',
permissions=mock.sentinel.permissions)

url = '/buckets/mybucket/collections/mycollection'
Expand All @@ -379,8 +379,8 @@ def test_collection_creation_issues_an_http_put(self):

def test_data_can_be_sent_on_creation(self):
self.client.create_collection(
'mycollection',
'testbucket',
collection='mycollection',
bucket='testbucket',
data={'foo': 'bar'})

self.session.request.assert_called_with(
Expand All @@ -391,21 +391,17 @@ def test_data_can_be_sent_on_creation(self):
headers=DO_NOT_OVERWRITE)

def test_collection_update_issues_an_http_put(self):
self.client.update_collection(
{'foo': 'bar'},
collection='mycollection',
permissions=mock.sentinel.permissions)
self.client.update_collection('mycollection',
data={'foo': 'bar'},
permissions=mock.sentinel.permissions)

url = '/buckets/mybucket/collections/mycollection'
self.session.request.assert_called_with(
'put', url, data={'foo': 'bar'},
permissions=mock.sentinel.permissions, headers=None)

def test_update_handles_if_match(self):
self.client.update_collection(
{'foo': 'bar'},
collection='mycollection',
if_match=1234)
self.client.update_collection('mycollection', data={'foo': 'bar'}, if_match=1234)

url = '/buckets/mybucket/collections/mycollection'
headers = {'If-Match': '"1234"'}
Expand All @@ -415,10 +411,9 @@ def test_update_handles_if_match(self):

def test_collection_update_use_an_if_match_header(self):
data = {'foo': 'bar', 'last_modified': '1234'}
self.client.update_collection(
data,
collection='mycollection',
permissions=mock.sentinel.permissions)
self.client.update_collection('mycollection',
data=data,
permissions=mock.sentinel.permissions)

url = '/buckets/mybucket/collections/mycollection'
self.session.request.assert_called_with(
Expand Down Expand Up @@ -551,7 +546,7 @@ def test_record_id_is_given_after_creation(self):

def test_generated_record_id_is_an_uuid(self):
mock_response(self.session)
self.client.create_record({'foo': 'bar'})
self.client.create_record(data={'foo': 'bar'})
id = self.session.request.mock_calls[0][1][1].split('/')[-1]

uuid_regexp = r'[\w]{8}-[\w]{4}-[\w]{4}-[\w]{4}-[\w]{12}'
Expand All @@ -560,7 +555,7 @@ def test_generated_record_id_is_an_uuid(self):
def test_records_handles_permissions(self):
mock_response(self.session)
self.client.create_record(
{'id': '1234', 'foo': 'bar'},
data={'id': '1234', 'foo': 'bar'},
permissions=mock.sentinel.permissions)
self.session.request.assert_called_with(
'put',
Expand Down
Loading

0 comments on commit 36f93ab

Please sign in to comment.