fix(abstractions): enhance request information sanitization, span naming, and option removal

packages/abstractions/kiota_abstractions/request_information.py

@@ -149,11 +149,11 @@ def add_request_options(self, options: Optional[list[RequestOption]]) -> None:
         for option in options:
             self.__request_options[option.get_key()] = option
 
-    def remove_request_options(self, options: list[RequestOption]) -> None:
+    def remove_request_options(self, options: Optional[list[RequestOption]]) -> None:
         if not options:
             return
         for option in options:
-            del self.__request_options[option.get_key()]
+            self.__request_options.pop(option.get_key(), None)
 
     def set_content_from_parsable(
         self,
@@ -189,15 +189,15 @@ def set_content_from_scalar(
         self,
         request_adapter: Optional["RequestAdapter"],
         content_type: Optional[str],
-        values: Union[T, list[T]],
+        values: Union[T, list[T], Enum],
     ) -> None:
         """Sets the request body from a scalar value with the specified content type.
 
         Args:
             request_adapter (Optional[RequestAdapter]): The adapter service to get the serialization
             writer from.
             content_type (Optional[str]): the content type to set.
-            values (Union[T, list[T]]): the scalar values to serialize
+            values (Union[T, list[T], Enum]): the scalar values to serialize
         """
         with tracer.start_as_current_span(
             self._create_parent_span_name("set_content_from_scalar")
@@ -218,17 +218,22 @@ def set_content_from_scalar(
                     date: writer.write_date_value,
                     timedelta: writer.write_timedelta_value,
                     time: writer.write_time_value,
+                    bytes: writer.write_bytes_value,
                 }
                 value_type = type(values)
                 span.set_attribute(self.REQUEST_TYPE_KEY, value_type.__name__)
                 writer_func = function_values.get(value_type, None)
                 if writer_func is None:
-                    exc = ValueError(
-                        f"Encountered an unknown type during serialization {value_type}"
-                    )
-                    span.record_exception(exc)
-                    raise exc
-                writer_func(None, values)
+                    if isinstance(values, Enum):
+                        writer.write_enum_value(None, values)
+                    else:
+                        exc = ValueError(
+                            f"Encountered an unknown type during serialization {value_type}"
+                        )
+                        span.record_exception(exc)
+                        raise exc
+                else:
+                    writer_func(None, values)
             self._set_content_and_content_type_header(writer, content_type)
 
     def set_stream_content(self, value: bytes, content_type: Optional[str] = None) -> None:
@@ -290,40 +295,44 @@ def _set_content_and_content_type_header(
         self.content = writer.get_serialized_content()
 
     def _get_sanitized_value(self, value: Any) -> Any:
-        """Replaces enum values with their string representation.
+        """Sanitizes parameter values for URI template expansion.
 
         Args:
             value (Any): The value to replace.
         """
-        sanitized_value = value
         if isinstance(value, Enum):
-            sanitized_value = value.value
-        elif isinstance(value, list) and all(isinstance(x, Enum) for x in value):
-            sanitized_value = ','.join([x.value for x in value])
-        elif isinstance(value, datetime):
-            timezone_info = value.tzinfo
-            if timezone_info is None:
-                timezone_info = timezone.utc
+            return value.value
+        if isinstance(value, (list, tuple, set)):
+            if not value:
+                return []
+            if all(isinstance(x, Enum) for x in value):
+                return ','.join([str(x.value) for x in value])
+            return [self._get_sanitized_value(x) for x in value]
+
+        sanitized_value = value
+        if isinstance(value, datetime):
+            timezone_info = value.tzinfo if value.tzinfo is not None else timezone.utc
             temp_date_with_tz_info = datetime(
                 value.year, value.month, value.day, value.hour, value.minute, value.second,
                 value.microsecond, timezone_info
             )
             sanitized_value = temp_date_with_tz_info.isoformat("T")
-        elif any([isinstance(value, UUID), isinstance(value, date), isinstance(value, time)]):
+        elif isinstance(value, (UUID, date, time)):
             sanitized_value = str(value)
         return sanitized_value
 
     def _decode_uri_string(self, uri: Optional[str]) -> str:
         """Decodes a URI encoded string."""
-        if uri and "%" in uri:
-            return unquote(uri)
+        if uri:
+            return unquote(uri) if "%" in uri else uri
         return ""
 
     def _create_parent_span_name(self, method: str) -> str:
         """Creates a parent span name for the given method."""
         uri_template = self._decode_uri_string(self.url_template)
-        parent_span_name = f"{method} - {uri_template}"
-        return parent_span_name
+        if uri_template:
+            return f"{method} - {uri_template}"
+        return method
 
     def _start_local_tracing_span(self, name: str, parent_span: trace.Span) -> trace.Span:
         """Helper method to start a span locally with the parent context."""

packages/abstractions/tests/test_request_information.py

@@ -277,3 +277,90 @@ def test_setting_a_raw_url_does_not_clear_another_requests_path_parameters():
     raw.url = "https://graph.microsoft.com/v1.0/me/messages?$skiptoken=abc"
 
     assert templated.url == "https://graph.microsoft.com/v1.0/users/bob/messages"
+
+
+def test_sets_empty_list_in_query_parameters():
+    """Tests that setting an empty list does not emit a dangling key in the expanded URL."""
+    request_info = RequestInformation(Method.GET, "https://example.com/me{?dataset*}")
+    request_info.query_parameters["dataset"] = []
+    assert request_info.url == "https://example.com/me"
+
+
+def test_sets_uuid_collection_in_query_parameters():
+    """Tests setting collection of UUIDs in query parameters expands without error."""
+    id1 = uuid.UUID("123e4567-e89b-12d3-a456-426614174000")
+    id2 = uuid.UUID("789e0123-e89b-12d3-a456-426614174001")
+    request_info = RequestInformation(Method.GET, "https://example.com/values{?ids*}")
+    request_info.query_parameters["ids"] = [id1, id2]
+    assert request_info.url == (
+        "https://example.com/values?ids=123e4567-e89b-12d3-a456-426614174000"
+        "&ids=789e0123-e89b-12d3-a456-426614174001"
+    )
+
+
+def test_sets_tuple_and_set_in_query_parameters():
+    """Tests setting tuple and set in query parameters expands properly."""
+    request_info = RequestInformation(Method.GET, "https://example.com/values{?ids*}")
+    request_info.query_parameters["ids"] = (1, 2)
+    assert request_info.url == "https://example.com/values?ids=1&ids=2"
+
+
+def test_set_content_from_scalar_bytes(
+    mock_request_information, mock_request_adapter, mock_serialization_writer_factory
+):
+    """Tests set_content_from_scalar with bytes value."""
+    writer = Mock(spec_set=SerializationWriter)
+    writer.get_serialized_content.return_value = b"raw-bytes"
+    mock_serialization_writer_factory.get_serialization_writer = Mock(return_value=writer)
+    mock_request_adapter.get_serialization_writer_factory = Mock(
+        return_value=mock_serialization_writer_factory
+    )
+
+    mock_request_information.set_content_from_scalar(
+        mock_request_adapter, "application/octet-stream", b"raw-bytes"
+    )
+    writer.write_bytes_value.assert_called_once_with(None, b"raw-bytes")
+    assert mock_request_information.content == b"raw-bytes"
+
+
+def test_set_content_from_scalar_enum(
+    mock_request_information, mock_request_adapter, mock_serialization_writer_factory
+):
+    """Tests set_content_from_scalar with Enum value."""
+    writer = Mock(spec_set=SerializationWriter)
+    writer.get_serialized_content.return_value = b"value1"
+    mock_serialization_writer_factory.get_serialization_writer = Mock(return_value=writer)
+    mock_request_adapter.get_serialization_writer_factory = Mock(
+        return_value=mock_serialization_writer_factory
+    )
+
+    mock_request_information.set_content_from_scalar(
+        mock_request_adapter, "text/plain", TestEnum.VALUE1
+    )
+    writer.write_enum_value.assert_called_once_with(None, TestEnum.VALUE1)
+    assert mock_request_information.content == b"value1"
+
+
+def test_remove_request_options_safe():
+    """Tests remove_request_options handles missing keys and empty/None gracefully."""
+    request_info = RequestInformation()
+    opt = Mock()
+    opt.get_key.return_value = "NonExistentKey"
+
+    request_info.remove_request_options([opt])
+    request_info.remove_request_options([])
+    request_info.remove_request_options(None)
+
+
+def test_create_parent_span_name():
+    """Tests _create_parent_span_name with unencoded and encoded URI templates."""
+    request_info = RequestInformation(Method.GET, "https://example.com/users")
+    assert request_info._create_parent_span_name("GET") == "GET - https://example.com/users"
+
+    request_info_encoded = RequestInformation(Method.GET, "https://example.com/users%20list")
+    assert request_info_encoded._create_parent_span_name(
+        "GET"
+    ) == "GET - https://example.com/users list"
+
+    request_info_none = RequestInformation(Method.GET)
+    assert request_info_none._create_parent_span_name("GET") == "GET"