Get rid of CookiedWrapperClass and Cookied altogether?

[michalrus]

Hey,

so I’ve been thinking, given CookiedWrapperClass’s making type inference for endpoints more/less inconvenient, could we maybe define a CookiedBis (cheesy name), i.e. a full combinator, used like:

type MyAPI = AuthProtect "cookie" :> CookiedBis :> Capture Int :> Get '[ JSON] Int

myHandler :: Server MyAPI
myHandler userSession someInt = return (someInt * 2)

And then in instance HasServer (CookiedBis :> subapi) we could use tweakResponse from https://hackage.haskell.org/package/servant-server-0.12/docs/Servant-Server-Internal-Router.html#v:tweakResponse to add the Set-Cookie: headers, without changing endpoint types.

I can experiment with this in our codebase and report back the results, but I’d love to hear your thoughs first, @zohl!

[michalrus]

E.g. these errors, when we don’t specify a concrete monad:

src/[…].hs:23:5: error:
    • Ambiguous type variable ‘f0’ arising from a use of ‘pure’
      prevents the constraint ‘(Applicative f0)’ from being solved.
      Probable fix: use a type annotation to specify what ‘f0’ should be.
      These potential instances exist:
        instance Applicative (Either e) -- Defined in ‘Data.Either’
        instance Applicative STM -- Defined in ‘GHC.Conc.Sync’
        instance Applicative IO -- Defined in ‘GHC.Base’
        ...plus six others
        ...plus 43 instances involving out-of-scope types
        (use -fprint-potential-instances to see them all)
    • In a stmt of a 'do' block: pure NoContent
      In the expression: do pure NoContent
      In the second argument of ‘($)’, namely
[…]

Normally, pure NoContent should be unambiguous in such a position.

[zohl]

Hi!

Sorry for the delay. Your idea sounds great! Gotta implement :)
I think we can change Cookied from type synonym to servant combinator, so the migration will be straightforward.
Also, this solution should benefit to the issue #43, since the developers don't have to keep in mind different pairs of type synonyms and handle-wrappers (in case of Cookied and OptionallyCookied and corresponding wrappers).

[michalrus]

Yes, so that’s wonderful. Thank you. =)

But Cookied right now is used like … :> Cookied NoContent, and if it was to be a regular combinator, it’d have to be … :> Cookied :> NoContent. But then, the users still have to get rid of cookied calls, so probably this change can’t be backwards compatible anyway.

[michalrus]

Hmmm, one more thought. Maybe the user shouldn’t specify AuthProtect "cookie" manually if they add … :> Cookied :> … to an endpoint. Because that AuthProtect combinator will add wrapped session to the endpoint handler type.

Maybe Cookied should add AuthHandler behind the scenes, and consume it at the same time. This could be possible in its HasServer instance.

Or maybe not use AuthHandler at all? 🤔

[zohl]

Hmm, things are getting complicated again... let's see. At the moment there are three types of modifiers:

• AuthHandler, that checks cookies, decodes them and provides an extra argument to handlers
• Cookied, that transparently renews the cookies
• AddHeader (actually, the same as Cookied), that allows us to create/update cookies via addSession

And we want to simplify user-code by removing cookied wrappers (keeping in mind issues #41 and #43).

As far as I understand we cannot write a servant combinator that adds a type-level header like this:

    type MyAPI = "foo" :> CookiedBis :> "bar" :> ... :> Get '[JSON] Int
    
    myHandler :: Server MyAPI
    myHandler userSession = do
      -- ...

      -- Here compiler "knows" the header's type because of 'CookiedBis' combinator.
      return (addHeader headerValue response)

As you said, it's still possible to add header on lower level using 'tweakResponse'.
And we also can migrate AuthHandler's functionality here, since this part of servant's API is about to be deprecated.

So, I think I should:

• rename Cookied to SetCookie, so we can clearly document endpoints that create/update cookies.
• implement Cookied combinator that fails if there no valid cookies or supplies decoded value to the handler.
• implement MaybeCookied combinator that does the same, except supplies Nothing if there is no cookies.

The changes will be breaking, but they can be trivially fixed:

1. In endpoints, that modify the cookies, we should replace Cookied with SetCookie (otherwise typecheck fails).
2. In endpoints, that do not modify the cookies, we should remove Cookied.
3. AuthHandler "auth-cookie" should be replaced with Cookied.

Are such changes alright with you?

[michalrus]

The changes will be breaking, but they can be trivially fixed:

Yes, the way to fix them is clear!

Are such changes alright with you?

Very! ♥ I’d do anything to get proper inference back. =)

[michalrus]

One more thought, which just came to me!

You can almost get good inference back with a simple type-level function:

type CookieProtect a
   = AuthProtect "cookie-auth"
     :> a

type family Unprotect (api :: k) :: k where
  Unprotect (Verb a b c (Cookied d)) = Verb a b c d
  Unprotect (CookieProtect api) = Unprotect api
  Unprotect (a
             :<|> b) = (Unprotect a
                        :<|> Unprotect b)
  Unprotect (a
             :> b) = (a
                      :> Unprotect b)
  Unprotect a = a

And now:

type SearchAPI
   = "search"
     :> CookieProtect (QueryParam "qp1" IPv4Range
                       :> QueryParams "qp2" UserId
                       :> QueryParam "qp3" UTCTime
                       :> Get '[ JSON] (Cookied (SearchResults (WithId SomeId Something))))

-- instead of restating all of those types manually, let the compiler do it in `sub`,
-- and the parameters will have proper types bound to them!
serveSearch :: ServerT SearchAPI IO
serveSearch = cookied sub
  where
    sub :: UserSession -> ServerT (Unprotect SearchAPI) IO
    sub UserSession {..} qp1 qp2 qp3 =
      error "unimplemented"