avalon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Tagunov <atagu...@mail.cnt.ru>
Subject Re[2]: [Patch] #13 resubmitted
Date Wed, 04 Jun 2003 09:05:35 GMT
Hello Berin!

AT> +    public void setContextDirectory( final URL url )
AT> Further investigation has shown that this patch _is_ needed.

BL> You can bind a File object directly like this:

BL> FortressConfig.setContextDirectory(
BL>      servletContext.getRealPath( "/WEB-INF/avalon" ) );

Yup, but as you say

BL> The thing is that some vendors like IBM
BL> WebSphere relocate everything and use a "classloader:" protocol.

I was speaking about exactly that!

In fact Servlet spec allowes everything to be run from an unpacked
.war file. It's a perfectly legitimate approach.

BL> We need to make it simple to work with--and either work with
BL> Files or URLs.

Justification for having both:

BL> SourceResolver--which can handle both URL and File.


BL> If you use the context root as a relative location for the
BL> log file (as is the default case), then you need a File, and
BL> not a URL.

A direct hit! That's why I would never use it for anything
not read-only.

We have a
    (File) servletContext.getAttribute( "javax.servlet.context.tempdir" )
for read-write access (and it's a guarateed File!) but it's not good for
logs either since the servlet container is allowed to wipe tempdir
on its restart.

So, I beleive that the location of the logs should be configured
some other way. Probably directly in a .xlog file or somewhere
in web.xml via some init parameter.

BL> The thing is I believe Fortress expects a File root, although
BL> it is primarily for the
BL> SourceResolver--which can handle both
BL> URL and File.

Well, I beleive that if a developer creates a web application
he should be aware that the applicatoin tree may probably be
read-only for him at runtime. (Or his webapp is not portable.).
If he makes a URL that may be readonly at runtime the context-root,
well, he's asked for it himself. He should know what he is doing.
Shouldn't he?

AT> and I get something like (writing by memory)
AT>     file:/C:/file/D:\goj\t331ah\webapps\tst\WEB-INF\avalon

BL> The problem here is a bug in the toExternalForm() method.
BL> This is most likely from your J2EE servlet container vendor.
BL> Notice that there is no colon after the second file?  There
BL> is also no protocol named "file/D" so that is why it fails.

Sorry about misleading you, Berin :-(
I beleive that it rather the following effect:

import java.io.File;

public class A{
    public static void main(String[] args) throws Exception
        final File inner = new File( "d:/foo.txt" );
        final String innerURL = inner.toURL().toExternalForm();
        final File doubleFile = new File( innerURL );
        System.out.println( doubleFile.toURL() );

this outputs

I have misleaded you by missing one colon in the offending output
(indeed I was in too a hurry and wrote that string by memory).

So it's all about
* Tomcat 3.1.1 (that's what my employer requires)
  returning file:D:<some-path> and FortressConfig
* me casting it to a string via .toExternalForm()
* FortressConfig creating new File( string )



    public void setContextDirectory( final String directory )
        m_context.put( ContextManagerConstants.CONTEXT_DIRECTORY, new File( directory ) );

will probably do something wrong with "classloader:...",
so I propose to have a

    public void setContextDirectory( final URL url )
        m_context.put( ContextManagerConstants.CONTEXT_DIRECTORY, url );


Maybe setContextDirectory() is not the best name as proposed in this
patch, maybe we should better to have

setContextDirectory( File )
setContextDirectory( String s ) { .. new File( s ) .. }
setContextURL( URL )
setContextURL( String u ){ .. new URL( u ) .. }


or maybe it would be better to stress we it's a context _root_

setContextRootURL( URL )
setContextRootURL( String url )


- Anton

Currently I do

        final Context defaultContext = FortressConfig.createDefaultConfig();
        m_initialFortressContext = new DefaultContext( defaultContext );
        final URL contextRoot = getOurContextRoot();
        m_initialFortressContext.put( ContainerConstants.CONTEXT_DIRECTORY, contextRoot );
        m_initialFortressContext.put( SERVLET_CONTEXT_CONTEXT_KEY, servletContext );
        final FortressConfig fortressConfig = new FortressConfig( m_initialFortressContext

so a workaround exists, moreover, it is necessary to do this to get
the servletContext in ( servlet logger factory from logkit will need
it), but still I beleive that it will be cleaner to add two
setContextURL() to FortressConfig

If approved will send a new patch - with two methods instead of one
and named setContextContextRoot( URL )

No compelling reason to do that before release,
(although servlet-writers like are likely to spend some time guessing
what they really should do :-)
just as the discussion continues
I'm sending in my replies :-)

Best regards,
 Anton                            mailto:atagunov@mail.cnt.ru

To unsubscribe, e-mail: dev-unsubscribe@avalon.apache.org
For additional commands, e-mail: dev-help@avalon.apache.org

View raw message