hit database less to improve api performance (#515)

Makefile

@@ -10,6 +10,7 @@ manage=$(python) $(manage-script)
 db=sqlite
 pyvenv=python3 -m venv
 yarn=yarn
+prod_version=12.0.0
 
 .PHONY: lint
 lint:
@@ -82,7 +83,11 @@ clean:
 
 .PHONY: test-data
 test-data: test-user
-	$(python) $(CURDIR)/scripts/development/testdata.py
+	PYTHONPATH="${PYTHONPATH}:$(CURDIR)/scripts/" $(python) -m development.testdata
+
+.PHONY: prod-data
+prod-data:
+	PYTHONPATH="${PYTHONPATH}:$(CURDIR)/scripts/" $(python) -m development.proddata $(prod_version)
 
 .PHONY: l10n
 l10n:

docs/storedeveloper.rst

@@ -27,6 +27,9 @@ make resetup
 make test-data
     downloads and sets up test apps, needs certificate validation to be disabled and a running server at http://127.0.0.1:8000
 
+make prod-data prod_version=12.0.0
+    similar to **make test-data** but installs all apps from production for a nextcloud version locally
+
 make l10n
     compiles and installs translations
 

nextcloudappstore/api/v1/release/pre-info.xslt

@@ -24,6 +24,9 @@
             <xsl:apply-templates select="types"/>
             <xsl:apply-templates select="documentation"/>
             <xsl:apply-templates select="category"/>
+            <xsl:if test="not(category)">
+                <category>tools</category>
+            </xsl:if>
             <xsl:copy-of select="website"/>
             <xsl:copy-of select="discussion"/>
             <xsl:copy-of select="bugs"/>

nextcloudappstore/api/v1/serializers.py

@@ -98,17 +98,13 @@ class ScreenshotSerializer(serializers.ModelSerializer):
 
 
 class AppSerializer(serializers.ModelSerializer):
-    releases = SerializerMethodField()
+    releases = AppReleaseSerializer(many=True, read_only=True)
     discussion = SerializerMethodField()
     screenshots = ScreenshotSerializer(many=True, read_only=True)
     authors = AuthorSerializer(many=True, read_only=True)
     translations = TranslatedFieldsField(shared_model=App)
     last_modified = DateTimeField(source='last_release')
 
-    def __init__(self, *args, **kwargs):
-        self.version = kwargs.pop('version')
-        super().__init__(*args, **kwargs)
-
     class Meta:
         model = App
         fields = (
@@ -122,21 +118,6 @@ class AppSerializer(serializers.ModelSerializer):
     def get_discussion(self, obj):
         return obj.discussion_url
 
-    def get_releases(self, obj):
-        releases = obj.releases.prefetch_related(
-            'translations',
-            'databases',
-            'licenses',
-            'phpextensiondependencies__php_extension',
-            'databasedependencies__database',
-            'shell_commands',
-        ).all()
-        if self.version:
-            data = [r for r in releases if r.is_compatible(self.version)]
-        else:
-            data = releases
-        return AppReleaseSerializer(data, many=True, read_only=True).data
-
 
 class UserSerializer(serializers.ModelSerializer):
     class Meta:

nextcloudappstore/api/v1/views.py

@@ -24,6 +24,7 @@ from nextcloudappstore.core.facades import read_file_contents
 from nextcloudappstore.core.models import App, AppRelease, Category, AppRating
 from nextcloudappstore.core.permissions import UpdateDeletePermission
 from nextcloudappstore.core.throttling import PostThrottle
+from nextcloudappstore.core.versioning import version_in_spec
 from nextcloudappstore.user.facades import update_token
 
 
@@ -46,10 +47,39 @@ class AppView(DestroyAPIView):
 
     def get(self, request, *args, **kwargs):
         version = self.kwargs['version']
-        working_apps = App.objects.get_compatible(version)
-        serializer = self.get_serializer(working_apps, many=True,
-                                         version=version)
-        return Response(serializer.data)
+        prefetch = [
+            'authors',
+            'screenshots',
+            'categories',
+            'translations',
+            'releases__translations',
+            'releases__phpextensiondependencies__php_extension',
+            'releases__databasedependencies__database',
+            'releases__shell_commands',
+            'releases__licenses',
+        ]
+        working_apps = App.objects.get_compatible(version, prefetch=prefetch)
+        serializer = self.get_serializer(working_apps, many=True)
+        data = self._filter_releases(serializer.data, version)
+        return Response(data)
+
+    def _filter_releases(self, data, version):
+        """
+        Story time: this was once done in serializers but turned out to cause
+        an extreme amount of queries. So we fetch everything by default and
+        then filter out unneeded releases that dont match the version
+        :param data: the serialized data
+        :param version: the Nextcloud version that we filter
+        :return: a filtered result set to be serialized
+        """
+
+        def is_compatible(release) -> bool:
+            spec = release['platform_version_spec'].replace(' ', ',')
+            return version_in_spec(version, spec)
+
+        for app in data:
+            app['releases'] = list(filter(is_compatible, app['releases']))
+        return data
 
 
 class AppRegisterView(APIView):

nextcloudappstore/core/models.py

@@ -36,18 +36,13 @@ class AppManager(TranslatableManager):
         query = reduce(lambda x, y: x & y, predicates, Q())
         return queryset.filter(query)
 
-    def get_compatible(self, platform_version, inclusive=False):
-        apps = App.objects.prefetch_related(
-            'releases',
-            'releases__translations',
-            'releases__databases',
-            'releases__licenses',
-            'releases__phpextensiondependencies__php_extension',
-            'releases__databasedependencies__database',
-            'releases__shell_commands',
-            'translations',
-            'screenshots'
-        ).all()
+    def get_compatible(self, platform_version, inclusive=False, prefetch=None,
+                       select=None):
+        qs = App.objects
+        if select is not None and len(select) > 0:
+            qs = qs.select_related(*select)
+        if prefetch is not None and len(prefetch) > 0:
+            qs = qs.prefetch_related(*prefetch)
 
         def app_filter(app):
             for release in app.releases.all():
@@ -55,7 +50,7 @@ class AppManager(TranslatableManager):
                     return True
             return False
 
-        return list(filter(app_filter, apps))
+        return list(filter(app_filter, qs.all()))
 
 
 class App(TranslatableModel):

nextcloudappstore/core/versioning.py

@@ -3,7 +3,7 @@ from functools import reduce
 from sys import maxsize
 from typing import Dict, Any, List
 
-from semantic_version import Version
+from semantic_version import Version, Spec
 
 SEMVER_REGEX = (r'(?:0|[1-9][0-9]*)'
                 r'\.(?:0|[1-9][0-9]*)'
@@ -148,3 +148,13 @@ def group_by_main_version(versions: GroupedVersions) -> GroupedVersions:
         return prev
 
     return reduce(reduction, versions.items(), {})
+
+
+def version_in_spec(version: str, spec: str) -> bool:
+    """
+    Checks if a string version is in a spec
+    :param version:
+    :param spec:
+    :return:
+    """
+    return Version(version) in Spec(spec)

nextcloudappstore/core/views.py

@@ -48,8 +48,12 @@ class AppRatingApi(ListAPIView):
 
 
 class AppDetailView(DetailView):
-    queryset = App.objects.prefetch_related('releases', 'screenshots', 'owner',
-                                            'co_maintainers')
+    queryset = App.objects.prefetch_related(
+        'releases',
+        'screenshots',
+        'co_maintainers',
+        'translations',
+    ).select_related('owner')
     template_name = 'app/detail.html'
     slug_field = 'id'
     slug_url_kwarg = 'id'
@@ -123,12 +127,12 @@ class AppDetailView(DetailView):
 
 class AppReleasesView(DetailView):
     queryset = App.objects.prefetch_related(
-        'releases',
-        'releases__databases',
-        'releases__licenses',
+        'translations',
+        'releases__translations',
         'releases__phpextensiondependencies__php_extension',
         'releases__databasedependencies__database',
-        'releases__shell_commands'
+        'releases__shell_commands',
+        'releases__licenses',
     )
     template_name = 'app/releases.html'
     slug_field = 'id'

scripts/development/__init__.py

@@ -0,0 +1,34 @@
+import sys
+from typing import Tuple
+
+from requests import post
+
+REGISTER_URL = 'http://127.0.0.1:8000/api/v1/apps'
+RELEASE_URL = 'http://127.0.0.1:8000/api/v1/apps/releases'
+ADMIN = ('admin', 'admin')
+
+
+def handle_response(response) -> None:
+    if response.status_code > 299:
+        msg = 'Request to url %s failed with status code %d'
+        print(msg % (response.url, response.status_code), file=sys.stderr)
+        print(response.text, file=sys.stderr)
+
+
+def import_app(certificate: str, signature: str, auth=Tuple[str, str]) -> None:
+    response = post(REGISTER_URL, auth=auth, json={
+        'signature': signature,
+        'certificate': certificate
+    })
+    handle_response(response)
+
+
+def import_release(url: str, signature: str, nightly: bool,
+                   auth=Tuple[str, str]) -> None:
+    print('Downloading app from %s' % url)
+    response = post(RELEASE_URL, auth=auth, json={
+        'download': url,
+        'signature': signature,
+        'nightly': nightly
+    })
+    handle_response(response)

scripts/development/proddata.py

@@ -0,0 +1,24 @@
+"""
+Queries the live API and import all releases locally for a Nextcloud version
+"""
+import sys
+
+from requests import get
+
+from . import import_app, ADMIN, import_release
+
+APPS_URL = 'https://apps.nextcloud.com/api/v1/platform/%s/apps.json'
+
+
+def main():
+    version = sys.argv[1]
+    apps = get(APPS_URL % version).json()
+    for app in apps:
+        import_app(app['certificate'], 'signature', ADMIN)
+        for release in app['releases']:
+            import_release(release['download'], release['signature'],
+                           release['isNightly'], ADMIN)
+
+
+if __name__ == '__main__':
+    main()

scripts/development/settings/base.py

@@ -21,3 +21,9 @@ LOGGING['handlers']['console'] = {
     'class': 'logging.StreamHandler',
 }
 LOGGING['loggers']['django']['handlers'] += ['console']
+
+# make it possible to run debug toolbar for api
+CSP_EXCLUDE_URL_PREFIXES = ('/api/v1',)
+
+REST_FRAMEWORK['DEFAULT_THROTTLE_RATES']['app_upload'] = '10000/day'
+REST_FRAMEWORK['DEFAULT_THROTTLE_RATES']['app_register'] = '10000/day'

scripts/development/testdata.py

@@ -1,4 +1,4 @@
-import requests
+from . import import_app, import_release, ADMIN
 
 twofactor_cert = """
 -----BEGIN CERTIFICATE-----
@@ -101,31 +101,13 @@ apps = [{
     }]
 }]
 
-admin = ('admin', 'admin')
+
+def main():
+    for app in apps:
+        import_app(app['certificate'], 'signature', ADMIN)
+        for release in app['releases']:
+            import_release(release['url'], 'signature', False, ADMIN)
 
 
-def handle_response(response):
-    if response.status_code > 299:
-        msg = 'Request to url %s failed with status code %d'
-        print(msg % (response.url, response.status_code))
-        print(response.text)
-
-
-for app in apps:
-    response = requests.post('http://127.0.0.1:8000/api/v1/apps',
-                             auth=admin,
-                             json={
-                                 'signature': 'signature',
-                                 'certificate': app['certificate']
-                             })
-    handle_response(response)
-    for release in app['releases']:
-        print('Downloading app from %s' % release['url'])
-        response = requests.post('http://127.0.0.1:8000/api/v1/apps/releases',
-                                 auth=admin,
-                                 json={
-                                     'download': release['url'],
-                                     'signature': 'signature',
-                                     'nightly': False
-                                 })
-        handle_response(response)
+if __name__ == '__main__':
+    main()