Feat: API v2 (#242)

* Introducing version | v parameter.

* Consistent arguments of API v2.

* Fix linting issues.

* Changed response for cancelTask command in v2.

* getStatus command response remains.

* Fixing mypy issues.

* Changing response for update command of v2.

* Changing the response for listAllStatus command of v2.

* Fix linting issue.

* Partial revert: keeping id naming.

* Minor: readability.

* Ref: readbility for getStatus command branching.

* Update octoprint_octorelay/__init__.py

* Ref: Ensure int for version

* Ref: todo for subject parameter name for next major release

* Type constraints for listing command.

* Fix linting issue.

* UI: Switching to API v2.

octoprint_octorelay/__init__.py

@@ -19,6 +19,7 @@
 )
 from .driver import Relay
 from .task import Task
+from .listing import Listing
 from .migrations import migrate
 from .model import Model, get_initial_model
 from .exceptions import HandlingException
@@ -101,9 +102,9 @@ def has_switch_permission(self):
             self._logger.warn(f"Failed to check relay switching permission, {exception}")
             return False
 
-    def handle_list_all_command(self):
+    def handle_list_all_command(self) -> Listing:
         self._logger.debug("Collecting information on all the relay states")
-        active_relays = []
+        active_relays: Listing = []
         settings = self._settings.get([], merged=True) # expensive
         for index in RELAY_INDEXES:
             if bool(settings[index]["active"]):
@@ -114,7 +115,7 @@ def handle_list_all_command(self):
                 active_relays.append({
                     "id": index,
                     "name": settings[index]["label_text"],
-                    "active": relay.is_closed(),
+                    "status": relay.is_closed(),
                 })
         return active_relays
 
@@ -152,33 +153,55 @@ def handle_cancel_task_command(self, subject: str, target: bool, owner: str) ->
     def on_api_command(self, command, data):
         # pylint: disable=too-many-return-statements
         self._logger.info(f"Received the API command {command} with parameters: {data}")
-        index = data.get("pin")
-        if command in [GET_STATUS_COMMAND, UPDATE_COMMAND] and index is None:
-            return flask.abort(400, description="Parameter pin is missing")
-        if command == LIST_ALL_COMMAND: # API command to get relay statuses
-            response = self.handle_list_all_command()
-            self._logger.info(f"Responding to {LIST_ALL_COMMAND} command: {response}")
+        version = int( data.get("version") or data.get("v") or 1 )
+        subject_param_name = "pin" if version == 1 else "subject" # todo remove pin when dropping v1
+        subject = data.get(subject_param_name)
+        target = data.get("target")
+        if command in [GET_STATUS_COMMAND, UPDATE_COMMAND] and subject is None:
+            return flask.abort(400, description=f"Parameter {subject_param_name} is missing")
+        # API command to list all the relays with their names and statuses
+        if command == LIST_ALL_COMMAND:
+            relays = self.handle_list_all_command()
+            response = list(map(lambda item: {
+                "id": item["id"],
+                "name": item["name"],
+                "active": item["status"]
+            }, relays)) if version == 1 else relays # todo remove ternary branch when dropping v1
+            self._logger.info(f"Responding {response} to {LIST_ALL_COMMAND} command")
             return flask.jsonify(response)
-        if command == GET_STATUS_COMMAND: # API command to get relay status
+        # API command to get relay status
+        if command == GET_STATUS_COMMAND:
+            is_closed = False # todo remove this when dropping v1
             try:
-                is_closed = self.handle_get_status_command(index)
-            except HandlingException:
-                is_closed = False # todo should just abort in the next version
-            self._logger.info(f"Responding to {GET_STATUS_COMMAND} command: {is_closed}")
-            return flask.jsonify({"status": is_closed})
-        if command == UPDATE_COMMAND: # API command to toggle the relay
-            target = data.get("target")
+                is_closed = self.handle_get_status_command(subject)
+            except HandlingException as exception:
+                if version != 1: # todo remove condition when dropping v1
+                    return flask.abort(exception.status)
+            self._logger.info(f"Responding {is_closed} to {GET_STATUS_COMMAND} command")
+            return flask.jsonify({ "status": is_closed })
+        # API command to toggle the relay
+        if command == UPDATE_COMMAND:
             try:
