Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-required fields with default values not seriailizing the default value in a simpleRestJson #1405

Closed
mattdavpir opened this issue Feb 16, 2024 · 4 comments

Comments

@mattdavpir
Copy link

Hey, I'm not sure if this is intended behaviour or not, but it was unexpected from my end.

It seems that when I return a default value for a non-required field it is being omitted from the generated JSON, e.g. for this example:

MyService .smithy

$version: "2.0"

namespace com.test

use alloy#simpleRestJson

@simpleRestJson
@httpBearerAuth
service MyService {
    version: "1.1.0",
    operations: [Overview]
}

@http(method: "GET", uri: "/overview", code: 200)
operation Overview {
    output: OverviewTable
}

structure OverviewTable {
    items: OverviewResponses
}

list OverviewResponses {
    member: OverviewResponse
}

structure OverviewResponse {
    stringField: String,
    sortable: Boolean = true
}

build.sbt

ThisBuild / version := "0.1.0-SNAPSHOT"

ThisBuild / scalaVersion := "2.13.12"


lazy val root = (project in file("."))
  .settings(
    name := "smithy-test",
    libraryDependencies ++= Seq(
      Dependencies.smithy4sCore,
      Dependencies.smithy4sHttp4s,
      Dependencies.http4sDsl,
      Dependencies.http4sEmberServer,
      Dependencies.zio,
      Dependencies.zioInteropCats,
    ),
    addCompilerPlugin(("org.typelevel" % "kind-projector" % "0.13.2").cross(CrossVersion.full)),
  )
  .enablePlugins(Smithy4sCodegenPlugin)

Dependencies.scala

import sbt.*

object Dependencies {
  private val smithy4sV = "0.18.8"
  private val http4sV = "0.23.25"
  private val zioV = "2.0.21"
  private val zioInteropCatsV = "23.1.0.0"

  val smithy4sCore = "com.disneystreaming.smithy4s" %% "smithy4s-core" % smithy4sV
  val smithy4sHttp4s = "com.disneystreaming.smithy4s" %% "smithy4s-http4s" % smithy4sV
  val http4sDsl = "org.http4s" %% "http4s-dsl" % http4sV
  val http4sEmberServer = "org.http4s" %% "http4s-ember-server" % http4sV

  val zio = "dev.zio" %% "zio" % zioV
  val zioInteropCats = "dev.zio" %% "zio-interop-cats" % zioInteropCatsV

}

Main.scala

import com.comcast.ip4s.{Host, Port}
import com.test.{MyService, OverviewResponse, OverviewTable}
import org.http4s.ember.server.EmberServerBuilder
import smithy4s.Transformation
import smithy4s.http4s.SimpleRestJsonBuilder
import zio.interop.catz.asyncInstance
import zio.{RIO, Task, ZIO, ZIOAppDefault}

import scala.util.chaining.scalaUtilChainingOps

object Main extends ZIOAppDefault {
  override def run = {
    MyServiceImpl.routes.flatMap { routes =>
      EmberServerBuilder.default[Task]
        .withHost(Host.fromString("127.0.0.1").get)
        .withPort(Port.fromInt(8080).get)
        .withHttpApp(routes.orNotFound)
        .build
        .use(_ => ZIO.never)
    }
  }

  class MyServiceImpl extends MyService.ErrorAware[ZIO[Any, *, *]] {
    override def overview(): ZIO[Any, Nothing, OverviewTable] = ZIO.succeed(
      OverviewTable(
        Some(
          List(
            OverviewResponse(sortable = true, stringField = Some("Hooray")),
            OverviewResponse(sortable = true, stringField = None),
            OverviewResponse(sortable = false, stringField = Some("Boo")),
          )
        )
      )
    )
  }

  object MyServiceImpl {

    def routes = {
      val service = new MyServiceImpl().transform(absorbErrors)

      SimpleRestJsonBuilder
        .routes(service)
        .make
        .pipe(ZIO.fromEither(_).orDie)
    }

    private def absorbErrors = new Transformation.AbsorbError[ZIO[Any, *, *], RIO[Any, *]] {
      def apply[E, A](fa: ZIO[Any, E, A], injectError: E => Throwable): ZIO[Any, Throwable, A] =
        fa.mapError(injectError)
    }
  }
}

When I hit the endpoint the response is:
{"items":[{"stringField":"Hooray"},{},{"sortable":false,"stringField":"Boo"}]}

I was expecting the "sortable": true to be included in the serialized objects, which was the behaviour in 0.17.8 (maybe an intentional change in v0.18.0, but I couldn't find a mention of it in the change logs). If I set @required on the sortable field it will serialize the default properly.

@daddykotex
Copy link
Contributor

Hey @mattdavpir , hope you're great. You are right there were some changes in 0.18.x regarding default and optional fields during encoding.

You can configure the behaviour though:

SimpleRestJsonBuilder
        .routes(service)
        .withExplicitDefaultsEncoding(true)
        .make
        .pipe(ZIO.fromEither(_).orDie)

That should give you what you want.

@daddykotex
Copy link
Contributor

Related #1290 and #1315

@Baccata
Copy link
Contributor

Baccata commented Feb 16, 2024

The behaviour is indeed working as intended now: non-required fields are skipped during serialisation when they have the default value.

We ought to document the expected behaviour better, but I'm making the decision to close this issue.

That being said, thank you very much for the detailed issue, it is very much appreciated and I would love it if every issue opened by users was written with a similar level of attention.

@mattdavpir
Copy link
Author

Ah brilliant, thanks for the quick responses!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants