Skip to content

Transmission Service validation and fixes #155554

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
+55 −32
Open

Transmission Service validation and fixes #155554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6cbcf99
Service validation and fixing required fields
andrew-codechimp Oct 31, 2025
6419135
Invert validation and fix entry_id
andrew-codechimp Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 38 additions & 32 deletions homeassistant/components/transmission/services.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

from functools import partial
import logging
from typing import cast

import voluptuous as vol

from homeassistant.config_entries import ConfigEntryState
from homeassistant.const import CONF_ID
from homeassistant.core import HomeAssistant, ServiceCall, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.exceptions import ServiceValidationError
from homeassistant.helpers import config_validation as cv, selector

from .const import (
Expand All @@ -23,7 +24,7 @@
SERVICE_START_TORRENT,
SERVICE_STOP_TORRENT,
)
from .coordinator import TransmissionConfigEntry, TransmissionDataUpdateCoordinator
from .coordinator import TransmissionDataUpdateCoordinator

_LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -67,63 +68,68 @@


def _get_coordinator_from_service_data(
hass: HomeAssistant, entry_id: str
call: ServiceCall,
) -> TransmissionDataUpdateCoordinator:
"""Return coordinator for entry id."""
entry: TransmissionConfigEntry | None = hass.config_entries.async_get_entry(
entry_id
)
if entry is None or entry.state is not ConfigEntryState.LOADED:
raise HomeAssistantError(f"Config entry {entry_id} is not found or not loaded")
return entry.runtime_data
config_entry_id: str = call.data[CONF_ENTRY_ID]
if not (entry := call.hass.config_entries.async_get_entry(config_entry_id)):
raise ServiceValidationError(
translation_domain=DOMAIN,
translation_key="integration_not_found",
translation_placeholders={"target": DOMAIN},
)
if entry.state is not ConfigEntryState.LOADED:
raise ServiceValidationError(
translation_domain=DOMAIN,
translation_key="not_loaded",
translation_placeholders={"target": entry.title},
)
return cast(TransmissionDataUpdateCoordinator, entry.runtime_data)


async def _async_add_torrent(service: ServiceCall) -> None:
"""Add new torrent to download."""
entry_id: str = service.data[CONF_ENTRY_ID]
coordinator = _get_coordinator_from_service_data(service.hass, entry_id)
coordinator = _get_coordinator_from_service_data(service)
torrent: str = service.data[ATTR_TORRENT]
download_path: str | None = service.data.get(ATTR_DOWNLOAD_PATH)
if torrent.startswith(
("http", "ftp:", "magnet:")
) or service.hass.config.is_allowed_path(torrent):
if download_path:
await service.hass.async_add_executor_job(
partial(
coordinator.api.add_torrent, torrent, download_dir=download_path
)
)
else:
await service.hass.async_add_executor_job(
coordinator.api.add_torrent, torrent
)
await coordinator.async_request_refresh()

if not (
torrent.startswith(("http", "ftp:", "magnet:"))
or service.hass.config.is_allowed_path(torrent)
):
raise ServiceValidationError(
translation_domain=DOMAIN,
translation_key="could_not_add_torrent",
)

if download_path:
await service.hass.async_add_executor_job(
partial(coordinator.api.add_torrent, torrent, download_dir=download_path)
)
else:
_LOGGER.warning("Could not add torrent: unsupported type or no permission")
await service.hass.async_add_executor_job(coordinator.api.add_torrent, torrent)
await coordinator.async_request_refresh()


async def _async_start_torrent(service: ServiceCall) -> None:
"""Start torrent."""
entry_id: str = service.data[CONF_ENTRY_ID]
coordinator = _get_coordinator_from_service_data(service.hass, entry_id)
coordinator = _get_coordinator_from_service_data(service)
torrent_id = service.data[CONF_ID]
await service.hass.async_add_executor_job(coordinator.api.start_torrent, torrent_id)
await coordinator.async_request_refresh()


