ring-swagger

ring-swagger & compojure-api
jdt 2018-08-22T11:28:39.000200Z

As discussed some days ago (https://clojurians.slack.com/archives/C06GSN6R2/p1534530179000100) ('m using Muuntaja 0.6.0-alpha3). However it's caused some unit tests break. I'm here to ask if it's a bug or a feature, since I know nothing of msgpack standards. Basically the unit test that is breaking does a msgpack.core/unpack from [clojure-msgpack 1.2.1] on the response from the compojure-api app, and the returned data is no longer compatible. It results in a java.lang.ClassCastException: java.lang.Byte cannot be cast to [B. So what the new msgpack code packing isn't what the old client msgpack code wants to see. I don't know if there's a standard that's supposed to ensure the format of the data doesn't change, but I'm hesitant to go changing client code to use a new msgpack tool when the old one has worked all this time.

jdt 2018-08-22T11:29:17.000100Z

time to read up on msgpack I guess, but could use some advice on whether this is a new alpha muuntaja bug, or whether I really should be upgrading the msgpack client.

jdt 2018-08-22T11:31:01.000100Z

My impression is that my client msgpack (unpack) code shouldn't be breaking, but perhaps the argument can be made that msgpack.core is old and broken and so it's time to upgrade.

jdt 2018-08-22T11:37:34.000100Z

Hmmm, 1.2.1 is the latest and greates of msgpack.core. Of course this may not be a msgpack issue at all, it could be the muuntaja and compojure-api alpha-23 upgrade have ... altered ... the encoding of what is returned by the route.

jdt 2018-08-22T11:39:44.000100Z

The route in question declares a prismatic schema return type of

(s/defschema DocumentLayout
  (sweet/describe (s/pred bytes?) "Protocol buffer binary encoded data."))

jdt 2018-08-22T11:43:09.000100Z

The only change in the broken APIs is the upgrade to compojure-api alpha23 and muuntaja 0.6.0-alpha3, and elimination of this line from the api format options:

-   :formats (m/create (-> m/default-options (msgpack-format/with-msgpack-format)))
Since that msgpack-format support is no longer present. I didn't see the need to declare any formats because the current defaults are supposed to include msgpack. Or do I need something afterall?

jdt 2018-08-22T11:44:22.000100Z

Client headers are (mock/header "accept" "application/msgpack")

jdt 2018-08-22T12:01:36.000100Z

Will re-examine the defaults, could have sworn current muuntaja said msgpack was a default supported format.

jdt 2018-08-22T12:16:39.000100Z

Did the obsoleted with-msgpack-format support "application/msgpack" where the new msgpack defaults do not?

ikitommi 2018-08-22T12:48:21.000100Z

@dave.tenny the muuntaja CHANGELOG describes the changes with msgpack: https://github.com/metosin/muuntaja/blob/master/CHANGELOG.md#060-alpha1

jdt 2018-08-22T12:59:15.000100Z

The problem is not anything about the request body, the problem is the client breaks decoding the HTTP response from the compojure-api app

jdt 2018-08-22T12:59:56.000100Z

So it's asking for "application/msgpack" as the "accept" header, which worked before, but now results in unpacking exceptions, perhaps because the server response isn't being encoded the way it was before.

jdt 2018-08-22T13:00:39.000100Z

We had no special code to support that header, with-msgpack-format and the old muuntaja seems like it must have been supporting it before but not now.

jdt 2018-08-22T13:00:43.000100Z

Or such is my guess.

jdt 2018-08-22T13:44:22.000100Z

I'm not sure which other parts of the breaking changes in changelog might apply here, unless it's a need to set the Content-Type where we weren't before, re:

**BREAKING**: if a "Content-Type" header is set in a response, the body will not be encoded, unless `:muuntaja/encode` is set

jdt 2018-08-22T14:43:47.000100Z

Interesting, if I change the client "accept" header from "application/msgpack" to "application/transit+msgpack", the content is sent back differently (but not correctly in either case). As far as I can tell we do NOT set any Content Type in the response.