-                state = self.handle_update_command(index, target if isinstance(target, bool) else None)
-                self._logger.debug(f"Responding to {UPDATE_COMMAND} command. Switched state to {state}")
-                return flask.jsonify({"status": "ok", "result": state})
+                state = self.handle_update_command(subject, target if isinstance(target, bool) else None)
+                self._logger.debug(f"Responding {state} to {UPDATE_COMMAND} command")
+                if version == 1:
+                    return flask.jsonify({ "status": "ok", "result": state }) # todo remove branch when dropping v1
+                return flask.jsonify({ "status": state })
             except HandlingException as exception: # todo: deprecate the behavior for 400, only abort in next version
-                return flask.jsonify({"status": "error"}) if exception.status == 400 else flask.abort(exception.status)
-        if command == CANCEL_TASK_COMMAND: # API command to cancel the postponed toggling task
-            self.handle_cancel_task_command(data["subject"], bool(data["target"]), data["owner"])
-            self._logger.debug(f"Responding to {CANCEL_TASK_COMMAND} command")
-            return flask.jsonify({"status": "ok"})
-        self._logger.warn(f"Unknown command {command}")
+                if version == 1 and exception.status == 400:
+                    return flask.jsonify({ "status": "error" }) # todo remove this branch when dropping v1
+                return flask.abort(exception.status)
+        # API command to cancel the postponed toggling task
+        if command == CANCEL_TASK_COMMAND:
+            cancelled = self.handle_cancel_task_command(
+                data.get("subject"), bool(target), data["owner"] # todo use subject after dropping v1
+            )
+            self._logger.debug(f"Responding {cancelled} to {CANCEL_TASK_COMMAND} command")
+            if version == 1:
+                return flask.jsonify({ "status": "ok" }) # todo remove this branch when dropping v1
+            return flask.jsonify({ "cancelled": cancelled })
+        # Unknown commands
+        self._logger.warn(f"Received unknown API command {command}")
         return flask.abort(400, description="Unknown command")
 
     def on_event(self, event, payload):

octoprint_octorelay/listing.py

@@ -0,0 +1,14 @@
+# -*- coding: utf-8 -*-
+from typing import List
+# todo after dropping 3.7 take TypedDict from typing instead
+from typing_extensions import TypedDict
+
+# false positive, see https://github.com/pylint-dev/pylint/issues/4166
+# pylint: disable=too-many-ancestors
+# pylint: disable=too-few-public-methods
+class Element(TypedDict):
+    id: str
+    name: str
+    status: bool
+
+Listing = List[Element]

setup.py

@@ -50,7 +50,7 @@ def get_version_and_cmdclass(pkg_path):
 plugin_license = "AGPLv3"
 
 # Any additional requirements besides OctoPrint should be listed here
-# todo after dropping 3.7 remove typing-extensions (used by model.py)
+# todo after dropping 3.7 remove typing-extensions (used by model.py and listing.py)
 plugin_requires = ["RPi.GPIO", "typing-extensions"]
 
 # --------------------------------------------------------------------------------------------------------------------

tests/test_init.py

@@ -827,14 +827,14 @@ def test_handle_list_all_command(self):
             "expectedJson": list(map(lambda index: {
                 "id": index,
                 "name": "TEST",
-                "active": False
+                "status": False
             }, RELAY_INDEXES))
         }, {
             "closed": True,
             "expectedJson": list(map(lambda index: {
                 "id": index,
                 "name": "TEST",
-                "active": True
+                "status": True
             }, RELAY_INDEXES))
         }]
         for case in cases:
@@ -999,7 +999,9 @@ def test_handle_cancel_task_command(self):
     @patch("flask.jsonify")
     def test_on_api_command(self, jsonify_mock):
         # Should call a handler and respond with expected payload
-        self.plugin_instance.handle_list_all_command = Mock(return_value=[])
+        self.plugin_instance.handle_list_all_command = Mock(return_value=[
+            {"id": "r1", "name": "Test", "status": True}
+        ])
         self.plugin_instance.handle_get_status_command = Mock(return_value=True)
         self.plugin_instance.handle_update_command = Mock(return_value=False)
         self.plugin_instance.handle_cancel_task_command = Mock(return_value=True)
