Plugins and multiple interceptors

[Peter Ledbrook]

Hi,

I have recently been looking into adding before interceptors to
controllers dynamically from a plugin. There were quite a few hurdles
to overcome and now I feel it may be better to shift attention to
support for multiple interceptors (
http://jira.codehaus.org/browse/GRAILS-92 ).

What I would like to see is the ability to add a new interceptor to a
controller from a plugin. Something like this:

  application.controllerClasses.each { controller ->
      // Check whether this controller has an 'accessControl' property.
      def prop = controller.getProperty('accessControl')

      // Add an interceptor if the property exists.
      if (prop != null) {
          def interceptor = {
              // This is the interceptor...
          }
          controller.beforeInterceptors << [ action: interceptor,
except: 'index' ]
      }
  }

The first question is, should the controller class manage the list of
interceptors, or the wrapper class (DefaultGrailsControllerClass)? One
idea I had is that maintaining a list of pre-configured interceptors
would reduce the processing burden when dealing with a request: at the
moment, the request handling code checks for the 'beforeInterceptor'
property on the controller. With the above approach, the handling code
would just have to run through the list of interceptors and check
whether each one is configured for the requested action.

Next, I think an interceptor's delegate should be set to the
controller before it is executed. This would give it access to the
controller's dynamic properties and methods. However, I'm not sure
whether the interceptor closure would have problems having its
delegate replaced like that.

The last part of the jigsaw would be how to support interceptors
declared directly in controllers. In my view, the wrapper class would
process the 'beforeInterceptor' property on its wrapped controller and
add it to the list. Is this feasible? I have looked at the code, but
I'm still not sure of the relationship between the controllers and the
controller wrapper class.

Thanks,

Peter

---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email

[graemer]

On 4/16/07, Peter Ledbrook <peter@...> wrote:

> Hi,
>
> I have recently been looking into adding before interceptors to
> controllers dynamically from a plugin. There were quite a few hurdles
> to overcome and now I feel it may be better to shift attention to
> support for multiple interceptors (
> http://jira.codehaus.org/browse/GRAILS-92 ).
>
> What I would like to see is the ability to add a new interceptor to a
> controller from a plugin. Something like this:
>
>   application.controllerClasses.each { controller ->
>       // Check whether this controller has an 'accessControl' property.
>       def prop = controller.getProperty('accessControl')
>
>       // Add an interceptor if the property exists.
>       if (prop != null) {
>           def interceptor = {
>               // This is the interceptor...
>           }
>           controller.beforeInterceptors << [ action: interceptor,
> except: 'index' ]
>       }
>   }
>
> The first question is, should the controller class manage the list of
> interceptors, or the wrapper class (DefaultGrailsControllerClass)? One
> idea I had is that maintaining a list of pre-configured interceptors
> would reduce the processing burden when dealing with a request: at the
> moment, the request handling code checks for the 'beforeInterceptor'
> property on the controller. With the above approach, the handling code
> would just have to run through the list of interceptors and check
> whether each one is configured for the requested action.

Sounds like an idea.

>
> Next, I think an interceptor's delegate should be set to the
> controller before it is executed. This would give it access to the
> controller's dynamic properties and methods. However, I'm not sure
> whether the interceptor closure would have problems having its
> delegate replaced like that.

We are trying to get away from using closure delegates within Grails
core as they impact performance. The reason is they use try/catch
blocks inside Groovy to fallback to the delegate, in a multi-threaded
environment this has a significant impact on performance as the JVM
pauses to capture the stack trace. You should use ExpandoMetaClass
instead.

>
> The last part of the jigsaw would be how to support interceptors
> declared directly in controllers. In my view, the wrapper class would
> process the 'beforeInterceptor' property on its wrapped controller and
> add it to the list. Is this feasible? I have looked at the code, but
> I'm still not sure of the relationship between the controllers and the
> controller wrapper class.

The wrapper class is responsible for evaluating the conventions within
the controller and configuring it appropriately so yes you have the
right idea

Cheers

>
> Thanks,
>
> Peter
>
> ---------------------------------------------------------------------
> To unsubscribe from this list please visit:
>
>     http://xircles.codehaus.org/manage_email
>
>

--
Graeme Rocher
Grails Project Lead
http://grails.org

---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email

[Marc Palmer Local]

On 16 Apr 2007, at 16:03, Peter Ledbrook wrote:

> Hi,
>
> I have recently been looking into adding before interceptors to
> controllers dynamically from a plugin. There were quite a few hurdles
> to overcome and now I feel it may be better to shift attention to
> support for multiple interceptors (
> http://jira.codehaus.org/browse/GRAILS-92 ).
>
> What I would like to see is the ability to add a new interceptor to a
> controller from a plugin. Something like this:
>
>  application.controllerClasses.each { controller ->
>      // Check whether this controller has an 'accessControl' property.
>      def prop = controller.getProperty('accessControl')
>
>      // Add an interceptor if the property exists.
>      if (prop != null) {
>          def interceptor = {
>              // This is the interceptor...
>          }
>          controller.beforeInterceptors << [ action: interceptor,
> except: 'index' ]
>      }
>  }

Can I suggest that we don't do this. I think this is a kludge.

There is an (as yet undocumented I think) plan to do for interceptors  
what UrlMappings has done for controllers.

i.e. XXXXInterceptors.groovy would allow you to define per-controller  
and per-action namespace interceptors:

userprofile.'*'.beforeInterceptor {
     if (!authenticated) redirect( controller:'user', view:'login')
}

userprofile.saveDetails.afterInterceptor {
     cacheService.refreshUserProfile( params.userId)
}

'*'.afterInterceptor {
     log.trace( 'request done')
}


I think this mechanism would be far superior to per-controller  
interceptor definition and chaining which by its very nature will get  
very messy very quickly.

It is already broken for inheritance. However getting inheritance  
into the above model could be tricky too. It's part proposal, part  
brain-fart.


Marc



~ ~ ~
Marc Palmer
Blog - http://www.anyware.co.uk/






---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email

[Dmitriy Kopylenko-3]

I like this! :-)

Cheers,
Dmitriy.

2007/4/16, Marc Palmer <marc@...>:

On 16 Apr 2007, at 16:03, Peter Ledbrook wrote:

> Hi,
>
> I have recently been looking into adding before interceptors to
> controllers dynamically from a plugin. There were quite a few hurdles
> to overcome and now I feel it may be better to shift attention to
> support for multiple interceptors (
> http://jira.codehaus.org/browse/GRAILS-92 ).
>
> What I would like to see is the ability to add a new interceptor to a
> controller from a plugin. Something like this:
>
>  application.controllerClasses.each { controller ->
>      // Check whether this controller has an 'accessControl' property.
>      def prop = controller.getProperty('accessControl')
>
>      // Add an interceptor if the property exists.
>      if (prop != null) {
>          def interceptor = {
>              // This is the interceptor...
>          }
>          controller.beforeInterceptors << [ action: interceptor,
> except: 'index' ]
>      }
>  }

Can I suggest that we don't do this. I think this is a kludge.

There is an (as yet undocumented I think) plan to do for interceptors
what UrlMappings has done for controllers.

i.e. XXXXInterceptors.groovy would allow you to define per-controller
and per-action namespace interceptors:

userprofile.'*'.beforeInterceptor {
     if (!authenticated) redirect( controller:'user', view:'login')
}

userprofile.saveDetails.afterInterceptor {
     cacheService.refreshUserProfile ( params.userId)
}

'*'.afterInterceptor {
     log.trace( 'request done')
}


I think this mechanism would be far superior to per-controller
interceptor definition and chaining which by its very nature will get
very messy very quickly.

It is already broken for inheritance. However getting inheritance
into the above model could be tricky too. It's part proposal, part
brain-fart.


Marc



~ ~ ~
Marc Palmer
Blog - http://www.anyware.co.uk/






---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email

[Alex Shneyderman]

I almost like it, but what would very helpfull if we could incorporate
in this solution a stack of interceptors rather than just a before and
after interceptor. Sure one could do whatever within the before and
after but I think its quite valuable to have a declarative way to
specify exactly the number and sequence of interceptors and of course
make interceptor an artifact it would be a very usefull feature. Make
it wireable just like any other artifact.

Also, I do not think that spelling it out is at all neccessary. Just
have an interceptor, if it happens to do something before and after it
is totally the matter of particular
implementation.

Is there a proposal on this? Maybe we should put something up on wiki?

BTW, what target version (grails), if any, are you guys thinking of?

On 4/17/07, Dmitriy Kopylenko <dmitriy.kopylenko@...> wrote:

> I like this! :-)
>
> Cheers,
> Dmitriy.
>
> 2007/4/16, Marc Palmer <marc@...>:
> >
> > On 16 Apr 2007, at 16:03, Peter Ledbrook wrote:
> >
> > > Hi,
> > >
> > > I have recently been looking into adding before interceptors to
> > > controllers dynamically from a plugin. There were quite a few hurdles
> > > to overcome and now I feel it may be better to shift attention to
> > > support for multiple interceptors (
> > > http://jira.codehaus.org/browse/GRAILS-92 ).
> > >
> > > What I would like to see is the ability to add a new interceptor to a
> > > controller from a plugin. Something like this:
> > >
> > >  application.controllerClasses.each { controller ->
> > >      // Check whether this controller has an 'accessControl' property.
> > >      def prop = controller.getProperty('accessControl')
> > >
> > >      // Add an interceptor if the property exists.
> > >      if (prop != null) {
> > >          def interceptor = {
> > >              // This is the interceptor...
> > >          }
> > >          controller.beforeInterceptors << [ action: interceptor,
> > > except: 'index' ]
> > >      }
> > >  }
> >
> > Can I suggest that we don't do this. I think this is a kludge.
> >
> > There is an (as yet undocumented I think) plan to do for interceptors
> > what UrlMappings has done for controllers.
> >
> > i.e. XXXXInterceptors.groovy would allow you to define per-controller
> > and per-action namespace interceptors:
> >
> > userprofile.'*'.beforeInterceptor {
> >      if (!authenticated) redirect( controller:'user', view:'login')
> > }
> >
> > userprofile.saveDetails.afterInterceptor {
> >      cacheService.refreshUserProfile ( params.userId)
> > }
> >
> > '*'.afterInterceptor {
> >      log.trace( 'request done')
> > }
> >
> >
> > I think this mechanism would be far superior to per-controller
> > interceptor definition and chaining which by its very nature will get
> > very messy very quickly.
> >
> > It is already broken for inheritance. However getting inheritance
> > into the above model could be tricky too. It's part proposal, part
> > brain-fart.
> >
> >
> > Marc
> >
> >
> >
> > ~ ~ ~
> > Marc Palmer
> > Blog - http://www.anyware.co.uk/
> >
> >
> >
> >
> >
> >
> >
> ---------------------------------------------------------------------
> > To unsubscribe from this list please visit:
> >
> >     http://xircles.codehaus.org/manage_email
> >
> >
>
>

--
Thanks,
Alex.

---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email

[Marc Palmer Local]

On 17 Apr 2007, at 07:52, Alex Shneyderman wrote:

> I almost like it, but what would very helpfull if we could incorporate
> in this solution a stack of interceptors rather than just a before and
> after interceptor. Sure one could do whatever within the before and
> after but I think its quite valuable to have a declarative way to
> specify exactly the number and sequence of interceptors and of course
> make interceptor an artifact it would be a very usefull feature. Make
> it wireable just like any other artifact.
>

I'm not sure we want an Interceptor artefact, if that is what you  
mean. Interceptors, as shown in my example and like the UrlMappings  
mechanism, are closures and as such can be put anywhere you like and  
referenced within the mapping/interceptor setup (caveat: might be  
some work required to make that happen cleanly).

Part of the example I gave rewritten to use closures from another  
source:

> userprofile.'*'.beforeInterceptor
> (MySecurityInterceptors.authRedirector)
>
> userprofile.saveDetails.afterInterceptor
> (MyCacheControlInterceptors.userProfileRefresh)

I'm not 100% happy with the syntax I have "proposed" ad hoc here, but  
I think it is a starting point. One thing is "for sure" (well nothing  
is) - interceptors should not be set up on a URI path basis.

In terms of a stack, that is what you have there. You can duplicate  
them etc.

If a plugin wants to add some, it will do so with a custom  
Interceptors.groovy or similar. However the order in which these are  
executed will need to be predefined/set in the code somehow.

i.e. we basically have the choice of "plugin interceptors are called  
before app interceptors" or "plugin interceptors are called after app  
interceptors". I don't see any really useful/meaningful way of  
injecting interceptors at a certain point in the stack from within  
plugins. You could possibly do this with arbitrary weighting values,  
but this is rather fragile imo and overkill.

If you really want to do something like that you can write a  
SuperWeightedComplexInterceptionChainPlugin that inserts just 2  
interceptors, before and after every controller action, and then  
sorts any other interceptors added via a SuperWeightedInterceptor  
arefact.

> Also, I do not think that spelling it out is at all neccessary. Just
> have an interceptor, if it happens to do something before and after it
> is totally the matter of particular
> implementation.
>

I'm not sure what you mean here. There has to be an indication of  
when it should be invoked.

> Is there a proposal on this? Maybe we should put something up on wiki?
>

No nothing yet. Feel free to start something if you have a clear idea.

The more I think about this, I wonder if actually we should turn this  
into a generic AOP advice style approach that can work on any  
artefact classes, instead of just controllers.

i.e. an AOPAdviceBuilder. Thoughts?

> BTW, what target version (grails), if any, are you guys thinking of?
>

Certainly not 0.5 :)

I'm going to take on the task of finalizing the 1.0 featureset with  
you guys as soon as I get a spare minute.

Marc


---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email

[Peter Ledbrook]

> No nothing yet. Feel free to start something if you have a clear idea.
>
> The more I think about this, I wonder if actually we should turn this
> into a generic AOP advice style approach that can work on any
> artefact classes, instead of just controllers.
>
> i.e. an AOPAdviceBuilder. Thoughts?

Well, I can certainly foresee a demand for AOP on services and
possibly domain objects. Out of interest, would Spring AOP work in
this context? Or would that only work with declared methods
(services), not closures (controllers)? I noticed that an AOPProxyBean
is already used for controllers, but I haven't got my head around how
it works yet.

> > BTW, what target version (grails), if any, are you guys thinking of?
> >
>
> Certainly not 0.5 :)
>
> I'm going to take on the task of finalizing the 1.0 featureset with
> you guys as soon as I get a spare minute.

I think I'll just go with an abstract controller with the before
interceptor defined on it for now.

Cheers,

Peter

---------------------------------------------------------------------
To unsubscribe from this list please visit:

    http://xircles.codehaus.org/manage_email