Geir, You're quick!
> -----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. > > > 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 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.) 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? > > > - 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? :) 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]>
