Community
Participate
Working Groups
This problem is similar to bug #218491 but represents another issue. There are special cases in getRemoteFileObject to determine if the requested path represents the root folder on the given filesystem: public IRemoteFile getRemoteFileObject(String folderOrFileName) { ... if (fofName.equals("/")) { //special case for Linux } ... if (fofName.endsWith(":")) { //special case for Windows } ... } On WinCE the root folder is "\" and this method fails to construct IRemoteFile for it because there is no such "special case". As a result all copy operations with destination folder \ fail with NPE. I am going to release my file subsystem for WinCE (bug #214887) soon, it is very easy to reproduce the bug with it.
The knowledge whether some path is a root or not really should not be in the subsystem in this case, but in the Service. I think, though, that you should be able to just subclass FileServiceSubSystem, override the getRemoteFileObject() method to do what you need, an instantiate the subclassed one in your custom SubSystemConfiguration. It's a little bit more code but it should be possible (you'll need to subclass FileServiceSubSystemConfiguration as well in order to instantiate your custom subsystem). Because I think there is a workaround I don't think that this is a major issue but rather an API Change enhancement request that I'd like to see by M6. DaveM what do you think?
(In reply to comment #1) > The knowledge whether some path is a root or not really should not be in the > subsystem in this case, but in the Service. > I think, though, that you should be able to just subclass FileServiceSubSystem, > override the getRemoteFileObject() method to do what you need, an instantiate > the subclassed one in your custom SubSystemConfiguration. > It's a little bit more code but it should be possible (you'll need to subclass > FileServiceSubSystemConfiguration as well in order to instantiate your custom > subsystem). > Because I think there is a workaround I don't think that this is a major issue > but rather an API Change enhancement request that I'd like to see by M6. > DaveM what do you think? I agree that for now you can override getRemoteFileObject(). In M6 we can add an API to the service layer.
Thanks guys, I will go with the override approach for now. I do have a custom subsystem already, so it will be relatively small change.
Just want to let you know that I've found one more place where this API is needed - RemoteFile#getParentPathFor. This method is causing infite loop in my case.
Ok - in the case of RemoteFile, that's an abstract class so it's intended to be overridden by extenders anyways. I see two options here: a) make RemoteFile#getParentPathFor() protected to allow extenders to override it (currently it is private), or b) move the code for path handling into the Service, adding new API for things like boolean IFileService#isRoot(String aPath); String IFileService#getParentPath(String aPath); String IFileService#concat(String aFolder, String aName); Rather than having this in the Service, we could also try to encapsulate absolute pathes in an IRemotePath object rather than using String. In that case, we'd want something like this: interface IRemotePathFactory { IRemotePath getRemotePath(String absPath); } where IRemotePath would provide similar methods as Eclipse IPath, and we'd have class MyFileService implements IFileService, IRemotePathFactory { ... } One important difference between IRemotePath and IHostFile would be, that all operations on IRemotePath can be performed without being connected to the remote system, and they cannot fail -- because they don't rely on being connected [[NB: This assumption could perhaps be hard to maintain for services like SSH that can connect to EITHER UNIX or Windows so they don't know much about the kind of remote pathes]] The change could even be backward compatible, because for "old-style" IFileService implementations which do not implement the new interface we could just fall back to the current implementation. I'd think, though, that if we go with the IRemotePath interface we'd want to replace all instances of "String" for path names by IRemotePath instances -- something like this has been requested in bug 162950 already. Marking this as related to bug 162950 for now. If we want such a big API change we better do it soon, so I'm putting in Priority P2. On a completely different note -- Radoslav, have you thought about normalizing your pathes in the WinCEFileService already? Such that only the service would know about the native kind of pathes (like "\foo\bar.txt") but would always pass these pathes up in a normalized way (like "/foo/bar.txt")? Note that the current FTPService performs path normalization for VMS connections: e.g. Remote VMS [PARENT.DIR]FILE.TXT;1 --> normalized /PARENT/DIR/FILE.TXT Normalizing the pathes in the Service would certainly make it easier for the RSE Subsystems to deal with them. The little disadvantage is that a user performs copy&paste of a normalized path it cannot immediately be used in a tool you have outside RSE which does not know about normalization. What do you think?
Thinking about this again, the IRemotePath approach seems to have a lot of advantages: * Path Normalization in the Service, allows caching of pathes or using IRemotePath objects as a key in other caches * Can always present remote pathes in their native forms in the UI rather than normalizing to a UNIX format -- better user experience * Improved code re-use We might even want to introduce the IRemotePath interface for the generic subsystems. Pathes are not necessarily something that only exists for files: a Path is a serialized representation of a context. Even SWT TreeViewer has this concept: TreePath ILazyTreePathContentProvider In RSE, the concept of an "absolutePath" representing a remote object is pretty deep at the core of our subsystems: See String IRemoteObjectIdentifier#getAbsolutePath(Object anOjbect); The big disadvantage of Strings is that from just a String one cannot know how to find the parent object or child object - unless they are normalized to a standard format (e.g. always use "/" as separator, but how to escape "/" then). Moving from String to IRemotePath everywhere seems like a really big change. We might want to think about a gradual approach, keeping String in some places for now but converting between String and IRemotePath where needed (using the IRemotePathFactory if provided).
(In reply to comment #4) > Just want to let you know that I've found one more place where this API is > needed - RemoteFile#getParentPathFor. This method is causing infite loop in my > case. So - for the short term, you have two possible workarounds: a) Normalize your pathes in the Service -- converting from \some\path to /some/path in the Service already. b) private RemoteFile#getParentPathFor() is currently only called from getEncoding() so you can override getEncoding() to ensure the bad method isn't called. (a) seems the safer choice because if you keep passing "\a\b" kind of pathes into RSE you might find other similar bugs [which might be a good thing though].
> So - for the short term, you have two possible workarounds: > > a) Normalize your pathes in the Service -- converting from \some\path to > /some/path in the Service already. > > b) private RemoteFile#getParentPathFor() is currently only called from > getEncoding() so you can override getEncoding() to ensure the bad method > isn't called. > I prefer option b) for now, at least for the first version of my file subsystem. Currently I am able to perform almost all file operations from RSE using the native paths without problems except those I've reported. For the long term I like your idea about IRemotePath and I believe it is a real solution to this kind of problems.
I just noticed that FileServiceSubSystem is final class :( It seems that I have no choice but to use linux paths and normalize them in the FileService ...
Hum, making a class final is a hard measure which I also don't quite understand in this case. Perhaps the original idea was that all customizations should be done via the Service alone, but this bug shows it isn't possible in practice. DaveM -- I'd be in favor of relieving this limitation and removing the "final" clause unless you have any objections. For now, it looks like you fortunately have the other option of normalizing paths. How much effort do you think will it be adding the normalization? I'm wondering whether we should quickly remove the "final" limitation even for M5 in order to avoid unnecessary work on your end.
I am afraid that users will not be able to use native paths for creating file filters because in this case getRemoteFileObject is called directly without using the FileService.
(In reply to comment #10) > Hum, making a class final is a hard measure which I also don't quite understand > in this case. Perhaps the original idea was that all customizations should be > done via the Service alone, but this bug shows it isn't possible in practice. > DaveM -- I'd be in favor of relieving this limitation and removing the "final" > clause unless you have any objections. > For now, it looks like you fortunately have the other option of normalizing > paths. How much effort do you think will it be adding the normalization? I'm > wondering whether we should quickly remove the "final" limitation even for M5 > in order to avoid unnecessary work on your end. I'm okay removing the final clause for now. We originally put it there because we thought all customizations should occur in the service layer. Apparently, it's not so easy to handle this particular case.
That's correct. When you decide to use normalized paths, users need to use normalized paths in all places in RSE - including the filters they set up. However we decide to finally fix this, I think it's right to remove the "final" clause for now. I'm tracking this separately through bug 219098 in order to document that the API change is complete for M5.
I am in favor of an API change to use the IRemotePath approach. IMHO you need at least getting this new API in while still offering the String based API (to be deprecated /removed later). In the long run I think that the diversity of RSE will make the String based API always be brittle and cumbersome. As we can never anticipate every usecase bugs are unavoidable. The VMS/WINCE issues are a good example. The IRemotePath (IHostPath for local?) approach clearly leads to a better API and as Martin has mentioned even can improve implementations by supplying additional information.
For IRemotePath (or IHostPath), I think we can continue the discussion in Bug 221556.
I'm closing this defect. Discussions for the remote path idea should continue with bug 221556. *** This bug has been marked as a duplicate of bug 221556 ***
