velocity-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gabriel Sidler <sid...@teamup.ch>
Subject Re: [Veltools] ToolboxManager (round 4 :-)
Date Thu, 02 May 2002 20:45:37 GMT
Nathan,
I finally got around to study your latest proposal regarding the tool
management. Overall I like the approach and think it is a good basis
to move on.

I'd like to make a few remarks/suggestions:

- Your proposed DTD for the toolbox configuration makes good sense
to me. The minor exception is that the 'scope' element seems to be
missing. Or, am I missing something?

- In our past discussion Dan argued for a destroy() method
as part of the ContextTool interface. This would be the place to do
clean-up work or release resources. Your current proposal doesn't
address this. What's your thinking on that.

- ServletToolInfo.java: Here you add scope support to tool info. I have
no problem with the implemenation of the class but think that the
name is not the most appropriate one. In my view the concept of 'scope'
is not limited to a servlet environment. In fact, no part of the class
implementation is dependent on the Servlet API. Assume that I want to implement
a toolbox manager for DVSL. In such an environment the scopes "request"
and "application" make perfect sense. I see two possible resolutions:
   - Rename the class to ScopedToolInfo
   - Merge ServletToolInfo and ContextToolInfo and name the class ContextToolInfo.
     I actually don't see an advantage of having two classes instead of one.
     Maybe I overlook something?

- ServletToolboxManager.getToolboxContext(): The instantiation of
   session tools should be synchronized, otherwise it might happen that
   the session-scoped tools are instantiated multiple times for the same
   session.

- I am quite uncomfortable with the proposed init method of ContextTool.

     public void init(Object initData);

   This is virtually moving us back to C-times where we worked with pointers
   and suffered from all kind of related problems. The interface is basically
   specified through the documentation. Yes, this approach
   keeps the interface between tools and the toolbox manager simple. But,
   the price is a problematic, underspecified, error-prone interface. That's
   too high a price. Especially since we are not talking about very many
   different init-cases. Currently we have identified two cases:
     - Tools that need access to the servlet environment
     - Tools that need access to the Velocity context
   I really think we should go the extra mile and handle these two cases
   with two specific interfaces.


Nathan wrote:
 > (Gabe, if you would really prefer to call this "ViewTool",  I can live with
 > it.  But i still think ContextTool will be more meaningful to users.)

I really would prefer ViewTool. From talking to people, my experience
is that ViewTool is more intuitive than ContextTool.
Furthermore, there is nothing about these tools that is specific to
the context. Much more, they are specific to the view layer. If you
consider a situation where the same tool is used with multiple view
technologies, then 'context tool' seems quite inappropriate.



That's all I have. I am looking forward to your feedback on the
mentioned points.

Gabe



Nathan Bubna wrote:

