Refactoring class structure: Factory class, Data and consumer (#309)

* Introduce tooldata class

Signed-off-by: Hiroshi Miura <miurahr@linux.com>

* Refactoring list command

- Introduce show_list function that is part of UI
- ListCommand class can be used as library to retrieve metadata.
- Rename ListCommand to MetadataFactory
- Rename action method to getList

Signed-off-by: Hiroshi Miura <miurahr@linux.com>

aqt/installer.py

@@ -48,7 +48,7 @@ from aqt.helper import (
     getUrl,
     setup_logging,
 )
-from aqt.metadata import ArchiveId, ListCommand, Version
+from aqt.metadata import ArchiveId, MetadataFactory, Version, show_list
 from aqt.updater import Updater
 
 try:
@@ -496,7 +496,7 @@ class Cli:
                 )
                 exit(1)
 
-        command = ListCommand(
+        meta = MetadataFactory(
             archive_id=ArchiveId(
                 args.category,
                 args.host,
@@ -511,10 +511,10 @@ class Cli:
             tool_name=args.tool,
             tool_long_listing=args.tool_long,
         )
-        return command.run()
+        return show_list(meta)
 
     def _make_list_parser(self, subparsers: argparse._SubParsersAction):
-        """Creates a subparser that works with the ListCommand, and adds it to the `subparsers` parameter"""
+        """Creates a subparser that works with the MetadataFactory, and adds it to the `subparsers` parameter"""
         list_parser: argparse.ArgumentParser = subparsers.add_parser(
             "list",
             formatter_class=argparse.RawDescriptionHelpFormatter,

aqt/logging.ini

@@ -32,6 +32,11 @@ handlers=NOTSET
 propagate=0
 qualname=aqt.installer
 
+[logger_aqt_list]
+level=INFO
+propagate=1
+qualname=aqt.list
+
 [logger_aqt_updater]
 level=INFO
 propagate=1

aqt/metadata.py

@@ -271,28 +271,49 @@ class ArchiveId:
         )
 
 
-class Table:
-    def __init__(self, head: List[str], rows: List[List[str]], max_width: int = 0):
-        # max_width is set to 0 by default: this disables wrapping of text table cells
-        self.head = head
-        self.rows = rows
-        self.max_width = max_width
+class ToolData:
+    """A data class hold tool details."""
+
+    head = [
+        "Tool Variant Name",
+        "Version",
+        "Release Date",
+        "Display Name",
+        "Description",
+    ]
+
+    def __init__(self, tool_data):
+        self.tool_data = tool_data
 
     def __format__(self, format_spec) -> str:
-        if format_spec == "":
-            table = Texttable(max_width=self.max_width)
-            table.set_deco(Texttable.HEADER)
-            table.header(self.head)
-            table.add_rows(self.rows, header=False)
-            return table.draw()
-        elif format_spec == "s":
+        if format_spec == "{s}":
             return str(self)
+        if format_spec == "":
+            max_width: int = 0
         else:
-            raise ValueError()
+            match = re.match(r"\{(.*):(\d*)t\}", format_spec)
+            if match:
+                g = match.groups()
+                max_width = 0 if g[1] == "" else int(g[1])
+            else:
+                raise ValueError("Wrong format")
+        table = Texttable(max_width=max_width)
+        table.set_deco(Texttable.HEADER)
+        table.header(self.head)
+        table.add_rows(self.rows, header=False)
+        return table.draw()
+
+    @property
+    def rows(self):
+        keys = ("Version", "ReleaseDate", "DisplayName", "Description")
+        return [
+            [name, *[content[key] for key in keys]]
+            for name, content in self.tool_data.items()
+        ]
 
 
-class ListCommand:
-    """Encapsulate all parts of the `aqt list` command"""
+class MetadataFactory:
+    """Retrieve metadata of Qt variations, versions, and descriptions from Qt site."""
 
     def __init__(
         self,
@@ -307,11 +328,11 @@ class ListCommand:
         tool_long_listing: Optional[str] = None,
     ):
         """
-        Construct ListCommand.
+        Construct MetadataFactory.
 
-        :param filter_minor:        When set, the ListCommand will filter out all versions of
+        :param filter_minor:        When set, the MetadataFactory will filter out all versions of
                                     Qt that don't match this minor version.
-        :param is_latest_version:   When True, the ListCommand will find all versions of Qt
+        :param is_latest_version:   When True, the MetadataFactory will find all versions of Qt
                                     matching filters, and only print the most recent version
         :param modules_ver:         Version of Qt for which to list modules
         :param extensions_ver:      Version of Qt for which to list extensions
@@ -351,33 +372,9 @@ class ListCommand:
             self.request_type = "versions"
             self._action = self.fetch_versions
 
-    def action(self) -> Union[List[str], Versions, Table]:
+    def getList(self) -> Union[List[str], Versions, ToolData]:
         return self._action()
 
-    def run(self) -> int:
-        try:
-            output = self.action()
-            if not output:
-                self.logger.info(
-                    "No {} available for this request.".format(self.request_type)
-                )
-                self.print_suggested_follow_up(self.logger.info)
-                return 1
-            if isinstance(output, Versions) or isinstance(output, Table):
-                print(format(output))
-            elif self.archive_id.is_tools():
-                print(*output, sep="\n")
-            else:
-                print(*output, sep=" ")
-            return 0
-        except CliInputError as e:
-            self.logger.error("Command line input error: {}".format(e))
-            return 1
-        except (ArchiveConnectionError, ArchiveDownloadError) as e:
-            self.logger.error("{}".format(e))
-            self.print_suggested_follow_up(self.logger.error)
-            return 1
-
     def fetch_modules(self, version: Version) -> List[str]:
         return self.get_modules_architectures_for_version(version=version)[0]
 
@@ -385,7 +382,7 @@ class ListCommand:
         return self.get_modules_architectures_for_version(version=version)[1]
 
     def fetch_extensions(self, version: Version) -> List[str]:
-        versions_extensions = ListCommand.get_versions_extensions(
+        versions_extensions = MetadataFactory.get_versions_extensions(
             self.fetch_http(self.archive_id.to_url()), self.archive_id.category
         )
         filtered = filter(
@@ -406,7 +403,7 @@ class ListCommand:
         def get_version(ver_ext: Tuple[Version, str]):
             return ver_ext[0]
 
-        versions_extensions = ListCommand.get_versions_extensions(
+        versions_extensions = MetadataFactory.get_versions_extensions(
             self.fetch_http(self.archive_id.to_url()), self.archive_id.category
         )
         versions = sorted(
@@ -420,7 +417,7 @@ class ListCommand:
 
     def fetch_tools(self) -> List[str]:
         html_doc = self.fetch_http(self.archive_id.to_url())
-        return list(ListCommand.iterate_folders(html_doc, "tools"))
+        return list(MetadataFactory.iterate_folders(html_doc, "tools"))
 
     def _fetch_tool_data(
         self, tool_name: str, keys_to_keep: Optional[Iterable[str]] = None
@@ -430,7 +427,7 @@ class ListCommand:
         xml = self.fetch_http(rest_of_url)
         modules = xml_to_modules(
             xml,
-            predicate=ListCommand._has_nonempty_downloads,
+            predicate=MetadataFactory._has_nonempty_downloads,
             keys_to_keep=keys_to_keep,
         )
         return modules
@@ -444,23 +441,10 @@ class ListCommand:
     ) -> Optional[Dict[str, str]]:
         # Get data for all the tool modules
         all_tools_data = self._fetch_tool_data(tool_name)
-        return ListCommand.choose_highest_version_in_spec(all_tools_data, simple_spec)
+        return self.choose_highest_version_in_spec(all_tools_data, simple_spec)
 
-    def fetch_tool_long_listing(self, tool_name: str) -> Table:
-        head = [
-            "Tool Variant Name",
-            "Version",
-            "Release Date",
-            "Display Name",
-            "Description",
-        ]
-        keys = ("Version", "ReleaseDate", "DisplayName", "Description")
-        tool_data = self._fetch_tool_data(tool_name, keys_to_keep=keys)
-        rows = [
-            [name, *[content[key] for key in keys]]
-            for name, content in tool_data.items()
-        ]
-        return Table(head, rows)
+    def fetch_tool_long_listing(self, tool_name: str) -> ToolData:
+        return ToolData(self._fetch_tool_data(tool_name))
 
     def validate_extension(self, qt_ver: Version) -> None:
         """
@@ -600,7 +584,8 @@ class ListCommand:
             )
 
         return map(
-            folder_to_version_extension, ListCommand.iterate_folders(html_doc, category)
+            folder_to_version_extension,
+            MetadataFactory.iterate_folders(html_doc, category),
         )
 
     @staticmethod
@@ -650,7 +635,7 @@ class ListCommand:
         # We want the names of modules, regardless of architecture:
         modules = xml_to_modules(
             xml,
-            predicate=ListCommand._has_nonempty_downloads,
+            predicate=MetadataFactory._has_nonempty_downloads,
             keys_to_keep=(),  # Just want names
         )
 
@@ -680,23 +665,50 @@ class ListCommand:
             return str(self.archive_id)
         return "{} with minor version {}".format(self.archive_id, self.filter_minor)
 
-    def print_suggested_follow_up(self, printer: Callable[[str], None]) -> None:
-        """Makes an informed guess at what the user got wrong, in the event of an error."""
-        base_cmd = "aqt {0.category} {0.host} {0.target}".format(self.archive_id)
-        if self.archive_id.extension:
-            msg = "Please use '{} --extensions <QT_VERSION>' to list valid extensions.".format(
-                base_cmd
-            )
-            printer(msg)
 
-        if self.archive_id.is_tools() and self.request_type == "tool variant names":
-            msg = "Please use '{}' to check what tools are available.".format(base_cmd)
-            printer(msg)
-        elif self.filter_minor is not None:
-            msg = "Please use '{}' to check that versions of {} exist with the minor version '{}'".format(
-                base_cmd, self.archive_id.category, self.filter_minor
-            )
-            printer(msg)
-        elif self.request_type in ("architectures", "modules", "extensions"):
-            msg = "Please use '{}' to show versions of Qt available".format(base_cmd)
-            printer(msg)
+def suggested_follow_up(meta: MetadataFactory, printer: Callable[[str], None]) -> None:
+    """Makes an informed guess at what the user got wrong, in the event of an error."""
+    base_cmd = "aqt {0.category} {0.host} {0.target}".format(meta.archive_id)
+    if meta.archive_id.extension:
+        msg = "Please use '{} --extensions <QT_VERSION>' to list valid extensions.\n".format(
+            base_cmd
+        )
+        printer(msg)
+
+    if meta.archive_id.is_tools() and meta.request_type == "tool variant names":
+        msg = "Please use '{}' to check what tools are available.".format(base_cmd)
+        printer(msg)
+    elif meta.filter_minor is not None:
+        msg = "Please use '{}' to check that versions of {} exist with the minor version '{}'".format(
+            base_cmd, meta.archive_id.category, meta.filter_minor
+        )
+        printer(msg)
+    elif meta.request_type in ("architectures", "modules", "extensions"):
+        msg = "Please use '{}' to show versions of Qt available".format(base_cmd)
+        printer(msg)
+
+
+def show_list(meta: MetadataFactory) -> int:
+    logger = getLogger("aqt.list")
+    try:
+        output = meta.getList()
+        if not output:
+            logger.info("No {} available for this request.".format(meta.request_type))
+            suggested_follow_up(meta, logger.info)
+            return 1
+        if isinstance(output, Versions):
+            print(format(output))
+        elif isinstance(output, ToolData):
+            print(format(output, "{:t}"))  # can set width "{:100t}"
+        elif meta.archive_id.is_tools():
+            print(*output, sep="\n")
+        else:
+            print(*output, sep=" ")
+        return 0
+    except CliInputError as e:
+        logger.error("Command line input error: {}".format(e))
+        return 1
+    except (ArchiveConnectionError, ArchiveDownloadError) as e:
+        logger.error("{}".format(e))
+        suggested_follow_up(meta, logger.error)
+        return 1

ci/logging.ini

@@ -31,6 +31,11 @@ level=DEBUG
 propagate=1
 qualname=aqt.installer
 
+[logger_aqt_list]
+level=INFO
+propagate=1
+qualname=aqt.list
+
 [logger_aqt_updater]
 level=INFO
 propagate=1

tests/test_list.py

@@ -6,7 +6,7 @@ from pathlib import Path
 import pytest
 
 from aqt.installer import Cli
-from aqt.metadata import ArchiveId, ListCommand, SimpleSpec, Version
+from aqt.metadata import ArchiveId, MetadataFactory, SimpleSpec, Version
 
 MINOR_REGEX = re.compile(r"^\d+\.(\d+)")
 
@@ -26,21 +26,21 @@ MINOR_REGEX = re.compile(r"^\d+\.(\d+)")
 )
 def test_list_versions_tools(monkeypatch, os_name, target, in_file, expect_out_file):
     _html = (Path(__file__).parent / "data" / in_file).read_text("utf-8")
