Skip to content
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

Add Linkplay mTLS/HTTPS and improve logging #124307

Merged
merged 13 commits into from
Sep 3, 2024
Merged
24 changes: 15 additions & 9 deletions homeassistant/components/linkplay/__init__.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
"""Support for LinkPlay devices."""

from dataclasses import dataclass

from aiohttp import ClientSession
from linkplay.bridge import LinkPlayBridge
from linkplay.discovery import linkplay_factory_bridge
from linkplay.discovery import linkplay_factory_httpapi_bridge
from linkplay.exceptions import LinkPlayRequestException

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_HOST
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryNotReady
from homeassistant.helpers.aiohttp_client import async_get_clientsession

from .const import PLATFORMS
from .utils import async_get_client_session


@dataclass
class LinkPlayData:
"""Data for LinkPlay."""

Expand All @@ -24,16 +29,17 @@
async def async_setup_entry(hass: HomeAssistant, entry: LinkPlayConfigEntry) -> bool:
"""Async setup hass config entry. Called when an entry has been setup."""

session = async_get_clientsession(hass)
if (
bridge := await linkplay_factory_bridge(entry.data[CONF_HOST], session)
) is None:
session: ClientSession = await async_get_client_session(hass)
bridge: LinkPlayBridge | None = None

Check warning on line 33 in homeassistant/components/linkplay/__init__.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/__init__.py#L32-L33

Added lines #L32 - L33 were not covered by tests

try:
bridge = await linkplay_factory_httpapi_bridge(entry.data[CONF_HOST], session)
except LinkPlayRequestException as exception:

Check warning on line 37 in homeassistant/components/linkplay/__init__.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/__init__.py#L35-L37

Added lines #L35 - L37 were not covered by tests
raise ConfigEntryNotReady(
f"Failed to connect to LinkPlay device at {entry.data[CONF_HOST]}"
)
) from exception

entry.runtime_data = LinkPlayData()
entry.runtime_data.bridge = bridge
entry.runtime_data = LinkPlayData(bridge=bridge)

Check warning on line 42 in homeassistant/components/linkplay/__init__.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/__init__.py#L42

Added line #L42 was not covered by tests
await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
return True

Expand Down
40 changes: 31 additions & 9 deletions homeassistant/components/linkplay/config_flow.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
"""Config flow to configure LinkPlay component."""

import logging
from typing import Any

from linkplay.discovery import linkplay_factory_bridge
from aiohttp import ClientSession
from linkplay.bridge import LinkPlayBridge
from linkplay.discovery import linkplay_factory_httpapi_bridge
from linkplay.exceptions import LinkPlayRequestException
import voluptuous as vol

from homeassistant.components import zeroconf
from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
from homeassistant.const import CONF_HOST, CONF_MODEL
from homeassistant.helpers.aiohttp_client import async_get_clientsession

from .const import DOMAIN
from .utils import async_get_client_session

_LOGGER = logging.getLogger(__name__)


class LinkPlayConfigFlow(ConfigFlow, domain=DOMAIN):
Expand All @@ -25,10 +31,15 @@ async def async_step_zeroconf(
) -> ConfigFlowResult:
"""Handle Zeroconf discovery."""

session = async_get_clientsession(self.hass)
bridge = await linkplay_factory_bridge(discovery_info.host, session)
session: ClientSession = await async_get_client_session(self.hass)
bridge: LinkPlayBridge | None = None

if bridge is None:
try:
bridge = await linkplay_factory_httpapi_bridge(discovery_info.host, session)
except LinkPlayRequestException:
_LOGGER.exception(
"Failed to connect to LinkPlay device at %s", discovery_info.host
)
return self.async_abort(reason="cannot_connect")

