diff --git a/.github/workflows/validate.yaml b/.github/workflows/validate.yaml index 20dc7526..07b5dce2 100644 --- a/.github/workflows/validate.yaml +++ b/.github/workflows/validate.yaml @@ -104,7 +104,11 @@ jobs: -r requirements.txt \ -r requirements_test.txt - - name: Tests suite + - name: Tests suite (HA integration — harness) run: | - pytest --cov=./custom_components/zaptec --cov-branch + pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch + + - name: Tests suite (API client — plain pytest, no harness) + run: | + pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 882bd3e2..087460f1 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -163,6 +163,24 @@ To run tests and check test coverage: report, or enable the "Coverage Gutters" extension to view the coverage directly in VSCode. +The suite runs as **two pytest invocations**, and `./scripts/test` runs both: + +- **HA-integration tests** (`tests/test_*.py`) run under the + `pytest-homeassistant-custom-component` harness, which autoloads on Linux. + Run directly with: + `pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch` +- **API-client tests** (`tests/zaptec/*`) test the vendored `zaptec/` client, + which is destined to become a standalone PyPI library (issue #257) and has no + Home Assistant dependency. They run as plain pytest with the harness disabled + (the harness blocks non-localhost sockets, which would break their live + `api.zaptec.com/api/constants` call): + `pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append` + +Because the harness (and its socket block) is process-wide, a bare `pytest` +is not the entry point — use `./scripts/test` or the two commands above. The +HA-integration tests require Linux; run them in the Dev Container (native +Windows is not supported for that half). `tests/zaptec/*` run anywhere. + HA requires [95% coverage](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/test-coverage/) for all core integration modules, and while HACS doesn't have the same requirements, reaching this level is still a goal for this integration. diff --git a/README.md b/README.md index 98b20e1a..d503ad7d 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,21 @@ Confirmed to work with Zaptec products * Disable [Zaptec Sense](https://help.zaptec.com/hc/en-GB/article/how-to-manage-zaptec-sense-in-the-zaptec-portal) (aka APM/Automatic Power Management). * Disable [stand-alone mode](https://help.zaptec.com/hc/en-GB/article/use-stand-alone-mode-for-troubleshooting-and-unstable-internet). +> [!NOTE] +> If the configured account only has the _User_ role on an installation, the +> integration still sets up and works normally for everything that doesn't +> need Owner/Service access (see [Known issues](#known-issues)). Trying to +> change the available current, the 3-to-1 phase switch current, a charger's +> settings, or send a charger command (e.g. restart) will fail with a clear +> error instead of a raw HTTP 403, and Home Assistant will show +> a persistent notice under *Settings → Repairs* naming the affected +> installation and the role it needs. Roles are granted per object, so an +> account can be _Owner_ on an installation and _User_ on one of its chargers: +> in that case the notice names the restricted chargers instead, one notice per +> installation. If this is expected for your setup, you can dismiss it with +> "Ignore" in the Repairs list — it won't come back unless the account's role +> actually changes. + # Known issues * Sending a _"deauthorize_and_stop"_ command will give an error. This is due to @@ -61,6 +76,14 @@ Confirmed to work with Zaptec products a workaround is to use the more frequently updated _Session total charge_ entity instead. This reduces the delay-issue, but has a separate drawback where a restart of Home Assistant during a charging session can give a fake spike in the logged consumption that needs to be manually edited using "Adjust sum" in the Statistics tab of the Developer tools dashboard. +* A Zaptec Portal user with only the _User_ role (no _Owner_ or _Service_) has + significantly reduced access: the installation hierarchy, firmware info, + individual charger detail/state, and the live update stream are all blocked + by the Zaptec API itself, and this integration additionally blocks changing + installation-level current limits, charger settings, and charger commands + (see [Requirements](#requirements)). Online/offline status and operating + mode keep working, since those are + included in the basic charger list the API returns regardless of role. ## Features missing from the API diff --git a/custom_components/zaptec/coordinator.py b/custom_components/zaptec/coordinator.py index cce4419a..7a8686ef 100644 --- a/custom_components/zaptec/coordinator.py +++ b/custom_components/zaptec/coordinator.py @@ -9,6 +9,7 @@ from typing import TYPE_CHECKING from homeassistant.core import HomeAssistant +from homeassistant.helpers import issue_registry as ir from homeassistant.helpers.debounce import Debouncer from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed @@ -18,7 +19,7 @@ ZAPTEC_POLL_CHARGER_TRIGGER_DELAYS, ZAPTEC_POLL_INSTALLATION_TRIGGER_DELAYS, ) -from .zaptec import Charger, Installation, Zaptec, ZaptecApiError, ZaptecBase +from .zaptec import Charger, Installation, Zaptec, ZaptecApiError, ZaptecBase, has_write_role if TYPE_CHECKING: from .manager import ZaptecConfigEntry, ZaptecManager @@ -121,6 +122,85 @@ async def _async_update_data(self) -> None: _LOGGER.exception("Fetching data failed") raise UpdateFailed(err) from err + if isinstance(self.options.zaptec_object, Installation): + self._check_installation_role(self.options.zaptec_object) + + def _check_installation_role(self, installation: Installation) -> None: + """Create or clear the Repair issues for insufficient write access. + + `installation/update` requires the Owner or Service role + (https://docs.zaptec.com/reference/api_installation_id_update_post), + as do `chargers/{id}/update` and `chargers/{id}/SendCommand/{id}`. + Roles are per object, so an account can be Owner on the installation + and User on one of its chargers; the two issues are mutually + exclusive, at most one per installation. If CurrentUserRoles hasn't + been observed yet, leave any existing issue alone rather than guessing. + + Deliberately calling async_create_issue() again every poll (rather + than only on the first observation) is safe and intentional: HA's + issue registry replaces the existing IssueEntry in place and does not + touch dismissed_version, so a user who has clicked "Ignore" on this + issue in Settings > Repairs stays ignored across every subsequent + poll as long as the role doesn't change. Only deleting the issue + (role becomes sufficient) and later recreating it (role becomes + insufficient again) resets that dismissal -- which is intentional, + since a real role change deserves fresh attention. + """ + roles = installation.get("current_user_roles") + if roles is None: + return + + name = str(installation.get("name", installation.qual_id)) + issue_id = f"insufficient_role_{installation.id}" + charger_issue_id = f"insufficient_charger_role_{installation.id}" + + if has_write_role(roles) is False: + # The installation-level warning covers the account's access to this + # installation; naming individual chargers on top of it would only + # repeat the same remedy. + ir.async_delete_issue(self.hass, DOMAIN, charger_issue_id) + self._create_role_issue( + issue_id, + "insufficient_role", + {"installation_name": name, "role": roles or "None"}, + ) + return + + ir.async_delete_issue(self.hass, DOMAIN, issue_id) + + # Chargers carry their own roles, populated by Charger.poll_info(); one + # that hasn't been polled yet reports None and is left out rather than + # assumed restricted. + restricted = sorted( + str(charger.get("name", charger.qual_id)) + for charger in installation.chargers + if has_write_role(charger.get("current_user_roles")) is False + ) + if not restricted: + ir.async_delete_issue(self.hass, DOMAIN, charger_issue_id) + return + + self._create_role_issue( + charger_issue_id, + "insufficient_charger_role", + {"installation_name": name, "chargers": ", ".join(restricted)}, + ) + + def _create_role_issue( + self, issue_id: str, translation_key: str, placeholders: dict[str, str] + ) -> None: + """Raise a non-fixable warning pointing at the Zaptec Portal.""" + ir.async_create_issue( + self.hass, + DOMAIN, + issue_id, + is_fixable=False, + severity=ir.IssueSeverity.WARNING, + translation_key=translation_key, + translation_placeholders=placeholders, + learn_more_url="https://portal.zaptec.com/", + ) + async def _trigger_poll(self, zaptec_obj: ZaptecBase) -> None: """Trigger a poll update sequence for the given object. diff --git a/custom_components/zaptec/translations/en.json b/custom_components/zaptec/translations/en.json index 90ca1066..69e9a492 100644 --- a/custom_components/zaptec/translations/en.json +++ b/custom_components/zaptec/translations/en.json @@ -192,5 +192,15 @@ "name": "Firmware update" } } + }, + "issues": { + "insufficient_charger_role": { + "title": "Limited access to chargers in {installation_name}", + "description": "The Zaptec account used by this integration does not have the Owner or Service role on the following charger(s) in installation \"{installation_name}\": {chargers}. Changing their settings or sending them commands, such as restart, requires the Owner or Service role.\n\nTo enable these controls, grant Owner or Service access for those chargers to this account in the Zaptec Portal." + }, + "insufficient_role": { + "title": "Limited access to {installation_name}", + "description": "The Zaptec account used by this integration only has the following role(s) on installation \"{installation_name}\": {role}. Changing the available current or the 3-to-1 phase switch current requires the Owner or Service role.\n\nTo enable these controls, grant Owner or Service access for this installation to this account in the Zaptec Portal." + } } } \ No newline at end of file diff --git a/custom_components/zaptec/translations/nb.json b/custom_components/zaptec/translations/nb.json index eff026ff..c969fed4 100644 --- a/custom_components/zaptec/translations/nb.json +++ b/custom_components/zaptec/translations/nb.json @@ -192,5 +192,15 @@ "name": "Fastvareoppdatering" } } + }, + "issues": { + "insufficient_charger_role": { + "title": "Begrenset tilgang til ladere i {installation_name}", + "description": "Zaptec-kontoen som brukes av denne integrasjonen har ikke Owner- eller Service-rollen på følgende lader(e) i installasjonen «{installation_name}»: {chargers}. Å endre innstillingene deres eller sende dem kommandoer, som omstart, krever Owner- eller Service-rollen.\n\nFor å aktivere disse kontrollene, gi Owner- eller Service-tilgang for disse laderne til denne kontoen i Zaptec Portal." + }, + "insufficient_role": { + "title": "Begrenset tilgang til {installation_name}", + "description": "Zaptec-kontoen som brukes av denne integrasjonen har kun følgende rolle(r) på installasjonen «{installation_name}»: {role}. Å endre tilgjengelig strøm eller 3-til-1-fase bytteterskel krever Owner- eller Service-rollen.\n\nFor å aktivere disse kontrollene, gi Owner- eller Service-tilgang for denne installasjonen til denne kontoen i Zaptec Portal." + } } } \ No newline at end of file diff --git a/custom_components/zaptec/translations/nl.json b/custom_components/zaptec/translations/nl.json index 0b72bc59..8de6f58c 100644 --- a/custom_components/zaptec/translations/nl.json +++ b/custom_components/zaptec/translations/nl.json @@ -192,5 +192,15 @@ "name": "Firmware" } } + }, + "issues": { + "insufficient_charger_role": { + "title": "Beperkte toegang tot laders in {installation_name}", + "description": "Het Zaptec-account dat door deze integratie wordt gebruikt heeft niet de rol Owner of Service op de volgende lader(s) in installatie \"{installation_name}\": {chargers}. Het wijzigen van hun instellingen of het versturen van opdrachten, zoals herstarten, vereist de rol Owner of Service.\n\nOm deze bedieningselementen in te schakelen, geef Owner- of Service-toegang voor die laders aan dit account in het Zaptec Portal." + }, + "insufficient_role": { + "title": "Beperkte toegang tot {installation_name}", + "description": "Het Zaptec-account dat door deze integratie wordt gebruikt heeft alleen de volgende rol(len) op installatie \"{installation_name}\": {role}. Het wijzigen van de beschikbare stroom of de 3-naar-1-fase omschakeldrempel vereist de rol Owner of Service.\n\nOm deze bedieningselementen in te schakelen, geef Owner- of Service-toegang voor deze installatie aan dit account in het Zaptec Portal." + } } } \ No newline at end of file diff --git a/custom_components/zaptec/translations/nn.json b/custom_components/zaptec/translations/nn.json index 15387942..a9cb062e 100644 --- a/custom_components/zaptec/translations/nn.json +++ b/custom_components/zaptec/translations/nn.json @@ -192,5 +192,15 @@ "name": "Fastvareoppdatering" } } + }, + "issues": { + "insufficient_charger_role": { + "title": "Avgrensa tilgang til ladarar i {installation_name}", + "description": "Zaptec-kontoen som blir brukt av denne integrasjonen har ikkje Owner- eller Service-rolla på følgjande ladar(ar) i installasjonen «{installation_name}»: {chargers}. Å endre innstillingane deira eller sende dei kommandoar, som omstart, krev Owner- eller Service-rolla.\n\nFor å aktivere desse kontrollane, gi Owner- eller Service-tilgang for desse ladarane til denne kontoen i Zaptec Portal." + }, + "insufficient_role": { + "title": "Avgrensa tilgang til {installation_name}", + "description": "Zaptec-kontoen som blir brukt av denne integrasjonen har berre følgjande rolle(r) på installasjonen «{installation_name}»: {role}. Å endre tilgjengeleg straum eller 3-til-1-fase bytteterskel krev Owner- eller Service-rolla.\n\nFor å aktivere desse kontrollane, gi Owner- eller Service-tilgang for denne installasjonen til denne kontoen i Zaptec Portal." + } } } diff --git a/custom_components/zaptec/translations/pl.json b/custom_components/zaptec/translations/pl.json index 545d8908..d622b2bd 100644 --- a/custom_components/zaptec/translations/pl.json +++ b/custom_components/zaptec/translations/pl.json @@ -192,5 +192,15 @@ "name": "Aktualizacja oprogramowania" } } + }, + "issues": { + "insufficient_charger_role": { + "title": "Ograniczony dostęp do ładowarek w {installation_name}", + "description": "Konto Zaptec używane przez tę integrację nie ma roli Owner ani Service dla następujących ładowarek w instalacji „{installation_name}”: {chargers}. Zmiana ich ustawień lub wysyłanie do nich poleceń, takich jak ponowne uruchomienie, wymaga roli Owner lub Service.\n\nAby włączyć te funkcje, nadaj temu kontu dostęp Owner lub Service dla tych ładowarek w portalu Zaptec." + }, + "insufficient_role": { + "title": "Ograniczony dostęp do {installation_name}", + "description": "Konto Zaptec używane przez tę integrację ma tylko następującą rolę (role) w instalacji „{installation_name}”: {role}. Zmiana dostępnego prądu lub progu przełączania 3-fazowego na 1-fazowe wymaga roli Owner lub Service.\n\nAby włączyć te funkcje, nadaj temu kontu dostęp Owner lub Service dla tej instalacji w portalu Zaptec." + } } } \ No newline at end of file diff --git a/custom_components/zaptec/translations/sv.json b/custom_components/zaptec/translations/sv.json index a7288fc5..c0407c78 100644 --- a/custom_components/zaptec/translations/sv.json +++ b/custom_components/zaptec/translations/sv.json @@ -192,5 +192,15 @@ "name": "Uppdatera mjukvara" } } + }, + "issues": { + "insufficient_charger_role": { + "title": "Begränsad åtkomst till laddare i {installation_name}", + "description": "Zaptec-kontot som används av den här integrationen har inte rollen Owner eller Service på följande laddare i installationen \"{installation_name}\": {chargers}. Att ändra deras inställningar eller skicka kommandon till dem, som omstart, kräver rollen Owner eller Service.\n\nFör att aktivera dessa kontroller, ge Owner- eller Service-åtkomst för dessa laddare till det här kontot i Zaptec Portal." + }, + "insufficient_role": { + "title": "Begränsad åtkomst till {installation_name}", + "description": "Zaptec-kontot som används av den här integrationen har endast följande roll(er) på installationen \"{installation_name}\": {role}. Att ändra tillgänglig ström eller 3-till-1-fas växlingströskeln kräver rollen Owner eller Service.\n\nFör att aktivera dessa kontroller, ge Owner- eller Service-åtkomst för den här installationen till det här kontot i Zaptec Portal." + } } } diff --git a/custom_components/zaptec/zaptec/__init__.py b/custom_components/zaptec/zaptec/__init__.py index a71ebc9a..954da3c2 100644 --- a/custom_components/zaptec/zaptec/__init__.py +++ b/custom_components/zaptec/zaptec/__init__.py @@ -2,10 +2,11 @@ from __future__ import annotations -from .api import Charger, Installation, Zaptec, ZaptecBase +from .api import Charger, Installation, Zaptec, ZaptecBase, has_write_role from .const import MISSING, RETRYABLE_HTTP_STATUSES, Missing from .exceptions import ( AuthenticationError, + InsufficientRoleError, RequestConnectionError, RequestDataError, RequestError, @@ -24,6 +25,7 @@ "AuthenticationError", "Charger", "Installation", + "InsufficientRoleError", "Missing", "Redactor", "RequestConnectionError", @@ -35,4 +37,5 @@ "ZaptecApiError", "ZaptecBase", "get_ocmf_max_reader_value", + "has_write_role", ] diff --git a/custom_components/zaptec/zaptec/api.py b/custom_components/zaptec/zaptec/api.py index cd818dd4..bfd00fe8 100644 --- a/custom_components/zaptec/zaptec/api.py +++ b/custom_components/zaptec/zaptec/api.py @@ -41,6 +41,7 @@ ) from .exceptions import ( AuthenticationError, + InsufficientRoleError, RequestConnectionError, RequestDataError, RequestError, @@ -65,6 +66,18 @@ StreamCallback = Callable[[dict], Awaitable[None]] +def has_write_role(roles: str | None) -> bool | None: + """Whether `CurrentUserRoles` permits writes, or None if not observed yet. + + The Owner and Service (reported by the API as Maintainer) roles permit + writes; a not-yet-observed role is reported as None so callers can choose + between blocking and letting the API answer with its own 403. + """ + if roles is None: + return None + return "Owner" in roles or "Maintainer" in roles + + class TLogExc(Protocol): """Protocol for logging exceptions.""" @@ -216,6 +229,30 @@ def state_to_attrs( out[kv] = value return out + def _require_write_role(self, action: str) -> None: + """Raise InsufficientRoleError if the current user lacks write access. + + `installation/update`, `chargers/{id}/update`, and + `chargers/{id}/SendCommand/{id}` all require the Owner or Service + (Maintainer) role (confirmed individually via docs.zaptec.com/reference + for each of the three endpoints). If CurrentUserRoles hasn't been + observed yet, let the request proceed and rely on the API's own 403 + response instead of guessing. + + `chargers/{id}/authorizecharge` and `chargers/{id}/localSettings` are + deliberately not gated by any caller of this method -- they aren't + documented anywhere, so there's no evidence for what role (if any) + they require. + """ + roles = self.get("current_user_roles") + if has_write_role(roles) is not False: + return + raise InsufficientRoleError( + f"{action} requires the Owner or Service role on {self.qual_id} " + f"(current role: {roles or 'None'}). Grant Owner or Service access " + "to this Zaptec object in the Zaptec Portal to enable this." + ) + class Installation(ZaptecBase): """Represents an installation.""" @@ -536,6 +573,8 @@ async def set_limit_current(self, **kwargs: Any) -> Any: Use availableCurrent for setting all phases at once. Use availableCurrentPhase* to set each phase individually. """ + self._require_write_role("Setting the installation current limit") + has_availablecurrent = kwargs.get("availableCurrent") is not None has_availablecurrentphases = [ kwargs.get(k) is not None @@ -579,6 +618,7 @@ async def set_limit_current(self, **kwargs: Any) -> Any: async def set_three_to_one_phase_switch_current(self, current: float) -> Any: """Set the 3 to 1-phase switch current.""" + self._require_write_role("Setting the 3-to-1 phase switch current") if not (0 <= current <= DEFAULT_MAX_CURRENT): raise ValueError(f"Current must be between 0 and {DEFAULT_MAX_CURRENT:.0f} amps") return await self.zaptec.request( @@ -713,6 +753,8 @@ async def command(self, command: str | int | CommandType) -> Any: # Check that we can run the command at this time self.is_command_valid(command, raise_value_error_if_invalid=True) + self._require_write_role(f"Sending the {command} command") + _LOGGER.debug("Command %s (%s)", command, cmdid) return await self.zaptec.request(f"chargers/{self.id}/SendCommand/{cmdid}", method="post") @@ -751,6 +793,8 @@ def is_command_valid(self, command: str, raise_value_error_if_invalid: bool = Fa async def set_settings(self, settings: dict[str, Any]) -> Any: """Set settings on the charger.""" + self._require_write_role("Setting charger parameters") + if any(key not in ZCONST.update_params for key in settings): raise ValueError(f"Unknown setting '{settings}'") diff --git a/custom_components/zaptec/zaptec/exceptions.py b/custom_components/zaptec/zaptec/exceptions.py index 75f6a96a..342d9921 100644 --- a/custom_components/zaptec/zaptec/exceptions.py +++ b/custom_components/zaptec/zaptec/exceptions.py @@ -9,6 +9,10 @@ class AuthenticationError(ZaptecApiError): """Authenatication failed.""" +class InsufficientRoleError(ZaptecApiError): + """The current Zaptec user's role does not permit this action.""" + + class RequestError(ZaptecApiError): """Failed to get the results from the API.""" diff --git a/custom_components/zaptec/zaptec/zconst.py b/custom_components/zaptec/zaptec/zconst.py index dc8e79a5..ccc46c04 100644 --- a/custom_components/zaptec/zaptec/zconst.py +++ b/custom_components/zaptec/zaptec/zconst.py @@ -199,8 +199,11 @@ def type_user_roles(self, val: int) -> str: if val == 0: return "None" # v != 0 is needed to avoid the 0 == 0 case leading to None always being included - roles = {k for k, v in self.get("UserRoles", {}).items() if v != 0 and (v & val) == v} - return ", ".join(roles) + # Ordered by bit value, so the rendered string is stable between runs + roles = sorted( + (v, k) for k, v in self.get("UserRoles", {}).items() if v != 0 and (v & val) == v + ) + return ", ".join(role for _, role in roles) ZCONST = ZConst() diff --git a/requirements.txt b/requirements.txt index 1adb323f..be7d8a98 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,5 +6,5 @@ ruff==0.15.22 # Copy from manifest.json to get this into the dev container # without needing to start HA azure-servicebus==7.14.3 -pydantic==2.13.1 +pydantic>=2.11.7,<2.14 aiolimiter==1.2.1 diff --git a/requirements_test.txt b/requirements_test.txt index cf21f7bc..be62f2c4 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,4 +1,6 @@ pytest pytest-asyncio pytest-mock -pytest-cov \ No newline at end of file +pytest-cov +pytest-homeassistant-custom-component==0.13.324; python_version >= "3.14" +pytest-homeassistant-custom-component==0.13.316; python_version < "3.14" diff --git a/scripts/test b/scripts/test index b3d15862..58923e90 100755 --- a/scripts/test +++ b/scripts/test @@ -5,8 +5,14 @@ set -e if [ "$1" == "--skip-api" ]; then export SKIP_ZAPTEC_API_TEST="true" fi + +# HA-integration tests run under the pytest-hacc harness (autoloads on Linux). +# API-client tests (tests/zaptec/*) run as plain pytest with the harness +# disabled, so their live constants call is not socket-blocked. Coverage +# from both is combined via --cov-append. # run tests with -s to display printouts and --log-cli-level to get logger output -pytest --cov=./custom_components/zaptec --cov-branch --log-cli-level=INFO -s +pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch --log-cli-level=INFO -s +pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append --log-cli-level=INFO -s # generate coverage report in html and xml coverage html diff --git a/tests/conftest.py b/tests/conftest.py index ba07a5c5..a65bdc79 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,11 +1,20 @@ """Zaptec testing configuration file.""" -import asyncio +from collections.abc import Callable, Iterable import os +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch +from homeassistant.const import CONF_PASSWORD, CONF_USERNAME +from homeassistant.core import HomeAssistant import pytest +from pytest_homeassistant_custom_component.common import MockConfigEntry +from custom_components.zaptec.const import DOMAIN +from custom_components.zaptec.manager import ZaptecManager +from custom_components.zaptec.zaptec import Charger, Installation from custom_components.zaptec.zaptec.api import Zaptec +from custom_components.zaptec.zaptec.utils import to_under @pytest.fixture(scope="session") @@ -24,12 +33,7 @@ def skip_if_user_disabled_api_tests() -> None: @pytest.fixture(scope="session") def zaptec_username(skip_if_user_disabled_api_tests, skip_if_in_github_actions) -> str: # noqa: ANN001 (the inputs are purely to create dependencies to the env-flags above) - """ - Get the zaptec username stored in env. - - Any test relying on this fixture will be skipped if the test is running - in Gihub Actions, or the user has disabled tests requiring API login. - """ + """Get the zaptec username from env, skipping if API-login tests are disabled.""" username = os.environ.get("ZAPTEC_USERNAME") assert username, ( "Missing username, either set it with \"export ZAPTEC_USERNAME='username'\" " @@ -40,12 +44,7 @@ def zaptec_username(skip_if_user_disabled_api_tests, skip_if_in_github_actions) @pytest.fixture(scope="session") def zaptec_password(skip_if_user_disabled_api_tests, skip_if_in_github_actions) -> str: # noqa: ANN001 - """ - Get the zaptec password stored in env. - - Any test relying on this fixture will be skipped if the test is running - in Gihub Actions, or the user has disabled tests requiring API login. - """ + """Get the zaptec password from env, skipping if API-login tests are disabled.""" password = os.environ.get("ZAPTEC_PASSWORD") assert password, ( "Missing password, either set it with \"export ZAPTEC_PASSWORD='password'\" " @@ -54,14 +53,129 @@ def zaptec_password(skip_if_user_disabled_api_tests, skip_if_in_github_actions) return password -@pytest.fixture(scope="session") -def zaptec_constants() -> dict: - """Get latest constants from Zaptec API.""" +def _backed_get(data: dict[str, Any]) -> Callable[..., Any]: + """Return a `.get()` implementation backed by `data`. + + Mirrors `ZaptecBase.__getitem__`'s key normalization (`to_under`) and + `Mapping.get`'s `None` default, which production code relies on (e.g. + `coordinator.py`'s `installation.get("current_user_roles")`). + """ + + def _get(key: str, default: Any = None) -> Any: + return data.get(to_under(key), default) + + return _get + + +def reseed(obj: MagicMock, data: dict[str, Any]) -> None: + """Replace the backing data of a double built by `make_charger`/`make_installation`. - async def get_zaptec_constants() -> dict: - async with Zaptec("N/A", "N/A") as zaptec: - # the constants API endpoint does not require login - const: dict = await zaptec.request("constants") - return const + Pass `INSTALLATION_DATA | {...}` / `CHARGER_DATA | {...}` rather than a bare + dict, so a test keeps the fixture's seeded keys instead of stripping them. + """ + obj.get.side_effect = _backed_get(data) + + +INSTALLATION_DATA = {"id": "inst-mock-1", "name": "Mock Home"} + +CHARGER_DATA = { + "id": "chg-mock-1", + "name": "Mock Charger", + # Keys read by entities under test; extend as needed for coverage. + "operating_mode": "Connected", + "charger_operation_mode": "Connected", +} - return asyncio.run(get_zaptec_constants()) + +def make_charger( + data: dict[str, Any], *, installation: MagicMock | None = None, charging: bool = False +) -> MagicMock: + """Build a spec'd Charger double backed by `data`. + + `model` is hardcoded rather than modeling `Charger.model`'s real + `ZCONST.serial_to_model` lookup — a deliberate simplification. + """ + charger = MagicMock(spec=Charger) + charger.id = data["id"] + charger.name = data.get("name", "Mock Charger") + charger.model = "Zaptec Charger" + charger.qual_id = f"Charger[{data['id'][-6:]}]" + charger.get.side_effect = _backed_get(data) + charger.is_charging.return_value = charging + charger.installation = installation + return charger + + +def make_installation(data: dict[str, Any], *, chargers: Iterable[MagicMock] = ()) -> MagicMock: + """Build a spec'd Installation double backed by `data`.""" + install = MagicMock(spec=Installation) + install.id = data["id"] + install.name = data.get("name", "Mock Installation") + install.model = "Zaptec Installation" + install.qual_id = f"Installation[{data['id'][-6:]}]" + install.get.side_effect = _backed_get(data) + install.chargers = list(chargers) + install.stream_main = AsyncMock(return_value=None) + install.stream_close = AsyncMock(return_value=None) + return install + + +@pytest.fixture +def mock_zaptec() -> MagicMock: + """A spec'd Zaptec client seeded with one installation and one charger. + + `__getitem__`/`__iter__`/`__contains__`/`__len__` are wired because `Zaptec` + is itself `Mapping[str, ZaptecBase]` in production, and real code (e.g. + `zaptec[deviceid]` in `__init__.py`/`coordinator.py`) indexes into it directly. + """ + installation = make_installation(dict(INSTALLATION_DATA)) + charger = make_charger(dict(CHARGER_DATA), installation=installation, charging=False) + installation.chargers = [charger] + + objects = {installation.id: installation, charger.id: charger} + + zaptec = MagicMock(spec=Zaptec) + zaptec.__getitem__.side_effect = objects.__getitem__ + zaptec.__iter__.side_effect = lambda: iter(objects) + zaptec.__contains__.side_effect = objects.__contains__ + zaptec.__len__.side_effect = lambda: len(objects) + zaptec.objects.return_value = list(objects.values()) + zaptec.installations = [installation] + zaptec.chargers = [charger] + zaptec.login = AsyncMock(return_value=None) + zaptec.build = AsyncMock(return_value=None) + zaptec.poll = AsyncMock(return_value=None) + zaptec.show_all_updates = False + zaptec.redact = MagicMock() + # Load-bearing, not incidental: __init__.py's startup debug-dump path does + # `message += manager.zaptec.redact.dumps()`, which setup_integration actually + # exercises. An unconfigured MagicMock here would raise TypeError on the +=. + zaptec.redact.dumps.return_value = "" + return zaptec + + +@pytest.fixture +def mock_config_entry() -> MockConfigEntry: + """A MockConfigEntry for the zaptec domain.""" + return MockConfigEntry( + domain=DOMAIN, + title="Mock Zaptec", + data={CONF_USERNAME: "user", CONF_PASSWORD: "pass"}, + entry_id="mock_entry_1", + ) + + +async def setup_integration( + hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_zaptec: MagicMock +) -> ZaptecManager: + """Set the integration up through the real async_setup, with a mocked client. + + Patches `custom_components.zaptec.Zaptec` — where `__init__.py` looks the name + up, per unittest.mock's patch-at-the-lookup rule — not the original definition + in `zaptec/api.py`, which `__init__.py`'s own import wouldn't see patched. + """ + mock_config_entry.add_to_hass(hass) + with patch("custom_components.zaptec.Zaptec", return_value=mock_zaptec): + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + return mock_config_entry.runtime_data diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py new file mode 100644 index 00000000..c6493f07 --- /dev/null +++ b/tests/test_coordinator.py @@ -0,0 +1,395 @@ +"""Behavior tests for ZaptecUpdateCoordinator, driven through the real harness.""" + +from datetime import timedelta +from unittest.mock import AsyncMock, MagicMock + +from homeassistant.core import HomeAssistant +from homeassistant.helpers import issue_registry as ir +import pytest +from pytest_homeassistant_custom_component.common import MockConfigEntry + +from custom_components.zaptec.const import ( + DOMAIN, + ZAPTEC_POLL_INTERVAL_CHARGING, + ZAPTEC_POLL_INTERVAL_IDLE, +) +from custom_components.zaptec.coordinator import ZaptecUpdateCoordinator, ZaptecUpdateOptions +from custom_components.zaptec.zaptec import ZaptecApiError +from tests.conftest import CHARGER_DATA, INSTALLATION_DATA, reseed, setup_integration + + +async def test_successful_poll_marks_last_update_success( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A successful poll leaves every coordinator reporting success.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + for coordinator in manager.all_coordinators: + assert coordinator.last_update_success is True + mock_zaptec.poll.assert_awaited() + + +async def test_poll_failure_sets_update_failed( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A ZaptecApiError during poll flips last_update_success to False.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + head = manager.head_coordinator + + mock_zaptec.poll.side_effect = ZaptecApiError("boom") + await head.async_refresh() + + assert head.last_update_success is False + + +async def test_device_coordinator_switches_interval_when_charging( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A charger's coordinator uses the shorter interval once it reports charging.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + charger_coord = manager.device_coordinators["chg-mock-1"] + idle_interval = charger_coord.update_interval + assert idle_interval == timedelta(seconds=ZAPTEC_POLL_INTERVAL_IDLE) + + # Flip the seeded charger to 'charging' and re-run the update-listener path. + mock_zaptec.chargers[0].is_charging.return_value = True + charger_coord.set_update_interval() + + assert charger_coord.update_interval == timedelta(seconds=ZAPTEC_POLL_INTERVAL_CHARGING) + # Also assert the relation directly, catching e.g. const.py setting + # CHARGING >= IDLE, which the equality asserts alone would miss. + assert charger_coord.update_interval < idle_interval + + +async def test_charging_update_interval_requires_charger_object( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, +) -> None: + """Constructing a coordinator with a charging interval on a non-Charger object errors. + + Skips `setup_integration` to hit the constructor-time guard directly: a + bare `manager=MagicMock()` auto-vivifies `self.zaptec` with no error before + the `isinstance(zaptec_object, Charger)` check runs. + """ + mock_config_entry.add_to_hass(hass) + + with pytest.raises(ValueError, match="Charging update interval requires a Charger object"): + ZaptecUpdateCoordinator( + hass, + entry=mock_config_entry, + manager=MagicMock(), + options=ZaptecUpdateOptions( + name="bad", + update_interval=60, + charging_update_interval=30, + tracked_devices=set(), + poll_args={}, + zaptec_object=object(), # not a Charger instance + ), + ) + + +async def test_trigger_poll_is_noop_without_zaptec_object( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """trigger_poll() on a coordinator with no bound zaptec object does nothing. + + `head_coordinator` is the one coordinator built with `zaptec_object=None` + (device coordinators always get a real Charger/Installation), satisfying + trigger_poll()'s no-op guard. + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + + await manager.head_coordinator.trigger_poll() + + assert manager.head_coordinator._trigger_task is None # noqa: SLF001 + + +async def test_trigger_poll_cancels_in_flight_task_and_reschedules( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A second trigger_poll() call cancels the running poll sequence and starts a new one.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + charger_coord = manager.device_coordinators["chg-mock-1"] + + # Collapse the real delays to zero, keeping real asyncio.sleep(0) checkpoints so + # the eagerly-started background task actually suspends and can be cancelled mid-flight. + monkeypatch.setattr( + "custom_components.zaptec.coordinator.ZAPTEC_POLL_CHARGER_TRIGGER_DELAYS", [0, 0, 0] + ) + + # HA's eager task factory runs the task immediately; it suspends at the + # first sleep(0) checkpoint and is left pending. + await charger_coord.trigger_poll() + first_task = charger_coord._trigger_task # noqa: SLF001 + assert first_task is not None + + # Second call sees the still-pending first task and cancels it before rescheduling. + await charger_coord.trigger_poll() + assert first_task.cancelled() + + second_task = charger_coord._trigger_task # noqa: SLF001 + assert second_task is not None + assert second_task is not first_task + await second_task + await hass.async_block_till_done() + + assert charger_coord._trigger_task is None # noqa: SLF001 + assert charger_coord.last_update_success is True + + +async def test_trigger_poll_triggers_child_charger_coordinators( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Polling an installation also triggers the poll sequence of its tracked chargers. + + Patches `asyncio.sleep` globally, rather than the delays-list constant (as the + cancel/reschedule test does), just to reach the loop's first iteration fast — + installations use their own delay constant this test doesn't care about. + `charger_coord.trigger_poll` is mocked to isolate "parent calls child" from + the child's own trigger_poll logic (covered elsewhere). + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + install_coord = manager.device_coordinators["inst-mock-1"] + charger_coord = manager.device_coordinators["chg-mock-1"] + + monkeypatch.setattr( + "custom_components.zaptec.coordinator.asyncio.sleep", AsyncMock(return_value=None) + ) + charger_coord.trigger_poll = AsyncMock() + + await install_coord.trigger_poll() + task = install_coord._trigger_task # noqa: SLF001 + assert task is not None + await task + await hass.async_block_till_done() + + charger_coord.trigger_poll.assert_awaited_once() + + +# --------------------------------------------------------------------------- +# Insufficient-role Repair issue (#311) +# --------------------------------------------------------------------------- + +ISSUE_ID = f"insufficient_role_{INSTALLATION_DATA['id']}" + + +async def test_insufficient_role_creates_repair_issue( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A User-only installation gets a Repair issue after the first poll.""" + reseed(mock_zaptec.installations[0], INSTALLATION_DATA | {"current_user_roles": "User"}) + + await setup_integration(hass, mock_config_entry, mock_zaptec) + + issue = ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) + assert issue is not None + assert issue.severity is ir.IssueSeverity.WARNING + assert issue.translation_key == "insufficient_role" + assert issue.translation_placeholders == {"installation_name": "Mock Home", "role": "User"} + # A repair flow this integration never registers would break the Repairs dialog. + assert issue.is_fixable is False + assert issue.learn_more_url == "https://portal.zaptec.com/" + + +async def test_repeated_polls_preserve_an_ignored_repair_issue( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """Polling again with an unchanged role keeps the user's "Ignore" dismissal. + + Regression guard for the "don't nag aware users" requirement: a + delete-then-recreate cycle would reset dismissed_version. + """ + installation = mock_zaptec.installations[0] + reseed(installation, INSTALLATION_DATA | {"current_user_roles": "User"}) + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators[INSTALLATION_DATA["id"]] + + ir.async_ignore_issue(hass, DOMAIN, ISSUE_ID, ignore=True) + dismissed_version = ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID).dismissed_version + assert dismissed_version is not None + + await coordinator.async_refresh() + # Vary the role within "still insufficient": the updated placeholder proves the + # check ran again, so the preserved dismissal can't pass by the issue being untouched. + reseed(installation, INSTALLATION_DATA | {"current_user_roles": "User, Guest"}) + await coordinator.async_refresh() + + issue = ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) + assert issue is not None + assert issue.translation_placeholders["role"] == "User, Guest" + assert issue.dismissed_version == dismissed_version + + +async def test_sufficient_role_clears_the_repair_issue( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """Regaining the Owner role removes an existing Repair issue on the next poll.""" + installation = mock_zaptec.installations[0] + reseed(installation, INSTALLATION_DATA | {"current_user_roles": "User"}) + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + assert ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) is not None + + reseed(installation, INSTALLATION_DATA | {"current_user_roles": "Owner"}) + await manager.device_coordinators[INSTALLATION_DATA["id"]].async_refresh() + + assert ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) is None + + +async def test_unknown_role_creates_no_repair_issue( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """No CurrentUserRoles observed yet -> no issue either way.""" + reseed(mock_zaptec.installations[0], INSTALLATION_DATA) + + await setup_integration(hass, mock_config_entry, mock_zaptec) + + assert ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) is None + + +async def test_unknown_role_leaves_an_existing_repair_issue( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A poll that no longer observes CurrentUserRoles must not clear the issue.""" + installation = mock_zaptec.installations[0] + reseed(installation, INSTALLATION_DATA | {"current_user_roles": "User"}) + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + assert ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) is not None + + reseed(installation, INSTALLATION_DATA) + await manager.device_coordinators[INSTALLATION_DATA["id"]].async_refresh() + + assert ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) is not None + + +async def test_charger_coordinator_skips_the_role_check( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """The role check is Installation-scoped: a charger poll never raises an issue.""" + reseed(mock_zaptec.chargers[0], CHARGER_DATA | {"current_user_roles": "User"}) + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + + await manager.device_coordinators[CHARGER_DATA["id"]].async_refresh() + + charger_issue_id = f"insufficient_role_{CHARGER_DATA['id']}" + assert ir.async_get(hass).async_get_issue(DOMAIN, charger_issue_id) is None + + +# --------------------------------------------------------------------------- +# Restricted-charger Repair issue +# --------------------------------------------------------------------------- + +CHARGER_ISSUE_ID = f"insufficient_charger_role_{INSTALLATION_DATA['id']}" + + +async def test_restricted_charger_creates_a_charger_repair_issue( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """An Owner installation holding a User-only charger warns about that charger.""" + reseed(mock_zaptec.installations[0], INSTALLATION_DATA | {"current_user_roles": "Owner"}) + reseed(mock_zaptec.chargers[0], CHARGER_DATA | {"current_user_roles": "User"}) + + await setup_integration(hass, mock_config_entry, mock_zaptec) + + issue = ir.async_get(hass).async_get_issue(DOMAIN, CHARGER_ISSUE_ID) + assert issue is not None + assert issue.translation_key == "insufficient_charger_role" + assert issue.translation_placeholders == { + "installation_name": "Mock Home", + "chargers": "Mock Charger", + } + assert ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) is None + + +async def test_insufficient_installation_role_supersedes_the_charger_issue( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """The two issues are mutually exclusive; the installation one wins.""" + installation = mock_zaptec.installations[0] + reseed(installation, INSTALLATION_DATA | {"current_user_roles": "Owner"}) + reseed(mock_zaptec.chargers[0], CHARGER_DATA | {"current_user_roles": "User"}) + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + assert ir.async_get(hass).async_get_issue(DOMAIN, CHARGER_ISSUE_ID) is not None + + reseed(installation, INSTALLATION_DATA | {"current_user_roles": "User"}) + await manager.device_coordinators[INSTALLATION_DATA["id"]].async_refresh() + + assert ir.async_get(hass).async_get_issue(DOMAIN, ISSUE_ID) is not None + assert ir.async_get(hass).async_get_issue(DOMAIN, CHARGER_ISSUE_ID) is None + + +async def test_charger_issue_clears_when_the_charger_role_is_granted( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """Granting Service on the charger removes the charger issue on the next poll.""" + charger = mock_zaptec.chargers[0] + reseed(mock_zaptec.installations[0], INSTALLATION_DATA | {"current_user_roles": "Owner"}) + reseed(charger, CHARGER_DATA | {"current_user_roles": "User"}) + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + assert ir.async_get(hass).async_get_issue(DOMAIN, CHARGER_ISSUE_ID) is not None + + reseed(charger, CHARGER_DATA | {"current_user_roles": "Maintainer"}) + await manager.device_coordinators[INSTALLATION_DATA["id"]].async_refresh() + + assert ir.async_get(hass).async_get_issue(DOMAIN, CHARGER_ISSUE_ID) is None + + +async def test_charger_with_unobserved_role_is_not_reported( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A charger that hasn't been polled yet is left out, not assumed restricted.""" + reseed(mock_zaptec.installations[0], INSTALLATION_DATA | {"current_user_roles": "Owner"}) + reseed(mock_zaptec.chargers[0], CHARGER_DATA) + + await setup_integration(hass, mock_config_entry, mock_zaptec) + + assert ir.async_get(hass).async_get_issue(DOMAIN, CHARGER_ISSUE_ID) is None diff --git a/tests/test_entity.py b/tests/test_entity.py new file mode 100644 index 00000000..cf7da4e9 --- /dev/null +++ b/tests/test_entity.py @@ -0,0 +1,264 @@ +"""Behavior tests for ZaptecBaseEntity, driven through the real harness.""" + +import logging +from unittest.mock import AsyncMock, MagicMock + +from homeassistant.core import HomeAssistant +import pytest +from pytest_homeassistant_custom_component.common import MockConfigEntry + +from custom_components.zaptec.const import KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK +from custom_components.zaptec.coordinator import ZaptecUpdateCoordinator +from custom_components.zaptec.entity import KeyUnavailableError, ZaptecBaseEntity +from custom_components.zaptec.zaptec import MISSING, ZaptecApiError +from tests.conftest import setup_integration + + +def _entity_from_coordinator( + coordinator: ZaptecUpdateCoordinator, *, key_not_in_skip_list: bool = False +) -> ZaptecBaseEntity: + """Return a real entity instance bound to `coordinator`. + + Reaches into the private `_listeners` dict since there's no public way to + list entities subscribed to a coordinator. `hasattr(candidate, "_log_value")` + filters out non-entity listeners (e.g. the coordinator's own + `set_update_interval`, also registered as a listener). + + `key_not_in_skip_list=True` skips entities whose `.key` is in + `KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK`, needed when asserting on the + "Getting value failed" log line those keys suppress. + """ + for cb, _context in coordinator._listeners.values(): # noqa: SLF001 + candidate = cb.__self__ + if not hasattr(candidate, "_log_value"): + continue + if key_not_in_skip_list and candidate.key in KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK: + continue + return candidate + raise AssertionError("no matching zaptec entity found") + + +async def _get_zaptec_entity(hass: HomeAssistant) -> str: + """Return one live zaptec entity_id whose value is backed by seeded data. + + Not every zaptec entity reads a key `mock_zaptec` seeds, and setup order + isn't guaranteed to surface a backed one first — skip unbacked entities. + """ + for state in hass.states.async_all(): + if state.entity_id.startswith( + ("sensor.", "binary_sensor.", "switch.", "number.") + ) and state.state not in ("unavailable", "unknown"): + return state.entity_id + raise AssertionError("no backed zaptec entity found") + + +async def test_entity_reports_value_from_zaptec( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A backed key surfaces as the entity's state (not 'unavailable'/'unknown').""" + await setup_integration(hass, mock_config_entry, mock_zaptec) + entity_id = await _get_zaptec_entity(hass) + state = hass.states.get(entity_id) + assert state.state not in ("unavailable", "unknown") + + +async def test_entity_stays_available_when_single_key_missing( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A single missing backing key does NOT mark the entity unavailable. + + `CoordinatorEntity.available` is driven by `coordinator.last_update_success`, + not `_attr_available` — `_handle_coordinator_update` catches the + `KeyUnavailableError` from `_update_from_zaptec`, so the poll still succeeds + and the entity keeps its previous value/state. + """ + await setup_integration(hass, mock_config_entry, mock_zaptec) + entity_id = await _get_zaptec_entity(hass) + + state_before = hass.states.get(entity_id) + assert state_before.state not in ("unavailable", "unknown") + + # Make every key lookup miss, then re-run a refresh so entities re-read. + mock_zaptec.chargers[0].get.side_effect = lambda _key, default=MISSING: default + manager = mock_config_entry.runtime_data + for coordinator in manager.all_coordinators: + await coordinator.async_refresh() + await hass.async_block_till_done() + + state_after = hass.states.get(entity_id) + assert state_after.state != "unavailable" + assert state_after.state == state_before.state + + +async def test_entity_unavailable_when_coordinator_poll_fails( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """The entity reports 'unavailable' when its coordinator's poll fails. + + `CoordinatorEntity.available` reflects `coordinator.last_update_success`, + not any per-key state. + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + entity_id = await _get_zaptec_entity(hass) + + mock_zaptec.poll.side_effect = ZaptecApiError("boom") + coordinator = manager.device_coordinators["chg-mock-1"] + await coordinator.async_refresh() + await hass.async_block_till_done() + + assert hass.states.get(entity_id).state == "unavailable" + + +async def test_log_value_logs_on_change_then_skips_when_unchanged( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + caplog: pytest.LogCaptureFixture, + enable_custom_integrations: None, +) -> None: + """_log_value logs when the tracked value changes and stays quiet when it doesn't. + + `_log_value` reads an arbitrary attribute via `getattr` and dedups against + `_prev_value`, purely to feed a debug log line — no `hass.states` to assert on. + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + entity.some_attr = "value1" + + with caplog.at_level(logging.DEBUG): + entity._log_value("some_attr") # noqa: SLF001 + assert "value1" in caplog.text + + caplog.clear() + with caplog.at_level(logging.DEBUG): + entity._log_value("some_attr") # noqa: SLF001 + assert caplog.text == "" + + +async def test_get_zaptec_value_returns_default_when_key_missing( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """_get_zaptec_value() returns the caller's default when the key isn't backed. + + Covers the one production call site that opts out of the `MISSING`-triggers- + `KeyUnavailableError` default (`sensor.py`'s `default={}` for `completed_session`). + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + + sentinel = object() + assert entity._get_zaptec_value(key="totally_missing_key", default=sentinel) is sentinel # noqa: SLF001 + + +async def test_get_zaptec_value_raises_when_intermediate_value_not_mapping( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A dotted key whose first segment resolves to a non-Mapping value raises. + + No shipped entity uses a dotted key today, so this exercises + `_get_zaptec_value`'s "obj isn't Mapping-like" branch directly, guarding it + for whenever one does. + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + + # "operating_mode" is seeded as a plain string, which has no `.get()`. + with pytest.raises(KeyUnavailableError): + entity._get_zaptec_value(key="operating_mode.sub") # noqa: SLF001 + + +async def test_log_zaptec_attribute_formats_none_str_iterable_and_scalar_keys( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """_log_zaptec_attribute formats None, a single key, an iterable, and a scalar. + + Pokes all four branches directly since the property only feeds a debug log + line. `str` (default `description.key`) and `Iterable` (sensor.py/update.py's + multi-key logging) are live; `None` and the scalar fallback are currently + unreachable in production but worth guarding. + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + + entity._log_zaptec_key = None # noqa: SLF001 + assert entity._log_zaptec_attribute == "" # noqa: SLF001 + + entity._log_zaptec_key = "foo" # noqa: SLF001 + assert entity._log_zaptec_attribute == ".foo" # noqa: SLF001 + + entity._log_zaptec_key = ["foo", "bar"] # noqa: SLF001 + assert entity._log_zaptec_attribute == ".foo and .bar" # noqa: SLF001 + + entity._log_zaptec_key = 42 # noqa: SLF001 + assert entity._log_zaptec_attribute == ".42" # noqa: SLF001 + + +async def test_log_unavailable_logs_error_and_recovery_transitions( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + caplog: pytest.LogCaptureFixture, + enable_custom_integrations: None, +) -> None: + """_log_unavailable logs the real exception on going unavailable, and logs recovery. + + Sets `_attr_available`/`_prev_available` directly since they're decoupled + from `CoordinatorEntity.available` (which reads `last_update_success`) — no + real coordinator refresh can drive both log transitions. + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator, key_not_in_skip_list=True) + + entity._prev_available = True # noqa: SLF001 + entity._attr_available = False # noqa: SLF001 + with caplog.at_level(logging.DEBUG): + entity._log_unavailable(RuntimeError("boom")) # noqa: SLF001 + assert f"Entity {entity.entity_id} is unavailable" in caplog.text + assert "Getting value failed" in caplog.text + + caplog.clear() + entity._prev_available = False # noqa: SLF001 + entity._attr_available = True # noqa: SLF001 + with caplog.at_level(logging.DEBUG): + entity._log_unavailable() # noqa: SLF001 + assert f"Entity {entity.entity_id} is available" in caplog.text + + +async def test_entity_trigger_poll_delegates_to_coordinator( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """ZaptecBaseEntity.trigger_poll() awaits the bound coordinator's trigger_poll().""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + + coordinator.trigger_poll = AsyncMock() + await entity.trigger_poll() + + coordinator.trigger_poll.assert_awaited_once() diff --git a/tests/test_init.py b/tests/test_init.py index 4082b177..6a00fb20 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -1,17 +1,22 @@ """Tests for custom_components.zaptec.__init__.""" from http import HTTPStatus +from unittest.mock import MagicMock +from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryError, ConfigEntryNotReady import pytest +from pytest_homeassistant_custom_component.common import MockConfigEntry from custom_components.zaptec import _config_entry_error +from custom_components.zaptec.manager import ZaptecManager from custom_components.zaptec.zaptec.exceptions import ( AuthenticationError, RequestConnectionError, RequestError, RequestTimeoutError, ) +from tests.conftest import setup_integration @pytest.mark.parametrize( @@ -22,7 +27,7 @@ # Connection/timeout are recoverable -> HA retries setup. (RequestTimeoutError("slow"), ConfigEntryNotReady), (RequestConnectionError("down"), ConfigEntryNotReady), - # Transient server statuses are recoverable -> HA retries setup (issue #392). + # Transient server statuses are recoverable -> HA retries setup. (RequestError("unavailable", HTTPStatus.SERVICE_UNAVAILABLE), ConfigEntryNotReady), (RequestError("too many", HTTPStatus.TOO_MANY_REQUESTS), ConfigEntryNotReady), # Other HTTP errors stay permanent. @@ -33,3 +38,20 @@ def test_config_entry_error_mapping(err: Exception, expected: type[Exception]) -> None: """Setup login errors map to the right Home Assistant config-entry error.""" assert isinstance(_config_entry_error(err), expected) + + +async def test_setup_entry_creates_manager_and_entities( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A full setup wires up the manager and registers at least one entity.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + + assert isinstance(manager, ZaptecManager) + assert mock_config_entry.runtime_data is manager + # Matches because mock_zaptec seeds "Mock Charger"/"Mock Home", which HA + # slugifies into "mock..." entity_ids. Update if that seed naming changes. + states = [s for s in hass.states.async_all() if s.entity_id.split(".")[1].startswith("mock")] + assert states, "expected at least one zaptec entity to be created" diff --git a/tests/zaptec/conftest.py b/tests/zaptec/conftest.py new file mode 100644 index 00000000..2b86570b --- /dev/null +++ b/tests/zaptec/conftest.py @@ -0,0 +1,37 @@ +"""Test configuration for the vendored Zaptec API client (tests/zaptec/*).""" + +import asyncio + +import pytest + +from custom_components.zaptec.zaptec.api import Zaptec + + +@pytest.fixture(scope="session") +def zaptec_constants() -> dict: + """Get latest constants from Zaptec API. + + Uses a self-contained event loop instead of `asyncio.run()`: under + pytest-hacc's `HassEventLoopPolicy`, `asyncio.run()` resets the thread's + event loop to `None` on exit, breaking later `asyncio_mode=auto` tests. + Saving/restoring the previous loop avoids clobbering that global state. + """ + + async def get_zaptec_constants() -> dict: + async with Zaptec("N/A", "N/A") as zaptec: + # the constants API endpoint does not require login + const: dict = await zaptec.request("constants") + return const + + try: + previous_loop = asyncio.get_event_loop() + except RuntimeError: + previous_loop = None + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + try: + return loop.run_until_complete(get_zaptec_constants()) + finally: + loop.close() + asyncio.set_event_loop(previous_loop) diff --git a/tests/zaptec/test_api.py b/tests/zaptec/test_api.py index 825b05d9..0f69135e 100644 --- a/tests/zaptec/test_api.py +++ b/tests/zaptec/test_api.py @@ -13,6 +13,7 @@ from custom_components.zaptec.zaptec.const import API_RETRIES from custom_components.zaptec.zaptec.exceptions import ( AuthenticationError, + InsufficientRoleError, RequestConnectionError, RequestDataError, RequestError, @@ -973,3 +974,189 @@ async def test_zaptec_async_context_manager_closes_internal_client() -> None: """Entering/exiting the context manager works with an internally-created client.""" async with Zaptec("user", "pass") as zap: assert isinstance(zap, Zaptec) + + +# --------------------------------------------------------------------------- +# Installation write-call role gating (#311) +# --------------------------------------------------------------------------- + + +@pytest.fixture +def user_roles(monkeypatch: pytest.MonkeyPatch) -> None: + """Populate ZCONST.UserRoles so CurrentUserRoles ints convert to role names.""" + monkeypatch.setitem(ZCONST, "UserRoles", {"User": 1, "Owner": 2, "Maintainer": 4}) + + +@pytest.mark.asyncio +async def test_set_limit_current_blocked_for_user_only_role(user_roles: None) -> None: + """A User-only role raises InsufficientRoleError without calling the API.""" + zap, session = _make_zaptec([]) + inst = Installation({"Id": "i1", "CurrentUserRoles": 1}, zap) + + with pytest.raises(InsufficientRoleError, match="Owner or Service"): + await inst.set_limit_current(availableCurrent=10) + assert session.calls == [] + + +@pytest.mark.asyncio +async def test_set_limit_current_allowed_for_owner_role(user_roles: None) -> None: + """An Owner role lets the call through to the API.""" + zap, session = _make_zaptec([FakeResponse(HTTPStatus.OK, json_data={})]) + inst = Installation({"Id": "i1", "CurrentUserRoles": 2}, zap) + + await inst.set_limit_current(availableCurrent=10) + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_set_limit_current_allowed_for_maintainer_role(user_roles: None) -> None: + """A Maintainer (Service) role lets the call through to the API.""" + zap, session = _make_zaptec([FakeResponse(HTTPStatus.OK, json_data={})]) + inst = Installation({"Id": "i1", "CurrentUserRoles": 4}, zap) + + await inst.set_limit_current(availableCurrent=10) + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_set_limit_current_allowed_when_role_unknown() -> None: + """No CurrentUserRoles observed yet -> fall through, let the API decide.""" + inst, session = _installation_with_session([FakeResponse(HTTPStatus.OK, json_data={})]) + + await inst.set_limit_current(availableCurrent=10) + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_set_three_to_one_phase_switch_current_blocked_for_user_only_role( + user_roles: None, +) -> None: + """A User-only role raises InsufficientRoleError without calling the API.""" + zap, session = _make_zaptec([]) + inst = Installation({"Id": "i1", "CurrentUserRoles": 1}, zap) + + with pytest.raises(InsufficientRoleError, match="Owner or Service"): + await inst.set_three_to_one_phase_switch_current(10) + assert session.calls == [] + + +@pytest.mark.asyncio +async def test_set_three_to_one_phase_switch_current_allowed_for_owner_role( + user_roles: None, +) -> None: + """An Owner role lets the call through to the API.""" + zap, session = _make_zaptec([FakeResponse(HTTPStatus.OK, json_data={})]) + inst = Installation({"Id": "i1", "CurrentUserRoles": 2}, zap) + + await inst.set_three_to_one_phase_switch_current(10) + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_set_settings_blocked_for_user_only_role(user_roles: None) -> None: + """A User-only role raises InsufficientRoleError without calling the API.""" + zap, session = _make_zaptec([]) + charger = Charger({"Id": "c1", "CurrentUserRoles": 1}, zap) + + with pytest.raises(InsufficientRoleError, match="Owner or Service"): + await charger.set_settings({"maxChargeCurrent": 16}) + assert session.calls == [] + + +@pytest.mark.asyncio +async def test_set_settings_allowed_for_owner_role(user_roles: None) -> None: + """An Owner role lets the call through to the API.""" + zap, session = _make_zaptec([FakeResponse(HTTPStatus.OK, json_data={})]) + charger = Charger({"Id": "c1", "CurrentUserRoles": 2}, zap) + + await charger.set_settings({"maxChargeCurrent": 16}) + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_set_settings_allowed_when_role_unknown() -> None: + """No CurrentUserRoles observed yet -> fall through, let the API decide.""" + charger, session = _charger_with_session([FakeResponse(HTTPStatus.OK, json_data={})]) + + await charger.set_settings({"maxChargeCurrent": 16}) + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_command_blocked_for_user_only_role( + user_roles: None, monkeypatch: pytest.MonkeyPatch +) -> None: + """A User-only role raises InsufficientRoleError without calling the API.""" + monkeypatch.setattr(ZCONST, "commands", {"restart_charger": 102}, raising=False) + zap, session = _make_zaptec([]) + charger = Charger({"Id": "c1", "CurrentUserRoles": 1}, zap) + + with pytest.raises(InsufficientRoleError, match="Owner or Service"): + await charger.command("restart_charger") + assert session.calls == [] + + +@pytest.mark.asyncio +async def test_command_allowed_for_maintainer_role( + user_roles: None, monkeypatch: pytest.MonkeyPatch +) -> None: + """A Maintainer (Service) role lets the call through to the API.""" + monkeypatch.setattr(ZCONST, "commands", {"restart_charger": 102}, raising=False) + zap, session = _make_zaptec([FakeResponse(HTTPStatus.OK, json_data={})]) + charger = Charger({"Id": "c1", "CurrentUserRoles": 4}, zap) + + await charger.command("restart_charger") + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_command_allowed_when_role_unknown(monkeypatch: pytest.MonkeyPatch) -> None: + """No CurrentUserRoles observed yet -> fall through, let the API decide.""" + monkeypatch.setattr(ZCONST, "commands", {"restart_charger": 102}, raising=False) + charger, session = _charger_with_session([FakeResponse(HTTPStatus.OK, json_data={})]) + + await charger.command("restart_charger") + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_command_authorize_charge_alias_not_gated(user_roles: None) -> None: + """The undocumented authorize_charge alias is never role-gated, even for User-only.""" + zap, session = _make_zaptec([FakeResponse(HTTPStatus.OK, json_data={})]) + charger = Charger({"Id": "c1", "CurrentUserRoles": 1}, zap) + + await charger.command("authorize_charge") + + method, url, _ = session.calls[-1] + assert method == "post" + assert url.endswith("chargers/c1/authorizecharge") + + +@pytest.mark.asyncio +async def test_authorize_charge_not_gated_for_user_only_role(user_roles: None) -> None: + """authorize_charge is undocumented and deliberately not role-gated.""" + zap, session = _make_zaptec([FakeResponse(HTTPStatus.OK, json_data={})]) + charger = Charger({"Id": "c1", "CurrentUserRoles": 1}, zap) + + await charger.authorize_charge() + + assert len(session.calls) == 1 + + +@pytest.mark.asyncio +async def test_set_hmi_brightness_not_gated_for_user_only_role(user_roles: None) -> None: + """set_hmi_brightness (localSettings) is undocumented and deliberately not role-gated.""" + zap, session = _make_zaptec([FakeResponse(HTTPStatus.OK, json_data={})]) + charger = Charger({"Id": "c1", "CurrentUserRoles": 1}, zap) + + await charger.set_hmi_brightness(0.5) + + assert len(session.calls) == 1 diff --git a/tests/zaptec/test_zconst.py b/tests/zaptec/test_zconst.py index 57351439..cf452577 100644 --- a/tests/zaptec/test_zconst.py +++ b/tests/zaptec/test_zconst.py @@ -55,13 +55,10 @@ def test_user_roles() -> None: assert ZCONST.type_user_roles(0) == "None" assert ZCONST.type_user_roles(1) == "User" assert ZCONST.type_user_roles(2) == "Owner" - user_role_3 = ZCONST.type_user_roles(3) - assert "Owner" in user_role_3 - assert "User" in user_role_3 + assert ZCONST.type_user_roles(3) == "User, Owner" assert ZCONST.type_user_roles(4) == "Maintainer" - user_role_5 = ZCONST.type_user_roles(5) - assert "User" in user_role_5 - assert "Maintainer" in user_role_5 + assert ZCONST.type_user_roles(5) == "User, Maintainer" + assert ZCONST.type_user_roles(7) == "User, Owner, Maintainer" def test_authentication_types() -> None: