From 4613f661b6a0f3fb66458b34713d1f81a02d89c9 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 6 Nov 2026 12:26:04 +0000 Subject: [PATCH] perf(api): list only the releases a server can pick in apps.json Related #1951 apps.json lists all releases of every app, about 18,000 on production, but a Nextcloud server keeps one per app. It filters the releases on their raw platform and PHP version specs, the nightly flag and whether the version contains "-", then takes the highest version left according to PHP's version_compare(). Of the releases that agree on those four values, only the highest can ever be picked, so leaving out the others changes nothing a server does. On production's catalogue that keeps 5,913 of 18,267 releases. The list falls from 16.1 MB to 5.8 MB with gzip, and on the local copy of production a build through Apache takes about 5 s instead of 9 to 10 s. The rule is a list serializer used only by apps.json, with a port of version_compare() as of PHP 8.4.22 and 8.5.7 for the comparison. Releases that compare equal to the newest of their set are all kept, because which of them a server picks depends on its sort order. A set with a version that ends in a character other than a letter or digit is kept whole: older PHP orders such versions inconsistently, and none exists in the catalogue today. platform//apps.json and appapi_apps.json are unchanged, and the API docs describe the rule. Signed-off-by: Oleksander Piskun --- docs/api/restapi.rst | 2 + nextcloudappstore/api/v1/serializers.py | 49 ++++++++ nextcloudappstore/api/v1/tests/test_app.py | 65 ++++++++++ nextcloudappstore/api/v1/views.py | 3 +- .../core/tests/test_versioning.py | 34 +++++ nextcloudappstore/core/versioning.py | 118 ++++++++++++++++++ 6 files changed, 270 insertions(+), 1 deletion(-) diff --git a/docs/api/restapi.rst b/docs/api/restapi.rst index 64e4e33f0c8..eb01627142c 100644 --- a/docs/api/restapi.rst +++ b/docs/api/restapi.rst @@ -490,6 +490,8 @@ Get All Apps and Releases ~~~~~~~~~~~~~~~~~~~~~~~~~ This route will return all releases to display inside Nextcloud's apps admin area. +.. note:: For each app, a release is left out when another release has the same **rawPlatformVersionSpec**, **rawPhpVersionSpec** and **isNightly**, is a pre-release (a version containing **-**) exactly when it is, and has a higher version. Nextcloud filters releases on those values and installs the highest version left, so such a release can never be picked. Releases that share those values are all kept if one of their versions ends in a character other than a letter or digit. :ref:`api-all-compatible-releases` still returns every compatible release. + * **Url**: GET /api/v1/apps.json * **Headers**: diff --git a/nextcloudappstore/api/v1/serializers.py b/nextcloudappstore/api/v1/serializers.py index 68a8d17ce5b..9361b0cf045 100644 --- a/nextcloudappstore/api/v1/serializers.py +++ b/nextcloudappstore/api/v1/serializers.py @@ -3,7 +3,11 @@ SPDX-License-Identifier: AGPL-3.0-or-later """ +from collections import defaultdict +from functools import cmp_to_key + from django.contrib.auth import get_user_model +from django.db.models.manager import BaseManager from parler_rest.fields import TranslatedFieldsField from parler_rest.serializers import TranslatableModelSerializer from rest_framework import serializers @@ -25,6 +29,7 @@ Video, ) from nextcloudappstore.core.validators import HttpsUrlValidator +from nextcloudappstore.core.versioning import php_version_compare class PhpExtensionDependencySerializer(serializers.ModelSerializer): @@ -224,6 +229,50 @@ def get_discussion(self, obj): return obj.discussion_url +class NewestReleasesSerializer(serializers.ListSerializer): + """ + Lists only the releases a Nextcloud server can pick. A server filters an app's releases on their raw platform and + PHP version specs, nightly flag and whether the version is a pre-release, then installs the highest version left + according to PHP's version_compare(). Of releases that agree on those four values, only the highest can be picked. + + A set with a version ending in a character other than an ASCII letter or digit is kept whole: PHP before 8.4.22 + and 8.5.7 orders such versions inconsistently, so which one a server picks depends on how it sorts. + """ + + def to_representation(self, data): + releases = list(data.all() if isinstance(data, BaseManager) else data) + groups = defaultdict(list) + for release in releases: + groups[self._group(release)].append(release) + selectable = set() + for group in groups.values(): + if not all(self._ends_in_letter_or_digit(release.version) for release in group): + selectable.update(id(release) for release in group) + continue + newest = max(group, key=cmp_to_key(lambda a, b: php_version_compare(a.version, b.version))) + selectable.update( + id(release) for release in group if php_version_compare(release.version, newest.version) == 0 + ) + return super().to_representation([release for release in releases if id(release) in selectable]) + + @staticmethod + def _ends_in_letter_or_digit(version): + return version[-1:].isascii() and version[-1:].isalnum() + + @staticmethod + def _group(release): + return ( + release.raw_platform_version_spec, + release.raw_php_version_spec, + release.is_nightly, + "-" in release.version, + ) + + +class AppWithNewestReleasesSerializer(AppSerializer): + releases = NewestReleasesSerializer(child=AppReleaseSerializer(), read_only=True) + + class AppApiAppSerializer(AppSerializer): releases = AppApiAppReleaseSerializer(many=True, read_only=True) diff --git a/nextcloudappstore/api/v1/tests/test_app.py b/nextcloudappstore/api/v1/tests/test_app.py index 4f16a947df6..74138dc3dcc 100644 --- a/nextcloudappstore/api/v1/tests/test_app.py +++ b/nextcloudappstore/api/v1/tests/test_app.py @@ -97,6 +97,71 @@ def test_appapi_includes_enterprise_with_valid_key(self): self.assertIn("news", ids) self.assertIn("enterprise", ids) + def _create_releases(self, app, releases): + for version, platform, php, is_nightly in releases: + AppRelease.objects.create( + app=app, + version=version, + platform_version_spec=platform, + raw_platform_version_spec=platform, + php_version_spec=php, + raw_php_version_spec=php, + is_nightly=is_nightly, + ) + + def test_apps_lists_newest_release_per_requirement_set(self): + app = App.objects.create(pk="news", owner=self.user) + self._create_releases( + app, + ( + ("1.9.0", ">=25.0.0", "*", False), + ("1.10.0", ">=25.0.0", "*", False), + ("1.2.0", ">=25.0.0", ">=8.1.0", False), + ("2.0.0", ">=26.0.0", "*", False), + ("2.1.0-beta.1", ">=26.0.0", "*", False), + ("2.1.0-beta.2", ">=26.0.0", "*", False), + ("2.0.0", ">=26.0.0", "*", True), + ), + ) + response = self.api_client.get(reverse("api:v1:apps")) + self.assertEqual(200, response.status_code) + releases = {(release["version"], release["is_nightly"]) for release in response.data[0]["releases"]} + self.assertEqual( + {("1.10.0", False), ("1.2.0", False), ("2.0.0", False), ("2.1.0-beta.2", False), ("2.0.0", True)}, + releases, + ) + + def test_apps_keeps_releases_a_server_cannot_tell_apart(self): + app = App.objects.create(pk="news", owner=self.user) + self._create_releases(app, (("3.0.0-a.1", ">=26.0.0", "*", False), ("3.0.0-alpha.1", ">=26.0.0", "*", False))) + response = self.api_client.get(reverse("api:v1:apps")) + self.assertEqual(200, response.status_code) + versions = {release["version"] for release in response.data[0]["releases"]} + self.assertEqual({"3.0.0-a.1", "3.0.0-alpha.1"}, versions) + + def test_apps_keeps_a_set_php_versions_disagree_on(self): + app = App.objects.create(pk="news", owner=self.user) + self._create_releases( + app, + ( + ("2.0.0-alpha.5", ">=26.0.0", "*", False), + ("2.0.0-alpha.6", ">=26.0.0", "*", False), + ("2.0.0-", ">=26.0.0", "*", False), + ), + ) + response = self.api_client.get(reverse("api:v1:apps")) + self.assertEqual(200, response.status_code) + versions = {release["version"] for release in response.data[0]["releases"]} + self.assertEqual({"2.0.0-alpha.5", "2.0.0-alpha.6", "2.0.0-"}, versions) + + def test_platform_lists_every_compatible_release(self): + app = App.objects.create(pk="news", owner=self.user) + self._create_releases(app, (("1.9.0", ">=9.1.1", "*", False), ("1.10.0", ">=9.1.1", "*", False))) + response = self.api_client.get(reverse("api:v1:app", kwargs={"version": "9.1.1"})) + self.assertEqual(200, response.status_code) + versions = {release["version"] for release in response.data[0]["releases"]} + self.assertEqual({"1.9.0", "1.10.0"}, versions) + def test_delete(self): App.objects.create(id="news", owner=self.user) url = reverse("api:v1:app-delete", kwargs={"pk": "news"}) diff --git a/nextcloudappstore/api/v1/views.py b/nextcloudappstore/api/v1/views.py index ad6bc5677e3..64a5ed9dbd6 100644 --- a/nextcloudappstore/api/v1/views.py +++ b/nextcloudappstore/api/v1/views.py @@ -38,6 +38,7 @@ AppRegisterSerializer, AppReleaseDownloadSerializer, AppSerializer, + AppWithNewestReleasesSerializer, CategorySerializer, NextcloudReleaseSerializer, ) @@ -118,7 +119,7 @@ class AppsView(EnterpriseFilterMixin, ListAPIView): .annotate(num_releases=Count("releases", filter=Q(releases__aa_is_system__isnull=True))) .filter(Q(num_releases__gt=0) | Q(is_integration=True)) ) - serializer_class = AppSerializer + serializer_class = AppWithNewestReleasesSerializer class AppApiAppsView(EnterpriseFilterMixin, ListAPIView): diff --git a/nextcloudappstore/core/tests/test_versioning.py b/nextcloudappstore/core/tests/test_versioning.py index 4d944607686..76731736143 100644 --- a/nextcloudappstore/core/tests/test_versioning.py +++ b/nextcloudappstore/core/tests/test_versioning.py @@ -16,6 +16,7 @@ pad_max_inc_version, pad_max_version, pad_min_version, + php_version_compare, raw_version, to_raw_spec, to_spec, @@ -89,3 +90,36 @@ def test_group_by_version(self): example["2.0.0"] = [1] result = group_by_main_version(example) self.assertDictEqual({"1": [2, 3], "2": [1]}, result) + + def test_php_version_compare(self): + # Expected values are PHP's version_compare() results, the same before and after PHP 8.4.22 and 8.5.7. + for version1, version2, expected in ( + ("1.0.0", "1.0.0", 0), + ("1.10.0", "1.9.0", 1), + ("1.0", "1.0.0", -1), + ("1.0.0-beta", "1.0.0", -1), + ("1.0.0-rc.1", "1.0.0-beta.2", 1), + ("2.0.0-beta.10", "2.0.0-beta.9", 1), + ("1.0.0-alpha", "1.0.0-dev", 1), + ("1.0.0-a", "1.0.0-alpha", 0), + ("1.0.0-RC1", "1.0.0-rc1", 0), + ("1.0.0-Beta", "1.0.0-beta", -1), + ("1.0.0-x", "1.0.0-dev", -1), + ("1.0.0pl1", "1.0.0", 1), + ("1.0.0-1", "1.0.0", 1), + ("1.0.0+build", "1.0.0", -1), + ("", "1", -1), + ): + with self.subTest(version1=version1, version2=version2): + self.assertEqual(expected, php_version_compare(version1, version2)) + self.assertEqual(-expected, php_version_compare(version2, version1)) + + def test_php_version_compare_trailing_dot(self): + # Results of PHP 8.4.22, 8.5.7 and later. Earlier versions return -1 for each. + for version1, version2, expected in ( + ("1.", "1", 0), + ("1.0.0-", "1.0.0", 0), + ("2.0.0-", "2.0.0-alpha.6", 1), + ): + with self.subTest(version1=version1, version2=version2): + self.assertEqual(expected, php_version_compare(version1, version2)) diff --git a/nextcloudappstore/core/versioning.py b/nextcloudappstore/core/versioning.py index a0a9d1787bb..de5fc55a3c3 100644 --- a/nextcloudappstore/core/versioning.py +++ b/nextcloudappstore/core/versioning.py @@ -161,3 +161,121 @@ def version_in_spec(version: str, spec: str) -> bool: :return: """ return Version(version) in Spec(spec) + + +_PHP_SPECIAL_FORMS = ( + (b"dev", 0), + (b"alpha", 1), + (b"a", 1), + (b"beta", 2), + (b"b", 2), + (b"RC", 3), + (b"rc", 3), + (b"#", 4), + (b"pl", 5), + (b"p", 5), +) +_PHP_LONG_MAX = 2**63 - 1 + + +def _is_digit(char: int) -> bool: + return 0x30 <= char <= 0x39 + + +def _is_alnum(char: int) -> bool: + return _is_digit(char) or 0x41 <= char <= 0x5A or 0x61 <= char <= 0x7A + + +def _php_canonicalize_version(version: bytes) -> bytes: + def is_digit(char: int) -> bool: + return _is_digit(char) and char != 0x2E + + def is_non_digit(char: int) -> bool: + return not _is_digit(char) and char != 0x2E + + result = bytearray(version[:1]) + previous = version[0] + for char in version[1:]: + if char in b"-_+": + if result[-1] != 0x2E: + result.append(0x2E) + elif (is_non_digit(previous) and is_digit(char)) or (is_digit(previous) and is_non_digit(char)): + if result[-1] != 0x2E: + result.append(0x2E) + result.append(char) + elif not _is_alnum(char): + if result[-1] != 0x2E: + result.append(0x2E) + else: + result.append(char) + previous = char + if result[-1:] == b".": + del result[-1] + return bytes(result) + + +def _php_compare_special_forms(form1: bytes, form2: bytes) -> int: + def order(form: bytes) -> int: + return next((order for name, order in _PHP_SPECIAL_FORMS if form.startswith(name)), -1) + + difference = order(form1) - order(form2) + return (difference > 0) - (difference < 0) + + +def _php_leading_number(part: bytes) -> int: + digits = len(part) - len(part.lstrip(b"0123456789")) + return min(int(part[:digits]), _PHP_LONG_MAX) + + +def _php_version_compare(version1: bytes, version2: bytes) -> int: + if not version1 or not version2: + return (len(version1) > 0) - (len(version2) > 0) + version1 = version1 if version1.startswith(b"#") else _php_canonicalize_version(version1) + version2 = version2 if version2.startswith(b"#") else _php_canonicalize_version(version2) + position1 = position2 = 0 + dot1: int | None = 0 + dot2: int | None = 0 + compare = 0 + while position1 < len(version1) and position2 < len(version2) and dot1 is not None and dot2 is not None: + dot1 = version1.find(b".", position1) + dot1 = None if dot1 < 0 else dot1 + dot2 = version2.find(b".", position2) + dot2 = None if dot2 < 0 else dot2 + part1 = version1[position1:dot1] + part2 = version2[position2:dot2] + numeric1 = part1[:1].isdigit() + numeric2 = part2[:1].isdigit() + if numeric1 and numeric2: + number1 = _php_leading_number(part1) + number2 = _php_leading_number(part2) + compare = (number1 > number2) - (number1 < number2) + elif not numeric1 and not numeric2: + compare = _php_compare_special_forms(part1, part2) + elif numeric1: + compare = _php_compare_special_forms(b"#N#", part2) + else: + compare = _php_compare_special_forms(part1, b"#N#") + if compare != 0: + break + if dot1 is not None: + position1 = dot1 + 1 + if dot2 is not None: + position2 = dot2 + 1 + if compare == 0: + if dot1 is not None: + rest = version1[position1:] + compare = 1 if rest[:1].isdigit() else _php_version_compare(rest, b"#N#") + elif dot2 is not None: + rest = version2[position2:] + compare = -1 if rest[:1].isdigit() else _php_version_compare(b"#N#", rest) + return compare + + +def php_version_compare(version1: str, version2: str) -> int: + """ + Compares two versions like PHP's version_compare(), which Nextcloud uses to pick the newest app release, as of + PHP 8.4.22 and 8.5.7. Those ignore a trailing dot after canonicalisation, so "1.0.0-" equals "1.0.0". Earlier + versions differ only for versions that end in a character other than an ASCII letter or digit. + :return: -1, 0 or 1 + """ + return _php_version_compare(version1.encode(), version2.encode())