From c255ca91d760ed03598557695355d617195bfd1f Mon Sep 17 00:00:00 2001 From: cbeauchesne Date: Mon, 28 Oct 2019 13:34:23 +0100 Subject: [PATCH] Add association history service #729 --- ...5a5ed3c76a8_add_index_on_associationlog.py | 41 +++++++ c2corg_api/models/association.py | 11 +- c2corg_api/tests/views/__init__.py | 60 ++++++++- c2corg_api/tests/views/test_area.py | 8 +- c2corg_api/tests/views/test_article.py | 11 +- c2corg_api/tests/views/test_book.py | 15 ++- c2corg_api/tests/views/test_image.py | 31 +++-- c2corg_api/tests/views/test_outing.py | 62 +++++++--- c2corg_api/tests/views/test_route.py | 31 ++--- c2corg_api/tests/views/test_waypoint.py | 35 +++--- c2corg_api/tests/views/test_xreport.py | 15 ++- c2corg_api/views/association_history.py | 115 ++++++++++++++++++ c2corg_api/views/validation.py | 7 ++ docker-compose.yml | 1 + 14 files changed, 362 insertions(+), 81 deletions(-) create mode 100644 alembic_migration/versions/85a5ed3c76a8_add_index_on_associationlog.py create mode 100644 c2corg_api/views/association_history.py diff --git a/alembic_migration/versions/85a5ed3c76a8_add_index_on_associationlog.py b/alembic_migration/versions/85a5ed3c76a8_add_index_on_associationlog.py new file mode 100644 index 000000000..ff390c154 --- /dev/null +++ b/alembic_migration/versions/85a5ed3c76a8_add_index_on_associationlog.py @@ -0,0 +1,41 @@ +"""Add index on AssociationLog + +Revision ID: 85a5ed3c76a8 +Revises: 83956b269661 +Create Date: 2019-10-25 21:20:55.476560 + +""" +from alembic import op + +# revision identifiers, used by Alembic. +revision = '85a5ed3c76a8' +down_revision = '83956b269661' +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_index('association_log_child_document_id_idx', + 'association_log', ['child_document_id'], + unique=False, + schema='guidebook') + op.create_index('association_log_parent_document_id_idx', + 'association_log', ['parent_document_id'], + unique=False, + schema='guidebook') + op.create_index('association_log_user_id_idx', + 'association_log', ['user_id'], + unique=False, + schema='guidebook') + + +def downgrade(): + op.drop_index('association_log_parent_document_id_idx', + table_name='association_log', + schema='guidebook') + op.drop_index('association_log_child_document_id_idx', + table_name='association_log', + schema='guidebook') + op.drop_index('association_log_user_id_idx', + table_name='association_log', + schema='guidebook') diff --git a/c2corg_api/models/association.py b/c2corg_api/models/association.py index 445098949..76281eb3e 100644 --- a/c2corg_api/models/association.py +++ b/c2corg_api/models/association.py @@ -85,20 +85,25 @@ class AssociationLog(Base): parent_document_id = Column( Integer, ForeignKey(schema + '.documents.document_id'), - nullable=False) + nullable=False, + index=True + ) parent_document = relationship( Document, primaryjoin=parent_document_id == Document.document_id) parent_document_type = Column(String(1), nullable=False) child_document_id = Column( Integer, ForeignKey(schema + '.documents.document_id'), - nullable=False) + nullable=False, + index=True + ) child_document = relationship( Document, primaryjoin=child_document_id == Document.document_id) child_document_type = Column(String(1), nullable=False) user_id = Column( - Integer, ForeignKey(users_schema + '.user.id'), nullable=False) + Integer, ForeignKey(users_schema + '.user.id'), nullable=False, + index=True) user = relationship( User, primaryjoin=user_id == User.id, viewonly=True) diff --git a/c2corg_api/tests/views/__init__.py b/c2corg_api/tests/views/__init__.py index fe909908b..439980e13 100644 --- a/c2corg_api/tests/views/__init__.py +++ b/c2corg_api/tests/views/__init__.py @@ -8,7 +8,7 @@ from c2corg_api.models.feed import DocumentChange from c2corg_api.models.route import Route from c2corg_api.models.user import User -from c2corg_api.models.user_profile import UserProfile +from c2corg_api.models.user_profile import UserProfile, USERPROFILE_TYPE from c2corg_api.scripts.es.sync import sync_es from c2corg_api.search import elasticsearch_config, search_documents from c2corg_api.tests import BaseTestCase @@ -1107,6 +1107,64 @@ def put_success_new_lang( return (body, document) + def _get_association_logs(self, document): + url = '/associations-history?d={}'.format( + document.document_id + ) + + resp = self.app.get(url, status=200) + self.assertIsInstance(resp.json['count'], int) + associations = resp.json["associations"] + + self.assertNotEqual( + len(associations), + 0, + "Need at least one association") + + for association in associations: + self._assert_association_log_structure( + association, + document.document_id) + + return associations + + def _assert_association_log_structure(self, log, document_id): + self.assertIsNotNone(log['written_at']) + self.assertIsInstance(log['is_creation'], bool) + + user = log['user'] + self.assertIsInstance(user['user_id'], int) + self.assertIsInstance(user['name'], str) + self.assertIsInstance(user['forum_username'], str) + self.assertIsInstance(user['robot'], bool) + self.assertIsInstance(user['moderator'], bool) + self.assertIsInstance(user['blocked'], bool) + + child = log['child_document'] + child_id = child['document_id'] + self.assertIsInstance(child_id, int) + self.assertIsInstance(child['type'], str) + self.assertIsInstance(child['locales'], list) + + parent = log['parent_document'] + parent_id = parent['document_id'] + self.assertIsInstance(parent_id, int) + self.assertIsInstance(parent['type'], str) + self.assertIsInstance(parent['locales'], list) + + self.assertTrue(child_id == document_id or parent_id == document_id) + + if parent["type"] == USERPROFILE_TYPE: + self.assertIsInstance(parent["name"], str) + + if child["type"] == USERPROFILE_TYPE: + self.assertIsInstance(child["name"], str) + + def _add_association(self, association, user_id): + """ used for setup """ + self.session.add(association) + self.session.add(association.get_log(user_id, is_creation=True)) + def get_locale(locales, lang): return next(filter(lambda locale: locale['lang'] == lang, locales), None) diff --git a/c2corg_api/tests/views/test_area.py b/c2corg_api/tests/views/test_area.py index 4290c2feb..20124c1ae 100644 --- a/c2corg_api/tests/views/test_area.py +++ b/c2corg_api/tests/views/test_area.py @@ -435,6 +435,9 @@ def test_put_success_new_lang(self): self.assertEquals(area.get_locale('es').title, 'Chartreuse') + def test_get_associations_history(self): + self._get_association_logs(self.area1) + def _assert_geometry(self, body): self.assertIsNotNone(body.get('geometry')) geometry = body.get('geometry') @@ -510,5 +513,8 @@ def _add_test_data(self): self.session.add(self.image) self.session.flush() - self.session.add(Association.create(self.area1, self.image)) + self._add_association( + Association.create(self.area1, self.image), + user_id + ) self.session.flush() diff --git a/c2corg_api/tests/views/test_article.py b/c2corg_api/tests/views/test_article.py index 86a8efa1f..96b2d1459 100644 --- a/c2corg_api/tests/views/test_article.py +++ b/c2corg_api/tests/views/test_article.py @@ -585,6 +585,9 @@ def test_put_as_non_author(self): self.assertEqual(len(body['errors']), 1) self.assertEqual(body['errors'][0]['name'], 'Forbidden') + def test_get_associations_history(self): + self._get_association_logs(self.article1) + def _add_test_data(self): self.article1 = Article(categories=['site_info'], activities=['hiking'], @@ -639,10 +642,10 @@ def _add_test_data(self): self.session.add(self.waypoint2) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.article1, - child_document=self.article4)) - self.session.add(Association.create( + child_document=self.article4), user_id) + self._add_association(Association.create( parent_document=self.article3, - child_document=self.article1)) + child_document=self.article1), user_id) self.session.flush() diff --git a/c2corg_api/tests/views/test_book.py b/c2corg_api/tests/views/test_book.py index 96e4dd09f..3589cab26 100644 --- a/c2corg_api/tests/views/test_book.py +++ b/c2corg_api/tests/views/test_book.py @@ -467,6 +467,9 @@ def test_put_success_new_lang(self): self.assertEquals(book1.get_locale('es').title, 'Escalades au Thaurac') + def test_get_associations_history(self): + self._get_association_logs(self.book1) + def _add_test_data(self): self.book1 = Book(activities=['hiking'], book_types=['biography']) @@ -565,13 +568,13 @@ def _add_test_data(self): self.session.add(self.route3) self.session.flush() - # self.session.add(Association.create( + # self._add_association(Association.create( # parent_document=self.book1, - # child_document=self.image2)) - self.session.add(Association.create( + # child_document=self.image2), user_id) + self._add_association(Association.create( parent_document=self.book1, - child_document=self.route3)) - self.session.add(Association.create( + child_document=self.route3), user_id) + self._add_association(Association.create( parent_document=self.book2, - child_document=self.waypoint2)) + child_document=self.waypoint2), user_id) self.session.flush() diff --git a/c2corg_api/tests/views/test_image.py b/c2corg_api/tests/views/test_image.py index 2fd380925..fcca360bb 100644 --- a/c2corg_api/tests/views/test_image.py +++ b/c2corg_api/tests/views/test_image.py @@ -28,6 +28,8 @@ class BaseTestImage(BaseDocumentTestRest): def _add_test_data(self): + user_id = self.global_userids['contributor'] + self.image = Image( filename='image.jpg', activities=['paragliding'], height=1500, @@ -54,19 +56,18 @@ def _add_test_data(self): article_type='collab') self.session.add(self.article1) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.article1, - child_document=self.image)) + child_document=self.image), user_id) self.book1 = Book(activities=['hiking'], book_types=['biography']) self.session.add(self.book1) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.book1, - child_document=self.image)) + child_document=self.image), user_id) - user_id = self.global_userids['contributor'] DocumentRest.create_new_version(self.image, user_id) self.image_version = self.session.query(DocumentVersion). \ @@ -95,9 +96,9 @@ def _add_test_data(self): self.image3, self.global_userids['contributor2']) DocumentRest.create_new_version(self.image4, user_id) - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.image, - child_document=self.image2)) + child_document=self.image2), user_id) self.waypoint = Waypoint( waypoint_type='summit', elevation=4, @@ -120,7 +121,10 @@ def _add_test_data(self): self.session.add(self.area) self.session.flush() - self.session.add(Association.create(self.area, self.image)) + self._add_association( + Association.create(self.area, self.image), + user_id + ) self.session.flush() self.outing1 = Outing( @@ -134,14 +138,14 @@ def _add_test_data(self): ) self.session.add(self.outing1) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.outing1, - child_document=self.image)) - self.session.add(Association( + child_document=self.image), user_id) + self._add_association(Association( parent_document_id=self.global_userids['contributor'], parent_document_type=USERPROFILE_TYPE, child_document_id=self.outing1.document_id, - child_document_type=OUTING_TYPE)) + child_document_type=OUTING_TYPE), user_id) update_feed_document_create(self.outing1, user_id) self.session.flush() @@ -817,6 +821,9 @@ def test_change_image_type_non_collaborative(self): self._prefix + '/' + str(self.image4.document_id), body, headers=headers, status=200) + def test_get_associations_history(self): + self._get_association_logs(self.image) + class TestImageListRest(BaseTestImage): diff --git a/c2corg_api/tests/views/test_outing.py b/c2corg_api/tests/views/test_outing.py index 1776e4150..d83845ffe 100644 --- a/c2corg_api/tests/views/test_outing.py +++ b/c2corg_api/tests/views/test_outing.py @@ -1018,6 +1018,16 @@ def test_history_no_lang(self): def test_history_no_doc(self): self.app.get('/document/99999/history/es', status=404) + def test_get_associations_history(self): + logs = self._get_association_logs(self.outing) + + self.assertEqual(len(logs), 4) + + # Third association (starting from the youngest) is a user + log = logs[2] + parent = log['parent_document'] + self.assertEqual(parent["type"], USERPROFILE_TYPE) + def _assert_geometry(self, body): self.assertIsNotNone(body.get('geometry')) geometry = body.get('geometry') @@ -1144,22 +1154,38 @@ def _add_test_data(self): gear='paraglider')) self.session.add(self.route) self.session.flush() - self.session.add(Association.create( - parent_document=self.waypoint, - child_document=self.route)) - self.session.add(Association.create( - parent_document=self.route, - child_document=self.outing)) - - self.session.add(Association( - parent_document_id=user_id, - parent_document_type=USERPROFILE_TYPE, - child_document_id=self.outing.document_id, - child_document_type=OUTING_TYPE)) - self.session.add(Association.create( - parent_document=self.outing, - child_document=self.image)) - self.session.add(Association.create( - parent_document=self.outing, - child_document=self.article1)) + + self._add_association( + Association.create( + parent_document=self.waypoint, + child_document=self.route + ), + user_id=user_id) + self._add_association( + Association.create( + parent_document=self.route, + child_document=self.outing + ), + user_id=user_id) + + self._add_association( + Association( + parent_document_id=user_id, + parent_document_type=USERPROFILE_TYPE, + child_document_id=self.outing.document_id, + child_document_type=OUTING_TYPE + ), + user_id=user_id) + self._add_association( + Association.create( + parent_document=self.outing, + child_document=self.image + ), + user_id=user_id) + self._add_association( + Association.create( + parent_document=self.outing, + child_document=self.article1 + ), + user_id=user_id) self.session.flush() diff --git a/c2corg_api/tests/views/test_route.py b/c2corg_api/tests/views/test_route.py index 70daf4c27..b500b2bc5 100644 --- a/c2corg_api/tests/views/test_route.py +++ b/c2corg_api/tests/views/test_route.py @@ -1084,6 +1084,9 @@ def test_update_prefix_title(self): self.assertEqual( locale_es.title_prefix, self.waypoint.get_locale('fr').title) + def test_get_associations_history(self): + self._get_association_logs(self.route) + def _add_test_data(self): self.route = Route( activities=['skitouring'], elevation_max=1500, elevation_min=700, @@ -1120,17 +1123,17 @@ def _add_test_data(self): article_type='collab') self.session.add(self.article1) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.route, - child_document=self.article1)) + child_document=self.article1), user_id) self.book1 = Book(activities=['hiking'], book_types=['biography']) self.session.add(self.book1) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.book1, - child_document=self.route)) + child_document=self.route), user_id) self.route2 = Route( activities=['skitouring'], elevation_max=1500, elevation_min=700, @@ -1196,15 +1199,15 @@ def _add_test_data(self): access='yep')) self.session.add(self.waypoint2) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.route, - child_document=self.route4)) - self.session.add(Association.create( + child_document=self.route4), user_id) + self._add_association(Association.create( parent_document=self.route4, - child_document=self.route)) - self.session.add(Association.create( + child_document=self.route), user_id) + self._add_association(Association.create( parent_document=self.waypoint, - child_document=self.route)) + child_document=self.route), user_id) # add a map topo_map = TopoMap( @@ -1230,9 +1233,9 @@ def _add_test_data(self): ) self.session.add(self.outing1) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.route, - child_document=self.outing1)) + child_document=self.outing1), user_id) self.outing2 = Outing( redirects_to=self.outing1.document_id, @@ -1246,9 +1249,9 @@ def _add_test_data(self): ) self.session.add(self.outing2) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.route, - child_document=self.outing2)) + child_document=self.outing2), user_id) self.session.flush() # add areas diff --git a/c2corg_api/tests/views/test_waypoint.py b/c2corg_api/tests/views/test_waypoint.py index e81e1c5af..f50bd47eb 100644 --- a/c2corg_api/tests/views/test_waypoint.py +++ b/c2corg_api/tests/views/test_waypoint.py @@ -1436,6 +1436,9 @@ def search_documents(_, __): self.assertEqual( documents[1]['document_id'], self.waypoint2.document_id) + def test_get_associations_history(self): + self._get_association_logs(self.waypoint) + def _add_test_data(self): self.waypoint = Waypoint( waypoint_type='summit', elevation=2203) @@ -1533,18 +1536,18 @@ def _add_test_data(self): self.session.add(self.route2) self.session.add(self.route3) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.waypoint, - child_document=self.waypoint4)) - self.session.add(Association.create( + child_document=self.waypoint4), user_id) + self._add_association(Association.create( parent_document=self.waypoint, - child_document=self.route1)) - self.session.add(Association.create( + child_document=self.route1), user_id) + self._add_association(Association.create( parent_document=self.waypoint, - child_document=self.route2)) - self.session.add(Association.create( + child_document=self.route2), user_id) + self._add_association(Association.create( parent_document=self.waypoint4, - child_document=self.route3)) + child_document=self.route3), user_id) # article self.article1 = Article( @@ -1552,9 +1555,9 @@ def _add_test_data(self): article_type='collab') self.session.add(self.article1) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.waypoint, - child_document=self.article1)) + child_document=self.article1), user_id) self.article2 = Article( categories=['site_info'], activities=['hiking'], @@ -1575,9 +1578,9 @@ def _add_test_data(self): ) self.session.add(self.outing1) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.route1, - child_document=self.outing1)) + child_document=self.outing1), user_id) self.outing2 = Outing( redirects_to=self.outing1.document_id, @@ -1602,12 +1605,12 @@ def _add_test_data(self): self.session.add(self.outing2) self.session.add(self.outing3) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.route1, - child_document=self.outing2)) - self.session.add(Association.create( + child_document=self.outing2), user_id) + self._add_association(Association.create( parent_document=self.route3, - child_document=self.outing3)) + child_document=self.outing3), user_id) # add a map self.topo_map1 = TopoMap( diff --git a/c2corg_api/tests/views/test_xreport.py b/c2corg_api/tests/views/test_xreport.py index b8b6f4d8a..0cef113cb 100644 --- a/c2corg_api/tests/views/test_xreport.py +++ b/c2corg_api/tests/views/test_xreport.py @@ -742,6 +742,9 @@ def test_put_as_non_author(self): self.assertEqual(len(body['errors']), 1) self.assertEqual(body['errors'][0]['name'], 'Forbidden') + def test_get_associations_history(self): + self._get_association_logs(self.xreport1) + def _add_test_data(self): self.xreport1 = Xreport(activities=['hiking'], event_type=['stone_fall']) @@ -765,11 +768,11 @@ def _add_test_data(self): filter(DocumentVersion.lang == 'en').first() user_id3 = self.global_userids['contributor3'] - self.session.add(Association( + self._add_association(Association( parent_document_id=user_id3, parent_document_type=USERPROFILE_TYPE, child_document_id=self.xreport1.document_id, - child_document_type=XREPORT_TYPE)) + child_document_type=XREPORT_TYPE), user_id) self.xreport2 = Xreport(activities=['hiking'], event_type=['avalanche'], @@ -825,10 +828,10 @@ def _add_test_data(self): self.session.add(self.route3) self.session.flush() - self.session.add(Association.create( + self._add_association(Association.create( parent_document=self.outing3, - child_document=self.xreport1)) - self.session.add(Association.create( + child_document=self.xreport1), user_id) + self._add_association(Association.create( parent_document=self.route3, - child_document=self.xreport1)) + child_document=self.xreport1), user_id) self.session.flush() diff --git a/c2corg_api/views/association_history.py b/c2corg_api/views/association_history.py new file mode 100644 index 000000000..3583d785d --- /dev/null +++ b/c2corg_api/views/association_history.py @@ -0,0 +1,115 @@ +from sqlalchemy import or_ +from sqlalchemy.orm import joinedload +from cornice.resource import resource, view +from c2corg_api.views import cors_policy +from c2corg_api.views.validation import ( + validate_pagination, + validate_user_id_not_required, + validate_document_id_not_required) +from c2corg_api.models import DBSession +from c2corg_api.models.user import User +from c2corg_api.models.document import Document, DocumentLocale +from c2corg_api.models.user_profile import USERPROFILE_TYPE +from c2corg_api.models.association import AssociationLog + +# max/default sizes of requests +LIMIT_MAX = 500 +LIMIT_DEFAULT = 50 + + +@resource(path='/associations-history', cors_policy=cors_policy) +class HistoryAssociationRest(object): + """ Unique class for returning history of a document's associations. + """ + def __init__(self, request): + self.request = request + + @view(validators=[validate_document_id_not_required, + validate_pagination, + validate_user_id_not_required]) + def get(self): + validated = self.request.validated + document_id = validated.get('d', None) + user_id = validated.get('u', None) + offset = validated.get('offset', 0) + limit = min(validated.get('limit', LIMIT_DEFAULT), LIMIT_MAX) + + user_join = joinedload(AssociationLog.user) \ + .load_only( + User.id, + User.name, + User.forum_username, + User.robot, + User.moderator, + User.blocked + ) + + child_join = joinedload(AssociationLog.child_document) \ + .load_only(Document.document_id, Document.type) \ + .joinedload(Document.locales) \ + .load_only(DocumentLocale.title, DocumentLocale.lang) + + parent_join = joinedload(AssociationLog.parent_document) \ + .load_only(Document.document_id, Document.type) \ + .joinedload(Document.locales) \ + .load_only(DocumentLocale.title, DocumentLocale.lang) + + query = DBSession.query(AssociationLog) \ + .options(user_join) \ + .options(parent_join) \ + .options(child_join) + + if document_id: + query = query.filter(or_( + AssociationLog.parent_document_id == document_id, + AssociationLog.child_document_id == document_id) + ) + + if user_id: + query = query.filter(AssociationLog.user_id == user_id) + + count = query.count() + + query = query.order_by(AssociationLog.id.desc()) \ + .limit(limit) \ + .offset(offset) \ + .all() + + return { + 'count': count, + 'associations': [serialize_association_log(log) for log in query] + } + + +def serialize_document(document): + result = { + 'document_id': document.document_id, + 'type': document.type, + 'locales': [serialize_locale(locale) for locale in document.locales], + } + + if document.type == USERPROFILE_TYPE: + result['name'] = document.name + + return result + + +def serialize_locale(locale): + return {'lang': locale.lang, 'title': locale.title} + + +def serialize_association_log(log): + return { + 'written_at': log.written_at.isoformat(), + 'is_creation': log.is_creation, + 'user': { + 'user_id': log.user.id, + 'name': log.user.name, + 'forum_username': log.user.forum_username, + 'robot': log.user.robot, + 'moderator': log.user.moderator, + 'blocked': log.user.blocked + }, + 'child_document': serialize_document(log.child_document), + 'parent_document': serialize_document(log.parent_document), + } diff --git a/c2corg_api/views/validation.py b/c2corg_api/views/validation.py index d3d1683e8..c624319bf 100644 --- a/c2corg_api/views/validation.py +++ b/c2corg_api/views/validation.py @@ -209,6 +209,13 @@ def validate_user_id_not_required(request, **kwargs): check_get_for_integer_property(request, 'u', False) +def validate_document_id_not_required(request, **kwargs): + """ + Checks for a non-required document id parameter. + """ + check_get_for_integer_property(request, 'd', False) + + def parse_datetime(time_raw): if time_raw is None or time_raw == '': return None diff --git a/docker-compose.yml b/docker-compose.yml index 07cdc2bd4..28feea290 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,6 +18,7 @@ services: redis_url: 'redis://redis:6379/' version: '' volumes: + - ./alembic_migration:/var/www/alembic_migration - ./c2corg_api:/var/www/c2corg_api - ./Makefile:/var/www/Makefile - ./common.ini.in:/var/www/common.ini.in