VFS cache as impl detail, to what extent

[alesj]

WRT:
- http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4203492#4203492

Afaics the VFS cache cannot be completely hidden from the user.
e.g. you want to create new roots that behave differently
- diff ExceptionHandler, diff options (copy, case-sensitivity, ...), ...
And that's exactly what VFS::getVFS(URI|URL) does.

Where I now see VFS::getRoot(URI|URL) just as a shortcut to
previously mentioned VFS::getVSF::getRoot to actually do work on
a more familiar VirtualFile api (as you can see, VFS has a bunch of deprecated methods).

What's the diff between VFS::getVSF::getRoot and VFS:getRoot?
It's in the step in the middle. ;-)
You can eventually do this (possible optimizations):

VFS vfs = VFS.getVFS(url);
// set some options on it
VFSUtils.enableCopy(vfs);
...
VirtualFile root = vfs.getRoot();

where "VirtualFile root = VFS.getRoot(url)" comes with defaults.

The much disputed VFS::getCachedFile is a cached counter weight to previous VFS::getRoot.
It will first check the cache, only then create a new root.
Most of the time we should be using this cached lookup,
unless you explicitly want new VFS root.

What might be a feasible plan is to introduce just the opposite method:
- VFS:createNewRoot(URI|URL)
and change the existing getRoot to use getCachedFile.

Wdyt?

[jaikiran]

"alesj" wrote:
Afaics the VFS cache cannot be completely hidden from the user.
e.g. you want to create new roots that behave differently
- diff ExceptionHandler, diff options (copy, case-sensitivity, ...), ...

I haven't read through any of previous discussions about VFS cache, so i might not be completely up to date about this. But, shouldn't these properties like the exception handler, the copy options be independent of the caching? Shouldn't the exception handler be configurable even when the cache is disabled?

[alesj]

"jaikiran" wrote:

But, shouldn't these properties like the exception handler, the copy options be independent of the caching? Shouldn't the exception handler be configurable even when the cache is disabled?

There is no connection to cache.
Re-read my post. ;-)

"alesj" wrote:

e.g. you want to create new roots that behave differently
- diff ExceptionHandler, diff options (copy, case-sensitivity, ...), ...
And that's exactly what VFS::getVFS(URI|URL) does.

The cache is not mentioned anywhere. :-)

[alesj]

"alesj" wrote:

There is no connection to cache.

OK, there is. :-)
If you use cache,
you inherit the configuration/options from the cache root.

[jaikiran]

"alesj" wrote:

"alesj" wrote:

There is no connection to cache.

OK, there is. :-)
If you use cache,
you inherit the configuration/options from the cache root.

I guess, that's what i am trying to understand :) So if i don't use cache, the configurations/options are not inherited?

[alesj]

"jaikiran" wrote:
So if i don't use cache, the configurations/options are not inherited?

They are always inherited.
e.g. children get it from the parent

Probably inherited isn't the right word there,
it should say "you get the options that were set at the root creation".
Where if you use that VFS::getVFS::(set-options)::getRoot steps, you can set your own options.

[alesj]

Moving Dimitris' comment here.

From the outside I understand that caching should be an internal implementation detail/optimization non visible to the standard user or accessible over the simplified api.

Before we had URLs, everybody knew how they worked, server was working, there was nothing to configure, everyone was happy. With VFS we have started talking about cashing and weird options, and locking problems and threads and reapers and timeouts and what not. This is a step back, not forward. Some of it might be necessary, but it should be mostly invisible to the end user.

The whole point is: we need to make it *simpler*. It has to be stable and fast for 80% of the cases. Our users shouldn't even know it exists - they shouldn't care and they shouldn't have to tinker with bean descriptors to make it work for them, except for rare circumstances.

That would make a successful design.

[alesj]

From the outside I understand that caching should be an internal implementation detail/optimization non visible to the standard user or accessible over the simplified api.

What can be more simple than VFS::getCacheFile? :-)

Before we had URLs, everybody knew how they worked, server was working, there was nothing to configure, everyone was happy. With VFS we have started talking about cashing and weird options, and locking problems and threads and reapers and timeouts and what not. This is a step back, not forward. Some of it might be necessary, but it should be mostly invisible to the end user.

Afaik, this was all there before.
My guess is, it was just scattered all over the place, hence you didn't notice it.

Reading Adrian's CL history overview supports my 'theory':
- http://www.jboss.org/community/docs/DOC-13267

A major motivation for this change is to move the handling of the file locking problems (seen on Windows) or file handle leaks (seen on Linux) into one place.

;-)

The whole point is: we need to make it *simpler*. It has to be stable and fast for 80% of the cases. Our users shouldn't even know it exists - they shouldn't care and they shouldn't have to tinker with bean descriptors to make it work for them, except for rare circumstances.

