EntityStreamingSupport does not work with custom ToEntityMarshaller or just custom Content-Type #424

Closed
johanandren opened this Issue Oct 24, 2016 · 7 comments

Projects

Done in Bug Hunting

3 participants

@johanandren
Contributor

Imported from akka/akka#21714, reported by @sebastianvoss

I have defined a custom ToEntityMarshaller for type User. When requesting /users it returns an empty JSON array. Only when I remove the implicit def userMarshaller it return the correct representation of the stream.

import akka.NotUsed
import akka.actor.ActorSystem
import akka.http.scaladsl.Http
import akka.http.scaladsl.common.{EntityStreamingSupport, JsonEntityStreamingSupport}
import akka.http.scaladsl.model.{HttpEntity, StatusCodes, _}
import akka.http.scaladsl.server.Directives._
import akka.stream.ActorMaterializer
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
import akka.http.scaladsl.marshalling.{Marshaller, ToEntityMarshaller, ToResponseMarshaller}
import akka.http.scaladsl.model.TransferEncodings.gzip
import akka.http.scaladsl.model.headers.{HttpEncoding, HttpEncodings}
import akka.http.scaladsl.model.ws.{Message, TextMessage}
import akka.stream.scaladsl.{Flow, Source}
import akka.util.ByteString
import spray.json.DefaultJsonProtocol
import spray.json.DefaultJsonProtocol._

import scala.concurrent.Future
import scala.io.StdIn
import scala.util.Random

final case class User(name: String, id: String)

trait UserProtocol extends DefaultJsonProtocol {

  import spray.json._

  implicit val userFormat = jsonFormat2(User)

  val `vnd.example.api.v1+json` =
    MediaType.applicationWithFixedCharset("vnd.example.api.v1+json", HttpCharsets.`UTF-8`)

  implicit def userMarshaller: ToEntityMarshaller[User] = Marshaller.oneOf(
    Marshaller.withFixedContentType(`vnd.example.api.v1+json`) { organisation =>
      HttpEntity(`vnd.example.api.v1+json`, organisation.toJson.compactPrint)
    })
}

object ApiServer extends App with UserProtocol {
  implicit val system = ActorSystem("api")
  implicit val materializer = ActorMaterializer()
  implicit val executionContext = system.dispatcher

  implicit val jsonStreamingSupport: JsonEntityStreamingSupport = EntityStreamingSupport.json()
    .withParallelMarshalling(parallelism = 10, unordered = false)

  // (fake) async database query api
  def dummyUser(id: String) = User(s"User $id", id.toString)

  def fetchUsers(): Source[User, NotUsed] = Source.fromIterator(() => Iterator.fill(10000) {
    val id = Random.nextInt()
    dummyUser(id.toString)
  })

  val route =
    pathPrefix("users") {
        get {
          complete(fetchUsers())
        }
    }

  val bindingFuture = Http().bindAndHandle(route, "localhost", 8080)

  println(s"Server online at http://localhost:8080/\nPress RETURN to stop...")
  StdIn.readLine()
  bindingFuture.flatMap(_.unbind()).onComplete(_ => system.terminate())
}
@jrudolph
Member
jrudolph commented Nov 1, 2016

There are several things at play here:

First, you import lots of implicit values into your scope that may (silently or not) conflict with each other. In this case:

// with `userMarshaller` in scope:
marshalling.this.Marshaller.fromEntityStreamingSupportAndEntityMarshaller[akka.http.scaladsl.server.User, akka.NotUsed](ApiServer.this.jsonStreamingSupport, ApiServer.this.userMarshaller)