@@ -1010,7 +1012,15 @@ def test_on_api_command(self, jsonify_mock):
                 "expectedMethod": self.plugin_instance.handle_list_all_command,
                 "expectedArguments": [],
                 "expectedOutcome": jsonify_mock,
-                "expectedPayload": [],
+                "expectedPayload": [{"id": "r1", "name": "Test", "active": True}],
+            },
+            {
+                "command": "listAllStatus",
+                "data": {"v": 2},
+                "expectedMethod": self.plugin_instance.handle_list_all_command,
+                "expectedArguments": [],
+                "expectedOutcome": jsonify_mock,
+                "expectedPayload": [{"id": "r1", "name": "Test", "status": True}],
             },
             {
                 "command": "getStatus",
@@ -1020,6 +1030,14 @@ def test_on_api_command(self, jsonify_mock):
                 "expectedOutcome": jsonify_mock,
                 "expectedPayload": {"status": True},
             },
+            {
+                "command": "getStatus",
+                "data": { "version": 2, "subject": "r4" },
+                "expectedMethod": self.plugin_instance.handle_get_status_command,
+                "expectedArguments": ["r4"],
+                "expectedOutcome": jsonify_mock,
+                "expectedPayload": {"status": True},
+            },
             {
                 "command": "update",
                 "data": { "pin": "r4", "target": True },
@@ -1028,16 +1046,34 @@ def test_on_api_command(self, jsonify_mock):
                 "expectedOutcome": jsonify_mock,
                 "expectedPayload": {"status": "ok", "result": False},
             },
+            {
+                "command": "update",
+                "data": { "v": 2, "subject": "r4", "target": True },
+                "expectedMethod": self.plugin_instance.handle_update_command,
+                "expectedArguments": ["r4", True],
+                "expectedOutcome": jsonify_mock,
+                "expectedPayload": {"status": False},
+            },
             {
                 "command": "cancelTask",
                 "data": { "subject": "r4", "owner": "STARTUP", "target": True },
                 "expectedMethod": self.plugin_instance.handle_cancel_task_command,
                 "expectedArguments": [ "r4", True, "STARTUP" ],
                 "expectedOutcome": jsonify_mock,
                 "expectedPayload": {"status": "ok"},
+            },
+            {
+                "command": "cancelTask",
+                "data": { "v": 2, "subject": "r4", "owner": "STARTUP", "target": True },
+                "expectedMethod": self.plugin_instance.handle_cancel_task_command,
+                "expectedArguments": [ "r4", True, "STARTUP" ],
+                "expectedOutcome": jsonify_mock,
+                "expectedPayload": {"cancelled": True},
             }
         ]
         for case in cases:
+            case["expectedMethod"].reset_mock()
+            case["expectedOutcome"].reset_mock()
             self.plugin_instance.on_api_command(case["command"], case["data"])
             case["expectedMethod"].assert_called_with(*case["expectedArguments"])
             case["expectedOutcome"].assert_called_with(case["expectedPayload"])
@@ -1047,27 +1083,70 @@ def test_on_api_command(self, jsonify_mock):
     def test_on_api_command__update_exceptions(self, jsonify_mock, abort_mock):
         # Should respond with a faulty HTTP code or status error when handler raises
         cases = [
-            { "status": 403, "expectedMethod": abort_mock, "expectedArgument": 403 },
-            { "status": 400, "expectedMethod": jsonify_mock, "expectedArgument": {"status": "error"} }
+            {
+                "payload": { "pin": "r4" },
+                "status": 403,
+                "expectedMethod": abort_mock,
+                "expectedArgument": 403
+            },
+            {
+                "payload": { "version": 2, "subject": "r4" },
+                "status": 403,
+                "expectedMethod": abort_mock,
+                "expectedArgument": 403
+            },
+            {
+                "payload": { "pin": "r4" },
+                "status": 400,
+                "expectedMethod": jsonify_mock,
+                "expectedArgument": { "status": "error"}
+            },
+            {
+                "payload": { "v": 2, "subject": "r4" },
+                "status": 400,
+                "expectedMethod": abort_mock,
+                "expectedArgument": 400
+            }
         ]
         for case in cases:
