[
https://issues.apache.org/jira/browse/SLING-4685?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14521583#comment-14521583
]
Timothee Maret commented on SLING-4685:
---------------------------------------
Generally, big +1 for extracting common code and allow to share it.
Handling the view management and event dispatching in a single class makes
total sense to me
since events depend exclusively of view changes. Moreover, having a stateful
ViewManager allows
to keep track of the current view (which is shared to listeners via a
reference) which allow to
invalidate it reliably on view change (and by extension, make sure all
componenents that have
kept a reference on the view, see the same effect).
I have a suggestion to minimize the complexity. The patch buffers events until
a view is valid and thus the INIT event can be dispatched.
I think we may benefit from not buffering the events both in terms of
simplicity (no need to mamange the queue) and reliability.
Instead of sending an INIT event when the view is "current", we could send the
INIT event with whatever state the current view is in.
This would remove the need for a queue and would comply IMO with the
description of the INIT event which is
{quote}
Informs the service about the initial topology state - this is only sent once
at bind-time and is the first one a TopologyEventListener receives (after the
implementation bundle was activated).
{quote}
As usual, listeners have to look at the isCurrent state to figure out if the
view can be used or not. The INIT event would still carry information, notably,
the listener has been bound and everything is setup.
So, IMO, the events could go as follow
INIT <- whenever a listener is bound, only sent to the listener being bound
(the view could be "current" or "not-current")
CHANGING <- whenever a topology is changing (the view is always "not-current")
CHANGED <- whenever the view has changed (the view could be "current" or
"not-current" in case of isolated mode)).
The golden rule still remain, whenever a listener deals with a view,
independently of wherever it comes from, it must check whether the view is
current or not.
wdyt ?
> 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)
