openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG][Scala] OneOf generates incorrect classes.

Open GerretS opened this issue 1 year ago • 4 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [x] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
  • [x] What's the actual output vs expected output?
  • [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

When generating a model using scala-sttp, oneOf in the OpenAPI generates case classes that can never match any actual valid json. I also tried some of the other scala generators and they seem to have the same problem.

openapi-generator version

Latest master

OpenAPI declaration file content or url

Link to OpenAPI yaml gist

Generation Details
java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -i /tmp/test/example.yaml -g scala-sttp -o /tmp/test/out
Steps to reproduce

If you look in the model package you will see that the specific animal types (Dog, Cat) are all perfectly fine. However, the top-level Example case class has a field of type Animal:

case class Example(
  requestId: Option[String] = None,
  animal: Option[Animal] = None
)

And here is the generated Animal case class:

case class Animal(
  dogName: String,
  foodType: Option[CapybaraFoodType] = None,
  catId: Int,
  capyBaraId: Option[Int] = None
)

There are two problems with this:

  • dogName and catId are only required in Dog and Cat respectively. This means that it is impossible to deserialize an Example containing a Dog unless you provide a catId for the Dog.
  • the foodType is set to CapybaraFoodType even though Dog and Cat each have their own FoodType schema. An Example containing a Cat or Dog with a CatFoodType or DogFoodType cannot be modelled at all, even though it's valid according to the spec.
Suggest a fix

The most straightforward fix would be to make Animal an empty trait, and have Cat, Dog and Capybara be subtypes of this trait. The Example then contains the Animal. Then, any (de)serializer could use the discriminator in the yaml to make an instance of the correct subtype.

Note that a trait is better than an abstract class in this case, because the OpenAPI spec allows an object to be reused among multiple oneOf statements for different objects, and a case class can extend multiple traits but only one abstract class.

I realize there might be more work to get the compatible serializer libs to work correctly with this new change, but since this case currently isn't supported at all. I would be very happy with just being able to generate a valid model as a first step.

GerretS avatar Oct 16 '24 16:10 GerretS

@GerretS thanks for reporting the issue.

I would suggest you try the java client generator to see if the implementation works for you.

If it does, we can try to plot it to the scala generators.

wing328 avatar Oct 17 '24 05:10 wing328

Hi @wing328 thanks for your response.

If I run the same command but with -g java, it generates a different structure which I haven't tested but it seems correct:

Example still has a field of type Animal. Animal itself is in this case a class extending AbstractOpenApiSchema. It implements a special TypeAdapter that contains methods to read/write json where it simply tries each concrete type in turn. Here's a snippet:

    // check if the actual instance is of the type `Dog`
                    if (value.getActualInstance() instanceof Dog) {
                        JsonElement element = adapterDog.toJsonTree((Dog)value.getActualInstance());
                        elementAdapter.write(out, element);
                        return;
                    }
                    // check if the actual instance is of the type `Cat`
                    if (value.getActualInstance() instanceof Cat) {
                        JsonElement element = adapterCat.toJsonTree((Cat)value.getActualInstance());
                        elementAdapter.write(out, element);
                        return;
                    }
              ...

Then there are public methods within an Animal instance that gives you the underlying type. You'd then have to do an instanceOf check and try casting it to the right type. A rather different solution from what I proposed for Scala but this does seem to work correctly. So it is indeed a bug in Scala specifically.

GerretS avatar Oct 17 '24 07:10 GerretS

@GerretS thanks for doing the test.

May I know if you've time to contribute a PR to add the same or similar implementation to the Scala client generator?

wing328 avatar Oct 17 '24 07:10 wing328

I tried to take a look but I don't really know where to get started on this. Perhaps if someone can give me some pointers on how and where the Scala generator is currently implemented.

GerretS avatar Oct 17 '24 08:10 GerretS