WebConfiguration vs KiFilter

[Les Hazlewood <lh...@apache.org>]

I've recently started thinking again how to better resolve the issues Kalle
brings up in this previous thread:
http://markmail.org/thread/ofk6dfavjn7wlafo

During my review of the code, it appears that our default WebConfiguration
implementation is really just an unnecessary abstraction away from a normal
Filter implementation.  Because our implementation requires the FilterConfig
object and then performs configuration steps based on FilterConfig values,
the code feels like the logic that exists in most filters, as if it should
just be in the KiFilter (or maybe an intermediate superclass) by default.
That is, our implementation is very filter specific, and a separate instance
doesn't buy us much in this case.

My question - does it make sense to just have the KiFilter (or an
intermediate superclass) do most of this stuff, and recommend integration
components just subclass it? (e.g. SpringKiFilter, TapestryKiFilter, etc)?

Typically I favor Composition over Inheritance, but it seems as if
subclassing might make more sense here.  Of course there could but
SpringWebConfiguration and TapestryWebConfiguration implementations instead
of subclassing the KiFilter, but if they have to work with FilterConfig
objects and other related concepts, why not just subclass KiFilter?  Then
the one method in WebConfiguration would just be a protected method in the
KiFilter.

Thoughts?

Les

Re: WebConfiguration vs KiFilter

[Kalle Korhonen <ka...@gmail.com>]

On Fri, May 1, 2009 at 8:59 PM, Les Hazlewood <lh...@apache.org> wrote:

> I'm not sure using an interface other than the Servlet spec provided ones
> would make a lot of sense.  I was very surprised to see the Tapestry
> interfaces nearly mirrored exactly the Servlet interfaces - I'm confused as
> to why they would choose to ignore something so common to web frameworks.


The reason is the whole Tapestry is implemented as a filter. Also, creating
an arbitrary pipeline (whether filters or any other services) is almost
trivial with Tapestry IoC container. You can certainly use plain servlet
filters before/after Tapestry filter, but then you cannot take advantage of
other Tapestry features in those filters. And furthermore, since the objects
are not directly controlled by the container, you can implement them as
shadow proxies, so you can do things like injecting a HttpServletRequest to
the constructor of the service, but expect that object to transparently
refer to the current request every time you call that service.


> We did, very early on, have our own separate interface but changed to using
> the Filter interface - somewhere around ~ 0.2 I believe.  After switching,
> it seemed to make a lot more sense and newcomers seemed to easily
> assimilate
> the concepts much faster.
> So maybe Tapestry's specific environment falls outside of the 80/20 rule?
> I'm not sure...


That may very well be.


> Your JSecurityConfiguration.java class seems like a really good solution.
> Any ideas of how we could support what you recommend in a clean manner?


In small iterations, but I think it roughly comes down to whether or not Ki
should use servlet filter or its own interface. Additionally, if KiFilter
would delegate the operations to a utility pojo, we'd get a little bit more
reuse. But that said, I'm not sure either that Ki classes should be
refactored just because of one framework does things differently.
Composition is more maintainable but always comes with a cost of more lines
of code and for the rest of the frameworks it might seem like an
unnecessary, extra layer of indirection.


> Also, take a look at the latest KiFilter source code.  It separates out a
> lot of the bind/unbind calls that could be called by subclasses.  Perhaps
> it
> would be a good idea to push these methods up to the highest superclass in
> the chain?  Maybe ServletContextSupport?  Then you could have a
> Tapestry-specific subclass of that and leverage those methods as necessary.


Didn't check the code yet, but sounds like this might be a safe and easy
step to take for better reusability (for me mostly) without convoluting Ki
code.


> Of course you'd still need your HandlerFilterChain to make use of all the
> Ki
> security Filters implementations, but that seems like a small thing to
> write
> given how simple it is and that you get to leverage existing functionality.
> When you said you had to create wrappers, I thought you were creating a
> wrapper _per_ existing authc/authz Filter - a real pain.  But it appears
> that your HandlerFilterChain is the only wrapper that is necessary.  If
> that
> is the case, it seems like a small price to pay to make things play nice in
> Tapestry-land.


Yes, you've done your homework. It's not only the HandlerFilterChain, but
also HttpServletRequestFilterWrapper that is needed but yes, it's certainly
nice I don't have to write specific wrappers for any filter. The only thing
I worry about is it gets a bit complicated for others reading the code
without apparent benefit.

What's there roughly works, and I do plan to update it to use the latest Ki
code before releasing anything. Thanks for taking my considerations into
account. Since we don't know exactly how things will evolve, it might be
wisest to keep taking baby steps and refactor things a bit at a time and
avoid making any drastic changes.

Kalle



On Fri, May 1, 2009 at 6:57 PM, Kalle Korhonen
> <ka...@gmail.com>wrote:
>
> > Thanks Les.
> >
> > Well, Tapestry might be the exception here, but *if* Ki would use
> > composition rather than inheritance, I would be able to use the bind()
> and
> > unbind() operations from the same class that KiFilter would delegate to,
> > rather than copying the same code to my Tapestry specific Ki filter,
> which
> > is what I current do. Tapestry doesn't use javax.servlet.Filter interface
> > but implements its own
> > org.apache.tastery5.services.HttpServletRequestFilter. For the same
> reason,
> > I have to create wrappers around the existing authc, authz.. etc. Ki
> > Filters
> > so I can use them in my Tapestry application without having to re-write
> > them
> > for Tapestry. If Ki had its own SecurityFilter, or similar root
> interface,
> > it'd make it easier to use Ki in web applications that do not use the
> > Servlet interface. The default Ki implementations obviously could use
> > inheritance to minimize overhead for maintaining wrapper objects.
> >
> > Kalle
> >
> > PS. some links for the current, not-yet-released implementation:
> > http://svn.codehaus.org/trails/trunk/tapestry-jsecurity/
> >
> >
> http://svn.codehaus.org/trails/trunk/tapestry-jsecurity/src/main/java/org/trailsframework/security/services/JSecurityConfiguration.java
> >
> >
> >
> > On Fri, May 1, 2009 at 3:29 PM, Les Hazlewood <lh...@apache.org>
> > wrote:
> >
> > > I've recently started thinking again how to better resolve the issues
> > Kalle
> > > brings up in this previous thread:
> > > http://markmail.org/thread/ofk6dfavjn7wlafo
> > >
> > > During my review of the code, it appears that our default
> > WebConfiguration
> > > implementation is really just an unnecessary abstraction away from a
> > normal
> > > Filter implementation.  Because our implementation requires the
> > > FilterConfig
> > > object and then performs configuration steps based on FilterConfig
> > values,
> > > the code feels like the logic that exists in most filters, as if it
> > should
> > > just be in the KiFilter (or maybe an intermediate superclass) by
> default.
> > > That is, our implementation is very filter specific, and a separate
> > > instance
> > > doesn't buy us much in this case.
> > >
> > > My question - does it make sense to just have the KiFilter (or an
> > > intermediate superclass) do most of this stuff, and recommend
> integration
> > > components just subclass it? (e.g. SpringKiFilter, TapestryKiFilter,
> > etc)?
> > >
> > > Typically I favor Composition over Inheritance, but it seems as if
> > > subclassing might make more sense here.  Of course there could but
> > > SpringWebConfiguration and TapestryWebConfiguration implementations
> > instead
> > > of subclassing the KiFilter, but if they have to work with FilterConfig
> > > objects and other related concepts, why not just subclass KiFilter?
>  Then
> > > the one method in WebConfiguration would just be a protected method in
> > the
> > > KiFilter.
> > >
> > > Thoughts?
> > >
> > > Les
> > >
> >
>

Re: WebConfiguration vs KiFilter

[Les Hazlewood <lh...@apache.org>]

Hi Kalle,

I'm not sure using an interface other than the Servlet spec provided ones
would make a lot of sense.  I was very surprised to see the Tapestry
interfaces nearly mirrored exactly the Servlet interfaces - I'm confused as
to why they would choose to ignore something so common to web frameworks.

We did, very early on, have our own separate interface but changed to using
the Filter interface - somewhere around ~ 0.2 I believe.  After switching,
it seemed to make a lot more sense and newcomers seemed to easily assimilate
the concepts much faster.

So maybe Tapestry's specific environment falls outside of the 80/20 rule?
I'm not sure...

Your JSecurityConfiguration.java class seems like a really good solution.
Any ideas of how we could support what you recommend in a clean manner?

Also, take a look at the latest KiFilter source code.  It separates out a
lot of the bind/unbind calls that could be called by subclasses.  Perhaps it
would be a good idea to push these methods up to the highest superclass in
the chain?  Maybe ServletContextSupport?  Then you could have a
Tapestry-specific subclass of that and leverage those methods as necessary.


Of course you'd still need your HandlerFilterChain to make use of all the Ki
security Filters implementations, but that seems like a small thing to write
given how simple it is and that you get to leverage existing functionality.
When you said you had to create wrappers, I thought you were creating a
wrapper _per_ existing authc/authz Filter - a real pain.  But it appears
that your HandlerFilterChain is the only wrapper that is necessary.  If that
is the case, it seems like a small price to pay to make things play nice in
Tapestry-land.

Thoughts?

Les

On Fri, May 1, 2009 at 6:57 PM, Kalle Korhonen
<ka...@gmail.com>wrote:

> Thanks Les.
>
> Well, Tapestry might be the exception here, but *if* Ki would use
> composition rather than inheritance, I would be able to use the bind() and
> unbind() operations from the same class that KiFilter would delegate to,
> rather than copying the same code to my Tapestry specific Ki filter, which
> is what I current do. Tapestry doesn't use javax.servlet.Filter interface
> but implements its own
> org.apache.tastery5.services.HttpServletRequestFilter. For the same reason,
> I have to create wrappers around the existing authc, authz.. etc. Ki
> Filters
> so I can use them in my Tapestry application without having to re-write
> them
> for Tapestry. If Ki had its own SecurityFilter, or similar root interface,
> it'd make it easier to use Ki in web applications that do not use the
> Servlet interface. The default Ki implementations obviously could use
> inheritance to minimize overhead for maintaining wrapper objects.
>
> Kalle
>
> PS. some links for the current, not-yet-released implementation:
> http://svn.codehaus.org/trails/trunk/tapestry-jsecurity/
>
> http://svn.codehaus.org/trails/trunk/tapestry-jsecurity/src/main/java/org/trailsframework/security/services/JSecurityConfiguration.java
>
>
>
> On Fri, May 1, 2009 at 3:29 PM, Les Hazlewood <lh...@apache.org>
> wrote:
>
> > I've recently started thinking again how to better resolve the issues
> Kalle
> > brings up in this previous thread:
> > http://markmail.org/thread/ofk6dfavjn7wlafo
> >
> > During my review of the code, it appears that our default
> WebConfiguration
> > implementation is really just an unnecessary abstraction away from a
> normal
> > Filter implementation.  Because our implementation requires the
> > FilterConfig
> > object and then performs configuration steps based on FilterConfig
> values,
> > the code feels like the logic that exists in most filters, as if it
> should
> > just be in the KiFilter (or maybe an intermediate superclass) by default.
> > That is, our implementation is very filter specific, and a separate
> > instance
> > doesn't buy us much in this case.
> >
> > My question - does it make sense to just have the KiFilter (or an
> > intermediate superclass) do most of this stuff, and recommend integration
> > components just subclass it? (e.g. SpringKiFilter, TapestryKiFilter,
> etc)?
> >
> > Typically I favor Composition over Inheritance, but it seems as if
> > subclassing might make more sense here.  Of course there could but
> > SpringWebConfiguration and TapestryWebConfiguration implementations
> instead
> > of subclassing the KiFilter, but if they have to work with FilterConfig
> > objects and other related concepts, why not just subclass KiFilter?  Then
> > the one method in WebConfiguration would just be a protected method in
> the
> > KiFilter.
> >
> > Thoughts?
> >
> > Les
> >
>

Re: WebConfiguration vs KiFilter

[Kalle Korhonen <ka...@gmail.com>]

Thanks Les.

Well, Tapestry might be the exception here, but *if* Ki would use
composition rather than inheritance, I would be able to use the bind() and
unbind() operations from the same class that KiFilter would delegate to,
rather than copying the same code to my Tapestry specific Ki filter, which
is what I current do. Tapestry doesn't use javax.servlet.Filter interface
but implements its own
org.apache.tastery5.services.HttpServletRequestFilter. For the same reason,
I have to create wrappers around the existing authc, authz.. etc. Ki Filters
so I can use them in my Tapestry application without having to re-write them
for Tapestry. If Ki had its own SecurityFilter, or similar root interface,
it'd make it easier to use Ki in web applications that do not use the
Servlet interface. The default Ki implementations obviously could use
inheritance to minimize overhead for maintaining wrapper objects.

Kalle

PS. some links for the current, not-yet-released implementation:
http://svn.codehaus.org/trails/trunk/tapestry-jsecurity/
http://svn.codehaus.org/trails/trunk/tapestry-jsecurity/src/main/java/org/trailsframework/security/services/JSecurityConfiguration.java



On Fri, May 1, 2009 at 3:29 PM, Les Hazlewood <lh...@apache.org> wrote:

> I've recently started thinking again how to better resolve the issues Kalle
> brings up in this previous thread:
> http://markmail.org/thread/ofk6dfavjn7wlafo
>
> During my review of the code, it appears that our default WebConfiguration
> implementation is really just an unnecessary abstraction away from a normal
> Filter implementation.  Because our implementation requires the
> FilterConfig
> object and then performs configuration steps based on FilterConfig values,
> the code feels like the logic that exists in most filters, as if it should
> just be in the KiFilter (or maybe an intermediate superclass) by default.
> That is, our implementation is very filter specific, and a separate
> instance
> doesn't buy us much in this case.
>
> My question - does it make sense to just have the KiFilter (or an
> intermediate superclass) do most of this stuff, and recommend integration
> components just subclass it? (e.g. SpringKiFilter, TapestryKiFilter, etc)?
>
> Typically I favor Composition over Inheritance, but it seems as if
> subclassing might make more sense here.  Of course there could but
> SpringWebConfiguration and TapestryWebConfiguration implementations instead
> of subclassing the KiFilter, but if they have to work with FilterConfig
> objects and other related concepts, why not just subclass KiFilter?  Then
> the one method in WebConfiguration would just be a protected method in the
> KiFilter.
>
> Thoughts?
>
> Les
>