fix(abstractions): safely handle missing values and normalize return in HeadersCollection.remove_value

packages/abstractions/kiota_abstractions/headers_collection.py

@@ -146,19 +146,24 @@ def remove_value(self, header_name: str, header_value: str) -> Union[bool, set[s
             header_value (str): The value to remove from the header.
 
         Returns:
-            bool: _description_
+            Union[bool, set[str]]: Remaining values set if header still has other values,
+            True if the value was removed and no values remain, or False if header or value
+            was not found.
         """
         if not header_name:
             raise ValueError("Header name cannot be null")
         if header_value is None:
             raise ValueError("Header value cannot be null")
         header_name = header_name.lower()
         values = self.try_get(header_name)
-        if values:
-            values.remove(header_value)  #type: ignore
+        if isinstance(values, set):
+            if header_value not in values:
+                return False
+            values.remove(header_value)
             if bool(values):
                 return values
-            return self.remove(header_name)
+            self.remove(header_name)
+            return True
 
         return False
 
@@ -169,7 +174,7 @@ def remove(self, header_name: str) -> Union[bool, set[str]]:
             header_name (str): The name of the header to remove.
 
         Returns:
-            bool: True if the header has been removed, False otherwise.
+            Union[bool, set[str]]: The removed set of values if present, False otherwise.
         """
         if not header_name:
             raise ValueError("Header name cannot be null")

packages/abstractions/tests/test_request_header.py

@@ -118,14 +118,15 @@ def test_removes_value_from_existing_header():
     """Tests removing a value from an existing header
     """
     headers = HeadersCollection()
-    headers.remove_value("header1", "value1")
+    assert headers.remove_value("header1", "value1") is False
     headers.add("header1", "value1")
     headers.add("header1", "value2")
     assert headers.contains("header1")
     assert headers.count() == 1
-    headers.remove_value("header1", "value1")
+    remaining = headers.remove_value("header1", "value1")
+    assert remaining == {"value2"}
     assert {"value2"} <= headers.try_get("header1")
-    headers.remove_value("header1", "value2")
+    assert headers.remove_value("header1", "value2") is True
     assert not headers.contains("header1")
     assert headers.count() == 0
 
@@ -303,3 +304,32 @@ def test_subscript_assignment_single_value_header():
     headers["Content-Type"] = ["application/json", "text/plain"]
     assert headers["content-type"] == {"application/json"}
 
+
+def test_remove_value_nonexistent_value_returns_false():
+    """Tests removing a nonexistent value from an existing header returns False without raising KeyError"""
+    headers = HeadersCollection()
+    headers.add("header1", ["value1", "value2"])
+    assert headers.count() == 1
+    assert headers.remove_value("header1", "nonexistent") is False
+    assert headers.get("header1") == {"value1", "value2"}
+    assert headers.count() == 1
+
+
+def test_remove_value_case_insensitive_header_name():
+    """Tests remove_value normalizes header name casing"""
+    headers = HeadersCollection()
+    headers.add("Content-Type", "application/json")
+    assert headers.remove_value("content-type", "application/json") is True
+    assert not headers.contains("content-type")
+    assert headers.count() == 0
+
+
+def test_remove_header_return_semantics():
+    """Tests that remove returns the popped set when present, and False when not found"""
+    headers = HeadersCollection()
+    assert headers.remove("nonexistent") is False
+    headers.add("header1", ["value1", "value2"])
+    removed = headers.remove("header1")
+    assert removed == {"value1", "value2"}
+    assert not headers.contains("header1")
+    assert headers.remove("header1") is False