diff --git a/.cursor/plans/purely_async_er2er_f2b9c60a.plan.md b/.cursor/plans/purely_async_er2er_f2b9c60a.plan.md new file mode 100644 index 0000000..f24800c --- /dev/null +++ b/.cursor/plans/purely_async_er2er_f2b9c60a.plan.md @@ -0,0 +1,68 @@ +--- +name: Purely async er2er +overview: "Remove the sync fallback so the connector uses only the async path: require AsyncERClient, drop asyncio.to_thread for client build and run_sync, and optionally remove now-unused sync-only code from er2er_syncher." +todos: [] +isProject: false +--- + +# Purely async er2er + +## Current flow + +```mermaid +flowchart LR + subgraph main [er2er_main extract] + A{ASYNC_ER_CLIENT_AVAILABLE?} + A -->|yes| B[build_async_er_clients] + A -->|no| C[asyncio.to_thread build_er_clients] + B --> D[async_run_sync] + C --> D + D --> E{Both AsyncERClient?} + E -->|yes| F[_async_run_sync_impl] + E -->|no| G[asyncio.to_thread run_sync] + end +``` + + + +After the change: only the top/left path (async clients + `_async_run_sync_impl`). No `to_thread`, no sync fallback. + +## 1. Require AsyncERClient in [er2er_main.py](cdip-integrations/er2er/er2er_main.py) + +- **Remove** the `else` branch (lines 91–99) that calls `asyncio.to_thread(build_er_clients, ...)`. +- **Require async:** When `ASYNC_ER_CLIENT_AVAILABLE` is false, raise a clear error before building clients (e.g. `ER2ERSync_ConfigError` or `RuntimeError`) with a message that AsyncERClient is required (e.g. "er-client async support is required; AsyncERClient could not be imported"). +- **Remove** the `build_er_clients` import from `er2er_syncher`. +- **Keep** the `if ASYNC_ER_CLIENT_AVAILABLE:` block but change it to: always call `build_async_er_clients`; if the flag is false, raise instead of falling back. + +Result: connector only builds async clients and fails fast if async is unavailable. + +## 2. Require AsyncERClient in [er2er_syncher.py](cdip-integrations/er2er/er2er_syncher.py) `async_run_sync` + +- **Remove** the fallback that runs `run_sync` via `asyncio.to_thread` (lines 1500–1508). +- **Require async clients:** If either client is not an `AsyncERClient` (or `AsyncERClient` is None), raise a clear error (e.g. `TypeError` or `ER2ERSync_ConfigError`) stating that both clients must be AsyncERClient instances. +- **Update** the `async_run_sync` docstring to say it requires AsyncERClient instances and no longer mentions a thread-pool fallback. + +Result: `async_run_sync` is purely async; no `to_thread`. + +## 3. Optional: remove sync-only code from er2er_syncher + +After (1) and (2), nothing in this repo calls `run_sync` or `build_er_clients`. [event_type_copy.py](cdip-integrations/er2er/event_type_copy.py) uses its own `ER2ER_Syncher` and sync `ERClient`; it does not import `run_sync` or `build_er_clients`. + +You can either: + +- **Keep** `run_sync`, `build_er_clients`, and the sync helpers (e.g. for a future CLI or tests), or +- **Remove** them as dead code: `run_sync`, `build_er_clients`, and the sync-only helpers used only by `run_sync`: `get_events_since_io`, `get_event_types_io`, `get_event_categories_io`, `sync_event_categories_io`, `sync_event_types_io`, `sync_events_io`, `copy_files_between_events_io`, `remove_files_from_event_io`, `create_dest_event_io`, `update_dest_event_io`. Pure helpers and shared types (e.g. `clean_event`, `SyncConfig`, `SyncResult`, `build_dest_event_sources`) stay; they are used by the async path. + +Recommendation: do (1) and (2) first; then decide whether to delete the sync-only functions in a follow-up. + +## Summary + + +| File | Change | +| ----------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | +| [er2er_main.py](cdip-integrations/er2er/er2er_main.py) | Require AsyncERClient; remove `build_er_clients` import and `asyncio.to_thread(build_er_clients, ...)` branch; raise if async unavailable. | +| [er2er_syncher.py](cdip-integrations/er2er/er2er_syncher.py) | In `async_run_sync`, remove `asyncio.to_thread(run_sync, ...)`; require both clients to be AsyncERClient; update docstring. | +| [er2er_syncher.py](cdip-integrations/er2er/er2er_syncher.py) (optional) | Remove `run_sync`, `build_er_clients`, and sync-only I/O helpers listed above. | + + +No changes to [event_type_copy.py](cdip-integrations/er2er/event_type_copy.py); it remains sync-only and does not use the removed code. diff --git a/erclient/api_paths.py b/erclient/api_paths.py index 99a8b46..ff26862 100644 --- a/erclient/api_paths.py +++ b/erclient/api_paths.py @@ -61,6 +61,17 @@ def event_types_patch_path(version: str, event_type: dict) -> str: raise ValueError(f"Event type id is required for v1.0 patching") +def event_type_delete_path(version: str, value: str) -> str: + """ + Path for deleting an event type by slug (value). + + v2.0 uses activity/eventtypes/{value}; + v1.0 uses activity/events/eventtypes/{value} (if the server supports it). + """ + version = normalize_version(version) + return f"{EVENT_TYPES_PATHS[version]}/{value}" + + def event_type_detail_path(version: str, value: str) -> str: """ Path for getting a single event type by name/slug. diff --git a/erclient/client.py b/erclient/client.py index 49c69d2..aa164a3 100644 --- a/erclient/client.py +++ b/erclient/client.py @@ -17,7 +17,8 @@ from requests.adapters import HTTPAdapter from urllib3.util.retry import Retry -from .api_paths import (DEFAULT_VERSION, VERSION_2_0, event_type_detail_path, +from .api_paths import (DEFAULT_VERSION, VERSION_2_0, + event_type_delete_path, event_type_detail_path, event_types_list_path, event_types_patch_path, normalize_version) from .er_errors import (ERClientBadCredentials, ERClientBadRequest, @@ -307,16 +308,16 @@ def remove_event_from_incident(self, event_id, incident_id, relationship_type='c result = self._delete( f'activity/event/{incident_id}/relationship/{relationship_type}/{event_id}/') - def _delete(self, path): + def _delete(self, path, base_url=None): headers = {'User-Agent': self.user_agent} headers.update(self.auth_headers()) + url = self._er_url(path, base_url) if (self._http_session): - response = self._http_session.delete( - self._er_url(path), headers=headers) + response = self._http_session.delete(url, headers=headers) else: - response = requests.delete(self._er_url(path), headers=headers) + response = requests.delete(url, headers=headers) if response.ok: return True @@ -333,6 +334,15 @@ def _delete(self, path): reason = 'unknown reason' raise ERClientPermissionDenied(reason) + if response.status_code == 409: # conflict + try: + detail = json.loads(response.text).get('detail', response.text) + except Exception: + detail = response.text + raise ERClientException( + f'Cannot delete: {detail}', + ) + raise ERClientException( f'Failed to delete: {response.status_code} {response.text}') @@ -558,6 +568,28 @@ def post_event_type(self, event_type, version=DEFAULT_VERSION): self.logger.debug('Result of event type post is: %s', result) return result + def delete_event_type(self, value, version=VERSION_2_0): + """ + Delete an event type by its slug (value). + + The das server returns 204 No Content on success, or 409 Conflict if + the event type has associated events or alert rules. + + :param value: The event type slug/value (e.g. "immobility_rep"). + :param version: API version segment. Defaults to v2.0 because the + destroy action is primarily supported on the v2 endpoint. + :returns: True on successful deletion. + :raises ERClientNotFound: If the event type does not exist. + :raises ERClientException: If the server returns 409 Conflict (event + type is in use) or another error. + """ + self.logger.debug('Deleting event type: %s (version %s)', value, version) + path = event_type_delete_path(version, value) + base_url = self._api_root(version) + result = self._delete(path, base_url=base_url) + self.logger.debug('Result of event type delete: %s', result) + return result + def post_report(self, data): payload = self._clean_event(data) self.logger.debug('Posting report: %s', payload) @@ -1544,6 +1576,28 @@ async def patch_event_type(self, event_type, version=DEFAULT_VERSION): # self.logger.debug('Result of event type patch is: %s', result) return result + async def delete_event_type(self, value, version=VERSION_2_0): + """ + Delete an event type by its slug (value). + + The das server returns 204 No Content on success, or 409 Conflict if + the event type has associated events or alert rules. + + :param value: The event type slug/value (e.g. "immobility_rep"). + :param version: API version segment. Defaults to v2.0 because the + destroy action is primarily supported on the v2 endpoint. + :returns: True on successful deletion. + :raises ERClientNotFound: If the event type does not exist. + :raises ERClientException: If the server returns 409 Conflict (event + type is in use) or another error. + """ + self.logger.debug('Deleting event type: %s (version %s)', value, version) + path = event_type_delete_path(version, value) + base_url = self._api_root(version) + result = await self._delete(path, base_url=base_url) + self.logger.debug('Result of event type delete: %s', result) + return result + async def get_subjectgroups( self, include_inactive=False, @@ -1710,7 +1764,6 @@ async def get_file(self, url): raise ERClientPermissionDenied(reason) raise ERClientException( f'Failed to get file: {response.status_code} {response.text}') - async def _post(self, path, payload, params=None, base_url=None): return await self._call(path, payload, "POST", params, base_url=base_url) diff --git a/tests/async_client/test_delete_event_type.py b/tests/async_client/test_delete_event_type.py new file mode 100644 index 0000000..d6c05f1 --- /dev/null +++ b/tests/async_client/test_delete_event_type.py @@ -0,0 +1,172 @@ +import httpx +import pytest +import respx + +from erclient.er_errors import ERClientException, ERClientNotFound, ERClientPermissionDenied + + +@pytest.mark.asyncio +async def test_delete_event_type_success_v2(er_client): + """delete_event_type() defaults to v2.0 and returns True on 204.""" + slug = "rainfall_rep" + api_root_v2 = er_client._api_root("v2.0") + async with respx.mock(base_url=api_root_v2, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/eventtypes/{slug}") + route.return_value = httpx.Response(httpx.codes.NO_CONTENT) + + result = await er_client.delete_event_type(slug) + + assert route.called + await er_client.close() + assert result is True + + +@pytest.mark.asyncio +async def test_delete_event_type_explicit_v2(er_client): + """delete_event_type(version="v2.0") uses .../api/v2.0/activity/eventtypes/{slug}.""" + slug = "immobility_rep" + api_root_v2 = er_client._api_root("v2.0") + async with respx.mock(base_url=api_root_v2, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/eventtypes/{slug}") + route.return_value = httpx.Response(httpx.codes.NO_CONTENT) + + result = await er_client.delete_event_type(slug, version="v2.0") + + assert route.called + await er_client.close() + assert result is True + + +@pytest.mark.asyncio +async def test_delete_event_type_v1_uses_v1_path(er_client): + """delete_event_type(version="v1.0") uses the v1.0 path.""" + slug = "some_event_type" + api_root_v1 = er_client._api_root("v1.0") + async with respx.mock(base_url=api_root_v1, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/events/eventtypes/{slug}") + route.return_value = httpx.Response(httpx.codes.NO_CONTENT) + + result = await er_client.delete_event_type(slug, version="v1.0") + + assert route.called + await er_client.close() + assert result is True + + +@pytest.mark.asyncio +async def test_delete_event_type_version_alias_v2(er_client): + """delete_event_type(version="v2") alias is normalized to v2.0.""" + slug = "test_type" + api_root_v2 = er_client._api_root("v2.0") + async with respx.mock(base_url=api_root_v2, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/eventtypes/{slug}") + route.return_value = httpx.Response(httpx.codes.NO_CONTENT) + + result = await er_client.delete_event_type(slug, version="v2") + + assert route.called + await er_client.close() + assert result is True + + +@pytest.mark.asyncio +async def test_delete_event_type_not_found_raises(er_client): + """delete_event_type() raises ERClientNotFound on 404.""" + slug = "nonexistent_type" + api_root_v2 = er_client._api_root("v2.0") + async with respx.mock(base_url=api_root_v2, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/eventtypes/{slug}") + route.return_value = httpx.Response( + httpx.codes.NOT_FOUND, + json={"detail": "Not found."} + ) + + with pytest.raises(ERClientNotFound): + await er_client.delete_event_type(slug) + + assert route.called + await er_client.close() + + +@pytest.mark.asyncio +async def test_delete_event_type_forbidden_raises(er_client): + """delete_event_type() raises ERClientPermissionDenied on 403.""" + slug = "protected_type" + api_root_v2 = er_client._api_root("v2.0") + async with respx.mock(base_url=api_root_v2, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/eventtypes/{slug}") + route.return_value = httpx.Response( + httpx.codes.FORBIDDEN, + json={"status": {"detail": "You do not have permission to perform this action."}} + ) + + with pytest.raises(ERClientPermissionDenied): + await er_client.delete_event_type(slug) + + assert route.called + await er_client.close() + + +@pytest.mark.asyncio +async def test_delete_event_type_conflict_raises(er_client): + """delete_event_type() raises ERClientException with detail on 409 Conflict.""" + slug = "in_use_type" + conflict_detail = ( + "Cannot delete Event Type 'In Use Type' because it is associated " + "with existing Events, and it is associated with existing Alert Rules." + ) + api_root_v2 = er_client._api_root("v2.0") + async with respx.mock(base_url=api_root_v2, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/eventtypes/{slug}") + route.return_value = httpx.Response( + httpx.codes.CONFLICT, + json={"detail": conflict_detail} + ) + + with pytest.raises(ERClientException, match="Cannot delete"): + await er_client.delete_event_type(slug) + + assert route.called + await er_client.close() + + +@pytest.mark.asyncio +async def test_delete_event_type_conflict_has_events_only(er_client): + """409 with only events dependency surfaces the correct detail.""" + slug = "events_only_type" + conflict_detail = ( + "Cannot delete Event Type 'Events Only' because it is associated " + "with existing Events." + ) + api_root_v2 = er_client._api_root("v2.0") + async with respx.mock(base_url=api_root_v2, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/eventtypes/{slug}") + route.return_value = httpx.Response( + httpx.codes.CONFLICT, + json={"detail": conflict_detail} + ) + + with pytest.raises(ERClientException, match="existing Events"): + await er_client.delete_event_type(slug) + + assert route.called + await er_client.close() + + +@pytest.mark.asyncio +async def test_delete_event_type_server_error_raises(er_client): + """delete_event_type() raises ERClientException on 500.""" + slug = "server_error_type" + api_root_v2 = er_client._api_root("v2.0") + async with respx.mock(base_url=api_root_v2, assert_all_called=False) as respx_mock: + route = respx_mock.delete(f"activity/eventtypes/{slug}") + route.return_value = httpx.Response( + httpx.codes.INTERNAL_SERVER_ERROR, + text="Internal Server Error" + ) + + with pytest.raises(ERClientException, match="Internal Server Error"): + await er_client.delete_event_type(slug) + + assert route.called + await er_client.close() diff --git a/tests/sync_client/test_delete_event_type.py b/tests/sync_client/test_delete_event_type.py new file mode 100644 index 0000000..a69dfee --- /dev/null +++ b/tests/sync_client/test_delete_event_type.py @@ -0,0 +1,194 @@ +import json +from unittest.mock import MagicMock, patch + +import pytest + +from erclient.client import ERClient +from erclient.er_errors import ERClientException, ERClientNotFound, ERClientPermissionDenied + + +def test_delete_event_type_success_defaults_to_v2(er_server_info): + """delete_event_type() defaults to v2.0 and hits .../api/v2.0/activity/eventtypes/{slug}.""" + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = True + mock_response.status_code = 204 + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + result = er_client.delete_event_type("rainfall_rep") + + assert result is True + mock_session_instance.delete.assert_called_once() + call_url = mock_session_instance.delete.call_args[0][0] + assert "/api/v2.0/" in call_url + assert "activity/eventtypes/rainfall_rep" in call_url + + +def test_delete_event_type_explicit_v2(er_server_info): + """delete_event_type(version="v2.0") uses v2.0 API path.""" + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = True + mock_response.status_code = 204 + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + result = er_client.delete_event_type("immobility_rep", version="v2.0") + + assert result is True + call_url = mock_session_instance.delete.call_args[0][0] + assert "/api/v2.0/" in call_url + assert "activity/eventtypes/immobility_rep" in call_url + + +def test_delete_event_type_v1(er_server_info): + """delete_event_type(version="v1.0") uses the v1.0 path.""" + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = True + mock_response.status_code = 204 + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + result = er_client.delete_event_type("some_type", version="v1.0") + + assert result is True + call_url = mock_session_instance.delete.call_args[0][0] + assert "/api/v1.0/" in call_url + assert "activity/events/eventtypes/some_type" in call_url + + +def test_delete_event_type_version_alias_v2(er_server_info): + """delete_event_type(version="v2") alias is normalized to v2.0.""" + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = True + mock_response.status_code = 204 + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + result = er_client.delete_event_type("test_type", version="v2") + + assert result is True + call_url = mock_session_instance.delete.call_args[0][0] + assert "/api/v2.0/" in call_url + assert "activity/eventtypes/test_type" in call_url + + +def test_delete_event_type_not_found(er_server_info): + """delete_event_type() raises ERClientNotFound on 404.""" + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = False + mock_response.status_code = 404 + mock_response.text = '{"detail": "Not found."}' + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + with pytest.raises(ERClientNotFound): + er_client.delete_event_type("nonexistent_type") + + +def test_delete_event_type_forbidden(er_server_info): + """delete_event_type() raises ERClientPermissionDenied on 403.""" + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = False + mock_response.status_code = 403 + mock_response.text = json.dumps({ + "status": {"detail": "You do not have permission to perform this action."} + }) + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + with pytest.raises(ERClientPermissionDenied): + er_client.delete_event_type("protected_type") + + +def test_delete_event_type_conflict_raises_with_detail(er_server_info): + """delete_event_type() raises ERClientException with detail on 409 Conflict.""" + conflict_detail = ( + "Cannot delete Event Type 'In Use Type' because it is associated " + "with existing Events, and it is associated with existing Alert Rules." + ) + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = False + mock_response.status_code = 409 + mock_response.text = json.dumps({"detail": conflict_detail}) + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + with pytest.raises(ERClientException, match="Cannot delete"): + er_client.delete_event_type("in_use_type") + + +def test_delete_event_type_conflict_events_only(er_server_info): + """409 with only events dependency surfaces the correct detail.""" + conflict_detail = ( + "Cannot delete Event Type 'Events Only' because it is associated " + "with existing Events." + ) + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = False + mock_response.status_code = 409 + mock_response.text = json.dumps({"detail": conflict_detail}) + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + with pytest.raises(ERClientException, match="existing Events"): + er_client.delete_event_type("events_only_type") + + +def test_delete_event_type_server_error(er_server_info): + """delete_event_type() raises ERClientException on 500.""" + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = False + mock_response.status_code = 500 + mock_response.text = "Internal Server Error" + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**er_server_info) + with pytest.raises(ERClientException, match="Failed to delete"): + er_client.delete_event_type("server_error_type") + + +def test_delete_event_type_base_url_assembles_api_path(er_server_info): + """When service_root is a base URL (no /api/), client assembles .../api/v2.0.""" + base_only = {**er_server_info, + "service_root": "https://something.pamdas.org"} + with patch("erclient.client.requests.Session") as mock_session: + mock_response = MagicMock() + mock_response.ok = True + mock_response.status_code = 204 + mock_session_instance = MagicMock() + mock_session_instance.delete.return_value = mock_response + mock_session.return_value = mock_session_instance + + er_client = ERClient(**base_only) + er_client.delete_event_type("test_slug") + + call_url = mock_session_instance.delete.call_args[0][0] + assert call_url.startswith("https://something.pamdas.org/api/v2.0/") + assert "activity/eventtypes/test_slug" in call_url diff --git a/tests/test_api_paths.py b/tests/test_api_paths.py index 7cd7789..ad95576 100644 --- a/tests/test_api_paths.py +++ b/tests/test_api_paths.py @@ -2,7 +2,8 @@ import pytest -from erclient.api_paths import (DEFAULT_VERSION, event_type_detail_path, +from erclient.api_paths import (DEFAULT_VERSION, event_type_delete_path, + event_type_detail_path, event_types_list_path, event_types_patch_path, normalize_version) @@ -69,3 +70,20 @@ def test_event_types_list_path_alias_v2(): def test_event_types_list_path_unsupported_raises(): with pytest.raises(ValueError, match="Unsupported API version"): event_types_list_path("v3") + + +def test_event_type_delete_path_v2(): + assert event_type_delete_path("v2.0", "my_slug") == "activity/eventtypes/my_slug" + + +def test_event_type_delete_path_v1(): + assert event_type_delete_path("v1.0", "my_slug") == "activity/events/eventtypes/my_slug" + + +def test_event_type_delete_path_alias(): + assert event_type_delete_path("v2", "my_slug") == "activity/eventtypes/my_slug" + + +def test_event_type_delete_path_unsupported_raises(): + with pytest.raises(ValueError, match="Unsupported API version"): + event_type_delete_path("v3", "my_slug")