Merge pull request #752 from c2corg/fix-wrong-doc-type

When asked doc type is wrong, returns 404 io 200

.travis.yml

@@ -1,3 +1,5 @@
+dist: trusty
+
 env:
   global:
   - secure: XV/MM2PoGNhAWuK4Us1cSmGFtXOMMmKyYm0Ir+mEQLEiNdrzBgiKEyY9ywwp8o+SYOs/uB6Hp3gJO4cRojZypg4Gcify0yJjNbyN9Lipkb7BvHXv/8RC+P/TlnXiRJDuTXbdtkEsXkEnGv4RdfcNHS12m2GZPBjzgQ1mznzP3DUEaAFzXJddN1n+Yk1Sc3rCRKiBYgAVzMBPEnT4KFZISvGsGdkae1I7Sq/IJrx599GeoI6dNRkHXURqNq3kcJj4jU4UUQbhKLn0+jpbX0fLQV+PRQ0sup0EGtwu0S8q+5RwniSOrNUtf552mUDwtAcI4mBbZmZ81n0Bf3qlRVqECuN+2qihN61+m5/eWhX8C195qEwy3EpHsgPBe52P54oEvgrXWGXbnYQC3rlegnXd8FUzQaq9G397kUraWgfd0OCp2zmPTLNIqCWpBqhYNfuB4xEP7fOTxugPJqvbPnZLHqqTkbK4jnsaKW50dV+CTKiUpo1dXfkCtNJbNH2wb6bjnztKkm8UUYLIMCH4WFz2wiRsUZ6xKS3eDbcAqPyveuf9Ki8oW8MyPFdOV2kOpnG79a9rtu2Nyg4FMgtlIjkl+nD+gkr+ElPCG59wOKPMa4gUKFQ1Z9h7EfZCRL6ux2RzRekNRuXNq5nebjVqO7dGUF7Z8X97xukjEban/t5nymU=

c2corg_api/models/cache_version.py

