pass object instead of parameters

Author: vladak

tools/src/main/python/opengrok_tools/utils/commandsequence.py

@@ -36,16 +36,59 @@
 import re
 
 
+API_TIMEOUT_PROPERTY = "api_timeout"
+ASYNC_API_TIMEOUT_PROPERTY = "async_api_timeout"
+HEADERS_PROPERTY = "headers"
+METHOD_PROPERTY = "method"
+URI_PROPERTY = "uri"
+
+
 class CommandConfigurationException(Exception):
     pass
 
 
+def check_call_config(call):
+    """
+    :param call: dictionary with API call configuration
+    """
+    if not isinstance(call, dict):
+        raise CommandConfigurationException("call value not a dictionary: {}".
+                                            format(call))
+
+    uri = call.get(URI_PROPERTY)
+    if not uri:
+        raise CommandConfigurationException(f"no '{URI_PROPERTY}' key present in {call}")
+
+    method = call.get(METHOD_PROPERTY)
+    if method and method.upper() not in ['GET', 'POST', 'PUT', 'DELETE']:
+        raise CommandConfigurationException(f"invalid HTTP method: {method}")
+
+    headers = call.get(HEADERS_PROPERTY)
+    if headers and not isinstance(headers, dict):
+        raise CommandConfigurationException("headers must be a dictionary")
+
+    call_timeout = call.get(API_TIMEOUT_PROPERTY)
+    if call_timeout:
+        try:
+            int(call_timeout)
+        except ValueError as exc:
+            raise CommandConfigurationException(f"{API_TIMEOUT_PROPERTY} not an integer", exc)
+
+    call_api_timeout = call.get(ASYNC_API_TIMEOUT_PROPERTY)
+    if call_api_timeout:
+        try:
+            int(call_api_timeout)
+        except ValueError as exc:
+            raise CommandConfigurationException(f"{ASYNC_API_TIMEOUT_PROPERTY} not an integer", exc)
+
+
 def check_command_property(command):
     """
     Check if the 'commands' parameter of CommandSequenceBase() has the right structure
     w.r.t. individual commands.
     :param command: command element
     """
+
     if not isinstance(command, dict):
         raise CommandConfigurationException("command '{}' is not a dictionary".format(command))
 
@@ -57,9 +100,42 @@ def check_command_property(command):
     if command_value and not isinstance(command_value, list):
         raise CommandConfigurationException("command value not a list: {}".
                                             format(command_value))
-    if call_value and not isinstance(call_value, dict):
-        raise CommandConfigurationException("call value not a dictionary: {}".
-                                            format(call_value))
+    if call_value:
+        check_call_config(call_value)
+
+
+class ApiCall:
+    """
+    Container class to store properties of API call.
+    """
+    def __init__(self, call_dict):
+        """
+        Initialize the object from a dictionary.
+        :param call_dict: dictionary
+        """
+        if not isinstance(call_dict, dict):
+            raise CommandConfigurationException(f"not a dictionary: {call_dict}")
+
+        self.uri = call_dict.get(URI_PROPERTY)
+        self.method = call_dict.get(METHOD_PROPERTY)
+        if not self.method:
+            self.method = "GET"
+
+        self.data = call_dict.get("data")
+
+        self.headers = call_dict.get(HEADERS_PROPERTY)
+        if not self.headers:
+            self.headers = {}
+
+        self.api_timeout = None
+        call_timeout = call_dict.get(API_TIMEOUT_PROPERTY)
+        if call_timeout:
+            self.api_timeout = call_timeout
+
+        self.async_api_timeout = None
+        call_api_timeout = call_dict.get(ASYNC_API_TIMEOUT_PROPERTY)
+        if call_api_timeout:
+            self.async_api_timeout = call_api_timeout
 
 
 class CommandSequenceBase:
@@ -168,7 +244,7 @@ def run(self):
         for command in self.commands:
             if command.get(CALL_PROPERTY):
                 try:
