diff --git a/changelogs/fragments/8323-refactor-homebrew-logic-module-utils.yml b/changelogs/fragments/8323-refactor-homebrew-logic-module-utils.yml new file mode 100644 index 0000000000..d29aed5ae4 --- /dev/null +++ b/changelogs/fragments/8323-refactor-homebrew-logic-module-utils.yml @@ -0,0 +1,2 @@ +minor_changes: + - "homebrew, homebrew_cask - refactor common argument validation logic into a dedicated ``homebrew`` module utils (https://github.com/ansible-collections/community.general/issues/8323, https://github.com/ansible-collections/community.general/pull/8324)." \ No newline at end of file diff --git a/plugins/module_utils/homebrew.py b/plugins/module_utils/homebrew.py new file mode 100644 index 0000000000..2816832109 --- /dev/null +++ b/plugins/module_utils/homebrew.py @@ -0,0 +1,115 @@ +# -*- coding: utf-8 -*- +# Copyright (c) Ansible project +# Simplified BSD License (see LICENSES/BSD-2-Clause.txt or https://opensource.org/licenses/BSD-2-Clause) +# SPDX-License-Identifier: BSD-2-Clause + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +import os +import re +from ansible.module_utils.six import string_types + + +def _create_regex_group_complement(s): + lines = (line.strip() for line in s.split("\n") if line.strip()) + chars = filter(None, (line.split("#")[0].strip() for line in lines)) + group = r"[^" + r"".join(chars) + r"]" + return re.compile(group) + + +class HomebrewValidate(object): + # class regexes ------------------------------------------------ {{{ + VALID_PATH_CHARS = r""" + \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) + \s # spaces + : # colons + {sep} # the OS-specific path separator + . # dots + \- # dashes + """.format( + sep=os.path.sep + ) + + VALID_BREW_PATH_CHARS = r""" + \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) + \s # spaces + {sep} # the OS-specific path separator + . # dots + \- # dashes + """.format( + sep=os.path.sep + ) + + VALID_PACKAGE_CHARS = r""" + \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) + . # dots + / # slash (for taps) + \+ # plusses + \- # dashes + : # colons (for URLs) + @ # at-sign + """ + + INVALID_PATH_REGEX = _create_regex_group_complement(VALID_PATH_CHARS) + INVALID_BREW_PATH_REGEX = _create_regex_group_complement(VALID_BREW_PATH_CHARS) + INVALID_PACKAGE_REGEX = _create_regex_group_complement(VALID_PACKAGE_CHARS) + # /class regexes ----------------------------------------------- }}} + + # class validations -------------------------------------------- {{{ + @classmethod + def valid_path(cls, path): + """ + `path` must be one of: + - list of paths + - a string containing only: + - alphanumeric characters + - dashes + - dots + - spaces + - colons + - os.path.sep + """ + + if isinstance(path, string_types): + return not cls.INVALID_PATH_REGEX.search(path) + + try: + iter(path) + except TypeError: + return False + else: + paths = path + return all(cls.valid_brew_path(path_) for path_ in paths) + + @classmethod + def valid_brew_path(cls, brew_path): + """ + `brew_path` must be one of: + - None + - a string containing only: + - alphanumeric characters + - dashes + - dots + - spaces + - os.path.sep + """ + + if brew_path is None: + return True + + return isinstance( + brew_path, string_types + ) and not cls.INVALID_BREW_PATH_REGEX.search(brew_path) + + @classmethod + def valid_package(cls, package): + """A valid package is either None or alphanumeric.""" + + if package is None: + return True + + return isinstance( + package, string_types + ) and not cls.INVALID_PACKAGE_REGEX.search(package) diff --git a/plugins/modules/homebrew.py b/plugins/modules/homebrew.py index 144d73a5a6..388682d924 100644 --- a/plugins/modules/homebrew.py +++ b/plugins/modules/homebrew.py @@ -179,9 +179,10 @@ changed_pkgs: ''' import json -import os.path import re +from ansible_collections.community.general.plugins.module_utils.homebrew import HomebrewValidate + from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.six import iteritems, string_types @@ -208,98 +209,7 @@ def _check_package_in_json(json_output, package_type): class Homebrew(object): '''A class to manage Homebrew packages.''' - # class regexes ------------------------------------------------ {{{ - VALID_PATH_CHARS = r''' - \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) - \s # spaces - : # colons - {sep} # the OS-specific path separator - . # dots - \- # dashes - '''.format(sep=os.path.sep) - - VALID_BREW_PATH_CHARS = r''' - \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) - \s # spaces - {sep} # the OS-specific path separator - . # dots - \- # dashes - '''.format(sep=os.path.sep) - - VALID_PACKAGE_CHARS = r''' - \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) - . # dots - / # slash (for taps) - \+ # plusses - \- # dashes - : # colons (for URLs) - @ # at-sign - ''' - - INVALID_PATH_REGEX = _create_regex_group_complement(VALID_PATH_CHARS) - INVALID_BREW_PATH_REGEX = _create_regex_group_complement(VALID_BREW_PATH_CHARS) - INVALID_PACKAGE_REGEX = _create_regex_group_complement(VALID_PACKAGE_CHARS) - # /class regexes ----------------------------------------------- }}} - # class validations -------------------------------------------- {{{ - @classmethod - def valid_path(cls, path): - ''' - `path` must be one of: - - list of paths - - a string containing only: - - alphanumeric characters - - dashes - - dots - - spaces - - colons - - os.path.sep - ''' - - if isinstance(path, string_types): - return not cls.INVALID_PATH_REGEX.search(path) - - try: - iter(path) - except TypeError: - return False - else: - paths = path - return all(cls.valid_brew_path(path_) for path_ in paths) - - @classmethod - def valid_brew_path(cls, brew_path): - ''' - `brew_path` must be one of: - - None - - a string containing only: - - alphanumeric characters - - dashes - - dots - - spaces - - os.path.sep - ''' - - if brew_path is None: - return True - - return ( - isinstance(brew_path, string_types) - and not cls.INVALID_BREW_PATH_REGEX.search(brew_path) - ) - - @classmethod - def valid_package(cls, package): - '''A valid package is either None or alphanumeric.''' - - if package is None: - return True - - return ( - isinstance(package, string_types) - and not cls.INVALID_PACKAGE_REGEX.search(package) - ) - @classmethod def valid_state(cls, state): ''' @@ -359,7 +269,7 @@ class Homebrew(object): @path.setter def path(self, path): - if not self.valid_path(path): + if not HomebrewValidate.valid_path(path): self._path = [] self.failed = True self.message = 'Invalid path: {0}.'.format(path) @@ -379,7 +289,7 @@ class Homebrew(object): @brew_path.setter def brew_path(self, brew_path): - if not self.valid_brew_path(brew_path): + if not HomebrewValidate.valid_brew_path(brew_path): self._brew_path = None self.failed = True self.message = 'Invalid brew_path: {0}.'.format(brew_path) @@ -404,7 +314,7 @@ class Homebrew(object): @current_package.setter def current_package(self, package): - if not self.valid_package(package): + if not HomebrewValidate.valid_package(package): self._current_package = None self.failed = True self.message = 'Invalid package: {0}.'.format(package) @@ -491,7 +401,7 @@ class Homebrew(object): # checks ------------------------------------------------------- {{{ def _current_package_is_installed(self): - if not self.valid_package(self.current_package): + if not HomebrewValidate.valid_package(self.current_package): self.failed = True self.message = 'Invalid package: {0}.'.format(self.current_package) raise HomebrewException(self.message) @@ -514,7 +424,7 @@ class Homebrew(object): return _check_package_in_json(data, "formulae") or _check_package_in_json(data, "casks") def _current_package_is_outdated(self): - if not self.valid_package(self.current_package): + if not HomebrewValidate.valid_package(self.current_package): return False rc, out, err = self.module.run_command([ @@ -526,7 +436,7 @@ class Homebrew(object): return rc != 0 def _current_package_is_installed_from_head(self): - if not Homebrew.valid_package(self.current_package): + if not HomebrewValidate.valid_package(self.current_package): return False elif not self._current_package_is_installed(): return False @@ -624,7 +534,7 @@ class Homebrew(object): # installed ------------------------------ {{{ def _install_current_package(self): - if not self.valid_package(self.current_package): + if not HomebrewValidate.valid_package(self.current_package): self.failed = True self.message = 'Invalid package: {0}.'.format(self.current_package) raise HomebrewException(self.message) @@ -685,7 +595,7 @@ class Homebrew(object): def _upgrade_current_package(self): command = 'upgrade' - if not self.valid_package(self.current_package): + if not HomebrewValidate.valid_package(self.current_package): self.failed = True self.message = 'Invalid package: {0}.'.format(self.current_package) raise HomebrewException(self.message) @@ -756,7 +666,7 @@ class Homebrew(object): # uninstalled ---------------------------- {{{ def _uninstall_current_package(self): - if not self.valid_package(self.current_package): + if not HomebrewValidate.valid_package(self.current_package): self.failed = True self.message = 'Invalid package: {0}.'.format(self.current_package) raise HomebrewException(self.message) @@ -805,7 +715,7 @@ class Homebrew(object): # linked --------------------------------- {{{ def _link_current_package(self): - if not self.valid_package(self.current_package): + if not HomebrewValidate.valid_package(self.current_package): self.failed = True self.message = 'Invalid package: {0}.'.format(self.current_package) raise HomebrewException(self.message) @@ -852,7 +762,7 @@ class Homebrew(object): # unlinked ------------------------------- {{{ def _unlink_current_package(self): - if not self.valid_package(self.current_package): + if not HomebrewValidate.valid_package(self.current_package): self.failed = True self.message = 'Invalid package: {0}.'.format(self.current_package) raise HomebrewException(self.message) diff --git a/plugins/modules/homebrew_cask.py b/plugins/modules/homebrew_cask.py index c992693b68..dc9aea5db8 100644 --- a/plugins/modules/homebrew_cask.py +++ b/plugins/modules/homebrew_cask.py @@ -158,6 +158,7 @@ import re import tempfile from ansible_collections.community.general.plugins.module_utils.version import LooseVersion +from ansible_collections.community.general.plugins.module_utils.homebrew import HomebrewValidate from ansible.module_utils.common.text.converters import to_bytes from ansible.module_utils.basic import AnsibleModule @@ -183,23 +184,6 @@ class HomebrewCask(object): '''A class to manage Homebrew casks.''' # class regexes ------------------------------------------------ {{{ - VALID_PATH_CHARS = r''' - \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) - \s # spaces - : # colons - {sep} # the OS-specific path separator - . # dots - \- # dashes - '''.format(sep=os.path.sep) - - VALID_BREW_PATH_CHARS = r''' - \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) - \s # spaces - {sep} # the OS-specific path separator - . # dots - \- # dashes - '''.format(sep=os.path.sep) - VALID_CASK_CHARS = r''' \w # alphanumeric characters (i.e., [a-zA-Z0-9_]) . # dots @@ -208,58 +192,10 @@ class HomebrewCask(object): @ # at symbol ''' - INVALID_PATH_REGEX = _create_regex_group_complement(VALID_PATH_CHARS) - INVALID_BREW_PATH_REGEX = _create_regex_group_complement(VALID_BREW_PATH_CHARS) INVALID_CASK_REGEX = _create_regex_group_complement(VALID_CASK_CHARS) # /class regexes ----------------------------------------------- }}} # class validations -------------------------------------------- {{{ - @classmethod - def valid_path(cls, path): - ''' - `path` must be one of: - - list of paths - - a string containing only: - - alphanumeric characters - - dashes - - dots - - spaces - - colons - - os.path.sep - ''' - - if isinstance(path, (string_types)): - return not cls.INVALID_PATH_REGEX.search(path) - - try: - iter(path) - except TypeError: - return False - else: - paths = path - return all(cls.valid_brew_path(path_) for path_ in paths) - - @classmethod - def valid_brew_path(cls, brew_path): - ''' - `brew_path` must be one of: - - None - - a string containing only: - - alphanumeric characters - - dashes - - dots - - spaces - - os.path.sep - ''' - - if brew_path is None: - return True - - return ( - isinstance(brew_path, string_types) - and not cls.INVALID_BREW_PATH_REGEX.search(brew_path) - ) - @classmethod def valid_cask(cls, cask): '''A valid cask is either None or alphanumeric + backslashes.''' @@ -321,7 +257,7 @@ class HomebrewCask(object): @path.setter def path(self, path): - if not self.valid_path(path): + if not HomebrewValidate.valid_path(path): self._path = [] self.failed = True self.message = 'Invalid path: {0}.'.format(path) @@ -341,7 +277,7 @@ class HomebrewCask(object): @brew_path.setter def brew_path(self, brew_path): - if not self.valid_brew_path(brew_path): + if not HomebrewValidate.valid_brew_path(brew_path): self._brew_path = None self.failed = True self.message = 'Invalid brew_path: {0}.'.format(brew_path) diff --git a/tests/unit/plugins/modules/test_homebrew.py b/tests/unit/plugins/modules/test_homebrew.py index f849b433df..d04ca4de58 100644 --- a/tests/unit/plugins/modules/test_homebrew.py +++ b/tests/unit/plugins/modules/test_homebrew.py @@ -2,23 +2,28 @@ # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) # SPDX-License-Identifier: GPL-3.0-or-later -from __future__ import (absolute_import, division, print_function) +from __future__ import absolute_import, division, print_function __metaclass__ = type from ansible_collections.community.general.tests.unit.compat import unittest -from ansible_collections.community.general.plugins.modules.homebrew import Homebrew +from ansible_collections.community.general.plugins.module_utils.homebrew import HomebrewValidate class TestHomebrewModule(unittest.TestCase): def setUp(self): - self.brew_app_names = [ - 'git-ssh', - 'awscli@1', - 'bash' + self.brew_app_names = ["git-ssh", "awscli@1", "bash"] + + self.invalid_names = [ + "git ssh", + "git*", ] def test_valid_package_names(self): for name in self.brew_app_names: - self.assertTrue(Homebrew.valid_package(name)) + self.assertTrue(HomebrewValidate.valid_package(name)) + + def test_invalid_package_names(self): + for name in self.invalid_names: + self.assertFalse(HomebrewValidate.valid_package(name))