-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Return repository_not_found for unknown ids in all websocket commands #5382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| """Helpers for the HACS websocket API.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| if TYPE_CHECKING: | ||
| from homeassistant.components import websocket_api | ||
|
|
||
| from ..base import HacsBase | ||
| from ..repositories.base import HacsRepository | ||
|
|
||
|
|
||
| def resolve_repository( | ||
| hacs: HacsBase, | ||
| connection: websocket_api.ActiveConnection, | ||
| msg: dict[str, Any], | ||
| repository_id: str, | ||
| ) -> HacsRepository | None: | ||
| """Resolve a repository by id, send an error when it is unknown. | ||
|
|
||
| A stale frontend can reference repositories that no longer exist, | ||
| the caller should return early when this returns None. | ||
| """ | ||
| if (repository := hacs.repositories.get_by_id(repository_id)) is None: | ||
| connection.send_error( | ||
| msg["id"], | ||
| "repository_not_found", | ||
| f"Repository with ID ({repository_id}) not found", | ||
| ) | ||
|
|
||
| return repository |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repositories-clear_new-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repositories-remove-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-beta-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-download-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-ignore-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-info-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-refresh-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-release_notes-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-releases-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-remove-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-state-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||
| { | ||||||||
| "tests/test_websocket.py::test_unknown_repository_returns_not_found[repository-version-]": { | ||||||||
| "https://api.github.com/repos/hacs/integration": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/custom_components/hacs/manifest.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/contents/hacs.json": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/git/trees/main": 1, | ||||||||
| "https://api.github.com/repos/hacs/integration/releases": 1 | ||||||||
| } | ||||||||
| } | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| """Tests for the HACS websocket API.""" | ||
|
|
||
| from collections.abc import Generator | ||
|
|
||
| from homeassistant.core import HomeAssistant | ||
| import pytest | ||
|
|
||
| from tests.common import WSClient | ||
|
|
||
| UNKNOWN_REPOSITORY_ID = "1337404" | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("command", "payload"), | ||
| [ | ||
| ("hacs/repository/info", {"repository_id": UNKNOWN_REPOSITORY_ID}), | ||
| ("hacs/repository/ignore", {"repository": UNKNOWN_REPOSITORY_ID}), | ||
| ("hacs/repository/state", {"repository": UNKNOWN_REPOSITORY_ID, "state": "new"}), | ||
| ("hacs/repository/version", {"repository": UNKNOWN_REPOSITORY_ID, "version": "1.0.0"}), | ||
| ("hacs/repository/beta", {"repository": UNKNOWN_REPOSITORY_ID, "show_beta": True}), | ||
| ("hacs/repository/download", {"repository": UNKNOWN_REPOSITORY_ID}), | ||
| ("hacs/repository/remove", {"repository": UNKNOWN_REPOSITORY_ID}), | ||
| ("hacs/repository/refresh", {"repository": UNKNOWN_REPOSITORY_ID}), | ||
| ("hacs/repository/release_notes", {"repository": UNKNOWN_REPOSITORY_ID}), | ||
| ("hacs/repository/releases", {"repository_id": UNKNOWN_REPOSITORY_ID}), | ||
| ("hacs/repositories/clear_new", {"repository": UNKNOWN_REPOSITORY_ID}), | ||
| ("hacs/repositories/remove", {"repository": UNKNOWN_REPOSITORY_ID}), | ||
| ], | ||
| ids=lambda value: value.replace("hacs/", "").replace("/", "-") | ||
| if isinstance(value, str) | ||
| else "", | ||
| ) | ||
| async def test_unknown_repository_returns_not_found( | ||
| hass: HomeAssistant, | ||
| setup_integration: Generator, | ||
| ws_client: WSClient, | ||
| command: str, | ||
| payload: dict, | ||
| ): | ||
| """Ensure all repository commands handle unknown repository ids.""" | ||
| response = await ws_client.send_and_receive_json(command, payload) | ||
|
|
||
| assert response["success"] is False | ||
| assert response["error"]["code"] == "repository_not_found" | ||
| assert ( | ||
| response["error"]["message"] | ||
| == f"Repository with ID ({UNKNOWN_REPOSITORY_ID}) not found" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note: It's not clear to me what these snapshots show. They seem to repeat a lot of data, which makes me think they're not testing something distinct.