From 9ef57818bec730db893338434fa78c6268bad245 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Mon, 23 Dec 2024 12:01:02 -0500 Subject: [PATCH] Addon Manager: refactor process_string_to_datetime (#18492) * Addon Manager: Refactor utilities tests to remove filesystem use * Addon Manager: Move process_date_string_to_python_datetime to utilities Also add unit tests and modify the exception type * Addon Manager: Add tests for other date separators * Addon Manager: Refactor to reduce duplication * Addon Manager: add explanation of why the function exists * Addon Manager: use exception chaining * Addon Manager: Remove unused test files --- src/Mod/AddonManager/Addon.py | 40 +------- .../AddonManagerTest/app/test_utilities.py | 94 +++++++++++++++---- .../data/bad_macro_metadata.FCStd | 37 -------- .../data/good_macro_metadata.FCStd | 37 -------- src/Mod/AddonManager/CMakeLists.txt | 2 - .../AddonManager/addonmanager_utilities.py | 35 +++++++ 6 files changed, 115 insertions(+), 130 deletions(-) delete mode 100644 src/Mod/AddonManager/AddonManagerTest/data/bad_macro_metadata.FCStd delete mode 100644 src/Mod/AddonManager/AddonManagerTest/data/good_macro_metadata.FCStd diff --git a/src/Mod/AddonManager/Addon.py b/src/Mod/AddonManager/Addon.py index 87f7c75aef..a8f1b51a0d 100644 --- a/src/Mod/AddonManager/Addon.py +++ b/src/Mod/AddonManager/Addon.py @@ -35,7 +35,7 @@ import xml.etree.ElementTree import addonmanager_freecad_interface as fci from addonmanager_macro import Macro import addonmanager_utilities as utils -from addonmanager_utilities import construct_git_url +from addonmanager_utilities import construct_git_url, process_date_string_to_python_datetime from addonmanager_metadata import ( Metadata, MetadataReader, @@ -251,49 +251,15 @@ class Addon: elif self.macro and self.macro.date: # Try to parse the date: try: - self._cached_update_date = self._process_date_string_to_python_datetime( + self._cached_update_date = process_date_string_to_python_datetime( self.macro.date ) - except SyntaxError as e: + except ValueError as e: fci.Console.PrintWarning(str(e) + "\n") else: fci.Console.PrintWarning(f"No update date info for {self.name}\n") return self._cached_update_date - def _process_date_string_to_python_datetime(self, date_string: str) -> datetime: - split_result = re.split(r"[ ./-]+", date_string.strip()) - if len(split_result) != 3: - raise SyntaxError( - f"In macro {self.name}, unrecognized date string '{date_string}' (expected YYYY-MM-DD)" - ) - - if int(split_result[0]) > 2000: # Assume YYYY-MM-DD - try: - year = int(split_result[0]) - month = int(split_result[1]) - day = int(split_result[2]) - return datetime(year, month, day) - except (OverflowError, OSError, ValueError): - raise SyntaxError( - f"In macro {self.name}, unrecognized date string {date_string} (expected YYYY-MM-DD)" - ) - elif int(split_result[2]) > 2000: - # Two possibilities, impossible to distinguish in the general case: DD-MM-YYYY and - # MM-DD-YYYY. See if the first one makes sense, and if not, try the second - if int(split_result[1]) <= 12: - year = int(split_result[2]) - month = int(split_result[1]) - day = int(split_result[0]) - else: - year = int(split_result[2]) - month = int(split_result[0]) - day = int(split_result[1]) - return datetime(year, month, day) - else: - raise SyntaxError( - f"In macro {self.name}, unrecognized date string '{date_string}' (expected YYYY-MM-DD)" - ) - @classmethod def from_macro(cls, macro: Macro): """Create an Addon object from a Macro wrapper object""" diff --git a/src/Mod/AddonManager/AddonManagerTest/app/test_utilities.py b/src/Mod/AddonManager/AddonManagerTest/app/test_utilities.py index cc00fd8dfd..66035acc74 100644 --- a/src/Mod/AddonManager/AddonManagerTest/app/test_utilities.py +++ b/src/Mod/AddonManager/AddonManagerTest/app/test_utilities.py @@ -21,8 +21,9 @@ # * * # *************************************************************************** +from datetime import datetime import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, patch, mock_open import os import sys import subprocess @@ -37,10 +38,11 @@ sys.path.append("../..") from AddonManagerTest.app.mocks import MockAddon as Addon from addonmanager_utilities import ( - recognized_git_location, - get_readme_url, get_assigned_string_literal, get_macro_version_from_file, + get_readme_url, + process_date_string_to_python_datetime, + recognized_git_location, run_interruptable_subprocess, ) @@ -49,9 +51,6 @@ class TestUtilities(unittest.TestCase): MODULE = "test_utilities" # file name without extension - def setUp(self): - pass - @classmethod def tearDownClass(cls): try: @@ -132,21 +131,22 @@ class TestUtilities(unittest.TestCase): result = get_assigned_string_literal(line) self.assertIsNone(result) - def test_get_macro_version_from_file(self): - if FreeCAD: - test_dir = os.path.join( - FreeCAD.getHomePath(), "Mod", "AddonManager", "AddonManagerTest", "data" - ) - good_file = os.path.join(test_dir, "good_macro_metadata.FCStd") - version = get_macro_version_from_file(good_file) + def test_get_macro_version_from_file_good_metadata(self): + good_metadata = """__Version__ = "1.2.3" """ + with patch("builtins.open", new_callable=mock_open, read_data=good_metadata): + version = get_macro_version_from_file("mocked_file.FCStd") self.assertEqual(version, "1.2.3") - bad_file = os.path.join(test_dir, "bad_macro_metadata.FCStd") - version = get_macro_version_from_file(bad_file) + def test_get_macro_version_from_file_missing_quotes(self): + bad_metadata = """__Version__ = 1.2.3 """ # No quotes + with patch("builtins.open", new_callable=mock_open, read_data=bad_metadata): + version = get_macro_version_from_file("mocked_file.FCStd") self.assertEqual(version, "", "Bad version did not yield empty string") - empty_file = os.path.join(test_dir, "missing_macro_metadata.FCStd") - version = get_macro_version_from_file(empty_file) + def test_get_macro_version_from_file_no_version(self): + good_metadata = "" + with patch("builtins.open", new_callable=mock_open, read_data=good_metadata): + version = get_macro_version_from_file("mocked_file.FCStd") self.assertEqual(version, "", "Missing version did not yield empty string") @patch("subprocess.Popen") @@ -222,6 +222,66 @@ class TestUtilities(unittest.TestCase): with patch("time.time", fake_time): run_interruptable_subprocess(["arg0", "arg1"], 0.1) + def test_process_date_string_to_python_datetime_non_numeric(self): + with self.assertRaises(ValueError): + process_date_string_to_python_datetime("TwentyTwentyFour-January-ThirtyFirst") + + def test_process_date_string_to_python_datetime_year_first(self): + result = process_date_string_to_python_datetime("2024-01-31") + expected_result = datetime(2024, 1, 31, 0, 0) + self.assertEqual(result, expected_result) + + def test_process_date_string_to_python_datetime_day_first(self): + result = process_date_string_to_python_datetime("31-01-2024") + expected_result = datetime(2024, 1, 31, 0, 0) + self.assertEqual(result, expected_result) + + def test_process_date_string_to_python_datetime_month_first(self): + result = process_date_string_to_python_datetime("01-31-2024") + expected_result = datetime(2024, 1, 31, 0, 0) + self.assertEqual(result, expected_result) + + def test_process_date_string_to_python_datetime_ambiguous(self): + """In the ambiguous case, the code should assume that the date is in the DD-MM-YYYY format.""" + result = process_date_string_to_python_datetime("01-12-2024") + expected_result = datetime(2024, 12, 1, 0, 0) + self.assertEqual(result, expected_result) + + def test_process_date_string_to_python_datetime_invalid_date(self): + with self.assertRaises(ValueError): + process_date_string_to_python_datetime("13-31-2024") + + def test_process_date_string_to_python_datetime_too_many_components(self): + with self.assertRaises(ValueError): + process_date_string_to_python_datetime("01-01-31-2024") + + def test_process_date_string_to_python_datetime_too_few_components(self): + """Month-Year-only dates are not supported""" + with self.assertRaises(ValueError): + process_date_string_to_python_datetime("01-2024") + + def test_process_date_string_to_python_datetime_unrecognizable(self): + """Two-digit years are not supported""" + with self.assertRaises(ValueError): + process_date_string_to_python_datetime("01-02-24") + + def test_process_date_string_to_python_datetime_valid_separators(self): + """Four individual separators are supported, plus any combination of multiple of those separators""" + valid_separators = [" ", ".", "/", "-", " - ", " / ", "--"] + for separator in valid_separators: + with self.subTest(separator=separator): + result = process_date_string_to_python_datetime(f"2024{separator}01{separator}31") + expected_result = datetime(2024, 1, 31, 0, 0) + self.assertEqual(result, expected_result) + + def test_process_date_string_to_python_datetime_invalid_separators(self): + """Only the four separators [ ./-] are supported: ensure others fail""" + invalid_separators = ["a", "\\", "|", "'", ";", "*", " \\ "] + for separator in invalid_separators: + with self.subTest(separator=separator): + with self.assertRaises(ValueError): + process_date_string_to_python_datetime(f"2024{separator}01{separator}31") + if __name__ == "__main__": unittest.main() diff --git a/src/Mod/AddonManager/AddonManagerTest/data/bad_macro_metadata.FCStd b/src/Mod/AddonManager/AddonManagerTest/data/bad_macro_metadata.FCStd deleted file mode 100644 index f3932bbfe3..0000000000 --- a/src/Mod/AddonManager/AddonManagerTest/data/bad_macro_metadata.FCStd +++ /dev/null @@ -1,37 +0,0 @@ -# -*- coding: utf-8 -*- - -# *************************************************************************** -# * Copyright (c) 2022 FreeCAD Project Association * -# * * -# * This file is part of the FreeCAD CAx development system. * -# * * -# * This library is free software; you can redistribute it and/or * -# * modify it under the terms of the GNU Lesser General Public * -# * License as published by the Free Software Foundation; either * -# * version 2.1 of the License, or (at your option) any later version. * -# * * -# * This library is distributed in the hope that it will be useful, * -# * but WITHOUT ANY WARRANTY; without even the implied warranty of * -# * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * -# * Lesser General Public License for more details. * -# * * -# * You should have received a copy of the GNU Lesser General Public * -# * License along with this library; if not, write to the Free Software * -# * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * -# * 02110-1301 USA * -# * * -# *************************************************************************** - -__Title__ = "Test Macro' # Mismatched quotes -__Author__ = Chris Hennes # Not in quotes -__Version__ = 1.2.3 # Not in quotes and not a number -__Date__ = "2022-2-25 # Missing quote -__Comment__ = """For use with the FreeCAD unit test suite""" # Triple-quotes not allowed -__Web__ = "https://freecad.org" -__Wiki__ = "" -__Icon__ = "" -__Help__ = "" -__Status__ = "" -__Requires__ = "" -__Communication__ = "" -__Files__ = "" diff --git a/src/Mod/AddonManager/AddonManagerTest/data/good_macro_metadata.FCStd b/src/Mod/AddonManager/AddonManagerTest/data/good_macro_metadata.FCStd deleted file mode 100644 index 2968bb5917..0000000000 --- a/src/Mod/AddonManager/AddonManagerTest/data/good_macro_metadata.FCStd +++ /dev/null @@ -1,37 +0,0 @@ -# -*- coding: utf-8 -*- - -# *************************************************************************** -# * Copyright (c) 2022 FreeCAD Project Association * -# * * -# * This file is part of the FreeCAD CAx development system. * -# * * -# * This library is free software; you can redistribute it and/or * -# * modify it under the terms of the GNU Lesser General Public * -# * License as published by the Free Software Foundation; either * -# * version 2.1 of the License, or (at your option) any later version. * -# * * -# * This library is distributed in the hope that it will be useful, * -# * but WITHOUT ANY WARRANTY; without even the implied warranty of * -# * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * -# * Lesser General Public License for more details. * -# * * -# * You should have received a copy of the GNU Lesser General Public * -# * License along with this library; if not, write to the Free Software * -# * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * -# * 02110-1301 USA * -# * * -# *************************************************************************** - -__Title__ = "Test Macro" -__Author__ = "Chris Hennes" -__Version__ = "1.2.3" -__Date__ = "2022-2-25" -__Comment__ = "For use with the FreeCAD unit test suite" -__Web__ = "https://freecad.org" -__Wiki__ = "" -__Icon__ = "" -__Help__ = "" -__Status__ = "" -__Requires__ = "" -__Communication__ = "" -__Files__ = "" diff --git a/src/Mod/AddonManager/CMakeLists.txt b/src/Mod/AddonManager/CMakeLists.txt index d376834d1c..58b15d132b 100644 --- a/src/Mod/AddonManager/CMakeLists.txt +++ b/src/Mod/AddonManager/CMakeLists.txt @@ -118,13 +118,11 @@ SET(AddonManagerTestsGui_SRCS SET(AddonManagerTestsFiles_SRCS AddonManagerTest/data/__init__.py AddonManagerTest/data/addon_update_stats.json - AddonManagerTest/data/bad_macro_metadata.FCStd AddonManagerTest/data/combination.xml AddonManagerTest/data/corrupted_metadata.zip AddonManagerTest/data/depends_on_all_workbenches.xml AddonManagerTest/data/DoNothing.FCMacro AddonManagerTest/data/git_submodules.txt - AddonManagerTest/data/good_macro_metadata.FCStd AddonManagerTest/data/good_package.xml AddonManagerTest/data/icon_cache.zip AddonManagerTest/data/icon_cache.zip.sha1 diff --git a/src/Mod/AddonManager/addonmanager_utilities.py b/src/Mod/AddonManager/addonmanager_utilities.py index 517d0fbbd6..b0e66ca00b 100644 --- a/src/Mod/AddonManager/addonmanager_utilities.py +++ b/src/Mod/AddonManager/addonmanager_utilities.py @@ -24,8 +24,12 @@ """ Utilities to work across different platforms, providers and python versions """ +from datetime import datetime +from typing import Optional, Any +import ctypes import os import platform +import re import shutil import stat import subprocess @@ -443,6 +447,37 @@ def run_interruptable_subprocess(args, timeout_secs: int = 10) -> subprocess.Com return subprocess.CompletedProcess(args, return_code, stdout, stderr) +def process_date_string_to_python_datetime(date_string: str) -> datetime: + """For modern macros the expected date format is ISO 8601, YYYY-MM-DD. For older macros this standard was not always + used, and various orderings and separators were used. This function tries to match the majority of those older + macros. Commonly-used separators are periods, slashes, and dashes.""" + + def raise_error(bad_string: str, root_cause: Exception = None): + raise ValueError( + f"Unrecognized date string '{bad_string}' (expected YYYY-MM-DD)" + ) from root_cause + + split_result = re.split(r"[ ./-]+", date_string.strip()) + if len(split_result) != 3: + raise_error(date_string) + + try: + split_result = [int(x) for x in split_result] + # The earliest possible year an addon can be created or edited is 2001: + if split_result[0] > 2000: + return datetime(split_result[0], split_result[1], split_result[2]) + elif split_result[2] > 2000: + # Generally speaking it's not possible to distinguish between DD-MM and MM-DD, so try the first, and + # only if that fails try the second + if split_result[1] <= 12: + return datetime(split_result[2], split_result[1], split_result[0]) + return datetime(split_result[2], split_result[0], split_result[1]) + else: + raise ValueError(f"Invalid year in date string '{date_string}'") + except ValueError as exception: + raise_error(date_string, exception) + + def get_main_am_window(): windows = QtWidgets.QApplication.topLevelWidgets() for widget in windows: