response header doesn't seem to work

[willgu903]

framework: akka-http
scala: 2.12.11
akka: 2.6.4
circe: 0.13.0

i tried the following example

https://github.com/twilio/guardrail/blob/master/modules/sample/src/main/resources/response-headers.yaml

the generated code is

trait Handler { def getFoo(respond: Resource.getFooResponse.type)(): scala.concurrent.Future[Resource.getFooResponse] }
object Resource {
  def routes(handler: Handler)(implicit mat: akka.stream.Materializer): Route = {
    {
      get(path("foo")(discardEntity(complete(handler.getFoo(getFooResponse)()))))
    }
  }
  sealed abstract class getFooResponse(val statusCode: StatusCode)
  case object getFooResponseOK extends getFooResponse(StatusCodes.OK)
  case object getFooResponseNoContent extends getFooResponse(StatusCodes.NoContent)
  object getFooResponse {
    implicit val getFooTRM: ToResponseMarshaller[getFooResponse] = Marshaller { implicit ec => 
      resp => getFooTR(resp)
    }
    implicit def getFooTR(value: getFooResponse)(implicit ec: scala.concurrent.ExecutionContext): scala.concurrent.Future[List[Marshalling[HttpResponse]]] = value match {
      case r: getFooResponseOK.type =>
        scala.concurrent.Future.successful(Marshalling.Opaque {
          () => HttpResponse(r.statusCode)
        } :: Nil)
      case r: getFooResponseNoContent.type =>
        scala.concurrent.Future.successful(Marshalling.Opaque {
          () => HttpResponse(r.statusCode)
        } :: Nil)
    }
    def apply[T](value: T)(implicit ev: T => getFooResponse): getFooResponse = ev(value)
    def OK: getFooResponse = getFooResponseOK
    def NoContent: getFooResponse = getFooResponseNoContent
  }
}

I couldn't find the interface for response headers. could you please help?

[blast-hardcheese]

Please pardon the late reply

I think this is actually a missing feature.

If you look at the definition in AkkaHttpServerGenerator.scala#L37-L57, you can see the object resp's .headers field is never accessed, whereas in Http4sServerGenerator.scala#L481-L505, the http4s generator clearly consumes that field through the binding of headers.

If you are available to contribute this, it would be greatly appreciated, currently my focus has been on performance improvements of the core generator, as well as better error messages for consumers. As a basic workaround, you can use x-server-raw-response: true in routes that you need to pass response headers in, which gives you the ability to emit a HttpResponse directly -- not a great workaround, but something that can be done in this case if you haven't managed to find a workaround yet.

Anyway, I do apologize for not responding sooner. Please feel free to reach out if you are interested in contributing but need help getting started 😁

[willgu903]

Thanks for your suggestions :)
I'm a rookie in this realm and still learning. Would love to contribute to this great project later as I'm climbing up the ladder.

[Peppi-Ressler]

Hi. I really need this feature in our team (Avast) and I am willing to make a PR. Probably during next week I will be able to deliver PR. Would you make some time for me? Thanks

[blast-hardcheese]

@Peppi-Ressler
Certainly, and thank you in advance for the contribution. The recent work towards a more modular codebase has been completed, no more merge conflicts expected.

[blast-hardcheese]

@Peppi-Ressler One other note -- Due to #1195, the intent is for future versions of sbt-guardrail to not depend directly on all modules, preferring instead to

addSbtPlugin("dev.guardrail" % "sbt-guardrail" % "0.66.0")
libraryDependencies += "dev.guardrail" %% "guardrail-scala-akka-http" % "0.66.1"

with the expectation that most users are using something like scala-steward or dependabot to bump library versions. This is to prevent unnecessary version bumps due to unrelated changes to frameworks your projects do not use.

Does this match with Avast's usage expectations?

I've been discussing it a bit in the gitter channel, but I'd like to make sure this isn't a surprise to you all.

[Peppi-Ressler]

Regarding PR itself I already have working version but I need to fix tests (also as this is my first meta encounter, I think there will be "few" things to fix). That modules sounds cool, I think it will be ok.

[Peppi-Ressler]

Ok, I've managed to add also tests. That meta part is little bit messy but as I said I don't have experience with that so I am ready to fix it based on your comments :)

[Peppi-Ressler]

@Peppi-Ressler One other note -- Due to #1195, the intent is for future versions of sbt-guardrail to not depend directly on all modules, preferring instead to

addSbtPlugin("dev.guardrail" % "sbt-guardrail" % "0.66.0")
libraryDependencies += "dev.guardrail" %% "guardrail-scala-akka-http" % "0.66.1"

with the expectation that most users are using something like scala-steward or dependabot to bump library versions. This is to prevent unnecessary version bumps due to unrelated changes to frameworks your projects do not use.

Does this match with Avast's usage expectations?

I've been discussing it a bit in the gitter channel, but I'd like to make sure this isn't a surprise to you all.

Only issue is our gradle plugin which will need some updates but it should not be a big deal

[blast-hardcheese]

@Peppi-Ressler Ah, understood. I put some effort to upgrading the guardrail version in the gradle plugin here, but I admit I am actually not much of a Groovy/Gradle expert. If you have your own internal plugin, then perhaps the repackaging notes will help you.

[Peppi-Ressler]

@Peppi-Ressler Ah, understood. I put some effort to upgrading the guardrail version in the gradle plugin here, but I admit I am actually not much of a Groovy/Gradle expert. If you have your own internal plugin, then perhaps the repackaging notes will help you.

Thank you I will take a look. When are you going to switch to modularized approach?

[blast-hardcheese]

When are you going to switch to modularized approach?

This was done to support a 1.0 release, to ensure no other major shifts will be done. I will wait until there is enough evidence that each module has a stable ABI before moving to the model where each user specifies their module dependencies explicitly.

One other note, since you are implementing in Gradle, in the next release of guardrail-core I will expose an abstract class that extends this trait with java semantics to make it easier to consume from languages other than Scala.

Presumably if you specify a newer version of one module, even in Gradle, it will override the built-in dependency

[blast-hardcheese]

guardrail-scala-akka-http 0.67.0 is releasing, please let me know if anything else is necessary to support you here

[Peppi-Ressler]

@blast-hardcheese thanks a lot. Last question: I pressume that I need to change my plugin in same way as you did here in order to actually use this new release right?

[blast-hardcheese]

@Peppi-Ressler Certainly -- if you are using the CLI style approach that we are using as well you will need to do so. 0.65.x was going back and forth trying to get the dependencies to work out, I would just go directly to 0.66.0

Edit: It actually applies to everything, not just the CLI approach

[blast-hardcheese]

Closing this issue now as I consider the issue to be resolved, though if anything else comes up either please feel free to comment here or open a new issue. Thanks for fixing this, @Peppi-Ressler!

[Peppi-Ressler]

Thank you for your quick response & release