I might have misunderstood the usage of :spec because I don't see any error when the :session route data is invalid. For example, the :store cannot be nil:

(def app
  (ring/ring-handler
   (ring/router
    ["/api"
     {:session    {:store nil}
      :middleware [mw/session-middleware]}
     ["/ping" handler]
     ["/pong" handler]])))

I might have misunderstood the usage of :spec because I don't see any error when the :session route data is invalid.

I forgot about the :validate key.

Route data validation is not on by default, here's the guide: https://cljdoc.org/d/metosin/reitit/0.3.9/doc/ring/route-data-validation

Hi @ikitommi @miikka, thanks for helping. I did another force push to fix another spec issue and include the test for the spec validation. Would you review it again when you have the time? Thanks.

One thing that comes to mind is that how should the middleware be configured. In reitit, there are options:

1. via middleware options
   pass the store etc. options for a create function, that is used to create the middleware. Interceptors use this is a lot and I think reitit 1.0.0 should do this too: it adds a clear phase to initialize one middleware once per application. Not the best option per today.

["/routes"
  {:middleware [(session/session-middleware {:store xyz})]}
 ["/with-session!" ...]]

2. via route-data, on by default
   if mw is mounted, it's on. Mounting a session-mw into top-level means there is no way to disable it for a certain route. Not optimal.

["/routes"
  {:middleware [session/session-middleware]}
 ["/with-session!" ...]]

3. via route-data, off by default
   mounting mw add just capabilities for a route. Only after adding a :session route data, enables it for certain routes. Most reitit-internal mw work like this, for example, content-negotiation & coercion.

["/routes"
  {:middleware [session/session-middleware]}
 ["/with-session!"  {:session xyz} ...]
 ["/without-session!" ...]]

would be good to have some coherence here. Just not sure what is the best way to do this. Comments?

I agree that option 2 is not optimal, so this PR will to be fixed.

My personal preference is option 3. I value the consistancy with other internal mw (option 3) more than the clarity from option 1.

The commit a3c64ba changes from option 2 to option 3. I think we need some more feedbacks to make a decision.

I think another reason to support option 3 (in general) is that the router code would be cleaner if anyone wants to have different configurations on routes. Say:

["/routes"
  {:middleware [session/session-middleware]}
 ["/with-session-mem!"  {:session {:store mem-store}} ...]
 ["/with-session-cookie!"  {:session {:store cookie-store}} ...]
 ["/without-session!" ...]]

Versus

["/routes"
 ["/with-session-mem!"  {:middleware [(session/session-middleware {:store mem-store})]} ...]
 ["/with-session-cookie!"  {:middleware [(session/session-middleware {:store cookie-store})]} ...]
 ["/without-session!" ...]]

I finally see this today: #264. Perhaps put this PR on hold till #264 makes a decision.