> Ok.  I've rewritten this stuff pretty heavily, so bear with me if this
> proposal gets a little long...
> 
> First, i think it'd be good to have a generic ToolboxManager interface.
> This should allow for varying implementations (dvsl, vvs, etc.)  So I figure
> we need a way to tell the manager what to manage, and a way to get the
> toolbox context.
> 
> public interface ToolboxManager
> {
>     void addTool(ToolInfo info);
> 
>     ToolboxContext getToolboxContext(Object initData);
> }
> 
> I think that's probably all we really need.  I don't want to put
> load(InputStream) here, in case we want to load some other way.
> 
> Now, the ToolInfo class should provide enough information for the manager to
> appropriately manage the tools.  I don't want to pass the tool directly,
> because I don't want to force programmers to hard-code info that's pertitent
> to the management of the tool, but not to the tool's actual functions.  So,
> I figure the bare bones ToolInfo should be something like:
> 
> public interface ToolInfo
> {
>     String getKey();
> 
>     String getClassname();
> 
>     Object getInstance(Object initData);
> }
> 
> You can get the key, the class of the tool, and an actual instance of the
> tool.
> At this point, you are probably wondering about this initData stuff.   Well,
> I decided to take a tip from Turbine.  Instead of having a new interface for
> every type of tool initialization people come up with, i figure we really
> only need one:
> 
> public interface ContextTool
> {
>     public void init(Object initData);
> }
> 
> (Gabe, if you would really prefer to call this "ViewTool",  I can live with
> it.  But i still think ContextTool will be more meaningful to users.)
> 
> since we're going to use ToolInfo objects to deal with info about the tool,
> we don't need to use interfaces as markers anymore.   so, there really isn't
> any point to having umpteen different virtually identical initialization
> interfaces.  we can just leave it up to the tool to deal with the initData
> as need be.  this has worked well for Turbine's PullService, and i think it
> will work well here.  We provide the hook for the initData to be passed to
> the toolbox manager on toolbox context requests, and implementations of the
> manager can then pass it on to the ToolInfo in order to get an initialized
> tool instance.  those tools which implement ContextTool would then
> appropriately cast the initData and use it.  The other obvious benefit of
> this is that we don't explicitly pass the ViewContext anymore (which is tied
> to the servlet environment).
> 
> ---
> Ok, that's the fundamentals.  Now to discuss the implementations I've
> attached to this email.
> 
> One major need in the current toolbox system is the ability to specify raw
> data constants as tools (String, Number, Boolean).  That's what the DataInfo
> class is for; it provides a fitting constructor to build the info and
> implements getInstance() efficiently.
> 
> The ContextToolInfo class offers an efficient means to deal with arbitrary
> classes as tools, and in particular, it makes clean use of the ContextTool
> interface to initialize tools appropriately.
> 
> ServletToolInfo extends ContextToolInfo to hold additional information about
> the tool's scope placement.
> 
> I can also envision someone creating a Poolable interface that offers a
> destroy()/reset() method, and implementing a PoolableToolInfo class that
> pools instances of Poolable tools in order to more efficiently implement
> getInstance().  I'm not convinced that these belong in this project though.
> keep it simple...
> 
> XMLToolboxManager is an implemenation of ToolboxManager that has no servlet
> environment ties and loads the toolbox from xml.  I would hope that this is
> a large step towards abstracting tool management.   Since we never really
> agreed on a common DTD for dvsl and vvs, i just made my own:
> 
> <?xml version="1.0"?>
> <!ELEMENT toolbox (tool*,data*)>
> <!ELEMENT tool    (key,class,#PCDATA)>
> <!ELEMENT data    (key,value)>
>     <!ATTLIST data type (string|number|boolean) "string">
> <!ELEMENT key     (#CDATA)>
> <!ELEMENT class   (#CDATA)>
> <!ELEMENT value   (#CDATA)>
> 
> feel free to criticize, improve, whatever as always.  i don't really have
> muc experience with DTD/XML stuff, so it may be totally whacked.  it would
> probably also be nice to have it defined as an XML Schema, but i didn't
> bother as i didn't put much thought in here to begin with.  Anyway, that's
> the layout XMLToolboxManager currently reads.  Note that i have implemented
> the raw data loading as well as the arbitrary class loading.  the actual
> management implemented is, however, quite minimal.  it may be useful in some
> instances, but for VVS, we really need...
> 
> ServletToolboxManager.   it extends XMLToolboxManager to appropriately read
> ServletToolInfo and efficiently implement the addTool and getToolboxContext
> methods for the VVS.
> 
> I haven't done any serious testing of the code, but i did update my current
> app to use it and it worked like a charm.  Of course, in that process, i
> also had to update all the existing struts and other tools to work with this
> system, so i will happily send patches for those if so desired.
> 
> Now,  you probably noticed that i didn't implement any of the tool logging
> stuff.  This is because I finally got around to really thinking about it,
> and i decided i don't like the idea of adding our own logging stuff.   There
> are plenty of well tested, designed, and used logging APIs out there
> already.  If you have a custom tool that really needs logging, I think it
> would be best if you used one of those.  As for the VVS tools that we have
> in this codebase,  most need virtually no logging, and the ones that
> reasonably do need logging have access to the ServletContext and can just
> log to that.   I really don't see good reason to starting getting all
> framework-ish and mess around with our own logging API.
> 
> If, of course, you really really want to write your own logging stuff, It
> should be easy enough to write your own ToolInfo/ToolboxManager
> implementations to take care of that.
> 
> So, in my code, i have removed:
> ViewToolLogger
> ServletViewToolLogger
> LogEnabledContextTool
> LogEnabledContextToolImpl
> 
> as well as the old tool interfaces:
> ContextViewTool
> ServletViewTool
> ThreadSafeViewTool
> 
> Well, i guess now's the time to begin the feedback/rejection process. :-)  I
> hope you'll take a decent look at the code first.  That should help prevent
> some misunderstandings.   I can also provide you with my new
> velocity-tools-view.jar  if you desire.
> 
> Things are picking up at work again, so i won't have much time for this in
> the next few weeks, but i'll do what i can.
> 
> Nathan Bubna
> nathan@esha.com
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --
> To unsubscribe, e-mail:   <mailto:velocity-dev-unsubscribe@jakarta.apache.org>
> For additional commands, e-mail: <mailto:velocity-dev-help@jakarta.apache.org>
> 


-- 
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland


--
To unsubscribe, e-mail:   <mailto:velocity-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:velocity-dev-help@jakarta.apache.org>


Mime
View raw message