You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Gabriel Sidler <si...@teamup.ch> on 2002/04/02 19:17:18 UTC
[VelTools] Changes
Hi,
I've just checked in a number of changes to the jakarta-velocity-tools
project. Your feedback is welcome. Here's a brief outline of the
major changes:
- Introduced four interfaces that establish contracts between
context tools and compatible toolbox managers:
- ThreadSafeContextTool: for thread-safe context tool, tool
instance is reused for the entire runtime
- LogEnabledContextTool: for context tools that need access
to a logging facility
- ServletContextTool: for context tools that need access to
the servlet envrionment
- ContextContextTool: for context tools that need access to
the Velocity context
These interfaces are all optional. Any object with public methods
and a public, empty constructor can be used as a context tool.
Its handling may not be the most optimal one, though.
- Introduced logging for context tools
- Modified VelocityViewServlet so that it now can be configured
with a configuration file. Proposed location is
WEB-INF/velocity.properties. This allows now the use of a global
macros file (default location WEB-INF/VM_global_library.vm)
- Enhanced ToolboxManager
- Made initialization of context tools thread-safe by
synchronizing where needed.
- Tools of unknown classes have now by default a life cycle
of 'request' (based on Bill's input). This is required
because otherwise we might run into problem with tools that
are not thread-safe.
- Added support for the four interfaces listed above.
- Improved logging and error handling.
- Globally replaced the term 'scope' by the term 'life cycle'.
This reflects more accurately what it is.
- Setup a library of context tools and corresponding documentation.
Named it VelLibrary. Added Nathan's contributed context tools to
the VelLibrary and modified them to make them thread-safe. The
directory structure is as discussed earlier:
/jakarta-velocity-tools/tools/
xdocs
src/
docs/
lib/
build.xml
Included in VelLibray are now four tools: MathTool, DateTool,
RequestParser and ToolLoader. Added all tools to package
o.a.v.tools.tools. This is not commited yet. Awaiting Geir's
approval to check-in the structure as proposed above!
- Defined a documentation structure and added a first draft of
most pages. A current snapshot of the documentation is online
at http://www.teamup.com/veltools Please have a look and let me
know if it makes sense. I presented VelTools as an umbrella
project for several small subprojects:
- VelLibray: A colletion of reusalbe context tools
- VelStruts: Velocity/Struts integration
- VelServlet: Velocity template rendering servlet
There is some overview documentation and each of the subprojects
has a set of mostly self-contained documentation. My assumption
is that most users wound't care much about the overview documentation
and browse directly to the subproject that they are interested in.
- Renamed some of the classes to improve consistency.
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Nathan Bubna <na...@esha.com>.
Gabe said:
> This is getting too emotional for my taste.
yeah, i am sorry about that; it is my fault. it was a rough week for
completely other reasons, so i was a bit on edge to begin with. my
frustration with your changes to the tools i submitted was not without
cause, but i lost my some of my composure and i shouldn't have.
> Let me just assure you
> that I am always open for good technical arguments and solutions.
> But, I can only support what I understand and what makes sense to me.
> And, I can only commit what I can support.
well, in the future, would you at least mind discussing major changes to
code i submit _before_ you commit them? if you can, i would appreciate
that.
> From my point of view, our exchange of opinions has led to a nice
> design of the Struts tools. The result has benefited from input
> of all sides. I am confident that we achieve the same here.
i agree. but i feel i should add that exchanging opinions _before_
committing things to cvs will lead to a more open and civil exchange.
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:
...
>>As I said, your approach is a valid approach. I just don't see
>>what justifies changing the current approach and adapting all the
>>tools.
>>
>
> hey, back the train up! most of the current approach was designed and
> decided on pretty much unilaterally by you. when i complained that you
> changed so much without any consensus, you said it was just because that was
> the easiest way to exchange code. as such, i give no weight on the "why
> change what's already there" argument. as far as i'm concerned the 'current
> approach' is mostly just your proposal. there's not much dependent code out
> there right now, there's been no releases, and the changes we're talking
> about making are really quite quick. i don't think we need any reason than
> "this other way is better or preferable" to justify changing the 'current
> approach'.
This is getting too emotional for my taste. Let me just assure you
that I am always open for good technical arguments and solutions.
But, I can only support what I understand and what makes sense to me.
And, I can only commit what I can support.
From my point of view, our exchange of opinions has led to a nice
design of the Struts tools. The result has benefited from input
of all sides. I am confident that we achieve the same here.
Gabe
>
> Nathan Bubna
> nathan@esha.com
>
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
> .
>
>
--
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Nathan Bubna <na...@esha.com>.
Gabe said:
> Yes, it's an approach. There are so many ways to do this...
yes, but not all of them are good ways.
> This
> proposal is quite a turn from the factory appraoch that we pursued
> so far. I don't see yet the advantages of it.
well i see them, and Daniel seems to as well. no worries, i'm not done
trying to persuade you. but if we can't, then at some point we'd best put
it to some sort of vote. i doubt any of us have the time, patience, and
energy to discuss this forever. i know i don't.
> My understanding is that the factory approach pursued so far is
> a more powerful pattern. getInstance() can be a simple init but
> it can also do smarter and more complex things like reusing itself,
> pooling instances, etc. The factory appraoch also has advantages with
> regard to the thread-safety of tools.
i don't think the tools should have any responsibility for reusing
themselves or pooling. that's putting way too much responsibility in what
should be relatively simple classes. and exactly what advantages do you see
in the factory approach for application tools that my proposal lacks? you're
not gonna convince me by saying there are advantages, you need to say what
they are.
> As I said, your approach is a valid approach. I just don't see
> what justifies changing the current approach and adapting all the
> tools.
hey, back the train up! most of the current approach was designed and
decided on pretty much unilaterally by you. when i complained that you
changed so much without any consensus, you said it was just because that was
the easiest way to exchange code. as such, i give no weight on the "why
change what's already there" argument. as far as i'm concerned the 'current
approach' is mostly just your proposal. there's not much dependent code out
there right now, there's been no releases, and the changes we're talking
about making are really quite quick. i don't think we need any reason than
"this other way is better or preferable" to justify changing the 'current
approach'.
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:
> Daniel said:
>
>>...I would much rather see the marker
>>interfaces pattern used to denote tool scoping (i.e. request, session,
>>global).
>>
>
> i think our thoughts are somewhat close on this one, but because some tools
> may be usable in more than one scope, i do not want to rely on interfaces
> for denoting scope. i do, however, want multiple interfaces to allow tools
> of various scopes to be initialized in an appropriate manner. this way
> developers can control placement of tools by the config file and, if they
> implement the correct interface, have them initialized appropriately. this
> will allow a variable-scope tool to implement multiple context tool/scope
> interfaces and then function properly according to the scope as denoted by
> the config file. if we rely on interfaces to denote scope, such behaviour
> becomes very difficult to implement. more on this below...
>
>
>>>>I much prefer the old ContexTool interface.
>>>>
>>>i don't really have any gripes with the old 'destroy' method. i don't
>>>really see how it is needed or used, but that's just me. perhaps you
>>>
> have
>
>>>good use for it. i do, however, take issue with the 'init' method. the
>>>issue being that it does not functions as a factory method, not an
>>>initialization method. thus i prefer the name 'getInstance'. do you
>>>
> have
>
>>>another suggestion?
>>>
>>Yes, I do. Keep the context tool lifecycle interface separate from
>>the factory interface. Using a single method of the context tool
>>interface to mix in both an initialization and factory implementation
>>is confusing, and introduces tight coupling. Clearly delineate
>>between the two distinct interfaces by supplying a separate manager or
>>factory entity.
>>
>
> ah! you're right. i knew there was still something bugging me about
> ContextTool--the meshing of factory/init behaviour. Really, as i look at
> it, we don't really need the factory behaviour here. i think it will be
> fine for the manager to create new instances using the class, and simply use
> the interfaces for initialization. so here's my (strong) suggestion...
>
> //this is not to be confused with the current
> //(and IMO poorly named) ServletContextTool
>
> public interface ServletContextTool {
> void init(ServletContext app);
> }
>
> and
>
> public interface ViewContextTool {
> void init(ViewContext ctx);
> }
Yes, it's an approach. There are so many ways to do this... This
proposal is quite a turn from the factory appraoch that we pursued
so far. I don't see yet the advantages of it.
My understanding is that the factory approach pursued so far is
a more powerful pattern. getInstance() can be a simple init but
it can also do smarter and more complex things like reusing itself,
pooling instances, etc. The factory appraoch also has advantages with
regard to the thread-safety of tools.
As I said, your approach is a valid approach. I just don't see
what justifies changing the current approach and adapting all the
tools.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Nathan Bubna" <na...@esha.com> writes:
> the problem is that since i can't presently think of a real need for
> them to be initialized differently, i'm not sure what should be
> passed differently to initialize it.
As long as a Velocity Context is being passed in in some fashion,
you've got your interface for passing arbitrary initialization data
right there.
>> I also very much need a shutdown()/destroy() method available for
>> reclaiming resources allocated during init(). Consider the case where
>> your initialization method creates some heavy-weight objects for your
>> request tool, and you're pooling instances of this tool. Each time
>> you need to service a new request, you grab an instance of your
>> context tool from the pool and initialize it. The request is
>> processed, and your tool is returned to the pool. Unfortunately,
>> because there's not standard way to release the tool's resources, the
>> instance wallows in the pool, bloated with un-reclaimed resources,
>> until it is pulled from the pool again to service a new request.
>
> ah! i didn't think of this. hmm. i suppose the question becomes whether or
> not this belongs in the ViewContextTool interface (or any future session
> tool interface). for the present at least, VVS's toolbox manager doesn't
> do any pooling.
It should pool, or users need to be able to plug in implementations
which can pool.
> so, i'm hesitant to support adding a destroy() method just yet. i
> think until the toolbox manager supports pooling of tools, this may
> be better left alone. even then, i wonder if it might not be better
> to put it into a separate interface (a Poolable interface perhaps?).
> i dunno, i haven't messed with pooling much. i should probably
> think about this more.
Gabe seemed in favor of the pooling interface as well. Since you both
came up with it separate, that's probably the way to go. I believe
that there's a Commons Pool package which might handle this.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Nathan Bubna <na...@esha.com>.
Daniel said:
> > using these, the toolbox manager can check if application tools
implement
> > ServletContextTool and initialize new instances appropriately, and use
> > ViewContextTool similarly for session and request tools. at this point
i do
> > not see any point in having separate interfaces for session and request
> > tools, passing the ViewContext gives them both the access to the current
> > request and session as should be.
>
> I like it.
cool, i'll send the code i've got. however, i'm not sure i'm still in
complete agreement with myself anymore. :-) i've been chewing on this and i
think it may be prudent to have a third interface to distinguish session
tools from request tools. the problem is that since i can't presently think
of a real need for them to be initialized differently, i'm not sure what
should be passed differently to initialize it. i also wouldn't know what to
call it (that would/should depend on what it does). any thoughts? of
course, if we don't do this now, it's not a big deal. it should be easy to
change this later.
> > i think these should be quite sufficient. so, i would like
> > ContextContextTool, the current ServletContextTool, and
> > ThreadSafeContextTool to go the way of the dinosaur. i'm ignoring the
> > logging issue until i look into it and can justify my having an opinion.
> > also, i actually have most of this implemented already, so i can send
code
> > if you like.
>
> I also very much need a shutdown()/destroy() method available for
> reclaiming resources allocated during init(). Consider the case where
> your initialization method creates some heavy-weight objects for your
> request tool, and you're pooling instances of this tool. Each time
> you need to service a new request, you grab an instance of your
> context tool from the pool and initialize it. The request is
> processed, and your tool is returned to the pool. Unfortunately,
> because there's not standard way to release the tool's resources, the
> instance wallows in the pool, bloated with un-reclaimed resources,
> until it is pulled from the pool again to service a new request.
ah! i didn't think of this. hmm. i suppose the question becomes whether or
not this belongs in the ViewContextTool interface (or any future session
tool interface). for the present at least, VVS's toolbox manager doesn't
do any pooling. so, i'm hesitant to support adding a destroy() method just
yet. i think until the toolbox manager supports pooling of tools, this may
be better left alone. even then, i wonder if it might not be better to put
it into a separate interface (a Poolable interface perhaps?). i dunno, i
haven't messed with pooling much. i should probably think about this more.
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Nathan Bubna" <na...@esha.com> writes:
> Daniel said:
> > ...I would much rather see the marker
>> interfaces pattern used to denote tool scoping (i.e. request, session,
>> global).
>
> i think our thoughts are somewhat close on this one, but because some tools
> may be usable in more than one scope, i do not want to rely on interfaces
> for denoting scope. i do, however, want multiple interfaces to allow tools
> of various scopes to be initialized in an appropriate manner. this way
> developers can control placement of tools by the config file and, if they
> implement the correct interface, have them initialized appropriately. this
> will allow a variable-scope tool to implement multiple context tool/scope
> interfaces and then function properly according to the scope as denoted by
> the config file. if we rely on interfaces to denote scope, such behaviour
> becomes very difficult to implement. more on this below...
I'll go with that.
>> Keep the context tool lifecycle interface separate from the factory
>> interface. Using a single method of the context tool interface to
>> mix in both an initialization and factory implementation is
>> confusing, and introduces tight coupling. Clearly delineate
>> between the two distinct interfaces by supplying a separate manager
>> or factory entity.
>
> ah! you're right. i knew there was still something bugging me about
> ContextTool--the meshing of factory/init behaviour. Really, as i look at
> it, we don't really need the factory behaviour here. i think it will be
> fine for the manager to create new instances using the class, and simply use
> the interfaces for initialization.
Exactly. FWIW, this is how Turbine's PullService works.
> so here's my (strong) suggestion...
>
> //this is not to be confused with the current
> //(and IMO poorly named) ServletContextTool
>
> public interface ServletContextTool {
> void init(ServletContext app);
> }
>
> and
>
> public interface ViewContextTool {
> void init(ViewContext ctx);
> }
>
> using these, the toolbox manager can check if application tools implement
> ServletContextTool and initialize new instances appropriately, and use
> ViewContextTool similarly for session and request tools. at this point i do
> not see any point in having separate interfaces for session and request
> tools, passing the ViewContext gives them both the access to the current
> request and session as should be.
I like it.
> i think these should be quite sufficient. so, i would like
> ContextContextTool, the current ServletContextTool, and
> ThreadSafeContextTool to go the way of the dinosaur. i'm ignoring the
> logging issue until i look into it and can justify my having an opinion.
> also, i actually have most of this implemented already, so i can send code
> if you like.
I also very much need a shutdown()/destroy() method available for
reclaiming resources allocated during init(). Consider the case where
your initialization method creates some heavy-weight objects for your
request tool, and you're pooling instances of this tool. Each time
you need to service a new request, you grab an instance of your
context tool from the pool and initialize it. The request is
processed, and your tool is returned to the pool. Unfortunately,
because there's not standard way to release the tool's resources, the
instance wallows in the pool, bloated with un-reclaimed resources,
until it is pulled from the pool again to service a new request.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] proposed change to ToolboxManager (round 3 i think)
Posted by Nathan Bubna <na...@esha.com>.
Gabe said:
> Regarding your secondary reason: I think you forget an important
> real case: An application scope tool that needs access to the
> Velocity context.
hmm. i'm having trouble imagining this one. the velocity context is
recreated on each request, so what business does/could an application tool
have with the velocity context?
...
> Well, what I really meant was *encourage* safe design. I am sure
> you forgive a non-native english writter some inaccuraccies. I
> sometimes really struggle to find the precise words.
heh. sorry, i'll try not to be too picky, but that is a hard thing for a
chronic perfectionist like me. :-)
> My point is not that you allow some things that don't make sense.
> I am saying that you overlook some real cases that should be allowed.
ok. i'll try to keep that frame of reference.
> >>Yes, one tool I really would like to have is a context inspector that
> >>shows me what tools are loaded and what variables are available.
> >>Today, for template designers the Velocity context is a black hole.
> >>
> > hey that could be a really cool tool for development; i'd like to see
that.
> > ...but it probably shouldn't be used in a production situation. anyway,
> > this tool can be written just fine as a ViewContext tool. there isn't
> > anything particulary inappropriate about such a tool having access to
other
> > request, session, and application data (unlike the case of
> > ServletContextTool, where it is highly inappropriate for them to access
> > request and session data). so, i don't think there's justification for
> > another interface here.
>
> There is no reason to tie such a tool to the Servlet environment.
> Such a tool is useful in any environment where Velocity is used.
hmm.... this is a really good point. such a tool could be quite useful for
developing in any environment. but as yet, it is the only tool i can think
of that legitimately fits this situation, and it is a tool unlikely to be
needed in production environments. as i do feel it is important to keep the
interfaces and toolbox manager design as simple as we reasonably can, i'm
not ready to endorse a separate interface for this situation.
as an alternative, this problem can be easily solved by extending a properly
designed context analyzer tool (i.e. not tied to the servlet environment) to
implement ViewContextTool so it can be easily used with the
ServletToolboxManager.
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] proposed change to ToolboxManager (round 3 i think)
Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:
> Gabe said:
>
...
> No. that's not basically what i'm saying. you took my "furthermore" reason
> (which is secondary) and called it my primary one. my primary reason was
> the lack of a ViewContext object at the point of initializing my application
> tools. if you had bothered to look at my code, you would see that. (now to
> rephrase and repeat my "furthermore"...)there is no point in putting off
> initialization of the application tools until the first request just so that
> we can give developers access to data they neither need nor should use for
> application tools.
>
Sorry for misinterpreting you. But, your primary reason was
related to your current implementation and I didn't think
that this is something that cannot be changed.
Regarding your secondary reason: I think you forget an important
real case: An application scope tool that needs access to the
Velocity context.
>>Our primary concern should be to provide a safe API to the template
>>
> designer.
>
> wha?! we're talking about the interfaces the tool developers will use in
> designing the tools! those interfaces only indirectly concern the API given
> to the template designers. it is up to the tool developers to provide a
> safe API to them. that is not our job here. we can *encourage* safe tool
> design and scope-placement, but it's not our job (nor our primary concern)
> to ensure it.
Well, what I really meant was *encourage* safe design. I am sure
you forgive a non-native english writter some inaccuraccies. I
sometimes really struggle to find the precise words.
>>>>- Regarding the scope-behaviour outlined above: Why can
>>>>ViewContextTools not be in application scope? Why can
>>>>ServletContextTools not be in request or session scope?
>>>>
>>>>
>>>i've answered the first question above. as for the second question:
>>>using a ServletContextTool in request or session scope is a waste of
>>>
> time.
>
>>>i would not want to encourage it by allowing it :-). if a developer
>>>
> has a
>
>>>tool that he wants to use in both application and session/request, then
>>>
> he
>
>>>need only implement both interfaces.
>>>
>>This is quite intransparent to me and makes things unnecessary complex.
>>
>
> makes perfect sense to me, and i think it's pretty simple. in any case, my
> toolbox manager code was shorter and simpler than yours, so in what way is
> my proposal more complex?
>
>
>>On the one side, you allow the free configuration of the scope in the
>>toolbox configuration file. On the other side, for certain classes
>>certain scopes are implied.
>>
>
> yes. that's essentially the goal. developers can put the tools wherever
> they want regardless of what interfaces they implement, but if they want
> their tools to be initialized with appropriate data, they must implement the
> appropriate interface.
>
>
>>In my view a ServletContextTool tool may very well live in request of
>>
> session
>
>>scope and a ViewContextTool may live in application scope. Think about the
>>following two cases:
>>
>>- A ServletContextTool tool has read-write instance variables and is
>> therefore not thread safe. It cannot go to application scope.
>>
> if a ServletContextTool is not thread-safe and can be mutated by templates,
> then the mistake is not in allowing it to be placed in the application
> scope. we have no culpability there. the mistakes are wholly on the part
> of the developer who should neither have created such a tool, nor should
> have then compounded it by placing the tool in the application scope. it is
> not our responsibility (nor should it be) to override their decision and
> place it in a different scope than they desire anymore than it is our
> responsibility to rewrite their tool to be thread-safe. you are trying to
> do too much.
>
>
>>- A ViewContextTool tool has read-only instance variables and is
>> thread-safe. It is initialized with a variable from Velocity context.
>> It should be possible to put it into application scope.
>>
>
> again, it is not our job to second-guess the developers decision about the
> scope-placement of the tool. if they screw up, then they screw up.
My point is not that you allow some things that don't make sense.
I am saying that you overlook some real cases that should be allowed.
...
>>Yes, one tool I really would like to have is a context inspector that
>>shows me what tools are loaded and what variables are available.
>>Today, for template designers the Velocity context is a black hole.
>>
>
> hey that could be a really cool tool for development; i'd like to see that.
> ...but it probably shouldn't be used in a production situation. anyway,
> this tool can be written just fine as a ViewContext tool. there isn't
> anything particulary inappropriate about such a tool having access to other
> request, session, and application data (unlike the case of
> ServletContextTool, where it is highly inappropriate for them to access
> request and session data). so, i don't think there's justification for
> another interface here.
There is no reason to tie such a tool to the Servlet environment.
Such a tool is useful in any environment where Velocity is used.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] proposed change to ToolboxManager (round 3 i think)
Posted by Nathan Bubna <na...@esha.com>.
Gabe said:
> >>There are several points I don't agree here:
> >>
> >>- The init() of your ServletContextTool passes a reference to
> >>
> >>ServletContext. The init() of ViewContextTool also passes a
> >>
> >>reference to ServletContext (as part of ViewContext). Why then
> >>two interfaces? ServletContextTool is really unnecessary in
> >>this case. You could just use interface ViewContextTool instead.
> >>
> >
> > No. there is no way to get a valid ViewContext at the initialization of
> > application tools in my implementation. yes, i could put off
> > initialization until the first call to getToolboxContext, but that's not
as
> > efficient. furthermore, application tools have no business messing
around
> > with the ViewContext. it gives access to request, session, and a
velocity
> > context all of which should be strictly off-limits to application
tools.
>
>
> Now you introduce a new aspect to this discussion. You are basically
saying
> that you need to protect the tool developers from messing with the wrong
> data.
No. that's not basically what i'm saying. you took my "furthermore" reason
(which is secondary) and called it my primary one. my primary reason was
the lack of a ViewContext object at the point of initializing my application
tools. if you had bothered to look at my code, you would see that. (now to
rephrase and repeat my "furthermore"...)there is no point in putting off
initialization of the application tools until the first request just so that
we can give developers access to data they neither need nor should use for
application tools.
> I don't think we should start worring about that aspect. A tools developer
> has endless possibilities to mess things up.
hey. you stole my line! :-)
> Our primary concern should be to provide a safe API to the template
designer.
wha?! we're talking about the interfaces the tool developers will use in
designing the tools! those interfaces only indirectly concern the API given
to the template designers. it is up to the tool developers to provide a
safe API to them. that is not our job here. we can *encourage* safe tool
design and scope-placement, but it's not our job (nor our primary concern)
to ensure it.
> For the sake of simplicity I suggest that we don't distinguish between
> ServletContextTool and ViewContextTool (unless I overlook some other
> important facts).
you did. you overlooked my primary reason for the distinction and i don't
think you fully grasped my secondary reason.
> >>- Regarding the scope-behaviour outlined above: Why can
> >>ViewContextTools not be in application scope? Why can
> >>ServletContextTools not be in request or session scope?
> >>
> > i've answered the first question above. as for the second question:
> > using a ServletContextTool in request or session scope is a waste of
time.
> > i would not want to encourage it by allowing it :-). if a developer
has a
> > tool that he wants to use in both application and session/request, then
he
> > need only implement both interfaces.
>
> This is quite intransparent to me and makes things unnecessary complex.
makes perfect sense to me, and i think it's pretty simple. in any case, my
toolbox manager code was shorter and simpler than yours, so in what way is
my proposal more complex?
> On the one side, you allow the free configuration of the scope in the
> toolbox configuration file. On the other side, for certain classes
> certain scopes are implied.
yes. that's essentially the goal. developers can put the tools wherever
they want regardless of what interfaces they implement, but if they want
their tools to be initialized with appropriate data, they must implement the
appropriate interface.
> In my view a ServletContextTool tool may very well live in request of
session
> scope and a ViewContextTool may live in application scope. Think about the
> following two cases:
>
> - A ServletContextTool tool has read-write instance variables and is
therefore
> not thread safe. It cannot go to application scope.
if a ServletContextTool is not thread-safe and can be mutated by templates,
then the mistake is not in allowing it to be placed in the application
scope. we have no culpability there. the mistakes are wholly on the part
of the developer who should neither have created such a tool, nor should
have then compounded it by placing the tool in the application scope. it is
not our responsibility (nor should it be) to override their decision and
place it in a different scope than they desire anymore than it is our
responsibility to rewrite their tool to be thread-safe. you are trying to
do too much.
> - A ViewContextTool tool has read-only instance variables and is
thread-safe.
> It is initialized with a variable from Velocity context. It should really
go
> to application scope.
again, it is not our job to second-guess the developers decision about the
scope-placement of the tool. if they screw up, then they screw up.
> > can you give a
> > better example of a tool which should have access to the velocity
context
> > only?
>
> Yes, one tool I really would like to have is a context inspector that
> shows me what tools are loaded and what variables are available.
> Today, for template designers the Velocity context is a black hole.
hey that could be a really cool tool for development; i'd like to see that.
...but it probably shouldn't be used in a production situation. anyway,
this tool can be written just fine as a ViewContext tool. there isn't
anything particulary inappropriate about such a tool having access to other
request, session, and application data (unlike the case of
ServletContextTool, where it is highly inappropriate for them to access
request and session data). so, i don't think there's justification for
another interface here.
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] proposed change to ToolboxManager (round 3 i think)
Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:
> From: "Gabriel Sidler" <si...@teamup.ch>
...
>>(BTW, I regards the 'view' package
>>not a servlet-specific but as the place for view renderers
>>for different environment.)
>>
>
> wha?! as far as i can tell, the 'view' package is all about the servlet
> environment. at least everthing in there to date sure is. i think you're
> wrong to take this view. (heh. no pun was intended there when i typed it,
> but what the hey :-)
The tools support is useful for non-servlet environments as well. I see
the servlet environment as the first environment that we focus on. That's
why this may seem all about servlets right now. Actually, most of
package view/tools is not servlet-specific. I don't suggest that we
go out of our way to make things generic but simply pay attention to
the possiblity that other environments might want to profit from the tools
support in the future.
I personally hope that DVSL gets to profit from the tools support in the
future.
>>There are several points I don't agree here:
>>
>>- The init() of your ServletContextTool passes a reference to
>>
>>ServletContext. The init() of ViewContextTool also passes a
>>
>>reference to ServletContext (as part of ViewContext). Why then
>>two interfaces? ServletContextTool is really unnecessary in
>>this case. You could just use interface ViewContextTool instead.
>>
>
> No. there is no way to get a valid ViewContext at the initialization of
> application tools in my implementation. yes, i could put off
> initialization until the first call to getToolboxContext, but that's not as
> efficient. furthermore, application tools have no business messing around
> with the ViewContext. it gives access to request, session, and a velocity
> context all of which should be strictly off-limits to application tools.
Now you introduce a new aspect to this discussion. You are basically saying
that you need to protect the tool developers from messing with the wrong
data.
I don't think we should start worring about that aspect. A tools developer
has endless possibilities to mess things up.
Our primary concern should be to provide a safe API to the template designer.
For the sake of simplicity I suggest that we don't distinguish between
ServletContextTool and ViewContextTool (unless I overlook some other
important facts).
>>- Regarding the scope-behaviour outlined above: Why can
>>ViewContextTools not be in application scope? Why can
>>ServletContextTools not be in request or session scope?
>>
>
> i've answered the first question above. as for the second question:
> using a ServletContextTool in request or session scope is a waste of time.
> i would not want to encourage it by allowing it :-). if a developer has a
> tool that he wants to use in both application and session/request, then he
> need only implement both interfaces.
This is quite intransparent to me and makes things unnecessary complex.
On the one side, you allow the free configuration of the scope in the
toolbox configuration file. On the other side, for certain classes
certain scopes are implied.
In my view a ServletContextTool tool may very well live in request of session
scope and a ViewContextTool may live in application scope. Think about the
following two cases:
- A ServletContextTool tool has read-write instance variables and is therefore
not thread safe. It cannot go to application scope.
- A ViewContextTool tool has read-only instance variables and is thread-safe.
It is initialized with a variable from Velocity context. It should really go
to application scope.
To keep things simple, I suggest that we treat 'scope' as a tool attribute
that is orthogonal to other attributes.
>>- Why drop ContextContextTool. The name may not be optimal
>>but what is does is useful. It passes a context that is not
>>coupled to the Servlet environment (like ViewContext is). The
>>ToolLoader tool uses this, for example. There is not reason
>>to couple the ToolLoader to the servlet environment an thereby
>>preventing it from being used in another environment.
>>
>
> hmm. the name is not simply non-optimal. it is atrocious! really. i mean
> that. i almost gagged on my morning snack when i saw that. :-)
Ok, ok, I promise that we will find a better name for this! A bad
name shouldn't be the reason to throw out a good tool. :-)
> on a more
> serious note, you have a point that ToolLoader need not be coupled to the
> servlet environment. however, i don't like ToolLoader. i think it should
> never be used. furthermore,
>
> class ToolLoader
> {
> public Object load(String classname) throws Exception
> {
> return Class.forName(classname).newInstance();
> }
> }
>
> #set( $myloadedtool = $toolloader.load("com.foo.MyLoadedTool") )
>
> is how i think it should be used and written (if at all). there is no good
> reason to instantiate (or pool) instances for each request.
That's a very good suggestion. I will take this up.
> can you give a
> better example of a tool which should have access to the velocity context
> only?
Yes, one tool I really would like to have is a context inspector that
shows me what tools are loaded and what variables are available.
Today, for template designers the Velocity context is a black hole.
...
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] proposed change to ToolboxManager (round 3 i think)
Posted by Nathan Bubna <na...@esha.com>.
----- Original Message -----
From: "Gabriel Sidler" <si...@teamup.ch>
To: "Velocity Developers List" <ve...@jakarta.apache.org>
Sent: Wednesday, April 03, 2002 2:13 PM
Subject: Re: [VelTools] proposed change to ToolboxManager (round 3 i think)
Gabe said:
> > you may have said this before but could you remind me, what was it that
was
> > wrong with simply "ToolboxManager"?)
>
>
> Well, the current toolbox manager is tied to the Servlet
> environment. I intend to implement toolbox managers for
> other environments. The name ToolboxManager should be
> kept free for code that is common to all of the toolbox
> manager implementations.
sure. that works for me.
> (BTW, I regards the 'view' package
> not a servlet-specific but as the place for view renderers
> for different environment.)
wha?! as far as i can tell, the 'view' package is all about the servlet
environment. at least everthing in there to date sure is. i think you're
wrong to take this view. (heh. no pun was intended there when i typed it,
but what the hey :-)
> On the other hand, a configurable life cycle has its value. For
> example, if we work with tools that we cannot change.
>
> Overall I come to the conclusion that the approach is reasonable
> and I can support it.
glad to hear it. i'd rather not have to roll my own code for this.
> There are several points I don't agree here:
>
> - The init() of your ServletContextTool passes a reference to
>
> ServletContext. The init() of ViewContextTool also passes a
>
> reference to ServletContext (as part of ViewContext). Why then
> two interfaces? ServletContextTool is really unnecessary in
> this case. You could just use interface ViewContextTool instead.
No. there is no way to get a valid ViewContext at the initialization of
application tools in my implementation. yes, i could put off
initialization until the first call to getToolboxContext, but that's not as
efficient. furthermore, application tools have no business messing around
with the ViewContext. it gives access to request, session, and a velocity
context all of which should be strictly off-limits to application tools.
> - Regarding the scope-behaviour outlined above: Why can
> ViewContextTools not be in application scope? Why can
> ServletContextTools not be in request or session scope?
i've answered the first question above. as for the second question:
using a ServletContextTool in request or session scope is a waste of time.
i would not want to encourage it by allowing it :-). if a developer has a
tool that he wants to use in both application and session/request, then he
need only implement both interfaces.
> - Why drop ContextContextTool. The name may not be optimal
> but what is does is useful. It passes a context that is not
> coupled to the Servlet environment (like ViewContext is). The
> ToolLoader tool uses this, for example. There is not reason
> to couple the ToolLoader to the servlet environment an thereby
> preventing it from being used in another environment.
hmm. the name is not simply non-optimal. it is atrocious! really. i mean
that. i almost gagged on my morning snack when i saw that. :-) on a more
serious note, you have a point that ToolLoader need not be coupled to the
servlet environment. however, i don't like ToolLoader. i think it should
never be used. furthermore,
class ToolLoader
{
public Object load(String classname) throws Exception
{
return Class.forName(classname).newInstance();
}
}
#set( $myloadedtool = $toolloader.load("com.foo.MyLoadedTool") )
is how i think it should be used and written (if at all). there is no good
reason to instantiate (or pool) instances for each request. can you give a
better example of a tool which should have access to the velocity context
only?
> - Why drop LogEnabledContextTool? I need this for the Struts stuff. We can
> always change. Having a first approach is better than nothing.
> The current approach is not that unreasonable.
i never said anything about dropping it. i think we need logging too. i'm
just not sure how best to do that, so I'm ignoring it for the time being.
hmm, i thought i said that. oh well, i meant to say it. does that count?
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] proposed change to ToolboxManager (round 3 i think)
Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:
> howdy folks,
>
> well, seeing the changes Gabe committed, i feel rather inclined to revise my
> earlier toolbox manager proposal and encourage, exhort, and otherwise
> suggest that you consider it in place of the present implementation. after
> all, as Gabe said, nothing is "set in stone" yet. :-) so...
>
> attached are the two interfaces i proposed as well as a revamped version of
> my previous manager that handles my proposed interfaces appropriately. there
> are also a few other changes to the manager taken from Gabe's
> ServletToolboxManager. (btw, i'm not completely sold on that name. Gabe,
> you may have said this before but could you remind me, what was it that was
> wrong with simply "ToolboxManager"?)
Well, the current toolbox manager is tied to the Servlet
environment. I intend to implement toolbox managers for
other environments. The name ToolboxManager should be
kept free for code that is common to all of the toolbox
manager implementations. (BTW, I regards the 'view' package
not a servlet-specific but as the place for view renderers
for different environment.)
>
> for those who'd rather not read the java, here's the basic ideas of this
> implementation.
>
> --scope-placement (lifecycle) of tools is completely determined by the
> config file. tools that do not have a scope denoted in the config are
> placed as 'request' by default.
I claim that in 99% of cases the life cycle of the tool is given
by its design. So, this approach is asking the developer to
configure something that has been fixed by the tool designer.
This introduces the potential for unnecessary misconfiguration.
Not very cool.
On the other hand, a configurable life cycle has its value. For
example, if we work with tools that we cannot change.
Overall I come to the conclusion that the approach is reasonable
and I can support it.
> --scope-behaviour (initialization) of tools is defined by interfaces in
> conjunction with the scope-placement. specifically:
>
> --application scope tools
> --all are instantiated only once (during loading)
> --those that implement ServletContextTool are initialized using the
> ServletContext
> --the instantiated and initialized tools are held in the manager
> --session scope tools
> --all are instantiated once per session
> --those that implement ViewContextTool are initialized using the
> ViewContext
> --the instantiated and initialized tools are held in the session
> --request scope tools
> --all are instantiated on each request
> --those that implement ViewContextTool are initialized using the
> ViewContext
There are several points I don't agree here:
- The init() of your ServletContextTool passes a reference to
ServletContext. The init() of ViewContextTool also passes a
reference to ServletContext (as part of ViewContext). Why then
two interfaces? ServletContextTool is really unnecessary in
this case. You could just use interface ViewContextTool instead.
- Regarding the scope-behaviour outlined above: Why can
ViewContextTools not be in application scope? Why can
ServletContextTools not be in request or session scope?
- Why drop ContextContextTool. The name may not be optimal
but what is does is useful. It passes a context that is not
coupled to the Servlet environment (like ViewContext is). The
ToolLoader tool uses this, for example. There is not reason
to couple the ToolLoader to the servlet environment an thereby
preventing it from being used in another environment.
- Why drop LogEnabledContextTool? I need this for the Struts stuff. We can
always change. Having a first approach is better than nothing.
The current approach is not that unreasonable.
My proposal is to keep the interfaces
- ServletContextTool (remove all the scope-related stuff)
- ContextContextTool (as is, but with possibly a better name)
- LogEnabledContextTool (as is for now)
ThreadSafeContextTool would be dropped.
Gabe
> that's pretty much the important stuff.
> if i may be allowed a little plug... i think you'll like this
> implementation--it's efficient, easy to configure and write tools for, and
> it handles variable-scope tools nicely. my one concern is that i may at
> some point see need to initialize session and request tools differently. in
> that case, it would be necessary to add a third interface to handle session
> tools differently from request tools. anyhow, that's an easy change to make
> should the need arise.
>
> Nathan Bubna
> nathan@esha.com
>
>
> ------------------------------------------------------------------------
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
--
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] proposed change to ToolboxManager (round 3 i think)
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Nathan Bubna" <na...@esha.com> writes:
> --scope-placement (lifecycle) of tools is completely determined by the
> config file. tools that do not have a scope denoted in the config are
> placed as 'request' by default.
>
> --scope-behaviour (initialization) of tools is defined by interfaces in
> conjunction with the scope-placement. specifically:
>
> --application scope tools
> --all are instantiated only once (during loading)
> --those that implement ServletContextTool are initialized using the
> ServletContext
> --the instantiated and initialized tools are held in the manager
> --session scope tools
> --all are instantiated once per session
> --those that implement ViewContextTool are initialized using the
> ViewContext
> --the instantiated and initialized tools are held in the session
> --request scope tools
> --all are instantiated on each request
> --those that implement ViewContextTool are initialized using the
> ViewContext
I like this.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
[VelTools] proposed change to ToolboxManager (round 3 i think)
Posted by Nathan Bubna <na...@esha.com>.
howdy folks,
well, seeing the changes Gabe committed, i feel rather inclined to revise my
earlier toolbox manager proposal and encourage, exhort, and otherwise
suggest that you consider it in place of the present implementation. after
all, as Gabe said, nothing is "set in stone" yet. :-) so...
attached are the two interfaces i proposed as well as a revamped version of
my previous manager that handles my proposed interfaces appropriately. there
are also a few other changes to the manager taken from Gabe's
ServletToolboxManager. (btw, i'm not completely sold on that name. Gabe,
you may have said this before but could you remind me, what was it that was
wrong with simply "ToolboxManager"?)
for those who'd rather not read the java, here's the basic ideas of this
implementation.
--scope-placement (lifecycle) of tools is completely determined by the
config file. tools that do not have a scope denoted in the config are
placed as 'request' by default.
--scope-behaviour (initialization) of tools is defined by interfaces in
conjunction with the scope-placement. specifically:
--application scope tools
--all are instantiated only once (during loading)
--those that implement ServletContextTool are initialized using the
ServletContext
--the instantiated and initialized tools are held in the manager
--session scope tools
--all are instantiated once per session
--those that implement ViewContextTool are initialized using the
ViewContext
--the instantiated and initialized tools are held in the session
--request scope tools
--all are instantiated on each request
--those that implement ViewContextTool are initialized using the
ViewContext
that's pretty much the important stuff.
if i may be allowed a little plug... i think you'll like this
implementation--it's efficient, easy to configure and write tools for, and
it handles variable-scope tools nicely. my one concern is that i may at
some point see need to initialize session and request tools differently. in
that case, it would be necessary to add a third interface to handle session
tools differently from request tools. anyhow, that's an easy change to make
should the need arise.
Nathan Bubna
nathan@esha.com
Re: [VelTools] Changes
Posted by Nathan Bubna <na...@esha.com>.
Daniel said:
> ...I would much rather see the marker
> interfaces pattern used to denote tool scoping (i.e. request, session,
> global).
i think our thoughts are somewhat close on this one, but because some tools
may be usable in more than one scope, i do not want to rely on interfaces
for denoting scope. i do, however, want multiple interfaces to allow tools
of various scopes to be initialized in an appropriate manner. this way
developers can control placement of tools by the config file and, if they
implement the correct interface, have them initialized appropriately. this
will allow a variable-scope tool to implement multiple context tool/scope
interfaces and then function properly according to the scope as denoted by
the config file. if we rely on interfaces to denote scope, such behaviour
becomes very difficult to implement. more on this below...
> >> I much prefer the old ContexTool interface.
> >
> > i don't really have any gripes with the old 'destroy' method. i don't
> > really see how it is needed or used, but that's just me. perhaps you
have
> > good use for it. i do, however, take issue with the 'init' method. the
> > issue being that it does not functions as a factory method, not an
> > initialization method. thus i prefer the name 'getInstance'. do you
have
> > another suggestion?
>
> Yes, I do. Keep the context tool lifecycle interface separate from
> the factory interface. Using a single method of the context tool
> interface to mix in both an initialization and factory implementation
> is confusing, and introduces tight coupling. Clearly delineate
> between the two distinct interfaces by supplying a separate manager or
> factory entity.
ah! you're right. i knew there was still something bugging me about
ContextTool--the meshing of factory/init behaviour. Really, as i look at
it, we don't really need the factory behaviour here. i think it will be
fine for the manager to create new instances using the class, and simply use
the interfaces for initialization. so here's my (strong) suggestion...
//this is not to be confused with the current
//(and IMO poorly named) ServletContextTool
public interface ServletContextTool {
void init(ServletContext app);
}
and
public interface ViewContextTool {
void init(ViewContext ctx);
}
using these, the toolbox manager can check if application tools implement
ServletContextTool and initialize new instances appropriately, and use
ViewContextTool similarly for session and request tools. at this point i do
not see any point in having separate interfaces for session and request
tools, passing the ViewContext gives them both the access to the current
request and session as should be.
i think these should be quite sufficient. so, i would like
ContextContextTool, the current ServletContextTool, and
ThreadSafeContextTool to go the way of the dinosaur. i'm ignoring the
logging issue until i look into it and can justify my having an opinion.
also, i actually have most of this implemented already, so i can send code
if you like.
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Nathan Bubna" <na...@esha.com> writes:
> Daniel said:
> > >> - ThreadSafeContextTool: for thread-safe context tool, tool
>> >> instance is reused for the entire runtime
>> >
>> > an empty interface used only to mark a type of tool?
> <strong>-1</strong> i
> > > see no good reason for this. i think this is useless and clutters up
> the
> > > API.
>>
>> Using an interface as a marker for instanceof operations is actually a
>> standard pattern in both Java and C++. Personally, I work under the
>> assumption that no request-scoped tool should be assumed thread-safe,
>> and that global-scoped tools are always thread-safe (I don't tend to
>> use session-scoped tools).
>
> being a standard pattern doesn't mean i have to like it. :-) though it may
> well be a legit practice, i think it's one that should be constrained to
> necessity. i think this is a frivolous usage.
I lean towards your thoughts here. I would much rather see the marker
interfaces pattern used to denote tool scoping (i.e. request, session,
global).
>> I much prefer the old ContexTool interface.
>
> i don't really have any gripes with the old 'destroy' method. i don't
> really see how it is needed or used, but that's just me. perhaps you have
> good use for it. i do, however, take issue with the 'init' method. the
> issue being that it does not functions as a factory method, not an
> initialization method. thus i prefer the name 'getInstance'. do you have
> another suggestion?
Yes, I do. Keep the context tool lifecycle interface separate from
the factory interface. Using a single method of the context tool
interface to mix in both an initialization and factory implementation
is confusing, and introduces tight coupling. Clearly delineate
between the two distinct interfaces by supplying a separate manager or
factory entity.
- Dan
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Nathan Bubna <na...@esha.com>.
Daniel said:
> >> - ThreadSafeContextTool: for thread-safe context tool, tool
> >> instance is reused for the entire runtime
> >
> > an empty interface used only to mark a type of tool?
<strong>-1</strong> i
> > see no good reason for this. i think this is useless and clutters up
the
> > API.
>
> Using an interface as a marker for instanceof operations is actually a
> standard pattern in both Java and C++. Personally, I work under the
> assumption that no request-scoped tool should be assumed thread-safe,
> and that global-scoped tools are always thread-safe (I don't tend to
> use session-scoped tools).
being a standard pattern doesn't mean i have to like it. :-) though it may
well be a legit practice, i think it's one that should be constrained to
necessity. i think this is a frivolous usage.
> I much prefer the old ContexTool interface.
i don't really have any gripes with the old 'destroy' method. i don't
really see how it is needed or used, but that's just me. perhaps you have
good use for it. i do, however, take issue with the 'init' method. the
issue being that it does not functions as a factory method, not an
initialization method. thus i prefer the name 'getInstance'. do you have
another suggestion?
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Nathan Bubna" <na...@esha.com> writes:
> Gabe said:
> > I've just checked in a number of changes to the jakarta-velocity-tools
>> project.
>
> i concur with Geir and Daniel. it's a pain to see all the changes in one
> big commit.
>
>> - Introduced four interfaces that establish contracts between
>> context tools and compatible toolbox managers:
>
> ugh. don't take this personally, but i really don't like this. when i
> suggested you write up some code, i didn't think you'd go committing all
> these changes without getting feedback first. anyway, many of my complaints
> are below. most others i have already voiced in previous emails.
The previous single ContextTool interface provided an elegant
simplicity to the API.
>> - ThreadSafeContextTool: for thread-safe context tool, tool
>> instance is reused for the entire runtime
>
> an empty interface used only to mark a type of tool? <strong>-1</strong> i
> see no good reason for this. i think this is useless and clutters up the
> API.
Using an interface as a marker for instanceof operations is actually a
standard pattern in both Java and C++. Personally, I work under the
assumption that no request-scoped tool should be assumed thread-safe,
and that global-scoped tools are always thread-safe (I don't tend to
use session-scoped tools).
>> - LogEnabledContextTool: for context tools that need access
>> to a logging facility
>
> not a fan of the name, and i thought we were going to hold off on
> discussing/implementing logging until some other things were settled.
I don't like the fact that it offers little interoperability in the
standard implementation.
> -1 on getDefaultLifeCycle()
> sorry, i don't see a real need for this to be hard-coded. why do you think
> this is necessary? and if not necessary, why clutter up the API?
I definitely see a need for each tool to declare the scope of its
usability, but I'm not sure I see a need for it to be hard-coded into
the tool. If anything, marker interfaces would be the way to go here.
>> - ContextContextTool: for context tools that need access to
>> the Velocity context
>
> Aaah! i'm gonna have nightmares about this! :-) seriously, that is an
> atrocious name! i would prefer calling it
> ContextToolsThatWouldLikeToBeGivenTheVelocityContextAndThenPlacedTherein.jav
> a :-P
> i'm kidding of course, what i'd really like is for it to not exist. after
> all, tools that implement ServletContextTool (formerly ContextTool) will
> have access to the velocity context by way of the ViewContext. what's your
> justification for having this extra interface? also, i thought when you
> said you wanted to use interfaces to identify context tools that you would
> be creating separate interfaces for each scope. i can sorta see reason for
> that, but this just baffles me.
The ContextContextTool name displeases me as well.
>> These interfaces are all optional. Any object with public methods
>> and a public, empty constructor can be used as a context tool.
>> Its handling may not be the most optimal one, though.
>
> sorry, but i highly doubt i will ever use these interfaces (if at all
> possible).
I much prefer the old ContexTool interface.
BTW, I'm not trying to be negative here -- I just didn't comment on
the things I completely agree with (Nathan and Geir already took care
of that for me).
- Dan
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:
> Gabe said:
>
>>I've just checked in a number of changes to the jakarta-velocity-tools
>>project.
>>
>
> i concur with Geir and Daniel. it's a pain to see all the changes in one
> big commit.
Sorry for that, I will commit more incrementally in the future.
Thanks for the feedback to all. I can assure you that I have
pondered back and forth most of the points you have mentioned.
There are a few common themes that seem to raise concern. I'll
try to address them in the following mails.
My personal ambition is to get to an alpha release for the
Velocity/Struts tools soon. The API of these tools seems to
be pretty stable now. Most open issues don't impact the
public API of these tools. We need some exposure to real
Struts users to push this ahead.
>>- Introduced four interfaces that establish contracts between
>> context tools and compatible toolbox managers:
>
> ugh. don't take this personally, but i really don't like this. when i
> suggested you write up some code, i didn't think you'd go committing all
> these changes without getting feedback first.
I don't take it personally! I just felt that with all the changes the
easiest way to exchange code is via CVS. Nothing is cast in stone,
anything can be changed.
More to follow...
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Nathan Bubna <na...@esha.com>.
Gabe said:
> I've just checked in a number of changes to the jakarta-velocity-tools
> project.
i concur with Geir and Daniel. it's a pain to see all the changes in one
big commit.
> - Introduced four interfaces that establish contracts between
> context tools and compatible toolbox managers:
ugh. don't take this personally, but i really don't like this. when i
suggested you write up some code, i didn't think you'd go committing all
these changes without getting feedback first. anyway, many of my complaints
are below. most others i have already voiced in previous emails.
> - ThreadSafeContextTool: for thread-safe context tool, tool
> instance is reused for the entire runtime
an empty interface used only to mark a type of tool? <strong>-1</strong> i
see no good reason for this. i think this is useless and clutters up the
API.
> - LogEnabledContextTool: for context tools that need access
> to a logging facility
not a fan of the name, and i thought we were going to hold off on
discussing/implementing logging until some other things were settled.
> - ServletContextTool: for context tools that need access to
> the servlet envrionment
-1 on the name. to me it implies javax.servlet.ServletContext and that's
not appropriate. i still prefer ViewContextTool.
-1 on getDefaultLifeCycle()
sorry, i don't see a real need for this to be hard-coded. why do you think
this is necessary? and if not necessary, why clutter up the API?
> - ContextContextTool: for context tools that need access to
> the Velocity context
Aaah! i'm gonna have nightmares about this! :-) seriously, that is an
atrocious name! i would prefer calling it
ContextToolsThatWouldLikeToBeGivenTheVelocityContextAndThenPlacedTherein.jav
a :-P
i'm kidding of course, what i'd really like is for it to not exist. after
all, tools that implement ServletContextTool (formerly ContextTool) will
have access to the velocity context by way of the ViewContext. what's your
justification for having this extra interface? also, i thought when you
said you wanted to use interfaces to identify context tools that you would
be creating separate interfaces for each scope. i can sorta see reason for
that, but this just baffles me.
> These interfaces are all optional. Any object with public methods
> and a public, empty constructor can be used as a context tool.
> Its handling may not be the most optimal one, though.
sorry, but i highly doubt i will ever use these interfaces (if at all
possible).
> - Modified VelocityViewServlet so that it now can be configured
> with a configuration file. Proposed location is
> WEB-INF/velocity.properties. This allows now the use of a global
> macros file (default location WEB-INF/VM_global_library.vm)
sweet! this is a very good thing. i was just thinking yesterday that i
should take a whack at this if no one else does soon. thanks for doing it!!
> - Enhanced ToolboxManager
> - Made initialization of context tools thread-safe by
> synchronizing where needed.
hmm. i'm not thrilled with your implementation, but i'll try to put off
hacking at it until we've discussed some of the issues above. i do see that
application scope tools should have some hook whereby they *may* be given
access to the ServletContext on initialization. that is one change i would
like to keep, albeit implemented in a different manner.
> - Tools of unknown classes have now by default a life cycle
> of 'request' (based on Bill's input). This is required
> because otherwise we might run into problem with tools that
> are not thread-safe.
+1 Bill is right, and it shouldn't kill us to create a new instance on each
request.
> - Globally replaced the term 'scope' by the term 'life cycle'.
> This reflects more accurately what it is.
eh... not sure i like that. as far as i'm aware, 'scope' is pretty much
the standard term for request/session/application matters. i'd rather use
the term developers are more likely to be familiar with.
> - Setup a library of context tools and corresponding documentation.
> Named it VelLibrary. Added Nathan's contributed context tools to
> the VelLibrary and modified them to make them thread-safe.
huh? i'm pretty sure that my tools (DateTool, MathTool, ParameterParsers)
already were thread-safe. what needed changing?
> Included in VelLibray are now four tools: MathTool, DateTool,
> RequestParser and ToolLoader. Added all tools to package
> o.a.v.tools.tools. This is not commited yet.
ugh. o.a.v.tools.tools? why the tools.tools? can we avoid this
redundancy?
Nathan Bubna
nathan@esha.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Daniel Rall wrote:
> Gabriel Sidler <si...@teamup.ch> writes:
>
>
>>Daniel Rall wrote:
>>
>>
>>>>I have no problem changing the names to best reflect what the method
>>>>does. We changed to getInstance() because most tools return new
>>>>instances from that method.
>>>>
>>>>
>>>Yeah, I completely disagree with the strategy of using that method to
>>>perform two distinct operations. See my message re: initialization
>>>vs. factory methods.
>>>
>>Saw that message but don't quite see the problem. getInstance() simply
>>returns an initialized instance. How would you split the functionality?
>>
>
> Move the factory functionality into another class whose sole job it is
> to hand out instances of the context tool. Using an instance method
> of the target object of a factory method doesn't make a whole lot of
> sense, since you already need an instance of said object type to use
> the factory method.
>
> For instance:
>
> MyTool factory = new MyTool();
> MyTool tool = (MyTool) factory.getInstance(context);
>
> I had to create an instance of MyTool using its ctor just to create an
> instance of it using getInstance(). Much more clear and efficient
> would be to use a separate factory, decoupling the thing being created
> from the thing creating:
>
> MyTool tool = (MyTool) ContextToolFactory.getInstance(MyTool.class, context);
>
> ContextToolFactory's getInstance() method would create a new instance
> of the MyTool class using its no-args constructor (invoking it via
> reflection), initialize the instance by calling init(context), and
> return it to the caller. To improve efficiency, either the ctor's
> Method object would be cached (keyed by MyTool's Class object), or the
> new instance would be clone()'d . I prefer the former option, as
> Velocity itself has proven that works very, very well. getInstance()
> is also a place where context tool pooling could easily be introduced.
> ContextToolFactory might also have a overload taking String instead of
> Class for use loading context tools out of a properties file (in a
> manner similiar to the way ToolLoader can load them from VTL). Etc.
Ok, I see your points and it makes all sense to me. The two advantages
I see for a class having its own factory method are these:
- The class itself "knows" best how to efficiently obtain a reference.
A tread-safe tool can just return the always same reference to itself.
Other tools would recreate an instance on every request.
- getInstance() is a thread-safe way to return an instance. The
approach with the init() method adds a method that is not thread-
safe. (sorry for repeatedly sticking to the thread-safe issues, but
in a multi-threaded envrionment it's an issue)
To some degree the current ServletToolboxManager has the role of a
ContextToolFactory as described above. For example, tool instances
that are used as factories are cached and reused.
...
>>I did some tests. Auto-loading the four struts tools (all request
>>scope) reduce template throughtput by about 5 - 10%. That's not
>>a problem but there is a clear impact. All I am saying is that
>>where reasonably possible we should strive to make tools thread-safe.
>>
>
> Right, but I wouldn't recommend doing it at the expense of the
> interface. I'm curious, how did you do your "auto-loading"?
Using the current implementation of ServletToolboxManager and
the Velocity/Struts example application (see CVS)
...
> I'm beginning to see the disconnect -- I like the tools to be
> cachable, not sharable, because thread-safety generally adds
> parameters to the API (not especially what I want for code called from
> VTL).
That's a good observation. Being thread-safe typically means to
avoid read-write instance variables. Instead, state is passed
as method parameters.
What happens oftentimes in real implementations of tools though,
is that people add two set of methods. One set works with the
instance variables and other set works with parameters that
are passed in. As a result, the thread-safe version of the tool
has the simpler API.
> I think about the top level interfaces with that in mind,
> whereas you are focussing on making the tools thread-safe.
Well, I would say it this way: I like to have a very clear
understanding of the thread-safety of tools. We have seen now
two examples of where a little clever engineering helped to
turn tools thread-safe with little or no negative impact on
the API. One example is your MultiViewsTools, the other example
is the ToolLoader.
There are certainly many tools where thread-safety doesn't
make sense. For example tools that have lots of mutatable
state. All four Struts tools are by design not thread-safe.
I agree with you that reuse shouldn't come at the price of
reduced userfriendlyness.
> Looking at things from the perspective that every context tool should
> be thread-safe, I don't see a lot of value in init/destroy (in fact, I
> fear them because they ruin the thread-safety of my interface if
> called while the object is in use), methods which are most useful for
> request-scoped objects. Looking at things from the easy-interface
> philosophy of request-scoped tools, I still find the init/destroy
> methods invaluable. An interface for a context tool designed to be
> per-request would include init/destroy and (perhaps) extend the
> interface used for a global/application context tool.
Well, looks like we have two alternatives:
o The getInstance() / destroy() approach (currently implemented):
A tool has a factory method getInstance() that is responsible for
returning instances.
- Facilitates thread-safety and reuse of tools
- Gives tools control over how to obtain instances
- Tool pooling is not possible because there is no way to
re-initialize a tool
o The init() / destroy() approach:
A global tool factory object creates tools and initializes them.
It may pool/cache tool instances for efficiency.
- init() is inherently not thread-safe, therefore tools are
strictly speaking not thread-safe
- approach facilitates the use of tool pooling
- tool implementation is somewhat simpler to than with the
first approach
I slowly get conviced that the init()/destroy() approach is the
way to go because it's more flexible and shifts complexity from
the tool to the toolbox manager. The price is that tools are not
thread-safe because of the init() method. But, it's probably
not unrealistic to make the assumption that template designers
don't call the init() method. Under this assumption we can reuse
tools and/or cache tools. For a first implementation I would forget
about caching tools, though.
> I can move MultiViewsTool to the tools package, but to me it really
> seems to be part of the servlet view mechanism, as it's clearly
> focused on HTTP content negotiation. The designation for the view
> package seems rather arbitrary, is there a description/email
> thread/etc. on why it is this way?
The distribution has a README that gives a brief overview of the
three packages.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Gabriel Sidler <si...@teamup.ch> writes:
> Daniel Rall wrote:
>
>>>I have no problem changing the names to best reflect what the method
>>>does. We changed to getInstance() because most tools return new
>>>instances from that method.
>>>
>> Yeah, I completely disagree with the strategy of using that method to
>> perform two distinct operations. See my message re: initialization
>> vs. factory methods.
>
> Saw that message but don't quite see the problem. getInstance() simply
> returns an initialized instance. How would you split the functionality?
Move the factory functionality into another class whose sole job it is
to hand out instances of the context tool. Using an instance method
of the target object of a factory method doesn't make a whole lot of
sense, since you already need an instance of said object type to use
the factory method.
For instance:
MyTool factory = new MyTool();
MyTool tool = (MyTool) factory.getInstance(context);
I had to create an instance of MyTool using its ctor just to create an
instance of it using getInstance(). Much more clear and efficient
would be to use a separate factory, decoupling the thing being created
from the thing creating:
MyTool tool = (MyTool) ContextToolFactory.getInstance(MyTool.class, context);
ContextToolFactory's getInstance() method would create a new instance
of the MyTool class using its no-args constructor (invoking it via
reflection), initialize the instance by calling init(context), and
return it to the caller. To improve efficiency, either the ctor's
Method object would be cached (keyed by MyTool's Class object), or the
new instance would be clone()'d . I prefer the former option, as
Velocity itself has proven that works very, very well. getInstance()
is also a place where context tool pooling could easily be introduced.
ContextToolFactory might also have a overload taking String instead of
Class for use loading context tools out of a properties file (in a
manner similiar to the way ToolLoader can load them from VTL). Etc.
> Would you please have a look at some of the Struts tools, for example
> the LinkTool. Do you see the problem there, too? What would you change?
Mainly what I state above, the location of the factory method. Why
does the code for object instantiation and initialization (via the
ctor) need to be in every tool instead of in a single factory class
which uses the context tool interface to properly initialize a --
possibly already pooled -- object?
>>>My concern is this: We are talking here about auto-loading context
>>>tools. destroy() would have to be called after *every* request for
>>>*every* tool that has a life cycle of 'request'. That's a lot of
>>>methods calls that likely don't do anything in most cases.
>>>
>> Do you see that as a bottleneck of some sort? Compared to the
>> overhead of the rest of the Velocity View apparatus, it's less than
>> nothing. Premature optimization is the root of all evil.
>
> I did some tests. Auto-loading the four struts tools (all request
> scope) reduce template throughtput by about 5 - 10%. That's not
> a problem but there is a clear impact. All I am saying is that
> where reasonably possible we should strive to make tools thread-safe.
Right, but I wouldn't recommend doing it at the expense of the
interface. I'm curious, how did you do your "auto-loading"?
[snip]
>> This change would make it impossible to instantiate from outside its
>> package (even using getInstance(), since it itself is an instance
>> method and you'd have a chicken and egg problem on your hands).
>
> You must keep the public empty constructor. Have a look
> at the attached code for what I mean.
Though it won't compile, it again causes me to question the value of a
factory mechanism in the API of a class which is supposed to be a
context tool. :-)
However, you have convinced me that this context tool is better as an
application-scoped tool.
>> The initialization activites that you suggest moving to the ctor are
>> better done at object initialization time than instantiation time, as
>> the application's configuration may differ between those two points in
>> time (thus the usefulness of the init() interface for initialization).
>
>> Never the less, I've taken your point to heart and applied changes
>> necessary to make the class thread-safe.
>
> Your version is still not thread-safe. init() mutates the instance
> variable defaultLanguage. Synchronization doesn't help here. My
> version solves this.
>
> We used the same pattern for most other tools so far.
You're right, it's not thread-safe if init() is called during normal
operation (which in my use case it would not be).
I'm beginning to see the disconnect -- I like the tools to be
cachable, not sharable, because thread-safety generally adds
parameters to the API (not especially what I want for code called from
VTL). I think about the top level interfaces with that in mind,
whereas you are focussing on making the tools thread-safe.
Looking at things from the perspective that every context tool should
be thread-safe, I don't see a lot of value in init/destroy (in fact, I
fear them because they ruin the thread-safety of my interface if
called while the object is in use), methods which are most useful for
request-scoped objects. Looking at things from the easy-interface
philosophy of request-scoped tools, I still find the init/destroy
methods invaluable. An interface for a context tool designed to be
per-request would include init/destroy and (perhaps) extend the
interface used for a global/application context tool.
>>>Finally, I suggest that you move your tool to the tools package.
>>>The rules are this:
>>>Package struts: All Struts related tools
>>>Package tools: All other tools
>>>Package view: View engines (like VSS) and supporting infrastructure
>>> but no tools.
>>>
>> So you see even View-specific tools like the MultiViewsTool content
>> negotiation tool as part ofthe Tools package? What's your reasoning
>> for wanting to keep View-specific context tools out of the View
>> package?
>
> Yes, my understanding was that tools are either in struts or
> tools package. Geir might want to confirm this.
I can move MultiViewsTool to the tools package, but to me it really
seems to be part of the servlet view mechanism, as it's clearly
focused on HTTP content negotiation. The designation for the view
package seems rather arbitrary, is there a description/email
thread/etc. on why it is this way?
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Daniel Rall wrote:
>>I have no problem changing the names to best reflect what the method
>>does. We changed to getInstance() because most tools return new
>>instances from that method.
>>
>
> Yeah, I completely disagree with the strategy of using that method to
> perform two distinct operations. See my message re: initialization
> vs. factory methods.
Saw that message but don't quite see the problem. getInstance() simply
returns an initialized instance. How would you split the functionality?
Would you please have a look at some of the Struts tools, for example
the LinkTool. Do you see the problem there, too? What would you change?
>>My concern is this: We are talking here about auto-loading context
>>tools. destroy() would have to be called after *every* request for
>>*every* tool that has a life cycle of 'request'. That's a lot of
>>methods calls that likely don't do anything in most cases.
>>
>
> Do you see that as a bottleneck of some sort? Compared to the
> overhead of the rest of the Velocity View apparatus, it's less than
> nothing. Premature optimization is the root of all evil.
I did some tests. Auto-loading the four struts tools (all request
scope) reduce template throughtput by about 5 - 10%. That's not
a problem but there is a clear impact. All I am saying is that
where reasonably possible we should strive to make tools thread-safe.
>>I'd like to propose the following improvement to your tool:
>>Replace the current init() method with the following two methods.
>>
>> /**
>> * new init()
>> */
>> public Object init(ViewContext context)
>> {
>> return new MultiViewsTool(context);
>> }
>>
>
> The initialization method should not be a factory method -- that's a
> mixing of metaphors which is confusing and leads to tight coupling.
> Please read my other message, and Nathan's response.
>
>
>> /**
>> * Internal constructor
>> */
>> protected MultiViewsTool(ViewContext context)
>> {
>> ServletContext sc = context.getServletContext();
>> defaultLanguage = (String) sc.getAttribute(DEFAULT_LANGUAGE_KEY);
>> if (defaultLanguage == null || defaultLanguage.trim().equals(""))
>> {
>> Context vc = context.getVelocityContext();
>> defaultLanguage = (String) vc.get(DEFAULT_LANGUAGE_KEY);
>> if (defaultLanguage == null || defaultLanguage.trim().equals(""))
>> {
>> // Use JVM default.
>> defaultLanguage = Locale.getDefault().getLanguage();
>> }
>> }
>> }
>>
>>This change makes your tool thread-safe and therefore the same
>>instance can be reused the entire runtime.
>>
>
> This change would make it impossible to instantiate from outside its
> package (even using getInstance(), since it itself is an instance
> method and you'd have a chicken and egg problem on your hands).
You must keep the public empty constructor. Have a look
at the attached code for what I mean.
> The initialization activites that you suggest moving to the ctor are
> better done at object initialization time than instantiation time, as
> the application's configuration may differ between those two points in
> time (thus the usefulness of the init() interface for initialization).
>
> Never the less, I've taken your point to heart and applied changes
> necessary to make the class thread-safe.
Your version is still not thread-safe. init() mutates the instance
variable defaultLanguage. Synchronization doesn't help here. My
version solves this.
We used the same pattern for most other tools so far.
>>Finally, I suggest that you move your tool to the tools package.
>>The rules are this:
>>Package struts: All Struts related tools
>>Package tools: All other tools
>>Package view: View engines (like VSS) and supporting infrastructure
>> but no tools.
>>
>
> So you see even View-specific tools like the MultiViewsTool content
> negotiation tool as part ofthe Tools package? What's your reasoning
> for wanting to keep View-specific context tools out of the View
> package?
Yes, my understanding was that tools are either in struts or
tools package. Geir might want to confirm this.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Gabriel Sidler <si...@teamup.ch> writes:
> Daniel Rall wrote:
>
>> You also changed the existing public API, removing the ContextTool
>> interface (since there's been no 1.0 release, no big deal). Just
>> after you checked in, I checked in a implementation of the ContextTool
>> interface, MultiViewsTool, which provides MultiViews-style content
>> negotiation to I18N'd resources.
>> I could switch over to your new getInstance() API, but I much
>> preferred the old API (init/destroy); the getInstance() factory method
>> doesn't seem especially appropriate as an instance method.
>
> I have no problem changing the names to best reflect what the method
> does. We changed to getInstance() because most tools return new
> instances from that method.
Yeah, I completely disagree with the strategy of using that method to
perform two distinct operations. See my message re: initialization
vs. factory methods.
>> What has replaced the destroy() method? If there is no replacement, what is
>> your justification for the removal of the functionality?
>
> I dropped this without a replacement. Is there really a need for
> a destroy()? I havn't seen one. If there is one, we can re-introduce
> it.
There sure is. See my message regarding pooling and reclamation of
context tool resources.
> My concern is this: We are talking here about auto-loading context
> tools. destroy() would have to be called after *every* request for
> *every* tool that has a life cycle of 'request'. That's a lot of
> methods calls that likely don't do anything in most cases.
Do you see that as a bottleneck of some sort? Compared to the
overhead of the rest of the Velocity View apparatus, it's less than
nothing. Premature optimization is the root of all evil.
> A more efficient way might be to define an interface
> CleanupEnabledContextTool that tools with cleanup needs have to implement.
That would be an acceptable compromise.
>> Which of the
>> new interfaces would you recommend for the MultiViewsTool?
>
> Since your tool needs access to the servlet environment, this would
> be interface ServletContextTool.
Done.
> I'd like to propose the following improvement to your tool:
> Replace the current init() method with the following two methods.
>
> /**
> * new init()
> */
> public Object init(ViewContext context)
> {
> return new MultiViewsTool(context);
> }
The initialization method should not be a factory method -- that's a
mixing of metaphors which is confusing and leads to tight coupling.
Please read my other message, and Nathan's response.
> /**
> * Internal constructor
> */
> protected MultiViewsTool(ViewContext context)
> {
> ServletContext sc = context.getServletContext();
> defaultLanguage = (String) sc.getAttribute(DEFAULT_LANGUAGE_KEY);
> if (defaultLanguage == null || defaultLanguage.trim().equals(""))
> {
> Context vc = context.getVelocityContext();
> defaultLanguage = (String) vc.get(DEFAULT_LANGUAGE_KEY);
> if (defaultLanguage == null || defaultLanguage.trim().equals(""))
> {
> // Use JVM default.
> defaultLanguage = Locale.getDefault().getLanguage();
> }
> }
> }
>
> This change makes your tool thread-safe and therefore the same
> instance can be reused the entire runtime.
This change would make it impossible to instantiate from outside its
package (even using getInstance(), since it itself is an instance
method and you'd have a chicken and egg problem on your hands). The
initialization activites that you suggest moving to the ctor are
better done at object initialization time than instantiation time, as
the application's configuration may differ between those two points in
time (thus the usefulness of the init() interface for initialization).
Never the less, I've taken your point to heart and applied changes
necessary to make the class thread-safe.
> Another point: Your Javadoc contains an example
> #parse ($multiviews.findLocalizedResource("header.vm"))
> but there is no method findLocalizedResource() with this signature.
Thanks Gabe, taken care of.
> Finally, I suggest that you move your tool to the tools package.
> The rules are this:
> Package struts: All Struts related tools
> Package tools: All other tools
> Package view: View engines (like VSS) and supporting infrastructure
> but no tools.
So you see even View-specific tools like the MultiViewsTool content
negotiation tool as part ofthe Tools package? What's your reasoning
for wanting to keep View-specific context tools out of the View
package?
> Sorry for the delay in answering. Just had a few other things
> going on.
No problem Gabe, we all do. We'll get things sorted out eventually.
- Dan
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Daniel Rall wrote:
...
> You also changed the existing public API, removing the ContextTool
> interface (since there's been no 1.0 release, no big deal). Just
> after you checked in, I checked in a implementation of the ContextTool
> interface, MultiViewsTool, which provides MultiViews-style content
> negotiation to I18N'd resources.
>
> I could switch over to your new getInstance() API, but I much
> preferred the old API (init/destroy); the getInstance() factory method
> doesn't seem especially appropriate as an instance method.
I have no problem changing the names to best reflect what the method
does. We changed to getInstance() because most tools return new
instances from that method.
> What has replaced the destroy() method? If there is no replacement, what is
> your justification for the removal of the functionality?
I dropped this without a replacement. Is there really a need for
a destroy()? I havn't seen one. If there is one, we can re-introduce
it.
My concern is this: We are talking here about auto-loading context
tools. destroy() would have to be called after *every* request for
*every* tool that has a life cycle of 'request'. That's a lot of
methods calls that likely don't do anything in most cases.
A more efficient way might be to define an interface
CleanupEnabledContextTool that tools with cleanup needs have to implement.
> Which of the
> new interfaces would you recommend for the MultiViewsTool?
Since your tool needs access to the servlet environment, this would
be interface ServletContextTool.
I'd like to propose the following improvement to your tool:
Replace the current init() method with the following two methods.
/**
* new init()
*/
public Object init(ViewContext context)
{
return new MultiViewsTool(context);
}
/**
* Internal constructor
*/
protected MultiViewsTool(ViewContext context)
{
ServletContext sc = context.getServletContext();
defaultLanguage = (String) sc.getAttribute(DEFAULT_LANGUAGE_KEY);
if (defaultLanguage == null || defaultLanguage.trim().equals(""))
{
Context vc = context.getVelocityContext();
defaultLanguage = (String) vc.get(DEFAULT_LANGUAGE_KEY);
if (defaultLanguage == null || defaultLanguage.trim().equals(""))
{
// Use JVM default.
defaultLanguage = Locale.getDefault().getLanguage();
}
}
}
This change makes your tool thread-safe and therefore the same
instance can be reused the entire runtime.
Another point: Your Javadoc contains an example
#parse ($multiviews.findLocalizedResource("header.vm"))
but there is no method findLocalizedResource() with this signature.
Finally, I suggest that you move your tool to the tools package.
The rules are this:
Package struts: All Struts related tools
Package tools: All other tools
Package view: View engines (like VSS) and supporting infrastructure
but no tools.
Sorry for the delay in answering. Just had a few other things
going on.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Regarding logging again:
Daniel Rall wrote:
> "Geir Magnusson Jr." <ge...@optonline.net> writes:
> [snipped Gabe's logging description]
>
>>I am not a fan of this either. As many reservations as I have about the
>>commons logging, we shouldn't be inventing another. I just proposed to the
>>commons-logger crowd that I add an interface LogUser or something to do the
>>same thing - then we can use the commons logging interface, and any context
>>tools we create are at least usable outside of the Velocity tools
>>environment in this respect.
>>
>>I am very much afraid of crafting too much special stuff into our general
>>tools... It's ok for integration with other systems, like struts, but if we
>>can keep things as clean and free from one-off contracts as possible, I
>>think that¹s better.
>>
>
> Indeed. Apache has enough logging interfaces floating around already
> that Velocity has no business trying to tackle the job. :)
>
>
>>Gabe said:
>>
>>>In general, I believe logging from context tools should be kept to
>>>the minimum. Access logging should be left to the underlying framework.
>>>
>>I also agree - I think that having components in commons needing a logger is
>>a bit weird, but that's just me... I think components should communicate
>>with the framework they are being used by, not some external resources like
>>that...
>>
>
> Agreed.
Everyone seems to agree. On what? Sorry, but can someone help me to
understand the conclusion of this discussion? What are the proposed
changes to the current implementation?
Thanks
Gabe ...struggling to keep up with the flood of mails...
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
[snipped Gabe's logging description]
> I am not a fan of this either. As many reservations as I have about the
> commons logging, we shouldn't be inventing another. I just proposed to the
> commons-logger crowd that I add an interface LogUser or something to do the
> same thing - then we can use the commons logging interface, and any context
> tools we create are at least usable outside of the Velocity tools
> environment in this respect.
>
> I am very much afraid of crafting too much special stuff into our general
> tools... It's ok for integration with other systems, like struts, but if we
> can keep things as clean and free from one-off contracts as possible, I
> think that¹s better.
Indeed. Apache has enough logging interfaces floating around already
that Velocity has no business trying to tackle the job. :)
> Gabe said:
>> In general, I believe logging from context tools should be kept to
>> the minimum. Access logging should be left to the underlying framework.
>
> I also agree - I think that having components in commons needing a logger is
> a bit weird, but that's just me... I think components should communicate
> with the framework they are being used by, not some external resources like
> that...
Agreed.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 8:31 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
> Regarding logging:
>
> Daniel Rall wrote:
>
>>> Gabe wrote:
>>>> - LogEnabledContextTool: for context tools that need access
>>>> to a logging facility
>>>>
>>
>> This should be directly compatible with one or more of the following
>> APIs: Velocity, Log4J, or Commons Logging. I didn't see support for
>> any of these without custom a log adapter.
>
>
> The current approach to logging from context tools is the following:
>
> - Context tools that need access to a logger instance implement
> interface LogEnabledContextTool. This interface defines a method
>
> public void setLogger(ContextToolLogger logger)
>
> A toolbox manager uses this method to pass a logger instance. Logger
> instances need to implement interface ContextToolLogger. The typical
> approach would be to write a wrapper that wraps the log system and
> implement ContextToolLogger.
>
> There is currently one implementation of ContextToolLogger. It is
> called ServletContextToolLogger and simply logs messages to the
> log facility provided for by the Servlet API.
>
> Wrapper for other log system (log3J, commons logging) can be added
> anytime without changes to the context tools. I haven't planned to
> write any such wrappers for now.
I am not a fan of this either. As many reservations as I have about the
commons logging, we shouldn't be inventing another. I just proposed to the
commons-logger crowd that I add an interface LogUser or something to do the
same thing - then we can use the commons logging interface, and any context
tools we create are at least usable outside of the Velocity tools
environment in this respect.
I am very much afraid of crafting too much special stuff into our general
tools... It's ok for integration with other systems, like struts, but if we
can keep things as clean and free from one-off contracts as possible, I
think that¹s better.
> My primary question at the moment is: Is the interface defined by
> ContextToolLogger powerful enough? Currently is simply defines
> a method
>
> public void log(String msg);
>
> In general, I believe logging from context tools should be kept to
> the minimum. Access logging should be left to the underlying framework.
I also agree - I think that having components in commons needing a logger is
a bit weird, but that's just me... I think components should communicate
with the framework they are being used by, not some external resources like
that...
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
Age and treachery will always triumph over youth and talent
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Regarding logging:
Daniel Rall wrote:
>> Gabe wrote:
>>> - LogEnabledContextTool: for context tools that need access
>>> to a logging facility
>>>
>
> This should be directly compatible with one or more of the following
> APIs: Velocity, Log4J, or Commons Logging. I didn't see support for
> any of these without custom a log adapter.
The current approach to logging from context tools is the following:
- Context tools that need access to a logger instance implement
interface LogEnabledContextTool. This interface defines a method
public void setLogger(ContextToolLogger logger)
A toolbox manager uses this method to pass a logger instance. Logger
instances need to implement interface ContextToolLogger. The typical
approach would be to write a wrapper that wraps the log system and
implement ContextToolLogger.
There is currently one implementation of ContextToolLogger. It is
called ServletContextToolLogger and simply logs messages to the
log facility provided for by the Servlet API.
Wrapper for other log system (log3J, commons logging) can be added
anytime without changes to the context tools. I haven't planned to
write any such wrappers for now.
My primary question at the moment is: Is the interface defined by
ContextToolLogger powerful enough? Currently is simply defines
a method
public void log(String msg);
In general, I believe logging from context tools should be kept to
the minimum. Access logging should be left to the underlying framework.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
> On 4/3/02 10:14 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
>
>> WRT the name parameter name "properties", I think it's now a poor
>> choice as it provides no namespace protection (to avoid name clashes).
>> While this was not a problem when looking only in the servlet-specific
>> initialization parameters, now that the servlet context is examined as
>> a fall back (when looking for a default or global path to the
>> properties file), it would make more sense to change the key to a more
>> standard naming convention. I suggest
>> org.apache.velocity.servlet.properties or
>> org.apache.velocity.servlet.propertiesPath as two examples of what I
>> consider to be standard namespace protection for globally accessible
>> hash keys. I'd be willing to switch the parameter name over, but make
>> the switch in a manner which allows the key "properties" to still work
>> in a deprecated fashion (i.e. it would print a warning if used).
>>
>
> +1 Make it so... As long as it's backwards compatible :)
Done. Question about deprecation logging in commit message...
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 10:14 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>
>> On 4/3/02 6:31 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>
>>> Daniel Rall wrote:
>>>
>>>> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>>>>>> - Modified VelocityViewServlet so that it now can be configured
>>>>>> with a configuration file. Proposed location is
>>>>>> WEB-INF/velocity.properties. This allows now the use of a global
>>>>>> macros file (default location WEB-INF/VM_global_library.vm)
>>>>>>
>>>>> How is that different than now? Isn't VelocityViewServlet configurable
>>>>> via
>>>>> the config file?
>>>>>
>>>>
>>>> Isn't this is how the vanilla VelocityServlet works? What makes the
>>>> VVS different in this regard?
>>>
>>>
>>> The difference is how VVS gets the path of where to look for the
>>> config file. It is now a parameter in web.xml.
>>
>> But the standard VelocityServlet lets you specify that also in the
>> init-params...
>>
>> I still fail to see the difference... It may not have been used in VVS
>> explicitly, but again, there is a long history of using "properties" as the
>> key to get vel props, and we'll continue with that to avoid confusion for
>> existing vel users...
>
> Not only does the standard VelocityServlet use servlet-specific
> initialization parameters, but a recent patch by myself will have it
> fall back to a context wide parameter if no servlet-specific param is
> defined:
>
> <http://cvs.apache.org/viewcvs/jakarta-velocity/src/java/org/apache/velocity/s
> ervlet/VelocityServlet.java.diff?r1=text&tr1=1.43&r2=text&tr2=1.44&diff_format
> =u>
>
> Comparing the two initVelocity(ServletConfig) implementations, I don't
> see any reason for the one in VVS to exist (in fact, it's a regression
> in functionality).
>
> WRT the name parameter name "properties", I think it's now a poor
> choice as it provides no namespace protection (to avoid name clashes).
> While this was not a problem when looking only in the servlet-specific
> initialization parameters, now that the servlet context is examined as
> a fall back (when looking for a default or global path to the
> properties file), it would make more sense to change the key to a more
> standard naming convention. I suggest
> org.apache.velocity.servlet.properties or
> org.apache.velocity.servlet.propertiesPath as two examples of what I
> consider to be standard namespace protection for globally accessible
> hash keys. I'd be willing to switch the parameter name over, but make
> the switch in a manner which allows the key "properties" to still work
> in a deprecated fashion (i.e. it would print a warning if used).
>
+1 Make it so... As long as it's backwards compatible :)
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
Java : the speed of Smalltalk with the simple elegance of C++...
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
> On 4/3/02 6:31 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>
>> Daniel Rall wrote:
>>
>>> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>>>>> - Modified VelocityViewServlet so that it now can be configured
>>>>> with a configuration file. Proposed location is
>>>>> WEB-INF/velocity.properties. This allows now the use of a global
>>>>> macros file (default location WEB-INF/VM_global_library.vm)
>>>>>
>>>> How is that different than now? Isn't VelocityViewServlet configurable via
>>>> the config file?
>>>>
>>>
>>> Isn't this is how the vanilla VelocityServlet works? What makes the
>>> VVS different in this regard?
>>
>>
>> The difference is how VVS gets the path of where to look for the
>> config file. It is now a parameter in web.xml.
>
> But the standard VelocityServlet lets you specify that also in the
> init-params...
>
> I still fail to see the difference... It may not have been used in VVS
> explicitly, but again, there is a long history of using "properties" as the
> key to get vel props, and we'll continue with that to avoid confusion for
> existing vel users...
Not only does the standard VelocityServlet use servlet-specific
initialization parameters, but a recent patch by myself will have it
fall back to a context wide parameter if no servlet-specific param is
defined:
<http://cvs.apache.org/viewcvs/jakarta-velocity/src/java/org/apache/velocity/servlet/VelocityServlet.java.diff?r1=text&tr1=1.43&r2=text&tr2=1.44&diff_format=u>
Comparing the two initVelocity(ServletConfig) implementations, I don't
see any reason for the one in VVS to exist (in fact, it's a regression
in functionality).
WRT the name parameter name "properties", I think it's now a poor
choice as it provides no namespace protection (to avoid name clashes).
While this was not a problem when looking only in the servlet-specific
initialization parameters, now that the servlet context is examined as
a fall back (when looking for a default or global path to the
properties file), it would make more sense to change the key to a more
standard naming convention. I suggest
org.apache.velocity.servlet.properties or
org.apache.velocity.servlet.propertiesPath as two examples of what I
consider to be standard namespace protection for globally accessible
hash keys. I'd be willing to switch the parameter name over, but make
the switch in a manner which allows the key "properties" to still work
in a deprecated fashion (i.e. it would print a warning if used).
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 6:31 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
> Daniel Rall wrote:
>
>> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>>>> - Modified VelocityViewServlet so that it now can be configured
>>>> with a configuration file. Proposed location is
>>>> WEB-INF/velocity.properties. This allows now the use of a global
>>>> macros file (default location WEB-INF/VM_global_library.vm)
>>>>
>>> How is that different than now? Isn't VelocityViewServlet configurable via
>>> the config file?
>>>
>>
>> Isn't this is how the vanilla VelocityServlet works? What makes the
>> VVS different in this regard?
>
>
> The difference is how VVS gets the path of where to look for the
> config file. It is now a parameter in web.xml.
But the standard VelocityServlet lets you specify that also in the
init-params...
I still fail to see the difference... It may not have been used in VVS
explicitly, but again, there is a long history of using "properties" as the
key to get vel props, and we'll continue with that to avoid confusion for
existing vel users...
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
"Now what do we do?"
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Daniel Rall wrote:
> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>>>- Modified VelocityViewServlet so that it now can be configured
>>> with a configuration file. Proposed location is
>>> WEB-INF/velocity.properties. This allows now the use of a global
>>> macros file (default location WEB-INF/VM_global_library.vm)
>>>
>>How is that different than now? Isn't VelocityViewServlet configurable via
>>the config file?
>>
>
> Isn't this is how the vanilla VelocityServlet works? What makes the
> VVS different in this regard?
The difference is how VVS gets the path of where to look for the
config file. It is now a parameter in web.xml.
Dan or Geir please have a look at VVS, specifically method initVelocity()
to make sure all is within the green range.
http://cvs.apache.org/viewcvs/jakarta-velocity-tools/view/src/java/org/apache/velocity/tools/view/servlet/VelocityViewServlet.java?rev=1.3&content-type=text/vnd.viewcvs-markup
Gabe
PS: More to follow... I am doing the easy stuff first :-)
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
> On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>
>> Hi,
>> I've just checked in a number of changes to the jakarta-velocity-tools
>> project. Your feedback is welcome. Here's a brief outline of the
>> major changes:
>
> Yikes! What a pile. Incremental is good too :)
Yes. Though your log message is fairly comprehensive, it's much
easier to digest changes when they're incremental.
>> - Introduced four interfaces that establish contracts between
>> context tools and compatible toolbox managers:
>> - ThreadSafeContextTool: for thread-safe context tool, tool
>> instance is reused for the entire runtime
>
> This one is weird - I don¹t remember if there was a conclusion, but wasn't
> this something that was indicated by the toolbox config when you specified
> the tool?
>
>> - LogEnabledContextTool: for context tools that need access
>> to a logging facility
This should be directly compatible with one or more of the following
APIs: Velocity, Log4J, or Commons Logging. I didn't see support for
any of these without custom a log adapter.
>> - ServletContextTool: for context tools that need access to
>> the servlet envrionment
>> - ContextContextTool: for context tools that need access to
>> the Velocity context
>> These interfaces are all optional. Any object with public methods
>> and a public, empty constructor can be used as a context tool.
>> Its handling may not be the most optimal one, though.
You also changed the existing public API, removing the ContextTool
interface (since there's been no 1.0 release, no big deal). Just
after you checked in, I checked in a implementation of the ContextTool
interface, MultiViewsTool, which provides MultiViews-style content
negotiation to I18N'd resources.
I could switch over to your new getInstance() API, but I much
preferred the old API (init/destroy); the getInstance() factory method
doesn't seem especially appropriate as an instance method. What has
replaced the destroy() method? If there is no replacement, what is
your justification for the removal of the functionality? Which of the
new interfaces would you recommend for the MultiViewsTool?
>> - Modified VelocityViewServlet so that it now can be configured
>> with a configuration file. Proposed location is
>> WEB-INF/velocity.properties. This allows now the use of a global
>> macros file (default location WEB-INF/VM_global_library.vm)
>
> How is that different than now? Isn't VelocityViewServlet configurable via
> the config file?
Isn't this is how the vanilla VelocityServlet works? What makes the
VVS different in this regard?
- Dan
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/4/02 4:25 PM, "Mark de Reeper" <ma...@yahoo.com> wrote:
> Thinking out loud....
>
> If a default Servlet based logger is useful in the VelocityViewServlet then
> why
> is it not also useful in the VelocityServlet?
It will be useful there. I just wanted *someone* besides me to play with
it. I am using it at a client now, and it's nice...
>
> The webapp loader would actually make a great default loader in the
> VelocityServlet, rather than a file loader based on the current dir. Most
> users
> end up overriding the file path to make it work for them. The behaviour of the
> webapp loader is probably better for most of the simple cases (and would make
> it easier for first timers too).
Yes, indeedy. But again, wanted to see someone else use it.
>
> If these are both true, why not move the initVelocity stuff back into the
> VelocityServlet?
We will, if <fill in repeated phrase here>
:)
>
> Also,
>
> Now that the loadConfiguration is being used, the VELOCITY_PROPERTIES const in
> VelocityViewServlet is probably redundent.
>
> Having read the JavaDoc for ServletContext.getResourceAsStream, it does look
> like a better way to load the properties.
>
>
> Cheers
>
> Mark
>
> --- Gabriel Sidler <si...@teamup.ch> wrote:
>> Daniel Rall wrote:
>>
>> ...
>>
>>>
>>> More than that, you should totally do away with the initVelocity()
>>> method in VVS. If it adds value, it probably makes sense to push it
>>> up into standard VS.
>>>
>>
>>
>> I just modified VelocityViewServlet such that initVelocity() uses
>> method loadConfiguration() of VelocityServlet.
>>
>> I don't see how we can get rid of initVelocity() entirely at the
>> moment. Its sets up the logger and the webapp resource loader.
>>
>>
>> One thing I noticed in VelocityServlet: The config file is now
>> read using normal file io. My understanding is, that it would be
>> better to use the getResourceAsStream() method of ServletContext.
>>
>>
>> Replace:
>>
>> Properties p = new Properties();
>> ...
>> p.load( new FileInputStream(propsFile) );
>>
>> with something like:
>>
>> InputStream is = getServletContext().getResourceAsStream(filename);
>> if ( is != null)
>> {
>> velProps.load(is);
>> }
>>
>>
>> getResourceAsStream deals with the conversion to a real path and
>> the fact the the .war file might not be unwrapped.
>>
>>
>> Gabe
>>
>>
>> --
>> Gabriel Sidler
>> Software Engineer, Eivycom GmbH, Zurich, Switzerland
>>
>>
>> --
>> To unsubscribe, e-mail:
>> <ma...@jakarta.apache.org>
>> For additional commands, e-mail:
>> <ma...@jakarta.apache.org>
>>
>
>
> __________________________________________________
> Do You Yahoo!?
> Yahoo! Tax Center - online filing with TurboTax
> http://taxes.yahoo.com/
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
Age and treachery will always triumph over youth and talent
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Mark de Reeper <ma...@yahoo.com>.
Thinking out loud....
If a default Servlet based logger is useful in the VelocityViewServlet then why
is it not also useful in the VelocityServlet?
The webapp loader would actually make a great default loader in the
VelocityServlet, rather than a file loader based on the current dir. Most users
end up overriding the file path to make it work for them. The behaviour of the
webapp loader is probably better for most of the simple cases (and would make
it easier for first timers too).
If these are both true, why not move the initVelocity stuff back into the
VelocityServlet?
Also,
Now that the loadConfiguration is being used, the VELOCITY_PROPERTIES const in
VelocityViewServlet is probably redundent.
Having read the JavaDoc for ServletContext.getResourceAsStream, it does look
like a better way to load the properties.
Cheers
Mark
--- Gabriel Sidler <si...@teamup.ch> wrote:
> Daniel Rall wrote:
>
> ...
>
> >
> > More than that, you should totally do away with the initVelocity()
> > method in VVS. If it adds value, it probably makes sense to push it
> > up into standard VS.
> >
>
>
> I just modified VelocityViewServlet such that initVelocity() uses
> method loadConfiguration() of VelocityServlet.
>
> I don't see how we can get rid of initVelocity() entirely at the
> moment. Its sets up the logger and the webapp resource loader.
>
>
> One thing I noticed in VelocityServlet: The config file is now
> read using normal file io. My understanding is, that it would be
> better to use the getResourceAsStream() method of ServletContext.
>
>
> Replace:
>
> Properties p = new Properties();
> ...
> p.load( new FileInputStream(propsFile) );
>
> with something like:
>
> InputStream is = getServletContext().getResourceAsStream(filename);
> if ( is != null)
> {
> velProps.load(is);
> }
>
>
> getResourceAsStream deals with the conversion to a real path and
> the fact the the .war file might not be unwrapped.
>
>
> Gabe
>
>
> --
> Gabriel Sidler
> Software Engineer, Eivycom GmbH, Zurich, Switzerland
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>
__________________________________________________
Do You Yahoo!?
Yahoo! Tax Center - online filing with TurboTax
http://taxes.yahoo.com/
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: [VelTools] Changes
Posted by Donnie Hale <do...@haleonline.net>.
> -----Original Message-----
> From: Gabriel Sidler [mailto:sidler@teamup.ch]
> Sent: Thursday, April 04, 2002 8:28 AM
> To: Velocity Developers List
> Subject: Re: [VelTools] Changes
>
>
> Daniel Rall wrote:
>
> ...
>
> >
> > More than that, you should totally do away with the initVelocity()
> > method in VVS. If it adds value, it probably makes sense to push it
> > up into standard VS.
> >
>
>
> I just modified VelocityViewServlet such that initVelocity() uses
> method loadConfiguration() of VelocityServlet.
>
> I don't see how we can get rid of initVelocity() entirely at the
> moment. Its sets up the logger and the webapp resource loader.
>
>
> One thing I noticed in VelocityServlet: The config file is now
> read using normal file io. My understanding is, that it would be
> better to use the getResourceAsStream() method of ServletContext.
>
>
> Replace:
>
> Properties p = new Properties();
> ...
> p.load( new FileInputStream(propsFile) );
>
> with something like:
>
> InputStream is = getServletContext().getResourceAsStream(filename);
> if ( is != null)
> {
> velProps.load(is);
> }
>
>
> getResourceAsStream deals with the conversion to a real path and
> the fact the the .war file might not be unwrapped.
FWIW, I noticed that recently as well. Wasn't sure if it was for an explicit
reason or not. If not, I think it should definitely use getResourceAsStream
from the ServletContext.
Donnie
>
>
> Gabe
>
>
> --
> Gabriel Sidler
> Software Engineer, Eivycom GmbH, Zurich, Switzerland
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Daniel Rall wrote:
...
>
> More than that, you should totally do away with the initVelocity()
> method in VVS. If it adds value, it probably makes sense to push it
> up into standard VS.
>
I just modified VelocityViewServlet such that initVelocity() uses
method loadConfiguration() of VelocityServlet.
I don't see how we can get rid of initVelocity() entirely at the
moment. Its sets up the logger and the webapp resource loader.
One thing I noticed in VelocityServlet: The config file is now
read using normal file io. My understanding is, that it would be
better to use the getResourceAsStream() method of ServletContext.
Replace:
Properties p = new Properties();
...
p.load( new FileInputStream(propsFile) );
with something like:
InputStream is = getServletContext().getResourceAsStream(filename);
if ( is != null)
{
velProps.load(is);
}
getResourceAsStream deals with the conversion to a real path and
the fact the the .war file might not be unwrapped.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 11:39 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>
>>>> We have decided that VVS, which is an 'application' of sorts, will
>>>> log into the servlet logger - this is to parallel how JSP does it's
>>>> logging...
>>>
>>> Okay. Is this what it's recommened for me to use in VS as well?
>>
>> Dunno - was hoping we get some testing in VVS and then move it into core if
>> people like it. Of course, the less in core the better.
>>
>> I am using it (ServletLogger) here at a client - will let you know how much
>> we like it - so far, so good.
>
> For now, I just used vanilla servlet API ServletContext.log(String)
> for the time being in VS. Feel free to change it.
>
Oh, for the deprecation warning? That's just peachy. I thought you meant
if we should change VS to use the ServletLogger by default.
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
Uncertainty breeds confusion and confusion breeds despair. - Ceki Gulcu
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
>>> We have decided that VVS, which is an 'application' of sorts, will
>>> log into the servlet logger - this is to parallel how JSP does it's
>>> logging...
>>
>> Okay. Is this what it's recommened for me to use in VS as well?
>
> Dunno - was hoping we get some testing in VVS and then move it into core if
> people like it. Of course, the less in core the better.
>
> I am using it (ServletLogger) here at a client - will let you know how much
> we like it - so far, so good.
For now, I just used vanilla servlet API ServletContext.log(String)
for the time being in VS. Feel free to change it.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 11:18 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>
>> On 4/3/02 10:51 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
>>
>>> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>>>
>>>> On 4/3/02 10:35 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
>>>>
>>>>> More than that, you should totally do away with the initVelocity()
>>>>> method in VVS. If it adds value, it probably makes sense to push it
>>>>> up into standard VS.
>>>>
>>>> Well..... Not sure. Once thing that the VVS does is use the
>>>> ServletLogger(), and I think that's important. The point of making this
>>>> overloadable is that derived classes can do weird things like this...
>>>
>>> Use of ServletLogger() in VVS doesn't add value in my book. ;)
>>>
>>> I like the standard Velocity logging API -- if you want logging to go
>>> to the servlet logs, reconfigure the main Velocity logger itself
>>> rather than doing one-offs like in VVS.
>>
>> What are you talking about? The ServletLogger is 100% conformant to the
>> regular logging API.
>
> I assumed your were talking about the logging which is part of the
> servlet API.
>
> http://java.sun.com/products/servlet/2.3/javadoc/javax/servlet/ServletContext.
> html#log(java.lang.String,%20java.lang.Throwable)
>
> Guess not.
That is what it uses underneath - it's a little wrapper that implements the
LogSystem API and then just shovels everything into the
ServletContext.log().
The idea is that if we are shooting for a drop-in replacement for JSP, we
need to behave the same way (or as close as we can w/o compromising our
principles :)
>
>> We have decided that VVS, which is an 'application' of sorts, will
>> log into the servlet logger - this is to parallel how JSP does it's
>> logging...
>
> Okay. Is this what it's recommened for me to use in VS as well?
Dunno - was hoping we get some testing in VVS and then move it into core if
people like it. Of course, the less in core the better.
I am using it (ServletLogger) here at a client - will let you know how much
we like it - so far, so good.
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
My inner cowboy needs to yodel.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
> On 4/3/02 10:51 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
>
>> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>>
>>> On 4/3/02 10:35 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
>>>
>>>> More than that, you should totally do away with the initVelocity()
>>>> method in VVS. If it adds value, it probably makes sense to push it
>>>> up into standard VS.
>>>
>>> Well..... Not sure. Once thing that the VVS does is use the
>>> ServletLogger(), and I think that's important. The point of making this
>>> overloadable is that derived classes can do weird things like this...
>>
>> Use of ServletLogger() in VVS doesn't add value in my book. ;)
>>
>> I like the standard Velocity logging API -- if you want logging to go
>> to the servlet logs, reconfigure the main Velocity logger itself
>> rather than doing one-offs like in VVS.
>
> What are you talking about? The ServletLogger is 100% conformant to the
> regular logging API.
I assumed your were talking about the logging which is part of the
servlet API.
http://java.sun.com/products/servlet/2.3/javadoc/javax/servlet/ServletContext.html#log(java.lang.String,%20java.lang.Throwable)
Guess not.
> We have decided that VVS, which is an 'application' of sorts, will
> log into the servlet logger - this is to parallel how JSP does it's
> logging...
Okay. Is this what it's recommened for me to use in VS as well?
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 10:51 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
> "Geir Magnusson Jr." <ge...@optonline.net> writes:
>
>> On 4/3/02 10:35 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
>>
>>> More than that, you should totally do away with the initVelocity()
>>> method in VVS. If it adds value, it probably makes sense to push it
>>> up into standard VS.
>>
>> Well..... Not sure. Once thing that the VVS does is use the
>> ServletLogger(), and I think that's important. The point of making this
>> overloadable is that derived classes can do weird things like this...
>
> Use of ServletLogger() in VVS doesn't add value in my book. ;)
>
> I like the standard Velocity logging API -- if you want logging to go
> to the servlet logs, reconfigure the main Velocity logger itself
> rather than doing one-offs like in VVS.
What are you talking about? The ServletLogger is 100% conformant to the
regular logging API. We have decided that VVS, which is an 'application' of
sorts, will log into the servlet logger - this is to parallel how JSP does
it's logging...
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
"The greatest pleasure in life is doing what people say you cannot do."
- Walter Bagehot
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
> On 4/3/02 10:35 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
>
>> More than that, you should totally do away with the initVelocity()
>> method in VVS. If it adds value, it probably makes sense to push it
>> up into standard VS.
>
> Well..... Not sure. Once thing that the VVS does is use the
> ServletLogger(), and I think that's important. The point of making this
> overloadable is that derived classes can do weird things like this...
Use of ServletLogger() in VVS doesn't add value in my book. ;)
I like the standard Velocity logging API -- if you want logging to go
to the servlet logs, reconfigure the main Velocity logger itself
rather than doing one-offs like in VVS.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 10:35 AM, "Daniel Rall" <dl...@finemaltcoding.com> wrote:
> Gabriel Sidler <si...@teamup.ch> writes:
>
>> Geir Magnusson Jr. wrote:
>>
>>> On 4/3/02 6:18 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>>
>>>> Geir Magnusson Jr. wrote:
>>>>
>>>>
>>>>> On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>>>>
>>>>>> - Modified VelocityViewServlet so that it now can be configured
>>>>>> with a configuration file. Proposed location is
>>>>>> WEB-INF/velocity.properties. This allows now the use of a global
>>>>>> macros file (default location WEB-INF/VM_global_library.vm)
>>>>>>
>>>>>>
>>>>> How is that different than now? Isn't VelocityViewServlet configurable
>>>>> via
>>>>> the config file?
>>>>>
>>>>
>>>> In the past it wasn't possible to specify a path where VelocityViewServlet
>>>> should look for the config file. There is now a optional parameter
>>>> 'velocity.properties' in web.xml that can be used to specify the location
>>>> of the config file.
>>>>
>>> We shouldn't re-invent the wheel here. The VelocityServlet, from
>>> which the
>>> view servlet is derived uses something different... They should be the
>>> same...
>>
>>
>> Yes, you are right. I overlooked the fact that VelocityServlet already
>> defines
>> a constant INIT_PROPS_KEY. I am going to modify VVS so that it also uses
>> this constant.
>
> More than that, you should totally do away with the initVelocity()
> method in VVS. If it adds value, it probably makes sense to push it
> up into standard VS.
Well..... Not sure. Once thing that the VVS does is use the
ServletLogger(), and I think that's important. The point of making this
overloadable is that derived classes can do weird things like this...
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
POC lives!
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Gabriel Sidler <si...@teamup.ch> writes:
> Geir Magnusson Jr. wrote:
>
>> On 4/3/02 6:18 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>
>>>Geir Magnusson Jr. wrote:
>>>
>>>
>>>>On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>>>
>>>>>- Modified VelocityViewServlet so that it now can be configured
>>>>>with a configuration file. Proposed location is
>>>>>WEB-INF/velocity.properties. This allows now the use of a global
>>>>>macros file (default location WEB-INF/VM_global_library.vm)
>>>>>
>>>>>
>>>>How is that different than now? Isn't VelocityViewServlet configurable via
>>>>the config file?
>>>>
>>>
>>>In the past it wasn't possible to specify a path where VelocityViewServlet
>>>should look for the config file. There is now a optional parameter
>>>'velocity.properties' in web.xml that can be used to specify the location
>>>of the config file.
>>>
>> We shouldn't re-invent the wheel here. The VelocityServlet, from
>> which the
>> view servlet is derived uses something different... They should be the
>> same...
>
>
> Yes, you are right. I overlooked the fact that VelocityServlet already defines
> a constant INIT_PROPS_KEY. I am going to modify VVS so that it also uses
> this constant.
More than that, you should totally do away with the initVelocity()
method in VVS. If it adds value, it probably makes sense to push it
up into standard VS.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 8:07 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
> Geir Magnusson Jr. wrote:
>
>> On 4/3/02 6:18 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>
>>
>>> Geir Magnusson Jr. wrote:
>>>
>>>
>>>> On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>>>
>>>>> - Modified VelocityViewServlet so that it now can be configured
>>>>> with a configuration file. Proposed location is
>>>>> WEB-INF/velocity.properties. This allows now the use of a global
>>>>> macros file (default location WEB-INF/VM_global_library.vm)
>>>>>
>>>>>
>>>> How is that different than now? Isn't VelocityViewServlet configurable via
>>>> the config file?
>>>>
>>>
>>> In the past it wasn't possible to specify a path where VelocityViewServlet
>>> should look for the config file. There is now a optional parameter
>>> 'velocity.properties' in web.xml that can be used to specify the location
>>> of the config file.
>>>
>>
>> We shouldn't re-invent the wheel here. The VelocityServlet, from which the
>> view servlet is derived uses something different... They should be the
>> same...
>
>
> Yes, you are right. I overlooked the fact that VelocityServlet already defines
> a constant INIT_PROPS_KEY. I am going to modify VVS so that it also uses
> this constant.
>
Cool beans...
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
The obvious solutions are challenging
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Gabriel Sidler <si...@teamup.ch> writes:
> Mark,
> I just change it such that VVS now uses loadConfiguration() of VS.
+1, perfect.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Mark,
I just change it such that VVS now uses loadConfiguration() of VS.
Gabe
Mark de Reeper wrote:
> Gabriel,
>
> I commented on the property loading a while back, see
> http://www.mail-archive.com/velocity-user@jakarta.apache.org/msg05297.html
>
> Why not reuse the property loading stuff that exists in the VelocityServlet
> instead of rewriting it?
>
> The issue will be, do you always add the webapp loading stuff, or only if their
> is no user defined properties.
>
> Note: There is a minor bug in my example as it only adds the useful stuff to
> the props as a result of an exception, the properties could also be empty from
> the loadProperties method.
>
> Cheers
>
> Mark
>
>
> --- Gabriel Sidler <si...@teamup.ch> wrote:
>
>>Geir Magnusson Jr. wrote:
>>
>>
>>>On 4/3/02 6:18 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>>
>>>
>>>
>>>>Geir Magnusson Jr. wrote:
>>>>
>>>>
>>>>
>>>>>On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>>>>
>>>>>
>>>>>>- Modified VelocityViewServlet so that it now can be configured
>>>>>>with a configuration file. Proposed location is
>>>>>>WEB-INF/velocity.properties. This allows now the use of a global
>>>>>>macros file (default location WEB-INF/VM_global_library.vm)
>>>>>>
>>>>>>
>>>>>>
>>>>>How is that different than now? Isn't VelocityViewServlet configurable
>>>>>
>>via
>>
>>>>>the config file?
>>>>>
>>>>>
>>>>In the past it wasn't possible to specify a path where VelocityViewServlet
>>>>should look for the config file. There is now a optional parameter
>>>>'velocity.properties' in web.xml that can be used to specify the location
>>>>of the config file.
>>>>
>>>>
>>>We shouldn't re-invent the wheel here. The VelocityServlet, from which the
>>>view servlet is derived uses something different... They should be the
>>>same...
>>>
>>
>>Yes, you are right. I overlooked the fact that VelocityServlet already
>>defines
>>a constant INIT_PROPS_KEY. I am going to modify VVS so that it also uses
>>this constant.
>>
>>Gabe
>>
>>
>>
>>
>>--
>>Gabriel Sidler
>>Software Engineer, Eivycom GmbH, Zurich, Switzerland
>>
>>
>>--
>>To unsubscribe, e-mail:
>><ma...@jakarta.apache.org>
>>For additional commands, e-mail:
>><ma...@jakarta.apache.org>
>>
>
>
> __________________________________________________
> Do You Yahoo!?
> Yahoo! Tax Center - online filing with TurboTax
> http://taxes.yahoo.com/
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
>
--
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Mark de Reeper <ma...@yahoo.com>.
Gabriel,
I commented on the property loading a while back, see
http://www.mail-archive.com/velocity-user@jakarta.apache.org/msg05297.html
Why not reuse the property loading stuff that exists in the VelocityServlet
instead of rewriting it?
The issue will be, do you always add the webapp loading stuff, or only if their
is no user defined properties.
Note: There is a minor bug in my example as it only adds the useful stuff to
the props as a result of an exception, the properties could also be empty from
the loadProperties method.
Cheers
Mark
--- Gabriel Sidler <si...@teamup.ch> wrote:
> Geir Magnusson Jr. wrote:
>
> > On 4/3/02 6:18 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
> >
> >
> >>Geir Magnusson Jr. wrote:
> >>
> >>
> >>>On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
> >>>
> >>>>- Modified VelocityViewServlet so that it now can be configured
> >>>>with a configuration file. Proposed location is
> >>>>WEB-INF/velocity.properties. This allows now the use of a global
> >>>>macros file (default location WEB-INF/VM_global_library.vm)
> >>>>
> >>>>
> >>>How is that different than now? Isn't VelocityViewServlet configurable
> via
> >>>the config file?
> >>>
> >>
> >>In the past it wasn't possible to specify a path where VelocityViewServlet
> >>should look for the config file. There is now a optional parameter
> >>'velocity.properties' in web.xml that can be used to specify the location
> >>of the config file.
> >>
> >
> > We shouldn't re-invent the wheel here. The VelocityServlet, from which the
> > view servlet is derived uses something different... They should be the
> > same...
>
>
> Yes, you are right. I overlooked the fact that VelocityServlet already
> defines
> a constant INIT_PROPS_KEY. I am going to modify VVS so that it also uses
> this constant.
>
> Gabe
>
>
>
>
> --
> Gabriel Sidler
> Software Engineer, Eivycom GmbH, Zurich, Switzerland
>
>
> --
> To unsubscribe, e-mail:
> <ma...@jakarta.apache.org>
> For additional commands, e-mail:
> <ma...@jakarta.apache.org>
>
__________________________________________________
Do You Yahoo!?
Yahoo! Tax Center - online filing with TurboTax
http://taxes.yahoo.com/
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Geir Magnusson Jr. wrote:
> On 4/3/02 6:18 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>
>
>>Geir Magnusson Jr. wrote:
>>
>>
>>>On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>>
>>>>- Modified VelocityViewServlet so that it now can be configured
>>>>with a configuration file. Proposed location is
>>>>WEB-INF/velocity.properties. This allows now the use of a global
>>>>macros file (default location WEB-INF/VM_global_library.vm)
>>>>
>>>>
>>>How is that different than now? Isn't VelocityViewServlet configurable via
>>>the config file?
>>>
>>
>>In the past it wasn't possible to specify a path where VelocityViewServlet
>>should look for the config file. There is now a optional parameter
>>'velocity.properties' in web.xml that can be used to specify the location
>>of the config file.
>>
>
> We shouldn't re-invent the wheel here. The VelocityServlet, from which the
> view servlet is derived uses something different... They should be the
> same...
Yes, you are right. I overlooked the fact that VelocityServlet already defines
a constant INIT_PROPS_KEY. I am going to modify VVS so that it also uses
this constant.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/3/02 6:18 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
> Geir Magnusson Jr. wrote:
>
>> On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>> - Modified VelocityViewServlet so that it now can be configured
>>> with a configuration file. Proposed location is
>>> WEB-INF/velocity.properties. This allows now the use of a global
>>> macros file (default location WEB-INF/VM_global_library.vm)
>>>
>>
>> How is that different than now? Isn't VelocityViewServlet configurable via
>> the config file?
>
>
> In the past it wasn't possible to specify a path where VelocityViewServlet
> should look for the config file. There is now a optional parameter
> 'velocity.properties' in web.xml that can be used to specify the location
> of the config file.
We shouldn't re-invent the wheel here. The VelocityServlet, from which the
view servlet is derived uses something different... They should be the
same...
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
Uncertainty breeds confusion and confusion breeds despair. - Ceki Gulcu
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Geir Magnusson Jr. wrote:
> On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
>>- Modified VelocityViewServlet so that it now can be configured
>> with a configuration file. Proposed location is
>> WEB-INF/velocity.properties. This allows now the use of a global
>> macros file (default location WEB-INF/VM_global_library.vm)
>>
>
> How is that different than now? Isn't VelocityViewServlet configurable via
> the config file?
In the past it wasn't possible to specify a path where VelocityViewServlet
should look for the config file. There is now a optional parameter
'velocity.properties' in web.xml that can be used to specify the location
of the config file.
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Geir Magnusson Jr." <ge...@optonline.net> writes:
>>>From a branding point of view, I don't think we should make it'VelXXX' but
> rather 'VelocityXXX' as 'Vel' is only our nickname, and if you haven't heard
> of velocity, it's hard to make the connection.
I agree with this. Use of the word Velocity in the Tools project
names adds value to the brand and brand-impact to the tools -- a great
symbiosis.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Geir Magnusson Jr. wrote:
>>From a branding point of view, I don't think we should make it'VelXXX' but
> rather 'VelocityXXX' as 'Vel' is only our nickname, and if you haven't heard
> of velocity, it's hard to make the connection.
>
> More inline...
...
> Gabe wrote:
>><proposal>
>>- VelTools is presented as an umbrella project for the following three
>> subprojects:
>> - VelLibray: A collection of reusable and well documented context tools
>> - VelStruts: Velocity/Struts integration
>> - VelServlet: General-purpose Velocity template rendering servlet
>>
>>
>
> Hm. The project is 'jakarta-velocity-tools' so making the jump to
> 'VelTools' is really a stretch.
>
> So maybe we refer to it as the Velocity Tools subproject, and have the parts
> be 'VelocityLibrary', 'VelocityStruts' and 'VelocityServlet'
Ok, makes sense to me. I'll change the documentation accordingly.
...
>>- Each subproject has it own, mostly independent set of documentation.
>> I think this is the best way to address the different needs of the
>> users. This way, a struts user can jump directly to the VelStruts
>> documentation. He doesn't need to know about VelTools or the other
>> subprojects, etc.
>>
>
> Independent in content, but with the same .dvsl from root...
Yes, but depends what you mean by 'root'.
All documentation currently is generated using the style sheet at
jakarta-velocity-tools/xdocs/site.dvsl. This is 'root' for me.
Note, that I have modified the originally check-in version of site.dvls.
I added several formatting rules to deal with the context tool documentation
as seen on
http://www.teamup.com/jakarta-velocity-tools/struts/docs/MessageTool.html
Gabe
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
>From a branding point of view, I don't think we should make it'VelXXX' but
rather 'VelocityXXX' as 'Vel' is only our nickname, and if you haven't heard
of velocity, it's hard to make the connection.
More inline...
On 4/3/02 5:56 AM, "Gabriel Sidler" <si...@teamup.ch> wrote:
> Let's start with the (hopefully) easy part: Documentation
>
> Geir Magnusson Jr. wrote:
>> Gabe wrote:
>
>>> - Setup a library of context tools and corresponding documentation.
>>> Named it VelLibrary. Added Nathan's contributed context tools to
>>> the VelLibrary and modified them to make them thread-safe. The
>>> directory structure is as discussed earlier:
>>>
>>> /jakarta-velocity-tools/tools/
>>> xdocs
>>> src/
>>> docs/
>>> lib/
>>> build.xml
>>>
>>> Included in VelLibray are now four tools: MathTool, DateTool,
>>> RequestParser and ToolLoader. Added all tools to package
>>> o.a.v.tools.tools. This is not commited yet. Awaiting Geir's
>>> approval to check-in the structure as proposed above!
>>>
>> That' looks great. But where does the 'VelLibrary' moniker come into it?
>
>
> Well, VelLibrary is a proposal. Here's my take on the whole documentation
> thing.
>
> jakarta-velocity-tool currently contains several interesting things:
>
> 1) A general-purpose servlet that renders Velocity templates. It supports
> automatic population of the context and a auto-loading tools.
>
> 2) A set of context tools that achieve Velocity/Struts integration.
>
> 3) A hopefully growing colletion of various reusable context tools.
>
> Depending on where you are coming from, you might be interested in one
> or the other or multiple. For example, a Struts user would be interested
> in 2) but he typically wouldn't care about the details of 1). A framework
> developer might be interested in 1) and 3) but not 2), etc.
>
> To address the documentation needs of the different users I took the
> following approach:
>
> <proposal>
> - VelTools is presented as an umbrella project for the following three
> subprojects:
> - VelLibray: A collection of reusable and well documented context tools
> - VelStruts: Velocity/Struts integration
> - VelServlet: General-purpose Velocity template rendering servlet
>
Hm. The project is 'jakarta-velocity-tools' so making the jump to
'VelTools' is really a stretch.
So maybe we refer to it as the Velocity Tools subproject, and have the parts
be 'VelocityLibrary', 'VelocityStruts' and 'VelocityServlet'
> - VelLibrary, VelStruts, VelServlet are my proposed names for the three
> subprojects. Dividing the project into the three clearly identifiable
> subprojects will make it more approachable and easier to document.
Agree on last sentence - but we already have that division... It's just the
names I think will be problematic.
> - Each subproject has it own, mostly independent set of documentation.
> I think this is the best way to address the different needs of the
> users. This way, a struts user can jump directly to the VelStruts
> documentation. He doesn't need to know about VelTools or the other
> subprojects, etc.
Independent in content, but with the same .dvsl from root...
> - The documentation of the VelTools umbrella project itself is limited to
> a brief overview of the different subprojects. All the interesting stuff
> is in the subprojects. Most users of sub-projects would never need
> to know about VelTools.
> </proposal>
+1
> Here is a snapshot of the current documenation to illustrate the
> proposal:
>
> If you are interested in the big picture, goto:
> http://www.teamup.com/jakarta-velocity-tools/docs/index.html
>
> If you are interested in Velocity-Struts integration, goto:
> http://www.teamup.com/jakarta-velocity-tools/struts/docs/index.html
>
> If you are interested in reuseable context tools, goto:
> http://www.teamup.com/jakarta-velocity-tools/docs/vellibrary.html
>
> If you are interested in a Velocity servlet, goto:
> http://www.teamup.com/jakarta-velocity-tools/view/docs/velservlet.html
>
>
> Makes sense?
>
>
> Gabe
>
>
> PS: More to follow...
>
> --
> Gabriel Sidler
> Software Engineer, Eivycom GmbH, Zurich, Switzerland
>
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
The cost of synchronization is much less that the cost of stupidity.
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by Gabriel Sidler <si...@teamup.ch>.
Let's start with the (hopefully) easy part: Documentation
Geir Magnusson Jr. wrote:
> Gabe wrote:
>>- Setup a library of context tools and corresponding documentation.
>> Named it VelLibrary. Added Nathan's contributed context tools to
>> the VelLibrary and modified them to make them thread-safe. The
>> directory structure is as discussed earlier:
>>
>> /jakarta-velocity-tools/tools/
>> xdocs
>> src/
>> docs/
>> lib/
>> build.xml
>>
>> Included in VelLibray are now four tools: MathTool, DateTool,
>> RequestParser and ToolLoader. Added all tools to package
>> o.a.v.tools.tools. This is not commited yet. Awaiting Geir's
>> approval to check-in the structure as proposed above!
>>
> That' looks great. But where does the 'VelLibrary' moniker come into it?
Well, VelLibrary is a proposal. Here's my take on the whole documentation
thing.
jakarta-velocity-tool currently contains several interesting things:
1) A general-purpose servlet that renders Velocity templates. It supports
automatic population of the context and a auto-loading tools.
2) A set of context tools that achieve Velocity/Struts integration.
3) A hopefully growing colletion of various reusable context tools.
Depending on where you are coming from, you might be interested in one
or the other or multiple. For example, a Struts user would be interested
in 2) but he typically wouldn't care about the details of 1). A framework
developer might be interested in 1) and 3) but not 2), etc.
To address the documentation needs of the different users I took the
following approach:
<proposal>
- VelTools is presented as an umbrella project for the following three
subprojects:
- VelLibray: A collection of reusable and well documented context tools
- VelStruts: Velocity/Struts integration
- VelServlet: General-purpose Velocity template rendering servlet
- VelLibrary, VelStruts, VelServlet are my proposed names for the three
subprojects. Dividing the project into the three clearly identifiable
subprojects will make it more approachable and easier to document.
- Each subproject has it own, mostly independent set of documentation.
I think this is the best way to address the different needs of the
users. This way, a struts user can jump directly to the VelStruts
documentation. He doesn't need to know about VelTools or the other
subprojects, etc.
- The documentation of the VelTools umbrella project itself is limited to
a brief overview of the different subprojects. All the interesting stuff
is in the subprojects. Most users of sub-projects would never need
to know about VelTools.
</proposal>
Here is a snapshot of the current documenation to illustrate the
proposal:
If you are interested in the big picture, goto:
http://www.teamup.com/jakarta-velocity-tools/docs/index.html
If you are interested in Velocity-Struts integration, goto:
http://www.teamup.com/jakarta-velocity-tools/struts/docs/index.html
If you are interested in reuseable context tools, goto:
http://www.teamup.com/jakarta-velocity-tools/docs/vellibrary.html
If you are interested in a Velocity servlet, goto:
http://www.teamup.com/jakarta-velocity-tools/view/docs/velservlet.html
Makes sense?
Gabe
PS: More to follow...
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [VelTools] Changes
Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 4/2/02 12:17 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:
> Hi,
> I've just checked in a number of changes to the jakarta-velocity-tools
> project. Your feedback is welcome. Here's a brief outline of the
> major changes:
Yikes! What a pile. Incremental is good too :)
>
> - Introduced four interfaces that establish contracts between
> context tools and compatible toolbox managers:
> - ThreadSafeContextTool: for thread-safe context tool, tool
> instance is reused for the entire runtime
This one is weird - I don¹t remember if there was a conclusion, but wasn't
this something that was indicated by the toolbox config when you specified
the tool?
> - LogEnabledContextTool: for context tools that need access
> to a logging facility
> - ServletContextTool: for context tools that need access to
> the servlet envrionment
> - ContextContextTool: for context tools that need access to
> the Velocity context
> These interfaces are all optional. Any object with public methods
> and a public, empty constructor can be used as a context tool.
> Its handling may not be the most optimal one, though.
>
> - Introduced logging for context tools
>
> - Modified VelocityViewServlet so that it now can be configured
> with a configuration file. Proposed location is
> WEB-INF/velocity.properties. This allows now the use of a global
> macros file (default location WEB-INF/VM_global_library.vm)
How is that different than now? Isn't VelocityViewServlet configurable via
the config file?
>
> - Enhanced ToolboxManager
> - Made initialization of context tools thread-safe by
> synchronizing where needed.
> - Tools of unknown classes have now by default a life cycle
> of 'request' (based on Bill's input). This is required
> because otherwise we might run into problem with tools that
> are not thread-safe.
+1
> - Added support for the four interfaces listed above.
> - Improved logging and error handling.
>
> - Globally replaced the term 'scope' by the term 'life cycle'.
> This reflects more accurately what it is.
>
> - Setup a library of context tools and corresponding documentation.
> Named it VelLibrary. Added Nathan's contributed context tools to
> the VelLibrary and modified them to make them thread-safe. The
> directory structure is as discussed earlier:
>
> /jakarta-velocity-tools/tools/
> xdocs
> src/
> docs/
> lib/
> build.xml
>
> Included in VelLibray are now four tools: MathTool, DateTool,
> RequestParser and ToolLoader. Added all tools to package
> o.a.v.tools.tools. This is not commited yet. Awaiting Geir's
> approval to check-in the structure as proposed above!
That' looks great. But where does the 'VelLibrary' moniker come into it?
>
> - Defined a documentation structure and added a first draft of
> most pages. A current snapshot of the documentation is online
> at http://www.teamup.com/veltools Please have a look and let me
> know if it makes sense. I presented VelTools as an umbrella
> project for several small subprojects:
> - VelLibray: A colletion of reusalbe context tools
> - VelStruts: Velocity/Struts integration
> - VelServlet: Velocity template rendering servlet
> There is some overview documentation and each of the subprojects
> has a set of mostly self-contained documentation. My assumption
> is that most users wound't care much about the overview documentation
> and browse directly to the subproject that they are interested in.
Prollie right...
> - Renamed some of the classes to improve consistency.
>
> --
> Gabriel Sidler
> Software Engineer, Eivycom GmbH, Zurich, Switzerland
>
>
> --
> To unsubscribe, e-mail: <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
--
Geir Magnusson Jr. geirm@optonline.net
System and Software Consulting
Age and treachery will always triumph over youth and talent
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>