-    monkeypatch.setattr(ListCommand, "fetch_http", lambda self, _: _html)
+    monkeypatch.setattr(MetadataFactory, "fetch_http", lambda self, _: _html)
 
     expected = json.loads(
         (Path(__file__).parent / "data" / expect_out_file).read_text("utf-8")
     )
 
     # Test 'aqt list tools'
-    tools = ListCommand(ArchiveId("tools", os_name, target)).action()
+    tools = MetadataFactory(ArchiveId("tools", os_name, target)).getList()
     assert tools == expected["tools"]
 
     for qt in ("qt5", "qt6"):
         for ext, expected_output in expected[qt].items():
             # Test 'aqt list qt'
             archive_id = ArchiveId(qt, os_name, target, ext if ext != "qt" else "")
-            all_versions = ListCommand(archive_id).action()
+            all_versions = MetadataFactory(archive_id).getList()
 
             if len(expected_output) == 0:
                 assert not all_versions
@@ -48,7 +48,7 @@ def test_list_versions_tools(monkeypatch, os_name, target, in_file, expect_out_f
                 assert f"{all_versions}" == "\n".join(expected_output)
 
             # Filter for the latest version only
-            latest_ver = ListCommand(archive_id, is_latest_version=True).action()
+            latest_ver = MetadataFactory(archive_id, is_latest_version=True).getList()
 
             if len(expected_output) == 0:
                 assert not latest_ver
@@ -59,18 +59,18 @@ def test_list_versions_tools(monkeypatch, os_name, target, in_file, expect_out_f
                 minor = int(MINOR_REGEX.search(row).group(1))
 
                 # Find the latest version for a particular minor version
-                latest_ver_for_minor = ListCommand(
+                latest_ver_for_minor = MetadataFactory(
                     archive_id,
                     filter_minor=minor,
                     is_latest_version=True,
-                ).action()
+                ).getList()
                 assert f"{latest_ver_for_minor}" == row.split(" ")[-1]
 
                 # Find all versions for a particular minor version
-                all_ver_for_minor = ListCommand(
+                all_ver_for_minor = MetadataFactory(
                     archive_id,
                     filter_minor=minor,
-                ).action()
+                ).getList()
                 assert f"{all_ver_for_minor}" == row
 
 
@@ -96,12 +96,12 @@ def test_list_architectures_and_modules(
         (Path(__file__).parent / "data" / expect_out_file).read_text("utf-8")
     )
 
-    monkeypatch.setattr(ListCommand, "fetch_http", lambda self, _: _xml)
+    monkeypatch.setattr(MetadataFactory, "fetch_http", lambda self, _: _xml)
 
-    modules = ListCommand(archive_id).fetch_modules(Version(version))
+    modules = MetadataFactory(archive_id).fetch_modules(Version(version))
     assert modules == expect["modules"]
 
-    arches = ListCommand(archive_id).fetch_arches(Version(version))
+    arches = MetadataFactory(archive_id).fetch_arches(Version(version))
     assert arches == expect["architectures"]
 
 
@@ -122,9 +122,9 @@ def test_tool_modules(monkeypatch, host: str, target: str, tool_name: str):
         (Path(__file__).parent / "data" / expect_out_file).read_text("utf-8")
     )
 
-    monkeypatch.setattr(ListCommand, "fetch_http", lambda self, _: _xml)
+    monkeypatch.setattr(MetadataFactory, "fetch_http", lambda self, _: _xml)
 
-    modules = ListCommand(archive_id).fetch_tool_modules(tool_name)
+    modules = MetadataFactory(archive_id).fetch_tool_modules(tool_name)
     assert modules == expect["modules"]
 
 
@@ -145,9 +145,9 @@ def test_tool_long_listing(monkeypatch, host: str, target: str, tool_name: str):
         (Path(__file__).parent / "data" / expect_out_file).read_text("utf-8")
     )
 