self.data[CONF_HOST] = discovery_info.host
Expand Down Expand Up @@ -66,14 +77,26 @@ async def async_step_user(
"""Handle a flow initialized by the user."""
errors: dict[str, str] = {}
if user_input:
session = async_get_clientsession(self.hass)
bridge = await linkplay_factory_bridge(user_input[CONF_HOST], session)
session: ClientSession = await async_get_client_session(self.hass)
bridge: LinkPlayBridge | None = None

try:
bridge = await linkplay_factory_httpapi_bridge(
user_input[CONF_HOST], session
)
except LinkPlayRequestException:
_LOGGER.exception(
"Failed to connect to LinkPlay device at %s", user_input[CONF_HOST]
)
errors["base"] = "cannot_connect"

if bridge is not None:
self.data[CONF_HOST] = user_input[CONF_HOST]
self.data[CONF_MODEL] = bridge.device.name

await self.async_set_unique_id(bridge.device.uuid)
await self.async_set_unique_id(
bridge.device.uuid, raise_on_progress=False
)
self._abort_if_unique_id_configured(
updates={CONF_HOST: self.data[CONF_HOST]}
)
Expand All @@ -83,7 +106,6 @@ async def async_step_user(
data={CONF_HOST: self.data[CONF_HOST]},
)

errors["base"] = "cannot_connect"
return self.async_show_form(
step_id="user",
data_schema=vol.Schema({vol.Required(CONF_HOST): str}),
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/linkplay/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@

DOMAIN = "linkplay"
PLATFORMS = [Platform.MEDIA_PLAYER]
CONF_SESSION = "session"
Copy link
Member

@MartinHjelmare MartinHjelmare Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't prefix constants with CONF_ unless it's a key used for configuration. Use eg DATA_SESSION instead.

2 changes: 1 addition & 1 deletion homeassistant/components/linkplay/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
"documentation": "https://www.home-assistant.io/integrations/linkplay",
"integration_type": "hub",
"iot_class": "local_polling",
"requirements": ["python-linkplay==0.0.8"],
"requirements": ["python-linkplay==0.0.9"],
"zeroconf": ["_linkplay._tcp.local."]
}
11 changes: 11 additions & 0 deletions homeassistant/components/linkplay/media_player.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@
PlayingMode.XLR: "XLR",
PlayingMode.HDMI: "HDMI",
PlayingMode.OPTICAL_2: "Optical 2",
PlayingMode.EXTERN_BLUETOOTH: "External Bluetooth",
PlayingMode.PHONO: "Phono",
PlayingMode.ARC: "ARC",
PlayingMode.COAXIAL_2: "Coaxial 2",
PlayingMode.TF_CARD_1: "SD Card 1",
PlayingMode.TF_CARD_2: "SD Card 2",
PlayingMode.CD: "CD",
PlayingMode.DAB: "DAB Radio",
PlayingMode.FM: "FM Radio",
PlayingMode.RCA: "RCA",
PlayingMode.UDISK: "USB",
}

SOURCE_MAP_INV: dict[str, PlayingMode] = {v: k for k, v in SOURCE_MAP.items()}
Expand Down
27 changes: 27 additions & 0 deletions homeassistant/components/linkplay/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

from typing import Final

from aiohttp import ClientSession
from linkplay.utils import async_create_unverified_client_session

from homeassistant.const import EVENT_HOMEASSISTANT_CLOSE
from homeassistant.core import Event, HomeAssistant, callback

from .const import CONF_SESSION, DOMAIN

MANUFACTURER_ARTSOUND: Final[str] = "ArtSound"
MANUFACTURER_ARYLIC: Final[str] = "Arylic"
MANUFACTURER_IEAST: Final[str] = "iEAST"
Expand Down Expand Up @@ -44,3 +52,22 @@
return MANUFACTURER_IEAST, MODELS_IEAST_AUDIOCAST_M5
case _:
return MANUFACTURER_GENERIC, MODELS_GENERIC


async def async_get_client_session(hass: HomeAssistant) -> ClientSession:
"""Get a ClientSession that can be used with LinkPlay devices."""
hass.data.setdefault(DOMAIN, {})
if CONF_SESSION not in hass.data[DOMAIN]:
clientsession: ClientSession = await async_create_unverified_client_session()

Check warning on line 61 in homeassistant/components/linkplay/utils.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/utils.py#L59-L61

Added lines #L59 - L61 were not covered by tests
dukeofphilberg marked this conversation as resolved.
Show resolved Hide resolved

@callback
def _async_close_websession(event: Event) -> None:

Check warning on line 64 in homeassistant/components/linkplay/utils.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/utils.py#L63-L64

Added lines #L63 - L64 were not covered by tests
"""Close websession."""
clientsession.detach()

Check warning on line 66 in homeassistant/components/linkplay/utils.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/utils.py#L66

Added line #L66 was not covered by tests

hass.bus.async_listen_once(EVENT_HOMEASSISTANT_CLOSE, _async_close_websession)
hass.data[DOMAIN][CONF_SESSION] = clientsession
return clientsession

Check warning on line 70 in homeassistant/components/linkplay/utils.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/utils.py#L68-L70

Added lines #L68 - L70 were not covered by tests

session: ClientSession = hass.data[DOMAIN][CONF_SESSION]
return session

Check warning on line 73 in homeassistant/components/linkplay/utils.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/linkplay/utils.py#L72-L73

Added lines #L72 - L73 were not covered by tests
2 changes: 1 addition & 1 deletion requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2316,7 +2316,7 @@ python-juicenet==1.1.0
python-kasa[speedups]==0.7.1

# homeassistant.components.linkplay
python-linkplay==0.0.8
python-linkplay==0.0.9

