diff --git a/fdroidserver/import_subcommand.py b/fdroidserver/import_subcommand.py index b8fb7093..03c36539 100644 --- a/fdroidserver/import_subcommand.py +++ b/fdroidserver/import_subcommand.py @@ -323,7 +323,7 @@ def main(): if git_modules.exists(): build.submodules = True - metadata.post_metadata_parse(app) + metadata.post_parse_yaml_metadata(app) app['Builds'].append(build) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 38d1f762..69b85283 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -607,78 +607,6 @@ def read_metadata(appids={}, sort_by_time=False): return apps -def sorted_builds(builds): - return sorted(builds, key=lambda build: build.versionCode) - - -def post_metadata_parse(app): - """Convert human-readable metadata data structures into consistent data structures. - - This also handles conversions that make metadata YAML behave - something like StrictYAML. Specifically, a field should have a - fixed value type, regardless of YAML 1.2's type auto-detection. - - """ - - def _normalize_type_string(v): - """YAML 1.2's booleans are all lowercase. - - Things like versionName are strings, but without quotes can be - numbers. Like "versionName: 1.0" would be a YAML float, but - should be a string. - - """ - if isinstance(v, bool): - return str(v).lower() - return str(v) - - for k, v in app.items(): - if fieldtype(k) == TYPE_LIST: - if isinstance(v, str): - app[k] = [v, ] - elif v: - app[k] = [str(i) for i in v] - elif fieldtype(k) == TYPE_INT: - if v: - app[k] = int(v) - elif fieldtype(k) == TYPE_STRING: - if v: - app[k] = _normalize_type_string(v) - else: - if type(v) in (float, int): - app[k] = str(v) - - builds = [] - if 'Builds' in app: - for build in app.get('Builds', []): - if not isinstance(build, Build): - build = Build(build) - for k, v in build.items(): - if not (v is None): - if flagtype(k) == TYPE_LIST: - if isinstance(v, str): - build[k] = [v] - elif flagtype(k) is TYPE_INT: - build[k] = v - elif flagtype(k) is TYPE_STRING: - build[k] = _normalize_type_string(v) - builds.append(build) - - app['Builds'] = sorted_builds(builds) - - -# Parse metadata for a single application. -# -# 'metadatapath' - the file path to read. The "Application ID" aka -# "Package Name" for the application comes from this -# filename. Pass None to get a blank entry. -# -# Returns a dictionary containing all the details of the application. There are -# two major kinds of information in the dictionary. Keys beginning with capital -# letters correspond directory to identically named keys in the metadata file. -# Keys beginning with lower case letters are generated in one way or another, -# and are not found verbatim in the metadata. -# # Known keys not originating from the metadata are: # # 'comments' - a list of comments from the metadata file. Each is @@ -687,9 +615,6 @@ def post_metadata_parse(app): # file. Where field is None, the comment goes at the # end of the file. Alternatively, 'build:version' is # for a comment before a particular build version. -# 'descriptionlines' - original lines of description as formatted in the -# metadata file. -# def parse_metadata(metadatapath): @@ -711,6 +636,25 @@ def parse_metadata(metadatapath): e.g. if the upstream developer includes some broken field, it can be overridden in the metadata file. + Parameters + ---------- + metadatapath + The file path to read. The "Application ID" aka "Package Name" + for the application comes from this filename. + + Raises + ------ + FDroidException when there are syntax errors. + + Returns + ------- + Returns a dictionary containing all the details of the + application. There are two major kinds of information in the + dictionary. Keys beginning with capital letters correspond + directory to identically named keys in the metadata file. Keys + beginning with lower case letters are generated in one way or + another, and are not found verbatim in the metadata. + """ metadatapath = Path(metadatapath) app = App() @@ -740,8 +684,13 @@ def parse_metadata(metadatapath): if k not in app: app[k] = v - post_metadata_parse(app) + builds = [] + for build in app.get('Builds', []): + builds.append(Build(build)) + if builds: + app['Builds'] = builds + # if only .fdroid.yml was found, then this finds the appid if not app.id: if app.get('Builds'): build = app['Builds'][-1] @@ -813,23 +762,77 @@ def parse_yaml_metadata(mf): def post_parse_yaml_metadata(yamldata): - """Transform yaml metadata to our internal data format.""" - for build in yamldata.get('Builds', []): - for flag in build.keys(): - _flagtype = flagtype(flag) + """Convert human-readable metadata data structures into consistent data structures. - if _flagtype is TYPE_SCRIPT: - if isinstance(build[flag], str): - build[flag] = [build[flag]] - elif _flagtype is TYPE_STRING: - # things like versionNames are strings, but without quotes can be numbers - if isinstance(build[flag], float) or isinstance(build[flag], int): - build[flag] = str(build[flag]) + This also handles conversions that make metadata YAML behave + something like StrictYAML. Specifically, a field should have a + fixed value type, regardless of YAML 1.2's type auto-detection. + + """ + + def _normalize_type_string(v): + """YAML 1.2's booleans are all lowercase. + + Things like versionName are strings, but without quotes can be + numbers. Like "versionName: 1.0" would be a YAML float, but + should be a string. + + """ + if isinstance(v, bool): + return str(v).lower() + return str(v) + + for k, v in yamldata.items(): + if fieldtype(k) == TYPE_LIST: + if isinstance(v, str): + yamldata[k] = [v, ] + elif v: + yamldata[k] = [str(i) for i in v] + elif fieldtype(k) == TYPE_INT: + if v: + yamldata[k] = int(v) + elif fieldtype(k) == TYPE_STRING: + if v: + yamldata[k] = _normalize_type_string(v) + else: + if type(v) in (float, int): + yamldata[k] = str(v) + + builds = [] + for build in yamldata.get('Builds', []): + for k, v in build.items(): + if v is None: + continue + + _flagtype = flagtype(k) + if _flagtype is TYPE_STRING: + build[k] = _normalize_type_string(v) elif _flagtype is TYPE_INT: + build[k] = v # versionCode must be int - if not isinstance(build[flag], int): - _warn_or_exception(_('{build_flag} must be an integer, found: {value}') - .format(build_flag=flag, value=build[flag])) + if not isinstance(v, int): + _warn_or_exception( + _('{build_flag} must be an integer, found: {value}').format( + build_flag=k, value=v + ) + ) + elif _flagtype in (TYPE_LIST, TYPE_SCRIPT): + if isinstance(v, str) or isinstance(v, int): + build[k] = [_normalize_type_string(v)] + else: + build[k] = v + # float and dict are here only to keep things compatible + if type(build[k]) not in (list, tuple, set, float, dict): + _warn_or_exception( + _('{build_flag} must be list or string, found: {value}').format( + build_flag=k, value=v + ) + ) + + builds.append(build) + + if builds: + yamldata['Builds'] = sorted(builds, key=lambda build: build['versionCode']) def write_yaml(mf, app): diff --git a/tests/import_subcommand.TestCase b/tests/import_subcommand.TestCase index cd11775e..eb41ae20 100755 --- a/tests/import_subcommand.TestCase +++ b/tests/import_subcommand.TestCase @@ -2,17 +2,20 @@ # http://www.drdobbs.com/testing/unit-testing-with-python/240165163 +import git import logging import optparse +import os import shutil import sys import tempfile import unittest +import yaml from unittest import mock from pathlib import Path import requests -from testcommon import TmpCwd +from testcommon import TmpCwd, mkdtemp localmodule = Path(__file__).resolve().parent.parent print('localmodule: ' + str(localmodule)) @@ -22,6 +25,7 @@ if localmodule not in sys.path: import fdroidserver.common import fdroidserver.import_subcommand import fdroidserver.metadata +from fdroidserver.exception import FDroidException class ImportTest(unittest.TestCase): @@ -32,6 +36,13 @@ class ImportTest(unittest.TestCase): self.basedir = localmodule / 'tests' fdroidserver.import_subcommand.options = mock.Mock() fdroidserver.import_subcommand.options.rev = None + os.chdir(self.basedir) + self._td = mkdtemp() + self.testdir = self._td.name + + def tearDown(self): + os.chdir(self.basedir) + self._td.cleanup() def test_import_gitlab(self): with tempfile.TemporaryDirectory() as testdir, TmpCwd(testdir): @@ -122,6 +133,32 @@ class ImportTest(unittest.TestCase): with self.assertRaises(ValueError): fdroidserver.import_subcommand.get_app_from_url(url) + @mock.patch('sys.argv', ['fdroid import', '-u', 'https://example.com/mystery/url']) + @mock.patch('fdroidserver.import_subcommand.clone_to_tmp_dir', lambda a: None) + def test_unrecognized_url(self): + """Test whether error is thrown when the RepoType was not found. + + clone_to_tmp_dir is mocked out to prevent this test from using + the network, if it gets past the code that throws the error. + + """ + with self.assertRaises(FDroidException): + fdroidserver.import_subcommand.main() + + @mock.patch('sys.argv', ['fdroid import', '-u', 'https://fake/git/url.git']) + @mock.patch('fdroidserver.import_subcommand.clone_to_tmp_dir', lambda a: Path('td')) + def test_main_local_git(self): + os.chdir(self.testdir) + git.Repo.init('td') + with Path('td/build.gradle').open('w') as fp: + fp.write('android { defaultConfig { applicationId "com.example" } }') + fdroidserver.import_subcommand.main() + with open('metadata/com.example.yml') as fp: + data = yaml.safe_load(fp) + self.assertEqual(data['Repo'], sys.argv[2]) + self.assertEqual(data['RepoType'], 'git') + self.assertEqual(1, len(data['Builds'])) + if __name__ == "__main__": parser = optparse.OptionParser() diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 7af539f9..0ef9bdc1 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -1178,15 +1178,11 @@ class PostMetadataParseTest(unittest.TestCase): def _post_metadata_parse_app_list(self, from_yaml, expected): app = {'AntiFeatures': from_yaml} metadata.post_parse_yaml_metadata(app) - metadata.post_metadata_parse(app) - del app['Builds'] return {'AntiFeatures': expected}, app def _post_metadata_parse_app_string(self, from_yaml, expected): app = {'Repo': from_yaml} metadata.post_parse_yaml_metadata(app) - metadata.post_metadata_parse(app) - del app['Builds'] return {'Repo': expected}, app def _post_metadata_parse_build_bool(self, from_yaml, expected): @@ -1194,7 +1190,6 @@ class PostMetadataParseTest(unittest.TestCase): app = {'Builds': [{'versionCode': 1, tested_key: from_yaml}]} post = copy.deepcopy(app) metadata.post_parse_yaml_metadata(post) - metadata.post_metadata_parse(post) del app['Builds'][0]['versionCode'] del post['Builds'][0]['versionCode'] for build in post['Builds']: @@ -1209,7 +1204,6 @@ class PostMetadataParseTest(unittest.TestCase): app = {'Builds': [{'versionCode': from_yaml}]} post = copy.deepcopy(app) metadata.post_parse_yaml_metadata(post) - metadata.post_metadata_parse(post) for build in post['Builds']: for k in list(build): if k != tested_key: @@ -1222,7 +1216,6 @@ class PostMetadataParseTest(unittest.TestCase): app = {'Builds': [{'versionCode': 1, tested_key: from_yaml}]} post = copy.deepcopy(app) metadata.post_parse_yaml_metadata(post) - metadata.post_metadata_parse(post) del app['Builds'][0]['versionCode'] del post['Builds'][0]['versionCode'] for build in post['Builds']: @@ -1237,7 +1230,6 @@ class PostMetadataParseTest(unittest.TestCase): app = {'Builds': [{'versionCode': 1, tested_key: from_yaml}]} post = copy.deepcopy(app) metadata.post_parse_yaml_metadata(post) - metadata.post_metadata_parse(post) del app['Builds'][0]['versionCode'] del post['Builds'][0]['versionCode'] for build in post['Builds']: @@ -1252,7 +1244,6 @@ class PostMetadataParseTest(unittest.TestCase): app = {'Builds': [{'versionCode': 1, tested_key: from_yaml}]} post = copy.deepcopy(app) metadata.post_parse_yaml_metadata(post) - metadata.post_metadata_parse(post) del app['Builds'][0]['versionCode'] del post['Builds'][0]['versionCode'] for build in post['Builds']: @@ -1269,8 +1260,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_app_string(123456, '123456')) self.assertEqual(*self._post_metadata_parse_build_bool(123456, 123456)) self.assertEqual(*self._post_metadata_parse_build_int(123456, 123456)) - self.assertEqual(*self._post_metadata_parse_build_list(123456, 123456)) - self.assertEqual(*self._post_metadata_parse_build_script(123456, 123456)) + self.assertEqual(*self._post_metadata_parse_build_list(123456, ['123456'])) + self.assertEqual(*self._post_metadata_parse_build_script(123456, ['123456'])) self.assertEqual(*self._post_metadata_parse_build_string(123456, '123456')) def test_post_metadata_parse_int_0(self): @@ -1279,8 +1270,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_app_string(0, 0)) self.assertEqual(*self._post_metadata_parse_build_bool(0, 0)) self.assertEqual(*self._post_metadata_parse_build_int(0, 0)) - self.assertEqual(*self._post_metadata_parse_build_list(0, 0)) - self.assertEqual(*self._post_metadata_parse_build_script(0, 0)) + self.assertEqual(*self._post_metadata_parse_build_list(0, ['0'])) + self.assertEqual(*self._post_metadata_parse_build_script(0, ['0'])) self.assertEqual(*self._post_metadata_parse_build_string(0, '0')) def test_post_metadata_parse_float_0_0(self): @@ -1373,9 +1364,9 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_app_string(False, False)) self.assertEqual(*self._post_metadata_parse_build_bool(False, False)) self.assertEqual(*self._post_metadata_parse_build_int(False, False)) - self.assertEqual(*self._post_metadata_parse_build_list(False, False)) - self.assertEqual(*self._post_metadata_parse_build_script(False, False)) - self.assertEqual(*self._post_metadata_parse_build_string(False, 'False')) + self.assertEqual(*self._post_metadata_parse_build_list(False, ['false'])) + self.assertEqual(*self._post_metadata_parse_build_script(False, ['false'])) + self.assertEqual(*self._post_metadata_parse_build_string(False, 'false')) def test_post_metadata_parse_true(self): with self.assertRaises(TypeError): @@ -1383,9 +1374,9 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_app_string(True, 'true')) self.assertEqual(*self._post_metadata_parse_build_bool(True, True)) self.assertEqual(*self._post_metadata_parse_build_int(True, True)) - self.assertEqual(*self._post_metadata_parse_build_list(True, True)) - self.assertEqual(*self._post_metadata_parse_build_script(True, True)) - self.assertEqual(*self._post_metadata_parse_build_string(True, 'True')) + self.assertEqual(*self._post_metadata_parse_build_list(True, ['true'])) + self.assertEqual(*self._post_metadata_parse_build_script(True, ['true'])) + self.assertEqual(*self._post_metadata_parse_build_string(True, 'true')) if __name__ == "__main__":