Skip to content

Commit

Permalink
Fix-2
Browse files Browse the repository at this point in the history
* Add test

* Add test

* Update docstring

* Don't count deleted objects in repo

* Count by ID instead of key

* Rename slice_annotations > slice_items

* Don't count or return deleted Annotations

* Add annotation generation fixture

* Remove unused imports

* Add deleted clause to search

* Update docs

* Add tests

* Style fixes

* Fix version
  • Loading branch information
alexandermendes authored Jul 24, 2018
1 parent 4389ed7 commit 04169a4
Show file tree
Hide file tree
Showing 16 changed files with 219 additions and 23 deletions.
26 changes: 26 additions & 0 deletions bin/generate_annotations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/usr/bin/env python

import sys

from explicates.core import db, create_app
from explicates.model.collection import Collection
from explicates.model.annotation import Annotation


app = create_app()


def generate_annotations(n, collection_key):
"""Add n Annotations to the AnnotationCollection with collection_key."""
with app.app_context():
collection = db.session.query(Collection).get(collection_key)
for i in range(n):
anno = Annotation(collection=collection)
db.session.add(anno)
db.session.commit()


if __name__ == '__main__':
n = int(sys.argv[1])
collection_key = int(sys.argv[2])
generate_annotations(n, collection_key)
19 changes: 18 additions & 1 deletion docs/search.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ The `fts` query accepts the following parameters for each field:
[Configuration](/setup.md#configuration) section for more details of
the available dictionaries.


## fts_phrase

Return Annotations where the specified keys contain a `query` phrase. The
Expand Down Expand Up @@ -146,3 +145,21 @@ The `fts_phrase` query accepts the following parameters for each field:

Note that all phrase queries will be treated as prefixes; to search for
exact phrases use [`contains`](/search.md#contains) instead.

## deleted

Return or include deleted Annotations.

```json
{
"deleted": "include"
}
```

The `deleted` query accepts the following parameters:

| key | description |
|---------|---------------------------------------|
| exclude | Exclude deleted Annotations (default) |
| include | Include deleted Annotations |
| only | Return only deleted Annotations |
4 changes: 2 additions & 2 deletions explicates/api/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def _get_container(self, collection_base, items=None, total=None,
n_pages = self._get_n_pages(items, out['total'], per_page)

if items:
items = self._slice_annotations(items, int(per_page), page)
items = self._slice_items(items, int(per_page), page)
if isinstance(page, int) and not items:
abort(404)
elif isinstance(page, int):
Expand All @@ -235,7 +235,7 @@ def _get_container(self, collection_base, items=None, total=None,

return out

def _slice_annotations(self, items, per_page, page=0):
def _slice_items(self, items, per_page, page=0):
"""Return a slice of items. """
start = page * per_page if page and page > 0 else 0
return items[start:start + per_page]
Expand Down
4 changes: 2 additions & 2 deletions explicates/api/collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def _get_collection(self, collection_id):
def get(self, collection_id):
"""Get a Collection."""
collection = self._get_collection(collection_id)
items = collection.annotations
items = collection.annotations.filter(Annotation.deleted == False)
container = self._get_container(collection, items=items)
return self._jsonld_response(container)

Expand All @@ -43,7 +43,7 @@ def put(self, collection_id):
"""Update a Collection."""
collection = self._get_collection(collection_id)
self._update(collection)
items = collection.annotations
items = collection.annotations.filter(Annotation.deleted == False)
container = self._get_container(collection, items=items)
return self._jsonld_response(container)

Expand Down
2 changes: 1 addition & 1 deletion explicates/api/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class SearchAPI(APIBase, MethodView):
def _filter_valid_params(self, data):
"""Return the valid search parameters."""
valid_keys = ['contains', 'collection', 'fts', 'fts_phrase', 'limit',
'range', 'order_by', 'offset']
'range', 'order_by', 'offset', 'deleted']
return {k: v for k, v in data.items() if k in valid_keys}

def get(self):
Expand Down
2 changes: 1 addition & 1 deletion explicates/model/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class BaseDomainObject(object):
#: The time at which the object was modified, after creation.
modified = Column(Text, onupdate=make_timestamp)

#: The time at which the object was deleted.
#: True if the object was deleted, False otherwise.
deleted = Column(Boolean, default=False)

#: The modifiable JSON data.
Expand Down
11 changes: 8 additions & 3 deletions explicates/model/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"""Collection model."""

from flask import url_for
from sqlalchemy import func
from sqlalchemy import func, and_
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.hybrid import hybrid_property
from sqlalchemy.orm import relationship
Expand All @@ -25,12 +25,17 @@ class Collection(db.Model, Base):

@hybrid_property
def total(self):
return self.annotations.count()
count_q = func.count(Annotation.id) \
.filter(Annotation.deleted == False) \
.filter(Annotation.collection_key == self.key)
return db.session.execute(db.session.query(count_q)).scalar()

@total.expression
def total(cls):
return (select([func.count(Annotation.id)]).
where(Annotation.collection_key == cls.key).
and_(
where(Annotation.collection_key == cls.key),
where(Annotation.deleted == False)).
label("total"))

@hybrid_property
Expand Down
5 changes: 3 additions & 2 deletions explicates/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ def filter_by(self, model_cls, **attrs):
return self.db.session.query(model_cls).filter_by(**attrs).all()

def count(self, model_cls):
"""Count all objects."""
return self.db.session.query(model_cls).count()
"""Count all non-deleted objects."""
count_q = func.count(model_cls.key).filter(model_cls.deleted == False)
return self.db.session.execute(self.db.session.query(count_q)).scalar()

def save(self, model_cls, obj):
"""Save an object."""
Expand Down
22 changes: 20 additions & 2 deletions explicates/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@ def __init__(self, db):
self.db = db

def search(self, contains=None, collection=None, fts=None, fts_phrase=None,
limit=None, range=None, order_by='created', offset=0):
limit=None, range=None, order_by='created', offset=0,
deleted=None):
"""Search for Annotations."""
clauses = []
clauses = [Annotation.deleted == False]
if deleted:
clauses = self._get_deleted_clause(deleted)

if contains:
contains_clause = self._get_contains_clause(contains)
clauses.append(contains_clause)
Expand Down Expand Up @@ -181,3 +185,17 @@ def _get_vector(self, col):
return _entity_descriptor(Annotation, col)
except InvalidRequestError:
return _entity_descriptor(Annotation, '_data')[col]

def _get_deleted_clause(self, data):
"""Return the deleted clause."""
if data.lower() == 'exclude':
return [Annotation.deleted == False]
elif data.lower() == 'include':
return []
elif data.lower() == 'only':
return [Annotation.deleted == True]
else:
msg_base = 'invalid "deleted" clause'
msg_suffix = 'value must be "include", "exclude" or "only"'
msg = '{0}: {1}'.format(msg_base, msg_suffix)
raise ValueError(msg)
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ nologcapture=1

[pycodestyle]
exclude=settings.py, settings_test.py, env/
ignore=E402, W504
ignore=E402, W504, E712
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

setup(
name='explicates',
version='0.1.0',
version='0.3.0',
packages=find_packages(),
install_requires=requirements,
author='Alexander Mendes',
Expand Down
12 changes: 6 additions & 6 deletions test/test_api/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,19 @@ def setUp(self):
self.api_base = APIBase()

@with_context
def test_slice_annotations(self):
"""Test first slice of Annotations returned."""
def test_slice_items(self):
"""Test first slice of items returned."""
collection = CollectionFactory()
annotations = AnnotationFactory.create_batch(4, collection=collection)
out = self.api_base._slice_annotations(collection.annotations, 2, 0)
out = self.api_base._slice_items(collection.annotations, 2, 0)
assert_equal(out, annotations[:2])

@with_context
def test_offset_slice_annotations(self):
"""Test offset slice of Annotations returned."""
def test_offset_slice_items(self):
"""Test offset slice of items returned."""
collection = CollectionFactory()
annotations = AnnotationFactory.create_batch(4, collection=collection)
out = self.api_base._slice_annotations(collection.annotations, 2, 1)
out = self.api_base._slice_items(collection.annotations, 2, 1)
assert_equal(out, annotations[2:])

@with_context
Expand Down
40 changes: 40 additions & 0 deletions test/test_api/test_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -601,3 +601,43 @@ def test_collection_validated_before_update(self, mock_validate):
schema = json.load(open(schema_path))
mock_validate.assert_called_once_with(bad_data, schema)
assert_not_equal(collection._data, bad_data)

@with_context
@freeze_time("1984-11-19")
def test_deleted_annotations_not_returned(self):
"""Test deleted Annotation not returned in AnnotationCollection."""
collection = CollectionFactory()
annotation = AnnotationFactory(collection=collection)
AnnotationFactory(collection=collection, deleted=True)
expected = {
'@context': 'http://www.w3.org/ns/anno.jsonld',
'id': url_for('api.collections', collection_id=collection.id),
'type': collection.data['type'],
'created': '1984-11-19T00:00:00Z',
'generated': '1984-11-19T00:00:00Z',
'total': 1,
'first': {
'id': url_for('api.collections', collection_id=collection.id,
page=0),
'type': 'AnnotationPage',
'startIndex': 0,
'items': [
{
'id': url_for('api.annotations',
collection_id=collection.id,
annotation_id=annotation.id),
'type': 'Annotation',
'body': annotation.data['body'],
'target': annotation.data['target'],
'created': '1984-11-19T00:00:00Z',
'generated': '1984-11-19T00:00:00Z',
'generator': current_app.config.get('GENERATOR')
}
]
}
}

endpoint = u'/annotations/{}/'.format(collection.id)
res = self.app_get_json_ld(endpoint)
data = json.loads(res.data.decode('utf8'))
assert_dict_equal(data, expected)
23 changes: 23 additions & 0 deletions test/test_model/test_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,26 @@ def test_total_returns_n_annotations(self):
db.session.add(annotation)
db.session.commit()
assert_equal(collection.total, 1)

@with_context
def test_total_does_not_count_deleted_annotations(self):
"""Test Collection total count does not include deleted Annotations."""
collection_data = {
'type': [
'AnnotationCollection',
'BasicContainer'
]
}
anno_data = {
'body': 'foo',
'target': 'bar'
}
collection = Collection(data=collection_data)
annotation = Annotation(collection=collection, data=anno_data)
deleted_annotation = Annotation(collection=collection, data=anno_data,
deleted=True)
db.session.add(collection)
db.session.add(annotation)
db.session.add(deleted_annotation)
db.session.commit()
assert_equal(collection.total, 1)
31 changes: 30 additions & 1 deletion test/test_repository.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# -*- coding: utf8 -*-

from nose.tools import *
from base import Test
from base import Test, db, with_context

from explicates.core import repo
from explicates.model.annotation import Annotation
Expand All @@ -28,3 +28,32 @@ def test_wrong_object_not_updated(self):
'target': 'bar'
})
assert_raises(ValueError, repo.update, Collection, annotation)

@with_context
def test_count_does_not_include_deleted_collections(self):
"""Test count does not include deleted AnnotationCollections."""
collection1 = Collection()
collection2 = Collection(deleted=True)
db.session.add(collection1)
db.session.add(collection2)
db.session.commit()
n = repo.count(Collection)
assert_equal(n, 1)

@with_context
def test_count_does_not_include_deleted_annotations(self):
"""Test count does not include deleted Annotations."""
collection = Collection()
anno1 = Annotation(deleted=True, collection=collection, data={
'body': 'foo',
'target': 'bar'
})
anno2 = Annotation(deleted=False, collection=collection, data={
'body': 'foo',
'target': 'bar'
})
db.session.add(anno1)
db.session.add(anno2)
db.session.commit()
n = repo.count(Annotation)
assert_equal(n, 1)
37 changes: 37 additions & 0 deletions test/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,40 @@ def test_collection_clause(self):
iri = 'foo'
clause = self.search._get_collection_clause(iri)
assert_equal(str(clause), 'collection.id = :id_1')

@with_context
def test_search_excludes_deleted_annotations_by_default(self):
"""Test search excludes deleted Annotations by default."""
anno = AnnotationFactory()
AnnotationFactory(deleted=True)
results = self.search.search()
assert_equal(results, [anno])

@with_context
def test_search_excludes_deleted_annotations_explicitly(self):
"""Test search excludes deleted Annotations explicitly."""
anno = AnnotationFactory()
AnnotationFactory(deleted=True)
results = self.search.search(deleted='exclude')
assert_equal(results, [anno])

@with_context
def test_search_includes_deleted_annotations(self):
"""Test search includes deleted Annotations."""
anno1 = AnnotationFactory()
anno2 = AnnotationFactory(deleted=True)
results = self.search.search(deleted='include')
assert_equal(results, [anno1, anno2])

@with_context
def test_search_returns_only_deleted_annotations(self):
"""Test search returns only deleted Annotations."""
anno = AnnotationFactory(deleted=True)
AnnotationFactory()
results = self.search.search(deleted='only')
assert_equal(results, [anno])

@with_context
def test_search_raises_when_invalid_deleted_value(self):
"""Test search raises ValueError with invalid deleted argumement."""
assert_raises(ValueError, self.search._get_deleted_clause, 'foo')

0 comments on commit 04169a4

Please sign in to comment.