Bug 244807 - System view does not handle restore from cache
Summary: System view does not handle restore from cache
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 3.0.1   Edit
Hardware: PC Windows XP
: P2 major (vote)
Target Milestone: 3.0.1   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-08-21 08:59 EDT by Don Yantzi CLA
Modified: 2008-09-04 06:04 EDT (History)
2 users (show)

See Also:


Attachments
patch to check if restoring cache (1.92 KB, patch)
2008-08-21 11:48 EDT, David McKnight CLA
no flags Details | Diff
Restore from cache patch for SubSytsem class (2.36 KB, patch)
2008-08-22 08:39 EDT, Don Yantzi CLA
no flags Details | Diff
Subsystem patch (with Contribution statement) (2.97 KB, patch)
2008-08-22 08:52 EDT, Don Yantzi CLA
dmcknigh: iplog+
Details | Diff
This is an update to the original patch (requires the original patch be applied first) (940 bytes, patch)
2008-08-27 14:43 EDT, Don Yantzi CLA
dmcknigh: iplog+
Details | Diff
Patch adding Javadocs for checkIsConnected (1.98 KB, patch)
2008-09-02 14:10 EDT, Martin Oberhuber CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Don Yantzi CLA 2008-08-21 08:59:55 EDT
The RestoreRemoteObjects.doRestore method forces a connect when restoring a filter reference. It conditions this check based on offline flag, but doesn't check the ss.getCacheManager().isRestoreFromMemento() flag.

else if (object instanceof ISystemFilterReference)
{
     ISystemFilterReference fref = (ISystemFilterReference)object;
     ISubSystem ss = fref.getSubSystem();
     if (!ss.isOffline()){
         if (!ss.isConnected()){
	     try
	     {
>>	         ss.connect(monitor, false);
Comment 1 David McKnight CLA 2008-08-21 11:48:47 EDT
Created attachment 110582 [details]
patch to check if restoring cache

I don't know much about the cache manager.  Don, could you tell me if this patch helps?
Comment 2 Don Yantzi CLA 2008-08-22 08:39:19 EDT
Created attachment 110669 [details]
Restore from cache patch for SubSytsem class
Comment 3 Don Yantzi CLA 2008-08-22 08:42:29 EDT
Dave, your patch works for the SystemViewPart class, but there are some additional changes that were required for SubSystem (the resolveFilterStrings methods were forcing a connect as well).

I've attached a patch. Required disclaimer:

I, Don Yantzi, declare that I developed attached code from scratch,
without referencing any 3rd party materials except material licensed under the
EPL. I am authorized by my employer to make this contribution under the EPL.
Comment 4 Don Yantzi CLA 2008-08-22 08:52:07 EDT
Created attachment 110671 [details]
Subsystem patch (with Contribution statement)

As advised by Kevin, I've added the contribution statement.
Comment 5 David McKnight CLA 2008-08-22 09:05:04 EDT
I've committed the patches to cvs.
Comment 6 Martin Oberhuber CLA 2008-08-25 11:36:06 EDT
It looks like the Javadoc for

   ISubSystem#checkIsConnected(IProgressMonitor)

should also be updated to indicate that an implicit connect is _not_ done in case the subsystem is marked offline or there is a cache manager and the contents appears to be cached?
Comment 7 Don Yantzi CLA 2008-08-27 14:40:58 EDT
The original patch I submitted has a bug. The ! is misplaced before getCacheManager() and there is an extra one before the call to isRestoreFromMemento:

Old line:
if (!isConnected() && !isOffline()  && (!(getCacheManager() != null) && !getCacheManager().isRestoreFromMemento())) 

New line:
if (!isConnected() && !isOffline()  && !((getCacheManager() != null) && getCacheManager().isRestoreFromMemento())) 

So the second part of the check is:
- if cache manager is null then the statement !((getCacheManager() != null) && getCacheManager().isRestoreFromMemento()) evaluates to true.

- if cache manager is not null and isRestoreMemento is true then the overall expression is false and the if statement is skipped 

- if cache manager is not null and isRestoreMemento is false then the overall expression is true and the if statement is executed

I'll submit an updated patch that builds on the original patch.
Comment 8 Don Yantzi CLA 2008-08-27 14:43:04 EDT
Created attachment 111107 [details]
This is an update to the original patch (requires the original patch be applied first)
Comment 9 David McKnight CLA 2008-08-28 11:10:15 EDT
I've committed the updated patch to cvs.
Comment 10 Martin Oberhuber CLA 2008-09-02 14:10:58 EDT
Created attachment 111512 [details]
Patch adding Javadocs for checkIsConnected

Would attached patch for ISubSystem#checkIsConnected() be appropriate?
Comment 11 Martin Oberhuber CLA 2008-09-04 06:04:41 EDT
I committed my patch since our ramp-down plan doesn't require review yet:

[244807] Improve ISubSystem#checkIsConnected() docs for offline and cache manager cases