Account for collections with no stable / final releases (#88)

* ignore runtime directory

* add prerelase test fixtures

* update units for prerelease

* add helper to compare collection versions with prerelease awareness

* use prerelease aware version comparison

* add changelog fragment

* removed commented out code

changelogs/fragments/88-prereleases.yml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - A collection that had only prereleases would cause a 500 error when visiting certain endpoints due to a lack of a ``latest_version`` key. The latest version is now the latest stable (non-prerelease) version if any exist, which is the same behavior as before, but if only prereleases exist, then ``latest_version`` will refer to the latest prerelease (https://github.com/briantist/galactory/issues/87).

galactory/utilities.py

@@ -145,23 +145,28 @@ def discover_collections(repo, namespace=None, name=None, version=None, fast_det
             yield coldata
 
 
+def _latest_collection_version(reference_collection: dict, difference_collection: dict, *, property: str = "semver"):
+    """Returns the latest of two collections."""
+    if (reference_collection[property].prerelease is None) == (difference_collection[property].prerelease is None):
+        return max(reference_collection, difference_collection, key=lambda x: x[property])
+
+    return reference_collection if reference_collection[property].prerelease is None else difference_collection
+
+
 def collected_collections(repo, namespace=None, name=None, scheme=None):
     collections = {}
 
     for c in discover_collections(repo, namespace=namespace, name=name, scheme=scheme):
         version = c['version']
-        ver = c['semver']
         col = collections.setdefault(c['fqcn'], {})
         versions = col.setdefault('versions', {})
         versions[version] = c
-        if not ver.prerelease:
-            try:
-                latest = col['latest']
-            except KeyError:
-                col['latest'] = c
-            else:
-                if ver > latest['semver']:
-                    col['latest'] = c
+        try:
+            latest = col['latest']
+        except KeyError:
+            col['latest'] = c
+        else:
+            col['latest'] = _latest_collection_version(latest, c)
 
     return collections
 

tests/data/runtime/.gitignore

@@ -0,0 +1,2 @@
+*
+!.gitignore

tests/fixtures/artifactory/virtual/subpath/briantist-devel-0.1.0-dev0.tar.gz/MANIFEST.json

@@ -0,0 +1,30 @@
+{
+    "collection_info": {
+        "namespace": "briantist",
+        "name": "devel",
+        "version": "0.1.0-dev0",
+        "authors": [
+            "your name <[email protected]>"
+        ],
+        "readme": "README.md",
+        "tags": [],
+        "description": "your collection description",
+        "license": [
+            "GPL-2.0-or-later"
+        ],
+        "license_file": null,
+        "dependencies": {},
+        "repository": "http://example.com/repository",
+        "documentation": "http://docs.example.com",
+        "homepage": "http://example.com",
+        "issues": "http://example.com/issue/tracker"
+    },
+    "file_manifest_file": {
+        "name": "FILES.json",
+        "ftype": "file",
+        "chksum_type": "sha256",
+        "chksum_sha256": "58f8efd1f1e4f7e2afba0b773b1ec58cd3518c3c1d91bd29c54fc4187297a739",
+        "format": 1
+    },
+    "format": 1
+}

tests/fixtures/artifactory/virtual/subpath/briantist-devel-0.1.0-dev1.tar.gz/MANIFEST.json

@@ -0,0 +1,30 @@
+{
+    "collection_info": {
+        "namespace": "briantist",
+        "name": "devel",
+        "version": "0.1.0-dev1",
+        "authors": [
+            "your name <[email protected]>"
+        ],
+        "readme": "README.md",
+        "tags": [],
+        "description": "your collection description",
+        "license": [
+            "GPL-2.0-or-later"
+        ],
+        "license_file": null,
+        "dependencies": {},
+        "repository": "http://example.com/repository",
+        "documentation": "http://docs.example.com",
+        "homepage": "http://example.com",
+        "issues": "http://example.com/issue/tracker"
+    },
+    "file_manifest_file": {
+        "name": "FILES.json",
+        "ftype": "file",
+        "chksum_type": "sha256",
+        "chksum_sha256": "58f8efd1f1e4f7e2afba0b773b1ec58cd3518c3c1d91bd29c54fc4187297a739",
+        "format": 1
+    },
+    "format": 1
+}

tests/fixtures/artifactory/virtual/subpath/briantist-whatever-0.2.0-dev0.tar.gz/MANIFEST.json

@@ -0,0 +1,30 @@
+{
+    "collection_info": {
+        "namespace": "briantist",
+        "name": "whatever",
+        "version": "0.2.0-dev0",
+        "authors": [
+            "your name <[email protected]>"
+        ],
+        "readme": "README.md",
+        "tags": [],
+        "description": "your collection description",
+        "license": [
+            "GPL-2.0-or-later"
+        ],
+        "license_file": null,
+        "dependencies": {},
+        "repository": "http://example.com/repository",
+        "documentation": "http://docs.example.com",
+        "homepage": "http://example.com",
+        "issues": "http://example.com/issue/tracker"
+    },
+    "file_manifest_file": {
+        "name": "FILES.json",
+        "ftype": "file",
+        "chksum_type": "sha256",
+        "chksum_sha256": "58f8efd1f1e4f7e2afba0b773b1ec58cd3518c3c1d91bd29c54fc4187297a739",
+        "format": 1
+    },
+    "format": 1
+}

tests/unit/utilities/test_collected_collections.py

@@ -25,7 +25,7 @@ def test_collected_collections_skip_missing_version(repository, props):
 
 
 @pytest.mark.parametrize('namespace', [None, 'community', 'briantist', 'fake'])
-@pytest.mark.parametrize('collection', [None, 'whatever', 'hashi_vault', 'fake'])
+@pytest.mark.parametrize('collection', [None, 'whatever', 'hashi_vault', 'fake', 'devel'])
 @pytest.mark.parametrize('scheme', [None, '', 'https'])
 def test_collected_collections_any(repository, discover_collections, namespace, collection, scheme, app_request_context):
     fqcn = None if any([namespace is None, collection is None]) else f"{namespace}.{collection}"
@@ -62,3 +62,23 @@ def test_collected_collections_any(repository, discover_collections, namespace,
             assert ver == vd
 
     assert cols <= len(contents)
+
+@pytest.mark.parametrize('namespace', ['briantist'])
+@pytest.mark.parametrize('collection', ['devel'])
+def test_collected_collections_pre_only(repository, discover_collections, namespace, collection, app_request_context):
+    collections = collected_collections(repository, namespace, collection)
+
+    contents = list(repository)
+
+    cols = 0
+
+    for col, data in collections.items():
+        assert col == f"{namespace}.{collection}"
+        assert 'versions' in data
+        assert 'latest' in data
+        assert data['latest']['version'] == '0.1.0-dev1'
+
+        for v, vd in data['versions'].items():
+            cols += 1
+
+    assert cols <= len(contents)

tests/unit/utilities/test_discover_collections.py

@@ -25,8 +25,8 @@ def test_discover_collections_skip_missing_version(repository, props):
 
 
 @pytest.mark.parametrize('namespace', [None, 'community', 'briantist', 'fake'])
-@pytest.mark.parametrize('collection', [None, 'whatever', 'hashi_vault', 'fake'])
-@pytest.mark.parametrize('version', [None, '2.5.0', '3.0.0', '0.1.0', '0.2.0', '0.0.0'])
+@pytest.mark.parametrize('collection', [None, 'whatever', 'hashi_vault', 'devel', 'fake'])
+@pytest.mark.parametrize('version', [None, '2.5.0', '3.0.0', '0.1.0', '0.2.0', '0.0.0', '0.1.0-dev0', '0.1.0-dev1', '0.2.0-dev0'])
 def test_discover_collections_any(repository, manifest_loader, namespace, collection, version, app_request_context):
     gen = discover_collections(repository, namespace, collection, version)
 

tests/unit/utilities/test_utilities.py

@@ -0,0 +1,38 @@
+# -*- coding: utf-8 -*-
+# (c) 2023 Brian Scholer (@briantist)
+
+import pytest
+from unittest import mock
+
+from datetime import datetime, timedelta
+from types import GeneratorType
+import semver
+
+from galactory.utilities import _latest_collection_version
+
+
+@pytest.mark.parametrize(["ref", "dif", "exp"], [
+    ("0.0.0-dev0", "0.0.0-dev1", "0.0.0-dev1"),
+    ("0.0.0", "0.0.0-dev1", "0.0.0"),
+    ("1.2.3", "1.2.4", "1.2.4"),
+    ("0.0.0-dev0", "0.0.0-dev1", "0.0.0-dev1"),
+    ("0.0.0", "99.99.99-dev1", "0.0.0"),
+])
+@pytest.mark.parametrize("prop", [None, "semver", "other"])
+def test_latest_collection_version(ref, dif, exp, prop):
+    reference = semver.VersionInfo.parse(ref)
+    difference = semver.VersionInfo.parse(dif)
+    expected = ref if exp == ref else dif
+
+    opprop = {}
+    if prop is None:
+        expprop = "semver"
+    else:
+        opprop["property"] = expprop = prop
+
+    # ensure we didn't mess up the test cases
+    assert expected in [reference, difference]
+
+    result = _latest_collection_version({expprop: reference}, {expprop: difference}, **opprop)
+
+    assert result[expprop] == expected