Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: addition of FormatChecker support #144

Merged
merged 1 commit into from
Jul 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Invenio-Records is a metadata storage module.
- Grzegorz Szpura <grzegorz.szpura@cern.ch>
- Guillaume Lastecoueres <PX9e@gmx.fr>
- Henning Weiler <henning.weiler@cern.ch>
- Jacopo Notarstefano <jacopo.notarstefano@cern.ch>
- Jaime García <jaime.garcia.llopis@cern.ch>
- Jan Aage Lavik <jan.age.lavik@cern.ch>
- Jan Iwaszkiewicz <jan.iwaszkiewicz@cern.ch>
Expand Down
5 changes: 4 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,4 +328,7 @@


# Example configuration for intersphinx: refer to the Python standard library.
intersphinx_mapping = {'https://docs.python.org/': None}
intersphinx_mapping = {
'https://docs.python.org/': None,
'https://python-jsonschema.readthedocs.io/en/latest': None,
}
44 changes: 38 additions & 6 deletions invenio_records/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,35 @@ def updated(self):
"""Get last updated timestamp."""
return self.model.updated if self.model else None

def validate(self):
"""Validate record according to schema defined in ``$schema`` key."""
def validate(self, **kwargs):
"""Validate record according to schema defined in ``$schema`` key.

This method accepts arbitrary kwargs to pass the ``format_checker``
keyword argument to the underlying JSON Schema implementation.

A ``format_checker`` is an instance of class
:class:`jsonschema.FormatChecker` containing business logic to validate
arbitrary formats. For example:

>>> from jsonschema import FormatChecker
>>> from jsonschema.validators import validate
>>> checker = FormatChecker()
>>> checker.checks('foo')(lambda el: el.startswith('foo'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checker returns not None, hence you need to add <function <lambda> at ...> on next line

<function <lambda> at ...>
>>> validate('foo', {'format': 'foo'}, format_checker=checker)

returns ``None``, which means that the validation was successful, while

>>> validate('bar', {'format': 'foo'}, format_checker=checker)
Traceback (most recent call last):
...
ValidationError: 'bar' is not a 'foo'
...

raises a :class:`jsonschema.exceptions.ValidationError`.
"""
if '$schema' in self and self['$schema'] is not None:
return _records_state.validate(self, self['$schema'])
return _records_state.validate(self, self['$schema'], **kwargs)
return True

def replace_refs(self):
Expand All @@ -94,8 +119,8 @@ class Record(RecordBase):
"""Define API for metadata creation and manipulation."""

@classmethod
def create(cls, data, id_=None):
"""Create a record instance and store it in database.
def create(cls, data, id_=None, **kwargs):
r"""Create a record instance and store it in database.

Procedure followed:

Expand All @@ -111,15 +136,22 @@ def create(cls, data, id_=None):

:param data: Dict with record metadata.
:param id_: Force the UUID for the record.
:param \**kwargs: See below.
:returns: A new Record instance.

:Keyword Arguments:
* **format_checker** --
An instance of class :class:`jsonschema.FormatChecker`, which
contains validation rules for formats. See
:func:`~invenio_records.api.RecordBase.validate` for details.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This style of documenting kwargs is lifted from http://stackoverflow.com/a/27724915/374865.

"""
from .models import RecordMetadata
with db.session.begin_nested():
record = cls(data)

before_record_insert.send(record)

record.validate()
record.validate(**kwargs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit worried about future compatibility here. Shall we open wider discussion if we should create e.g. validate_kwargs=None or simply pop the known keys from kwargs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I forgot about Record.create when writing #142, so maybe we should wait for more opinions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacquerie In order to move forward, we can keep the **kwargs, but explicitly mention arguments in docstring that you know you are using and that are being forwarded to validate method.


record.model = RecordMetadata(id=id_, json=record)

Expand Down
14 changes: 8 additions & 6 deletions invenio_records/ext.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,17 @@ def __init__(self, app, entry_point_group=None):
self.ref_resolver_cls = ref_resolver_factory(self.resolver)
self.loader_cls = json_loader_factory(self.resolver)

def validate(self, data, schema):
def validate(self, data, schema, **kwargs):
"""Validate data using schema with ``JSONResolver``."""
if not isinstance(schema, dict):
schema = {'$ref': schema}
return validate(data, schema,
types=self.app.config.get(
"RECORDS_VALIDATION_TYPES", {}
),
resolver=self.ref_resolver_cls.from_schema(schema))
return validate(
data,
schema,
resolver=self.ref_resolver_cls.from_schema(schema),
types=self.app.config.get('RECORDS_VALIDATION_TYPES', {}),
**kwargs
)

def replace_refs(self, data):
"""Replace the JSON reference objects with ``JsonRef``."""
Expand Down
25 changes: 25 additions & 0 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import pytest
from jsonresolver import JSONResolver
from jsonresolver.contrib.jsonref import json_loader_factory
from jsonschema import FormatChecker
from jsonschema.exceptions import ValidationError
from sqlalchemy.orm.exc import NoResultFound

from invenio_records import Record
Expand Down Expand Up @@ -321,3 +323,26 @@ def test_record_dump(app, db):
record_dump = record.dumps()
record_dump['foo']['bar'] = 'Spam'
assert record_dump['foo']['bar'] != record['foo']['bar']


def test_validate_with_format(app, db):
"""Test that validation can accept custom format rules."""
with app.app_context():
checker = FormatChecker()
checker.checks('foo')(lambda el: el.startswith('foo'))
record = Record.create({
'bar': 'foo',
'$schema': {
'properties': {
'bar': {'format': 'foo'}
}
}
})

assert record.validate(format_checker=checker) is None

record['bar'] = 'bar'

with pytest.raises(ValidationError) as excinfo:
record.validate(format_checker=checker)
assert "'bar' is not a 'foo'" in str(excinfo.value)