// without `userMarshaller` in scope:
marshalling.this.Marshaller.fromEntityStreamingSupportAndEntityMarshaller[akka.http.scaladsl.server.User, akka.NotUsed](ApiServer.this.jsonStreamingSupport, akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonMarshaller[akka.http.scaladsl.server.User](ApiServer.this.userFormat, akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonMarshaller$default$2[akka.http.scaladsl.server.User])

So, without your custom marshaller in scope, it will use the defaults which will create an entity with a content-type of application/json.

I guess you created the custom marshaller to support a custom content-type. This doesn't work. Instead, the recommended way to do this is to call JsonEntityStreamingSupport.withContentType:

implicit val jsonStreamingSupport: JsonEntityStreamingSupport = EntityStreamingSupport.json()
    .withParallelMarshalling(parallelism = 10, unordered = false)
    .withContentType(`vnd.example.api.v1+json`.toContentType)

Unfortunately, this will again lead to the same problem that no data is generated. I don't understand what's going on exactly but it may be a bug in streaming json support.

@ktoso do you have an idea, what the problem could be?

@jrudolph jrudolph added this to the backlog milestone Nov 1, 2016
@jrudolph jrudolph changed the title from Issue with rendering when using custom ToEntityMarshaller to EntityStreamingSupport does not work with custom ToEntityMarshaller or just custom Content-Type Nov 1, 2016
@ktoso
Member
ktoso commented Nov 21, 2016

I'm looking into this.

@ktoso ktoso added 3 - in progress and removed 1 - triaged labels Nov 21, 2016
@ktoso ktoso modified the milestone: 10.0.1, backlog Nov 21, 2016
@ktoso
Member
ktoso commented Nov 21, 2016

my train o of thought from the chat here:

whoop, got the bug i think...
such a pain :worried:
lack of content type represented in the marshaller's type is the culprit basically, let's see what i can make out of this hm

Konrad Malawski @ktoso 17:10
I see what's going on with the entity streaming support for String elements... I'm on the edge if it needs an api change...
:\
hacking on

Konrad Malawski @ktoso 17:16
well for json i can special handle things, which kind of actually makes sense... but would be nice to have it in the infra
thinking :~
long story short:
provided marshaller goes to text/plain
is WithOpenCharset
we expect only WithFixedContentType in the entity streaming marshaller creator
also, it's not the same content type - we expect JSON, right (logical), json elements in a json stream
however string is special
we can render a json array [ "one", "two" ]
when only given a text/plain marshaller...
I can do 2.5 things, please advice:
1. blow up if we detect no marshaller
i.e. "you only have text/plain marshaller, and you try to render Source[String], plz provide json renderer for string!"
2. perhaps add application/json marshallers for String / Int
those would resolve hm
though risking ambigious implicits
2.5 I can special handle JSON and KNOW that we can render text elements as single "bla" elements
_

Any opinions?

@ktoso
Member
ktoso commented Nov 21, 2016

Option 3. would be to rewrite the infra to allow selecting / prefering element types... but that'll break compat, and be rather complex :(

@ktoso
Member
ktoso commented Nov 21, 2016 edited

Workaround if anyone wants to "just stream Source[String, _]":
Provide this implicit:

implicit val stringFormat = Marshaller[String, ByteString] { ec  s 
    Future.successful {
      List(Marshalling.WithFixedContentType(ContentTypes.`application/json`, () 
        ByteString("\"" + s + "\"")) // "raw string" to be rendered as json element in our stream must be enclosed by ""
      )
    }
  }
@ktoso
Member
ktoso commented Nov 21, 2016 edited

In the above example the content type should be set on jsonStreamingSupport as well, so in both places.

@ktoso
Member
ktoso commented Nov 21, 2016

The above example then works, here's the full listing with the fix:

/*
 * Copyright (C) 2009-2016 Lightbend Inc. <http://www.lightbend.com>
 */

package akka.http.scaladsl.server

import akka.NotUsed
import akka.actor.ActorSystem
import akka.http.scaladsl.Http
import akka.http.scaladsl.common.{ EntityStreamingSupport, JsonEntityStreamingSupport }
import akka.http.scaladsl.model.{ HttpEntity, StatusCodes, _ }
import akka.http.scaladsl.server.Directives._
import akka.stream.ActorMaterializer
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
import akka.http.scaladsl.marshalling.{ Marshaller, ToEntityMarshaller, ToResponseMarshaller }
import akka.http.scaladsl.model.TransferEncodings.gzip
import akka.http.scaladsl.model.headers.{ HttpEncoding, HttpEncodings }
import akka.http.scaladsl.model.ws.{ Message, TextMessage }
import akka.stream.scaladsl.{ Flow, Source }
import akka.util.ByteString
import spray.json.DefaultJsonProtocol
import spray.json.DefaultJsonProtocol._

import scala.concurrent.Future
import scala.io.StdIn
import scala.util.Random

final case class User(name: String, id: String)

trait UserProtocol extends DefaultJsonProtocol {

  import spray.json._

  implicit val userFormat = jsonFormat2(User)

  val `vnd.example.api.v1+json` =
    MediaType.applicationWithFixedCharset("vnd.example.api.v1+json", HttpCharsets.`UTF-8`)
  val ct = ContentType.apply(`vnd.example.api.v1+json`) // THIS IS THE FIX~~~~~~~~~~~~

  implicit def userMarshaller: ToEntityMarshaller[User] = Marshaller.oneOf(
    Marshaller.withFixedContentType(`vnd.example.api.v1+json`) { organisation 
      HttpEntity(`vnd.example.api.v1+json`, organisation.toJson.compactPrint)
    })
}

object ApiServer extends App with UserProtocol {
  implicit val system = ActorSystem("api")
  implicit val materializer = ActorMaterializer()
  implicit val executionContext = system.dispatcher

  implicit val jsonStreamingSupport: JsonEntityStreamingSupport = EntityStreamingSupport.json()
    .withContentType(ct) // THIS IS THE FIX~~~~~~~~~~~~
    .withParallelMarshalling(parallelism = 10, unordered = false)

  // (fake) async database query api
  def dummyUser(id: String) = User(s"User $id", id.toString)

  def fetchUsers(): Source[User, NotUsed] = Source.fromIterator(()  Iterator.fill(10000) {
    val id = Random.nextInt()
    dummyUser(id.toString)
  })

  val route =
    pathPrefix("users") {
      get {
        complete(fetchUsers())
      }
    }

  val bindingFuture = Http().bindAndHandle(route, "localhost", 8080)

  println(s"Server online at http://localhost:8080/\nPress RETURN to stop...")
  StdIn.readLine()
  bindingFuture.flatMap(_.unbind()).onComplete(_  system.terminate())
}

If the above would be run without the fix, we currently blow up with a more helpful info than just rendering empty output (after my PR is merged that is):

[ERROR] [11/21/2016 23:11:48.099] [api-akka.actor.default-dispatcher-5] [akka.actor.ActorSystemImpl(api)] Outgoing response stream error
akka.http.scaladsl.marshalling.NoStrictlyCompatibleElementMarshallingAvailableException: None of the available marshallings (List(WithFixedContentType(application/vnd.example.api.v1+json,<function0>))) directly match the ContentType requested by the top-level streamed entity (application/json). Please provide an implicit `Marshaller[akka.http.scaladsl.server.User, HttpEntity]` that can render akka.http.scaladsl.server.User as [application/json]
@ktoso ktoso modified the milestone: 10.0.0 "first stable", 10.0.1 Nov 21, 2016
@ktoso ktoso self-assigned this Nov 21, 2016
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Nov 21, 2016
@ktoso ktoso =core #424 add more explicit error logging for entity streaming
- dont just render empty responses if unable to put streaming support
  together with the provided marshaller

Could be better than that, yes, but this is our status quo so at least
being vocal about it.
316fd6d
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Nov 21, 2016
@ktoso ktoso =core #424 add full example with custom content type e124a22
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Nov 22, 2016
@ktoso ktoso =core #424 add more explicit error logging for entity streaming
- dont just render empty responses if unable to put streaming support
  together with the provided marshaller

Could be better than that, yes, but this is our status quo so at least
being vocal about it.
9ca48cf
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Nov 22, 2016
@ktoso ktoso =core #424 add full example with custom content type 877b1be
@ktoso ktoso modified the milestone: 10.0.0 - Akka HTTP "X", 10.0.1 Nov 22, 2016
@jrudolph jrudolph modified the milestone: 10.0.1, 10.0.2 Dec 22, 2016
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 6, 2017
@ktoso ktoso =core #424 add more explicit error logging for entity streaming
- dont just render empty responses if unable to put streaming support
  together with the provided marshaller

Could be better than that, yes, but this is our status quo so at least
being vocal about it.
7e6513b
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 6, 2017
@ktoso ktoso =core #424 add full example with custom content type 8e1e798
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 6, 2017
@ktoso ktoso =core #424 add full example with custom content type 6067412
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 9, 2017
@ktoso ktoso =core #424 add more explicit error logging for entity streaming
- dont just render empty responses if unable to put streaming support
  together with the provided marshaller

Could be better than that, yes, but this is our status quo so at least
being vocal about it.
f52dd95
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 9, 2017
@ktoso ktoso =core #424 add full example with custom content type 4424b63
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 10, 2017
@ktoso ktoso =core #424 add full example with custom content type 18ad90b
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 10, 2017
@ktoso ktoso =core #424 add more explicit error logging for entity streaming
- dont just render empty responses if unable to put streaming support
  together with the provided marshaller

Could be better than that, yes, but this is our status quo so at least
being vocal about it.
51b74a8
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 10, 2017
@ktoso ktoso =core #424 add full example with custom content type f09dde5
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 10, 2017
@ktoso ktoso =core #424 add full example with custom content type 9b623fe
@ktoso ktoso added a commit to ktoso/akka-http that referenced this issue Jan 10, 2017
@ktoso ktoso =core #424 add full example with custom content type af60911
@ktoso ktoso closed this in #565 Jan 19, 2017
@ktoso ktoso added a commit that referenced this issue Jan 19, 2017
@ktoso ktoso =core #424 fix stream marshalling, better errors, more examples (#565)
* =core #424 add more explicit error logging for entity streaming
- dont just render empty responses if unable to put streaming support
  together with the provided marshaller

Could be better than that, yes, but this is our status quo so at least
being vocal about it.

* =core #424 add full example with custom content type
238e7fb
@ktoso ktoso removed the 3 - in progress label Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment