Fix some decoding of subsubtypes in general and std::Object in particular (#938)

I ran into a few things here:
 - The object.pyx code to handle decoding to subclasses only looks
   at `__subclasses__`, and so misses grandchildren types. I clearly
   remember thinking about this and testing the grandchild case,
   but apparently I missed something there.
 - Descendants of std::Object in particular get immediately screwy,
   because user-facing types don't inherit from std::Object directly,
   but instead their `__shapes__` do.
 - The new "`tname__` in `__shapes__`" mechanism (#926) was putting
   wrong stuff in the map in some cases.

My approach is to explicitly indicate which classes should be
considered the canonical decoding classes with a `__gel_is_canonical__
= True` field on the class. And for now, I'm populating it explicitly,
though we can probably write a bunch of nasty logic to figure it out
too.

Author: msullivan

gel/_internal/_codegen/_models/_pydantic.py

@@ -4358,6 +4358,9 @@ def _write_base_object_type_body(
         include_tname: bool = False,
     ) -> None:
         if include_tname:
+            if objtype.name in CORE_OBJECTS:
+                self.write('__gel_is_canonical__ = True')
+
             type_name = objtype.schemapath
             literal = self.import_name("typing", "Literal")
             field = self.import_name("pydantic", "Field")
@@ -4734,6 +4737,7 @@ def write_object_type(
             ),
         ):
             self.write(f'"""type {objtype.name}"""')
+            self.write('__gel_is_canonical__ = True')
             self.write()
             pointers = _get_object_type_body(objtype)
             if pointers:
@@ -4746,9 +4750,6 @@ def write_object_type(
                         localns=localns,
                     )
                     self._write_model_attribute(ptr.name, ptr_type)
-            else:
-                self.write("pass")
-                self.write()
 
     def _write_model_attribute(self, name: str, anno: str) -> None:
         decl = f"{name}: {anno}"

gel/_internal/_qb/_reflection.py

@@ -35,6 +35,11 @@ class GelSchemaMetadata:
     class __gel_reflection__:  # noqa: N801
         name: ClassVar[SchemaPath]
 
+    # Whether this class is a "canonical" type - that is, the primary
+    # representation of a database type, not an internal __shape__ class,
+    # or a user-defined subtype, or anything else.
+    __gel_is_canonical__: ClassVar[bool]
+
 
 class GelSourceMetadata(GelSchemaMetadata):
     class __gel_reflection__(GelSchemaMetadata.__gel_reflection__):  # noqa: N801

gel/_internal/_qbmodel/_abstract/_base.py

@@ -167,30 +167,13 @@ def __new__(
         reflection = cls.__gel_reflection__
         if (
             # The class registry only tracks the canonical base instances,
-            # which are the ones with a base that directly declares 'tname__'
-            # but do not declare it themselves.
-            #
-            # Eg.
-            #
-            #   default/__init__.py :
-            #     class Foo(base_shapes.Foo):
-            #       ...
-            #
-            #   __shapes__/default/__init__.py :
-            #     class Foo(__Foo_default_shape__, __gel_shape__="RequiredId"):
-            #       tname__: Literal["default::Foo"] = Field(
-            #         "default::Foo", alias="__tname__")
-            #
-            # The user facing default.Foo does not directly declare tname__,
-            # but base_shapes.Foo does.
-            #
-            # For a derived `type Bar extending Foo`, default.Bar does not
-            # directly declare tname__, but base_shapes.Bar does.
+            # which are declared with __gel_is_canonical__ = True.
             (tname := getattr(reflection, "name", None)) is not None
-            and any('tname__' in base.__dict__ for base in bases)
-            and 'tname__' not in namespace
+            and namespace.get('__gel_is_canonical__')
         ):
             mcls.__gel_class_registry__[str(tname)] = cls
+        else:
+            cls.__gel_is_canonical__ = False
         cls.__gel_shape__ = __gel_shape__
         return cls
 

gel/protocol/codecs/object.pyx

@@ -358,26 +358,27 @@ cdef class ObjectCodec(BaseNamedRecordCodec):
             self.cached_return_type = return_type
             self.cached_return_type_proxy = None
 
-        # Store base type's tname in the mapping *also* to exclude
-        # subclasses of base type, e.g. if we have this:
-        #
-        #    class CustomContent(default.Content):
-        #        pass
-        #
-        # then default.Content.__subclasses__() will contain
-        # CustomContent, which we don't want to be there.
-
-        tname_map = {str(refl.name): self.cached_return_type}
-
-        for ch in self.cached_return_type.__subclasses__():
+        # Build a map of descendant types that are marked as being
+        # canonical targets.  Make sure to descend through types not
+        # marked canonical, though, because the
+        # std::Object/std::BaseObject type only get inherited via the
+        # __shapes__ types, and we'll need to descend through that to
+        # get to the real ones.
+        worklist = [self.cached_return_type]
+        tname_map = {}
+        while worklist:
+            ch = worklist.pop()
             try:
                 sub_name = ch.__gel_reflection__.name
+                canonical = ch.__gel_is_canonical__
             except AttributeError:
                 pass
             else:
-                # setdefault only sets it if isn't set already, so
-                # custom subtypes won't be picked up.
-                tname_map.setdefault(str(sub_name), ch)
+                sname = str(sub_name)
+                if sname not in tname_map:
+                    worklist.extend(ch.__subclasses__())
+                    if canonical:
+                        tname_map[sname] = ch
 
         subs = []
         dlists = []

tests/dbsetup/orm.gel

@@ -82,7 +82,7 @@ type Party extending ExclusivelyNamed {
 
 # Object with required multi link, has link props
 type Raid extending ExclusivelyNamed {
-    required multi members: User {
+    required multi members: Named {
         role: str;
         rank: int64;
     };

tests/test_model_generator.py

@@ -261,7 +261,6 @@ def test_modelgen_06(self):
         self.assertIsInstance(user, default.User)
         self.assertIsInstance(user, default.CustomUser)
 
-    @tb.xfail("links to std::Object don't decoded to the object's actual type")
     def test_modelgen_07(self):
         from models.orm import default, std
 
@@ -1375,7 +1374,6 @@ def test_modelgen_pydantic_apis_11d(self):
         self.assertPydanticPickles(t)
         self.assertPydanticSerializes(t)
 
-    @tb.xfail("links to std::Object don't decoded to the object's actual type")
     def test_modelgen_pydantic_apis_11e(self):
         # Test model_dump() and model_dump_json() on models;
         # test *single required* link serialization in all combinations
@@ -1392,7 +1390,6 @@ def test_modelgen_pydantic_apis_11e(self):
         self.assertPydanticPickles(t)
         self.assertPydanticSerializes(t)
 
-    @tb.xfail("links to std::Object don't decoded to the object's actual type")
     def test_modelgen_pydantic_apis_11f(self):
         # Test model_dump() and model_dump_json() on models;
         # test *single required* link serialization in all combinations