Since this is getting complicated, let me summarize where I am coming
from. I will answer Jason's comments inline after.
Premise : I don't think we should eliminate java.util.Properties as a
mechanism for initializing Velocity. This is not an internals agument -
we can do what we need to internally as long as we don't hang ourselves
in the process. It is a 'public interface' discussion, and I think our
public requirements and usage patterns should be as conventional as
possible if there is no downside to doing so.
I believe that the current options of initialization
Runtime.init() : uses defaults
Runtime.init( filename ) : defaults + props in the file
Runtime.init( Properties ) : defaults + props in the Properties
are complete, although I am all for an addition of a fourth
Runtime.init( Configuration ) : defaults + propse in the Configuration
(not impl yet)
if people want that.
I want it to be clear - it's not "Properties exclusive of all else" - as
a user (and as a consultant, recommending Velocity on commercial clients
), I think the Properties support is important.
Note also that orbiting this discussion is a set of methods that Turbine
uses for initialization.
Some reasons :
* java.util.Properties is a standard Java class that ever programmer is
familiar with
* java.util.Properties is already used in applications to hold
configuration information, so supporting this helps in integration into
existing systems. For example, Turbine uses a Properties (although it
uses a special API to set specific Velocity properties before init().)
* Using a standard properties mechanism like this makes Velocity more
immediately 'familiar' to people using Velocity. There isn't a new
class, no matter how easy, to learn, slightly lowering the learning
curve.
* we have supported using a Properies object for initialization since
November, and we haven't run into any problems
I believe that this issue came to be because:
* to support multiple template paths, it is necessary to have a
'multi-valued property' in order to have some sort of mechanism to hold
the path elements. There were three possibilities to express this:
# use a multi-valued property (quibble about separator later)
resource.loader.1.resource.path = <dir1>,<dir2>,<dir3>
# use distinct scalar properties
resource.loader.1.resource.path.1 = <dir1>
resource.loader.1.resource.path.2 = <dir2>
resource.loader.1.resource.path.3 = <dir3>
# use identical scalar properties
resource.loader.1.resource.path = <dir1>
resource.loader.1.resource.path = <dir2>
resource.loader.1.resource.path = <dir3>
and the one that can't be supported by a Properties, the third, was
chosen. The other two methods would work fine via a Properties, and
further have the benefit that it's clear to the casual reader what is
going on.
An argument was made that Turbine would have to change a little to deal
with this. I don't think it would be that hard, quite frankly, and am
offering my commitment to help work things out.
It's not entirely clear that Configuration as a 'SuperProperties' is
easy for anyone to just use - the issue with scalars being turned into
vectors when set again may be just a bug. I am not sure.
more inline...
Jason van Zyl wrote:
>
> "Geir Magnusson Jr." wrote:
>
> > Jason van Zyl wrote:
> > >
> > > "Geir Magnusson Jr." wrote:
> > >
> > > > [SNIP]
> > > > Can we just support both then? The 'same value key' problem is somewhat
> > > > artificial, because you and john wanted to do, for multiple paths,
> > > >
> > > > path = foo
> > > > path = bar
> > > > path = woogie
> > >
> > > Which is equivalent to
> > > path = foo,bar,woogie
> > >
> > > With the Configuration class, and we wanted it because we use
> > > that in turbine and the classes have been used for a long time and
> > > work.
>
> >
> > > >
> > > >
> > > > rather than
> > > >
> > > > path = foo:bar:woogie
> > > >
> > > > or
> > > >
> > > > path.1 = foo
> > > > path.2 = bar
> > > > path.3 = woogie
> > > >
> > > > (The latter way is nicer - you don't have to worry that the separator
> > > > is a valid character in the data )
> > >
> > > This will not work. The name of the property has to be the same in
> > > order to dynamically set the resource path. In Turbine we might
> > > do:
> >
> > Sorry. Of course I meant :
> >
> > resource.loader.1.resource.path.1 = foo
> > resource.loader.1.resource.path.2 = bar
> >
> > (just too lazy to type it)
> >
> > > 1)
> > > Runtime.setSourceProperty(FILE_RESOURCE_PATH, path1);
> > > Runtime.setSourceProperty(FILE_RESOURCE_PATH, path2);
> > >
> > > I do not want to have to do
> > >
> > > 1)
> > > Runtime.setProperty("resource.loader.1.file.resource.path.1", path1);
> > > Runtime.setProperty("resource.loader.1.file.resource.path.2", path1);
> > >
> > > I don't have a safe constant to work with, and the file loader might not be
> > > the first loader. If the defaults for the file resource loader get overriden
> > > then this code will break. 1) is always guaranteed to work.
> >
> > I still think that we should punt on these methods to Runtime since a
> > single call to init() can do all of this, and I think that hardwiring
> > FILE_RESOURCE_PATH as a constant isn't a good idea as it isn't safe
> > because it makes assumptions about the public.name of the loader...but
> > ignoring that, what do these really have to do with each other?
>
> We agreed that the file resource loader was a standard loader did we not?
> And as a standard loader the setting of its configuration can be set
> as it is done above. I think the class resource loader should be a standard
> loader too. With these two, I think 99% of most cases are handled.
We agreed it is the default loader specified in our default properties,
and if that changes, Turbine won't work. At the least, we should be
able to ask for the name of the 'default' loader from Velocity via a
method, or something like that - fewer assumptions. I am just worried
about what happens if something changes in the defaults....
> >
> > FILE_LOADER_RESOURCE_PATH isn't defined as
> > "resource.loader.1.file.resource.path". It is "file.resource.path",
> > which isn't a real property for configuring a loader. This is a
> > source-specific mechanism dependant upon the ResourceManager taking it
> > apart to deduce the public name of the loader. The other part
> > 'resource.path' is passed into the Configuration object handling that
> > loader, and it does the right thing, tacking it on the end. I do like
> > this hack, and the Configuration class seems rather nice, BTW.
>
> I don't consider it a hack, I think it provides the easiest way to
> embed velocity. No external velocity props is required, you
> have access to the standard loaders and you can set the
> config for the loader very easily using the constant.
All I mean is that there should be a deterministic way of doing this,
rather than relying on the public name of the file loader in the default
properties file to remain in synch with a constant in RuntimeConstants.
> >
> >
> > But, that said, making the props
> >
> > resource.loader.1.resource.path.1 = foo
> > resource.loader.1.resource.path.2 = bar
> >
> > would have no effect on that. It would require a change in loading the
> > Configuration to deal with the trailing integer, bit you already are
> > dealing with the complexity of the multiple resource loader definitions
> > and ordering.
>
> Yes it would have an effect. Turbine doesn't have a velocity properties
> file and depends on the the defaults that velocity provides. Now for
> Turbine I'm on top of the changes but with your method I would have
> to the
>
> Runtime.setProperties("resource.loader.1.resource.path.1", path1)
>
> But this requires people to dig through the source and find out
> what the defaults are to get the property right. Why should I
> have to know what that string is? I just want to set a resource
> path.
I believe you are relying on faith and prayer that the default
properties set and the string in RuntimeConstants stays the same -
futher, since the value of the constant FILE_RESOURCE_PATH is embedded
into Turbine's bytecode when compiled, if we change the value (not the
constant name, the value) in Velocity, then Turbine will have to be
recompiled. We know this - we spent hours chasing this down.
If there was a 'live' way of doing it, a method getDefaultLoader() or
something, then you could just do something like
for( int i = 0; i < mypatharr.length; i++)
Runtime.setProperties( Runtime.getDefaultLoader() +
".resource.path." + i, mypatharr[i]);
which doesn't seem too onerous. You could even hardwire a constant
RESOURCE_PATH to avoid the string.
Further, it would mean we could dump the whole setProperties() method,
and you could do something like :
Properties p = new Properties();
for( int i = 0; i < mypatharr.length; i++)
p.setProperty( Runtime.getDefaultLoader() + ".resource.path." + i,
mypatharr[i]);
Runtime.init(p);
Just as easy.
And anyway, this is a bit of a tangent. I bet we could fix it so we
keep the current hardwired token, use properties compatibile with a
Properties, and Turbine would be happy.
At least, we could make a runtime method addSourceProperty() or
something for the multi-valued ones.
> And if we decided to change the defaults, people who didn't stay
> on top of changes would find their apps broken.
Yep. And if we had a method to get this value, they wouldn't.
> So let me get this clear, apps that have no velocity properties
> are supposed use
>
> Runtime.setProperties("resource.loader.1.resource.path.1", path1)
>
> to set a resource path?
No. It should be deterministic - see above.
> >
> >
> > What I am saying is that changing the multi-path property to be as I
> > suggest would have *no* effect on the way Turbine prefers to load
> > things, or those Runtime methods in general. They wouldn't have to
> > change at all. The only thing that would change is how the
> > Configuration / Configurations / ExtendedProperties deals with the new
> > property spec.
>
> If I'm correct in how I think you want to specify a resource path
> then it certainly has an effect. But clarify this if I misunderstand.
I think what I wrote above re the for() loop should do that, but I am
happy to demonstrate how it can be done. I understand that you are
happy in Turbine depending upon the default properties in Velocity, and
I think thats fine. I do it all the time in other things.
I just think that this whole issue can be cleared up if we don't
hardwire the magic cookie for the default loader but make it accessable
via a method call [I think you would agree that is safer all around],
and make the multi-path property names distinguishable. Then Turbine
can stay the same, and our init() methods can stay the same.
> > >
> > > That's why I want to do the change now before we launch. Get
> > > rid of it now while we can!
> >
> > I just don't think we need to force everyone to use an entirely new
> > class (for everyone except Turbine - and currently, Turbine doesn't use
> > it to init Velocity, and the Velocity version isn't the same as the
> > Turbine version) to handle application-level properties specification
> > methods, since I believe (and am happy to prove) that we could easily
> > acommodate both.
>
> We're not forcing people to do anything, we haven't even released?
We would be forcing them to use a Configuration, as they couldn't use a
Properties.
> Sure you could accomdate it with free text definition of properties
> but I don't think that's a good thing. I think the constant is a much
> safer mechanism. Users of velocity will use the file loader that we
> provide and I think the way turbine uses velocity is the most
> convenient.
If you mean by 'free text', having people put in the whoel string, I
agree - 'free text' would be a bad idea.
As as for convenience, I disagree. And with a little tweak to the whole
FILE_LOADER constant business (see above), we could it could use a
Properties too. I have two clients using it in production, and both use
files or Properties to init.
And I am about to go work with a third today to integrate Velocity into
their existing production system. I will report back on how their
system deals with configuration parameters.
> How about we poll the users? I think the way turbine does it is easiest,
> you use properties. How about we ask the users?
No problem. There are more uses to this than Turbine. I am also a
user, and my clients are users :)
What are we going to ask? I think the question is important. This
whole thing exists (I believe) because of the choice made for the
multi-path property tokens, and I think we could fix it by changing
those.
> > I just don't see the downside - I think we can get everything we want at
> > once - arrange things so that we support what I suppose lots of Java
> > programmers (including the person who wrote TurbineVelocityService) use
> > for application configuration... java.util.Properties.
>
> If I have to use free text definition of properties then I definitely
> have a problem with it.
First, I don't want to, nor could I, force you as a Turbine user to do
anything you don't want to do, as long as Turbine doesn't force Velocity
in a direction away from general-ness, I don't care what Turbine does.
Second, Turbines method of init is a separate issue - if we can have
Properties-compaitible properties, and turbine can still use its
approach, would you be satisfied?
Just to be clear, how do you define 'free text'? Strings in code? I
agree - bad idea.
> >
> >
> > > > > >
> > > > > > Everything that we want to do could be achieved with the
> > > > > > java.util.Properties.
> > > > >
> > > > > I don't think so, the Properties class doesn't behave the way the
> > > > > Configuration class does and it's the Configuration class that has
> > > > > always been the core configuration class. To make a Properties
> > > > > class work with the Configuration class you would have to add
> > > > > a hack in the runtime to parse CSV lines when the Configuration
> > > > > class does this already.
> > > >
> > > > Or if we made a simple change to what a multi-path spec is, to
> > > >
> > > > path.1 = foo
> > > > path.2 = bar
> > > >
> > > > wouldn't the issue just go away?
> > >
> > > No, as explained above.
> >
> > I wasn't convinced - see above.
> >
> > > >
> > > >
> > > > > > If Configuration makes life easier internally to
> > > > > > justify the added code, then I am all for it, but I still think that we
> > > > > > should support the standard Properties object as a full functional
> > > > > > method of init.
> > > > >
> > > > > Just for the record, I don't consider the using a Properties object to
> > > > > init the runtime the standard way. The app that has been using Velocity
> > > > > the longest, turbine, has never used this method.
> > > >
> > > > And looking at TurbineVelocityService, it uses a Properties internally
> > > > (so some apps *do* use the Properties class for configuration
> > > > management), and knows where its properties file is. I think the
> > > > following code would make Turbine compatible with using j.u.Properties
> > > > to initialize :
> > >
> > > It doesn't work the way you think it does.
> >
> > Ok. I figured that I messed it up somewhere - The current Turbine code
> > is :
> >
> > /*
> > * Get the properties for this Service
> > */
> > Properties props = getProperties();
> >
> > String vProperties = TurbineServlet.getRealPath(
> > props.getProperty("properties", null));
> >
> > String vLog = TurbineServlet.getRealPath(
> > props.getProperty("log", null));
> >
> > String vTemplates = TurbineServlet.getRealPath(
> > props.getProperty("templates", null));
> >
> > [SNIPPING UNRELATED CODE FOR BREVITY]
> >
> > try
> > {
> > Runtime.setProperties(vProperties);
> > Runtime.setSourceProperty(Runtime.FILE_RESOURCE_LOADER_PATH,
> > vTemplates);
> > Runtime.setProperty(Runtime.RUNTIME_LOG, vLog);
> > Runtime.init();
> > encoding = Runtime.getString(Runtime.TEMPLATE_ENCODING);
> > Log.info("VelocityService: encoding=" + encoding);
> > }
> > catch(Exception e)
> > ...
> >
> > And I suggested, for placement in the try block :
> >
> > > >
> > > > Properties p = new Properties();
> > > > p.load( vProperties );
> > > > p.setProperty(Runtime.FILE_RESOURCE_LOADER_PATH, vTemplates);
> > > > p.setProperty(Runtime.RUNTIME_LOG, vLog);
> > > > Runtime.init();
> > >
> > > There is no velocity properties file.
> >
> > Well, you are setting a set of properties in the first line in the try
> > block - but I know that - you depend upon the velocity defaults.
>
> The velocity properties file is empty, that line will go away.
> Take a look at an example turbine app and you will see that.
How does it go away? That line is in the source code of the
TurbineVelocityService.
> What you are looking at will become
>
> Runtime.setResourceProperty(Runtime.FILE_RESOURCE_LOADER_PATH, path)
> Runtime.setProperty(Runtime.RUNTIME_LOG, log)
> Runtime.init()
>
> I think this is the simplest, most reliable way to embed velocity. I don't
> need to create an object to initialize. And for each other path I just
> add another line like the first. How much easier could it get?
Well, it could be
Runtime.setAdditionalResourceProperty( ....)
...
...
and then we can have Properties compatible tokens.
And doesn't the whole thing go away if you switch to jar'd templates and
the ClasspathResourceLoader for easy deployment in servlet containers?
It becomes :
Runtime.setProperty( Runtime.RUNTIME_LOG, log);
Runtime.init();
or
Properties p = new Properties()
p.setProperty( Runtime.RUNTIME_LOG, log);
Runtime.init( p );
and we could dump the whole Runtime.setProperty() /
setResourceProperty() machinery?
> >
> >
> > But yes, I see that it wouldn't work as suggested, because of the
> > dependence upon the default properties, so the first setProperty()
> > wouldn't do the right thing, as FILE_RESOURCE_LOADER_PATH isn't a real
> > property.
>
> Of course it's a real property? Shall we make people look up what the
> property is for the log file? In order to set it's value?
No - it's not a 'real property', it's a magic cookie of sorts. It's
value is 'file.resource.path'
Could I really drop that into a properties file? I don't think so - it
depends upon the special code in setResourceProperty() to deal with it.
The log file makes sense, as it isn't something dynamic nor dependent
upon a property value. We define our log property to be 'runtime.log',
and that's fixed.
The FILE_RESOURCE_LOADER_PATH depends upon the *value* of the 'real
property' resource.loader.N.public.name, I think.
Isn't that how it works?
> > You could keep the Turbine code the same, change the resource path
> > property to include the trailing integer, and people could still use
> > Properties.
>
> So something like this:
>
> Runtime.setResourceProperty(SOME_CONSTANT + ".1", path1)
>
> Here we are protected against changes in the defaults, but looks
> terrible compared to the method that will be used.
See above.
> or aere you suggesting:
>
> Runtime.setResourceProperty("resource.loader.1.resource.path.1", path1)
>
> which is just plain dangerous because the defaults may change. using
> free text to set a runtime property is not a good idea.
"dangerous because the defaults may change" ?
Turbine is 100% dependant upon the defaults not changing, and further,
will break if we decide to change the value of a string constant.
> >
> >
> >
> > > >
> > > >
> > > > It's one line of code more than the current implementation. It's
> > > > structurally the same : the current implementation just uses Runtime as
> > > > a Properties, and the above uses a Properties as a Properties.
> > >
> > > So why create another object if you don't have to?
> >
> > If we only accepted a Configuration object, wouldn't everyone have to
> > create a new Configuration object and load it from their Properties?
> > And we are talking init() time anyway...
>
> They would use it exactly as a properties file. I don't see
> the problem.
I know. <sigh> :)
> >
> >
> > > >
> > > >
> > > > Of course, there may be something subtle about Turbine where that
> > > > wouldn't work. I am not sure.
> > > >
> > > > If we got rid of those methods from Runtime, Runtime would get smaller,
> > > > and clean up the initialization semantics w/o undue burden on Turbine.
> > > > Further, in general one wouldn't be tempted to call
> > > > Runtime.setProperty() after init(), since those methods wouldn't exist.
> > >
> > > But we very well may want to in the future.
> >
> > I agree.
> >
> > > >
> > > >
> > > > The rule would be 1) optionally setup the properties (in a Properties
> > > > or Configuration) and then 2) call init() [with no arg for defaults,
> > > > with a filename for vel to read, with a Properties if you have one, or a
> > > > Configuration if you use that.]
> > > >
> > > > And I don't consider either the 'standard way' - I just see the
> > > > java.util.Properties as an easy way using a standard class that people
> > > > could already use in their own applications (like Turbine seems to) - so
> > > > they could just glom together the velocity properties with their
> > > > properties for easy configuration, and just use that.
> > > >
> > > > I think giving people the choice of
> > > >
> > > > init() : use defaults
> > > > init( filename ) : use filename
> > > > init( Properties ) :
> > > > init( Configuration );
> > >
> > > >
> > > > would make it more convenient.
> > >
> > > If the init(Props) is removed before we release then people won't
> > > know what they're missing. I would say deprecate the use of
> > > init(Props) to give people fair warning.
> >
> > All I am saying is that we don't have to - we could work it out, or I
> > believe we could and am willing to help, try to get this to work where
> > we *can* support init(Props).
> >
> > Is there any other downside other than the current bit regarding
> > multi-path, which I believe we can fix?
>
> I will not accept using free text for setting runtime properties, that
> is my objection.
Re 'free text' if you mean letting people put arbitrary strings into
things, I agree. But the constants are dangerous too - the best may be
a real configuration API.
> And I think we feel differently about the status
> of loaders. I feel that the file loader and the class loader deserve
> to be first class, they are core to velocity and providing constants
> for their use provides the best way to embed velocity.
No, I agree about first-class status, only because they would/will be
the most popular. I think that the constants are dangerous though... a
real API might be the way to do this.
> I don't need a properties file, but in order to accommodate them
> I would have to use a method initialization that I think is dangerous
> i.e. using free text to set the resource paths. That's the only way
> I see your way working. If you use a constant then your back
> doing what is already done. I think the constant is good and
> I think the idea of standard class loaders is good.
great. so we seem to agree on everything - if I can make Turbine happy,
not use 'free text', eliminate the 'Hail Mary' constant approach, have
Properties compatible multi-path properties, then all is well?
> I don't think making people use a Configuration class
> is really a big deal. If we can't get this sorted out before
> tomorrow night then we can just remove from the todo
> and concentrate on the other items for release.
We will have to fix something either way.
geir
--
Geir Magnusson Jr. [EMAIL PROTECTED]
Developing for the web? See http://jakarta.apache.org/velocity/
