index-v2 'mirrors' fully settable from config

This lets mirrors: in config.yml be the same list-of-dicts format as it is
in index-v2.  This also includes a data format conversion to maintain the
right format for the old, unchanging index v0 and v1 formats.

#928
#1107

examples/config.yml

@@ -220,6 +220,15 @@
 # mirrors:
 #   - https://foo.bar/fdroid
 #   - http://foobarfoobarfoobar.onion/fdroid
+#
+# Or additional metadata can also be included by adding key/value pairs:
+#
+# mirrors:
+#   - url: https://foo.bar/fdroid
+#     countryCode: BA
+#   - url: http://foobarfoobarfoobar.onion/fdroid
+#
+
 
 # optionally specify which identity file to use when using rsync or git over SSH
 #

fdroidserver/index.py

@@ -91,7 +91,7 @@ def make(apps, apks, repodir, archive):
         repodict['address'] = archive_url
         if 'archive_web_base_url' in common.config:
             repodict["webBaseUrl"] = common.config['archive_web_base_url']
-        urlbasepath = os.path.basename(urllib.parse.urlparse(archive_url).path)
+        repo_section = os.path.basename(urllib.parse.urlparse(archive_url).path)
     else:
         repodict['name'] = common.config['repo_name']
         repodict['icon'] = common.config.get('repo_icon', common.default_config['repo_icon'])
@@ -99,27 +99,9 @@ def make(apps, apks, repodir, archive):
         if 'repo_web_base_url' in common.config:
             repodict["webBaseUrl"] = common.config['repo_web_base_url']
         repodict['description'] = common.config['repo_description']
-        urlbasepath = os.path.basename(urllib.parse.urlparse(common.config['repo_url']).path)
+        repo_section = os.path.basename(urllib.parse.urlparse(common.config['repo_url']).path)
 
-    mirrorcheckfailed = False
+    add_mirrors_to_repodict(repo_section, repodict)
-    mirrors = []
-    for mirror in common.config.get('mirrors', []):
-        base = os.path.basename(urllib.parse.urlparse(mirror).path.rstrip('/'))
-        if common.config.get('nonstandardwebroot') is not True and base != 'fdroid':
-            logging.error(_("mirror '%s' does not end with 'fdroid'!") % mirror)
-            mirrorcheckfailed = True
-        # must end with / or urljoin strips a whole path segment
-        if mirror.endswith('/'):
-            mirrors.append(urllib.parse.urljoin(mirror, urlbasepath))
-        else:
-            mirrors.append(urllib.parse.urljoin(mirror + '/', urlbasepath))
-    for mirror in common.config.get('servergitmirrors', []):
-        for url in get_mirror_service_urls(mirror):
-            mirrors.append(url + '/' + repodir)
-    if mirrorcheckfailed:
-        raise FDroidException(_("Malformed repository mirrors."))
-    if mirrors:
-        repodict['mirrors'] = mirrors
 
     requestsdict = collections.OrderedDict()
     for command in ('install', 'uninstall'):
@@ -713,16 +695,11 @@ def v2_repo(repodict, repodir, archive):
         repo["icon"] = config["archive" if archive else "repo"]["icon"]
 
     repo["address"] = repodict["address"]
+    if "mirrors" in repodict:
+        repo["mirrors"] = repodict["mirrors"]
     if "webBaseUrl" in repodict:
         repo["webBaseUrl"] = repodict["webBaseUrl"]
 
-    if "mirrors" in repodict:
-        repo["mirrors"] = [{"url": mirror} for mirror in repodict["mirrors"]]
-
-        # the first entry is traditionally the primary mirror
-        if repodict['address'] not in repodict["mirrors"]:
-            repo["mirrors"].insert(0, {"url": repodict['address'], "isPrimary": True})
-
     repo["timestamp"] = repodict["timestamp"]
 
     antiFeatures = load_locale("antiFeatures", repodir)
