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 <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
