Hi
BS>> As to clustering, see writeObject() and readObject() methods of
OmniFaces
BS>> o:socket SocketChannelManager class (on which Mojarra's f:websocket
BS>> WebsocketChannelManager class is largely based).
Please note I'm writing an implementation of f:websocket from scratch, so
I'm
trying do not see omnifaces code. I have checked the methods and anyway it
doesn't solve the problem related to Session migration, because this code is
not detecting when the Session instance has been migrated. But it supports
the
point that the token is not application scope, because you need to move
the related data using a session scope bean. But as I said, this is
implementation specific, doesn't change the spec.
BS>> As to dropping "connected", I don't understand why you consider it an
BS>> application wide setting instead of a socket specific setting. How do
you
BS>> want to let the client have control over manually opening a specific
BS>> socket via jsf.push.open("channel") during e.g. an onclick event only?
BS>> See also javax.faces.push.Push javadoc and o:socket showcase.
My mistake here, I just read this in the javadoc:
"... You can use the optional connected attribute to control whether to
auto-connect the web socket or not. ..."
But I was thinking about how the websockets connects in case of a
disconnection between the client and the server.
In other words, what I wanted to say is the way how the client attempts to
reconnect to the server is something that you would like to control using a
web config param. How many attemps, the timeout and so on. Maybe pass
these config as an array in jsf.push.init(...) is a good idea.
Please note I haven't implemented this attribute on my code yet, so I'm
reporting on the mailing list what I'm seeing. I have not checked the
showcase, because I do not want to bias my own judgment. The idea is
the documentation in the spec should be good enough to give an idea
about how it works. That's the way we ensure spec's quality. So, I have
not digested every single line, but I'm on the way.
regards,
Leonardo Uribe
2016-10-14 2:39 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
> Hi,
>
> As to clustering, see writeObject() and readObject() methods of OmniFaces
> o:socket SocketChannelManager class (on which Mojarra's f:websocket
> WebsocketChannelManager class is largely based).
>
> As to dropping "connected", I don't understand why you consider it an
> application wide setting instead of a socket specific setting. How do you
> want to let the client have control over manually opening a specific socket
> via jsf.push.open("channel") during e.g. an onclick event only? See also
> javax.faces.push.Push javadoc and o:socket showcase.
>
> Cheers, B
>
> On Fri, Oct 14, 2016 at 4:19 AM, Leonardo Uribe <leonardo.uribe_at_irian.at>
> wrote:
>
>> Hi
>>
>> BS>> I do agree that the full url should be generated by the server
>> BS>> and passed to the client. I will look into it.
>>
>> Ok, thanks. I just want to add that the "port" attribute is something that
>> should be set using a global web config param, because this is common
>> for all the application and depends on the deployment configuration.
>> Please also consider if "connect" can be a global web config param instead
>> an attribute too.
>>
>> BS>> The "token" is application scoped not session scoped.
>>
>> The logic behind this is something I have been thinking for a long time.
>>
>> Before start, I want to be clear that the suggested proposal is provide
>> a point in the spec to allow customize the token generation by security
>> purposes. The way how a token is used is considered an implementation
>> specific concern, but in order to enrich the understanding of this
>> component I'll put my thoughts about this here.
>>
>>
>> There is a precedent that suggest an application scope token is not a
>> good
>> idea. See:
>>
>> - https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-694
>> Current Flash specification breaks clustering
>> - https://java.net/jira/browse/JAVASERVERFACES-1449
>> Modify flash to support clustering
>>
>> The description says: "... Currently the flash implementation is entirely
>> stored in the Application scope. This makes it unclusterable. Make it so
>> data stored in the flash is cluster friendly. ..."
>>
>> Now, as you already know, f:websocket requires to register an Endpoint
>> and in that place there is no HttpSession (there is on handshake),
>> so from that point the only valid CDI beans are application scoped.
>> So, the active websocket Session instances should be registered in an
>> application scoped bean.
>>
>> There are two points to keep in mind:
>>
>> - Clustering relies on session replication, and application scope is not
>> replicated among nodes.
>> - Application scope means a potential for memory leaks, if the map does
>> not
>> have proper cleanup measures.
>>
>> If you make the token application scope, that means multiple views share
>> the
>> same token, but it also means there is no way to keep track of each
>> connection in the application scope bean. So, at the end you will end up
>> with
>> stale session instances, and that's not good.
>>
>> If the websocket lifetime is shorter than the view lifetime, it is only
>> natural that the token should be view scope. That means, for each
>> combination
>> of channel, scope and user it should be one channel token per view.
>>
>> The channel token is generated on render response time, so the way to do
>> it
>> is generate the token and store it in a view scope bean. If @PreDestroy is
>> properly implemented for view scope beans, we can rely on that to destroy
>> in
>> a ordered way the token and the associated websocket Session instances
>> from
>> the application scope bean. If the session is destroyed, @PreDestroy will
>> be called anyway, so with this we can avoid the memory leak.
>>
>> If you see the problem from a "cluster" point of view, if the token is
>> application scope and the session is migrated to another node, the token
>> will
>> not be registered in the application scope of the target node, so the
>> connection will not be accepted. Please note there is no CDI session scope
>> in Endpoint.onOpen, but there is on the handshake request, so the
>> solution
>> is to register the valid tokens on session scope, so they can be validated
>> later in that point. It could be variants to avoid store the tokens on
>> session scope, but the problem is this feature is "CDI centric", so it is
>> hard to initialize a FacesContext instance on the Endpoint. It can be
>> done,
>> but we need more "plumbing" to do it. In that case, we need a method
>> on ViewHandler to validate the websocket token (remember
>> ResponseStateManager is tied to RenderKit, and here there is view and
>> no renderkit reference).
>>
>> Websocket spec says this about clustering:
>>
>> "... Websocket implementations that are part of a distributed container
>> may
>> need to migrate websocket sessions from one node to another in the case
>> of a
>> failover. ..."
>>
>> This blows up any storage of Session into an application scope bean,
>> because
>> it says the session can be moved between nodes, so in that case, the
>> migrated
>> Session will not be registered into the application scope bean. But this
>> can be accepted, because probably in that case the websocket connection
>> could be reopened, and the problem silently dissapear. This could be a
>> potential websocket spec pitfall, where the spec says that the websocket
>> sessions can be moved between nodes, but does not provide a way to
>> identify
>> when this has happened and take proper actions (like register the Session
>> in
>> the application scope bean).
>>
>>
>>
>>
>> In conclusion, the important thing about the websocket token is that it
>> helps
>> to identify the connection and its associated properties. If two
>> f:websocket
>> declarations share the same channel, user and scope, both should have the
>> same token "at least" at view level. I think both implementations can
>> agree
>> on that assumption.
>>
>> No, thinking on jsf.push.init(...) I would like to propose this structure
>> for
>> the EG consideration:
>>
>> jsf.push.init(clientId, url, onopen, onmessage, onclose, behaviorScripts);
>>
>> removing "port" and "connected" (use web config parameters for that).
>>
>> The clientId helps to identify from which tag these properties belong, and
>> make possible to reuse the same websocket connection among f:websocket
>> declarations. I'll explain this in another email, this is a work in
>> progress,
>> but for now I'm sure we could agree with this function declaration.
>>
>> regards,
>>
>> Leonardo Uribe
>>
>> 2016-10-13 2:10 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
>>
>>> Hi,
>>>
>>> The "token" is application scoped not session scoped. I do agree that
>>> the full url should be generated by the server and passed to the client. I
>>> will look into it.
>>>
>>> Cheers ,B
>>>
>>> On Tue, Oct 4, 2016, 05:36 Leonardo Uribe <leonardo.uribe_at_irian.at>
>>> wrote:
>>>
>>>> Hi
>>>>
>>>> Following a more exhaustive review of f:websockets, I have found some
>>>> inconsistencies related to the way how a connection is specified.
>>>>
>>>>
>>>>
>>>> Please note there is no mention of jsf.push.init(...) in the
>>>> javascript documentation, but the package jsf.push is there.
>>>>
>>>> Here is an example:
>>>>
>>>> <script type="text/javascript">
>>>> jsf.push.init(8080,'clock?1e4ac202-2b21-434f-8431-de5d5e4d37fe',
>>>> socketOpen,socketListener,socketClose,true);</script>
>>>>
>>>> If you use "jsf" javascript package, every method there should be
>>>> specified
>>>> without exceptions. I know this feature is a "work in progress" but I
>>>> just
>>>> wanted to notice this fact.
>>>>
>>>>
>>>>
>>>>
>>>> Other thing to notice is there is a session token used to identify the
>>>> connection. This is something related to state management, so I was
>>>> expecting
>>>> a method inside ResponseStateManager, in the same way there is a method
>>>> there called:
>>>>
>>>> public String getCryptographicallyStrongTokenFromSession(FacesContext
>>>> context)
>>>>
>>>> to handle protected views. Something like:
>>>>
>>>> public String getCryptographicallyStrongToke
>>>> nForWebsocketSession(FacesContext context)
>>>>
>>>> It is only the token generation what we need here, not the websocket
>>>> session handling.
>>>>
>>>>
>>>>
>>>> At last, there is a problem with passing params to assemble the url used
>>>> for the connection in the client. It has been a long standing policy in
>>>> JSF
>>>> to generate urls on the server and pass them to the client. But in the
>>>> javascript documentation a new variable was added called:
>>>>
>>>> jsf.contextpath
>>>>
>>>> with description "... The result of calling
>>>> ExternalContext.getRequestContextPath(). ..."
>>>>
>>>> It looks like something recently added, so I guess it should be
>>>> something
>>>> related to jsf.push.init(...) script.
>>>>
>>>> The problem here is there is no way for the server to override this
>>>> path,
>>>> and in cases like portlets for example, it is necessary to provide a way
>>>> to override this part properly. So the url used to connect must be built
>>>> fully on the server and use the known ViewHandler - ExternalContext
>>>> pattern. Additionally, embed this variable in the js file breaks any
>>>> caching strategy. If the content is dynamic, embed it as an inline
>>>> script,
>>>> but in this case it is better to avoid that.
>>>>
>>>> regards,
>>>>
>>>> Leonardo Uribe
>>>>
>>>>
>>
>