-    monkeypatch.setattr(ListCommand, "fetch_http", lambda self, _: _xml)
+    monkeypatch.setattr(MetadataFactory, "fetch_http", lambda self, _: _xml)
 
-    table = ListCommand(archive_id).fetch_tool_long_listing(tool_name)
+    table = MetadataFactory(archive_id).fetch_tool_long_listing(tool_name)
     assert table.rows == expect["long_listing"]
 
 
@@ -196,7 +196,7 @@ def test_list_cli(
         ver_to_replace = ver.replace(".", "")
         return text.replace(ver_to_replace, desired_version)
 
-    monkeypatch.setattr(ListCommand, "fetch_http", _mock)
+    monkeypatch.setattr(MetadataFactory, "fetch_http", _mock)
 
     expected_modules_arches = json.loads(
         (Path(__file__).parent / "data" / xmlexpect).read_text("utf-8")
@@ -275,7 +275,7 @@ def test_list_choose_tool_by_version(simple_spec, expected_name):
         "mytool.350": {"Version": "3.5.0", "Name": "mytool.350"},
         "mytool.300": {"Version": "3.0.0", "Name": "mytool.300"},
     }
-    item = ListCommand.choose_highest_version_in_spec(tools_data, simple_spec)
+    item = MetadataFactory.choose_highest_version_in_spec(tools_data, simple_spec)
     if item is not None:
         assert item["Name"] == expected_name
     else:
@@ -311,7 +311,7 @@ def test_list_invalid_extensions(
     def _mock(_, rest_of_url: str) -> str:
         return ""
 
-    monkeypatch.setattr(ListCommand, "fetch_http", _mock)
+    monkeypatch.setattr(MetadataFactory, "fetch_http", _mock)
 
     cat = "qt" + version[0]
     host = "windows"