diff --git a/pg_backup_api/pg_backup_api/logic/utility_controller.py b/pg_backup_api/pg_backup_api/logic/utility_controller.py index 8bc878c..afa27b3 100644 --- a/pg_backup_api/pg_backup_api/logic/utility_controller.py +++ b/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}" diff --git a/pg_backup_api/pg_backup_api/server_operation.py b/pg_backup_api/pg_backup_api/server_operation.py index fcdeb01..443a077 100644 --- a/pg_backup_api/pg_backup_api/server_operation.py +++ b/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) diff --git a/pg_backup_api/pg_backup_api/tests/test_server_operation.py b/pg_backup_api/pg_backup_api/tests/test_server_operation.py index da81632..00a0e86 100644 --- a/pg_backup_api/pg_backup_api/tests/test_server_operation.py +++ b/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, )