api-defaults will set content-types automatically which is incompatible with wrap-json-response

[jiacai2050]

I think most of developers will meet this issue, and solution to this issue is simple after I dig into src of both wrap-json-response and api-defaults

Is it more reasonable to set content-types default to false in api-defaults or add an option to set content-type forcefully in wrap-json-response ?

[weavejester]

Sorry, I don't understand what you're trying to say. Did you mean "incompatible", and if so, why is it incompatible? wrap-content-types won't set the content type if its already been set by other middleware.

[jiacai2050]

Oh, sorry. It's "incompatible". I already updated the title.

I will show some codes to reproduce the incompatible issue.

(def app
  (-> handler
      (wrap-defaults api-defaults)
      wrap-json-response))

If I define app like above, wrap-json-response will not work, because Content-Type is already set in wrap-defaults.

I can work around this simply by switching the order of the two middlewares, but demos I search from google always put wrap-defaults, handler/api or handler/site at first then other middlewares following. Orders of middleware do matter a lot than I think at first, so in order to be newbie-friendly, ring-defaults can do something like I said in first comment. What do you think ?

[weavejester]

wrap-defaults should typically be the outermost middleware applied. I can add a note to the README to emphasize that.

[jiacai2050]

Documents are less useful than defaults.

From my point of view, api-defaults should make sure response is in json format. is it more reasonable to add wrap-json-response in api-defaults ?

[weavejester]

No, because an API is not necessarily going to return JSON, and because wrap-defaults needs to be the outermost middleware for other reasons. For example, wrap-params is part of wrap-defaults, and consumes the body InputStream.

[jiacai2050]

Got your point.

For your convenience, I have drawn a flow chart to demo how middlewares work. you can add it in README.

you can edit this diagram here.

[weavejester]

Thanks for the diagram, but the Ring-Defaults library isn't the place to be explaining middleware. The Ring wiki might be a better choice. However, your diagrams are also both a little inaccurate, I'm afraid.

[jiacai2050]

Is this accurate ?

[weavejester]

No, sorry. The diagram should look more like:

 +---------------+
 |  Middleware   |
 |  +---------+  |             +---------+      +--------+
 |  |         |<-- request ----|         |      |        |
 |  | Handler |  |             | Adapter |<---->| Client |
 |  |         |--- response -->|         |      |        |
 |  +---------+  |             +---------+      +--------+
 +---------------+

The middleware should wrap the handler, and there's only one adapter per connection to the client. In more complicated setups you can combine multiple handlers, e.g. Compojure works by combining "routes" (handlers that can return nil), and middleware can be applied to individual routes, as well as the top-level handler.

[jiacai2050]

Oh, thanks. My understanding is almost the same as what your diagram express. And I do know handlers can be combined just like blueprints in Flask, which is a great feature for modularity.

The point I want to emphasize is

Outmost middleware (middleware 3 in my diagram) will be the first to recevie the request, and the last to receive the response

[mpas]

Glad i struck this issue as my application did not work.. This due all the demos i see on the internet that totally ignore the content type. Is this now somewhere in the documentation?