From 8abbbd7b0c99818428a0fa5319c29d7528131085 Mon Sep 17 00:00:00 2001
From: mreid-tt <943378+mreid-tt@users.noreply.github.com>
Date: Wed, 3 Jan 2024 00:34:48 -0400
Subject: [PATCH] fix download counter

- rewrite catalog download links using md5 hashes for id
- allow downloads of noarch builds to pass arch checks
- rewrite nas tests for new url structure
---
 spkrepo/tests/test_nas.py | 36 ++++++++++++++++++------------------
 spkrepo/views/nas.py      | 37 +++++++++++++++++++++++++++----------
 2 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/spkrepo/tests/test_nas.py b/spkrepo/tests/test_nas.py
index eff207b..deb79ae 100644
--- a/spkrepo/tests/test_nas.py
+++ b/spkrepo/tests/test_nas.py
@@ -560,9 +560,9 @@ def test_generic(self):
         response = self.client.get(
             url_for(
                 "nas.download",
-                architecture_id=architecture.id,
-                firmware_build=4458,
-                build_id=build.id,
+                md5=build.md5,
+                arch=architecture.code,
+                build=4458,
             ),
             environ_base={"REMOTE_ADDR": "127.0.0.1"},
             headers={"User-Agent": "My User Agent"},
@@ -592,9 +592,9 @@ def test_wrong_build(self):
         response = self.client.get(
             url_for(
                 "nas.download",
-                architecture_id=architecture.id,
-                firmware_build=4458,
-                build_id=build.id + 1,
+                md5=build.md5 + "1",
+                arch=architecture.code,
+                build=4458,
             )
         )
         self.assert404(response)
@@ -613,9 +613,9 @@ def test_inactive_build(self):
         response = self.client.get(
             url_for(
                 "nas.download",
-                architecture_id=architecture.id,
-                firmware_build=4458,
-                build_id=build.id,
+                md5=build.md5,
+                arch=architecture.code,
+                build=4458,
             )
         )
         self.assert403(response)
@@ -634,9 +634,9 @@ def test_wrong_architecture(self):
         response = self.client.get(
             url_for(
                 "nas.download",
-                architecture_id=10,
-                firmware_build=4458,
-                build_id=build.id,
+                md5=build.md5,
+                arch=Architecture.find(10).code if Architecture.find(10) else "",
+                build=4458,
             )
         )
         self.assert404(response)
@@ -655,9 +655,9 @@ def test_incorrect_architecture(self):
         response = self.client.get(
             url_for(
                 "nas.download",
-                architecture_id=Architecture.find("cedarview").id,
-                firmware_build=4458,
-                build_id=build.id,
+                md5=build.md5,
+                arch=Architecture.find("cedarview").code,
+                build=4458,
             )
         )
         self.assert400(response)
@@ -676,9 +676,9 @@ def test_incorrect_firmware_build(self):
         response = self.client.get(
             url_for(
                 "nas.download",
-                architecture_id=architecture.id,
-                firmware_build=1593,
-                build_id=build.id,
+                md5=build.md5,
+                arch=architecture.code,
+                build=1593,
             )
         )
         self.assert400(response)
diff --git a/spkrepo/views/nas.py b/spkrepo/views/nas.py
index 9cbeeed..7be3095 100644
--- a/spkrepo/views/nas.py
+++ b/spkrepo/views/nas.py
@@ -144,6 +144,10 @@ def get_catalog(arch, build, language, beta):
     # fill the catalog
     entries = []
     for b in latest_build.all():
+        # correct any missing md5 hashes
+        if b.md5 is None:
+            b.md5 = b.calculate_md5()
+            db.session.commit()
         entry = {
             "package": b.version.package.name,
             "version": b.version.version_string,
@@ -154,7 +158,11 @@ def get_catalog(arch, build, language, beta):
                 language, b.version.descriptions["enu"]
             ).description,
             "link": url_for(
-                ".data", path=b.path, arch=arch, build=build, _external=True
+                ".download",
+                md5=b.md5,
+                arch=arch,
+                build=build,
+                _external=True,
             ),
             "thumbnail": [
                 url_for(".data", path=icon.path, _external=True)
@@ -251,25 +259,34 @@ def catalog():
     return Response(json.dumps(catalog), mimetype="application/json")
 
 
-@nas.route("/download/<int:architecture_id>/<int:firmware_build>/<int:build_id>")
-def download(architecture_id, firmware_build, build_id):
+@nas.route("/download/<string:md5>/<string:arch>/<int:build>")
+def download(md5, arch, build):
     # check build
-    build = Build.query.get_or_404(build_id)
-    if not build.active:
+    build_obj = Build.query.filter_by(md5=md5).one_or_none()
+
+    if build_obj is None:
+        abort(404)
+    elif not build_obj.active:
         abort(403)
 
     # architecture
-    architecture = Architecture.query.get_or_404(architecture_id)
+    architecture = Architecture.query.filter_by(code=arch).one_or_none()
 
+    if architecture is None:
+        abort(404)
     # check consistency
-    if architecture not in build.architectures or firmware_build < build.firmware.build:
+    elif build < build_obj.firmware.build:
+        abort(400)
+    elif architecture not in build_obj.architectures and not any(
+        arch.code == "noarch" for arch in build_obj.architectures
+    ):
         abort(400)
 
     # insert in database
     download = Download(
-        build=build,
+        build=build_obj,
         architecture=architecture,
-        firmware_build=firmware_build,
+        firmware_build=build,
         ip_address=request.remote_addr,
         user_agent=request.user_agent.string,
     )
@@ -277,7 +294,7 @@ def download(architecture_id, firmware_build, build_id):
     db.session.commit()
 
     # redirect
-    return redirect(url_for(".data", path=build.path))
+    return redirect(url_for(".data", path=build_obj.path))
 
 
 @nas.route("/<path:path>")