Skip to content

Commit cdb9f1f

Browse files
committed
fixed edge bug with Type_Safe validation on types, where we needed to walk the __mro__ to see if one of the base classes is also a match
1 parent 1e7b74a commit cdb9f1f

File tree

4 files changed

+145
-5
lines changed

4 files changed

+145
-5
lines changed

osbot_utils/helpers/cache_on_self/Cache_Storage.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from weakref import WeakKeyDictionary
2-
from typing import Any, Dict, List
2+
from typing import Any, List
33

44
class Cache_Storage: # Handles all cache storage without polluting instance attributes
55

osbot_utils/type_safe/type_safe_core/shared/Type_Safe__Validation.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,14 @@ def check_if__type_matches__obj_annotation__for_attr(self, target, attr_name, va
208208
if type_arg == value:
209209
return True
210210
if isinstance(type_arg, (str, ForwardRef)): # Handle forward reference
211-
type_arg = target.__class__ # If it's a forward reference, the target class should be the containing class
211+
forward_name = type_arg.__forward_arg__ if isinstance(type_arg, ForwardRef) else type_arg
212+
resolved_type = None # Walk MRO to find the actual class
213+
for base_cls in target.__class__.__mro__:
214+
if base_cls.__name__ == forward_name:
215+
resolved_type = base_cls
216+
break
217+
type_arg = resolved_type if resolved_type else target.__class__ # Fallback to instance class if not found
218+
212219
return isinstance(value, type) and issubclass(value, type_arg) # Check that value is a type and is subclass of type_arg
213220
else:
214221
return isinstance(value, type)

tests/unit/type_safe/type_safe_core/_bugs/test_Type_Safe__bugs.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import re
2+
from typing import Type
3+
14
import pytest
25
from unittest import TestCase
36
from osbot_utils.type_safe.Type_Safe import Type_Safe
@@ -68,4 +71,3 @@ def __init__(self): # this will make the __annotations__ to
6871

6972
assert an_class.an_str == 'new_value'
7073
assert an_class.an_bool == False
71-

tests/unit/type_safe/type_safe_core/_regression/test_Type_Safe__regression.py

Lines changed: 133 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,8 +413,7 @@ class Custom_Complex(Complex_Node): pass
413413
custom_complex = Custom_Complex() # Should not raise TypeError
414414

415415
# And type checking should work properly
416-
with self.assertRaises(ValueError):
417-
custom_complex.node_type = Complex_Node # Doesn't Allow base class
416+
custom_complex.node_type = Complex_Node # Allow base class
418417
custom_complex.node_type = Custom_Complex # Allow self
419418

420419
with self.assertRaises(ValueError):
@@ -1704,3 +1703,135 @@ class An_Class(Type_Safe):
17041703
# An_Class(an_custom_safe_float = Safe_Float(0.5))
17051704

17061705
assert type(An_Class(an_custom_safe_float = Safe_Float(0.5)).an_custom_safe_float) is Custom_Safe_Float
1706+
1707+
1708+
def test__regression__forward_ref__inheritance__type_annotation_resolves_to_wrong_class(self):
1709+
# Bug: When a subclass inherits a Type[ForwardRef] annotation from a parent class,
1710+
# the ForwardRef is incorrectly resolved to the subclass instead of the
1711+
# actual class named in the ForwardRef.
1712+
#
1713+
# Root cause: In Type_Safe__Validation.check_if__type_matches__obj_annotation__for_attr(),
1714+
# ForwardRef handling uses `target.__class__` which is the instance's class,
1715+
# not the class actually named in the ForwardRef.
1716+
1717+
class Base_Class(Type_Safe):
1718+
node_type: Type['Base_Class'] = None # ForwardRef to Base_Class
1719+
1720+
class Sub_Class(Base_Class): # Inherits node_type annotation
1721+
pass
1722+
1723+
# Verify inheritance is set up correctly
1724+
assert issubclass(Sub_Class, Base_Class) is True
1725+
1726+
# Direct use on Base_Class works correctly
1727+
assert Base_Class( ).node_type is None
1728+
assert Base_Class(node_type=Base_Class).node_type is Base_Class
1729+
assert Base_Class(node_type=Sub_Class ).node_type is Sub_Class # Subclass is valid for Type[Base_Class]
1730+
1731+
# Setter on Base_Class also works
1732+
base_instance = Base_Class()
1733+
base_instance.node_type = Base_Class # Works correctly
1734+
assert base_instance.node_type is Base_Class
1735+
1736+
base_instance_2 = Base_Class()
1737+
base_instance_2.node_type = Sub_Class # Subclass assignment works
1738+
assert base_instance_2.node_type is Sub_Class
1739+
1740+
# BUG: Sub_Class inherits Type['Base_Class'] but ForwardRef resolves to Sub_Class
1741+
# This causes the validation to check: issubclass(Base_Class, Sub_Class) -> False
1742+
# When it SHOULD check: issubclass(Base_Class, Base_Class) -> True
1743+
1744+
sub_instance = Sub_Class()
1745+
# error_message = "On Sub_Class, invalid type for attribute 'node_type'. Expected 'typing.Type[ForwardRef('Base_Class')]' but got '<class"
1746+
# with pytest.raises(ValueError, match=re.escape(error_message)):
1747+
# sub_instance.node_type = Base_Class # BUG: should work but fails
1748+
sub_instance.node_type = Base_Class # FIXED
1749+
1750+
# with pytest.raises(ValueError, match=re.escape(error_message)):
1751+
# Sub_Class(node_type=Base_Class) # BUG: constructor also fails
1752+
Sub_Class(node_type=Base_Class) # FIXED
1753+
# Note: Assigning Sub_Class works because issubclass(Sub_Class, Sub_Class) is True
1754+
# (the wrong resolution accidentally passes for this case)
1755+
sub_instance_2 = Sub_Class()
1756+
sub_instance_2.node_type = Sub_Class # This works (accidentally)
1757+
assert sub_instance_2.node_type is Sub_Class
1758+
1759+
def test__regression__forward_ref__inheritance__with_setter_method(self):
1760+
# Same bug manifests when using setter methods
1761+
1762+
class Schema__Node(Type_Safe):
1763+
node_type: Type['Schema__Node'] = None
1764+
1765+
def set_node_type(self, node_type: Type['Schema__Node'] = None):
1766+
self.node_type = node_type or self.__class__
1767+
return self
1768+
1769+
class Schema__Node__Extended(Schema__Node):
1770+
extra_field: str = ''
1771+
1772+
# Base class setter works correctly
1773+
node = Schema__Node()
1774+
node.set_node_type(Schema__Node)
1775+
assert node.node_type is Schema__Node
1776+
1777+
# BUG: Subclass setter fails when setting to base class type
1778+
extended_node = Schema__Node__Extended()
1779+
error_message = "On Schema__Node__Extended, invalid type for attribute 'node_type'. Expected 'typing.Type[ForwardRef('Schema__Node')]' but got '<class"
1780+
# with pytest.raises(ValueError, match=re.escape(error_message)):
1781+
# extended_node.set_node_type(Schema__Node) # BUG: should work but fails
1782+
extended_node.set_node_type(Schema__Node) # FIXED: now works
1783+
1784+
def test__regression__forward_ref__inheritance__deep_hierarchy(self):
1785+
# Bug also affects deeper inheritance chains
1786+
1787+
class Level_0(Type_Safe):
1788+
type_ref: Type['Level_0'] = None
1789+
1790+
class Level_1(Level_0):
1791+
pass
1792+
1793+
class Level_2(Level_1):
1794+
pass
1795+
1796+
# All levels should accept Level_0 (the ForwardRef target)
1797+
assert Level_0(type_ref=Level_0).type_ref is Level_0 # Works
1798+
assert Level_0(type_ref=Level_1).type_ref is Level_1 # Works
1799+
assert Level_0(type_ref=Level_2).type_ref is Level_2 # Works
1800+
1801+
# error_message = "On Level_1, invalid type for attribute 'type_ref'. Expected 'typing.Type[ForwardRef('Level_0')]' but got '<class"
1802+
# with pytest.raises(ValueError, match=re.escape(error_message)):
1803+
# Level_1(type_ref=Level_0) # BUG: should work
1804+
Level_1(type_ref=Level_0) # FIXED
1805+
1806+
# error_message_2 = "On Level_2, invalid type for attribute 'type_ref'. Expected 'typing.Type[ForwardRef('Level_0')]' but got '<class"
1807+
# with pytest.raises(ValueError, match=re.escape(error_message_2)):
1808+
# Level_2(type_ref=Level_0) # BUG: should work
1809+
Level_2(type_ref=Level_0) # FIXED
1810+
1811+
# with pytest.raises(ValueError, match=re.escape(error_message_2)):
1812+
# Level_2(type_ref=Level_1) # BUG: should work (Level_1 is subclass of Level_0)
1813+
Level_2(type_ref=Level_1) # FIXED
1814+
1815+
def test__regression__forward_ref__expected_behavior_after_fix(self):
1816+
# This documents what SHOULD work after the bug is fixed
1817+
# Currently all these raise ValueError
1818+
1819+
class Parent(Type_Safe):
1820+
ref: Type['Parent'] = None
1821+
1822+
class Child(Parent):
1823+
pass
1824+
1825+
# After fix, all of these should work:
1826+
# - Child instance should accept Parent type (Parent is subclass of Parent)
1827+
# - Child instance should accept Child type (Child is subclass of Parent)
1828+
# - The ForwardRef 'Parent' should resolve to Parent class, not to Child
1829+
1830+
# For now, document that these fail:
1831+
# with pytest.raises(ValueError):
1832+
# Child(ref=Parent) # BUG: fails but should work
1833+
Child(ref=Parent) # FIXED
1834+
1835+
# This works (because Child is subclass of incorrectly-resolved Child)
1836+
child = Child(ref=Child)
1837+
assert child.ref is Child # Works (accidentally correct)

0 commit comments

Comments
 (0)