On 1/9/02 12:50 AM, "Donnie Hale" <[EMAIL PROTECTED]> wrote:
> Geir, > > You're quick! So they say :) > >> -----Original Message----- >> From: Geir Magnusson Jr. [mailto:[EMAIL PROTECTED]] >> Sent: Wednesday, January 09, 2002 12:25 AM >> To: Velocity Developer's List >> Subject: Re: Enhancements for velocity-tools "view" >> >> >> On 1/9/02 12:06 AM, "Donnie Hale" <[EMAIL PROTECTED]> wrote: >> >>> I've attached some possible enhancements for the "view" piece of >>> velocity-tools. It's mainly modified servlet and webapp loader >> classes. The >>> modified servlet allows the webapp loader which is used to be >> configurable >>> based on a servlet <init-param> in web.xml. The webapp loader >> looks for an >>> <init-param> named "loader.page.prefix" to be used as a default root for >>> velocity templates. I've attached a web.xml file which has those two >>> parameters set. >>> >> >> I think this should be done in the conventional way via the engine >> configuration, as all other loaders are configured. Why the one-off? >> >> >> Next, if you want to submit a patch, great. However, your copy >> of my loader >> won't cut it as it's the same thing with some minor mods - I think the >> prefix idea is a good one (I've already mentioned that), but we >> should just >> patch what we have unless it's really different. > > > I wasn't sure at this point if you'd prefer a diff or a new version for, > perhaps, easier perusal. Diffs are good if the changes are small - if large, you are right - I personally like the whole thing... > > >> >>> To run the "simple" example: >>> >>> - drop the source files in the >>> view/src/java/org/apache/velocity/tools/view/servlet directory >>> - rebuild the velocity-tools-view .jar file >>> - drop the web.xml in the view/examples/simple/WEB-INF >> directory (rename the >>> existing one if desired) >>> - move (or copy) view/examples/simple/index.vm to a new "pages" >> directory >>> under WEB-INF (as specified in the <init-param> of the new web.xml file) >>> - re-war the simple example and redeploy it (or redeploy it however you >>> choose for your environment) >>> >>> That example failed for me when "index.vm" was in its original >> location (as >>> expected) and succeeded when I moved it to the new "pages" directory. >>> >>> Issues: >>> >>> - The webapp loader is still not as decoupled from the servlet >> as I'd like. >>> Geir - I think it would be a better approach if the >> servletConfig parameter >>> could be passed to the webapp loader's init() method in the >>> ExtendedProperties object that's passed to it rather than in an >> "application >>> attribute"; maybe that's just me. In any case, the servlet and >> the loader >>> need to agree on a property/attribute name for the >> servletConfig which isn't >>> related to either the webapp loader's classname or the >> servlet's classname. >>> Unfortunately, until the webapp loader gets that servletConfig >> object, it >>> has no way to find out any other parameterable values. >> >> We could do it either way, in the normal configuration, or via the app >> attribute. Saying it's not as decoupled from the servlet doesn't >> make sense >> - what is coupled? >> >> And if we keep the app attribute, what's wrong with the loader >> classname as >> the attribute? > > > If using the loader classname as the attribute, with it changing according > to which loader is configured, then that makes sense. For some reason my > mind was hung up on each loader needing to use the same attribute name. But > I see that just "loader.class.getName()" is OK in the loader for any loader, > and the servlet can use the loader classname from its configuration. The idea is this : When we configure loaders, we already have a convention for configuration : resource.loader = <loadername> [,<loadername2>...] <loadername>.resource.loader.class = {CLASSNAME} <loadername>.resource.loader.property = value etc So, for something like this, you would do resource.loader = webapploader webapploader.resource.loader.prefix = templates Note this is the expression of the 'configuration values' as properties. What really matters is how it's done in code via setProperty(), hence : Velocity.setProperty( "resource.loader", "webapp" ); Velocity.setProperty( "webapp.resource.loader.class", "org.apache.velocity.tools.view.servlet.WebappLoader" ); Now, I actually forget the reason why I used the app context, but it could just as well do something like Velocity.setProperty( "webapp.resource.loader.servletcontext", servletcontext ); And then the bit with the app context goes away. This does make it conform... > > >> >>> >>> - The WebappLoaderAppContext interface isn't used in this >> implementation, as >>> it exposes the ServletContext and it's really the ServletConfig >> that needs >>> exposed. You can get from ServletConfig to ServletContext, but not vice >>> versa; and you need ServletConfig to read <init-param>s for the servlet. >>> Anyway, I didn't want to change that interface for this; but it's easy >>> enough to do that if desired. >> >> I don't think that forcing it through the init-params is right - >> there is no >> reason to believe that is the only way people will configure it. For >> example, Turbine has it's own configuration repository, of which the >> velocity parameters are a subset, so Turbine inits Velocity using the >> parameters within it. >> >> You could imagine that others do it different ways. To explicitly tie an >> application parameter to web.xml rather than leave it up to the >> application >> (where thy could use <init-param> if they wanted) is too restrictive, I >> think, with little upside. > > That makes sense. Two thoughts come to mind, then. 1) Get those properties > in the ExtendedProperties passed to the loader's init() method. (Not sure > how that would be done from, for example, the servlet.) See above > 2) Define an > interface that a loader would use to get properties; and require loader > users to implement that interface as some kind of adapter to their config > repository. Then stuff that in the app attributes. Perhaps something like > that exists already? There is a 'contract' already... It's not a formal Interface contract, but serves us well so far. > > >> >>> - I took the opportunity to use "init()" in the servlet rather than >>> "init(config)" just to make sure that things worked as expected >> given all >>> the recent discussions about this issue on the user list. >> >> And we still didn't resolve the issue. >> >>> Let me know if there are any question. I hope the line >> terminators are right >>> and that I followed the coding standards. ;) >> >> Well, for some reason, member vars migrated to the bottom of the class... > > Oops. :( I likely missed that when perusing the standards. Definitely a > strong personal preference of mine, my thinking being that someone looking > at a class would start out caring quite little about the member variables > used in its implementation but would care deeply about how to construct one > and what the public interface is. Just one man's opinion, of course; I can > live with the alternative (guess I have to, don't I? :) That makes sense given your enthusiasm for the use of members in the 'contract' :) > > Donnie > > >> >>> Donnie >>> >>> -- >>> To unsubscribe, e-mail: > <mailto:[EMAIL PROTECTED]> >> For additional commands, e-mail: > <mailto:[EMAIL PROTECTED]> >> > > -- > Geir Magnusson Jr. [EMAIL PROTECTED] > System and Software Consulting > "We will be judged not by the monuments we build, but by the monuments we > destroy" - Ada Louise Huxtable > > > -- > To unsubscribe, e-mail: > <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: > <mailto:[EMAIL PROTECTED]> > > > > -- > To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: <mailto:[EMAIL PROTECTED]> > -- Geir Magnusson Jr. [EMAIL PROTECTED] System and Software Consulting "Whoever would overthrow the liberty of a nation must begin by subduing the freeness of speech." - Benjamin Franklin -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