-                    call_rest_api(command.get(CALL_PROPERTY),
+                    call_rest_api(ApiCall(command.get(CALL_PROPERTY)),
                                   {PROJECT_SUBST: self.name,
                                    URL_SUBST: self.url},
                                   self.http_headers,
@@ -230,7 +306,7 @@ def run_cleanup(self):
         for cleanup_cmd in self.cleanup:
             if cleanup_cmd.get(CALL_PROPERTY):
                 try:
-                    call_rest_api(cleanup_cmd.get(CALL_PROPERTY),
+                    call_rest_api(ApiCall(cleanup_cmd.get(CALL_PROPERTY)),
                                   {PROJECT_SUBST: self.name,
                                    URL_SUBST: self.url},
                                   self.http_headers,

tools/src/main/python/opengrok_tools/utils/mirror.py

@@ -41,7 +41,8 @@
 from .opengrok import get_repos, get_repo_type, get_uri, delete_project_data
 from .hook import run_hook
 from .command import Command
-from .restful import call_rest_api, do_api_call, get_call_props
+from .commandsequence import ApiCall
+from .restful import call_rest_api, do_api_call
 
 from ..scm.repofactory import get_repository
 from ..scm.repository import RepositoryException
@@ -351,27 +352,29 @@ def run_command(cmd, project_name):
 
 def handle_disabled_project(config, project_name, disabled_msg, headers=None,
                             timeout=None, api_timeout=None):
+
     disabled_command = config.get(DISABLED_CMD_PROPERTY)
     if disabled_command:
         logger = logging.getLogger(__name__)
 
         if disabled_command.get(CALL_PROPERTY):
             call = disabled_command.get(CALL_PROPERTY)
-            uri, _, data, _ = get_call_props(call)
-            text = None
+            api_call = ApiCall(call)
 
+            text = None
+            data = api_call.data
             if type(data) is dict:
                 text = data.get("text")
 
             # Is this perhaps OpenGrok API call to supply a Message for the UI ?
             # If so and there was a string supplied, append it to the message text.
-            if text and uri.find("/api/v1/") > 0 and type(disabled_msg) is str:
+            if text and api_call.uri.find("/api/v1/") > 0 and type(disabled_msg) is str:
                 logger.debug("Appending text to message: {}".
                              format(disabled_msg))
-                data["text"] = text + ": " + disabled_msg
+                api_call.data["text"] = text + ": " + disabled_msg
 
             try:
-                call_rest_api(call, {PROJECT_SUBST: project_name},
+                call_rest_api(api_call, {PROJECT_SUBST: project_name},
                               http_headers=headers, timeout=timeout, api_timeout=api_timeout)
             except RequestException as e:
                 logger.error("API call failed for disabled command of "

tools/src/main/python/opengrok_tools/utils/restful.py

@@ -143,49 +143,14 @@ def subst(src, substitutions):
     return src
 
 
-def get_call_props(call):
-    """
-    Retrieve the basic properties of a call.
-    :param call: dictionary
-    :return: URI, HTTP method, data, headers
-    """
-
-    logger = logging.getLogger(__name__)
-
-    uri = call.get("uri")
-    if not uri:
-        raise Exception(f"no 'uri' key present in {call}")
-    if not is_web_uri(uri):
-        raise Exception(f"not a valid URI: {uri}")
-
-    method = call.get("method")
-    if not method:
-        logger.debug(f"no 'method' key in {call}, using GET")
-        method = "GET"
-
-    data = call.get("data")
-
-    try:
-        headers = call.get("headers")
-        if headers and not isinstance(headers, dict):
-            raise Exception("headers must be a dictionary")
-    except IndexError:
-        headers = {}
-
-    if headers is None:
-        headers = {}
-
-    return uri, method, data, headers
-
-
 def call_rest_api(call, substitutions=None, http_headers=None, timeout=None, api_timeout=None):
     """
     Make REST API call. Occurrence of the pattern in the URI
     (first part of the command) or data payload will be replaced by the name.
 
     Default content type is application/json.
 
-    :param call: dictionary describing the properties of the API call
+    :param call: ApiCall object
     :param substitutions: dictionary of pattern:value for command and/or
                           data substitution
     :param http_headers: optional dictionary of HTTP headers to be appended
@@ -196,28 +161,31 @@ def call_rest_api(call, substitutions=None, http_headers=None, timeout=None, api
 
     logger = logging.getLogger(__name__)
 
-    uri, verb, data, headers = get_call_props(call)
-
-    logger.debug(f"Headers from the call structure: {headers}")
+    logger.debug(f"Headers from the ApiCall object: {call.headers}")
+    headers = call.headers
     if http_headers:
-        logger.debug("Updating HTTP headers for call {} with {}".
+        logger.debug("Updating HTTP headers for API call {} with {}".
                      format(call, http_headers))
         headers.update(http_headers)
 
     logger.debug("Performing URI substitutions")
-    uri = subst(uri, substitutions)
+    uri = subst(call.uri, substitutions)
     logger.debug(f"URI after the substitutions: {uri}")
 
-    call_timeout = call.get("api_timeout")
+    if not is_web_uri(uri):
+        raise Exception(f"not a valid URI: {uri}")
+
+    call_timeout = call.api_timeout
     if call_timeout:
         logger.debug(f"Setting connect/read API timeout based on the call to {call_timeout}")
         timeout = call_timeout
 
-    call_api_timeout = call.get("async_api_timeout")
+    call_api_timeout = call.async_api_timeout
     if call_api_timeout:
         logger.debug(f"Setting async API timeout based on the call to {call_api_timeout}")
         api_timeout = call_api_timeout
 
+    data = call.data
     if data:
         header_names = [x.lower() for x in headers.keys()]
         if CONTENT_TYPE.lower() not in header_names:
@@ -235,4 +203,4 @@ def call_rest_api(call, substitutions=None, http_headers=None, timeout=None, api
         data = subst(data, substitutions)
         logger.debug("entity data: {}".format(data))
 
-    return do_api_call(verb, uri, headers=headers, data=data, timeout=timeout, api_timeout=api_timeout)
+    return do_api_call(call.method, uri, headers=headers, data=data, timeout=timeout, api_timeout=api_timeout)

tools/src/test/python/test_mirror.py

@@ -230,7 +230,7 @@ def mock_call_rest_api(command, b, http_headers=None, timeout=None, api_timeout=
         assert mirror_project(config, project_name, False, False,
                               None, None) == CONTINUE_EXITVAL
         verify(opengrok_tools.utils.mirror). \
-            call_rest_api(config.get(DISABLED_CMD_PROPERTY).get(CALL_PROPERTY),
+            call_rest_api(ANY,
                           {PROJECT_SUBST: project_name},
                           http_headers=None, timeout=None, api_timeout=None)
 
@@ -242,14 +242,10 @@ def test_disabled_command_api_text_append(monkeypatch):
 
     text_to_append = "foo bar"
 
-    def mock_call_rest_api(command, b, http_headers=None, timeout=None, api_timeout=None):
-        disabled_command = config.get(DISABLED_CMD_PROPERTY)
-        assert disabled_command
-        command_args = disabled_command.get(COMMAND_PROPERTY)
-        assert command_args
-        data = command_args[2]
+    def mock_call_rest_api(call, b, http_headers=None, timeout=None, api_timeout=None):
+        call_data = call.data
         assert data
-        text = data.get("text")
+        text = call_data.get("text")
         assert text
         assert text.find(text_to_append)
 
@@ -265,9 +261,9 @@ def mock_call_rest_api(command, b, http_headers=None, timeout=None, api_timeout=
                 'text': 'disabled project'}
         config = {
             DISABLED_CMD_PROPERTY: {
-                COMMAND_PROPERTY: [
-                    "http://localhost:8080/source/api/v1/foo",
-                    "POST", data]
+                CALL_PROPERTY: {
+                    "uri": "http://localhost:8080/source/api/v1/foo",
+                    "method": "POST", "data": data}
             },
             PROJECTS_PROPERTY: {
                 project_name: {

tools/src/test/python/test_restful.py

@@ -32,6 +32,7 @@
 
 from opengrok_tools.utils.restful import call_rest_api,\
     CONTENT_TYPE, APPLICATION_JSON, do_api_call
+from opengrok_tools.utils.commandsequence import ApiCall
 
 
 def test_replacement(monkeypatch):
@@ -64,7 +65,7 @@ def mock_do_api_call(verb, uri, **kwargs):
         with monkeypatch.context() as m:
             m.setattr("opengrok_tools.utils.restful.do_api_call",
                       mock_do_api_call)
-            assert call_rest_api(call, {pattern: value}). \
+            assert call_rest_api(ApiCall(call), {pattern: value}). \
                 status_code == okay_status
 
 
@@ -74,7 +75,7 @@ def test_unknown_method():
     pattern = "%FOO%"
     value = "BAR"
     with pytest.raises(Exception):
-        call_rest_api(call, {pattern: value})
+        call_rest_api(ApiCall(call), {pattern: value})
 
 
 def test_content_type(monkeypatch):
@@ -98,7 +99,7 @@ def mock_response(verb, uri, **kwargs):
             with monkeypatch.context() as m:
                 m.setattr("opengrok_tools.utils.restful.do_api_call",
                           mock_response)
-                call_rest_api(call)
+                call_rest_api(ApiCall(call))
 
 
 def test_headers_timeout(monkeypatch):
@@ -110,8 +111,8 @@ def test_headers_timeout(monkeypatch):
     headers = {'Tatsuo': 'Yasuko'}
     expected_timeout = 42
     expected_api_timeout = 24
-    command = {"uri": "http://localhost:8080/source/api/v1/bar",
-               "method": "GET", "data": "data", "headers": headers}
+    call = {"uri": "http://localhost:8080/source/api/v1/bar",
+            "method": "GET", "data": "data", "headers": headers}
     extra_headers = {'Mei': 'Totoro'}
 
     def mock_do_api_call(verb, uri, **kwargs):
@@ -124,7 +125,7 @@ def mock_do_api_call(verb, uri, **kwargs):
     with monkeypatch.context() as m:
         m.setattr("opengrok_tools.utils.restful.do_api_call",
                   mock_do_api_call)
-        call_rest_api(command,
+        call_rest_api(ApiCall(call),
                       http_headers=extra_headers,
                       timeout=expected_timeout,
                       api_timeout=expected_api_timeout)
@@ -171,7 +172,7 @@ def mock_response(uri, headers, data, params, proxies, timeout):
     with monkeypatch.context() as m:
         m.setattr("requests.put", mock_response)
         with pytest.raises(HTTPError):
-            call_rest_api({"uri": 'http://foo', "method": 'PUT', "data": 'data'})
+            call_rest_api(ApiCall({"uri": 'http://foo', "method": 'PUT', "data": 'data'}))
 
 
 def test_invalid_command_none():
@@ -181,14 +182,14 @@ def test_invalid_command_none():
 
 def test_invalid_command_uknown_key():
     with pytest.raises(Exception):
-        call_rest_api({"foo": "bar"})
+        call_rest_api(ApiCall({"foo": "bar"}))
 
 
 def test_invalid_command_list():
     with pytest.raises(Exception):
-        call_rest_api(["foo", "bar"])
+        call_rest_api(ApiCall(["foo", "bar"]))
 
 
 def test_invalid_command_bad_uri():
     with pytest.raises(Exception):
-        call_rest_api({"uri": "foo", "method": "PUT", "data": "data", "headers": "headers"})
+        call_rest_api(ApiCall({"uri": "foo", "method": "PUT", "data": "data", "headers": "headers"}))