Skip to content

Commit

Permalink
Merge pull request Kinto#763 from Kinto/drop-unique-fields
Browse files Browse the repository at this point in the history
  • Loading branch information
leplatrem committed Aug 17, 2016
2 parents 54ae65e + ab417f3 commit 7d65522
Show file tree
Hide file tree
Showing 18 changed files with 69 additions and 433 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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**

Expand Down
7 changes: 0 additions & 7 deletions docs/api/1.x/_details-patch-object.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <error-responses>`).
20 changes: 0 additions & 20 deletions docs/api/1.x/_details-post-list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,3 @@ In the JSON request payloads, an optional ``permissions`` attribute can be provi
The :ref:`current user id <api-current-userid>` **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 <error-responses>`.
1 change: 0 additions & 1 deletion docs/api/1.x/_status-patch-object.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
1 change: 0 additions & 1 deletion docs/api/1.x/_status-post-list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 0 additions & 1 deletion docs/api/1.x/_status-put-object.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
28 changes: 0 additions & 28 deletions docs/api/1.x/errors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
=================

Expand Down
2 changes: 1 addition & 1 deletion docs/core/quickstart.rst
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ Resource customization
----------------------

See :ref:`the resource documentation <resource>` to specify custom URLs,
schemaless resources, read-only fields, unicity constraints, record pre-processing...
schemaless resources, read-only fields, record pre-processing...


Advanced initialization
Expand Down
11 changes: 4 additions & 7 deletions docs/core/resource.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ Full example
class Options:
readonly_fields = ('device',)
unique_fields = ('url',)
@resource.register()
Expand Down Expand Up @@ -85,8 +84,8 @@ Override the base schema to add extra fields using the `Colander API <http://doc
genre = colander.SchemaNode(colander.String(),
validator=colander.OneOf(['Sci-Fi', 'Comedy']))
See the :ref:`resource schema options <resource-schema>` to define *schema-less* resources or specify rules
for unicity or readonly.
See the :ref:`resource schema options <resource-schema>` to define *schema-less*
resources or specify rules like readonly fields.


.. _resource-permissions:
Expand Down Expand Up @@ -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
Expand Down
72 changes: 18 additions & 54 deletions kinto/core/resource/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
26 changes: 8 additions & 18 deletions kinto/core/resource/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -167,30 +166,26 @@ 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
modification in storage.
.. 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
"""
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand Down
8 changes: 1 addition & 7 deletions kinto/core/resource/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 7d65522

Please sign in to comment.