Unfortunately we only learn the details by examples.
But that's called OSS and community feedback. ;-)

[dimitris]

Why VFS::getCacheFile needs to be part of the API? One implementation could support caching, another one not.

[alesj]

Why VFS::getCacheFile needs to be part of the API? One implementation could support caching, another one not.

Sure, hence my proposed change:

Index: src/main/java/org/jboss/virtual/VFS.java
===================================================================
--- src/main/java/org/jboss/virtual/VFS.java (revision 83078)
+++ src/main/java/org/jboss/virtual/VFS.java (working copy)
@@ -158,6 +158,20 @@
 }

 /**
+ * Create new root
+ *
+ * @param rootURI the root url
+ * @return the virtual file
+ * @throws IOException if there is a problem accessing the VFS
+ * @throws IllegalArgumentException if the rootURL
+ */
+ public static VirtualFile createNewRoot(URI rootURI) throws IOException
+ {
+ VFS vfs = getVFS(rootURI);
+ return vfs.getRoot();
+ }
+
+ /**
 * Get the root virtual file
 *
 * @param rootURI the root uri
@@ -167,8 +181,9 @@
 */
 public static VirtualFile getRoot(URI rootURI) throws IOException
 {
- VFS vfs = getVFS(rootURI);
- return vfs.getRoot();
+ VFSCache cache = VFSCacheFactory.getInstance();
+ VirtualFile file = cache.getFile(rootURI);
+ return (file != null) ? file : createNewRoot(rootURI);
 }

 /**
@@ -182,12 +197,12 @@
 * @return the cached virtual file
 * @throws IOException for any error
 * @throws IllegalArgumentException if the rootURL is null
+ * @deprecated use getRoot
 */
+ @Deprecated
 public static VirtualFile getCachedFile(URI rootURI) throws IOException
 {
- VFSCache cache = VFSCacheFactory.getInstance();
- VirtualFile file = cache.getFile(rootURI);
- return (file != null) ? file : getRoot(rootURI);
+ return getRoot(rootURI);
 }

 /**
@@ -202,8 +217,8 @@
 @SuppressWarnings("deprecation")
 public static VirtualFile getVirtualFile(URI rootURI, String name) throws IOException
 {
- VFS vfs = getVFS(rootURI);
- return vfs.findChild(name);
+ VirtualFile root = getRoot(rootURI);
+ return root.findChild(name);
 }

 /**
@@ -225,20 +240,35 @@
 }

 /**
- * Get the root virtual file
+ * Create new root
 *
 * @param rootURL the root url
 * @return the virtual file
 * @throws IOException if there is a problem accessing the VFS
 * @throws IllegalArgumentException if the rootURL
 */
- public static VirtualFile getRoot(URL rootURL) throws IOException
+ public static VirtualFile createNewRoot(URL rootURL) throws IOException
 {
 VFS vfs = getVFS(rootURL);
 return vfs.getRoot();
 }

 /**
+ * Get the root virtual file
+ *
+ * @param rootURL the root url
+ * @return the virtual file
+ * @throws IOException if there is a problem accessing the VFS
+ * @throws IllegalArgumentException if the rootURL
+ */
+ public static VirtualFile getRoot(URL rootURL) throws IOException
+ {
+ VFSCache cache = VFSCacheFactory.getInstance();
+ VirtualFile file = cache.getFile(rootURL);
+ return (file != null) ? file : createNewRoot(rootURL);
+ }
+
+ /**
 * Get cached file.
 *
 * If VFSContext matching the rootURL parameter is cached
@@ -249,12 +279,12 @@
 * @return the cached virtual file
 * @throws IOException for any error
 * @throws IllegalArgumentException if the rootURL is null
+ * @deprecated use getRoot
 */
+ @Deprecated
 public static VirtualFile getCachedFile(URL rootURL) throws IOException
 {
- VFSCache cache = VFSCacheFactory.getInstance();
- VirtualFile file = cache.getFile(rootURL);
- return (file != null) ? file : getRoot(rootURL);
+ return getRoot(rootURL);
 }

 /**
@@ -269,8 +299,8 @@
 @SuppressWarnings("deprecation")
 public static VirtualFile getVirtualFile(URL rootURL, String name) throws IOException
 {
- VFS vfs = getVFS(rootURL);
- return vfs.findChild(name);
+ VirtualFile root = getRoot(rootURL);
+ return root.findChild(name);
 }

 /**

So, in non-cached env, createNewRoot == getRoot.

[dimitris]

I like it.

[alesj]

I like it.

Finally some positive VFS vibe from your side. :-)