@@ -878,9 +855,18 @@ def make_v1(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_
         raise TypeError(repr(obj) + " is not JSON serializable")
 
     output = collections.OrderedDict()
-    output['repo'] = repodict
+    output['repo'] = repodict.copy()
     output['requests'] = requestsdict
 
+    # index-v1 only supports a list of URL strings for additional mirrors
+    mirrors = []
+    for mirror in repodict.get('mirrors', []):
+        url = mirror['url']
+        if url != repodict['address']:
+            mirrors.append(mirror['url'])
+    if mirrors:
+        output['repo']['mirrors'] = mirrors
+
     # establish sort order of the index
     v1_sort_packages(packages, fdroid_signing_key_fingerprints)
 
@@ -1096,8 +1082,11 @@ def make_v0(apps, apks, repodir, repodict, requestsdict, fdroid_signing_key_fing
     repoel.setAttribute("version", str(repodict['version']))
 
     addElement('description', repodict['description'], doc, repoel)
+    # index v0 only supports a list of URL strings for additional mirrors
     for mirror in repodict.get('mirrors', []):
-        addElement('mirror', mirror, doc, repoel)
+        url = mirror['url']
+        if url != repodict['address']:
+            addElement('mirror', url, doc, repoel)
 
     root.appendChild(repoel)
 
@@ -1407,6 +1396,83 @@ def extract_pubkey():
     return hexlify(pubkey), repo_pubkey_fingerprint
 
 
+def add_mirrors_to_repodict(repo_section, repodict):
+    """Convert config into final dict of mirror metadata for the repo.
+
+    Internally and in index-v2, mirrors is a list of dicts, but it can
+    be specified in the config as a string or list of strings.  Also,
+    index v0 and v1 use a list of URL strings as the data structure.
+
+    The first entry is traditionally the primary mirror and canonical
+    URL.  'mirrors' should not be present in the index if there is
+    only the canonical URL, and no other mirrors.
+
+    The metadata items for each mirror entry are sorted by key to
+    ensure minimum diffs in the index files.
+
+    """
+    mirrors_config = common.config.get('mirrors', [])
+    if type(mirrors_config) not in (list, tuple):
+        mirrors_config = [mirrors_config]
+
+    mirrorcheckfailed = False
+    mirrors = []
+    urls = set()
+    for mirror in mirrors_config:
+        if isinstance(mirror, str):
+            mirror = {'url': mirror}
+        elif not isinstance(mirror, dict):
+            logging.error(
+                _('Bad entry type "{mirrortype}" in mirrors config: {mirror}').format(
+                    mirrortype=type(mirror), mirror=mirror
+                )
+            )
+            mirrorcheckfailed = True
+            continue
+        config_url = mirror['url']
+        base = os.path.basename(urllib.parse.urlparse(config_url).path.rstrip('/'))
+        if common.config.get('nonstandardwebroot') is not True and base != 'fdroid':
+            logging.error(_("mirror '%s' does not end with 'fdroid'!") % config_url)
+            mirrorcheckfailed = True
+        # must end with / or urljoin strips a whole path segment
+        if config_url.endswith('/'):
+            mirror['url'] = urllib.parse.urljoin(config_url, repo_section)
+        else:
+            mirror['url'] = urllib.parse.urljoin(config_url + '/', repo_section)
+        mirrors.append(mirror)
+        if mirror['url'] in urls:
+            mirrorcheckfailed = True
+            logging.error(
+                _('Duplicate entry "%s" in mirrors config!') % mirror['url']
+            )
+        urls.add(mirror['url'])
+    for mirror in common.config.get('servergitmirrors', []):
+        for url in get_mirror_service_urls(mirror):
+            mirrors.append({'url': url + '/' + repo_section})
+    if mirrorcheckfailed:
+        raise FDroidException(_("Malformed repository mirrors."))
+
+    if not mirrors:
+        return
+
+    repodict['mirrors'] = []
+    canonical_url = repodict['address']
+    found_primary = False
+    for mirror in mirrors:
+        if canonical_url == mirror['url']:
+            found_primary = True
+            mirror['isPrimary'] = True
+            sortedmirror = dict()
+            for k in sorted(mirror.keys()):
+                sortedmirror[k] = mirror[k]
+            repodict['mirrors'].insert(0, sortedmirror)
+        else:
+            repodict['mirrors'].append(mirror)
+
+    if repodict['mirrors'] and not found_primary:
+        repodict['mirrors'].insert(0, {'isPrimary': True, 'url': repodict['address']})
+
+
 def get_mirror_service_urls(url):
     """Get direct URLs from git service for use by fdroidclient.
 

tests/index.TestCase

@@ -8,6 +8,7 @@ import optparse
 import os
 import sys
 import unittest
+import yaml
 import zipfile
 from unittest.mock import patch
 import requests
@@ -28,6 +29,7 @@ import fdroidserver.metadata
 import fdroidserver.net
 import fdroidserver.signindex
 import fdroidserver.publish
+from fdroidserver.exception import FDroidException
 from testcommon import TmpCwd, mkdtemp
 from pathlib import Path
 
@@ -418,6 +420,11 @@ class IndexTest(unittest.TestCase):
             'address': 'https://example.com/fdroid/repo',
             'description': 'This is just a test',
             'icon': 'blahblah',
+            'mirrors': [
+                {'isPrimary': True, 'url': 'https://example.com/fdroid/repo'},
+                {'extra': 'data', 'url': 'http://one/fdroid/repo'},
+                {'url': 'http://two/fdroid/repo'},
+            ],
             'name': 'test',
             'timestamp': datetime.datetime.now(),
             'version': 12,
@@ -507,6 +514,26 @@ class IndexTest(unittest.TestCase):
         self.assertTrue(os.path.exists(os.path.join('repo', 'index_unsigned.jar')))
         self.assertFalse(os.path.exists(os.path.join('repo', 'index.jar')))
 
+    def test_make_v1_with_mirrors(self):
+        os.chdir(self.testdir)
+        os.mkdir('repo')
+        repodict = {
+            'address': 'https://example.com/fdroid/repo',
+            'mirrors': [
+                {'isPrimary': True, 'url': 'https://example.com/fdroid/repo'},
+                {'extra': 'data', 'url': 'http://one/fdroid/repo'},
+                {'url': 'http://two/fdroid/repo'},
+            ],
+        }
+        fdroidserver.index.make_v1({}, [], 'repo', repodict, {}, {})
+        index_v1 = Path('repo/index-v1.json')
+        self.assertTrue(index_v1.exists())
+        with index_v1.open() as fp:
+            self.assertEqual(
+                json.load(fp)['repo']['mirrors'],
+                ['http://one/fdroid/repo', 'http://two/fdroid/repo'],
+            )
+
     def test_github_get_mirror_service_urls(self):
         for url in [
             'git@github.com:foo/bar',
@@ -656,16 +683,82 @@ class IndexTest(unittest.TestCase):
 
     def test_add_mirrors_to_repodict(self):
         """Test based on the contents of tests/config.py"""
-        repodict = dict()
+        repodict = {'address': fdroidserver.common.config['repo_url']}
         fdroidserver.index.add_mirrors_to_repodict('repo', repodict)
         self.assertEqual(
             repodict['mirrors'],
             [
-                'http://foobarfoobarfoobar.onion/fdroid/repo',
+                {'isPrimary': True, 'url': 'https://MyFirstFDroidRepo.org/fdroid/repo'},
-                'https://foo.bar/fdroid/repo',
+                {'url': 'http://foobarfoobarfoobar.onion/fdroid/repo'},
+                {'url': 'https://foo.bar/fdroid/repo'},
             ],
         )
 
+    def test_custom_config_yml_with_mirrors(self):
+        """Test based on custom contents of config.yml"""
+        os.chdir(self.testdir)
+        repo_url = 'https://example.com/fdroid/repo'
+        with open('config.yml', 'w') as fp:
+            yaml.dump({'repo_url': repo_url, 'mirrors': ['http://one/fdroid', ]}, fp)
+        os.system('cat config.yml')
+        fdroidserver.common.config = None
+        fdroidserver.common.read_config(Options)
+        repodict = {'address': fdroidserver.common.config['repo_url']}
+        fdroidserver.index.add_mirrors_to_repodict('repo', repodict)
+        self.assertEqual(
+            repodict['mirrors'],
+            [
+                {'url': 'https://example.com/fdroid/repo', 'isPrimary': True},
+                {'url': 'http://one/fdroid/repo'},
+            ]
+        )
+
+    def test_no_mirrors_config(self):
+        fdroidserver.common.config = dict()
+        repodict = {'address': 'https://example.com/fdroid/repo'}
+        fdroidserver.index.add_mirrors_to_repodict('repo', repodict)
+        self.assertFalse('mirrors' in repodict)
+
+    def test_add_metadata_to_canonical_in_mirrors_config(self):
+        """It is possible to add extra metadata to the canonical URL"""
+        fdroidserver.common.config = {
+            'repo_url': 'http://one/fdroid/repo',
+            'mirrors': [
+                {'url': 'http://one/fdroid', 'extra': 'data'},
+                {'url': 'http://two/fdroid'},
+            ],
+        }
+        repodict = {'address': fdroidserver.common.config['repo_url']}
+        fdroidserver.index.add_mirrors_to_repodict('repo', repodict)
+        self.assertEqual(
+            repodict['mirrors'],
+            [
+                {'extra': 'data', 'isPrimary': True, 'url': 'http://one/fdroid/repo'},
+                {'url': 'http://two/fdroid/repo'},
+            ],
+        )
+
+    def test_duplicate_primary_in_mirrors_config(self):
+        """There can be only one primary mirror aka canonical URL"""
+        fdroidserver.common.config = {
+            'repo_url': 'http://one/fdroid',
+            'mirrors': [
+                {'url': 'http://one/fdroid', 'countryCode': 'SA'},
+                {'url': 'http://two/fdroid'},
+                {'url': 'http://one/fdroid'},
+            ],
+        }
+        repodict = {'address': fdroidserver.common.config['repo_url']}
+        with self.assertRaises(FDroidException):
+            fdroidserver.index.add_mirrors_to_repodict('repo', repodict)
+
+    def test_bad_type_in_mirrors_config(self):
+        for i in (1, 2.3, b'asdf'):
+            fdroidserver.common.config = {'mirrors': i}
+            repodict = dict()
+            with self.assertRaises(FDroidException):
+                fdroidserver.index.add_mirrors_to_repodict('repo', repodict)
+
 
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))

tests/repo/entry.json

@@ -3,7 +3,7 @@
   "version": 20002,
   "index": {
     "name": "/index-v2.json",
-    "sha256": "e791cdb7e258f0ad37a1cc6af9a62f9d75253f41348c7841524c888b2daf105c",
+    "sha256": "07fa4500736ae77fcc6434e4d70ab315b8e018aef52c2afca9f2834ddc73747d",
     "size": 32946,
     "numPackages": 10
   },

tests/repo/index-v2.json

@@ -16,8 +16,8 @@
     "address": "https://MyFirstFDroidRepo.org/fdroid/repo",
     "mirrors": [
       {
-        "url": "https://MyFirstFDroidRepo.org/fdroid/repo",
+        "isPrimary": true,
-        "isPrimary": true
+        "url": "https://MyFirstFDroidRepo.org/fdroid/repo"
       },
       {
         "url": "http://foobarfoobarfoobar.onion/fdroid/repo"