Registery access Execution.current() vs Handler Context and sharing state with Service

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Registery access Execution.current() vs Handler Context and sharing state with Service

jeff-blaisdell
Problem:
In most RatPack examples, and the book a very typical pattern for resolving authentication is to have a preceding handler that inspects the headers, checks for valid authorization, adds some sort of User/Authentication details to the Handler's Context/Registry, and finally calls ctx.next() to continue the chain.  This then provides access to the request's auth details from all subsequent handlers.  However, because a Service is not a Handler, it does not seem to have access to this information due to differing registry scoping.  So the question is what is the best way to share this information?

Options I've been considering:
1) Instead of adding to the Handler ctx, add it to Execution.current().  There has been concern this might not always be bound to the correct request and we could potentially pull out the wrong user's details.

2) Create something akin to Spring Security's SecurityContextHolder utilizing interceptors like RatPack's MDCInterceptor

3) Don't rely on context / registry to retrieve auth/user details within a Service.  Instead require every service method to take in a User argument in order to access the necessary information.


#3 seems the most common in examples I've found, however in a non-trivial application it seems to really break the DRY principal.
Reply | Threaded
Open this post in threaded view
|

Re: Registery access Execution.current() vs Handler Context and sharing state with Service

jeff-blaisdell

john [2:36 PM]  
Where did you hear about Execution outliving a request?

[2:36]  
If you add it to the current execution using a Handler it won't leak to other requests. Executions aren't reused.

kyleboon [2:37 PM]  
danny told me that

john [2:38 PM]  
@danhyun: why you lyin'!?

danhyun [2:38 PM]  
background tasks

[2:38]  
no?

[2:39]  
you could also get out of the way up front

[2:39]  
and have execution continue after the fact

[2:39]  
 ```get {
  response.send('ok')
  execution.fork {
     println 'life goes on'
  }
}
```

john [2:40 PM]  
But who cares?

rahulsom [2:40 PM]  
desmond and molly

john [2:40 PM]  
He's just looking for place to stash some data.

[2:41]  
If you put data into the execution it won't leak to a different request or execution segment. (edited)

danhyun [2:42 PM]  
Sorry catching up here -- what's the argument against `Context#next(Registry)`

[2:43]  
If you're using Groovy you can use InjectionHandlers

john [2:43 PM]  
They don't want to litter service method sigs with the extra object that contains the oauth token

danhyun [2:43 PM]  
They allow you to write handlers like this

```get { MyAuthToken auth ->
  // here we go
}
```

[2:44]  
or even in Java `MyHandler extends InjectionHandler { void handle(Context ctx, MyAuthToken token) { } }`

john [2:44 PM]  
That's not the issue

jeff.blaisdell [2:44 PM]  
normally wouldn’t care, but when you’re going to have a lot of service methods, seems icky to pass MyAuthToken to everyone

john [2:44 PM]  
Inside the Handler they are calling a service method that needs the data

kyleboon [2:44 PM]  
just add the data to the method signature, it’s a better solution than calling Execution.current() deep into the call chain

danhyun [2:44 PM]  
In that case why not borrow some concepts from functional programming

[2:45]  
Have a function that embeds the auth token in the functions you wish to use

john [2:45 PM]  
@kyleboon: perhaps

[2:45]  
But doing that shouldn't be unreliable or "bad" per say

danhyun [2:45 PM]  
You could then register this set of auth aware functions in the registry

[2:46]  
There's a similar pattern for creating "pure" services that take in the resources per method

[2:46]  
but you don't want to keep passing the resource into the method call (edited)

[2:46]  
https://gist.github.com/danhyun/fda27d5682b7dbed151b

john [2:47 PM]  
I'm simply addressing the statement that Execution.current().add(object) is unsafe.

[2:47]  
I don't see how that's true.

[2:47]  
If you don't like the pattern, so be it

[2:47]  
But that code is safe AFAIK

danhyun [2:48 PM]  
You don't know when code is going to by called by who

[2:48]  
in general

[2:48]  
making the args an explicit part of the contract help tease that apart

[2:49]  
The other bit is that `Registry#get()` blows up if it's not present

[2:49]  
so if you don't like that you start using `maybeGet` then try unwrapping yourself

[2:50]  
John's right though there's nothing inherently incorrect or bad about `Execution.current().add(object)`

[2:50]  
it's about deferencing it

danveloper [2:51 PM]  
if it were me, I would use the `Execution` as well

jordan.howe [2:53 PM]  
So the danger is that the request blows up if I do Execution.current().get(nonExistentObject)? I just need to test if it exists before I make that call, then?

john [2:53 PM]  
Just call .maybeGet which will return an Optional

danveloper [2:54 PM]  
^

john [2:54 PM]  
This "danger" exists for any registry

[2:54]  
Context, execution, system

jordan.howe [2:55 PM]  
Yeah, I was responding to danny's point about Registry#get() blowing up if object not present. (edited)

danhyun [2:55 PM]  
Yes so you would use `maybGet` and safely handle

[2:55]  
and that's fine

jordan.howe [2:55 PM]  
thanks all, sounds like a plan

danveloper [2:56 PM]  
there’s another option with Guice

[2:56]  
you can have a RequestScoped component

[2:56]  
that could hold the keys

[2:56]  
or w/e it is i can’t remember

[2:56]  
and inject it into your non-singleton service component

[2:57]  
that’s assuming your service component isn’t doing any load-time bootstrapping that’s too heavy

[2:57]  
and that you can modify its local fields

[2:57]  
oh snap

[2:57]  
`RequestScope` is internal :blush:

[2:58]  
nvm

[2:58]  
ignore me

new messages
kyleboon [2:59 PM]  
i make a point to never do what @danveloper  does

rus [3:01 PM]  
fwiw I think I would use the execution too. It’s analogous to threadlocal which is a common pattern for storing security context.

kyleboon [3:02 PM]  
but one that isn’t fun to test

danhyun [3:02 PM]  
you can "preload" the context for your tests

danveloper [3:02 PM]  
^

danhyun [3:02 PM]  
Ratpack is good about giving you the ability to set the state before hand

[3:02]  
I'll talk about this next week at Gr8confUS
Reply | Threaded
Open this post in threaded view
|

Re: Registery access Execution.current() vs Handler Context and sharing state with Service

Luke Daley
Administrator
In reply to this post by jeff-blaisdell
There's no issue with using #1. Handling of a request is bound to one and only one execution. It's effectively the Ratpack equivalent of SecurityContextHolder. It's analogous to using thread local storage, so just inherits all of the traditional drawbacks of that (but that doesn't mean that it should never be used).

If you're using Guice, there's another alternative in using scoped objects.

https://github.com/google/guice/wiki/Scopes

Ratpack provides two (as yet undocumented) scopes:

https://github.com/ratpack/ratpack/blob/master/ratpack-guice/src/main/java/ratpack/guice/RequestScoped.java
https://github.com/ratpack/ratpack/blob/master/ratpack-guice/src/main/java/ratpack/guice/ExecutionScoped.java

What you could do is have a “holder” type object bound that is `@RequestScoped` and inject this into the services where it is need as per usual constructor injection.

This is effectively doing the same thing as storing data in the `Execution` (that's actually what it's doing under the hood).