Community
Participate
Working Groups
If you have a directory expanded and refresh it's parent the sub directory will be collapsed after the refresh. This happens only on Remote Systems not on Local. Steps to Reproduce: 1. Expand a directory inside another directory. 2. Refresh the parent of the directory you expanded. The sub directory has now been collapsed. -----------Enter bugs above this line----------- TM 2.0M7 Testing installation : eclipse-SDK-3.3M7 RSE install : I20070517-0600 java.runtime : Sun 1.5.0_06-b05 os.name: : Windows XP 5.1, Service Pack 2 ------------------------------------------------
This is a problem due to using deferred queries, and it's been like this since the very first version of openRSE as soon as deferred queries were activated. At some point, we chose to make deferred queries the default because they cannot ever block the UI. I have an idea of how to fix this, but I'm not sure if it will work out. The idea is that in the IContextObject, which we are passing to the adapters already now for getChildren(), we add another field for a callback (Runnable). Then the SystemDeferredTreeContentManager, when it is finished with the deferred query and showing the children, can invoke the callback. Receiver of the callback would be the SystemView, such that it is able to continue refreshing the children of the parent. Dave -- do you think this approach will work? I thought I might need such callbacks anyways, also in order to address bug #176461 (expand nodes to arbitrary level).
Solving this issue is likely to resolve bug 187285 as well (similar issue except it results from a cut and paste). Please inform me if there is any improvements as Dave has asked me to look into that one and see if I can familiarize myself better with the code.
Try to address for RC3
I just saw this one now although I must have glanced at it a while back. I haven't had a chance to think about this so I'm not sure what the best solution is. Callbacks may be the way to go - I guess that would be trying to use the callback after we call SystemView.add()?
I think this could go in SystemDeferredTreeContentManager.addChildren(): super.addChildren(parent, children, monitor); if (parent instanceof IContextObject) { IContextObject ctx = (IContextObject)parent; IRSECallback cb = ctx.getCallback(); if (cb!=null) { done(Status.OK_STATUS, children); } } _pendingQueries.remove(parent); And the SystemView, when assembling the IContextObject, would need to add the callback.
*** Bug 187285 has been marked as a duplicate of this bug. ***
*** Bug 191724 has been marked as a duplicate of this bug. ***
Changing this is too risky for 2.0 - deferring
Setting major since duplicate bug #191724 is major
Not really a major loss of functionality, but certainly a must-fix for early in 3.0
Dave - I won't get to this in 3.0.1 can you have a look?
(In reply to comment #11) > Dave - I won't get to this in 3.0.1 can you have a look? We currently don't have a getCallback() API in IContextObject. Martin, since we can't add API in 3.0.1 is the sort of solution you had in mind even possible for this release?
Created attachment 110369 [details] patch to defer expansion via callbacks This patch provides an internal implementation of IContextObject, ContextObjectWithViewer, that has additional info for the Viewer and an IRSECallback. When there are deferred queries and items need to be expanded after the query, we create a callback do to that (which gets called in SystemView.add() after creating children as a result of a query). Kevin, could you try out this patch?
Created attachment 110372 [details] updated patch I updated the patch because of a couple cleanups.
Together with this patch, I would recommend adding something like this to IContextObject's class Javadoc: noimplement - Extending or implementing this interface in client code is strongly discouraged, may break your code in a future major release, and will likely lead to degradation of RSE behavior. We could actually take this text as a blueprint for those interfaces where we "forgot" to add an actual API Tooling @noimplement tag in 3.0 -- see also bug 226561. When we go 4.0, we can replace the blueprint by an actual @noimplement.
Notes: 1. If I refresh a filter everything expanded under it is not expanded. 2. For our product's subsystem it seems to stop after refreshing the first level below the object to be refreshed. Works fine on Linux for me though.
I've updated the code to handle filters and added an additional check for determining whether an expanded child is a child of another expanded child (since the patch didn't work for the iseries objects subsystem sub children). Kevin, let me know if you need anything more to get this working for iseries objects.
Kevin, as per our discussion, I'm marking this fixed. The remaining iseries issues appear to be not with the RSE code.
In the SystemView#ExpandRemoteObjects callback, we are now iterating over a list of items that had been created when the callback was constructed. Given that it may take some time before the callback is called, shouldn't we check Widget#isDisposed() on each of the items that we are iterating over? - Eclipse could be in the process of shutting down, or the connection could have got disconnected before the callback is called. Reopening to clarify since this is critical for proper operation.
Also, why isn't the signature of SystemView#getContextObject() changed to return a ContextObjectWithViewer object rather than an IContextObject. This would make the cast to a ContextObjectWithViewer unnecessary for callers.
(In reply to comment #20) > Also, why isn't the signature of > > SystemView#getContextObject() > > changed to return a ContextObjectWithViewer object rather than an > IContextObject. This would make the cast to a ContextObjectWithViewer > unnecessary for callers. > I left the signature as IContextObject since we should really be using the interface. Due to the fact that we can't add API in 3.0.1, we were not able to change IContextObject to add the new getCallback() method - that will be added in 3.1. If we took the approach of making the signature of getContextObject() return ContextObjectWithViewer then we would end up having to change it back to IContextObject in the 3.1 release.
I've added an item.isDisposed() check to ExpandRemoteObjects.execute() for the event that somehow an item is removed before the callback gets called.
(In reply to comment #21) > in 3.1. If we took the approach of making the signature of getContextObject() > return ContextObjectWithViewer then we would end up having to change it back to > IContextObject in the 3.1 release. I cannot see why. SystemView is internal, so we can return whatever is most appropriate for us and therefore it makes most sense to return the most specific information we have -- the actual class. We don't need to care about interfaces at this point since we are on the implementation side.
(In reply to comment #23) > (In reply to comment #21) > > in 3.1. If we took the approach of making the signature of getContextObject() > > return ContextObjectWithViewer then we would end up having to change it back to > > IContextObject in the 3.1 release. > > I cannot see why. SystemView is internal, so we can return whatever is most > appropriate for us and therefore it makes most sense to return the most > specific information we have -- the actual class. > > We don't need to care about interfaces at this point since we are on the > implementation side. > I understand that SystemView is internal and therefore we can use what is most appropriate for us. I guess my question is, as a rule, if we have all the APIs we need in an interface, should we always use the specific implementation when we're in an internal class?
(In reply to comment #24) I'd say whatever we need. "Hiding" the real implementation behind an interface even from internal clients can be a good thing, because it allows changing the implementation without impacting too many clients. On the other hand, if we know we'll need to access stuff that's not exposed in the interface, better expose the class. Casting stuff is generally very bad practice since it costs performance, blows up the code, and sacrifices type safety. So if there is a rule of thumb, then the rule is: avoid casting wherever possible.
I've changed the signature of getContextObject() to return ContextObjectWithViewer and removed the unnecessary casts.
*** Bug 198651 has been marked as a duplicate of this bug. ***
