On Fri, May 1, 2009 at 8:59 PM, Les Hazlewood <[email protected]> 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 > <[email protected]>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 <[email protected]> > > 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 > > > > > >
