Restapi refactor

prospector-more.yml

@@ -2,9 +2,6 @@ inherits: prospector
 
 strictness: medium
 
-ignore-paths:
-  - restapi/
-
 pylint:
   options:
     docstring-min-length: 20

readthedocs/restapi/client.py

@@ -1,3 +1,4 @@
+"""Simple client to access our API with Slumber credentials."""
 import logging
 
 from slumber import API, serialize

readthedocs/restapi/permissions.py

@@ -1,3 +1,4 @@
+"""Defines access permissions for the API."""
 from rest_framework import permissions
 from readthedocs.privacy.backend import AdminPermission
 

readthedocs/restapi/serializers.py

@@ -1,3 +1,4 @@
+"""Defines serializers for each of our models."""
 from rest_framework import serializers
 
 from readthedocs.builds.models import Build, BuildCommandResult, Version

readthedocs/restapi/signals.py

@@ -1,3 +1,4 @@
+"""We define custom Django signals to trigger when a footer is rendered."""
 import django.dispatch
 
 footer_response = django.dispatch.Signal(

readthedocs/restapi/urls.py

@@ -1,3 +1,4 @@
+"""Define routes between URL paths and views/endpoints."""
 from django.conf.urls import url, include
 
 from rest_framework import routers

readthedocs/restapi/utils.py

@@ -1,16 +1,15 @@
+"""Utility functions that are used by both views and celery tasks."""
 import hashlib
 import logging
 
-import requests
-
 from readthedocs.builds.constants import NON_REPOSITORY_VERSIONS
 from readthedocs.builds.models import Version
 from readthedocs.search.indexes import PageIndex, ProjectIndex, SectionIndex
 
 log = logging.getLogger(__name__)
 
 
-def sync_versions(project, versions, type):
+def sync_versions(project, versions, type):  # pylint: disable=redefined-builtin
     """Update the database with the current versions from the repository."""
     # Bookkeeping for keeping tag/branch identifies correct
     verbose_names = [v['verbose_name'] for v in versions]
@@ -39,8 +38,8 @@ def sync_versions(project, versions, type):
                     type=type,
                     machine=False,
                 )
-                log.info("(Sync Versions) Updated Version: [%s=%s] " % (
-                    version['verbose_name'], version['identifier']))
+                log.info("(Sync Versions) Updated Version: [%s=%s] ",
+                         version['verbose_name'], version['identifier'])
         else:
             # New Version
             created_version = Version.objects.create(
@@ -51,7 +50,7 @@ def sync_versions(project, versions, type):
             )
             added.add(created_version.slug)
     if added:
-        log.info("(Sync Versions) Added Versions: [%s] " % ' '.join(added))
+        log.info("(Sync Versions) Added Versions: [%s] ", ' '.join(added))
     return added
 
 
@@ -72,26 +71,20 @@ def delete_versions(project, version_data):
 
     if to_delete_qs.count():
         ret_val = {obj.slug for obj in to_delete_qs}
-        log.info("(Sync Versions) Deleted Versions: [%s]" % ' '.join(ret_val))
+        log.info("(Sync Versions) Deleted Versions: [%s]", ' '.join(ret_val))
         to_delete_qs.delete()
         return ret_val
     else:
         return set()
 
 
-def index_search_request(version, page_list, commit, project_scale, page_scale,
-                         section=True, delete=True):
-    """Update search indexes with build output JSON
-
-    In order to keep sub-projects all indexed on the same shard, indexes will be
-    updated using the parent project's slug as the routing value.
-    """
-    project = version.project
-
-    log_msg = ' '.join([page['path'] for page in page_list])
+def log_page_list(project, page_list):
+    log_msg = ' '.join(page['path'] for page in page_list)
     log.info("Updating search index: project=%s pages=[%s]",
              project.slug, log_msg)
 
