Adjust code and unit tests to reflect the final design of `barman con…

…fig-switch`

Previous to this commit we had draft code and unit tests as we were still
working on the implementation of `barman config-switch` command.

Now that the design of `barman config-switch` is mature, we are adjusting
this branch so it gets compliant with the expected implementation of that
command.

References: BAR-125.

Signed-off-by: Israel Barth Rubio <[email protected]>

pg_backup_api/pg_backup_api/logic/utility_controller.py

@@ -156,7 +156,7 @@ def servers_operations_post(server_name: str,
     :param request: the flask request that has been received by the routing
         function.
 
-        Should contain a JSON body with a key ``type``, which identified the
+        Should contain a JSON body with a key ``type``, which identifies the
         type of the operation. The rest of the content depends on the type of
         operation being requested:
 
@@ -168,11 +168,10 @@ def servers_operations_post(server_name: str,
             * ``remote_ssh_command``: SSH command to connect to the target
               machine.
 
-        * ``config_switch`` -- TODO: update arguments:
+        * ``config_switch``:
 
-            * ``to``: some argument;
-            * ``be``: some other argument;
-            * ``defined``: some other argument.
+            * ``model_name``: the name of the model to be applied; or
+            * ``reset``: if you want to unapply a currently active model.
 
     :return: if *server_name* and the JSON body informed through the
         ``POST`` request are valid, return a JSON response containing a key
@@ -217,7 +216,6 @@ def servers_operations_post(server_name: str,
         operation = RecoveryOperation(server_name)
         cmd = f"pg-backup-api recovery --server-name {server_name}"
     elif op_type == OperationType.CONFIG_SWITCH:
-        # TODO: define the logic for performing a config switch operation
         operation = ConfigSwitchOperation(server_name)
         cmd = f"pg-backup-api config-switch --server-name {server_name}"
 

pg_backup_api/pg_backup_api/server_operation.py

@@ -646,13 +646,12 @@ class ConfigSwitchOperation(Operation):
     """
     Contain information and logic to process a config switch operation.
 
-    :cvar REQUIRED_ARGUMENTS: required arguments when creating a config switch
+    :cvar POSSIBLE_ARGUMENTS: possible arguments when creating a config switch
         operation.
     :cvar TYPE: enum type of this operation.
     """
 
-    # TODO: define which arguments will be required
-    REQUIRED_ARGUMENTS = ("to", "be", "defined",)
+    POSSIBLE_ARGUMENTS = ("model_name", "reset",)
     TYPE = OperationType.CONFIG_SWITCH
 
     @classmethod
@@ -664,19 +663,39 @@ def _validate_job_content(cls, content: Dict[str, Any]) -> None:
             job file.
 
         :raises:
-            :exc:`MalformedContent`: if the set of options in *content* is
-                either missing required keys.
+            :exc:`MalformedContent`: if the set of options in *content* is not
+                compliant with the supported options and how to use them.
         """
-        required_args: Set[str] = set(cls.REQUIRED_ARGUMENTS)
-        missing_args = required_args - set(content.keys())
-
-        if missing_args:
+        # One of :attr:`POSSIBLE_ARGUMENTS` must be specified, but not both
+        if not any(arg in content for arg in cls.POSSIBLE_ARGUMENTS):
             msg = (
-                "Missing required arguments: "
-                f"{', '.join(sorted(missing_args))}"
+                "One among the following arguments must be specified: "
+                f"{', '.join(sorted(cls.POSSIBLE_ARGUMENTS))}"
+            )
+            raise MalformedContent(msg)
+        elif all(arg in content for arg in cls.POSSIBLE_ARGUMENTS):
+            msg = (
+                "Only one among the following arguments should be specified: "
+                f"{', '.join(sorted(cls.POSSIBLE_ARGUMENTS))}"
             )
             raise MalformedContent(msg)
 
+        for key, type_ in [
+            ("model_name", str,),
+            ("reset", bool,),
+        ]:
+            if key in content and not isinstance(content[key], type_):
+                msg = (
+                    f"`{key}` is expected to be a `{type_}`, but a "
+                    f"`{type(content[key])}` was found instead: "
+                    f"`{content[key]}`."
+                )
+                raise MalformedContent(msg)
+
+        if "reset" in content and content["reset"] is False:
+            msg = "Value of `reset` key, if present, can only be `True`"
+            raise MalformedContent(msg)
+
     def write_job_file(self, content: Dict[str, Any]) -> None:
         """
         Write the job file with *content*.
@@ -698,44 +717,43 @@ def write_job_file(self, content: Dict[str, Any]) -> None:
 
     def _get_args(self) -> List[str]:
         """
-        Get arguments for running ``barman config switch`` command.
+        Get arguments for running ``barman config-switch`` command.
 
-        :return: list of arguments for ``barman config switch`` command.
+        :return: list of arguments for ``barman config-switch`` command.
         """
         job_content = self.read_job_file()
 
-        # TODO: define which arguments will be used
-        to = job_content.get("to")
-        be = job_content.get("be")
-        defined = job_content.get("defined")
+        model_name = job_content.get("model_name")
+        reset = job_content.get("reset")
 
         if TYPE_CHECKING:  # pragma: no cover
-            assert isinstance(to, str)
-            assert isinstance(be, str)
-            assert isinstance(defined, str)
+            assert model_name is None or isinstance(model_name, str)
+            assert reset is None or isinstance(reset, bool)
 
-        return [
-            self.server.name,
-            to,
-            be,
-            defined,
-        ]
+        ret = [self.server.name]
+
+        if model_name:
+            ret.append(model_name)
+        elif reset:
+            ret.append("--reset")
+
+        return ret
 
     def _run_logic(self) -> \
             Tuple[Union[str, bytearray, memoryview], Union[int, Any]]:
         """
         Logic to be ran when executing the config switch operation.
 
-        Run ``barman config switch`` command with the configured arguments.
+        Run ``barman config-switch`` command with the configured arguments.
 
         Will be called when running :meth:`Operation.run`.
 
         :return: a tuple consisting of:
 
-            * ``stdout``/``stderr`` of ``barman config switch``;
-            * exit code of ``barman config switch``.
+            * ``stdout``/``stderr`` of ``barman config-switch``;
+            * exit code of ``barman config-switch``.
         """
-        cmd = ["barman", "config", "switch"] + self._get_args()
+        cmd = ["barman", "config-switch"] + self._get_args()
         return self._run_subprocess(cmd)
 
 

pg_backup_api/pg_backup_api/tests/test_server_operation.py

@@ -378,7 +378,7 @@ def test_read_job_file_ok(self, op_server):
     def test_read_output_file_file_does_not_exist(self, op_server):
         """Test :meth:`OperationServer._read_output_file`.
 
-        Ensure and exception is raised if the file does not exist.
+        Ensure an exception is raised if the file does not exist.
         """
         id = "SOME_OP_ID"
 
@@ -785,7 +785,7 @@ def test__validate_job_content_content_missing_keys(self, content,
                                                         operation):
         """Test :meth:`RecoveryOperation._validate_job_content`.
 
-        Ensure and exception is raised if the content is missing keys.
+        Ensure an exception is raised if the content is missing keys.
         """
         with pytest.raises(MalformedContent) as exc:
             operation._validate_job_content(content)
@@ -885,49 +885,93 @@ def operation(self):
         """
         return ConfigSwitchOperation(_BARMAN_SERVER)
 
-    # TODO: adjust the parameter names once the final design is defined
-    @pytest.mark.parametrize("content,missing_keys", [
-        ({}, "be, defined, to",),
-        ({"to": "SOME_TO"},
-         "be, defined"),
-        ({"be": "SOME_BE"},
-         "defined, to",),
-        ({"defined": "SOME_DEFINED"},
-         "be, to",),
-        ({"to": "SOME_TO",
-          "be": "SOME_BE"},
-         "defined"),
-        ({"to": "SOME_TO",
-          "defined": "SOME_DEFINED"},
-         "be"),
-        ({"be": "SOME_BE",
-          "defined": "SOME_DEFINED"},
-         "to"),
-    ])
-    def test__validate_job_content_content_missing_keys(self, content,
-                                                        missing_keys,
-                                                        operation):
+    def test__validate_job_content_content_missing_both_keys(self, operation):
         """Test :meth:`ConfigSwitchOperation._validate_job_content`.
 
-        Ensure and exception is raised if the content is missing keys.
+        Ensure an exception is raised if the content is missing both
+        ``model_name`` and ``reset`` keys.
         """
         with pytest.raises(MalformedContent) as exc:
-            operation._validate_job_content(content)
+            operation._validate_job_content({})
 
-        assert str(exc.value) == f"Missing required arguments: {missing_keys}"
+        assert str(exc.value) == (
+            "One among the following arguments must be specified: "
+            "model_name, reset"
+        )
 
-    def test__validate_job_content_ok(self, operation):
+    def test__validate_job_content_content_contains_both_keys(self, operation):
         """Test :meth:`ConfigSwitchOperation._validate_job_content`.
 
-        Ensure execution is fine if everything is filled as expected.
+        Ensure an exception is raised if the content has both ``model_name``
+        and ``reset`` keys.
         """
-        # TODO: adjust the parameter names once the final design is defined
-        content = {
-            "to": "SOME_TO",
-            "be": "SOME_BE",
-            "defined": "SOME_DEFINED",
-        }
-        operation._validate_job_content(content)
+        with pytest.raises(MalformedContent) as exc:
+            operation._validate_job_content({
+                "model_name": "SOME_MODEL",
+                "reset": True,
+            })
+
+        assert str(exc.value) == (
+            "Only one among the following arguments should be specified: "
+            "model_name, reset"
+        )
+
+    @pytest.mark.parametrize("model_name", [1, 1.0, True, None])
+    def test__validate_job_content_invalid_model_name_type(self, model_name,
+                                                           operation):
+        """Test :meth:`ConfigSwitchOperation._validate_job_content`.
+
+        Ensure an exception is raised if ``model_name`` has a value of an
+        invalid type.
+        """
+        with pytest.raises(MalformedContent) as exc:
+            operation._validate_job_content({"model_name": model_name})
+
+        assert str(exc.value) == (
+            f"`model_name` is expected to be a `{str}`, but a "
+            f"`{type(model_name)}` was found instead: `{model_name}`."
+        )
+
+    @pytest.mark.parametrize("reset", [1, 1.0, "true", None])
+    def test__validate_job_content_invalid_reset_type(self, reset, operation):
+        """Test :meth:`ConfigSwitchOperation._validate_job_content`.
+
+        Ensure an exception is raised if ``reset`` has a value of an
+        invalid type.
+        """
+        with pytest.raises(MalformedContent) as exc:
+            operation._validate_job_content({"reset": reset})
+
+        assert str(exc.value) == (
+            f"`reset` is expected to be a `{bool}`, but a "
+            f"`{type(reset)}` was found instead: `{reset}`."
+        )
+
+    def test__validate_job_content_invalid_reset_value(self, operation):
+        """Test :meth:`ConfigSwitchOperation._validate_job_content`.
+
+        Ensure an exception is raised if ``reset`` has an invalid value.
+        """
+        with pytest.raises(MalformedContent) as exc:
+            operation._validate_job_content({"reset": False})
+
+        assert str(exc.value) == (
+            "Value of `reset` key, if present, can only be `True`"
+        )
+
+    def test__validate_job_content_apply_model_ok(self, operation):
+        """Test :meth:`ConfigSwitchOperation._validate_job_content`.
+
+        Ensure execution is fine if only a valid ``model_name`` is given.
+        """
+        operation._validate_job_content({"model_name": "SOME_MODEL"})
+
+    def test__validate_job_content_reset_model_ok(self, operation):
+        """Test :meth:`ConfigSwitchOperation._validate_job_content`.
+
+        Ensure execution is fine if only a valid ``reset`` is given.
+        """
+        operation._validate_job_content({"reset": True})
 
     @patch("pg_backup_api.server_operation.Operation.time_event_now")
     @patch("pg_backup_api.server_operation.Operation.write_job_file")
@@ -937,7 +981,6 @@ def test_write_job_file(self, mock_write_job_file, mock_time_event_now,
 
         Ensure the underlying methods are called as expected.
         """
-        # TODO: adjust the parameter names once the final design is defined
         content = {
             "SOME": "CONTENT",
         }
@@ -956,25 +999,28 @@ def test_write_job_file(self, mock_write_job_file, mock_time_event_now,
             mock.assert_called_once_with(extended_content)
             mock_write_job_file.assert_called_once_with(extended_content)
 
-    def test__get_args(self, operation):
+    def test__get_args_apply_model(self, operation):
         """Test :meth:`ConfigSwitchOperation._get_args`.
 
-        Ensure it returns the correct arguments for ``barman recover``.
+        Ensure it returns the correct arguments for ``barman config-switch``
+        when ``model_name`` is given.
         """
-        # TODO: adjust the parameter names once the final design is defined
         with patch.object(operation, "read_job_file") as mock:
-            mock.return_value = {
-                "to": "SOME_TO",
-                "be": "SOME_BE",
-                "defined": "SOME_DEFINED",
-            }
+            mock.return_value = {"model_name": "SOME_MODEL"}
 
-            expected = [
-                operation.server.name,
-                "SOME_TO",
-                "SOME_BE",
-                "SOME_DEFINED",
-            ]
+            expected = [operation.server.name, "SOME_MODEL"]
+            assert operation._get_args() == expected
+
+    def test__get_args_reset_model(self, operation):
+        """Test :meth:`ConfigSwitchOperation._get_args`.
+
+        Ensure it returns the correct arguments for ``barman config-switch``
+        when ``reset`` is given.
+        """
+        with patch.object(operation, "read_job_file") as mock:
+            mock.return_value = {"reset": True}
+
+            expected = [operation.server.name, "--reset"]
             assert operation._get_args() == expected
 
     @patch("pg_backup_api.server_operation.Operation._run_subprocess")
@@ -994,5 +1040,5 @@ def test__run_logic(self, mock_get_args, mock_run_subprocess, operation):
 
         mock_get_args.assert_called_once()
         mock_run_subprocess.assert_called_once_with(
-            ["barman", "config", "switch"] + arguments,
+            ["barman", "config-switch"] + arguments,
         )