Community
Participate
Working Groups
Created attachment 96942 [details] Synchronizes enablement state of initiallly enabled activitiy with enabledList Build ID: I20080330-1350 Steps To Reproduce: 1. Create an activity which is controlled by an expression which is always true. 2. Look directly after the startup at the enabled activities list. 3. Bug. The activities list doesn't contain the activity. More Information: The solution patch may look overdrawn simple, but I don't see any problems with it. What do you think?
Looking at the last bit of code: if (newRef && activity.isEnabled()) { + Map activityEventMap = new HashMap(); + activityEventMap.put(activity.getId(), activityEvent); + Set deltaActivityIds = new HashSet(); + deltaActivityIds.add(activity.getId()); + updateListeners(true, activityEventMap, deltaActivityIds, + previousActivityIds); + } I dont think that's necessary. The results of the calls to updateActivity() are aggregated and massaged into one ActivityManagerEvent. With this code you'll have a bunch of little events, one for each expression activity. I've removed that code and your test still works. Can you verify that this is needed and respin the patch if necessary please?
(In reply to comment #1) > Looking at the last bit of code: > > if (newRef && activity.isEnabled()) { > + Map activityEventMap = new HashMap(); > + activityEventMap.put(activity.getId(), > activityEvent); > + Set deltaActivityIds = new HashSet(); > + deltaActivityIds.add(activity.getId()); > + updateListeners(true, activityEventMap, > deltaActivityIds, > + previousActivityIds); > + } > > > I dont think that's necessary. The results of the calls to updateActivity() > are aggregated and massaged into one ActivityManagerEvent. With this code > you'll have a bunch of little events, one for each expression activity. I've > removed that code and your test still works. Can you verify that this is > needed and respin the patch if necessary please? Summary: OK, get rid of it. In detail: I'm really the last one who would insist on that feature. I just wanted to make absolute sure that every listener gets notified. Of course every test runs fine, since no listener-becomes-notified checks are included in the unit tests. But when I think about it: it's not really necessary. Since the method is a private function, and should only get called by methods which should care about updates themselves. It's just that it's an action (self adding to enabledList) which are the methods, which call the udpateActivity function, aren't totally aware of, since especially this update is normally done by themselves. But there is the enabledChanged variable, which is put into the returned ActivityEvent, so it should be OK to remove the notifier call.
Fix in HEAD accounting for the change I suggested.
Verified in I20080429-0100. Jan, if you have time, I'd appreciate it if you could verify as well.
(In reply to comment #4) > Verified in I20080429-0100. Jan, if you have time, I'd appreciate it if you > could verify as well. Verified in I20080429-0100.
