Download Reference Manual
The Developer's Library for D
About Wiki Forums Source Search Contact

Ticket #1372 (assigned defect)

Opened 9 years ago

Last modified 8 years ago

VirtualFolder.folder does not work right for nested folders

Reported by: JarrettBillingsley Assigned to: kris (accepted)
Priority: normal Milestone: 2.0
Component: IO Version: 0.99.7 Dominik
Keywords: Cc:

Description

Consider the case where you have a root VirtualFolder?, and inside is a VirtualFolder? "foo", and inside that is any kind of folder named "bar".

If you do root.folder("foo"), what happens is that you get an error like "'' is not a recognized member of 'bar'".

Why does this happen? Because VirtualFolder?.folder does the following: it finds the first path separator, looks up the name before that in its list of mounts, and if found, calls .folder on that child folder with the remainder of the path as an argument. But there is no path separator in "foo". So it tries to call child.folder(""). This will obviously always fail.

What should it do instead? It should work like FileFolder?, returning an instance of a class that derives from VfsFolderEntry?. No such class for VirtualFolders? yet exists. That new class should implement the logic for looking up/creating sub-folders.

Attachments

vfolderpatch.diff (12.5 kB) - added by JarrettBillingsley on 11/23/08 19:43:45.

Change History

11/23/08 17:28:38 changed by kris

  • status changed from new to assigned.

might be worth changing line 226 to return 'child' when 'tail' is empty ... I'll have a go at it later today.

thanks Jarrett

11/23/08 18:19:48 changed by JarrettBillingsley

I've actually made a good deal of progress towards making VirtualFolders? work a lot like FileFolders? already. Why don't I get that done and post the patch?

11/23/08 19:43:45 changed by JarrettBillingsley

  • attachment vfolderpatch.diff added.

11/23/08 19:44:28 changed by JarrettBillingsley

The attached diff should fix this, as well as some other issues.

01/18/09 00:03:32 changed by larsivi

could you properly qualify this? the summary doesn't explain the issue you want to be solved I think

01/18/09 01:10:40 changed by kris

  • milestone changed from 0.99.8 to 0.99.9.

01/21/09 20:51:17 changed by JarrettBillingsley

OK, more or less the issue is that FileFolders adhere to the specification laid out in the VFS documentation, while VirtualFolders do not. For one, VirtualFolders perform subfolder lookup incorrectly, causing them to false negative (saying there is no subfolder when there obviously is one). For two, they throw an exception when there is no subfolder, which is incorrect - it should instead return a proxy object which returns "false" from exists(), as well as making it possible to create that nonexistent folder.

This is what the patch does. It creates some new proxy objects to make VirtualFolders work the same way as FileFolders, as closely as possible.

01/21/09 21:00:46 changed by kris

from what you describe, it sounds like the first part is a bug?

01/21/09 22:28:47 changed by JarrettBillingsley

Sure, but even if the false negatives are fixed, it still doesn't work right, since it'll still throw an exception if a subfolder really doesn't exist. To fix the entire problem, the proxy classes have to be introduced. Hence, the extent of the patch.

06/05/09 01:26:46 changed by JarrettBillingsley

Sigh, could I please get some feedback on this? I think I've justified the patch fairly soundly..

06/05/09 03:21:24 changed by kris

Honestly, I'm not seeing the problem :(

Can you provide some sample VfsFolder calls that illustrate the concern?

06/05/09 04:05:43 changed by JarrettBillingsley

	auto root = new VirtualFolder("root");
	auto foo = new VirtualFolder("foo");
	root.mount(foo);

	auto f = root.folder("foo"); // problem 1
	Stdout.formatln("{}", root.folder("bar").exists()); // problem 2

As explained in the original post, the line marked 'problem 1' this gives the error tango.core.Exception.VfsException: '' is not a recognized member of 'foo'. As explained in my followup post, this obviously buggy - yes, there is a subfolder named 'foo', and I didn't ask for a folder whose name is an empty string - but even if you fix that, the line marked 'problem 2' still behaves inconsistently with the spec by throwing an exception if you try to get a subfolder that doesn't exist. It should instead return a proxy object whose exists() function returns false. This is exactly what the patch does.

For contrast, consider you have a real directory structure as described here: in the current folder you have "foo" and nothing else. The following code works properly and in accordance with what is laid out in the docs:

	auto root = new FileFolder(".");

	auto f = root.folder("foo"); // gets me foo, since it exists, duh
	Stdout.formatln("{}", root.folder("bar").exists()); // prints 'false'

Note that the only thing that has changed is what 'root' is defined as.

My patch makes the first snippet do the same thing as the second.

(follow-up: ↓ 14 ) 06/05/09 04:50:58 changed by kris

Seems like you're assuming that, once constructed and assembled, a virtual folder should behave exactly like a concrete file folder? It doesn't, and was never intended to. Instead, it is a virtual aggregation of other folder instances - somewhat specialized.

You're quite right that it's not currently possible to extract a VirtualFolder from within another, post construction: auto f = root.folder("foo"). What good would it do? You can't mount anything useful into it since the agnostic type returned would not allow you to do so (yes, you could cast it, but that requires others to know that particular name is a virtual folder). The one valid case is extracting it in order to pass to another function as merely a VfsFolder, but that does not require the kind of conversion you seem to have in mind.

VirtualFolder was intended merely as a means to combine several discrete file systems together (local, remote, zip, ftp, whatever) into a single cohesive view. Its intent is as a path-oriented, pass-through mechanism only. The patch suggests that it should be something else entirely?

Perhaps Larsivi has some ideas on this, too. As an aside, Jarrett, please try to drop the sarcasm?

06/05/09 16:26:43 changed by larsivi

I think I can see some (less than obvious) usecases for this, and I don't see a good reason to restrict this behaviour, and I agree with the argumentation that it should adhere to the interface.

When it comes to mounting in the sub-folder, this could be done prior to mounting "foo" in "root", in which case that wouldn't be an issue. Also, finding out what the actual type is should be a solvable problem if necessary.

(in reply to: ↑ 12 ) 06/05/09 17:51:11 changed by JarrettBillingsley

Replying to kris:

Seems like you're assuming that, once constructed and assembled, a virtual folder should behave exactly like a concrete file folder? It doesn't, and was never intended to. Instead, it is a virtual aggregation of other folder instances - somewhat specialized.

I wasn't aware of that. I thought it would be much more useful if it behaved just like a real filesystem folder, but was handled entirely in memory.

You're quite right that it's not currently possible to extract a VirtualFolder from within another, post construction: auto f = root.folder("foo"). What good would it do? You can't mount anything useful into it since the agnostic type returned would not allow you to do so (yes, you could cast it, but that requires others to know that particular name is a virtual folder). The one valid case is extracting it in order to pass to another function as merely a VfsFolder, but that does not require the kind of conversion you seem to have in mind.

I don't have any conversions or virtual-folder-specific functionality in mind, I just want to be able to access subfolders and to query the structure of the virtual folder hierarchy like any other filesystem. "Does this path exist?" doesn't seem like an operation that should throw an exception in any circumstance, and it doesn't for any of the other VFS drivers.

VirtualFolder was intended merely as a means to combine several discrete file systems together (local, remote, zip, ftp, whatever) into a single cohesive view. Its intent is as a path-oriented, pass-through mechanism only. The patch suggests that it should be something else entirely?

Again, I wasn't aware that was what it was designed to be. Yes, I am suggesting that it be something more than that. I wasn't aware that making a suggestion would be met with such hostility, especially in a pre-release library!

11/09/09 03:16:19 changed by kris

  • milestone changed from 0.99.9 to 2.0.