OneOf for sealed classes imported from another module

[f1qwase]

Hi @dzikoysk thank you for the great support of @OneOf for sealed classes. Unfortunately, recently I faced a new issue connected to it.

Previously I had only one submodule requiring OpenAPI spec but recently I got another one. And this second one uses some API classes from the first one. And there I discovered that automatic filling and mapping of @OneOf is not working for sealed classes imported from another module.

After looking into the annotation processor code I realized that it happens because the processor searches for candidate classes for oneOf list – (subtypes of the given sealed class) among the list of classes annotated with @DiscrimnatorMappingName which doesn't include any classes from an external module.

I couldn't find any quick solution to this problem and have to duplicate such classes in every module that uses them.
Do you have any idea of how to make @OneOf work properly (the same way as it works in its own module)? I'm ready to try to implement it if you have one.

[dzikoysk]

It's a little bit tricky, because annotation processing is supposed to work only for current sources - other modules behave quite the same as external dependencies, these sources were already processed and compiled before, so there's no point to repeat that process over and over again.

I don't quite remember the limitations of AP, but assuming we can get an annotation from an external class, maybe we could just extend OneOf annotation, so there's a possibility to explicitly list all mappings. So instead of:

@OneOf(
    discriminator = Discriminator(
        property = DiscriminatorProperty(
            name = "type",
            type = String::class,
            injectInMappings = true
        )
    )
)
sealed interface Union

@DiscriminatorMappingName("class-a")
data class A(val a: Int) : Union

@DiscriminatorMappingName("class-b")
@OpenApiName("B")
data class C(val b: String) : Union

we could allow defining it like that:

@OneOf(
    discriminator = Discriminator(
        property = DiscriminatorProperty(
            name = "type",
            type = String::class,
            mappings = [
              Mapping("class-a", A::class),
              Mapping("class-b", B::class),
            ]
        )
    )
)
sealed interface Union

data class A(val a: Int) : Union

@OpenApiName("B")
data class C(val b: String) : Union

I think that then we could be able to get it from Union::class 🤔

[f1qwase]

Thanks for a quick reply @dzikoysk

Duplicating all classes with their dependencies becomes too massive, so for now I ended up with a code pretty close to your second example with one difference: the A and B classes are also required to be added to @OneOf.value list explicilty to be listed in oneOf in resulting schema.

This is not enough though, because if I do so, injectMapping is not working (debug revealed that it happens because subclasses are first added to references from @OneOf.value (without discriminator injected) and then NOT added from mappings (with discrimator injected) because they are already in references (without discriminator).

---

I like that the approach you are suggesting allows usuto take the list of classes for oneOf values directly from mappings, it also should resolve the issue with injecting discrimator property. The only thing that worries me is that this approach assumes that oneOf values can be set in 4 different ways:

• manually from @OneOf.value
• from classes annotated with @DiscrimnatorMappingName
• from @OneOf.discriminator.property.mapping // new in this approach
• from @OneOf.discriminator.mapping // you didn't mention it but it looks consistent to support it as well, since creating OneOf.values from @OneOf.discriminator.property.mapping is supported

Such a number of options to do the same looks a bit complicated especially when they are allowed all at the same time and there is no clear priority of one over another.

---

There are several things I would like to discuss:

where to have mappings

Now they are in @Discriminator.mappings. You propose to add them to @DiscriminatorProperty while I would rather move them to @OneOf but keeping them in @Discrimintor is fine, I just don't understand why to add another mappings to @DiscriminatorProperty.

mapping source priority

There are multiple sets of classes we want to use as mapping values:

1. classes from Discriminator.mappings
2. subclasses of given classes annotated with DiscriminatorMappingName

Now Discriminator.mappings are taken if not empty, otherwise the annotated classes are taken. To me silently ignoring annotated classes is not an obvious behaviour, so I would like to propose several alternatives:

1. throw an error if both are present
2. merge them (if there is no conflicts, e.g. same class has one name in annotation and another one in mapping)

oneOf values (content of oneOf list in spec) source priority

Same as mappings (we want to get oneOf values from mapping values, don't we?) plus @OneOf.values itself – pretty much the same proposal: either forbid more than one source or merge them

–––

Please tell what do you think of all this.
Sorry for this wall of text and many thanks for your time.
I would like implement changes myself but I need your expirience and of coarse approval of what to implement.

[dzikoysk]

The only thing that worries me is that this approach assumes that oneOf values can be set in 4 different ways:
where to have mappings [...]

I agree, let's keep it as simple as possible. I didn't pay that much attention into details, I'm completely fine with having mappings at root level, so directly on @OneOf annotation. Let's focus on that, because it should be enough to cover this case.

mapping source priority [...]

I'd personally throw an exception for now - it seems to be simpler. In case someone will face an issue with that, we can always go back to this discussion and implement that having a real world use-case.

---

So summing up - we'll have pretty much something like that:

@OneOf(
    discriminator = Discriminator(
        property = DiscriminatorProperty(
            name = "type",
            type = String::class,
        )
    ),
    mappings = [
        Mapping("class-a", A::class),
        Mapping("class-b", B::class),
    ]
)
sealed interface Union

1. Whenever mappings are used, DiscriminatorMappingName will throw an exception to avoid duplicated mappings.
2. Whenever mappings are used, it replaces default values - in case both are used, it also throws an exception.

If it looks correct to you, feel free to open a PR 🙏