Fix bug with default value of field with discriminator on JSON Schema generation

[vovanbo]

Use case:

@dataclass
class A:
    value = 'a'

@dataclass
class B:
    value = 'b'

@dataclass
class Main:
    value: Annotated[
        A | B,
        Discriminator(field='value', include_supertypes=True)
    ] | None = None

On JSON Schema generation of dataclass with discriminator field and default value an error caused:

    def _compile(self, spec: ValueSpec, lines: CodeLines) -> None:
        if spec.builder.get_config().debug:
            print(f"{type_name(spec.builder.cls)}:")
            print(lines.as_text())
>       exec(lines.as_text(), spec.builder.globals, spec.builder.__dict__)
E         File "<string>", line 8
E           return mashumaro.jsonschema.schema._default.<locals>.CC.__mashumaro_x_variants_72cfc86c11f1448d83799e7e1b77f74c__[discriminator].__mashumaro_from_dict__(value)
E                                                       ^
E       SyntaxError: invalid syntax

The reason is dynamic dataclass generated in _default method in schema.py.

This fix is about avoid usage of "in method" class generation. Instead use globals with unique class name for each unique arguments.

[pep8speaks]

Hello @vovanbo! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2023-12-06 12:12:00 UTC

[Fatal1ty]

Hi @vovanbo

Good catch! It's related to this issue:

• https://github.com/Fatal1ty/mashumaro/pull/180

Actually, it's better to fix this part not only for the JSON Schema builder, but also in general in how discriminated types are handled. It's pretty easy:

diff --git a/mashumaro/core/meta/types/unpack.py b/mashumaro/core/meta/types/unpack.py
index 575a536..58bf223 100644
--- a/mashumaro/core/meta/types/unpack.py
+++ b/mashumaro/core/meta/types/unpack.py
@@ -253,6 +253,11 @@ class DiscriminatedUnionUnpackerBuilder(AbstractUnpackerBuilder):
     def _get_variants_map(self, spec: ValueSpec) -> str:
         variants_attr = self._get_variants_attr(spec)
         if spec.builder.is_nailed:
+            tn = type_name(spec.builder.cls)
+            if "<locals>" in tn:
+                tn = clean_id(tn)
+                spec.builder.ensure_object_imported(spec.builder.cls, tn)
+                return f"{tn}.{variants_attr}"
             return f"{type_name(spec.builder.cls)}.{variants_attr}"
         else:
             return f"{spec.cls_attrs_name}.{variants_attr}"

I'm going to create an issue to track all these "<locals>" problems and fix them all together. There must be a way to avoid duplicating this snippet in different places (I'm currently thinking of adding an additional parameter to the type_name function). You're welcome to use my diff in this PR or just wait until I come up with a general solution.

[Fatal1ty]

Here is the issue to track. I'll try to make some time for it this weekend.

• https://github.com/Fatal1ty/mashumaro/issues/182

[vovanbo]

Thank you for the quick response! It seems your solution is good enough. In my PR, tests broke when using dataclasses declared inside tests, similar to your example here: https://github.com/Fatal1ty/mashumaro/issues/182. I think this PR should be closed. I hope some tests from this PR will be useful for you ;)

[Fatal1ty]

Thank you for the quick response! It seems your solution is good enough. In my PR, tests broke when using dataclasses declared inside tests, similar to your example here: #182. I think this PR should be closed. I hope some tests from this PR will be useful for you ;)

Thank you for the tests! I’m sure they will be useful!