# homeassistant.components.lirc
# python-lirc==1.2.3
Expand Down
2 changes: 1 addition & 1 deletion requirements_test_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1834,7 +1834,7 @@ python-juicenet==1.1.0
python-kasa[speedups]==0.7.1

# homeassistant.components.linkplay
python-linkplay==0.0.8
python-linkplay==0.0.9

# homeassistant.components.matter
python-matter-server==6.3.0
Expand Down
9 changes: 7 additions & 2 deletions tests/components/linkplay/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from collections.abc import Generator
from unittest.mock import AsyncMock, patch

from aiohttp import ClientSession
from linkplay.bridge import LinkPlayBridge, LinkPlayDevice
import pytest

Expand All @@ -14,11 +15,15 @@

@pytest.fixture
def mock_linkplay_factory_bridge() -> Generator[AsyncMock]:
"""Mock for linkplay_factory_bridge."""
"""Mock for linkplay_factory_httpapi_bridge."""

with (
patch(
"homeassistant.components.linkplay.config_flow.linkplay_factory_bridge"
"homeassistant.components.linkplay.config_flow.async_get_client_session",
return_value=AsyncMock(spec=ClientSession),
),
patch(
"homeassistant.components.linkplay.config_flow.linkplay_factory_httpapi_bridge",
) as factory,
):
bridge = AsyncMock(spec=LinkPlayBridge)
Expand Down
48 changes: 33 additions & 15 deletions tests/components/linkplay/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
from ipaddress import ip_address
from unittest.mock import AsyncMock

from linkplay.exceptions import LinkPlayRequestException
import pytest

from homeassistant.components.linkplay.const import DOMAIN
from homeassistant.components.zeroconf import ZeroconfServiceInfo
from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF
Expand Down Expand Up @@ -47,10 +50,9 @@
)


@pytest.mark.usefixtures("mock_linkplay_factory_bridge", "mock_setup_entry")
async def test_user_flow(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test user setup config flow."""
result = await hass.config_entries.flow.async_init(
Expand All @@ -74,10 +76,9 @@ async def test_user_flow(
assert result["result"].unique_id == UUID


@pytest.mark.usefixtures("mock_linkplay_factory_bridge")
async def test_user_flow_re_entry(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test user setup config flow when an entry with the same unique id already exists."""

Expand Down Expand Up @@ -105,10 +106,9 @@ async def test_user_flow_re_entry(
assert result["reason"] == "already_configured"


@pytest.mark.usefixtures("mock_linkplay_factory_bridge", "mock_setup_entry")
async def test_zeroconf_flow(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test Zeroconf flow."""
result = await hass.config_entries.flow.async_init(
Expand All @@ -133,10 +133,9 @@ async def test_zeroconf_flow(
assert result["result"].unique_id == UUID


@pytest.mark.usefixtures("mock_linkplay_factory_bridge")
async def test_zeroconf_flow_re_entry(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test Zeroconf flow when an entry with the same unique id already exists."""

Expand All @@ -160,16 +159,35 @@ async def test_zeroconf_flow_re_entry(
assert result["reason"] == "already_configured"


async def test_flow_errors(
@pytest.mark.usefixtures("mock_setup_entry")
async def test_zeroconf_flow_errors(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
) -> None:
"""Test flow when the device discovered through Zeroconf cannot be reached."""

# Temporarily make the mock_linkplay_factory_bridge throw an exception
mock_linkplay_factory_bridge.side_effect = (LinkPlayRequestException("Error"),)

result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_ZEROCONF},
data=ZEROCONF_DISCOVERY,
)

assert result["type"] is FlowResultType.ABORT
assert result["reason"] == "cannot_connect"


@pytest.mark.usefixtures("mock_setup_entry")
async def test_user_flow_errors(
hass: HomeAssistant,
mock_linkplay_factory_bridge: AsyncMock,
mock_setup_entry: AsyncMock,
) -> None:
"""Test flow when the device cannot be reached."""

# Temporarily store bridge in a separate variable and set factory to return None
bridge = mock_linkplay_factory_bridge.return_value
mock_linkplay_factory_bridge.return_value = None
# Temporarily make the mock_linkplay_factory_bridge throw an exception
mock_linkplay_factory_bridge.side_effect = (LinkPlayRequestException("Error"),)

result = await hass.config_entries.flow.async_init(
DOMAIN,
Expand All @@ -188,8 +206,8 @@ async def test_flow_errors(
assert result["step_id"] == "user"
assert result["errors"] == {"base": "cannot_connect"}

# Make linkplay_factory_bridge return a mock bridge again
mock_linkplay_factory_bridge.return_value = bridge
# Make mock_linkplay_factory_bridge_exception no longer throw an exception
mock_linkplay_factory_bridge.side_effect = None

result = await hass.config_entries.flow.async_configure(
result["flow_id"],
Expand Down
Loading