Community
Participate
Working Groups
Expanding the "My Home" filter node is leading to an error. Expanding the "Root" filter works fine. org.eclipse.rse.services.clientserver.messages.SystemMessageException: An unexpected error occurred. at org.eclipse.rse.subsystems.files.core.servicesubsystem.FileServiceSubSystem.getRemoteFileObject(FileServiceSubSystem.java:264) at org.eclipse.rse.subsystems.files.core.subsystems.RemoteFileSubSystem.internalResolveFilterString(RemoteFileSubSystem.java:645) at org.eclipse.rse.subsystems.files.core.subsystems.RemoteFileSubSystem.internalResolveFilterStrings(RemoteFileSubSystem.java:468) at org.eclipse.rse.core.subsystems.SubSystem.resolveFilterStrings(SubSystem.java:2281) at org.eclipse.rse.internal.ui.view.SystemViewFilterReferenceAdapter.internalGetChildren(SystemViewFilterReferenceAdapter.java:464) at org.eclipse.rse.internal.ui.view.SystemViewFilterReferenceAdapter.getChildren(SystemViewFilterReferenceAdapter.java:283) at org.eclipse.rse.internal.ui.view.SystemViewFilterReferenceAdapter.getChildren(SystemViewFilterReferenceAdapter.java:291) at org.eclipse.rse.ui.operations.SystemFetchOperation.execute(SystemFetchOperation.java:431) at org.eclipse.rse.ui.operations.SystemFetchOperation.run(SystemFetchOperation.java:179) at org.eclipse.rse.ui.view.AbstractSystemViewAdapter.fetchDeferredChildren(AbstractSystemViewAdapter.java:2290) at org.eclipse.ui.progress.DeferredTreeContentManager$1.run(DeferredTreeContentManager.java:234) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)
Pressing "F5" after the expand failed now leads to the error node "File 'root' not found" below the "My Home" filter.
Created attachment 131017 [details] patch to connect() if there is no user home variable set I'm not all that familiar with the SftpFileService code so I don't know whether this is the best way to go about fixing this but, from what I can see, fUserHome isn't set because connect() has never been called. So one way to deal with this problem is to call connect(). Martin, is there a better way to fix this?
Hi David, I've applied the patch and now it prints "File 'root' not found" directly. The output to the console is: getUserHome user home=/root get file getting channel So it should have the correct home "/root"? Cheers, Uwe :)
(In reply to comment #3) > Hi David, > I've applied the patch and now it prints "File 'root' not found" directly. > > The output to the console is: > > getUserHome > user home=/root > get file > getting channel > > So it should have the correct home "/root"? > > Cheers, Uwe :) > In your case, the user home is supposed to be "/root"? Are you able to expand /root via the "Root" filter?
>In your case, the user home is supposed to be "/root"? Yes. >Are you able to expand /root via the "Root" filter? Yes. It works via the "Root" filter even if the subsystem haven't been connected already.
Created attachment 131147 [details] patch to call connect() on initService() Here's a different patch that calls SftpFileService.connect() from initService(), which was the original behaviour. The connect() code had previously been commented out due to bug 227135. Martin, could you explain why connect() needed to be taken out?
Created attachment 131228 [details] patch to call connect() on initService() and to detect root parent in getUserHome() Combinded patch for missing connect() in initService() and wrong parent detection in getUserHome()
Hi David, it still shows "File 'root' not found" if expanding the "My Home" filter. The connect() in initService() is called. In SftpFileService#getUserHome(), fUserHome is correctly "/root", but in lines 862/863, the leading / is removed. This leads to name="root" and parent="". IMO, the parent is incorrect here too and is contributing to the problem. If changing to code to detect "/" as parent additionally to the initService() fix, it works fine. I've attached a patch combinding both fixes for review.
(In reply to comment #6) > Martin, could you explain why connect() needed to be taken out? Javadocs of initService() explicitly specify that initService() is *not* meant to connect. I'd appreciate finding a different solution for the problem. I'll not be able to look at this before next week because I'm currently out of office.
(In reply to comment #9) > (In reply to comment #6) > > Martin, could you explain why connect() needed to be taken out? > > Javadocs of initService() explicitly specify that initService() is *not* meant > to connect. I'd appreciate finding a different solution for the problem. > This method gets called for each service after the connector service has connected. At this point in time, I would expect the service to be connected and functional. You wrote the javadoc for IService.initService() which states "not yet expected to open a connection to a particular remote system" but it's not clear whether you're referring to the connector service being connected or something else. Perhaps the javadocs need to be revisited.
Created attachment 132103 [details] patch with updated javadoc for IService As per discussion in the committer meeting, I've committed the patch for SftpFileService. Here I've attached updated javadoc for IService. The description is pretty simple but I'm not sure anything more needs to be said.
Marking this as fixed.
Dave, the commit is incomplete. It missing the change to SftpFileService, line 864. From String parent = fUserHome.substring(0, lastSlash); To String parent = lastSlash > 0 ? fUserHome.substring(0, lastSlash) : lastSlash == 0 ? "/" : ""; //$NON-NLS-1$ //$NON-NLS-2$ The connect alone does not solve the issue. As said before, only both changes together are doing the trick. Do you want me to commit this change directly?
Hm... in the lastSlash < 0 case, we have e.g. file = "fooBar" so what would we expect the parent of such a path to be? An empty parent ("") seems odd to me. Either "/" or "." would make more sense I'd guess. What is the semantics of such a relative path anyways?
(In reply to comment #13) > Dave, the commit is incomplete. It missing the change to SftpFileService, line > 864. > > From > String parent = fUserHome.substring(0, lastSlash); > > To > > String parent = lastSlash > 0 ? fUserHome.substring(0, > lastSlash) : lastSlash == 0 ? "/" : ""; //$NON-NLS-1$ //$NON-NLS-2$ > > The connect alone does not solve the issue. As said before, only both changes > together are doing the trick. Do you want me to commit this change directly? > Oh right! Thanks from reminding me. I've added this line and committed the change.
Dave, thanks. I've just verified that it works now. Martin, In reply to Comment #14: >Hm... in the lastSlash < 0 case, we have e.g. file = "fooBar" >so what would we expect the parent of such a path to be? An empty parent ("?) seems odd to me. Either ?/?or ?." would make more sense I?d guess. I don't know what a good default for lastSlash < 0 would be. Most logical seems ".". However, this case had not been handled before, substring would have been thrown an IndexOutOfBoundsException. As there are no bugzillas regarding such an exception, it might not run into this case at all.
(In reply to comment #16) > ".". However, this case had not been handled before, substring would have been > thrown an IndexOutOfBoundsException. As there are no bugzillas regarding such > an exception, it might not run into this case at all. I guess then I'd like to have an exception thrown (assertionFailed or IndexOutOfBounds) in order to make sure that invalid input (i.e. relative path) is cought as early as possible.
