swagger-akka-http / swagger-scala-module

Swagger support for scala
Apache License 2.0
10 stars 10 forks source link

Improper or Alternative Handling of Required Properties #204

Closed leedsalex closed 1 year ago

leedsalex commented 1 year ago

In the current state, a non-annotated model with a default value, such as:

case class ModelWithInt(int: Int = 0)

will resolve to the following schema:

ModelWithInt:
  type: object
  properties:
    int:
      type: integer
      default: 0

However, most code generators will interpret this schema as:

case class ModelWithInt(int: Option[Int] = Some(0))

In other languages, where Option types don't exist, this property would be a pointer or something similar.

I propose that we change the ability to use the default value as a determining factor for optionality (i.e marking a property required) a configurable option.

leedsalex commented 1 year ago

I realize this is a new change mentioned in the README:

v2.7 takes default values into account - either those specified in Scala contructors or via swagger annotations. A field might be marked as not required if a default value is specified.

I can see how this would be beneficial to some people, however, scala JSON libraries such as spray-json do not interpret default values in a similar fashion.

pjfanning commented 1 year ago

Can't you just use v2.6? It's impossible to keep everyone happy.

pjfanning commented 1 year ago

If you want to do a PR, there is a possibility to add a new setting like https://github.com/swagger-akka-http/swagger-scala-module/blob/develop/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala#L41

The existing main branch behaviour would remain the default but a configuration could make it behave differently.

leedsalex commented 1 year ago

I will happily do a PR to add this behavior as a setting, thanks!