@@ -163,7 +163,7 @@ def _format_cache_key(document_id, lang, version, doc_type=None,
     return cache_key
 
 
-def get_cache_key(document_id, lang, document_type=None,
+def get_cache_key(document_id, lang, document_type,
                   custom_cache_key=None):
     """ Returns an identifier which reflects the version of a document and
     all its associated documents. This identifier is used as cache key

c2corg_api/tests/views/__init__.py

@@ -3,7 +3,8 @@
 import urllib.parse
 import urllib.error
 
-from c2corg_api.models.cache_version import CacheVersion
+from c2corg_api.caching import cache_document_detail
+from c2corg_api.models.cache_version import CacheVersion, get_cache_key
 from c2corg_api.models.feed import DocumentChange
 from c2corg_api.models.route import Route
 from c2corg_api.models.user import User
@@ -12,6 +13,7 @@
 from c2corg_api.search import elasticsearch_config, search_documents
 from c2corg_api.tests import BaseTestCase
 from dateutil import parser as datetime_parser
+from dogpile.cache.api import NO_VALUE
 
 
 class BaseTestRest(BaseTestCase):
@@ -243,6 +245,39 @@ def get(self, reference, user=None, check_title=True, ignore_checks=False):
             self.assertCountEqual(available_langs, ['en', 'fr'])
         return body
 
+    def get_caching(self, reference, user=None):
+        headers = {} if not user else \
+            self.add_authorization_header(username=user)
+
+        url = '{0}/{1}'.format(self._prefix, str(reference.document_id))
+        cache_key = get_cache_key(
+            reference.document_id,
+            None,
+            document_type=self._doc_type)
+
+        cache_value = cache_document_detail.get(cache_key)
+
+        self.assertEqual(cache_value, NO_VALUE)
+
+        # check that the response is cached
+        self.app.get(url, headers=headers, status=200)
+
+        cache_value = cache_document_detail.get(cache_key)
+        self.assertNotEqual(cache_value, NO_VALUE)
+
+        # check that values are returned from the cache
+        fake_cache_value = {'document': 'fake doc'}
+        cache_document_detail.set(cache_key, fake_cache_value)
+
+        response = self.app.get(url, headers=headers, status=200)
+        body = response.json
+        self.assertEqual(body, fake_cache_value)
+
+        # check that cache handles document types
+        prefix = "/routes" if self._prefix != "/routes" else "/waypoint"
+        url = '{0}/{1}'.format(prefix, str(reference.document_id))
+        self.app.get(url, headers=headers, status=404)
+
     def get_version(self, reference, reference_version, user=None):
         headers = {} if not user else \
             self.add_authorization_header(username=user)

c2corg_api/tests/views/test_area.py

@@ -90,6 +90,9 @@ def test_get_new_lang(self):
     def test_get_404(self):
         self.get_404()
 
+    def test_get_caching(self):
+        self.get_caching(self.area1)
+
     def test_get_info(self):
         body, locale = self.get_info(self.area1, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/tests/views/test_article.py

@@ -123,7 +123,7 @@ def test_get_version_caching(self):
                 self._prefix, str(self.article1.document_id),
                 str(self.article1_version.id))
         cache_key = '{0}-{1}'.format(
-            get_cache_key(self.article1.document_id, 'en'),
+            get_cache_key(self.article1.document_id, 'en', ARTICLE_TYPE),
             self.article1_version.id)
 
         cache_value = cache_document_version.get(cache_key)
@@ -143,6 +143,9 @@ def test_get_version_caching(self):
         body = response.json
         self.assertEqual(body, fake_cache_value)
 
+    def test_get_caching(self):
+        self.get_caching(self.article1)
+
     def test_get_info(self):
         body, locale = self.get_info(self.article1, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/tests/views/test_book.py

@@ -125,7 +125,7 @@ def test_get_version_caching(self):
                 self._prefix, str(self.book1.document_id),
                 str(self.book1_version.id))
         cache_key = '{0}-{1}'.format(
-            get_cache_key(self.book1.document_id, 'en'),
+            get_cache_key(self.book1.document_id, 'en', BOOK_TYPE),
             self.book1_version.id)
 
         cache_value = cache_document_version.get(cache_key)
@@ -145,6 +145,9 @@ def test_get_version_caching(self):
         body = response.json
         self.assertEqual(body, fake_cache_value)
 
+    def test_get_caching(self):
+        self.get_caching(self.book1)
+
     def test_get_info(self):
         body, locale = self.get_info(self.book1, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/tests/views/test_image.py

@@ -327,6 +327,9 @@ def test_post_error(self):
         self.assertEqual(len(errors), 1)
         self.assertCorniceRequired(errors[0], 'filename')
 
+    def test_get_caching(self):
+        self.get_caching(self.image)
+
     def test_get_info(self):
         body, locale = self.get_info(self.image, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/tests/views/test_outing.py

@@ -209,6 +209,9 @@ def test_get_new_lang(self):
     def test_get_404(self):
         self.get_404()
 
+    def test_get_caching(self):
+        self.get_caching(self.outing)
+
     def test_get_info(self):
         body, locale = self.get_info(self.outing, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/tests/views/test_route.py

@@ -203,6 +203,9 @@ def test_get_edit(self):
         self.assertNotIn('images', associations)
         self.assertNotIn('users', associations)
 
+    def test_get_caching(self):
+        self.get_caching(self.route)
+
     def test_get_info(self):
         body, locale = self.get_info(self.route, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/tests/views/test_topo_map.py

@@ -74,6 +74,9 @@ def test_get_new_lang(self):
     def test_get_404(self):
         self.get_404()
 
+    def test_get_caching(self):
+        self.get_caching(self.map1)
+
     def test_get_info(self):
         body, locale = self.get_info(self.map1, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/tests/views/test_user_profile.py

@@ -128,6 +128,9 @@ def test_get_new_lang(self):
     def test_get_404(self):
         self.get_404(user='contributor')
 
+    def test_get_caching(self):
+        self.get_caching(self.profile1, user='contributor')
+
     def test_get_info(self):
         body, locale = self.get_info(self.profile1, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/tests/views/test_waypoint.py

@@ -2,7 +2,7 @@
 import json
 from unittest.mock import patch
 
-from c2corg_api.caching import cache_document_detail, cache_document_listing, \
+from c2corg_api.caching import cache_document_listing, \
     cache_document_history, cache_document_version
 from c2corg_common.utils import caching
 from c2corg_api.models.area import Area
@@ -29,7 +29,7 @@
     Waypoint, WaypointLocale, ArchiveWaypoint, ArchiveWaypointLocale,
     WAYPOINT_TYPE)
 from c2corg_api.models.document import (
-    DocumentGeometry, ArchiveDocumentGeometry, DocumentLocale)
+    DocumentGeometry, ArchiveDocumentGeometry, DocumentLocale, DOCUMENT_TYPE)
 from c2corg_api.models.document_topic import DocumentTopic
 from c2corg_api.views.document import DocumentRest
 
@@ -268,7 +268,7 @@ def test_get_version_caching(self):
                 self._prefix, str(self.waypoint.document_id),
                 str(self.waypoint_version.id))
         cache_key = '{0}-{1}'.format(
-            get_cache_key(self.waypoint.document_id, 'en'),
+            get_cache_key(self.waypoint.document_id, 'en', WAYPOINT_TYPE),
             self.waypoint_version.id)
 
         cache_value = cache_document_version.get(cache_key)
@@ -336,26 +336,7 @@ def test_get_etag(self):
                                 status=304, headers=headers)
 
     def test_get_caching(self):
-        waypoint_id = self.waypoint.document_id
-        cache_key = get_cache_key(waypoint_id, None)
-
-        cache_value = cache_document_detail.get(cache_key)
-        self.assertEqual(cache_value, NO_VALUE)
-
-        # check that the response is cached
-        self.app.get(self._prefix + '/' + str(waypoint_id), status=200)
-
-        cache_value = cache_document_detail.get(cache_key)
-        self.assertNotEqual(cache_value, NO_VALUE)
-
-        # check that values are returned from the cache
-        fake_cache_value = {'document_id': 'fake_id'}
-        cache_document_detail.set(cache_key, fake_cache_value)
-
-        response = self.app.get(
-            self._prefix + '/' + str(waypoint_id), status=200)
-        body = response.json
-        self.assertEqual(body, fake_cache_value)
+        self.get_caching(self.waypoint)
 
     def test_get_cache_down(self):
         """ Check that the request does not fail even if Redis errors.
@@ -1392,7 +1373,7 @@ def test_history_etag(self):
 
     def test_history_caching(self):
         waypoint_id = self.waypoint.document_id
-        cache_key = get_cache_key(waypoint_id, 'fr')
+        cache_key = get_cache_key(waypoint_id, 'fr', DOCUMENT_TYPE)
 
         cache_value = cache_document_history.get(cache_key)
         self.assertEqual(cache_value, NO_VALUE)

c2corg_api/tests/views/test_xreport.py

@@ -199,7 +199,7 @@ def test_get_version_caching(self):
                 self._prefix, str(self.xreport1.document_id),
                 str(self.xreport1_version.id))
         cache_key = '{0}-{1}'.format(
-            get_cache_key(self.xreport1.document_id, 'en'),
+            get_cache_key(self.xreport1.document_id, 'en', XREPORT_TYPE),
             self.xreport1_version.id)
 
         cache_value = cache_document_version.get(cache_key)
@@ -219,6 +219,9 @@ def test_get_version_caching(self):
         body = response.json
         self.assertEqual(body, fake_cache_value)
 
+    def test_get_caching(self):
+        self.get_caching(self.xreport1)
+
     def test_get_info(self):
         body, locale = self.get_info(self.xreport1, 'en')
         self.assertEqual(locale.get('lang'), 'en')

c2corg_api/views/area.py

@@ -39,7 +39,10 @@ def collection_get(self):
 
     @view(validators=[validate_id, validate_lang_param, validate_cook_param])
     def get(self):
-        return self._get(Area, schema_area, include_areas=False)
+        return self._get(
+            area_documents_config,
+            schema_area,
+            include_areas=False)
 
     @restricted_json_view(
             schema=schema_create_area,
@@ -74,7 +77,7 @@ class AreaInfoRest(DocumentInfoRest):
 
     @view(validators=[validate_id, validate_lang])
     def get(self):
-        return self._get_document_info(Area)
+        return self._get_document_info(area_documents_config)
 
 
 def insert_associations(area, user_id):
@@ -101,4 +104,4 @@ class AreaVersionRest(DocumentVersionRest):
     @view(validators=[validate_id, validate_lang, validate_version_id])
     def get(self):
         return self._get_version(
-            ArchiveArea, ArchiveDocumentLocale, schema_area)
+            ArchiveArea, AREA_TYPE, ArchiveDocumentLocale, schema_area)

c2corg_api/views/article.py

@@ -46,7 +46,7 @@ def collection_get(self):
     @view(validators=[validate_id, validate_lang_param, validate_cook_param])
     def get(self):
         return self._get(
-            Article, schema_article, include_areas=False,
+            article_documents_config, schema_article, include_areas=False,
             set_custom_fields=set_author)
 
     @restricted_json_view(
@@ -94,14 +94,17 @@ class ArticlesVersionRest(DocumentVersionRest):
     @view(validators=[validate_id, validate_lang, validate_version_id])
     def get(self):
         return self._get_version(
-            ArchiveArticle, ArchiveDocumentLocale, schema_article)
+            ArchiveArticle,
+            ARTICLE_TYPE,
+            ArchiveDocumentLocale,
+            schema_article)
 
 
 @resource(path='/articles/{id}/{lang}/info', cors_policy=cors_policy)
 class ArticlesInfoRest(DocumentInfoRest):
     @view(validators=[validate_id, validate_lang])
     def get(self):
-        return self._get_document_info(Article)
+        return self._get_document_info(article_documents_config)
 
 
 def set_author(article):

c2corg_api/views/book.py

@@ -40,7 +40,10 @@ def collection_get(self):
 
     @view(validators=[validate_id, validate_lang_param, validate_cook_param])
     def get(self):
-        return self._get(Book, schema_book, include_areas=False)
+        return self._get(
+            book_documents_config,
+            schema_book,
+            include_areas=False)
 
     @restricted_json_view(
             schema=schema_create_book,
@@ -66,11 +69,11 @@ class BooksVersionRest(DocumentVersionRest):
     @view(validators=[validate_id, validate_lang, validate_version_id])
     def get(self):
         return self._get_version(
-            ArchiveBook, ArchiveDocumentLocale, schema_book)
+            ArchiveBook, BOOK_TYPE, ArchiveDocumentLocale, schema_book)
 
 
 @resource(path='/books/{id}/{lang}/info', cors_policy=cors_policy)
 class BooksInfoRest(DocumentInfoRest):
     @view(validators=[validate_id, validate_lang])
     def get(self):
-        return self._get_document_info(Book)
+        return self._get_document_info(book_documents_config)

c2corg_api/views/document.py

@@ -62,6 +62,7 @@ class DocumentRest(object):
     def __init__(self, request):
         self.request = request
 
+    # TODO: remove doc_type, it's in documents_config
     def _collection_get(self, doc_type, documents_config):
         validated = self.request.validated
         meta_params = {
@@ -105,8 +106,8 @@ def _search_documents_paginated(
 
         return document_ids, total
 
-    def _get(self, clazz, schema, clazz_locale=None, adapt_schema=None,
-             include_maps=False, include_areas=True,
+    def _get(self, document_config, schema, clazz_locale=None,
+             adapt_schema=None, include_maps=False, include_areas=True,
              set_custom_associations=None, set_custom_fields=None,
              custom_cache_key=None):
         id = self.request.validated['id']
@@ -131,13 +132,17 @@ def _get(self, clazz, schema, clazz_locale=None, adapt_schema=None,
 
         def create_response():
             return self._get_in_lang(
-                id, lang, clazz, schema, editing_view, clazz_locale,
-                adapt_schema, include_maps, include_areas,
+                id, lang, document_config.clazz, schema, editing_view,
+                clazz_locale, adapt_schema, include_maps, include_areas,
                 set_custom_associations, set_custom_fields,
                 cook_locale=cook)
 
         if not editing_view:
-            cache_key = get_cache_key(id, lang, custom_cache_key)
+            cache_key = get_cache_key(
+                id,
+                lang,
+                document_type=document_config.document_type,
+                custom_cache_key=custom_cache_key)
 
             if cache_key:
                 # set and check the etag: if the etag value provided in the

c2corg_api/views/document_history.py

@@ -1,7 +1,7 @@
 from c2corg_api.caching import cache_document_history
 from c2corg_api.models import DBSession
 from c2corg_api.models.cache_version import get_cache_key
-from c2corg_api.models.document import DocumentLocale
+from c2corg_api.models.document import DocumentLocale, DOCUMENT_TYPE
 from c2corg_api.models.document_history import DocumentVersion
 from c2corg_api.views import cors_policy, etag_cache
 from c2corg_api.views.document_version import serialize_version
@@ -28,7 +28,12 @@ def get(self):
         def create_response():
             return self._get_history(document_id, lang)
 
-        cache_key = get_cache_key(document_id, lang)
+        # history entry point does no precise document type.
+        cache_key = get_cache_key(
+            document_id,
+            lang,
+            document_type=DOCUMENT_TYPE)
+
         if not cache_key:
             raise HTTPNotFound(
                 'no version for document {0}'.format(document_id))