From 81f55dc498b62c92edfc00e251509160de512604 Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Fri, 9 Oct 2026 17:18:16 +0200 Subject: [PATCH 1/8] feat(cf): add channel property lookup and matching helpers Add property_value() for canonical and custom property names, and has_property() for name-only checks or exact CFProperty matches. Cover enum and string names, missing properties, empty and None values, exact matches, and first-match lookup semantics. --- server/recceiver/cf/model.py | 21 +++++++++- server/tests/unit/cf/test_model.py | 66 ++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/server/recceiver/cf/model.py b/server/recceiver/cf/model.py index 8dff6d26..4f817299 100644 --- a/server/recceiver/cf/model.py +++ b/server/recceiver/cf/model.py @@ -1,6 +1,6 @@ import enum from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Union class PVStatus(enum.Enum): @@ -57,6 +57,25 @@ class CFChannel: owner: str properties: List[CFProperty] + def has_property(self, prop: Union[CFProperty, CFPropertyName, str]) -> bool: + """Check for a property name or an exact CFProperty match. + + Name-only checks ignore owner and value, including None or empty values. + A CFProperty checks name, owner, and value using dataclass equality. + """ + if isinstance(prop, CFProperty): + return prop in self.properties + property_name = prop.value if isinstance(prop, CFPropertyName) else prop + return any(existing.name == property_name for existing in self.properties) + + def property_value(self, name: Union[CFPropertyName, str]) -> Optional[str]: + """Return the first matching property's value, or None if absent. + + Accepts a canonical CFPropertyName or a string for custom properties. + """ + property_name = name.value if isinstance(name, CFPropertyName) else name + return next((prop.value for prop in self.properties if prop.name == property_name), None) + def as_dict(self) -> Dict[str, Any]: """Serialise to the dict shape expected by pyCFClient.""" return { diff --git a/server/tests/unit/cf/test_model.py b/server/tests/unit/cf/test_model.py index 5a917107..6c114b58 100644 --- a/server/tests/unit/cf/test_model.py +++ b/server/tests/unit/cf/test_model.py @@ -1,3 +1,5 @@ +import pytest + from recceiver.cf.model import CFChannel, CFProperty, CFPropertyName, IOCInfo, PVStatus @@ -39,6 +41,70 @@ def test_from_dict_roundtrip(self): class TestCFChannel: + @pytest.mark.parametrize("name", [CFPropertyName.TIME, "time"]) + @pytest.mark.parametrize("value", ["timestamp", None, ""]) + def test_has_property_accepts_enum_or_string_regardless_of_value(self, name, value): + channel = CFChannel("PV:1", "admin", [CFProperty("time", "admin", value)]) + assert channel.has_property(name) + + def test_has_property_accepts_custom_property_name(self): + channel = CFChannel("PV:1", "admin", [CFProperty("custom", "admin", "value")]) + assert channel.has_property("custom") + + @pytest.mark.parametrize("properties", [[], [CFProperty("pvStatus", "admin", "Active")]]) + def test_has_property_returns_false_when_name_absent(self, properties): + channel = CFChannel("PV:1", "admin", properties) + assert not channel.has_property(CFPropertyName.TIME) + + def test_has_property_matches_equal_property(self): + channel = CFChannel("PV:1", "admin", [CFProperty("pvStatus", "admin", "Inactive")]) + assert channel.has_property(CFProperty("pvStatus", "admin", "Inactive")) + + @pytest.mark.parametrize( + "prop", + [ + CFProperty("other", "admin", "Inactive"), + CFProperty("pvStatus", "other", "Inactive"), + CFProperty("pvStatus", "admin", "Active"), + ], + ) + def test_has_property_requires_matching_name_owner_and_value(self, prop): + channel = CFChannel("PV:1", "admin", [CFProperty("pvStatus", "admin", "Inactive")]) + assert not channel.has_property(prop) + + def test_has_property_returns_false_for_exact_match_on_empty_channel(self): + channel = CFChannel("PV:1", "admin", []) + assert not channel.has_property(CFProperty("pvStatus", "admin", "Inactive")) + + @pytest.mark.parametrize("name", [CFPropertyName.TIME, "time"]) + def test_property_value_accepts_enum_or_string(self, name): + channel = CFChannel( + "PV:1", + "admin", + [CFProperty("pvStatus", "admin", "Active"), CFProperty("time", "admin", "timestamp")], + ) + assert channel.property_value(name) == "timestamp" + + def test_property_value_accepts_custom_property(self): + channel = CFChannel("PV:1", "admin", [CFProperty("custom", "admin", "custom value")]) + assert channel.property_value("custom") == "custom value" + + @pytest.mark.parametrize("properties", [[], [CFProperty("pvStatus", "admin", "Active")]]) + def test_property_value_returns_none_when_absent(self, properties): + channel = CFChannel("PV:1", "admin", properties) + assert channel.property_value(CFPropertyName.TIME) is None + + @pytest.mark.parametrize("value", [None, ""]) + def test_property_value_preserves_none_or_empty_value(self, value): + channel = CFChannel("PV:1", "admin", [CFProperty("time", "admin", value)]) + assert channel.property_value(CFPropertyName.TIME) == value + + def test_property_value_returns_first_match(self): + channel = CFChannel( + "PV:1", "admin", [CFProperty("time", "admin", "first"), CFProperty("time", "admin", "second")] + ) + assert channel.property_value(CFPropertyName.TIME) == "first" + def test_from_dict_roundtrip(self): ch = CFChannel( name="PV:1", From 548e4f589432b5c16db853a5d432a983beb6227c Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Fri, 9 Oct 2026 17:25:57 +0200 Subject: [PATCH 2/8] refactor(cf): move property list merging into the model Keep existing merge semantics and add direct coverage for replacement, preservation of unmanaged properties, omission of managed properties, and non-mutation. --- server/recceiver/cf/model.py | 18 ++++++++++++- server/recceiver/cf/processor.py | 17 +------------ server/tests/unit/cf/test_model.py | 41 +++++++++++++++++++++++++++++- 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/server/recceiver/cf/model.py b/server/recceiver/cf/model.py index 4f817299..3863a17b 100644 --- a/server/recceiver/cf/model.py +++ b/server/recceiver/cf/model.py @@ -1,6 +1,6 @@ import enum from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Optional, Set, Union class PVStatus(enum.Enum): @@ -94,6 +94,22 @@ def from_dict(cls, channel_dict: Dict[str, Any]) -> "CFChannel": ) +def _merge_property_lists( + new_properties: List[CFProperty], channel: CFChannel, managed_properties: Optional[Set[str]] = None +) -> List[CFProperty]: + """Merge two property lists; new_properties wins on name collision. + + Properties in channel not in new_properties are kept unless they are + managed by this recceiver (in which case the absence is intentional). + """ + managed = managed_properties or set() + new_property_names = [p.name for p in new_properties] + for old_property in channel.properties: + if old_property.name not in new_property_names and old_property.name not in managed: + new_properties = new_properties + [old_property] + return new_properties + + @dataclass class IOCInfo: """Runtime state for a connected IOC. The .id property is the primary key.""" diff --git a/server/recceiver/cf/processor.py b/server/recceiver/cf/processor.py index 4ae97c75..dca3ba2b 100755 --- a/server/recceiver/cf/processor.py +++ b/server/recceiver/cf/processor.py @@ -23,6 +23,7 @@ IOCMissingInfoError, PVStatus, RecordInfo, + _merge_property_lists, ) from recceiver.processors import ConfigAdapter @@ -879,22 +880,6 @@ def create_default_properties( ) -def _merge_property_lists( - new_properties: List[CFProperty], channel: CFChannel, managed_properties: Optional[Set[str]] = None -) -> List[CFProperty]: - """Merge two property lists; new_properties wins on name collision. - - Properties in channel not in new_properties are kept unless they are - managed by this recceiver (in which case the absence is intentional). - """ - managed = managed_properties or set() - new_property_names = [p.name for p in new_properties] - for old_property in channel.properties: - if old_property.name not in new_property_names and old_property.name not in managed: - new_properties = new_properties + [old_property] - return new_properties - - def get_current_time(timezone: Optional[str] = None) -> str: """Return the current time as a string, localised if a timezone is given.""" if timezone: diff --git a/server/tests/unit/cf/test_model.py b/server/tests/unit/cf/test_model.py index 6c114b58..9bfb8699 100644 --- a/server/tests/unit/cf/test_model.py +++ b/server/tests/unit/cf/test_model.py @@ -1,6 +1,6 @@ import pytest -from recceiver.cf.model import CFChannel, CFProperty, CFPropertyName, IOCInfo, PVStatus +from recceiver.cf.model import CFChannel, CFProperty, CFPropertyName, IOCInfo, PVStatus, _merge_property_lists class TestIOCInfo: @@ -40,6 +40,45 @@ def test_from_dict_roundtrip(self): assert CFProperty.from_dict(original.as_dict()) == original +class TestMergePropertyLists: + def test_new_property_replaces_old_property_by_name(self): + channel = CFChannel("PV:1", "admin", [CFProperty("pvStatus", "old-owner", "Active")]) + updated = CFProperty("pvStatus", "new-owner", "Inactive") + assert _merge_property_lists([updated], channel, {"pvStatus"}) == [updated] + + def test_preserves_unmanaged_properties_in_order(self): + custom = CFProperty("custom", "user", "value") + other = CFProperty("other", "user", "another value") + channel = CFChannel("PV:1", "admin", [custom, other]) + updated = CFProperty("pvStatus", "admin", "Active") + assert _merge_property_lists([updated], channel, {"pvStatus"}) == [updated, custom, other] + + def test_removes_omitted_managed_properties(self): + custom = CFProperty("custom", "user", "value") + channel = CFChannel("PV:1", "admin", [CFProperty("archive", "admin", "old value"), custom]) + updated = CFProperty("pvStatus", "admin", "Active") + assert _merge_property_lists([updated], channel, {"archive", "pvStatus"}) == [updated, custom] + + @pytest.mark.parametrize("managed", [None, set()]) + def test_empty_new_properties_preserve_existing_when_nothing_is_managed(self, managed): + existing = CFProperty("custom", "user", "value") + channel = CFChannel("PV:1", "admin", [existing]) + assert _merge_property_lists([], channel, managed) == [existing] + + def test_empty_new_properties_remove_all_managed_properties(self): + channel = CFChannel("PV:1", "admin", [CFProperty("archive", "admin", "old value")]) + assert _merge_property_lists([], channel, {"archive"}) == [] + + def test_does_not_mutate_either_input_list(self): + existing = CFProperty("custom", "user", "value") + channel = CFChannel("PV:1", "admin", [existing]) + updated = CFProperty("pvStatus", "admin", "Active") + new_properties = [updated] + assert _merge_property_lists(new_properties, channel) == [updated, existing] + assert new_properties == [updated] + assert channel.properties == [existing] + + class TestCFChannel: @pytest.mark.parametrize("name", [CFPropertyName.TIME, "time"]) @pytest.mark.parametrize("value", ["timestamp", None, ""]) From 44e0f1b39850edcceccf5adce6e02693f313100d Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Fri, 9 Oct 2026 17:57:46 +0200 Subject: [PATCH 3/8] refactor(integration): use adapter in integration tests Migrate factories, fixtures, queries, and property payloads to PyCFClientAdapter and typed CFChannel/CFProperty objects. Keep property inspection and assertion behaviour explicit for separate follow-up commits. --- server/tests/integration/cf_client.py | 77 ++++++----- server/tests/integration/test_bash_ioc.py | 101 +++++--------- .../integration/test_multiple_recceiver.py | 75 +++++------ server/tests/integration/test_single_ioc.py | 126 +++++++++--------- 4 files changed, 177 insertions(+), 202 deletions(-) diff --git a/server/tests/integration/cf_client.py b/server/tests/integration/cf_client.py index 73dd0604..84e17f9b 100644 --- a/server/tests/integration/cf_client.py +++ b/server/tests/integration/cf_client.py @@ -1,14 +1,17 @@ import logging import time -from typing import Callable +from typing import Callable, Sequence, Union from channelfinder import ChannelFinderClient from testcontainers.compose import DockerCompose +from recceiver.cf.adapter import PyCFClientAdapter +from recceiver.cf.model import CFChannel, CFProperty, CFPropertyName, PVStatus + LOG: logging.Logger = logging.getLogger(__name__) -ACTIVE_PROPERTY = {"name": "pvStatus", "owner": "admin", "value": "Active", "channels": []} -INACTIVE_PROPERTY = {"name": "pvStatus", "owner": "admin", "value": "Inactive", "channels": []} +ACTIVE_PROPERTY = CFProperty(CFPropertyName.PV_STATUS.value, "admin", PVStatus.ACTIVE.value) +INACTIVE_PROPERTY = CFProperty(CFPropertyName.PV_STATUS.value, "admin", PVStatus.INACTIVE.value) MAX_WAIT_SECONDS = 180 TIME_PERIOD_INCREMENT = 2 DEFAULT_CHANNEL_NAME = "IOC1-1:ai:test" @@ -18,38 +21,50 @@ BASE_IOC_CHANNEL_COUNT = BASE_ALIAS_COUNT + BASE_RECORD_COUNT -def channel_match(channel0, channel1, properties_to_match: list[str]): - assert channel0["name"] == channel1["name"] - assert channel0["owner"] == channel1["owner"] - - for prop in channel0["properties"]: - assert not (prop["name"] in properties_to_match and prop not in channel1["properties"]), ( - f"Property {prop} not found in channel {channel1['name']}" - ) +def channel_match( + channel0: CFChannel, channel1: CFChannel, properties_to_match: Sequence[Union[CFPropertyName, str]] +) -> None: + assert channel0.name == channel1.name + assert channel0.owner == channel1.owner + property_names = {name.value if isinstance(name, CFPropertyName) else name for name in properties_to_match} + for prop in channel0.properties: + if prop.name in property_names: + assert prop in channel1.properties, f"Property {prop} not found in channel {channel1.name}" -def channels_match(channels_begin, channels_end, properties_to_match: list[str]): - for channel_index, channel in enumerate(channels_begin): - channel_match(channel, channels_end[channel_index], properties_to_match) +def channels_match( + channels_begin: list[CFChannel], + channels_end: list[CFChannel], + properties_to_match: Sequence[Union[CFPropertyName, str]], +) -> None: + for index, channel in enumerate(channels_begin): + channel_match(channel, channels_end[index], properties_to_match) -def check_channel_count(cf_client: ChannelFinderClient, expected_channel_count, name="*"): - channels = cf_client.find(name=name) +def check_channel_count(adapter: PyCFClientAdapter, expected_channel_count: int, name: str = "*") -> bool: + channels = adapter.find_by_names([name]) LOG.debug("Found %s channels", len(channels)) return len(channels) == expected_channel_count -def check_channel_property(cf_client: ChannelFinderClient, name="*", prop=ACTIVE_PROPERTY): - channels = cf_client.find(name=name) - active_channels = (prop in channel["properties"] for channel in channels) - return all(active_channels) +def check_channel_property(adapter: PyCFClientAdapter, name: str = "*", prop: CFProperty = ACTIVE_PROPERTY) -> bool: + channels = adapter.find_by_names([name]) + return all(prop in channel.properties for channel in channels) + +def find_ioc_channels(adapter: PyCFClientAdapter, ioc_name: str) -> list[CFChannel]: + return [ + channel + for channel in adapter.find_by_names(["*"]) + if any(prop.name == CFPropertyName.IOC_NAME.value and prop.value == ioc_name for prop in channel.properties) + ] -def wait_for_sync(cf_client: ChannelFinderClient, check: Callable[[ChannelFinderClient], bool]) -> bool: + +def wait_for_sync(adapter: PyCFClientAdapter, check: Callable[[PyCFClientAdapter], bool]) -> bool: time_period_to_wait_seconds = 1 total_seconds_waited = 0 while total_seconds_waited < MAX_WAIT_SECONDS: - if check(cf_client): + if check(adapter): return True time.sleep(time_period_to_wait_seconds) total_seconds_waited += time_period_to_wait_seconds @@ -57,19 +72,17 @@ def wait_for_sync(cf_client: ChannelFinderClient, check: Callable[[ChannelFinder return False -def create_client_and_wait(compose: DockerCompose, expected_channel_count) -> ChannelFinderClient: +def create_adapter_and_wait(compose: DockerCompose, expected_channel_count: int) -> PyCFClientAdapter: LOG.info("Waiting for channels to sync") - cf_client = create_client_from_compose(compose) - assert wait_for_sync( - cf_client, lambda cf_client: check_channel_count(cf_client, expected_channel_count=expected_channel_count) - ) - return cf_client + adapter = create_adapter_from_compose(compose) + assert wait_for_sync(adapter, lambda adapter: check_channel_count(adapter, expected_channel_count)) + return adapter -def create_client_from_compose(compose: DockerCompose) -> ChannelFinderClient: +def create_adapter_from_compose(compose: DockerCompose) -> PyCFClientAdapter: cf_host, cf_port = compose.get_service_host_and_port("cf", 8080) cf_url = f"http://{cf_host if cf_host else 'localhost'}:{cf_port}/ChannelFinder" - # wait for channels to sync LOG.info("CF URL: %s", cf_url) - cf_client = ChannelFinderClient(BaseURL=cf_url, username="admin", password="password") - return cf_client + return PyCFClientAdapter( + ChannelFinderClient(BaseURL=cf_url, username="admin", password="password"), size_limit=10000 + ) diff --git a/server/tests/integration/test_bash_ioc.py b/server/tests/integration/test_bash_ioc.py index 6f2918eb..0a607ad0 100644 --- a/server/tests/integration/test_bash_ioc.py +++ b/server/tests/integration/test_bash_ioc.py @@ -3,18 +3,19 @@ from pathlib import Path from typing import Optional -from channelfinder import ChannelFinderClient from docker.models.containers import Container from testcontainers.compose import DockerCompose from docker import DockerClient +from recceiver.cf.adapter import PyCFClientAdapter +from recceiver.cf.model import CFProperty from .cf_client import ( BASE_IOC_CHANNEL_COUNT, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY, check_channel_property, - create_client_and_wait, + create_adapter_and_wait, wait_for_sync, ) from .docker_compose import ComposeFixtureFactory @@ -40,13 +41,7 @@ def stream_logs(exec_result, cmd: str): LOG.debug(line.decode().strip()) exec_result = container.exec_run(command, tty=True, stream=True, environment=env) - log_thread = threading.Thread( - target=stream_logs, - args=( - exec_result, - command, - ), - ) + log_thread = threading.Thread(target=stream_logs, args=(exec_result, command)) log_thread.start() @@ -58,100 +53,66 @@ def start_ioc(setup_compose: DockerCompose, db_file: Optional[str] = None) -> Co return docker_ioc -def restart_ioc( - ioc_container: Container, cf_client: ChannelFinderClient, channel_name: str, new_db_file: str -) -> Container: +def restart_ioc(ioc_container: Container, cf_adapter: PyCFClientAdapter, channel_name: str, new_db_file: str) -> None: ioc_container.stop() LOG.info("Waiting for channels to go inactive") assert wait_for_sync( - cf_client, - lambda cf_client: check_channel_property(cf_client, name=channel_name, prop=INACTIVE_PROPERTY), + cf_adapter, + lambda adapter: check_channel_property(adapter, name=channel_name, prop=INACTIVE_PROPERTY), ) ioc_container.start() - docker_exec_new_command(ioc_container, "./demo /ioc/st.cmd", env={"DB_FILE": new_db_file}) - # Detach by not waiting for the thread to finish LOG.debug("ioc1-1 restart") - assert wait_for_sync(cf_client, lambda cf_client: check_channel_property(cf_client, name=channel_name)), ( + assert wait_for_sync(cf_adapter, lambda adapter: check_channel_property(adapter, name=channel_name)), ( "ioc1-1 failed to restart and sync" ) class TestRemoveInfoTag: def test_remove_infotag(self, setup_compose: DockerCompose) -> None: - """ - Test that removing an infotag from a record works - """ - test_channel_count = 1 - # Arrange + """Removing an infotag from a record removes its CF property.""" docker_ioc = start_ioc(setup_compose, db_file="test_remove_infotag_before.db") - LOG.info("Waiting for channels to sync") - cf_client = create_client_and_wait(setup_compose, expected_channel_count=test_channel_count) - - # Check ioc1-1 has ai:test with info tag "archive" - LOG.debug('Checking ioc1-1 has ai:test with info tag "archive"') - channels = cf_client.find(name=DEFAULT_CHANNEL_NAME) - TEST_INFO_TAG = {"name": "archive", "owner": "admin", "value": "testing", "channels": []} - - assert any(TEST_INFO_TAG in ch["properties"] for ch in channels), ( - "Info tag 'archive' not found in channel before removal" + cf_adapter = create_adapter_and_wait(setup_compose, expected_channel_count=1) + info_tag = CFProperty("archive", "admin", "testing") + channels = cf_adapter.find_by_names([DEFAULT_CHANNEL_NAME]) + assert any(info_tag in channel.properties for channel in channels), ( + "Info tag 'archive' not found before removal" ) - # Act - restart_ioc(docker_ioc, cf_client, DEFAULT_CHANNEL_NAME, "test_remove_infotag_after.db") + restart_ioc(docker_ioc, cf_adapter, DEFAULT_CHANNEL_NAME, "test_remove_infotag_after.db") - # Assert - channels = cf_client.find(name=DEFAULT_CHANNEL_NAME) - LOG.debug("archive channels: %s", channels) - assert all(TEST_INFO_TAG not in ch["properties"] for ch in channels), ( + channels = cf_adapter.find_by_names([DEFAULT_CHANNEL_NAME]) + assert all(info_tag not in channel.properties for channel in channels), ( "Info tag 'archive' still found in channel after removal" ) class TestRemoveChannel: def test_remove_channel(self, setup_compose: DockerCompose) -> None: # noqa: F811 - """ - Test that removing a channel works correctly. - """ - # Arrange + """Removing a channel marks it Inactive while keeping the live record Active.""" docker_ioc = start_ioc(setup_compose, db_file="test_remove_channel_before.db") - LOG.info("Waiting for channels to sync") - cf_client = create_client_and_wait(setup_compose, expected_channel_count=2) - - # Check ioc1-1 has base channel - LOG.debug("Checking ioc1-1 has both channels before removal") - check_channel_property(cf_client, name=DEFAULT_CHANNEL_NAME) + cf_adapter = create_adapter_and_wait(setup_compose, expected_channel_count=2) second_channel_name = f"{DEFAULT_CHANNEL_NAME}-2" - check_channel_property(cf_client, name=second_channel_name) + check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) + check_channel_property(cf_adapter, name=second_channel_name) - # Act - restart_ioc(docker_ioc, cf_client, DEFAULT_CHANNEL_NAME, "test_remove_channel_after.db") + restart_ioc(docker_ioc, cf_adapter, DEFAULT_CHANNEL_NAME, "test_remove_channel_after.db") - # Assert - check_channel_property(cf_client, name=second_channel_name, prop=INACTIVE_PROPERTY) - check_channel_property(cf_client, name=DEFAULT_CHANNEL_NAME) + check_channel_property(cf_adapter, name=second_channel_name, prop=INACTIVE_PROPERTY) + check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) class TestRemoveAlias: def test_remove_alias(self, setup_compose: DockerCompose) -> None: # noqa: F811 - """ - Test that removing an alias works correctly. - """ - # Arrange + """Removing an alias marks it Inactive while keeping its record Active.""" docker_ioc = start_ioc(setup_compose) - LOG.info("Waiting for channels to sync") - cf_client = create_client_and_wait(setup_compose, expected_channel_count=BASE_IOC_CHANNEL_COUNT) - - # Check before alias status - LOG.debug('Checking ioc1-1 has ai:base_pv3 has an Active alias"') + cf_adapter = create_adapter_and_wait(setup_compose, expected_channel_count=BASE_IOC_CHANNEL_COUNT) channel_alias_name = f"{DEFAULT_CHANNEL_NAME}:alias" - check_channel_property(cf_client, name=DEFAULT_CHANNEL_NAME) - check_channel_property(cf_client, name=channel_alias_name) + check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) + check_channel_property(cf_adapter, name=channel_alias_name) - # Act - restart_ioc(docker_ioc, cf_client, DEFAULT_CHANNEL_NAME, "test_remove_alias_after.db") + restart_ioc(docker_ioc, cf_adapter, DEFAULT_CHANNEL_NAME, "test_remove_alias_after.db") - # Assert - check_channel_property(cf_client, name=DEFAULT_CHANNEL_NAME) - check_channel_property(cf_client, name=channel_alias_name, prop=INACTIVE_PROPERTY) + check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) + check_channel_property(cf_adapter, name=channel_alias_name, prop=INACTIVE_PROPERTY) diff --git a/server/tests/integration/test_multiple_recceiver.py b/server/tests/integration/test_multiple_recceiver.py index 35f95bef..438a3a43 100644 --- a/server/tests/integration/test_multiple_recceiver.py +++ b/server/tests/integration/test_multiple_recceiver.py @@ -2,16 +2,17 @@ from pathlib import Path import pytest -from channelfinder import ChannelFinderClient from testcontainers.compose import DockerCompose -from .cf_client import BASE_ALIAS_COUNT, BASE_IOC_CHANNEL_COUNT, DEFAULT_CHANNEL_NAME, create_client_and_wait +from recceiver.cf.adapter import PyCFClientAdapter +from recceiver.cf.model import CFProperty, CFPropertyName + +from .cf_client import BASE_ALIAS_COUNT, BASE_IOC_CHANNEL_COUNT, DEFAULT_CHANNEL_NAME, create_adapter_and_wait from .docker_compose import ComposeFixtureFactory LOG: logging.Logger = logging.getLogger(__name__) RECSYNC_RESTART_DELAY = 30 -# Number of channels expected in the default setup IOC_COUNT = 4 EXPECTED_DEFAULT_CHANNEL_COUNT = IOC_COUNT * BASE_IOC_CHANNEL_COUNT @@ -21,44 +22,42 @@ @pytest.fixture(scope="class") -def cf_client(setup_compose: DockerCompose): # noqa: F811 - return create_client_and_wait(setup_compose, EXPECTED_DEFAULT_CHANNEL_COUNT) +def cf_adapter(setup_compose: DockerCompose) -> PyCFClientAdapter: # noqa: F811 + return create_adapter_and_wait(setup_compose, EXPECTED_DEFAULT_CHANNEL_COUNT) class TestMultipleRecceiver: - def test_number_of_channels_and_channel_name(self, cf_client: ChannelFinderClient) -> None: - channels = cf_client.find(name="*") + def test_number_of_channels_and_channel_name(self, cf_adapter: PyCFClientAdapter) -> None: + channels = cf_adapter.find_by_names(["*"]) assert len(channels) == EXPECTED_DEFAULT_CHANNEL_COUNT - assert channels[0]["name"] == DEFAULT_CHANNEL_NAME - - # Smoke Test Default Properties - def test_number_of_aliases_and_alais_property(self, cf_client: ChannelFinderClient) -> None: - channels = cf_client.find(property=[("alias", "*")]) - assert len(channels) == IOC_COUNT * BASE_ALIAS_COUNT - assert channels[0]["name"] == DEFAULT_CHANNEL_NAME + ":alias" - assert { - "name": "alias", - "value": DEFAULT_CHANNEL_NAME, - "owner": "admin", - "channels": [], - } in channels[0]["properties"] - - def test_number_of_record_desc_and_property(self, cf_client: ChannelFinderClient) -> None: - channels = cf_client.find(property=[("recordDesc", "*")]) + assert channels[0].name == DEFAULT_CHANNEL_NAME + + def test_number_of_aliases_and_alais_property(self, cf_adapter: PyCFClientAdapter) -> None: + aliases = [ + channel + for channel in cf_adapter.find_by_names(["*"]) + if any(prop.name == CFPropertyName.ALIAS.value for prop in channel.properties) + ] + assert len(aliases) == IOC_COUNT * BASE_ALIAS_COUNT + assert aliases[0].name == DEFAULT_CHANNEL_NAME + ":alias" + assert CFProperty(CFPropertyName.ALIAS.value, "admin", DEFAULT_CHANNEL_NAME) in aliases[0].properties + + def test_number_of_record_desc_and_property(self, cf_adapter: PyCFClientAdapter) -> None: + channels = [ + channel + for channel in cf_adapter.find_by_names(["*"]) + if any(prop.name == CFPropertyName.RECORD_DESC.value for prop in channel.properties) + ] assert len(channels) == EXPECTED_DEFAULT_CHANNEL_COUNT - assert { - "name": "recordDesc", - "value": "testdesc", - "owner": "admin", - "channels": [], - } in channels[0]["properties"] - - def test_number_of_record_type_and_property(self, cf_client: ChannelFinderClient) -> None: - channels = cf_client.find(property=[("recordType", "*")]) + expected = CFProperty(CFPropertyName.RECORD_DESC.value, "admin", "testdesc") + assert expected in channels[0].properties + + def test_number_of_record_type_and_property(self, cf_adapter: PyCFClientAdapter) -> None: + channels = [ + channel + for channel in cf_adapter.find_by_names(["*"]) + if any(prop.name == CFPropertyName.RECORD_TYPE.value for prop in channel.properties) + ] assert len(channels) == EXPECTED_DEFAULT_CHANNEL_COUNT - assert { - "name": "recordType", - "value": "ai", - "owner": "admin", - "channels": [], - } in channels[0]["properties"] + expected = CFProperty(CFPropertyName.RECORD_TYPE.value, "admin", "ai") + assert expected in channels[0].properties diff --git a/server/tests/integration/test_single_ioc.py b/server/tests/integration/test_single_ioc.py index 5dacf812..41a30b4a 100644 --- a/server/tests/integration/test_single_ioc.py +++ b/server/tests/integration/test_single_ioc.py @@ -3,17 +3,20 @@ from pathlib import Path import pytest -from channelfinder import ChannelFinderClient from testcontainers.compose import DockerCompose +from recceiver.cf.adapter import PyCFClientAdapter +from recceiver.cf.model import CFProperty, CFPropertyName + from .cf_client import ( BASE_IOC_CHANNEL_COUNT, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY, channels_match, check_channel_property, - create_client_and_wait, - create_client_from_compose, + create_adapter_and_wait, + create_adapter_from_compose, + find_ioc_channels, wait_for_sync, ) from .docker_compose import ( @@ -25,7 +28,15 @@ start_container, ) -PROPERTIES_TO_MATCH = ["pvStatus", "recordType", "recordDesc", "alias", "hostName", "iocName", "recceiverID"] +PROPERTIES_TO_MATCH = [ + CFPropertyName.PV_STATUS, + CFPropertyName.RECORD_TYPE, + CFPropertyName.RECORD_DESC, + CFPropertyName.ALIAS, + CFPropertyName.HOSTNAME, + CFPropertyName.IOC_NAME, + CFPropertyName.RECCEIVER_ID, +] LOG: logging.Logger = logging.getLogger(__name__) @@ -35,41 +46,40 @@ @pytest.fixture(scope="class") -def cf_client(setup_compose: DockerCompose) -> ChannelFinderClient: # noqa: F811 - return create_client_and_wait(setup_compose, expected_channel_count=BASE_IOC_CHANNEL_COUNT) +def cf_adapter(setup_compose: DockerCompose) -> PyCFClientAdapter: # noqa: F811 + return create_adapter_and_wait(setup_compose, expected_channel_count=BASE_IOC_CHANNEL_COUNT) class TestRestartIOC: - def test_channels_same_after_restart(self, setup_compose: DockerCompose, cf_client: ChannelFinderClient) -> None: # noqa: F811 - channels_begin = cf_client.find(name="*") + def test_channels_same_after_restart(self, setup_compose: DockerCompose, cf_adapter: PyCFClientAdapter) -> None: # noqa: F811 + channels_begin = cf_adapter.find_by_names(["*"]) restart_container(setup_compose, "ioc1-1") - assert wait_for_sync(cf_client, lambda cf_client: check_channel_property(cf_client, DEFAULT_CHANNEL_NAME)) - channels_end = cf_client.find(name="*") + assert wait_for_sync(cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME)) + channels_end = cf_adapter.find_by_names(["*"]) assert len(channels_begin) == len(channels_end) channels_match(channels_begin, channels_end, PROPERTIES_TO_MATCH) def test_manual_channels_same_after_restart( self, setup_compose: DockerCompose, # noqa: F811 - cf_client: ChannelFinderClient, + cf_adapter: PyCFClientAdapter, ) -> None: - test_property = {"name": "test_property", "owner": "testowner"} - cf_client.set(properties=[test_property]) - test_property_value = test_property | {"value": "test_value"} - channels = cf_client.find(name=DEFAULT_CHANNEL_NAME) - channels[0]["properties"] = [test_property_value] - cf_client.set(property=test_property) - channels_begin = cf_client.find(name="*") + test_property = CFProperty("test_property", "testowner", "test_value") + cf_adapter.set_property(test_property.name, test_property.owner) + channels = cf_adapter.find_by_names([DEFAULT_CHANNEL_NAME]) + channels[0].properties = [test_property] + cf_adapter.set_property(test_property.name, test_property.owner) + channels_begin = cf_adapter.find_by_names(["*"]) restart_container(setup_compose, "ioc1-1") - assert wait_for_sync(cf_client, lambda cf_client: check_channel_property(cf_client, DEFAULT_CHANNEL_NAME)) - channels_end = cf_client.find(name="*") + assert wait_for_sync(cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME)) + channels_end = cf_adapter.find_by_names(["*"]) assert len(channels_begin) == len(channels_end) - channels_match(channels_begin, channels_end, PROPERTIES_TO_MATCH + ["test_property"]) + channels_match(channels_begin, channels_end, PROPERTIES_TO_MATCH + [test_property.name]) -def check_connection_active(cf_client: ChannelFinderClient) -> bool: +def check_connection_active(adapter: PyCFClientAdapter) -> bool: try: - cf_client.find(name="*") + adapter.find_by_names(["*"]) except Exception: return False return True @@ -79,89 +89,81 @@ class TestRestartChannelFinder: def test_status_property_works_after_cf_restart( self, setup_compose: DockerCompose, # noqa: F811 - cf_client: ChannelFinderClient, + cf_adapter: PyCFClientAdapter, ) -> None: - # Arrange - # Act restart_container(setup_compose, "cf") - refreshed_cf_client = create_client_from_compose(setup_compose) - assert wait_for_sync(refreshed_cf_client, check_connection_active) + refreshed_adapter = create_adapter_from_compose(setup_compose) + assert wait_for_sync(refreshed_adapter, check_connection_active) - # Assert shutdown_container(setup_compose, "ioc1-1") assert wait_for_sync( - refreshed_cf_client, - lambda client: check_channel_property(client, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), + refreshed_adapter, + lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) - channels_inactive = refreshed_cf_client.find(property=[("iocName", "IOC1-1")]) - assert all(INACTIVE_PROPERTY in ch["properties"] for ch in channels_inactive) + channels_inactive = find_ioc_channels(refreshed_adapter, "IOC1-1") + assert all(INACTIVE_PROPERTY in channel.properties for channel in channels_inactive) class TestShutdownChannelFinder: def test_status_property_works_between_cf_down( self, setup_compose: DockerCompose, # noqa: F811 - cf_client: ChannelFinderClient, + cf_adapter: PyCFClientAdapter, ) -> None: - # Arrange cf_container_id = shutdown_container(setup_compose, "cf") time.sleep(10) # Wait to ensure CF is down while IOC is down - # Act shutdown_container(setup_compose, "ioc1-1") time.sleep(10) # Wait to ensure CF is down while IOC is down start_container(setup_compose, container_id=cf_container_id) - refreshed_cf_client = create_client_from_compose(setup_compose) - assert wait_for_sync(refreshed_cf_client, check_connection_active) + refreshed_adapter = create_adapter_from_compose(setup_compose) + assert wait_for_sync(refreshed_adapter, check_connection_active) - # Assert assert wait_for_sync( - refreshed_cf_client, - lambda client: check_channel_property(client, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), + refreshed_adapter, + lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) - channels_inactive = refreshed_cf_client.find(property=[("iocName", "IOC1-1")]) - assert all(INACTIVE_PROPERTY in ch["properties"] for ch in channels_inactive) + channels_inactive = find_ioc_channels(refreshed_adapter, "IOC1-1") + assert all(INACTIVE_PROPERTY in channel.properties for channel in channels_inactive) class TestCleanStopRecceiver: def test_clean_stop_marks_channels_inactive( - self, setup_compose: DockerCompose, cf_client: ChannelFinderClient + self, setup_compose: DockerCompose, cf_adapter: PyCFClientAdapter ) -> None: # noqa: F811 shutdown_container(setup_compose, "recc1") assert wait_for_sync( - cf_client, - lambda client: check_channel_property(client, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), + cf_adapter, + lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) - channels_inactive = cf_client.find(property=[("iocName", "IOC1-1")]) - assert all(INACTIVE_PROPERTY in ch["properties"] for ch in channels_inactive) + channels_inactive = find_ioc_channels(cf_adapter, "IOC1-1") + assert all(INACTIVE_PROPERTY in channel.properties for channel in channels_inactive) class TestCleanStartRecceiver: def test_startup_sweep_marks_stale_channels_inactive( - self, setup_compose: DockerCompose, cf_client: ChannelFinderClient + self, setup_compose: DockerCompose, cf_adapter: PyCFClientAdapter ) -> None: # noqa: F811 - # Kill recceiver hard — cleanOnStop does NOT run, channels stay Active in CF - recc1_id = kill_container(setup_compose, "recc1") - # Stop IOC so it cannot reconnect when the recceiver comes back + # SIGKILL bypasses cleanOnStop, leaving channels Active in CF. + receiver_id = kill_container(setup_compose, "recc1") shutdown_container(setup_compose, "ioc1-1") - # Start recceiver — cleanOnStart sweep should mark the stale channels Inactive - start_container(setup_compose, container_id=recc1_id) + start_container(setup_compose, container_id=receiver_id) assert wait_for_sync( - cf_client, - lambda client: check_channel_property(client, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), + cf_adapter, + lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) - channels_inactive = cf_client.find(property=[("iocName", "IOC1-1")]) - assert all(INACTIVE_PROPERTY in ch["properties"] for ch in channels_inactive) + channels_inactive = find_ioc_channels(cf_adapter, "IOC1-1") + assert all(INACTIVE_PROPERTY in channel.properties for channel in channels_inactive) class TestMoveIocHost: def test_move_ioc_host( self, setup_compose: DockerCompose, # noqa: F811 - cf_client: ChannelFinderClient, + cf_adapter: PyCFClientAdapter, ) -> None: - channels_begin = cf_client.find(name="*") + channels_begin = cf_adapter.find_by_names(["*"]) clone_container(setup_compose, "ioc1-1-new", host_name="ioc1-1") - wait_for_sync(cf_client, lambda cf_client: check_channel_property(cf_client, DEFAULT_CHANNEL_NAME)) - channels_end = cf_client.find(name="*") + wait_for_sync(cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME)) + channels_end = cf_adapter.find_by_names(["*"]) assert len(channels_begin) == len(channels_end) From 8d002ebda9cdba3a88f917a3e3635f863a505f08 Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Fri, 9 Oct 2026 17:27:34 +0200 Subject: [PATCH 4/8] refactor(cf): merge properties through the channel model Replace the free merge helper with non-mutating CFChannel.merged_properties(). Preserve name-based replacement, managed-property omission, ordering, and alias construction without changing parent channels. --- server/recceiver/cf/model.py | 30 ++++++++++++-------------- server/recceiver/cf/processor.py | 34 ++++++++++-------------------- server/tests/unit/cf/test_model.py | 14 ++++++------ 3 files changed, 32 insertions(+), 46 deletions(-) diff --git a/server/recceiver/cf/model.py b/server/recceiver/cf/model.py index 3863a17b..23142b9f 100644 --- a/server/recceiver/cf/model.py +++ b/server/recceiver/cf/model.py @@ -57,6 +57,20 @@ class CFChannel: owner: str properties: List[CFProperty] + def merged_properties( + self, new_properties: List[CFProperty], managed_properties: Optional[Set[str]] = None + ) -> List[CFProperty]: + """Return merged properties without modifying either input list. + + New properties win on name collision. Existing properties are retained + unless replaced or omitted from the supplied managed property names. + """ + managed = managed_properties or set() + new_names = {prop.name for prop in new_properties} + return new_properties + [ + prop for prop in self.properties if prop.name not in new_names and prop.name not in managed + ] + def has_property(self, prop: Union[CFProperty, CFPropertyName, str]) -> bool: """Check for a property name or an exact CFProperty match. @@ -94,22 +108,6 @@ def from_dict(cls, channel_dict: Dict[str, Any]) -> "CFChannel": ) -def _merge_property_lists( - new_properties: List[CFProperty], channel: CFChannel, managed_properties: Optional[Set[str]] = None -) -> List[CFProperty]: - """Merge two property lists; new_properties wins on name collision. - - Properties in channel not in new_properties are kept unless they are - managed by this recceiver (in which case the absence is intentional). - """ - managed = managed_properties or set() - new_property_names = [p.name for p in new_properties] - for old_property in channel.properties: - if old_property.name not in new_property_names and old_property.name not in managed: - new_properties = new_properties + [old_property] - return new_properties - - @dataclass class IOCInfo: """Runtime state for a connected IOC. The .id property is the primary key.""" diff --git a/server/recceiver/cf/processor.py b/server/recceiver/cf/processor.py index dca3ba2b..eb545d53 100755 --- a/server/recceiver/cf/processor.py +++ b/server/recceiver/cf/processor.py @@ -23,7 +23,6 @@ IOCMissingInfoError, PVStatus, RecordInfo, - _merge_property_lists, ) from recceiver.processors import ConfigAdapter @@ -681,9 +680,8 @@ def _handle_channel_is_old( last_ioc_info and must orphan the channel instead when it is absent. """ cf_channel.owner = last_ioc_info.owner - cf_channel.properties = _merge_property_lists( + cf_channel.properties = cf_channel.merged_properties( create_default_properties(ioc_info, recceiverid, self.channel_ioc_ids, self.iocs, cf_channel), - cf_channel, self.managed_properties, ) channels.append(cf_channel) @@ -699,11 +697,10 @@ def _handle_channel_is_old( if last_alias_ioc_info is None: continue alias_channel.owner = last_alias_ioc_info.owner - alias_channel.properties = _merge_property_lists( + alias_channel.properties = alias_channel.merged_properties( create_default_properties( ioc_info, recceiverid, self.channel_ioc_ids, self.iocs, cf_channel ), - alias_channel, self.managed_properties, ) channels.append(alias_channel) @@ -717,12 +714,11 @@ def _orphan_channel( record_info_by_name: Dict[str, RecordInfo], ) -> None: """Channel exists in CF but has no known IOC — mark inactive.""" - cf_channel.properties = _merge_property_lists( + cf_channel.properties = cf_channel.merged_properties( [ CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.INACTIVE.value), CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), - ], - cf_channel, + ] ) channels.append(cf_channel) log.debug("Add orphaned channel %s with no IOC: %s", cf_channel, ioc_info) @@ -730,12 +726,11 @@ def _orphan_channel( if cf_channel.name in record_info_by_name: for alias_name in record_info_by_name[cf_channel.name].aliases: alias_channel = CFChannel(alias_name, "", []) - alias_channel.properties = _merge_property_lists( + alias_channel.properties = alias_channel.merged_properties( [ CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.INACTIVE.value), CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), - ], - alias_channel, + ] ) channels.append(alias_channel) log.debug("Add orphaned alias %s with no IOC: %s", alias_channel, ioc_info) @@ -752,12 +747,11 @@ def _handle_channel_old_and_new( ) -> None: """Channel exists in CF with the same iocid — mark active and update time.""" log.debug("Channel %s exists in Channelfinder with same iocid %s", cf_channel.name, iocid) - cf_channel.properties = _merge_property_lists( + cf_channel.properties = cf_channel.merged_properties( [ CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.ACTIVE.value), CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), ], - cf_channel, self.managed_properties, ) channels.append(cf_channel) @@ -769,24 +763,22 @@ def _handle_channel_old_and_new( for alias_name in record_info_by_name[cf_channel.name].aliases: if alias_name in old_channels: alias_channel = CFChannel(alias_name, "", []) - alias_channel.properties = _merge_property_lists( + alias_channel.properties = alias_channel.merged_properties( [ CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.ACTIVE.value), CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), ], - alias_channel, self.managed_properties, ) channels.append(alias_channel) new_channels.remove(alias_name) else: - aprops = _merge_property_lists( + aprops = cf_channel.merged_properties( [ CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.ACTIVE.value), CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), CFProperty(CFPropertyName.ALIAS.value, ioc_info.owner, cf_channel.name), ], - cf_channel, self.managed_properties, ) channels.append(CFChannel(alias_name, ioc_info.owner, aprops)) @@ -809,11 +801,7 @@ def _update_existing_channel_diff_iocid( ) -> None: """Update a channel that exists in CF but is moving to a new IOC.""" existing_channel = existing_channels[channel_name] - existing_channel.properties = _merge_property_lists( - new_properties, - existing_channel, - self.managed_properties, - ) + existing_channel.properties = existing_channel.merged_properties(new_properties, self.managed_properties) channels.append(existing_channel) log.debug("Add existing channel with different IOC: %s", existing_channel) if self.cf_config.alias_enabled and channel_name in record_info_by_name: @@ -821,7 +809,7 @@ def _update_existing_channel_diff_iocid( for alias_name in record_info_by_name[channel_name].aliases: if alias_name in existing_channels: ach = existing_channels[alias_name] - ach.properties = _merge_property_lists(alias_properties, ach, self.managed_properties) + ach.properties = ach.merged_properties(alias_properties, self.managed_properties) channels.append(ach) else: channels.append(CFChannel(alias_name, ioc_info.owner, alias_properties)) diff --git a/server/tests/unit/cf/test_model.py b/server/tests/unit/cf/test_model.py index 9bfb8699..ebbde708 100644 --- a/server/tests/unit/cf/test_model.py +++ b/server/tests/unit/cf/test_model.py @@ -1,6 +1,6 @@ import pytest -from recceiver.cf.model import CFChannel, CFProperty, CFPropertyName, IOCInfo, PVStatus, _merge_property_lists +from recceiver.cf.model import CFChannel, CFProperty, CFPropertyName, IOCInfo, PVStatus class TestIOCInfo: @@ -44,37 +44,37 @@ class TestMergePropertyLists: def test_new_property_replaces_old_property_by_name(self): channel = CFChannel("PV:1", "admin", [CFProperty("pvStatus", "old-owner", "Active")]) updated = CFProperty("pvStatus", "new-owner", "Inactive") - assert _merge_property_lists([updated], channel, {"pvStatus"}) == [updated] + assert channel.merged_properties([updated], {"pvStatus"}) == [updated] def test_preserves_unmanaged_properties_in_order(self): custom = CFProperty("custom", "user", "value") other = CFProperty("other", "user", "another value") channel = CFChannel("PV:1", "admin", [custom, other]) updated = CFProperty("pvStatus", "admin", "Active") - assert _merge_property_lists([updated], channel, {"pvStatus"}) == [updated, custom, other] + assert channel.merged_properties([updated], {"pvStatus"}) == [updated, custom, other] def test_removes_omitted_managed_properties(self): custom = CFProperty("custom", "user", "value") channel = CFChannel("PV:1", "admin", [CFProperty("archive", "admin", "old value"), custom]) updated = CFProperty("pvStatus", "admin", "Active") - assert _merge_property_lists([updated], channel, {"archive", "pvStatus"}) == [updated, custom] + assert channel.merged_properties([updated], {"archive", "pvStatus"}) == [updated, custom] @pytest.mark.parametrize("managed", [None, set()]) def test_empty_new_properties_preserve_existing_when_nothing_is_managed(self, managed): existing = CFProperty("custom", "user", "value") channel = CFChannel("PV:1", "admin", [existing]) - assert _merge_property_lists([], channel, managed) == [existing] + assert channel.merged_properties([], managed) == [existing] def test_empty_new_properties_remove_all_managed_properties(self): channel = CFChannel("PV:1", "admin", [CFProperty("archive", "admin", "old value")]) - assert _merge_property_lists([], channel, {"archive"}) == [] + assert channel.merged_properties([], {"archive"}) == [] def test_does_not_mutate_either_input_list(self): existing = CFProperty("custom", "user", "value") channel = CFChannel("PV:1", "admin", [existing]) updated = CFProperty("pvStatus", "admin", "Active") new_properties = [updated] - assert _merge_property_lists(new_properties, channel) == [updated, existing] + assert channel.merged_properties(new_properties) == [updated, existing] assert new_properties == [updated] assert channel.properties == [existing] From 6860558f78e701843ce9379c8b4ccc98449f98cc Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Fri, 9 Oct 2026 17:38:23 +0200 Subject: [PATCH 5/8] feat(cf): add channel property object lookup Add CFChannel.property() for canonical and custom names. Reuse it for value and name-existence lookups while preserving exact-property matching. --- server/recceiver/cf/model.py | 12 ++++++++---- server/tests/unit/cf/test_model.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/server/recceiver/cf/model.py b/server/recceiver/cf/model.py index 23142b9f..42b83c34 100644 --- a/server/recceiver/cf/model.py +++ b/server/recceiver/cf/model.py @@ -79,16 +79,20 @@ def has_property(self, prop: Union[CFProperty, CFPropertyName, str]) -> bool: """ if isinstance(prop, CFProperty): return prop in self.properties - property_name = prop.value if isinstance(prop, CFPropertyName) else prop - return any(existing.name == property_name for existing in self.properties) + return self.property(prop) is not None + + def property(self, name: Union[CFPropertyName, str]) -> Optional[CFProperty]: + """Return the first property matching an enum or string name, or None.""" + property_name = name.value if isinstance(name, CFPropertyName) else name + return next((prop for prop in self.properties if prop.name == property_name), None) def property_value(self, name: Union[CFPropertyName, str]) -> Optional[str]: """Return the first matching property's value, or None if absent. Accepts a canonical CFPropertyName or a string for custom properties. """ - property_name = name.value if isinstance(name, CFPropertyName) else name - return next((prop.value for prop in self.properties if prop.name == property_name), None) + prop = self.property(name) + return prop.value if prop is not None else None def as_dict(self) -> Dict[str, Any]: """Serialise to the dict shape expected by pyCFClient.""" diff --git a/server/tests/unit/cf/test_model.py b/server/tests/unit/cf/test_model.py index ebbde708..8858eb71 100644 --- a/server/tests/unit/cf/test_model.py +++ b/server/tests/unit/cf/test_model.py @@ -80,6 +80,27 @@ def test_does_not_mutate_either_input_list(self): class TestCFChannel: + @pytest.mark.parametrize("name", [CFPropertyName.TIME, "time"]) + def test_property_returns_matching_object(self, name): + expected = CFProperty("time", "user", "timestamp") + channel = CFChannel("PV:1", "admin", [CFProperty("pvStatus", "admin", "Active"), expected]) + assert channel.property(name) is expected + + def test_property_accepts_custom_name_and_none_value(self): + expected = CFProperty("custom", "user", None) + channel = CFChannel("PV:1", "admin", [expected]) + assert channel.property("custom") is expected + + @pytest.mark.parametrize("properties", [[], [CFProperty("pvStatus", "admin", "Active")]]) + def test_property_returns_none_when_absent(self, properties): + channel = CFChannel("PV:1", "admin", properties) + assert channel.property(CFPropertyName.TIME) is None + + def test_property_returns_first_match(self): + first = CFProperty("time", "admin", "first") + channel = CFChannel("PV:1", "admin", [first, CFProperty("time", "admin", "second")]) + assert channel.property(CFPropertyName.TIME) is first + @pytest.mark.parametrize("name", [CFPropertyName.TIME, "time"]) @pytest.mark.parametrize("value", ["timestamp", None, ""]) def test_has_property_accepts_enum_or_string_regardless_of_value(self, name, value): From af690ce229c6a69125d5cbb111617488412bdcf9 Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Fri, 9 Oct 2026 17:59:28 +0200 Subject: [PATCH 6/8] refactor(integration): use channel property object lookup in integration tests Replace property-list inspection with CFChannel.property(), property_value(), and has_property(). Keep assertion-strengthening changes separate. --- server/tests/integration/cf_client.py | 6 +++--- server/tests/integration/test_bash_ioc.py | 4 ++-- .../integration/test_multiple_recceiver.py | 20 ++++++------------- server/tests/integration/test_single_ioc.py | 8 ++++---- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/server/tests/integration/cf_client.py b/server/tests/integration/cf_client.py index 84e17f9b..ab2a9257 100644 --- a/server/tests/integration/cf_client.py +++ b/server/tests/integration/cf_client.py @@ -29,7 +29,7 @@ def channel_match( property_names = {name.value if isinstance(name, CFPropertyName) else name for name in properties_to_match} for prop in channel0.properties: if prop.name in property_names: - assert prop in channel1.properties, f"Property {prop} not found in channel {channel1.name}" + assert channel1.has_property(prop), f"Property {prop} not found in channel {channel1.name}" def channels_match( @@ -49,14 +49,14 @@ def check_channel_count(adapter: PyCFClientAdapter, expected_channel_count: int, def check_channel_property(adapter: PyCFClientAdapter, name: str = "*", prop: CFProperty = ACTIVE_PROPERTY) -> bool: channels = adapter.find_by_names([name]) - return all(prop in channel.properties for channel in channels) + return all(channel.has_property(prop) for channel in channels) def find_ioc_channels(adapter: PyCFClientAdapter, ioc_name: str) -> list[CFChannel]: return [ channel for channel in adapter.find_by_names(["*"]) - if any(prop.name == CFPropertyName.IOC_NAME.value and prop.value == ioc_name for prop in channel.properties) + if channel.property_value(CFPropertyName.IOC_NAME) == ioc_name ] diff --git a/server/tests/integration/test_bash_ioc.py b/server/tests/integration/test_bash_ioc.py index 0a607ad0..97cdffec 100644 --- a/server/tests/integration/test_bash_ioc.py +++ b/server/tests/integration/test_bash_ioc.py @@ -76,14 +76,14 @@ def test_remove_infotag(self, setup_compose: DockerCompose) -> None: cf_adapter = create_adapter_and_wait(setup_compose, expected_channel_count=1) info_tag = CFProperty("archive", "admin", "testing") channels = cf_adapter.find_by_names([DEFAULT_CHANNEL_NAME]) - assert any(info_tag in channel.properties for channel in channels), ( + assert any(channel.property(info_tag.name) == info_tag for channel in channels), ( "Info tag 'archive' not found before removal" ) restart_ioc(docker_ioc, cf_adapter, DEFAULT_CHANNEL_NAME, "test_remove_infotag_after.db") channels = cf_adapter.find_by_names([DEFAULT_CHANNEL_NAME]) - assert all(info_tag not in channel.properties for channel in channels), ( + assert all(not channel.has_property(info_tag) for channel in channels), ( "Info tag 'archive' still found in channel after removal" ) diff --git a/server/tests/integration/test_multiple_recceiver.py b/server/tests/integration/test_multiple_recceiver.py index 438a3a43..038ed54b 100644 --- a/server/tests/integration/test_multiple_recceiver.py +++ b/server/tests/integration/test_multiple_recceiver.py @@ -33,31 +33,23 @@ def test_number_of_channels_and_channel_name(self, cf_adapter: PyCFClientAdapter assert channels[0].name == DEFAULT_CHANNEL_NAME def test_number_of_aliases_and_alais_property(self, cf_adapter: PyCFClientAdapter) -> None: - aliases = [ - channel - for channel in cf_adapter.find_by_names(["*"]) - if any(prop.name == CFPropertyName.ALIAS.value for prop in channel.properties) - ] + aliases = [channel for channel in cf_adapter.find_by_names(["*"]) if channel.has_property(CFPropertyName.ALIAS)] assert len(aliases) == IOC_COUNT * BASE_ALIAS_COUNT assert aliases[0].name == DEFAULT_CHANNEL_NAME + ":alias" - assert CFProperty(CFPropertyName.ALIAS.value, "admin", DEFAULT_CHANNEL_NAME) in aliases[0].properties + assert aliases[0].has_property(CFProperty(CFPropertyName.ALIAS.value, "admin", DEFAULT_CHANNEL_NAME)) def test_number_of_record_desc_and_property(self, cf_adapter: PyCFClientAdapter) -> None: channels = [ - channel - for channel in cf_adapter.find_by_names(["*"]) - if any(prop.name == CFPropertyName.RECORD_DESC.value for prop in channel.properties) + channel for channel in cf_adapter.find_by_names(["*"]) if channel.has_property(CFPropertyName.RECORD_DESC) ] assert len(channels) == EXPECTED_DEFAULT_CHANNEL_COUNT expected = CFProperty(CFPropertyName.RECORD_DESC.value, "admin", "testdesc") - assert expected in channels[0].properties + assert channels[0].has_property(expected) def test_number_of_record_type_and_property(self, cf_adapter: PyCFClientAdapter) -> None: channels = [ - channel - for channel in cf_adapter.find_by_names(["*"]) - if any(prop.name == CFPropertyName.RECORD_TYPE.value for prop in channel.properties) + channel for channel in cf_adapter.find_by_names(["*"]) if channel.has_property(CFPropertyName.RECORD_TYPE) ] assert len(channels) == EXPECTED_DEFAULT_CHANNEL_COUNT expected = CFProperty(CFPropertyName.RECORD_TYPE.value, "admin", "ai") - assert expected in channels[0].properties + assert channels[0].has_property(expected) diff --git a/server/tests/integration/test_single_ioc.py b/server/tests/integration/test_single_ioc.py index 41a30b4a..6545fbae 100644 --- a/server/tests/integration/test_single_ioc.py +++ b/server/tests/integration/test_single_ioc.py @@ -101,7 +101,7 @@ def test_status_property_works_after_cf_restart( lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) channels_inactive = find_ioc_channels(refreshed_adapter, "IOC1-1") - assert all(INACTIVE_PROPERTY in channel.properties for channel in channels_inactive) + assert all(channel.has_property(INACTIVE_PROPERTY) for channel in channels_inactive) class TestShutdownChannelFinder: @@ -124,7 +124,7 @@ def test_status_property_works_between_cf_down( lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) channels_inactive = find_ioc_channels(refreshed_adapter, "IOC1-1") - assert all(INACTIVE_PROPERTY in channel.properties for channel in channels_inactive) + assert all(channel.has_property(INACTIVE_PROPERTY) for channel in channels_inactive) class TestCleanStopRecceiver: @@ -137,7 +137,7 @@ def test_clean_stop_marks_channels_inactive( lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) channels_inactive = find_ioc_channels(cf_adapter, "IOC1-1") - assert all(INACTIVE_PROPERTY in channel.properties for channel in channels_inactive) + assert all(channel.has_property(INACTIVE_PROPERTY) for channel in channels_inactive) class TestCleanStartRecceiver: @@ -153,7 +153,7 @@ def test_startup_sweep_marks_stale_channels_inactive( lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) channels_inactive = find_ioc_channels(cf_adapter, "IOC1-1") - assert all(INACTIVE_PROPERTY in channel.properties for channel in channels_inactive) + assert all(channel.has_property(INACTIVE_PROPERTY) for channel in channels_inactive) class TestMoveIocHost: From 9db3b08c88c69e892f3da8e81ae44447aaa1c511 Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Fri, 9 Oct 2026 17:30:57 +0200 Subject: [PATCH 7/8] refactor(cf): build standard properties through the IOC model Add IOCInfo.channel_properties() and status_properties() and use them for channel creation, active updates, orphaning, and aliases. Preserve reassignment semantics with a non-mutating IOC copy: previous identity, current owner and timestamp. Cover property ordering, fresh objects, and unchanged source IOC state. --- server/recceiver/cf/model.py | 20 ++++++++ server/recceiver/cf/processor.py | 69 ++++---------------------- server/tests/unit/cf/test_model.py | 34 ++++++++++++- server/tests/unit/cf/test_processor.py | 28 ++++++++++- 4 files changed, 89 insertions(+), 62 deletions(-) diff --git a/server/recceiver/cf/model.py b/server/recceiver/cf/model.py index 42b83c34..e28acf1a 100644 --- a/server/recceiver/cf/model.py +++ b/server/recceiver/cf/model.py @@ -129,6 +129,26 @@ class IOCInfo: def id(self) -> str: return f"{self.host}:{self.port}" + def status_properties(self, status: PVStatus) -> List[CFProperty]: + """Build status and timestamp properties using this IOC's owner.""" + return [ + CFProperty(CFPropertyName.PV_STATUS.value, self.owner, status.value), + CFProperty(CFPropertyName.TIME.value, self.owner, self.time), + ] + + def channel_properties(self, recceiverid: str) -> List[CFProperty]: + """Build the standard Active channel properties for this IOC.""" + return ( + [ + CFProperty(CFPropertyName.HOSTNAME.value, self.owner, self.hostname), + CFProperty(CFPropertyName.IOC_NAME.value, self.owner, self.ioc_name), + CFProperty(CFPropertyName.IOC_ID.value, self.owner, self.id), + CFProperty(CFPropertyName.IOC_IP.value, self.owner, self.ioc_ip), + ] + + self.status_properties(PVStatus.ACTIVE) + + [CFProperty(CFPropertyName.RECCEIVER_ID.value, self.owner, recceiverid)] + ) + @dataclass class RecordInfo: diff --git a/server/recceiver/cf/processor.py b/server/recceiver/cf/processor.py index eb545d53..0ab836da 100755 --- a/server/recceiver/cf/processor.py +++ b/server/recceiver/cf/processor.py @@ -2,6 +2,7 @@ import logging import time from collections import defaultdict +from dataclasses import replace from typing import Callable, Dict, List, Optional, Set from channelfinder import ChannelFinderClient @@ -593,15 +594,7 @@ def _process_new_channels( genuinely new channels are created fresh. Appends results to channels. """ for channel_name in new_channels: - new_properties = create_ioc_properties( - ioc_info.owner, - ioc_info.time, - recceiverid, - ioc_info.hostname, - ioc_info.ioc_name, - ioc_info.ioc_ip, - ioc_info.id, - ) + new_properties = ioc_info.channel_properties(recceiverid) record_info = record_info_by_name.get(channel_name) if record_info: if self.cf_config.record_type_enabled and record_info.record_type: @@ -714,12 +707,7 @@ def _orphan_channel( record_info_by_name: Dict[str, RecordInfo], ) -> None: """Channel exists in CF but has no known IOC — mark inactive.""" - cf_channel.properties = cf_channel.merged_properties( - [ - CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.INACTIVE.value), - CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), - ] - ) + cf_channel.properties = cf_channel.merged_properties(ioc_info.status_properties(PVStatus.INACTIVE)) channels.append(cf_channel) log.debug("Add orphaned channel %s with no IOC: %s", cf_channel, ioc_info) if self.cf_config.alias_enabled: @@ -727,10 +715,7 @@ def _orphan_channel( for alias_name in record_info_by_name[cf_channel.name].aliases: alias_channel = CFChannel(alias_name, "", []) alias_channel.properties = alias_channel.merged_properties( - [ - CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.INACTIVE.value), - CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), - ] + ioc_info.status_properties(PVStatus.INACTIVE) ) channels.append(alias_channel) log.debug("Add orphaned alias %s with no IOC: %s", alias_channel, ioc_info) @@ -748,11 +733,7 @@ def _handle_channel_old_and_new( """Channel exists in CF with the same iocid — mark active and update time.""" log.debug("Channel %s exists in Channelfinder with same iocid %s", cf_channel.name, iocid) cf_channel.properties = cf_channel.merged_properties( - [ - CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.ACTIVE.value), - CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), - ], - self.managed_properties, + ioc_info.status_properties(PVStatus.ACTIVE), self.managed_properties ) channels.append(cf_channel) log.debug("Add existing channel with same IOC: %s", cf_channel) @@ -764,21 +745,14 @@ def _handle_channel_old_and_new( if alias_name in old_channels: alias_channel = CFChannel(alias_name, "", []) alias_channel.properties = alias_channel.merged_properties( - [ - CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.ACTIVE.value), - CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), - ], - self.managed_properties, + ioc_info.status_properties(PVStatus.ACTIVE), self.managed_properties ) channels.append(alias_channel) new_channels.remove(alias_name) else: aprops = cf_channel.merged_properties( - [ - CFProperty(CFPropertyName.PV_STATUS.value, ioc_info.owner, PVStatus.ACTIVE.value), - CFProperty(CFPropertyName.TIME.value, ioc_info.owner, ioc_info.time), - CFProperty(CFPropertyName.ALIAS.value, ioc_info.owner, cf_channel.name), - ], + ioc_info.status_properties(PVStatus.ACTIVE) + + [CFProperty(CFPropertyName.ALIAS.value, ioc_info.owner, cf_channel.name)], self.managed_properties, ) channels.append(CFChannel(alias_name, ioc_info.owner, aprops)) @@ -832,21 +806,6 @@ def _create_new_channel( log.debug("Add new alias: %s from %s", alias, channel_name) -def create_ioc_properties( - owner: str, ioc_time: str, recceiverid: str, host_name: str, ioc_name: str, ioc_ip: str, iocid: str -) -> List[CFProperty]: - """Build the standard set of IOC-level CF properties for a channel.""" - return [ - CFProperty(CFPropertyName.HOSTNAME.value, owner, host_name), - CFProperty(CFPropertyName.IOC_NAME.value, owner, ioc_name), - CFProperty(CFPropertyName.IOC_ID.value, owner, iocid), - CFProperty(CFPropertyName.IOC_IP.value, owner, ioc_ip), - CFProperty(CFPropertyName.PV_STATUS.value, owner, PVStatus.ACTIVE.value), - CFProperty(CFPropertyName.TIME.value, owner, ioc_time), - CFProperty(CFPropertyName.RECCEIVER_ID.value, owner, recceiverid), - ] - - def create_default_properties( ioc_info: IOCInfo, recceiverid: str, @@ -854,18 +813,10 @@ def create_default_properties( iocs: Dict[str, IOCInfo], cf_channel: CFChannel, ) -> List[CFProperty]: - """Build IOC properties using the last known IOC for a channel.""" + """Use the last IOC's identity with the current transaction's owner and time.""" channel_name = cf_channel.name last_ioc_info = iocs[channels_iocs[channel_name][-1]] - return create_ioc_properties( - ioc_info.owner, - ioc_info.time, - recceiverid, - last_ioc_info.hostname, - last_ioc_info.ioc_name, - last_ioc_info.ioc_ip, - last_ioc_info.id, - ) + return replace(last_ioc_info, owner=ioc_info.owner, time=ioc_info.time).channel_properties(recceiverid) def get_current_time(timezone: Optional[str] = None) -> str: diff --git a/server/tests/unit/cf/test_model.py b/server/tests/unit/cf/test_model.py index 8858eb71..8455ec86 100644 --- a/server/tests/unit/cf/test_model.py +++ b/server/tests/unit/cf/test_model.py @@ -4,10 +4,40 @@ class TestIOCInfo: - def test_id_combines_host_and_port(self): - ioc = IOCInfo(host="1.2.3.4", hostname="h", ioc_name="n", ioc_ip="1.2.3.4", owner="o", time="t", port=5064) + @pytest.fixture + def ioc(self): + return IOCInfo(host="1.2.3.4", hostname="h", ioc_name="n", ioc_ip="1.2.3.4", owner="o", time="t", port=5064) + + def test_id_combines_host_and_port(self, ioc): assert ioc.id == "1.2.3.4:5064" + def test_channel_properties_include_identity_status_time_and_receiver_in_order(self, ioc): + assert ioc.channel_properties("receiver") == [ + CFProperty("hostName", "o", "h"), + CFProperty("iocName", "o", "n"), + CFProperty("iocid", "o", "1.2.3.4:5064"), + CFProperty("iocIP", "o", "1.2.3.4"), + CFProperty("pvStatus", "o", "Active"), + CFProperty("time", "o", "t"), + CFProperty("recceiverID", "o", "receiver"), + ] + + @pytest.mark.parametrize("status", [PVStatus.ACTIVE, PVStatus.INACTIVE]) + def test_status_properties_use_ioc_owner_and_timestamp(self, ioc, status): + assert ioc.status_properties(status) == [ + CFProperty("pvStatus", "o", status.value), + CFProperty("time", "o", "t"), + ] + + def test_channel_properties_return_fresh_objects_without_mutating_ioc(self, ioc): + first = ioc.channel_properties("receiver") + first[0].value = "changed" + first.pop() + second = ioc.channel_properties("receiver") + assert len(second) == 7 + assert second[0] == CFProperty("hostName", "o", "h") + assert ioc.hostname == "h" + class TestPVStatus: def test_active_value(self): diff --git a/server/tests/unit/cf/test_processor.py b/server/tests/unit/cf/test_processor.py index 2d29f362..ee6ce8c3 100644 --- a/server/tests/unit/cf/test_processor.py +++ b/server/tests/unit/cf/test_processor.py @@ -1,4 +1,5 @@ import time +from dataclasses import replace import pytest from requests import RequestException @@ -6,7 +7,7 @@ from twisted.internet.address import IPv4Address from recceiver.cf.model import CFChannel, CFProperty, CFPropertyName, PVStatus, RecordInfo -from recceiver.cf.processor import CFProcessor +from recceiver.cf.processor import CFProcessor, create_default_properties from recceiver.recast import Transaction from tests.unit.cf.conftest import DEFAULT_RECCEIVER_ID, make_channel, make_ioc from tests.unit.cf.mock_adapter import MockCFAdapter @@ -33,6 +34,31 @@ def make_processor_with_mock(): return proc, adapter +class TestDefaultProperties: + def test_uses_last_ioc_identity_but_current_owner_and_time_without_mutating_iocs(self): + previous = make_ioc() + current = replace(previous, host="current-host", hostname="current-name", owner="current-owner", time="now") + previous_snapshot = replace(previous) + current_snapshot = replace(current) + channel = CFChannel("PV:1", "admin", []) + + properties = create_default_properties( + current, DEFAULT_RECCEIVER_ID, {channel.name: [current.id, previous.id]}, {previous.id: previous}, channel + ) + + assert properties == [ + CFProperty("hostName", "current-owner", previous.hostname), + CFProperty("iocName", "current-owner", previous.ioc_name), + CFProperty("iocid", "current-owner", previous.id), + CFProperty("iocIP", "current-owner", previous.ioc_ip), + CFProperty("pvStatus", "current-owner", "Active"), + CFProperty("time", "current-owner", "now"), + CFProperty("recceiverID", "current-owner", DEFAULT_RECCEIVER_ID), + ] + assert previous == previous_snapshot + assert current == current_snapshot + + class TestRemoveChannel: def test_missing_iocid_does_not_raise(self): proc = make_processor() From 28a7753714936fecaa2f952d533eebb01b67753b Mon Sep 17 00:00:00 2001 From: Sky Brewer Date: Fri, 9 Oct 2026 17:59:30 +0200 Subject: [PATCH 8/8] test(integration): strengthen channel and property assertions Compare channels by name rather than response order and reject empty-result property checks. Write the manual property before testing its preservation, assert removal outcomes, and check all relevant channels. Add unit coverage for typed integration helpers and the adapter factory boundary. --- server/tests/integration/cf_client.py | 10 +- server/tests/integration/test_bash_ioc.py | 30 +++-- .../integration/test_multiple_recceiver.py | 17 ++- server/tests/integration/test_single_ioc.py | 15 ++- .../tests/unit/test_integration_cf_helpers.py | 123 ++++++++++++++++++ 5 files changed, 168 insertions(+), 27 deletions(-) create mode 100644 server/tests/unit/test_integration_cf_helpers.py diff --git a/server/tests/integration/cf_client.py b/server/tests/integration/cf_client.py index ab2a9257..77f3a15e 100644 --- a/server/tests/integration/cf_client.py +++ b/server/tests/integration/cf_client.py @@ -37,8 +37,12 @@ def channels_match( channels_end: list[CFChannel], properties_to_match: Sequence[Union[CFPropertyName, str]], ) -> None: - for index, channel in enumerate(channels_begin): - channel_match(channel, channels_end[index], properties_to_match) + beginning = {channel.name: channel for channel in channels_begin} + ending = {channel.name: channel for channel in channels_end} + assert len(channels_begin) == len(channels_end) + assert beginning.keys() == ending.keys() + for name, channel in beginning.items(): + channel_match(channel, ending[name], properties_to_match) def check_channel_count(adapter: PyCFClientAdapter, expected_channel_count: int, name: str = "*") -> bool: @@ -49,7 +53,7 @@ def check_channel_count(adapter: PyCFClientAdapter, expected_channel_count: int, def check_channel_property(adapter: PyCFClientAdapter, name: str = "*", prop: CFProperty = ACTIVE_PROPERTY) -> bool: channels = adapter.find_by_names([name]) - return all(channel.has_property(prop) for channel in channels) + return bool(channels) and all(channel.has_property(prop) for channel in channels) def find_ioc_channels(adapter: PyCFClientAdapter, ioc_name: str) -> list[CFChannel]: diff --git a/server/tests/integration/test_bash_ioc.py b/server/tests/integration/test_bash_ioc.py index 97cdffec..259f108d 100644 --- a/server/tests/integration/test_bash_ioc.py +++ b/server/tests/integration/test_bash_ioc.py @@ -75,15 +75,15 @@ def test_remove_infotag(self, setup_compose: DockerCompose) -> None: docker_ioc = start_ioc(setup_compose, db_file="test_remove_infotag_before.db") cf_adapter = create_adapter_and_wait(setup_compose, expected_channel_count=1) info_tag = CFProperty("archive", "admin", "testing") - channels = cf_adapter.find_by_names([DEFAULT_CHANNEL_NAME]) - assert any(channel.property(info_tag.name) == info_tag for channel in channels), ( - "Info tag 'archive' not found before removal" + assert wait_for_sync( + cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, info_tag) ) restart_ioc(docker_ioc, cf_adapter, DEFAULT_CHANNEL_NAME, "test_remove_infotag_after.db") channels = cf_adapter.find_by_names([DEFAULT_CHANNEL_NAME]) - assert all(not channel.has_property(info_tag) for channel in channels), ( + assert channels + assert all(not channel.has_property(info_tag.name) for channel in channels), ( "Info tag 'archive' still found in channel after removal" ) @@ -94,13 +94,16 @@ def test_remove_channel(self, setup_compose: DockerCompose) -> None: # noqa: F8 docker_ioc = start_ioc(setup_compose, db_file="test_remove_channel_before.db") cf_adapter = create_adapter_and_wait(setup_compose, expected_channel_count=2) second_channel_name = f"{DEFAULT_CHANNEL_NAME}-2" - check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) - check_channel_property(cf_adapter, name=second_channel_name) + assert check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) + assert check_channel_property(cf_adapter, name=second_channel_name) restart_ioc(docker_ioc, cf_adapter, DEFAULT_CHANNEL_NAME, "test_remove_channel_after.db") - check_channel_property(cf_adapter, name=second_channel_name, prop=INACTIVE_PROPERTY) - check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) + assert wait_for_sync( + cf_adapter, + lambda adapter: check_channel_property(adapter, name=second_channel_name, prop=INACTIVE_PROPERTY), + ) + assert check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) class TestRemoveAlias: @@ -109,10 +112,13 @@ def test_remove_alias(self, setup_compose: DockerCompose) -> None: # noqa: F811 docker_ioc = start_ioc(setup_compose) cf_adapter = create_adapter_and_wait(setup_compose, expected_channel_count=BASE_IOC_CHANNEL_COUNT) channel_alias_name = f"{DEFAULT_CHANNEL_NAME}:alias" - check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) - check_channel_property(cf_adapter, name=channel_alias_name) + assert check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) + assert check_channel_property(cf_adapter, name=channel_alias_name) restart_ioc(docker_ioc, cf_adapter, DEFAULT_CHANNEL_NAME, "test_remove_alias_after.db") - check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) - check_channel_property(cf_adapter, name=channel_alias_name, prop=INACTIVE_PROPERTY) + assert check_channel_property(cf_adapter, name=DEFAULT_CHANNEL_NAME) + assert wait_for_sync( + cf_adapter, + lambda adapter: check_channel_property(adapter, name=channel_alias_name, prop=INACTIVE_PROPERTY), + ) diff --git a/server/tests/integration/test_multiple_recceiver.py b/server/tests/integration/test_multiple_recceiver.py index 038ed54b..fc905dd1 100644 --- a/server/tests/integration/test_multiple_recceiver.py +++ b/server/tests/integration/test_multiple_recceiver.py @@ -30,13 +30,18 @@ class TestMultipleRecceiver: def test_number_of_channels_and_channel_name(self, cf_adapter: PyCFClientAdapter) -> None: channels = cf_adapter.find_by_names(["*"]) assert len(channels) == EXPECTED_DEFAULT_CHANNEL_COUNT - assert channels[0].name == DEFAULT_CHANNEL_NAME + assert DEFAULT_CHANNEL_NAME in {channel.name for channel in channels} def test_number_of_aliases_and_alais_property(self, cf_adapter: PyCFClientAdapter) -> None: - aliases = [channel for channel in cf_adapter.find_by_names(["*"]) if channel.has_property(CFPropertyName.ALIAS)] + aliases = { + channel.name: channel + for channel in cf_adapter.find_by_names(["*"]) + if channel.has_property(CFPropertyName.ALIAS) + } assert len(aliases) == IOC_COUNT * BASE_ALIAS_COUNT - assert aliases[0].name == DEFAULT_CHANNEL_NAME + ":alias" - assert aliases[0].has_property(CFProperty(CFPropertyName.ALIAS.value, "admin", DEFAULT_CHANNEL_NAME)) + alias_name = DEFAULT_CHANNEL_NAME + ":alias" + assert alias_name in aliases + assert aliases[alias_name].has_property(CFProperty(CFPropertyName.ALIAS.value, "admin", DEFAULT_CHANNEL_NAME)) def test_number_of_record_desc_and_property(self, cf_adapter: PyCFClientAdapter) -> None: channels = [ @@ -44,7 +49,7 @@ def test_number_of_record_desc_and_property(self, cf_adapter: PyCFClientAdapter) ] assert len(channels) == EXPECTED_DEFAULT_CHANNEL_COUNT expected = CFProperty(CFPropertyName.RECORD_DESC.value, "admin", "testdesc") - assert channels[0].has_property(expected) + assert all(channel.has_property(expected) for channel in channels) def test_number_of_record_type_and_property(self, cf_adapter: PyCFClientAdapter) -> None: channels = [ @@ -52,4 +57,4 @@ def test_number_of_record_type_and_property(self, cf_adapter: PyCFClientAdapter) ] assert len(channels) == EXPECTED_DEFAULT_CHANNEL_COUNT expected = CFProperty(CFPropertyName.RECORD_TYPE.value, "admin", "ai") - assert channels[0].has_property(expected) + assert all(channel.has_property(expected) for channel in channels) diff --git a/server/tests/integration/test_single_ioc.py b/server/tests/integration/test_single_ioc.py index 6545fbae..033cf33a 100644 --- a/server/tests/integration/test_single_ioc.py +++ b/server/tests/integration/test_single_ioc.py @@ -56,7 +56,6 @@ def test_channels_same_after_restart(self, setup_compose: DockerCompose, cf_adap restart_container(setup_compose, "ioc1-1") assert wait_for_sync(cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME)) channels_end = cf_adapter.find_by_names(["*"]) - assert len(channels_begin) == len(channels_end) channels_match(channels_begin, channels_end, PROPERTIES_TO_MATCH) def test_manual_channels_same_after_restart( @@ -66,14 +65,14 @@ def test_manual_channels_same_after_restart( ) -> None: test_property = CFProperty("test_property", "testowner", "test_value") cf_adapter.set_property(test_property.name, test_property.owner) - channels = cf_adapter.find_by_names([DEFAULT_CHANNEL_NAME]) - channels[0].properties = [test_property] - cf_adapter.set_property(test_property.name, test_property.owner) + cf_adapter.update_property(test_property, [DEFAULT_CHANNEL_NAME]) + assert wait_for_sync( + cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, test_property) + ) channels_begin = cf_adapter.find_by_names(["*"]) restart_container(setup_compose, "ioc1-1") assert wait_for_sync(cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME)) channels_end = cf_adapter.find_by_names(["*"]) - assert len(channels_begin) == len(channels_end) channels_match(channels_begin, channels_end, PROPERTIES_TO_MATCH + [test_property.name]) @@ -101,6 +100,7 @@ def test_status_property_works_after_cf_restart( lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) channels_inactive = find_ioc_channels(refreshed_adapter, "IOC1-1") + assert channels_inactive assert all(channel.has_property(INACTIVE_PROPERTY) for channel in channels_inactive) @@ -124,6 +124,7 @@ def test_status_property_works_between_cf_down( lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) channels_inactive = find_ioc_channels(refreshed_adapter, "IOC1-1") + assert channels_inactive assert all(channel.has_property(INACTIVE_PROPERTY) for channel in channels_inactive) @@ -137,6 +138,7 @@ def test_clean_stop_marks_channels_inactive( lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) channels_inactive = find_ioc_channels(cf_adapter, "IOC1-1") + assert channels_inactive assert all(channel.has_property(INACTIVE_PROPERTY) for channel in channels_inactive) @@ -153,6 +155,7 @@ def test_startup_sweep_marks_stale_channels_inactive( lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME, INACTIVE_PROPERTY), ) channels_inactive = find_ioc_channels(cf_adapter, "IOC1-1") + assert channels_inactive assert all(channel.has_property(INACTIVE_PROPERTY) for channel in channels_inactive) @@ -164,6 +167,6 @@ def test_move_ioc_host( ) -> None: channels_begin = cf_adapter.find_by_names(["*"]) clone_container(setup_compose, "ioc1-1-new", host_name="ioc1-1") - wait_for_sync(cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME)) + assert wait_for_sync(cf_adapter, lambda adapter: check_channel_property(adapter, DEFAULT_CHANNEL_NAME)) channels_end = cf_adapter.find_by_names(["*"]) assert len(channels_begin) == len(channels_end) diff --git a/server/tests/unit/test_integration_cf_helpers.py b/server/tests/unit/test_integration_cf_helpers.py new file mode 100644 index 00000000..366c7168 --- /dev/null +++ b/server/tests/unit/test_integration_cf_helpers.py @@ -0,0 +1,123 @@ +from unittest.mock import Mock + +import pytest + +from recceiver.cf.adapter import PyCFClientAdapter +from recceiver.cf.model import CFChannel, CFProperty, CFPropertyName +from tests.integration.cf_client import ( + ACTIVE_PROPERTY, + INACTIVE_PROPERTY, + channels_match, + check_channel_count, + check_channel_property, + create_adapter_from_compose, + find_ioc_channels, +) + + +@pytest.fixture +def adapter(): + client = Mock() + client.findByArgs.return_value = [] + return PyCFClientAdapter(client), client + + +def test_property_check_rejects_missing_channels(adapter): + cf_adapter, _ = adapter + assert not check_channel_property(cf_adapter, "MISSING:PV") + + +def test_property_check_uses_typed_properties_and_ignores_api_metadata(adapter): + cf_adapter, client = adapter + channel = CFChannel("PV:1", "admin", [ACTIVE_PROPERTY]) + payload = channel.as_dict() + payload["properties"][0]["channels"] = [] + client.findByArgs.return_value = [payload] + assert check_channel_property(cf_adapter, "PV:1") + client.findByArgs.assert_called_once_with([("~name", "PV:1")]) + + +@pytest.mark.parametrize( + "prop", + [INACTIVE_PROPERTY, CFProperty("pvStatus", "other-owner", "Active"), CFProperty("other", "admin", "Active")], +) +def test_property_check_requires_matching_name_owner_and_value(adapter, prop): + cf_adapter, client = adapter + client.findByArgs.return_value = [CFChannel("PV:1", "admin", [prop]).as_dict()] + assert not check_channel_property(cf_adapter, "PV:1") + + +def test_property_check_requires_all_matching_channels_to_have_property(adapter): + cf_adapter, client = adapter + client.findByArgs.return_value = [ + CFChannel("PV:1", "admin", [ACTIVE_PROPERTY]).as_dict(), + CFChannel("PV:2", "admin", [INACTIVE_PROPERTY]).as_dict(), + ] + assert not check_channel_property(cf_adapter) + + +def test_channel_count_uses_adapter_name_query(adapter): + cf_adapter, client = adapter + client.findByArgs.return_value = [CFChannel("PV:1", "admin", []).as_dict()] + assert check_channel_count(cf_adapter, 1) + assert not check_channel_count(cf_adapter, 2) + assert client.findByArgs.call_args.args == ([("~name", "*")],) + + +@pytest.mark.parametrize("property_name", [CFPropertyName.PV_STATUS, "pvStatus"]) +def test_channel_matching_is_independent_of_result_order(property_name): + first = CFChannel("PV:1", "admin", [ACTIVE_PROPERTY]) + second = CFChannel("PV:2", "admin", [INACTIVE_PROPERTY]) + channels_match([first, second], [second, first], [property_name]) + + +def test_channel_matching_rejects_different_names(): + with pytest.raises(AssertionError): + channels_match([CFChannel("PV:1", "admin", [])], [CFChannel("PV:2", "admin", [])], []) + + +def test_channel_matching_rejects_different_owners(): + with pytest.raises(AssertionError): + channels_match([CFChannel("PV:1", "admin", [])], [CFChannel("PV:1", "other", [])], []) + + +def test_channel_matching_rejects_changed_selected_properties(): + with pytest.raises(AssertionError): + channels_match( + [CFChannel("PV:1", "admin", [ACTIVE_PROPERTY])], + [CFChannel("PV:1", "admin", [INACTIVE_PROPERTY])], + [CFPropertyName.PV_STATUS], + ) + + +def test_channel_matching_ignores_unselected_timestamp_changes(): + channels_match( + [CFChannel("PV:1", "admin", [ACTIVE_PROPERTY, CFProperty("time", "admin", "before")])], + [CFChannel("PV:1", "admin", [ACTIVE_PROPERTY, CFProperty("time", "admin", "after")])], + [CFPropertyName.PV_STATUS], + ) + + +def test_ioc_filter_uses_model_property_lookup(adapter): + cf_adapter, client = adapter + record = CFChannel("PV:1", "admin", [CFProperty("iocName", "admin", "IOC1")]) + alias = CFChannel("PV:1:alias", "admin", [CFProperty("iocName", "admin", "IOC1")]) + other = CFChannel("PV:2", "admin", [CFProperty("iocName", "admin", "IOC2")]) + missing = CFChannel("PV:3", "admin", []) + client.findByArgs.return_value = [channel.as_dict() for channel in [record, alias, other, missing]] + assert find_ioc_channels(cf_adapter, "IOC1") == [record, alias] + + +def test_factory_wraps_raw_client_at_the_boundary(monkeypatch): + client_factory = Mock() + monkeypatch.setattr("tests.integration.cf_client.ChannelFinderClient", client_factory) + compose = Mock() + compose.get_service_host_and_port.return_value = ("", 12345) + cf_adapter = create_adapter_from_compose(compose) + assert isinstance(cf_adapter, PyCFClientAdapter) + client_factory.assert_called_once_with( + BaseURL="http://localhost:12345/ChannelFinder", username="admin", password="password" + ) + client_factory.return_value.findByArgs.return_value = [] + assert cf_adapter.find_by_names(["*"]) == [] + client_factory.return_value.findByArgs.assert_called_once_with([("~name", "*"), ("~size", 10000)])