Skip to content

Commit 884db34

Browse files
committed
Fix Sequence[Union[...]] validation only requiring one element to match
_is_valid's Sequence[Union[X, Y]] branch flattened the per-element check into a single any() across (value, union_arg) pairs: return any(isinstance(val, union_arg) for val in value for union_arg in union_args) This means the check passes as soon as ONE element matches ONE type in the union, regardless of whether other elements in the sequence match anything at all. E.g. _is_valid(Sequence[Union[str, int]], ["a", 3.14]) returns True even though 3.14 is neither a str nor an int. This is reachable through real user-facing validation: collection.tenants add/remove/update/upsert all validate their `tenants` argument against Sequence[Union[str, Tenant, ...]] (weaviate/collections/tenants/executor.py). A list mixing a valid tenant name/object with something invalid (e.g. an accidental int, or a stray dict) currently passes validation silently and gets sent to the request-building code instead of failing fast with a clear WeaviateInvalidInputError - the exact thing this argument validation exists to prevent. Fixed to require every element to match at least one union member: all(any(...) for val in value), matching the existing (correct) logic for the non-Union Sequence[X] branch two lines below it. Added regression tests in test/collection/test_validator.py - verified they fail against the pre-fix code (3 failures) and pass with the fix.
1 parent ae327ca commit 884db34

2 files changed

Lines changed: 29 additions & 2 deletions

File tree

test/collection/test_validator.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from typing import Any, List
1+
from typing import Any, List, Sequence, Union
22

33
import numpy as np
44
import pandas as pd
@@ -37,3 +37,28 @@ def test_validator(inputs: Any, expected: List[Any], error: bool) -> None:
3737
_validate_input(_ValidateArgument(expected=expected, name="test", value=inputs))
3838
else:
3939
_validate_input(_ValidateArgument(expected=expected, name="test", value=inputs))
40+
41+
42+
@pytest.mark.parametrize(
43+
"inputs,expected,error",
44+
[
45+
# every element matches one of the union members -> valid
46+
(["a", 1, "b", 2], [Sequence[Union[str, int]]], False),
47+
(["a", "b"], [Sequence[Union[str, int]]], False),
48+
([1, 2], [Sequence[Union[str, int]]], False),
49+
# one element (a float) matches neither union member -> must be rejected.
50+
# regression test: the old implementation flattened the per-element check
51+
# into a single any() across (value, union_arg) pairs, so it only took one
52+
# element matching one type to pass the whole sequence, even if other
53+
# elements didn't match anything.
54+
(["a", 3.14], [Sequence[Union[str, int]]], True),
55+
([3.14, "a"], [Sequence[Union[str, int]]], True),
56+
([1, 3.14], [Sequence[Union[str, int]]], True),
57+
],
58+
)
59+
def test_validator_sequence_of_union(inputs: Any, expected: List[Any], error: bool) -> None:
60+
if error:
61+
with pytest.raises(WeaviateInvalidInputError):
62+
_validate_input(_ValidateArgument(expected=expected, name="test", value=inputs))
63+
else:
64+
_validate_input(_ValidateArgument(expected=expected, name="test", value=inputs))

weaviate/validator.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ def _is_valid(expected: Any, value: Any) -> bool:
5656
if len(args) == 1:
5757
if get_origin(args[0]) is Union:
5858
union_args = get_args(args[0])
59-
return any(isinstance(val, union_arg) for val in value for union_arg in union_args)
59+
return all(
60+
any(isinstance(val, union_arg) for union_arg in union_args) for val in value
61+
)
6062
else:
6163
return all(isinstance(val, args[0]) for val in value)
6264
# bool is a subclass of int, so isinstance(True, int) is True. Reject a

0 commit comments

Comments
 (0)