Vojtech Szocs has posted comments on this change.

Change subject: webadmin,userportal: Synchronize refresh.
......................................................................


Patch Set 1:

(3 comments)

....................................................
File 
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/Frontend.java
Line 270:                     }
Line 271:                 } finally {
Line 272:                     raiseQueryCompleteEvent(queryType, 
callback.getContext());
Line 273:                     if (isEventQuery(queryType, parameters) && 
!((List<?>)result.getReturnValue()).isEmpty()) {
Line 274:                         raiseOperationCompleteEvent(operation);
> How would one distinguish between the EventListModel generating the call and 
> some other model generating the call? From the frontend perspective both are 
> identical.

Indeed, from Frontend class perspective, both are identical. This is why I 
thought we could put such logic into specific model provider instead - allowing 
us to put logic closer to the model itself.

For example, we could attach some listener to given model via model provider, 
i.e. EventModelProvider which is Provider<EventListModel> would override 
onCommonModelChange method and register listener to get notified when there's a 
change it model's items - we could use "LastEvent" property change for our 
purpose - if "LastEvent" property changes, we know there are new event items.

> This does raise the question will this generate an endless loop of refreshes 
> in the case of Hosts/Events sub tab.

Good point. Looking at the code, following can happen:

Trigger HostEventListModel search query via item selection change:
* user selects some host - EventListModel#calculateEntitiesChanged returns true
* EventListModel#onEntityChanged executes, which triggers 
HostEventListModel#onEntityContentChanged
* HostEventListModel calls getSearchCommand().execute() which performs "events: 
host.name=xxx" search query

Trigger HostEventListModel search query via refresh timer:
* EventListModel#timer has default refresh rate 5 seconds (I guess this is the 
same for all sub tab models)
* when this timer executes, HostEventListModel calls refreshModel which 
performs "events: host.name=xxx" search query

Long story short, I'd just suggest to put "operation complete due to event 
query complete" logic more closer to specific model (into model provider, 
outside Frontend class) so that we avoid problematic cases like 
HostEventListModel.

What do you think?
Line 275:                     }
Line 276:                 }
Line 277:             }
Line 278: 


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ListModel.java
Line 20: {
Line 21:     /**
Line 22:      * The GWT event bus.
Line 23:      */
Line 24:     private EventBus eventBus;
OK, fair enough :-)
Line 25: 
Line 26:     public static EventDefinition selectedItemChangedEventDefinition;
Line 27:     private Event privateSelectedItemChangedEvent;
Line 28: 


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
Line 388:                 setIsQueryFirstTime(true);
Line 389:                 syncSearch();
Line 390:                 setIsQueryFirstTime(false);
Line 391:                 getTimer().start();
Line 392:                 if (getEventBus() != null) {
Well, if we would set EventBus via SearchableListModel provider, the EventBus 
!= null invariant would never get broken.
Line 393:                     //Make sure to unregister any existing handler, 
before adding a new one.
Line 394:                     unregisterOperationCompleteHandler();
Line 395:                     //Register to listen for operation complete 
events.
Line 396:                     operationCompleteHandlerRegistration = 
getEventBus().addHandler(VdcOperationCompleteEvent.getType(),


-- 
To view, visit http://gerrit.ovirt.org/21057
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa59712f8f74426b0ca6a132664167f42fb8d7b2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to