+
+def index_project(project, project_scale):
     project_obj = ProjectIndex()
     project_obj.index_document(data={
         'id': project.pk,
@@ -105,18 +98,76 @@ def index_search_request(version, page_list, commit, project_scale, page_scale,
         'weight': project_scale,
     })
 
+
+def index_pages(routes, index_list, project):
     page_obj = PageIndex()
-    section_obj = SectionIndex()
-    index_list = []
-    section_index_list = []
-    routes = [project.slug]
-    routes.extend([p.parent.slug for p in project.superprojects.all()])
+    for route in routes:
+        page_obj.bulk_index(index_list, parent=project.slug, routing=route)
+
+
+def delete_pages(version, commit, project):
+    page_obj = PageIndex()
+    log.info("Deleting files not in commit: %s", commit)
+    # TODO: AK Make sure this works
+    delete_query = {
+        "query": {
+            "bool": {
+                "must": [
+                    {"term": {"project": project.slug, }},
+                    {"term": {"version": version.slug, }},
+                ],
+                "must_not": {
+                    "term": {
+                        "commit": commit
+                    }
+                }
+            }
+        }
+    }
+    page_obj.delete_document(body=delete_query)
+
+
+class SectionIndexer(object):
+
+    """Stores section metadata. Wraps state between calls."""
+
+    def __init__(self):
+        self.section_index_list = []
+        self.section_obj = SectionIndex()
+
+    def __call__(self, page, project, version, page_scale, page_id, routes):
+        for section in page['sections']:
+            self.section_index_list.append({
+                'id': (hashlib
+                       .md5('-'.join([project.slug, version.slug,
+                                     page['path'], section['id']]))
+                       .hexdigest()),
+                'project': project.slug,
+                'version': version.slug,
+                'path': page['path'],
+                'page_id': section['id'],
+                'title': section['title'],
+                'content': section['content'],
+                'weight': page_scale,
+            })
+        for route in routes:
+            self.section_obj.bulk_index(self.section_index_list, parent=page_id,
+                                        routing=route)
+
+
+def generate_index_list(version, project, page_list, commit, project_scale,
+                        page_scale, routes, index_sections):
+    """Generate an index dict per page.
+
+    If requested, also index the relevant page sections.
+    """
+    section_indexer = SectionIndexer()
     for page in page_list:
-        log.debug("Indexing page: %s:%s" % (project.slug, page['path']))
+        log.debug("Indexing page: %s:%s", project.slug, page['path'])
         page_id = (hashlib
                    .md5('-'.join([project.slug, version.slug, page['path']]))
                    .hexdigest())
-        index_list.append({
+        yield {
             'id': page_id,
             'project': project.slug,
             'version': version.slug,
@@ -127,45 +178,29 @@ def index_search_request(version, page_list, commit, project_scale, page_scale,
             'taxonomy': None,
             'commit': commit,
             'weight': page_scale + project_scale,
-        })
-        if section:
-            for section in page['sections']:
-                section_index_list.append({
-                    'id': (hashlib
-                           .md5('-'.join([project.slug, version.slug,
-                                         page['path'], section['id']]))
-                           .hexdigest()),
-                    'project': project.slug,
-                    'version': version.slug,
-                    'path': page['path'],
-                    'page_id': section['id'],
-                    'title': section['title'],
-                    'content': section['content'],
-                    'weight': page_scale,
-                })
-            for route in routes:
-                section_obj.bulk_index(section_index_list, parent=page_id,
-                                       routing=route)
+        }
+        if index_sections:
+            section_indexer(page, project, version, page_scale, routes, page_id)
 
-    for route in routes:
-        page_obj.bulk_index(index_list, parent=project.slug, routing=route)
+
+def index_search_request(version, page_list, commit, project_scale, page_scale,
+                         section=True, delete=True):
+    """Update search indexes with build output JSON
+
+    In order to keep sub-projects all indexed on the same shard, indexes will be
+    updated using the parent project's slug as the routing value.
+    """
+    project = version.project
+    log_page_list(project, page_list)
+    routes = [project.slug]
+    routes.extend(p.parent.slug for p in project.superprojects.all())
+
+    index_project(project, project_scale)
+    index_list = list(generate_index_list(
+        version, project, page_list, commit, project_scale, page_scale,
+        routes, index_sections=section
+    ))
+    index_pages(routes, index_list, project)
 
     if delete:
-        log.info("Deleting files not in commit: %s", commit)
-        # TODO: AK Make sure this works
-        delete_query = {
-            "query": {
-                "bool": {
-                    "must": [
-                        {"term": {"project": project.slug, }},
-                        {"term": {"version": version.slug, }},
-                    ],
-                    "must_not": {
-                        "term": {
-                            "commit": commit
-                        }
-                    }
-                }
-            }
-        }
-        page_obj.delete_document(body=delete_query)
+        delete_pages(version, commit, project)

readthedocs/restapi/views/core_views.py

@@ -1,3 +1,4 @@
+"""Utility endpoints relating to canonical urls, embedded content, etc."""
 from rest_framework import decorators, permissions, status
 from rest_framework.renderers import JSONRenderer
 from rest_framework.response import Response