diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 73d4bc76d..8e481c8e7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,6 +18,7 @@ This document describes changes between each past release. - Redis listener is not part of the core anymore. (#712) Use ``kinto.event_listeners.redis.use = kinto_redis.listeners`` instead of ``kinto.event_listeners.redis.use = kinto.core.listeners.redis`` +- Notion of unique fields was dropped from ``kinto.core`` resources. **Protocol** diff --git a/docs/api/1.x/_details-patch-object.rst b/docs/api/1.x/_details-patch-object.rst index 6b0382d1a..12e2107e2 100644 --- a/docs/api/1.x/_details-patch-object.rst +++ b/docs/api/1.x/_details-patch-object.rst @@ -39,10 +39,3 @@ See :ref:`api-permissions-payload`. .. ---------------- .. If a read-only field is modified, a |status-400| error is returned. - - -.. Conflicts -.. --------- - -.. If changing a object field violates a field unicity constraint, a |status-409| -.. error response is returned (see :ref:`error channel `). diff --git a/docs/api/1.x/_details-post-list.rst b/docs/api/1.x/_details-post-list.rst index 26f6157b9..d8ba95e36 100644 --- a/docs/api/1.x/_details-post-list.rst +++ b/docs/api/1.x/_details-post-list.rst @@ -33,23 +33,3 @@ In the JSON request payloads, an optional ``permissions`` attribute can be provi The :ref:`current user id ` **is always added** among the ``write`` principals. See :ref:`api-permissions-payload`. - -.. Feature of Kinto.core not used in Kinto - -.. Conflicts -.. --------- - -.. Since some fields can be defined as unique per collection, some conflicts -.. may appear when creating records. - -.. .. note:: - -.. Empty values are not taken into account for field unicity. - -.. .. note:: - -.. Deleted records are not taken into account for field unicity. - -.. If a conflict occurs, an error response is returned with status |status-409|. -.. A ``details`` attribute in the response provides the offending record and -.. field name. See :ref:`dedicated section about errors `. diff --git a/docs/api/1.x/_status-patch-object.rst b/docs/api/1.x/_status-patch-object.rst index cf633f4b7..4a51292fc 100644 --- a/docs/api/1.x/_status-patch-object.rst +++ b/docs/api/1.x/_status-patch-object.rst @@ -7,6 +7,5 @@ HTTP Status Codes * |status-403|: The user is not allowed to perform the operation, or the resource is not accessible * |status-403|: The object does not exist or was deleted * |status-406|: The client doesn't accept supported responses Content-Type. -* |status-409|: If modifying this object violates a field unicity constraint * |status-412|: Record changed since value in ``If-Match`` header * |status-415|: The client request was not sent with a correct Content-Type. diff --git a/docs/api/1.x/_status-post-list.rst b/docs/api/1.x/_status-post-list.rst index 89cc40f65..5cc430557 100644 --- a/docs/api/1.x/_status-post-list.rst +++ b/docs/api/1.x/_status-post-list.rst @@ -7,6 +7,5 @@ HTTP Status Codes * |status-401|: The request is missing authentication headers * |status-403|: The user is not allowed to perform the operation, or the resource is not accessible * |status-406|: The client doesn't accept supported responses Content-Type -* |status-409|: Unicity constraint on fields is violated * |status-412|: List has changed since value in ``If-Match`` header * |status-415|: The client request was not sent with a correct Content-Type diff --git a/docs/api/1.x/_status-put-object.rst b/docs/api/1.x/_status-put-object.rst index 11519f407..6a79e8ec4 100644 --- a/docs/api/1.x/_status-put-object.rst +++ b/docs/api/1.x/_status-put-object.rst @@ -7,6 +7,5 @@ HTTP Status Code * |status-401|: The request is missing authentication headers * |status-403|: The user is not allowed to perform the operation, or the resource is not accessible * |status-406|: The client doesn't accept supported responses Content-Type. -* |status-409|: If replacing this object violates a field unicity constraint * |status-412|: Record was changed or deleted since value in ``If-Match`` header. * |status-415|: The client request was not sent with a correct Content-Type. diff --git a/docs/api/1.x/errors.rst b/docs/api/1.x/errors.rst index a8fd9dc57..639bfb1ec 100644 --- a/docs/api/1.x/errors.rst +++ b/docs/api/1.x/errors.rst @@ -76,34 +76,6 @@ provided in the ``details`` field: } -Conflict errors -=============== - -When a record violates unicity constraints, a |status-409| error response -is returned. - -Additional information about conflicting record and field name will be -provided in the ``details`` field. - -:: - - { - "code": 409, - "errno": 122, - "error": "Conflict", - "message": "Conflict of field url on record eyjafjallajokull" - "info": "https://server/docs/api.html#errors", - "details": { - "field": "url", - "record": { - "id": "eyjafjallajokull", - "last_modified": 1430140411480, - "url": "http://mozilla.org" - } - } - } - - Validation errors ================= diff --git a/docs/core/quickstart.rst b/docs/core/quickstart.rst index a3227d254..5a578ba79 100644 --- a/docs/core/quickstart.rst +++ b/docs/core/quickstart.rst @@ -200,7 +200,7 @@ Resource customization ---------------------- See :ref:`the resource documentation ` to specify custom URLs, -schemaless resources, read-only fields, unicity constraints, record pre-processing... +schemaless resources, read-only fields, record pre-processing... Advanced initialization diff --git a/docs/core/resource.rst b/docs/core/resource.rst index 151208d96..e2ad7ff6d 100644 --- a/docs/core/resource.rst +++ b/docs/core/resource.rst @@ -27,7 +27,6 @@ Full example class Options: readonly_fields = ('device',) - unique_fields = ('url',) @resource.register() @@ -85,8 +84,8 @@ Override the base schema to add extra fields using the `Colander API ` to define *schema-less* resources or specify rules -for unicity or readonly. +See the :ref:`resource schema options ` to define *schema-less* +resources or specify rules like readonly fields. .. _resource-permissions: @@ -136,10 +135,8 @@ a custom model can be plugged-in: class TrackedModel(resource.Model): - def create_record(self, record, parent_id=None, unique_fields=None): - record = super(TrackedModel, self).create_record(record, - parent_id, - unique_fields) + def create_record(self, record, parent_id=None): + record = super(TrackedModel, self).create_record(record, parent_id) trackid = index.track(record) record['trackid'] = trackid return record diff --git a/kinto/core/resource/__init__.py b/kinto/core/resource/__init__.py index 627f92e81..c8dc0dfbc 100644 --- a/kinto/core/resource/__init__.py +++ b/kinto/core/resource/__init__.py @@ -8,8 +8,7 @@ from pyramid.decorator import reify from pyramid.security import Everyone from pyramid.httpexceptions import (HTTPNotModified, HTTPPreconditionFailed, - HTTPNotFound, HTTPConflict, - HTTPServiceUnavailable) + HTTPNotFound, HTTPServiceUnavailable) from kinto.core import logger from kinto.core import Service @@ -292,7 +291,7 @@ def collection_get(self): def collection_post(self): """Model ``POST`` endpoint: create a record. - If the new record conflicts against a unique field constraint, the + If the new record id conflicts against an existing one, the posted record is ignored, and the existing record is returned, with a ``200`` status. @@ -306,31 +305,27 @@ def collection_post(self): Add custom behaviour by overriding :meth:`kinto.core.resource.UserResource.process_record` """ - existing = None new_record = self.request.validated.get('data', {}) try: + # Since ``id`` does not belong to schema, it is not in validated + # data. Must look up in body. id_field = self.model.id_field - # Since ``id`` does not belong to schema, look up in body. new_record[id_field] = _id = self.request.json['data'][id_field] self._raise_400_if_invalid_id(_id) existing = self._get_record_or_404(_id) except (HTTPNotFound, KeyError, ValueError): - pass + existing = None self._raise_412_if_modified(record=existing) - new_record = self.process_record(new_record) - try: - unique_fields = self.mapping.get_option('unique_fields') - record = self.model.create_record(new_record, - unique_fields=unique_fields) + if existing: + record = existing + action = ACTIONS.READ + else: + new_record = self.process_record(new_record) + record = self.model.create_record(new_record) self.request.response.status_code = 201 action = ACTIONS.CREATE - except storage_exceptions.UnicityError as e: - record = e.record - # failed to write - action = ACTIONS.READ - return self.postprocess(record, action=action) def collection_delete(self): @@ -426,17 +421,11 @@ def put(self): new_record = self.process_record(post_record, old=existing) - try: - unique = self.mapping.get_option('unique_fields') - if existing and not tombstones: - record = self.model.update_record(new_record, - unique_fields=unique) - else: - record = self.model.create_record(new_record, - unique_fields=unique) - self.request.response.status_code = 201 - except storage_exceptions.UnicityError as e: - self._raise_conflict(e) + if existing and not tombstones: + record = self.model.update_record(new_record) + else: + record = self.model.create_record(new_record) + self.request.response.status_code = 201 timestamp = record[self.model.modified_field] self._add_timestamp_header(self.request.response, timestamp=timestamp) @@ -495,13 +484,8 @@ def patch(self): # Save in storage if necessary. if changed_fields or self.force_patch_update: - try: - unique_fields = self.mapping.get_option('unique_fields') - new_record = self.model.update_record( - new_record, - unique_fields=unique_fields) - except storage_exceptions.UnicityError as e: - self._raise_conflict(e) + new_record = self.model.update_record(new_record) + else: # Behave as if storage would have added `id` and `last_modified`. for extra_field in [self.model.modified_field, @@ -833,26 +817,6 @@ def _raise_412_if_modified(self, record=None): self._add_timestamp_header(response, timestamp=current_timestamp) raise response - def _raise_conflict(self, exception): - """Helper to raise conflict responses. - - :param exception: the original unicity error - :type exception: :class:`kinto.core.storage.exceptions.UnicityError` - :raises: :exc:`~pyramid:pyramid.httpexceptions.HTTPConflict` - """ - field = exception.field - record_id = exception.record[self.model.id_field] - message = 'Conflict of field %s on record %s' % (field, record_id) - details = { - "field": field, - "existing": exception.record, - } - response = http_error(HTTPConflict(), - errno=ERRORS.CONSTRAINT_VIOLATED, - message=message, - details=details) - raise response - def _raise_400_if_id_mismatch(self, new_id, record_id): """Raise 400 if the `new_id`, within the request body, does not match the `record_id`, obtained from request path. diff --git a/kinto/core/resource/model.py b/kinto/core/resource/model.py index d48176fde..d26ab5d0f 100644 --- a/kinto/core/resource/model.py +++ b/kinto/core/resource/model.py @@ -141,7 +141,7 @@ def get_record(self, record_id, parent_id=None): modified_field=self.modified_field, auth=self.auth) - def create_record(self, record, parent_id=None, unique_fields=None): + def create_record(self, record, parent_id=None): """Create a record in the collection. Override to perform actions or post-process records after their @@ -157,7 +157,6 @@ def create_record(self, record): :param dict record: record to store :param str parent_id: optional filter for parent id - :param tuple unique_fields: list of fields that should remain unique :returns: the newly created record. :rtype: dict @@ -167,12 +166,11 @@ def create_record(self, record): parent_id=parent_id, record=record, id_generator=self.id_generator, - unique_fields=unique_fields, id_field=self.id_field, modified_field=self.modified_field, auth=self.auth) - def update_record(self, record, parent_id=None, unique_fields=None): + def update_record(self, record, parent_id=None): """Update a record in the collection. Override to perform actions or post-process records after their @@ -180,17 +178,14 @@ def update_record(self, record, parent_id=None, unique_fields=None): .. code-block:: python - def update_record(self, record, parent_id=None,unique_fields=None): - record = super(MyModel, self).update_record(record, - parent_id, - unique_fields) + def update_record(self, record, parent_id=None): + record = super(MyModel, self).update_record(record, parent_id) subject = 'Record {} was changed'.format(record[self.id_field]) send_email(subject) return record :param dict record: record to store :param str parent_id: optional filter for parent id - :param tuple unique_fields: list of fields that should remain unique :returns: the updated record. :rtype: dict """ @@ -200,7 +195,6 @@ def update_record(self, record, parent_id=None,unique_fields=None): parent_id=parent_id, object_id=record_id, record=record, - unique_fields=unique_fields, id_field=self.id_field, modified_field=self.modified_field, auth=self.auth) @@ -279,15 +273,13 @@ def get_record(self, record_id, parent_id=None): annotated[self.permissions_field] = permissions return annotated - def create_record(self, record, parent_id=None, unique_fields=None): + def create_record(self, record, parent_id=None): """Create record and set specified permissions. The current principal is added to the owner (``write`` permission). """ permissions = record.pop(self.permissions_field, {}) - record = super(ShareableModel, self).create_record(record, - parent_id, - unique_fields) + record = super(ShareableModel, self).create_record(record, parent_id) record_id = record[self.id_field] perm_object_id = self.get_permission_object_id(record_id) self.permission.replace_object_permissions(perm_object_id, permissions) @@ -298,7 +290,7 @@ def create_record(self, record, parent_id=None, unique_fields=None): annotated[self.permissions_field] = permissions return annotated - def update_record(self, record, parent_id=None, unique_fields=None): + def update_record(self, record, parent_id=None): """Update record and the specified permissions. If no permissions is specified, the current permissions are not @@ -307,9 +299,7 @@ def update_record(self, record, parent_id=None, unique_fields=None): The current principal is added to the owner (``write`` permission). """ permissions = record.pop(self.permissions_field, {}) - record = super(ShareableModel, self).update_record(record, - parent_id, - unique_fields) + record = super(ShareableModel, self).update_record(record, parent_id) record_id = record[self.id_field] perm_object_id = self.get_permission_object_id(record_id) self.permission.replace_object_permissions(perm_object_id, permissions) diff --git a/kinto/core/resource/schema.py b/kinto/core/resource/schema.py index 18c065f66..d11485f91 100644 --- a/kinto/core/resource/schema.py +++ b/kinto/core/resource/schema.py @@ -19,14 +19,8 @@ class Product(ResourceSchema): reference = colander.SchemaNode(colander.String()) class Options: - unique_fields = ('reference',) + readonly_fields = ('reference',) """ - unique_fields = tuple() - """Fields that must have unique values for the user collection. - During records creation and modification, a conflict error will be - raised if unicity is about to be violated. - """ - readonly_fields = tuple() """Fields that cannot be updated. Values for fields will have to be provided either during record creation, through default values using diff --git a/kinto/core/storage/__init__.py b/kinto/core/storage/__init__.py index eb8df457e..e49e1efcf 100644 --- a/kinto/core/storage/__init__.py +++ b/kinto/core/storage/__init__.py @@ -69,7 +69,7 @@ def collection_timestamp(self, collection_id, parent_id, auth=None): raise NotImplementedError def create(self, collection_id, parent_id, record, id_generator=None, - unique_fields=None, id_field=DEFAULT_ID_FIELD, + id_field=DEFAULT_ID_FIELD, modified_field=DEFAULT_MODIFIED_FIELD, auth=None): """Create the specified `object` in this `collection_id` for this `parent_id`. @@ -84,8 +84,7 @@ def create(self, collection_id, parent_id, record, id_generator=None, :param str collection_id: the collection id. :param str parent_id: the collection parent. - - :param dict object: the object to create. + :param dict record: the object to create. :returns: the newly created object. :rtype: dict @@ -112,7 +111,7 @@ def get(self, collection_id, parent_id, object_id, raise NotImplementedError def update(self, collection_id, parent_id, object_id, record, - unique_fields=None, id_field=DEFAULT_ID_FIELD, + id_field=DEFAULT_ID_FIELD, modified_field=DEFAULT_MODIFIED_FIELD, auth=None): """Overwrite the `object` with the specified `object_id`. @@ -124,13 +123,10 @@ def update(self, collection_id, parent_id, object_id, record, This will update the collection timestamp. - :raises: :exc:`kinto.core.storage.exceptions.UnicityError` - :param str collection_id: the collection id. :param str parent_id: the collection parent. - :param str object_id: unique identifier of the object - :param dict object: the object to update or create. + :param dict record: the object to update or create. :returns: the updated object. :rtype: dict diff --git a/kinto/core/storage/memory.py b/kinto/core/storage/memory.py index 7106e017a..3040bcd33 100644 --- a/kinto/core/storage/memory.py +++ b/kinto/core/storage/memory.py @@ -4,7 +4,7 @@ from kinto.core import utils from kinto.core.storage import ( - StorageBase, exceptions, Filter, + StorageBase, exceptions, DEFAULT_ID_FIELD, DEFAULT_MODIFIED_FIELD, DEFAULT_DELETED_FIELD) from kinto.core.utils import COMPARISON @@ -46,30 +46,6 @@ def set_record_timestamp(self, collection_id, parent_id, record, record[modified_field] = timestamp return record - def check_unicity(self, collection_id, parent_id, record, - unique_fields, id_field, for_creation=False): - """Check that the specified record does not violates unicity - constraints defined in the resource's mapping options. - """ - if for_creation and id_field in record: - # If id is provided by client, check that no record conflicts. - unique_fields = (unique_fields or tuple()) + (id_field,) - - if not unique_fields: - return - - unicity_rules = get_unicity_rules(collection_id, parent_id, record, - unique_fields=unique_fields, - id_field=id_field, - for_creation=for_creation) - for filters in unicity_rules: - existing, count = self.get_all(collection_id, parent_id, - filters=filters, - id_field=id_field) - if count > 0: - field = filters[0].field - raise exceptions.UnicityError(field, existing[0]) - def apply_filters(self, records, filters): """Filter the specified records, using basic iteration. """ @@ -210,18 +186,23 @@ def _bump_timestamp(self, collection_id, parent_id, record=None, return current def create(self, collection_id, parent_id, record, id_generator=None, - unique_fields=None, id_field=DEFAULT_ID_FIELD, + id_field=DEFAULT_ID_FIELD, modified_field=DEFAULT_MODIFIED_FIELD, auth=None): - self.check_unicity(collection_id, parent_id, record, - unique_fields=unique_fields, - id_field=id_field, - for_creation=True) - id_generator = id_generator or self.id_generator record = record.copy() - _id = record.setdefault(id_field, id_generator()) + if id_field in record: + # Raise unicity error if record with same id already exists. + try: + existing = self.get(collection_id, parent_id, record[id_field]) + raise exceptions.UnicityError(id_field, existing) + except exceptions.RecordNotFoundError: + pass + else: + record[id_field] = id_generator() + self.set_record_timestamp(collection_id, parent_id, record, modified_field=modified_field) + _id = record[id_field] self._store[parent_id][collection_id][_id] = record self._cemetery[parent_id][collection_id].pop(_id, None) return record @@ -236,16 +217,12 @@ def get(self, collection_id, parent_id, object_id, return collection[object_id].copy() def update(self, collection_id, parent_id, object_id, record, - unique_fields=None, id_field=DEFAULT_ID_FIELD, + id_field=DEFAULT_ID_FIELD, modified_field=DEFAULT_MODIFIED_FIELD, auth=None): record = record.copy() record[id_field] = object_id - self.check_unicity(collection_id, parent_id, record, - unique_fields=unique_fields, - id_field=id_field) - self.set_record_timestamp(collection_id, parent_id, record, modified_field=modified_field) self._store[parent_id][collection_id][object_id] = record @@ -350,33 +327,6 @@ def delete_all(self, collection_id, parent_id, filters=None, return deleted -def get_unicity_rules(collection_id, parent_id, record, unique_fields, - id_field, for_creation): - """Build filter to target existing records that violate the resource - unicity rules on fields. - - :returns: a list of list of filters - """ - rules = [] - for field in set(unique_fields): - value = record.get(field) - - # None values cannot be considered unique. - if value is None: - continue - - filters = [Filter(field, value, COMPARISON.EQ)] - - if not for_creation: - object_id = record[id_field] - exclude = Filter(id_field, object_id, COMPARISON.NOT) - filters.append(exclude) - - rules.append(filters) - - return rules - - def apply_sorting(records, sorting): """Sort the specified records, using cumulative python sorting. """ diff --git a/kinto/core/storage/postgresql/__init__.py b/kinto/core/storage/postgresql/__init__.py index af81d3028..f9a2c4fe1 100644 --- a/kinto/core/storage/postgresql/__init__.py +++ b/kinto/core/storage/postgresql/__init__.py @@ -6,7 +6,7 @@ from kinto.core import logger from kinto.core.storage import ( - StorageBase, exceptions, Filter, + StorageBase, exceptions, DEFAULT_ID_FIELD, DEFAULT_MODIFIED_FIELD, DEFAULT_DELETED_FIELD) from kinto.core.storage.postgresql.client import create_from_config from kinto.core.utils import COMPARISON, json @@ -215,12 +215,20 @@ def collection_timestamp(self, collection_id, parent_id, auth=None): return record['last_modified'] def create(self, collection_id, parent_id, record, id_generator=None, - unique_fields=None, id_field=DEFAULT_ID_FIELD, + id_field=DEFAULT_ID_FIELD, modified_field=DEFAULT_MODIFIED_FIELD, auth=None): id_generator = id_generator or self.id_generator record = record.copy() - record_id = record.setdefault(id_field, id_generator()) + if id_field in record: + # Raise unicity error if record with same id already exists. + try: + existing = self.get(collection_id, parent_id, record[id_field]) + raise exceptions.UnicityError(id_field, existing) + except exceptions.RecordNotFoundError: + pass + else: + record[id_field] = id_generator() query = """ WITH delete_potential_tombstone AS ( @@ -235,16 +243,12 @@ def create(self, collection_id, parent_id, record, id_generator=None, from_epoch(:last_modified)) RETURNING id, as_epoch(last_modified) AS last_modified; """ - placeholders = dict(object_id=record_id, + placeholders = dict(object_id=record[id_field], parent_id=parent_id, collection_id=collection_id, last_modified=record.get(modified_field), data=json.dumps(record)) with self.client.connect() as conn: - # Check that it does violate the resource unicity rules. - self._check_unicity(conn, collection_id, parent_id, record, - unique_fields, id_field, modified_field, - for_creation=True) result = conn.execute(query, placeholders) inserted = result.fetchone() @@ -278,7 +282,7 @@ def get(self, collection_id, parent_id, object_id, return record def update(self, collection_id, parent_id, object_id, record, - unique_fields=None, id_field=DEFAULT_ID_FIELD, + id_field=DEFAULT_ID_FIELD, modified_field=DEFAULT_MODIFIED_FIELD, auth=None): query_create = """ @@ -313,9 +317,6 @@ def update(self, collection_id, parent_id, object_id, record, record[id_field] = object_id with self.client.connect() as conn: - # Check that it does violate the resource unicity rules. - self._check_unicity(conn, collection_id, parent_id, record, - unique_fields, id_field, modified_field) # Create or update ? query = """ SELECT id FROM records @@ -733,70 +734,6 @@ def _format_sorting(self, sorting, id_field, modified_field): safe_sql = 'ORDER BY %s' % (', '.join(sorts)) return safe_sql, holders - def _check_unicity(self, conn, collection_id, parent_id, record, - unique_fields, id_field, modified_field, - for_creation=False): - """Check that no existing record (in the current transaction snapshot) - violates the resource unicity rules. - """ - # If id is provided by client, check that no record conflicts. - if for_creation and id_field in record: - unique_fields = (unique_fields or tuple()) + (id_field,) - - if not unique_fields: - return - - query = """ - SELECT id - FROM records - WHERE parent_id = :parent_id - AND collection_id = :collection_id - AND (%(conditions_filter)s) - AND %(condition_record)s - LIMIT 1; - """ - safeholders = dict() - placeholders = dict(parent_id=parent_id, - collection_id=collection_id) - - # Transform each field unicity into a query condition. - filters = [] - for field in set(unique_fields): - value = record.get(field) - if value is None: - continue - sql, holders = self._format_conditions( - [Filter(field, value, COMPARISON.EQ)], - id_field, - modified_field, - prefix=field) - filters.append(sql) - placeholders.update(**holders) - - # All unique fields are empty in record - if not filters: - return - - safeholders['conditions_filter'] = ' OR '.join(filters) - - # If record is in database, then exclude it of unicity check. - if not for_creation: - object_id = record[id_field] - sql, holders = self._format_conditions( - [Filter(id_field, object_id, COMPARISON.NOT)], - id_field, - modified_field) - safeholders['condition_record'] = sql - placeholders.update(**holders) - else: - safeholders['condition_record'] = 'TRUE' - - result = conn.execute(query % safeholders, placeholders) - if result.rowcount > 0: - existing = result.fetchone() - record = self.get(collection_id, parent_id, existing['id']) - raise exceptions.UnicityError(unique_fields[0], record) - def load_from_config(config): settings = config.get_settings() diff --git a/kinto/plugins/default_bucket/test_plugin.py b/kinto/plugins/default_bucket/test_plugin.py index a1df572d1..fbf2758ec 100644 --- a/kinto/plugins/default_bucket/test_plugin.py +++ b/kinto/plugins/default_bucket/test_plugin.py @@ -162,7 +162,8 @@ def test_parent_collection_is_taken_from_the_one_created_in_batch(self): with mock.patch.object(self.storage, 'get', wraps=self.storage.get) as patched: self.app.post_json('/batch', batch, headers=self.headers) - self.assertEqual(patched.call_count, 0) + # Called twice only: bucket + collection ids unicity. + self.assertEqual(patched.call_count, 2) def test_parent_collection_is_taken_from_the_one_checked_in_batch(self): # Create it first. @@ -179,7 +180,8 @@ def test_parent_collection_is_taken_from_the_one_checked_in_batch(self): with mock.patch.object(self.storage, 'get', wraps=self.storage.get) as patched: self.app.post_json('/batch', batch, headers=self.headers) - self.assertEqual(patched.call_count, 0) + # Called twice only: bucket + collection ids unicity. + self.assertEqual(patched.call_count, 2) def test_collection_id_is_validated(self): collection_url = '/buckets/default/collections/__files__/records' diff --git a/kinto/tests/core/resource/test_views.py b/kinto/tests/core/resource/test_views.py index 968043642..2fb56d622 100644 --- a/kinto/tests/core/resource/test_views.py +++ b/kinto/tests/core/resource/test_views.py @@ -653,57 +653,6 @@ def test_invalid_body_returns_json_formatted_error(self): 'name': 'permissions.read'}]}) -class ConflictErrorsTest(BaseWebTest, unittest.TestCase): - def setUp(self): - super(ConflictErrorsTest, self).setUp() - - body = {'data': MINIMALIST_RECORD} - resp = self.app.post_json(self.collection_url, - body, - headers=self.headers) - self.record = resp.json['data'] - - def unicity_failure(*args, **kwargs): - raise storage_exceptions.UnicityError('city', {'id': 42}) - - for operation in ('create', 'update'): - patch = mock.patch.object(self.storage, operation, - side_effect=unicity_failure) - patch.start() - - def test_post_returns_200_with_existing_record(self): - body = {'data': MINIMALIST_RECORD} - resp = self.app.post_json(self.collection_url, - body, - headers=self.headers) - self.assertEqual(resp.json['data'], {'id': 42}) - - def test_put_returns_409(self): - body = {'data': MINIMALIST_RECORD} - self.app.put_json(self.get_item_url(), - body, - headers=self.headers, - status=409) - - def test_patch_returns_409(self): - body = {'data': {'name': 'Psylo'}} - self.app.patch_json(self.get_item_url(), - body, - headers=self.headers, - status=409) - - def test_409_error_gives_detail_about_field_and_record(self): - body = {'data': MINIMALIST_RECORD} - resp = self.app.put_json(self.get_item_url(), - body, - headers=self.headers, - status=409) - self.assertEqual(resp.json['message'], - 'Conflict of field city on record 42') - self.assertEqual(resp.json['details']['field'], 'city') - self.assertEqual(resp.json['details']['existing'], {'id': 42}) - - class CacheControlTest(BaseWebTest, unittest.TestCase): collection_url = '/toadstools' diff --git a/kinto/tests/core/test_storage.py b/kinto/tests/core/test_storage.py index 9d6527c4c..f61a1f2a6 100644 --- a/kinto/tests/core/test_storage.py +++ b/kinto/tests/core/test_storage.py @@ -121,14 +121,12 @@ def tearDown(self): super(BaseTestStorage, self).tearDown() self.storage.flush() - def create_record(self, record=None, id_generator=None, - unique_fields=None, **kwargs): + def create_record(self, record=None, id_generator=None, **kwargs): record = record or self.record kw = self.storage_kw.copy() kw.update(**kwargs) return self.storage.create(record=record, id_generator=id_generator, - unique_fields=unique_fields, **kw) def test_raises_backend_error_if_error_occurs_on_client(self): @@ -706,89 +704,6 @@ def test_update_ignores_specified_last_modified_if_equal(self): self.assertGreater(timestamp, timestamp_before) -class FieldsUnicityTest(object): - def test_does_not_fail_if_no_unique_fields_at_all(self): - self.create_record({'phone': '0033677'}) - self.create_record({'phone': '0033677'}, unique_fields=tuple()) - - def test_cannot_insert_duplicate_field(self): - self.create_record({'phone': '0033677'}) - self.assertRaises(exceptions.UnicityError, - self.create_record, - {'phone': '0033677'}, - unique_fields=('phone',)) - - def test_unicity_exception_gives_record_and_field(self): - record = self.create_record({'phone': '0033677'}) - try: - self.create_record({'phone': '0033677'}, - unique_fields=('phone',)) - except exceptions.UnicityError as e: - error = e - self.assertEqual(error.field, 'phone') - self.assertDictEqual(error.record, record) - - def test_unicity_is_by_parent_id(self): - self.create_record({'phone': '0033677'}) - self.create_record({'phone': '0033677'}, - unique_fields=('phone',), - parent_id=self.other_parent_id, - auth=self.other_auth) # not raising - - def test_unicity_is_for_non_null_values(self): - r = self.create_record({'phone': None}, unique_fields=('phone',)) - # not raising with None value - self.create_record({'phone': None}, unique_fields=('phone',)) - self.storage.update(object_id=r['id'], record={'phone': None}, - unique_fields=('phone',), **self.storage_kw) - - def test_unicity_does_not_apply_to_deleted_records(self): - record = self.create_record({'phone': '0033677'}) - self.storage.delete(object_id=record['id'], **self.storage_kw) - self.create_record({'phone': None}, unique_fields=('phone',)) - - def test_unicity_applies_to_one_of_all_fields_specified(self): - self.create_record({'phone': 'abc', 'line': '1'}) - self.assertRaises(exceptions.UnicityError, - self.create_record, - {'phone': 'efg', 'line': '1'}, - unique_fields=('phone', 'line')) - - def test_updating_with_same_id_does_not_raise_unicity_error(self): - record = self.create_record({'phone': '0033677'}) - self.storage.update(object_id=record['id'], - record=record, - unique_fields=('phone',), - **self.storage_kw) - - def test_updating_raises_unicity_error(self): - self.create_record({'phone': 'number'}) - record = self.create_record({'phone': '0033677'}) - self.assertRaises(exceptions.UnicityError, - self.storage.update, - object_id=record['id'], - record={'phone': 'number'}, - unique_fields=('phone',), - **self.storage_kw) - - def test_unicity_detection_supports_special_characters(self): - record = self.create_record() - values = ['b', 'http://moz.org', u"#131 \u2014 ujson", - "C:\\\\win32\\hosts"] - for value in values: - self.create_record({'phone': value}) - try: - error = None - self.storage.update(object_id=record['id'], - record={'phone': value}, - unique_fields=('phone',), - **self.storage_kw) - except exceptions.UnicityError as e: - error = e - msg = 'UnicityError not raised with %s' % value - self.assertIsNotNone(error, msg) - - class DeletedRecordsTest(object): def _get_last_modified_filters(self): start = self.storage.collection_timestamp(**self.storage_kw) @@ -1227,7 +1142,6 @@ def test_parent_cannot_update_other_parent_record(self): class StorageTest(ThreadMixin, - FieldsUnicityTest, TimestampsTest, DeletedRecordsTest, ParentRecordAccessTest,