Fix the cross-provider review's findings on APK alternatives

One malformed or unreachable repo no longer hides the others; skip bad
index entries; a refreshed raw index outdates its reduced copy; style the
dialog like the others; validate package ids with PKG_RE.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
saphidandClaude Opus 5.5 committed 2026-09-28 15:15:11 +10:00
1 parent 50405ccf88
commit a1fa4ce140
5 files changed
+44 -9

No files matched your search

+28
View File
@@ -4,6 +4,7 @@ import json
import os import os
import sys import sys
import tempfile import tempfile
import time
import unittest import unittest
from concurrent.futures import ThreadPoolExecutor from concurrent.futures import ThreadPoolExecutor
from unittest.mock import patch from unittest.mock import patch
@@ -95,6 +96,33 @@ class VersionsTest(unittest.TestCase):
fetch.assert_called_once() fetch.assert_called_once()
self.assertTrue(os.path.exists(raw)) self.assertTrue(os.path.exists(raw))
def test_malformed_entries_are_skipped(self):
raw = os.path.join(self.tmp.name, 'odd.json')
with open(raw, 'w') as f:
json.dump({'packages': {'a.b': {'versions': {'x': {'manifest': {}}, 'y': None, 'z': build(4)}},
'c.d': {'versions': None}, 'e.f': []}}, f)
self.assertEqual([v['version_code'] for v in frame_catalog._reduce_index(raw)['a.b']], [4])
def test_one_failing_repo_keeps_the_others(self):
real = frame_catalog.load_index
def load(repo, cached_only=False):
if 'izzy' in repo:
raise KeyError('file')
return real(repo, cached_only=cached_only)
with patch.object(frame_catalog, 'load_index', side_effect=load):
result = versions.alternatives('org.example.app')
self.assertEqual([v['version_code'] for v in result['versions']], [3, 2, 1])
self.assertEqual(len(result['errors']), 1)
def test_newer_raw_index_outdates_reduced_copy(self):
repo = versions.REPOS[0][1]
frame_catalog.load_index(repo)
raw = os.path.join(self.tmp.name, 'data', 'index-v2.json')
with open(raw, 'w') as f:
json.dump({'packages': {'org.example.app': {'versions': {'x': build(8)}}}}, f)
os.utime(raw, ns=(time.time_ns() + 10**9,) * 2)
self.assertEqual(frame_catalog.load_index(repo)['org.example.app'][0]['version_code'], 8)
def test_concurrent_requests_share_download(self): def test_concurrent_requests_share_download(self):
raw = os.path.join(self.tmp.name, 'data', 'index-v2.json') raw = os.path.join(self.tmp.name, 'data', 'index-v2.json')
os.remove(raw) os.remove(raw)
+1 -1
View File
@@ -50,7 +50,7 @@ def _versions(package, cached_only=False):
for source, repo in REPOS: for source, repo in REPOS:
try: try:
index = frame_catalog.load_index(repo, cached_only=cached_only) index = frame_catalog.load_index(repo, cached_only=cached_only)
except (OSError, ValueError) as e: except Exception as e: # one bad repo (dropped download, odd index) mustn't hide the others
errors.append(f'Could not check {source}: {e}') errors.append(f'Could not check {source}: {e}')
continue continue
for v in index.get(package, []): for v in index.get(package, []):
+10 -3
View File
@@ -92,8 +92,13 @@ def _reduce_index(path):
continue continue
found = True found = True
for package in reader.members(): for package in reader.members():
records = [] records, entry = [], reader.value()
for v in reader.value().get('versions', {}).values(): versions = entry.get('versions') if isinstance(entry, dict) else None
for v in (versions.values() if isinstance(versions, dict) else ()):
# Skip malformed entries rather than losing the whole repo.
if not (isinstance(v, dict) and isinstance(v.get('manifest'), dict)
and isinstance(v.get('file'), dict) and v['file'].get('name')):
continue
if not installable(v): if not installable(v):
continue continue
m, file = v['manifest'], v['file'] m, file = v['manifest'], v['file']
@@ -121,7 +126,9 @@ def load_index(repo, cached_only=False):
path = raw + '.installable-v1' path = raw + '.installable-v1'
with _index_lock: with _index_lock:
mtime = os.stat(path).st_mtime_ns if os.path.exists(path) else None mtime = os.stat(path).st_mtime_ns if os.path.exists(path) else None
if mtime is not None and (cached_only or time.time() - mtime / 1e9 < 86400): # A newer raw index (the catalogue script refreshed it) outdates the reduced copy.
newer_raw = mtime is not None and os.path.exists(raw) and os.stat(raw).st_mtime_ns > mtime
if mtime is not None and (cached_only or (time.time() - mtime / 1e9 < 86400 and not newer_raw)):
cached = _indexes.get(path) cached = _indexes.get(path)
if cached is None or cached[0] != mtime: if cached is None or cached[0] != mtime:
with open(path) as f: with open(path) as f:
+4 -4
View File
@@ -221,10 +221,10 @@
.and-grid { display: grid; grid-template-columns: minmax(0, 1fr) minmax(0, 2fr); gap: 22px; align-items: start; } .and-grid { display: grid; grid-template-columns: minmax(0, 1fr) minmax(0, 2fr); gap: 22px; align-items: start; }
.and-col { display: grid; gap: 22px; align-content: start; } .and-col { display: grid; gap: 22px; align-content: start; }
.rep-item .s { white-space: normal; } .rep-item .s { white-space: normal; }
#repDlg, #titleDlg, #wiDlg { background: #1e2329; color: var(--text); border: 1px solid rgba(255,255,255,.1); border-radius: 4px; #repDlg, #titleDlg, #wiDlg, #apkAltDlg { background: #1e2329; color: var(--text); border: 1px solid rgba(255,255,255,.1); border-radius: 4px;
padding: 22px; width: min(560px, 92vw); box-shadow: 0 20px 60px rgba(0,0,0,.6); } padding: 22px; width: min(560px, 92vw); box-shadow: 0 20px 60px rgba(0,0,0,.6); }
#repDlg::backdrop, #titleDlg::backdrop, #wiDlg::backdrop { background: rgba(0,0,0,.55); } #repDlg::backdrop, #titleDlg::backdrop, #wiDlg::backdrop, #apkAltDlg::backdrop { background: rgba(0,0,0,.55); }
#repDlg h2, #titleDlg h2, #wiDlg h2 { margin: 0 0 14px; font-size: 15px; letter-spacing: 1.5px; text-transform: uppercase; color: var(--bright); } #repDlg h2, #titleDlg h2, #wiDlg h2, #apkAltDlg h2 { margin: 0 0 14px; font-size: 15px; letter-spacing: 1.5px; text-transform: uppercase; color: var(--bright); }
#repForm label, #titleForm label { display: block; font-size: 12.5px; color: var(--muted); margin-top: 10px; } #repForm label, #titleForm label { display: block; font-size: 12.5px; color: var(--muted); margin-top: 10px; }
#repForm label input[type=text], #repForm textarea, #titleForm label input, #titleForm label select { margin-top: 5px; } #repForm label input[type=text], #repForm textarea, #titleForm label input, #titleForm label select { margin-top: 5px; }
#titleForm select { width: 100%; background: rgba(0,0,0,.28); color: var(--text); border: 1px solid transparent; #titleForm select { width: 100%; background: rgba(0,0,0,.28); color: var(--text); border: 1px solid transparent;
@@ -555,7 +555,7 @@
<dialog id="apkAltDlg" aria-labelledby="apkAltTitle"> <dialog id="apkAltDlg" aria-labelledby="apkAltTitle">
<h2 id="apkAltTitle">Try another APK version</h2> <h2 id="apkAltTitle">Try another APK version</h2>
<p id="apkAltReason"></p> <p id="apkAltReason"></p>
<div id="apkAltVersions"></div> <div class="list" id="apkAltVersions"></div>
<p class="sub" id="apkAltNote"></p> <p class="sub" id="apkAltNote"></p>
<div id="apkAltLinks"></div> <div id="apkAltLinks"></div>
<p class="sub" id="apkAltErrors"></p> <p class="sub" id="apkAltErrors"></p>
+1 -1
View File
@@ -448,7 +448,7 @@ def open_thing(body):
def apk_versions(query): def apk_versions(query):
args = parse_qs(query, keep_blank_values=True) args = parse_qs(query, keep_blank_values=True)
packages, codes = args.get('package', []), args.get('code', []) packages, codes = args.get('package', []), args.get('code', [])
if len(packages) != 1 or not re.fullmatch(r'[A-Za-z][A-Za-z0-9_]*(?:\.[A-Za-z][A-Za-z0-9_]*)+', packages[0]): if len(packages) != 1 or not frame_android.PKG_RE.match(packages[0]):
raise Failure('invalid Android package id', 400) raise Failure('invalid Android package id', 400)
if codes and (len(codes) != 1 or not re.fullmatch(r'[0-9]{1,19}', codes[0])): if codes and (len(codes) != 1 or not re.fullmatch(r'[0-9]{1,19}', codes[0])):
raise Failure('invalid version code', 400) raise Failure('invalid version code', 400)