[jboss-dev] How to fix vfs?

[Ales Justin]

> On Tue, 2009-02-17 at 17:31 +0200, Dimitris Andreadis wrote:
>> https://jira.jboss.org/jira/browse/JBAS-6513
> 
> This should be fixed with Ales's latest changes?

Just about to check.

>> https://jira.jboss.org/jira/browse/JBCTS-900
>>
> 
> This is broken because of ...

There is some weird URL construction.
The "lib/common.jar" is actually on .ear not on .jar.
No idea who constructs this ...

> The main reason I originally looked at the VFS code was because
> it was unusable due to inconsistencies between paths (not URI/URLs)
> that may or not may not end with a /

Exactly.

I on the other hand ended up with some external things expecting "/" at 
the end.
If I remember correctly it was Seam's Facelets stuff.

Hence I changed that every non-leaf ends with "/" to make it consistent.

> Sometimes they did and sometimes they didn't and it was very random
> (even internally within the same VFS call).

Yup.
Some test had it some didn't.

> I changed the code so that the internal VFS paths always used the
> version without the trailing / which meant
> 1) It was always consistent
> 2) You didn't have to play around with complicated path matching
> or string manipulation within the critical (i.e. potentially slow)
> sections of the code.

I guess I can bring that back ...

> This change
> http://viewvc.jboss.org/cgi-bin/viewvc.cgi/jbossas/projects/vfs/trunk/src/main/java/org/jboss/virtual/VFSUtils.java?r1=83684&r2=83683&pathrev=83684
> and its current usage within the VFS make all that for nothing.

This can be changed as part of previous task.

> e.g. in DefaultVFSRegistry.getFile(URI)
> 
>       VFSContext context = getCache().findContext(uri);
>       if (context != null)
>       {
> 
> // THIS ALWAYS ADD AS A / TO THE END OF THE PATH
> // IT SHOULD NOT, ITS USED AS AN INTERNAL VFS PATH (SEE BELOW)
> 
>          String relativePath = VFSUtils.getRelativePath(context, uri);
>          for (TempInfo ti : context.getTempInfos())
>          {
>             String path = ti.getPath();
>             if (relativePath.startsWith(path))
>             {
> 
> // THIS WILL FAIL BECAUSE OF THE / ON THE INTERNAL VFS PATH
> // IT TRANSLATES TO root.getChild(path + "/")

I don't think this will fail,
as we mostly check tokens not exact path.
(in StructuredVirtualFileHandler; which is 99% of VFS handlers)

>                String subpath = relativePath.substring(path.length());
>                VirtualFileHandler child = findHandler(ti.getHandler(),
> subpath, true);
>                if (child != null)
>                   return child.getVirtualFile();
>             }
>          }
> 
> If you include the URL <-> URI mapping in that code, its also likely to
> be far too much string manipulation/searching to be performant under any
> kind of load.

How they say, impl first, optimize later? :-)

>>From the part of the stacktrace that is failing, there's at least 5
> potentially expensive operations always performed.
> 
> at
> org.jboss.virtual.plugins.registry.DefaultVFSRegistry.findHandler(DefaultVFSRegistry.java:107)
> 
> 1) stripProtocol => StringBuffer construction
> 2) substring in VFSUtils.getRelativePath()
> 3) relativePath.startsWith()
> 4) relativePath.substring()
> 
> at
> org.jboss.virtual.plugins.registry.DefaultVFSRegistry.getFile(DefaultVFSRegistry.java:81) 
> at
> 
> 5) URL -> URI mapping
> 
> org.jboss.virtual.plugins.registry.DefaultVFSRegistry.getFile(DefaultVFSRegistry.java:119) 
> at
> org.jboss.virtual.protocol.AbstractVFSHandler.openConnection(AbstractVFSHandler.java:71)

Again, part of optimization.
But I definitely agree, it should be changed.