From 5abab9ba52ba128671ed1b62d341ba5464980a0a Mon Sep 17 00:00:00 2001 From: ANDROIDHASSAN <84630851+ANDROIDHASSAN@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:06:54 +0530 Subject: [PATCH 1/2] fix(api): keep ListUpdate.clear in the SIP *_fields helpers update_inbound_trunk_fields, update_outbound_trunk_fields and update_dispatch_rule_fields copied only set/add/remove from a ListUpdate argument, so ListUpdate(clear=True) reached the server as an empty update and the list was left unchanged. Copy the whole message instead, through one helper shared by the five list arguments. --- livekit-api/livekit/api/sip_service.py | 43 ++++++------------ tests/api/test_sip_list_update.py | 63 ++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 30 deletions(-) create mode 100644 tests/api/test_sip_list_update.py diff --git a/livekit-api/livekit/api/sip_service.py b/livekit-api/livekit/api/sip_service.py index c222cdf0..0c269488 100644 --- a/livekit-api/livekit/api/sip_service.py +++ b/livekit-api/livekit/api/sip_service.py @@ -57,6 +57,14 @@ def _as_sip_error(err: ServerError) -> ServerError: return err +def _set_list_update(dst: ListUpdate, value: ListUpdate | list[str]) -> None: + """Copy a list argument of the *_fields helpers into the request's ListUpdate.""" + if isinstance(value, ListUpdate): + dst.CopyFrom(value) # keeps clear as well as set/add/remove + else: + dst.set.extend(value) + + class SipService(Service): """Client for LiveKit SIP Service API @@ -193,26 +201,11 @@ async def update_inbound_trunk_fields( media=media, ) if numbers is not None: - if isinstance(numbers, ListUpdate): - update.numbers.set.extend(numbers.set) - update.numbers.add.extend(numbers.add) - update.numbers.remove.extend(numbers.remove) - else: - update.numbers.set.extend(numbers) + _set_list_update(update.numbers, numbers) if allowed_addresses is not None: - if isinstance(allowed_addresses, ListUpdate): - update.allowed_addresses.set.extend(allowed_addresses.set) - update.allowed_addresses.add.extend(allowed_addresses.add) - update.allowed_addresses.remove.extend(allowed_addresses.remove) - else: - update.allowed_addresses.set.extend(allowed_addresses) + _set_list_update(update.allowed_addresses, allowed_addresses) if allowed_numbers is not None: - if isinstance(allowed_numbers, ListUpdate): - update.allowed_numbers.set.extend(allowed_numbers.set) - update.allowed_numbers.add.extend(allowed_numbers.add) - update.allowed_numbers.remove.extend(allowed_numbers.remove) - else: - update.allowed_numbers.set.extend(allowed_numbers) + _set_list_update(update.allowed_numbers, allowed_numbers) return await self._client.request( SVC, @@ -376,12 +369,7 @@ async def update_outbound_trunk_fields( media=media, ) if numbers is not None: - if isinstance(numbers, ListUpdate): - update.numbers.set.extend(numbers.set) - update.numbers.add.extend(numbers.add) - update.numbers.remove.extend(numbers.remove) - else: - update.numbers.set.extend(numbers) + _set_list_update(update.numbers, numbers) return await self._client.request( SVC, @@ -652,12 +640,7 @@ async def update_dispatch_rule_fields( attributes=attributes, ) if trunk_ids is not None: - if isinstance(trunk_ids, ListUpdate): - update.trunk_ids.set.extend(trunk_ids.set) - update.trunk_ids.add.extend(trunk_ids.add) - update.trunk_ids.remove.extend(trunk_ids.remove) - else: - update.trunk_ids.set.extend(trunk_ids) + _set_list_update(update.trunk_ids, trunk_ids) return await self._client.request( SVC, diff --git a/tests/api/test_sip_list_update.py b/tests/api/test_sip_list_update.py new file mode 100644 index 00000000..b68243b1 --- /dev/null +++ b/tests/api/test_sip_list_update.py @@ -0,0 +1,63 @@ +# Copyright 2026 LiveKit, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import asyncio +from typing import Any, List, Optional, Union + +import pytest + +from livekit.api.sip_service import SipService +from livekit.protocol.models import ListUpdate + +# (method, list argument, field of request.update) +LIST_FIELDS = [ + ("update_inbound_trunk_fields", "numbers", "numbers"), + ("update_inbound_trunk_fields", "allowed_addresses", "allowed_addresses"), + ("update_inbound_trunk_fields", "allowed_numbers", "allowed_numbers"), + ("update_outbound_trunk_fields", "numbers", "numbers"), + ("update_dispatch_rule_fields", "trunk_ids", "trunk_ids"), +] + + +def _sent_update(method: str, arg: str, value: Optional[Union[ListUpdate, List[str]]]) -> Any: + sent: List[Any] = [] + + async def request( + service: str, rpc: str, data: Any, headers: Any, response_class: Any, **kwargs: Any + ) -> Any: + sent.append(data) + return response_class() + + svc = SipService(None, "https://example.livekit.cloud", "key", "secret") # type: ignore[arg-type] + svc._client.request = request # type: ignore[method-assign, assignment] + asyncio.run(getattr(svc, method)("ID_1", **{arg: value})) + return sent[0].update + + +@pytest.mark.parametrize(("method", "arg", "field"), LIST_FIELDS) +def test_list_update_clear_is_sent(method: str, arg: str, field: str) -> None: + update = _sent_update(method, arg, ListUpdate(clear=True)) + assert getattr(update, field) == ListUpdate(clear=True) + + +@pytest.mark.parametrize(("method", "arg", "field"), LIST_FIELDS) +def test_list_and_list_update_values_are_still_sent(method: str, arg: str, field: str) -> None: + assert getattr(_sent_update(method, arg, ["a", "b"]), field) == ListUpdate(set=["a", "b"]) + change = ListUpdate(add=["a"], remove=["b"]) + assert getattr(_sent_update(method, arg, change), field) == change + + +@pytest.mark.parametrize(("method", "arg", "field"), LIST_FIELDS) +def test_omitted_list_is_not_sent(method: str, arg: str, field: str) -> None: + assert not _sent_update(method, arg, None).HasField(field) From 71e1c73fc3896d063d289d44bab3b6192ab936dc Mon Sep 17 00:00:00 2001 From: ANDROIDHASSAN <84630851+ANDROIDHASSAN@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:15:58 +0530 Subject: [PATCH 2/2] fix(api): leave an empty list argument out of the SIP update numbers=[] or ListUpdate() put an empty ListUpdate into the request, which the server rejects ("unsupported list update operation"), failing the whole update including its other fields. An empty value now leaves the field unset, so the list is not changed. --- livekit-api/livekit/api/sip_service.py | 7 +++++-- tests/api/test_sip_list_update.py | 11 +++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/livekit-api/livekit/api/sip_service.py b/livekit-api/livekit/api/sip_service.py index 0c269488..f16fa3f8 100644 --- a/livekit-api/livekit/api/sip_service.py +++ b/livekit-api/livekit/api/sip_service.py @@ -59,9 +59,12 @@ def _as_sip_error(err: ServerError) -> ServerError: def _set_list_update(dst: ListUpdate, value: ListUpdate | list[str]) -> None: """Copy a list argument of the *_fields helpers into the request's ListUpdate.""" + # The server rejects an empty ListUpdate, failing the whole update, so an empty + # value ([] or ListUpdate()) leaves the field unset: the list is not changed. if isinstance(value, ListUpdate): - dst.CopyFrom(value) # keeps clear as well as set/add/remove - else: + if value.ByteSize(): + dst.CopyFrom(value) # keeps clear as well as set/add/remove + elif value: dst.set.extend(value) diff --git a/tests/api/test_sip_list_update.py b/tests/api/test_sip_list_update.py index b68243b1..ac708d4a 100644 --- a/tests/api/test_sip_list_update.py +++ b/tests/api/test_sip_list_update.py @@ -59,5 +59,12 @@ def test_list_and_list_update_values_are_still_sent(method: str, arg: str, field @pytest.mark.parametrize(("method", "arg", "field"), LIST_FIELDS) -def test_omitted_list_is_not_sent(method: str, arg: str, field: str) -> None: - assert not _sent_update(method, arg, None).HasField(field) +@pytest.mark.parametrize( + "value", [None, [], ListUpdate()], ids=["omitted", "empty-list", "empty-update"] +) +def test_omitted_or_empty_list_is_not_sent( + method: str, arg: str, field: str, value: Optional[Union[ListUpdate, List[str]]] +) -> None: + # the server rejects an empty ListUpdate ("unsupported list update operation"), + # which would fail the whole update, including its other fields + assert not _sent_update(method, arg, value).HasField(field)