Should we drop Remote.withContext?

[Tarmil]

As @OnurGumus remarked here, we technically don't need Remote.withContext. It is possible to access the current request's in an async-safe way by injecting IHttpContextAccessor.

Code like this:

let myHandler : MyApi =
    {
        login = Remote.withContext <| fun http username -> async {
            return! http.AsyncSignIn(username)
        }
    }

can be written like this:

type MyHandler(http: IHttpContextAccessor) =
    inherit RemoteHandler<MyApi>()

    override this.Handler =
        {
            login = fun username -> async {
                return! http.HttpContext.AsyncSignIn(username)
            }
        }

So, should we remove Remote.withContext, and also maybe have Remote.authorize not pass HttpContext anymore?

Arguments for keeping Remote.withContext:

• It's short to write and allows you to forgo DI altogether if you don't have anything else you need to inject.
• Its API is consistent with Remote.authorize.

Arguments for removing Remote.withContext:

• It's redundant.
• It needs a surprising amount of plumbing in Bolero itself and removing it would simplify a bunch of code.

[granicz]

I would just remove Remote.withContext and use DI instead - I suspect it will look much more familiar to most Blazor devs anyhow. Where does that put Remote.authorize?

[Tarmil]

Remote.authorize still needs to be implemented as a wrapper function; but it would probably make sense not to pass the HttpContext as an extra argument to the user function anymore.

[kos59125]

I have some leverages with Giraffe HTTP handlers. Remote.withContext is useful for Bolero to combine with Giraffe.

[Tarmil]

Thinking more about it, I realized that it would actually be cleaner and a little bit more optimal to remove the Remote module altogether, and instead introduce an injected helper, let's call it IRemoteContext, which would provide a property HttpContext and methods Authorize and AuthorizeWith. So a handler like this:

let myRemoteHandler =
    {
        login = Remote.withContext <| fun http username -> async {
            return! http.AsyncSignIn(username)
        }
        getUsername = Remote.authorize <| fun http () -> async {
            return http.User.Identity.Name
        }
        getAdminStuff = Remote.authorizeWith [AuthorizeAttribute(Role = "admin")] <| fun http () -> async {
            return "super secret stuff"
        }
    }

would become something like this:

type MyRemoteHandler(ctx: IRemoteContext) =
    inherit RemoteHandler<MyRemote>()

    override this.Handler =
        {
            login = fun username -> async {
                return! ctx.HttpContext.AsyncSignIn(username)
            }
            getUsername = ctx.Authorize <| fun () -> async {
                return ctx.HttpContext.User.Identity.Name
            }
            getAdminStuff = ctx.AuthorizeWith [AuthorizeAttribute(Role = "admin")] <| fun () -> async {
                return "super secret stuff"
            }
        }

@kos59125 I think this use case can be fulfilled with an additional overload on IServiceCollection.AddRemoting that takes a IRemoteContext -> 'Handler rather than just a 'Handler. Something like this:

let myRemoteHandler (ctx: IRemoteContext) =
    {
        // same as above
    }

// ...
    services.AddRemoting(myRemoteHandler)

Would that work well with Giraffe?

[kos59125]

@Tarmil

Would that work well with Giraffe?

Maybe no, for my case. But I think I may adapt Giraffe handlers to DI style with some changes.

Here's my current code, FYR:

// Giraffe handler to Bolero world
let withGiraffeHandler (f:HttpContext -> 'req -> Async<'resp>) (handler:HttpHandler) =
   fun ctx req -> async {
      let h = handler earlyReturn
      match! h ctx |> Async.AwaitTask with
      | Some(ctx) when ctx.Response.HasStarted -> return invalidOp "Giraffe handler must not start response"
      | Some(ctx) -> return! f ctx req
      | None -> return Unchecked.defaultof<'resp>  // the handler rejects user request, response body will be "null"
   }
   |> Remote.withContext

// Giraffe handler
let forbidden : HttpHandler =
   setStatusCode StatusCodes.Status403Forbidden
   >=> handleContext (fun _ -> skipPipeline)
let checkroot : HttpHandler = fun next ctx -> task {
   match ctx.TryUsername() with
   | Some("root") -> return! next ctx
   | _ -> return! forbidden next ctx
}

// Bolero handler
member __.Handler = {
   DoSomething =
      checkroot
      |> Remote.withGiraffeHandler (fun ctx req -> async {
         // do something for user "root"
      })
}

[Tarmil]

I see a member __.Handler, so this is a RemoteHandler<_> class? If so, you should be able to take DI by adding arguments to its constructor.

[kos59125]

Yes, __.Handler is a member of RemoteHandler.

For Giraffe's handlers, HttpContext appears in function argument. I just feel weird HttpContext is also given from out of the function. With Remote.withContext, both Giraffe and Bolero works in function world.

[Tarmil]

With my proposal you would pass the IRemoteContext to withGiraffeHandler, which would in turn pass its .HttpContext to the function, so it's the same all along. I think that's fine.

[kos59125]

I understand. But, unfortunately, it fails with NullReferenceException.

// ConfigureServices
services
   .AddHttpContextAccessor()
   .AddRemoting<MyRemoteHandler>()

// fix withGiraffeHandler from my code above
let withGiraffeHandler (ctx:HttpContext) f handler =
   fun req -> async { (* use ctx here *) }

// RemoteHandler
type MyRemoteHandler(http:IHttpContextAccessor) =
   interface RemoteHandler<> with
      member __.Handler = {
         DoSomething =
            checkroot  // giraffe handler
            |> withGiraffeHandler http.HttpContext (fun req -> async { ... })
      }

maybe because of this?

[Tarmil]

Yes, you need to pass http to withGiraffeHandler and then access http.HttpContext from within the fun req -> ... so that it is set correctly. The same will apply with IRemoteContext.

[kos59125]

Thanks, I get it. It works perfectly and I understand I don't need change my Giraffe handlers.