+            case["expectedMethod"].reset_mock()
             self.plugin_instance.handle_update_command = Mock(side_effect=HandlingException(case["status"]))
-            self.plugin_instance.on_api_command("update", {"pin": "r4"})
+            self.plugin_instance.on_api_command("update", case["payload"])
             case["expectedMethod"].assert_called_with(case["expectedArgument"])
 
+    @patch("flask.abort")
     @patch("flask.jsonify")
-    def test_om_api_command__get_status_exception(self, jsonify_mock):
+    def test_om_api_command__get_status_exception(self, jsonify_mock, abort_mock):
         # Should respond with status false when handler raises
+        cases = [
+            {
+                "payload": { "pin": "r4" },
+                "expectedMethod": jsonify_mock,
+                "expectedArgument": {"status": False}
+            },
+            {
+                "payload": { "v": 2, "subject": "r4" },
+                "expectedMethod": abort_mock,
+                "expectedArgument": 400
+            }
+        ]
         self.plugin_instance.handle_get_status_command = Mock(side_effect=HandlingException(400))
-        self.plugin_instance.on_api_command("getStatus", {"pin": "r4"})
-        jsonify_mock.assert_called_with({"status": False})
+        for case in cases:
+            case["expectedMethod"].reset_mock()
+            self.plugin_instance.on_api_command("getStatus", case["payload"])
+            case["expectedMethod"].assert_called_with(case["expectedArgument"])
 
     @patch("flask.abort")
     def test_on_api_command__missing_parameters(self, abort_mock):
-        commands = ["getStatus", "update"]
-        for command in commands:
-            self.plugin_instance.on_api_command(command, {})
-            abort_mock.assert_called_with(400, description="Parameter pin is missing")
+        cases = [
+            { "command": "getStatus", "version": 1, "missing": "pin" },
+            { "command": "update", "version": 1, "missing": "pin" },
+            { "command": "getStatus", "version": 2, "missing": "subject" },
+            { "command": "update", "version": 2, "missing": "subject" }
+        ]
+        for case in cases:
+            self.plugin_instance.on_api_command(case["command"], { "version": case["version"] })
+            abort_mock.assert_called_with(400, description=f"Parameter {case['missing']} is missing")
 
     @patch("flask.abort")
     def test_on_api_command__unknown(self, abort_mock):

ts/helpers/actions.spec.ts

@@ -30,7 +30,8 @@ describe("Actions", () => {
         upcoming: null,
       });
       expect(apiMock).toHaveBeenCalledWith("octorelay", "update", {
-        pin: "r1",
+        v: 2,
+        subject: "r1",
       });
     });
 
@@ -56,7 +57,8 @@ describe("Actions", () => {
       elementMock.modal.mockClear();
       elementMock.on.mock.calls[1][1](); // confirm
       expect(apiMock).toHaveBeenCalledWith("octorelay", "update", {
-        pin: "r2",
+        v: 2,
+        subject: "r2",
       });
       expect(elementMock.modal).toHaveBeenCalledWith("hide");
     });
@@ -70,6 +72,7 @@ describe("Actions", () => {
         deadline: 86400,
       });
       expect(apiMock).toHaveBeenCalledWith("octorelay", "cancelTask", {
+        v: 2,
         owner: "PRINTING_STARTED",
         subject: "r2",
         target: true,

ts/helpers/actions.ts

@@ -3,7 +3,7 @@ import { ownCode } from "./const";
 
 export const toggleRelay = (key: string, relay: Relay) => {
   const command = () =>
-    OctoPrint.simpleApiCommand(ownCode, "update", { pin: key });
+    OctoPrint.simpleApiCommand(ownCode, "update", { v: 2, subject: key });
   if (!relay.confirm_off) {
     return command();
   }
@@ -28,6 +28,7 @@ export const toggleRelay = (key: string, relay: Relay) => {
 
 export const cancelTask = (key: string, { owner, target }: Task) =>
   OctoPrint.simpleApiCommand(ownCode, "cancelTask", {
+    v: 2,
     subject: key,
     owner,
     target,