diff --git a/CHANGELOG.md b/CHANGELOG.md index 17ddaacda..215e2e8a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Ongoing + +- Fix setting changes after a timeout or a 504 error. The retry sent a read instead of the change, so the change was reported as done even when the gateway never got it. + ## v1.14.8 - Fix for issue [#933](https://github.com/plugwise/python-plugwise/issues/933) via PR [#934](https://github.com/plugwise/python-plugwise/pull/934) diff --git a/plugwise/smilecomm.py b/plugwise/smilecomm.py index e770ec7d2..471b7b7eb 100644 --- a/plugwise/smilecomm.py +++ b/plugwise/smilecomm.py @@ -97,7 +97,7 @@ async def _request( exc, ) raise ConnectionFailedError from exc - return await self._request(command, retry - 1) + return await self._request(command, retry - 1, method=method, data=data) if resp.status == 504: if retry < 1: @@ -108,7 +108,7 @@ async def _request( "504 Gateway Timeout", ) raise ConnectionFailedError - return await self._request(command, retry - 1) + return await self._request(command, retry - 1, method=method, data=data) return await self._request_validate(resp, method) diff --git a/tests/test_generic.py b/tests/test_generic.py index 4a6af75c0..2cd1ad451 100644 --- a/tests/test_generic.py +++ b/tests/test_generic.py @@ -1,10 +1,13 @@ """Test Plugwise module generic functionality.""" +import asyncio from unittest.mock import patch +from plugwise.smilecomm import SmileComm import pytest import aiohttp +from aiohttp import test_utils, web from .test_init import _LOGGER, TestPlugwise, pw_exceptions @@ -66,3 +69,92 @@ async def test_connect_timeout(self, timeout_test): except pw_exceptions.PlugwiseException: setup_result = True assert setup_result + + @staticmethod + async def _start_rules_server(calls, put_handler): + """Start a mock Smile that records the method and body of each request.""" + + async def put(request): + calls.append(("PUT", await request.text())) + return await put_handler(request, calls) + + async def get(request): + calls.append(("GET", await request.text())) + return web.Response(text="") + + app = web.Application() + if put_handler is not None: + app.router.add_route("PUT", "/core/rules{tail:.*}", put) + app.router.add_route("GET", "/core/rules{tail:.*}", get) + server = test_utils.TestServer( + app, port=test_utils.unused_port(), scheme="http", host="127.0.0.1" + ) + await server.start_server() + return server + + @pytest.mark.asyncio + async def test_put_retried_as_put_after_504(self): + """Test that a retried PUT keeps its method and body.""" + calls = [] + + async def timeout_once(request, calls): + if len(calls) == 1: + raise web.HTTPGatewayTimeout() + raise web.HTTPAccepted(text="") + + server = await self._start_rules_server(calls, timeout_once) + comm = SmileComm( + server.host, "pw", server.port, 10, username="smile", websession=None + ) + data = "" + try: + # pylint: disable-next=protected-access + await comm._request("/core/rules;id=abc", method="put", data=data) + finally: + await comm.close_connection() + await server.close() + assert calls == [("PUT", data), ("PUT", data)] + + @pytest.mark.asyncio + async def test_put_dropped_raises(self): + """Test that a PUT failing on every retry raises instead of turning into a GET.""" + calls = [] + + async def drop_connection(request, calls): + request.transport.close() + await asyncio.sleep(0.1) + + server = await self._start_rules_server(calls, drop_connection) + comm = SmileComm( + server.host, "pw", server.port, 10, username="smile", websession=None + ) + try: + with pytest.raises(pw_exceptions.ConnectionFailedError): + # pylint: disable-next=protected-access + await comm._request( + "/core/rules;id=abc", method="put", data="" + ) + finally: + await comm.close_connection() + await server.close() + assert calls + assert all(call == ("PUT", "") for call in calls) + + @pytest.mark.asyncio + async def test_put_not_allowed_raises(self): + """Test that a 405 on PUT raises.""" + calls = [] + server = await self._start_rules_server(calls, None) + comm = SmileComm( + server.host, "pw", server.port, 10, username="smile", websession=None + ) + try: + with pytest.raises(pw_exceptions.ConnectionFailedError): + # pylint: disable-next=protected-access + await comm._request( + "/core/rules;id=abc", method="put", data="" + ) + finally: + await comm.close_connection() + await server.close() + assert not calls