async def _async_stop_torrent(service: ServiceCall) -> None:
"""Stop torrent."""
entry_id: str = service.data[CONF_ENTRY_ID]
coordinator = _get_coordinator_from_service_data(service.hass, entry_id)
coordinator = _get_coordinator_from_service_data(service)
torrent_id = service.data[CONF_ID]
await service.hass.async_add_executor_job(coordinator.api.stop_torrent, torrent_id)
await coordinator.async_request_refresh()


async def _async_remove_torrent(service: ServiceCall) -> None:
"""Remove torrent."""
entry_id: str = service.data[CONF_ENTRY_ID]
coordinator = _get_coordinator_from_service_data(service.hass, entry_id)
coordinator = _get_coordinator_from_service_data(service)
torrent_id = service.data[CONF_ID]
delete_data = service.data[ATTR_DELETE_DATA]
await service.hass.async_add_executor_job(
Expand Down
6 changes: 6 additions & 0 deletions homeassistant/components/transmission/services.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
add_torrent:
fields:
entry_id:
required: true
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Marking this field as required: true is redundant since it's already declared as vol.Required() in the Python service schema at line 33 of services.py. The Python schema validation takes precedence, and adding required: true in the YAML is unnecessary duplication. While not harmful, this redundancy should be removed for consistency with Home Assistant patterns, where required validation is typically handled only in the Python schema.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@andrew-codechimp andrew-codechimp Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

These are marked as required as they don't show as mandatory on the UI otherwise and require a checkbox ticking.

selector:
config_entry:
integration: transmission
Expand All @@ -18,6 +19,7 @@ add_torrent:
remove_torrent:
fields:
entry_id:
required: true
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Marking this field as required: true is redundant since it's already declared as vol.Required() in the Python service schema at line 49 of services.py. The Python schema validation takes precedence, and adding required: true in the YAML is unnecessary duplication. While not harmful, this redundancy should be removed for consistency with Home Assistant patterns, where required validation is typically handled only in the Python schema.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above

selector:
config_entry:
integration: transmission
Expand All @@ -27,24 +29,28 @@ remove_torrent:
selector:
text:
delete_data:
required: true
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Marking this field as required: true is redundant since it's already declared with a default value (vol.Optional(ATTR_DELETE_DATA, default=DEFAULT_DELETE_DATA)) in the Python service schema at line 52 of services.py. Fields with default values are optional by nature, so adding required: true creates a contradiction between the YAML and Python definitions. This should be removed.

Suggested change
required: true

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above

default: false
selector:
boolean:

start_torrent:
fields:
entry_id:
required: true
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Marking this field as required: true is redundant since it's already declared as vol.Required() in the Python service schema at line 58 of services.py. The Python schema validation takes precedence, and adding required: true in the YAML is unnecessary duplication. While not harmful, this redundancy should be removed for consistency with Home Assistant patterns, where required validation is typically handled only in the Python schema.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above

selector:
config_entry:
integration: transmission
id:
required: true
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Marking this field as required: true is redundant since it's already declared as vol.Required() in the Python service schema at line 58 of services.py. The Python schema validation takes precedence, and adding required: true in the YAML is unnecessary duplication. While not harmful, this redundancy should be removed for consistency with Home Assistant patterns, where required validation is typically handled only in the Python schema.

Suggested change
required: true

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above

example: 123
selector:
text:

stop_torrent:
fields:
entry_id:
required: true
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

Marking this field as required: true is redundant since it's already declared as vol.Required() in the Python service schema at line 62 of services.py. The Python schema validation takes precedence, and adding required: true in the YAML is unnecessary duplication. While not harmful, this redundancy should be removed for consistency with Home Assistant patterns, where required validation is typically handled only in the Python schema.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above

selector:
config_entry:
integration: transmission
Expand Down
11 changes: 11 additions & 0 deletions homeassistant/components/transmission/strings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@
}
}
},
"exceptions": {
"could_not_add_torrent": {
"message": "Could not add torrent: unsupported type or no permission."
},
"integration_not_found": {
"message": "Integration \"{target}\" not found in registry."
},
"not_loaded": {
"message": "{target} is not loaded."
}
},
"options": {
"step": {
"init": {
Expand Down
Loading