[jira] [Comment Edited] (SLING-4685) Introduce generic discovery.commons.ViewStateManager sharable for impls

["Stefan Egli (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14521622#comment-14521622 ] 

Stefan Egli edited comment on SLING-4685 at 4/30/15 3:11 PM:
-------------------------------------------------------------

While I agree that the listener must always check if a view is current - when it receives the INIT event, at that very moment, the view must be current. 

The API in {{TopologyEvent.getOldView()}} says:
{quote}
This is null in case of <code>TOPOLOGY_INIT</code>
{quote}
so the old (ie not current) view must be null - while at the same time {{TopologyEvent.getNewView()}} says:
{quote}
Returns the view which is currently (i.e. newly) valid.
{quote}

So while the description of the INIT event might leave this aspect open, the getNew and getOldView methods clarify this enough IMO: an INIT can only contain a current view - thus *requires delay* (not exactly similar, but SLING-3750 also goes in this direction).

Re your point about
bq. CHANGED <- whenever the view has changed (the view could be "current" or "not-current" in case of isolated mode)).
I think that one goes similarly: CHANGED must *always* contain a new, valid view - that view cannot be 'not current' at time of the {{handleTopologyEvent}} call - that's the contract. That's not to say that as soon as the method quits, it cannot change to 'not current' - but at the invocation and duration of that method it must (and that was the intention of the API - that INIT and CHANGED are only sent once the view is stable).


was (Author: egli):
While I agree that the listener must always check if a view is current - when it receives the INIT event, at that very moment, the view must be current. 

The API in {{TopologyEvent.getOldView()}} says:
{quote}
This is null in case of <code>TOPOLOGY_INIT</code>
{quote}
so the old (ie not current) view cannot be null - while at the same time {{TopologyEvent.getNewView()}} says:
{quote}
Returns the view which is currently (i.e. newly) valid.
{quote}

So while the description of the INIT event might leave this aspect open, the getNew and getOldView methods clarify this enough IMO: an INIT can only contain a current view - thus *requires delay* (not exactly similar, but SLING-3750 also goes in this direction).

Re your point about
bq. CHANGED <- whenever the view has changed (the view could be "current" or "not-current" in case of isolated mode)).
I think that one goes similarly: CHANGED must *always* contain a new, valid view - that view cannot be 'not current' at time of the {{handleTopologyEvent}} call - that's the contract. That's not to say that as soon as the method quits, it cannot change to 'not current' - but at the invocation and duration of that method it must (and that was the intention of the API - that INIT and CHANGED are only sent once the view is stable).

> Introduce generic discovery.commons.ViewStateManager sharable for impls
> -----------------------------------------------------------------------
>
>                 Key: SLING-4685
>                 URL: https://issues.apache.org/jira/browse/SLING-4685
>             Project: Sling
>          Issue Type: Improvement
>          Components: Extensions
>            Reporter: Stefan Egli
>            Assignee: Stefan Egli
>             Fix For: Discovery Commons 1.0.0
>
>         Attachments: SLING-4685.patch
>
>
> With multiple discovery implementations existing/upcoming it starts to become valuable to share code that can be shared. And with the introduction of discovery.commons we have a nice place for this.
> As a first thing, I propose to introduce a discovery.commons.ViewStateManager (the name can be changed if wished of course): this one is capable of managing a number of TopologyChangeListeners, can be in deactivated/activated state and can react on {{handleChanging}} and {{handleNewView}} events and translates all of those to correct events that it sends to the registered listeners accordingly.
> This also takes into account the fact that the TOPOLOGY_INIT should be sent only when the first valid view is available - which is flagged to ViewStateManager via {{handleNewView}}.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)