From 8523e027b97698b8db3fc169bdda7683f895faa5 Mon Sep 17 00:00:00 2001 From: michael-richey Date: Fri, 17 Jul 2026 10:59:11 -0400 Subject: [PATCH 1/2] feat(users): key users by handle; add --use-v1-user-api flag Users were identified and linked by email, but the destination enforces user uniqueness on the handle (email is not unique). Two users that share an email but have distinct handles collapsed onto one derived handle. Switch resource_mapping_key to attributes.handle (exact case) so distinct handles stay distinct. Keep handle out of the read-only v2 create/update payload (pop before POST/PATCH) and out of update diffs (via exclude_regex_paths), while retaining it for mapping and v1 user creation. Also wire the opt-in --use-v1-user-api flag (default off; env DD_USE_V1_USER_API) through the CLI options and Configuration; the v1 create path lands in a follow-up. Add pytest-cov to test extras. Co-Authored-By: Claude Opus 4.8 --- datadog_sync/commands/shared/options.py | 12 ++ datadog_sync/constants.py | 1 + datadog_sync/model/users.py | 14 +- datadog_sync/utils/configuration.py | 3 + setup.cfg | 1 + tests/unit/test_users.py | 181 ++++++++++++++++++++++++ 6 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_users.py diff --git a/datadog_sync/commands/shared/options.py b/datadog_sync/commands/shared/options.py index a519b6ec7..1de6ea08a 100644 --- a/datadog_sync/commands/shared/options.py +++ b/datadog_sync/commands/shared/options.py @@ -591,6 +591,18 @@ def click_config_file_provider(ctx: Context, opts: CustomOptionClass, value: Non help="Allow self-lockout when syncing restriction policies.", cls=CustomOptionClass, ), + option( + "--use-v1-user-api", + required=False, + envvar=constants.DD_USE_V1_USER_API, + type=bool, + default=False, + show_default=True, + help="Create users via the v1 API (/api/v1/user) instead of v2. v2 cannot set a " + "handle (it derives one from the email), which collides when users share an email; " + "v1 accepts an explicit handle so each user keeps its own. Off by default.", + cls=CustomOptionClass, + ), ] diff --git a/datadog_sync/constants.py b/datadog_sync/constants.py index d62d84854..6c8fefd50 100644 --- a/datadog_sync/constants.py +++ b/datadog_sync/constants.py @@ -29,6 +29,7 @@ DD_SHOW_PROGRESS_BAR = "DD_SHOW_PROGRESS_BAR" DD_VERIFY_SSL_CERTIFICATES = "DD_VERIFY_SSL_CERTIFICATES" DD_ALLOW_PARTIAL_PERMISSIONS_ROLES = "DD_ALLOW_PARTIAL_PERMISSIONS_ROLES" +DD_USE_V1_USER_API = "DD_USE_V1_USER_API" DD_SYNC_JSON = "DD_SYNC_JSON" DD_DATADOG_HOST_OVERRIDE = "DD_DATADOG_HOST_OVERRIDE" diff --git a/datadog_sync/model/users.py b/datadog_sync/model/users.py index 5b4e5a795..3f8730573 100644 --- a/datadog_sync/model/users.py +++ b/datadog_sync/model/users.py @@ -30,7 +30,11 @@ class Users(BaseResource): "attributes.status", "attributes.verified", "attributes.service_account", - "attributes.handle", + # NOTE: attributes.handle is deliberately NOT excluded here. It is the + # user mapping key (resource_mapping_key below) and the payload for the + # v1 create fallback, so it must survive prep_resource. It is popped + # manually before the v2 POST/PATCH (v2 treats handle as read-only) and + # kept out of update diffs via deep_diff_config.exclude_regex_paths below. "attributes.icon", "attributes.modified_at", "attributes.mfa_enabled", @@ -40,7 +44,11 @@ class Users(BaseResource): "relationships.org", "relationships.team_roles", ], - resource_mapping_key="attributes.email", + resource_mapping_key="attributes.handle", + # Handle is read-only in v2 and is popped from the create/update payload; + # exclude it from update diffs so a source-vs-destination handle difference + # never drives a spurious PATCH of a field v2 will not accept. + deep_diff_config={"ignore_order": True, "exclude_regex_paths": [r".*\['handle'\]"]}, ) # Additional Users specific attributes pagination_config = PaginationConfig( @@ -82,6 +90,7 @@ async def create_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: destination_client = self.config.destination_client resource["attributes"].pop("disabled", None) + resource["attributes"].pop("handle", None) resp = await destination_client.post(self.resource_config.base_path, {"data": resource}) return _id, resp["data"] @@ -94,6 +103,7 @@ async def update_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: await self.update_user_roles(self.config.state.destination[self.resource_type][_id]["id"], diff) resource["id"] = self.config.state.destination[self.resource_type][_id]["id"] resource.pop("relationships", None) + resource["attributes"].pop("handle", None) resp = await destination_client.patch( self.resource_config.base_path + f"/{self.config.state.destination[self.resource_type][_id]['id']}", {"data": resource}, diff --git a/datadog_sync/utils/configuration.py b/datadog_sync/utils/configuration.py index 755897606..41b391286 100644 --- a/datadog_sync/utils/configuration.py +++ b/datadog_sync/utils/configuration.py @@ -109,6 +109,7 @@ class Configuration(object): max_workers_per_type: Dict[str, int] = field(default_factory=dict) command: str = "" allow_partial_permissions_roles: List[str] = field(default_factory=list) + use_v1_user_api: bool = False resources: Dict[str, BaseResource] = field(default_factory=dict) resources_arg: List[str] = field(default_factory=list) # --id-file: id-targeted import via stdin or file payload. @@ -435,6 +436,7 @@ def build_config(cmd: Command, **kwargs: Optional[Any]) -> Configuration: max_workers_per_type = _parse_max_workers_per_type(max_workers_per_type_raw, known_resource_types) create_global_downtime = kwargs.get("create_global_downtime") validate = kwargs.get("validate") + use_v1_user_api = kwargs.get("use_v1_user_api") or False verify_ddr_status = kwargs.get("verify_ddr_status") backup_before_reset = not kwargs.get("do_not_backup") show_progress_bar = kwargs.get("show_progress_bar") @@ -691,6 +693,7 @@ def build_config(cmd: Command, **kwargs: Optional[Any]) -> Configuration: emit_json=emit_json, command=cmd.value, allow_partial_permissions_roles=allow_partial_permissions_roles, + use_v1_user_api=use_v1_user_api, id_payload=id_payload, max_concurrent_reads=max_concurrent_reads, transient_failure_threshold_pct=transient_failure_threshold_pct, diff --git a/setup.cfg b/setup.cfg index 0b06fe831..e9bc0462e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -56,6 +56,7 @@ tests = black==24.3.0 pytest==8.1.1 pytest-black + pytest-cov pytest-console-scripts pytest-recording==0.13.2 pytest-retry==1.7.0 diff --git a/tests/unit/test_users.py b/tests/unit/test_users.py new file mode 100644 index 000000000..36084f61d --- /dev/null +++ b/tests/unit/test_users.py @@ -0,0 +1,181 @@ +# Unless explicitly stated otherwise all files in this repository are licensed +# under the 3-clause BSD style license (see LICENSE). +# This product includes software developed at Datadog (https://www.datadoghq.com/). +# Copyright 2019 Datadog, Inc. + +"""Tests for handle-keyed users + the --use-v1-user-api flag. + +The Datadog destination enforces user uniqueness on the ``handle``, not the +``email`` — multiple users may share an email. Keying ``_existing_resources_map`` +by email collapsed distinct-handle users onto one derived handle and caused a +409 Conflict on the second create. These tests cover switching the user mapping +key to the exact-case handle (PR1) and wiring the opt-in --use-v1-user-api flag. + +Tests ``a``/``a2``/``a3``/``f`` are red against ``resource_mapping_key= +"attributes.email"`` and green after the switch to ``"attributes.handle"`` plus +the manual handle pop before the v2 POST. Test ``g`` is a green/green guard that +handle stays excluded from update diffs across the excluded_attributes -> +exclude_regex_paths migration. ``p1``/``p2``/``p3`` guard the flag wiring. +""" + +import asyncio +from unittest.mock import AsyncMock + +from click.testing import CliRunner + +from datadog_sync.cli import cli +from datadog_sync.constants import Command +from datadog_sync.model.users import Users +from datadog_sync.utils.configuration import build_config +from datadog_sync.utils.resource_utils import check_diff + + +def _make_user(handle, email, user_id, name="User"): + """Build a user dict shaped like the v2 GET/create response.""" + return { + "id": user_id, + "type": "users", + "attributes": { + "handle": handle, + "email": email, + "name": name, + "disabled": False, + }, + "relationships": {"roles": {"data": []}}, + } + + +def _base_kwargs(tmp_path): + """Minimal build_config kwargs that avoid network/validation.""" + return dict( + resources="users", + resource_per_file=True, + source_api_key="k", + source_app_key="k", + destination_api_key="k", + destination_app_key="k", + source_api_url="https://example.com", + destination_api_url="https://example.com", + storage_type="local", + source_resources_path=str(tmp_path / "source"), + destination_resources_path=str(tmp_path / "dest"), + max_workers=1, + send_metrics=False, + verify_ddr_status=False, + validate=False, + show_progress_bar=False, + allow_self_lockout=False, + force_missing_dependencies=False, + skip_failed_resource_connections=False, + ) + + +class TestHandleMappingKey: + def test_mapping_key_is_exact_case_handle(self, mock_config): + """a: mapping key is the handle, preserved exact-case (no lowercasing).""" + instance = Users(mock_config) + resource = {"attributes": {"handle": "User-A@example.com", "email": "shared@example.com"}} + assert instance.get_resource_mapping_key(resource) == "User-A@example.com" + + def test_map_keeps_shared_email_distinct_by_handle(self, mock_config): + """a2: two destination users sharing an email but with distinct handles + remain two entries in the map (email keying would collapse to one).""" + instance = Users(mock_config) + dest = [ + _make_user("user-a@example.com", "shared@example.com", "dest-a"), + _make_user("user-b@example.com", "shared@example.com", "dest-b"), + ] + instance.get_resources = AsyncMock(return_value=dest) + asyncio.run(instance.map_existing_resources()) + assert set(instance._existing_resources_map.keys()) == { + "user-a@example.com", + "user-b@example.com", + } + + def test_source_matched_by_handle_not_email(self, mock_config): + """a3: against a pre-existing destination user, a source user is matched + by handle — a same-email/different-handle source is NOT a match.""" + instance = Users(mock_config) + instance.get_resources = AsyncMock( + return_value=[_make_user("user-a@example.com", "shared@example.com", "dest-a")] + ) + asyncio.run(instance.map_existing_resources()) + + same_handle = _make_user("user-a@example.com", "shared@example.com", "src-a") + assert instance.get_resource_mapping_key(same_handle) in instance._existing_resources_map + + diff_handle_same_email = _make_user("user-b@example.com", "shared@example.com", "src-b") + assert instance.get_resource_mapping_key(diff_handle_same_email) not in instance._existing_resources_map + + +class TestV2CreatePayload: + def test_v2_post_body_excludes_handle_and_disabled(self, mock_config): + """f: the v2 create body must not carry read-only handle or disabled.""" + instance = Users(mock_config) + instance._existing_resources_map = {} + mock_config.destination_client.post = AsyncMock( + return_value={"data": {"id": "dest-x", "attributes": {}}} + ) + src = _make_user("user-a@example.com", "shared@example.com", "src-a") + + asyncio.run(instance.create_resource("src-a", src)) + + mock_config.destination_client.post.assert_called_once() + _, body = mock_config.destination_client.post.call_args.args + attrs = body["data"]["attributes"] + assert "handle" not in attrs + assert "disabled" not in attrs + + def test_v2_patch_body_excludes_handle(self, mock_config): + """f2: the v2 update (PATCH) body must not carry the read-only handle.""" + instance = Users(mock_config) + _id = "src-a" + mock_config.state.destination["users"][_id] = _make_user( + "user-a@example.com", "shared@example.com", "dest-a" + ) + # A differing name forces a diff -> the PATCH branch. + src = _make_user("user-a@example.com", "shared@example.com", "dest-a", name="New Name") + mock_config.destination_client.patch = AsyncMock( + return_value={"data": {"id": "dest-a", "attributes": {}}} + ) + + asyncio.run(instance.update_resource(_id, src)) + + mock_config.destination_client.patch.assert_called_once() + _, body = mock_config.destination_client.patch.call_args.args + assert "handle" not in body["data"]["attributes"] + + +class TestHandleDiffExclusion: + def test_handle_excluded_from_update_diff(self): + """g: two users differing only by handle produce no diff (guards that + handle stays diff-excluded after moving off excluded_attributes).""" + dest = _make_user("user-a@example.com", "shared@example.com", "same-id") + src = _make_user("user-b@example.com", "shared@example.com", "same-id") + assert not check_diff(Users.resource_config, dest, src) + + +class TestV1UserApiFlagWiring: + def test_build_config_flag_true(self, tmp_path): + """p1: --use-v1-user-api flows from kwargs into Configuration.""" + cfg = build_config(Command.IMPORT, use_v1_user_api=True, **_base_kwargs(tmp_path)) + assert cfg.use_v1_user_api is True + + def test_build_config_flag_default_false(self, tmp_path): + """p1: absent flag defaults to False (guards a typo'd kwarg key).""" + cfg = build_config(Command.IMPORT, **_base_kwargs(tmp_path)) + assert cfg.use_v1_user_api is False + + def test_sync_accepts_v1_flag(self): + """p2: the sync command recognizes the flag (exit 2 == usage error).""" + result = CliRunner(mix_stderr=False).invoke( + cli, ["sync", "--use-v1-user-api=true", "--validate=false"] + ) + assert result.exit_code != 2 + + def test_migrate_accepts_v1_flag(self): + """p3: migrate reuses @sync_options, so it recognizes the flag too.""" + result = CliRunner(mix_stderr=False).invoke( + cli, ["migrate", "--use-v1-user-api=true", "--validate=false"] + ) + assert result.exit_code != 2 From dcd41dde6c9fdf90ff5d6dfef9fadd74c4655412 Mon Sep 17 00:00:00 2001 From: michael-richey Date: Fri, 17 Jul 2026 12:12:36 -0400 Subject: [PATCH 2/2] feat(users): create users via v1 API when --use-v1-user-api is set The destination derives a user's handle from its email on v2 create, so two source users that share an email collapse onto one handle: the first create silently takes a handle that may belong to another user, and the second create 409s and can never succeed. When --use-v1-user-api is set, create users via POST /api/v1/user, which accepts an explicit handle, so every user keeps its own handle and no create collides. The v1 response is the legacy user shape, so re-resolve the v2 UUID by exact-case handle (with a small, sleep-free re-query loop for read-after-write visibility) and return that record, since state and downstream references (roles, team memberships) key on the v2 UUID. Any failure re-raises so the apply loop records it and continues (DR-safe). With the flag off, user creation is unchanged (v2). Co-Authored-By: Claude Opus 4.8 --- datadog_sync/model/users.py | 58 ++++++++++++- tests/unit/test_users.py | 157 +++++++++++++++++++++++++++++++++++- 2 files changed, 209 insertions(+), 6 deletions(-) diff --git a/datadog_sync/model/users.py b/datadog_sync/model/users.py index 3f8730573..52403428e 100644 --- a/datadog_sync/model/users.py +++ b/datadog_sync/model/users.py @@ -32,7 +32,7 @@ class Users(BaseResource): "attributes.service_account", # NOTE: attributes.handle is deliberately NOT excluded here. It is the # user mapping key (resource_mapping_key below) and the payload for the - # v1 create fallback, so it must survive prep_resource. It is popped + # v1 user creation, so it must survive prep_resource. It is popped # manually before the v2 POST/PATCH (v2 treats handle as read-only) and # kept out of update diffs via deep_diff_config.exclude_regex_paths below. "attributes.icon", @@ -88,13 +88,63 @@ async def create_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: self.config.state.destination[self.resource_type][_id] = self._existing_resources_map[key] return await self.update_resource(_id, resource) + attributes = resource["attributes"] + if self.config.use_v1_user_api: + # v2 create cannot set a handle (it derives one from the email), which + # collapses distinct-handle users that share an email onto one handle. + # v1 accepts an explicit handle, so each user keeps its own. + return _id, await self._create_via_v1( + attributes.get("handle"), attributes.get("name"), attributes.get("email") + ) + destination_client = self.config.destination_client - resource["attributes"].pop("disabled", None) - resource["attributes"].pop("handle", None) + # handle is read-only in v2 (derived from email) and must not be sent. + attributes.pop("disabled", None) + attributes.pop("handle", None) resp = await destination_client.post(self.resource_config.base_path, {"data": resource}) - return _id, resp["data"] + async def _create_via_v1(self, handle: Optional[str], name: Optional[str], email: Optional[str]) -> Dict: + """Create the user via the v1 API, which accepts an explicit handle. + + v2 ``POST /api/v2/users`` cannot set a handle (it is derived from the + email), so users that share an email collapse onto one handle and later + creates 409 — and the v2 "winner" is created with the wrong handle. v1 + ``POST /api/v1/user`` accepts a distinct handle. The v1 response is the + legacy user shape, so re-resolve the v2 UUID by handle and return that + record — state and downstream references (roles, team_memberships) key + on the v2 UUID. + """ + if not handle: + raise ValueError("v1 user creation requires a handle") + + destination_client = self.config.destination_client + await destination_client.post("/api/v1/user", {"handle": handle, "name": name, "email": email}) + + user = await self._get_destination_user_by_handle(handle) + if user is None: + raise ValueError("v1-created user not found by handle after create") + return user + + async def _get_destination_user_by_handle(self, handle: str) -> Optional[Dict]: + """Return the destination user whose handle matches exactly, or None. + + Transient HTTP errors are already retried by the client's + ``request_with_retry``; this adds a small, sleep-free re-query loop only + to absorb read-after-write visibility lag after a v1 create. + """ + destination_client = self.config.destination_client + for _ in range(3): + resp = await destination_client.paginated_request(destination_client.get)( + self.resource_config.base_path, + pagination_config=self.pagination_config, + params={"filter": handle}, + ) + for user in resp: + if user.get("attributes", {}).get("handle") == handle: + return user + return None + async def update_resource(self, _id: str, resource: Dict) -> Tuple[str, Dict]: destination_client = self.config.destination_client diff --git a/tests/unit/test_users.py b/tests/unit/test_users.py index 36084f61d..8a74a1db8 100644 --- a/tests/unit/test_users.py +++ b/tests/unit/test_users.py @@ -19,15 +19,24 @@ """ import asyncio -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, MagicMock +import pytest from click.testing import CliRunner from datadog_sync.cli import cli from datadog_sync.constants import Command from datadog_sync.model.users import Users from datadog_sync.utils.configuration import build_config -from datadog_sync.utils.resource_utils import check_diff +from datadog_sync.utils.resource_utils import CustomClientHTTPError, check_diff + + +def _http_error(status): + """Build a CustomClientHTTPError with the given status code.""" + response = MagicMock() + response.status = status + response.message = "Conflict" if status == 409 else "Error" + return CustomClientHTTPError(response) def _make_user(handle, email, user_id, name="User"): @@ -111,6 +120,7 @@ def test_source_matched_by_handle_not_email(self, mock_config): class TestV2CreatePayload: def test_v2_post_body_excludes_handle_and_disabled(self, mock_config): """f: the v2 create body must not carry read-only handle or disabled.""" + mock_config.use_v1_user_api = False instance = Users(mock_config) instance._existing_resources_map = {} mock_config.destination_client.post = AsyncMock( @@ -179,3 +189,146 @@ def test_migrate_accepts_v1_flag(self): cli, ["migrate", "--use-v1-user-api=true", "--validate=false"] ) assert result.exit_code != 2 + + +def _mock_paginated(config, pages): + """Make destination_client.paginated_request(get)(...) yield ``pages`` in order.""" + inner = AsyncMock(side_effect=pages) + config.destination_client.paginated_request = MagicMock(return_value=inner) + return inner + + +def _post_paths(mock_config): + return [c.args[0] for c in mock_config.destination_client.post.call_args_list] + + +class TestV2CreatePath: + def test_flag_off_uses_v2_not_v1(self, mock_config): + """b: with the flag off, create goes through the v2 endpoint and never + touches the v1 API (preserves the default upstream behavior).""" + mock_config.use_v1_user_api = False + instance = Users(mock_config) + instance._existing_resources_map = {} + mock_config.destination_client.post = AsyncMock(return_value={"data": {"id": "dest-x", "attributes": {}}}) + + asyncio.run(instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a"))) + + assert _post_paths(mock_config) == ["/api/v2/users"] + + +class TestV1CreatePath: + def test_flag_on_uses_v1_with_handle_not_v2(self, mock_config): + """d1: with the flag on, create posts to /api/v1/user carrying the handle, + never calls the v2 users endpoint, and returns the reconciled v2 UUID.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + dest_user = _make_user("user-a@example.com", "shared@example.com", "dest-uuid-a") + mock_config.destination_client.post = AsyncMock(return_value={"data": {}}) + _mock_paginated(mock_config, [[dest_user]]) + + _id, r = asyncio.run( + instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a")) + ) + + v1_calls = [c for c in mock_config.destination_client.post.call_args_list if c.args[0] == "/api/v1/user"] + assert len(v1_calls) == 1 + assert v1_calls[0].args[1]["handle"] == "user-a@example.com" + assert "/api/v2/users" not in _post_paths(mock_config) + assert r["id"] == "dest-uuid-a" + + def test_shared_email_distinct_handles_both_created_via_v1(self, mock_config): + """The core fix. Two users share an email but have distinct handles: one + handle equals the shared email, the other differs. Under v2 the first + create derives its handle from the email and steals the second user's + handle, so the second 409s and can never be created. Via v1 each user is + created with its OWN handle, so both succeed.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + dest_a = _make_user("abc@example.com", "jsmith@example.com", "dest-a") + dest_b = _make_user("jsmith@example.com", "jsmith@example.com", "dest-b") + mock_config.destination_client.post = AsyncMock(return_value={"data": {}}) + _mock_paginated(mock_config, [[dest_a], [dest_b]]) + + asyncio.run(instance.create_resource("src-a", _make_user("abc@example.com", "jsmith@example.com", "src-a"))) + asyncio.run(instance.create_resource("src-b", _make_user("jsmith@example.com", "jsmith@example.com", "src-b"))) + + v1_handles = [ + c.args[1]["handle"] + for c in mock_config.destination_client.post.call_args_list + if c.args[0] == "/api/v1/user" + ] + assert v1_handles == ["abc@example.com", "jsmith@example.com"] + assert "/api/v2/users" not in _post_paths(mock_config) + + def test_requires_handle(self, mock_config): + """d0: v1 creation raises when there is no handle to send.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + with pytest.raises(ValueError): + asyncio.run(instance._create_via_v1("", "Person Name", "e@example.com")) + + def test_v1_post_failure_reraises(self, mock_config): + """d4: a v1 POST failure propagates so the apply loop counts it failed + and continues (DR-safe).""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + mock_config.destination_client.post = AsyncMock(side_effect=_http_error(400)) + with pytest.raises(CustomClientHTTPError): + asyncio.run( + instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a")) + ) + + +class TestReconcileByHandle: + def test_no_exact_match_raises(self, mock_config): + """If no exact-handle match is ever found after the v1 create, it raises.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + instance._existing_resources_map = {} + mock_config.destination_client.post = AsyncMock(return_value={"data": {}}) + other = _make_user("user-b@example.com", "shared@example.com", "dest-b") + _mock_paginated(mock_config, [[other], [other], [other]]) + with pytest.raises(ValueError): + asyncio.run( + instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a")) + ) + + def test_requeries_on_empty_then_matches(self, mock_config): + """d3b: read-after-write — an empty first page then a match re-queries + (exactly two calls) rather than giving up on the first empty result.""" + instance = Users(mock_config) + match = _make_user("user-a@example.com", "shared@example.com", "dest-uuid-a") + inner = _mock_paginated(mock_config, [[], [match]]) + user = asyncio.run(instance._get_destination_user_by_handle("user-a@example.com")) + assert user["id"] == "dest-uuid-a" + assert inner.call_count == 2 + + def test_selects_exact_case_handle_among_candidates(self, mock_config): + """d5: with multiple filter candidates, only the exact-case handle wins.""" + instance = Users(mock_config) + a = _make_user("user-a@example.com", "shared@example.com", "dest-a") + b = _make_user("user-b@example.com", "shared@example.com", "dest-b") + _mock_paginated(mock_config, [[b, a]]) + user = asyncio.run(instance._get_destination_user_by_handle("user-a@example.com")) + assert user["id"] == "dest-a" + + +class TestUpdatePathRegression: + def test_existing_handle_takes_update_path_no_create(self, mock_config): + """e: an existing destination handle routes to the update path — no v1 or + v2 create, no duplicate.""" + mock_config.use_v1_user_api = True + instance = Users(mock_config) + dest_user = _make_user("user-a@example.com", "shared@example.com", "dest-a") + instance._existing_resources_map = {"user-a@example.com": dest_user} + mock_config.destination_client.post = AsyncMock() + mock_config.destination_client.patch = AsyncMock(return_value={"data": dest_user}) + + _id, r = asyncio.run( + instance.create_resource("src-a", _make_user("user-a@example.com", "shared@example.com", "src-a")) + ) + mock_config.destination_client.post.assert_not_called() + assert r["id"] == "dest-a"