Hi,
I see a simple way to improve this.
At
https://github.com/apache/wicket/blob/master/wicket-cdi-1.1/src/main/java/org/apache/wicket/cdi/ConversationPropagator.java#L207
there is a logic to not activate conversation
for BufferedResponseRequestHandler.
With the following check we can disable it for WebSocketRequestHandler too:
diff --git
i/wicket-cdi-1.1/src/main/java/org/apache/wicket/cdi/ConversationPropagator.java
w/wicket-cdi-1.1/src/main/java/org/apache/wicket/cdi/ConversationPropagator.java
index 7a85d98..738a7bc 100644
---
i/wicket-cdi-1.1/src/main/java/org/apache/wicket/cdi/ConversationPropagator.java
+++
w/wicket-cdi-1.1/src/main/java/org/apache/wicket/cdi/ConversationPropagator.java
@@ -36,6 +36,7 @@ import
org.apache.wicket.request.handler.resource.ResourceReferenceRequestHandle
import org.apache.wicket.request.mapper.parameter.PageParameters;
import org.apache.wicket.request.resource.PackageResourceReference;
import org.apache.wicket.util.lang.Args;
+import org.apache.wicket.util.lang.Classes;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -209,6 +210,8 @@ public class ConversationPropagator extends
AbstractRequestCycleListener
// we do not care about pages that are
being rendered from a
// buffer
return false;
+ } else if
("org.apache.wicket.protocol.ws.api.WebSocketRequestHandler".equals(Classes.name(handler.getClass())))
{
+ return false;
}
}
return true;
It is error prone for refactoring but there won't be dependency to
websocket-core.jar. I don't expect renaming WSRH anytime soon!
WDYT ?
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Sat, Oct 25, 2014 at 8:09 AM, Martin Grigorov <[email protected]>
wrote:
> Hi,
>
> Thanks for the investigation, John!
>
> WebSocket communication is not intercepted by servlet filters and DI
> frameworks cannot prepare their proxies for request and session scoped
> beans.
>
> This is documented in the guide.
>
> With ver. 4.1.0 Spring introduced support for scoped beans in their impl
> of web sockets. But it works on a higher level...
> On Oct 24, 2014 11:17 PM, "John Sarman" <[email protected]> wrote:
>
>> The last comment on that issue states:
>> It’s the responsibility of Websocket spec to implement these scopes. We
>> should ensure they take the point before closing the ticket.
>>
>> I personally would down vote the suggestion that wicket-native-websocket
>> should assume responsibility to implement the scopes, and just explicitly
>> state this feature is not supported.
>> Thanks for the current status of why this does not work.
>>
>> John
>>
>> On Fri, Oct 24, 2014 at 4:05 PM, Emond Papegaaij <
>> [email protected]>
>> wrote:
>>
>> > As far as I know, combining CDI's RequestScoped and SessionScoped beans
>> > with websockets is stil not supported by the spec. You can find
>> progress on
>> > this here: https://issues.jboss.org/browse/CDI-370
>> >
>> > Best regards,
>> > Emond
>> >
>> > On Fri, Oct 24, 2014 at 9:54 PM, John Sarman <[email protected]>
>> wrote:
>> >
>> > > Ok I was able to trigger a ContextNotActiveException. This exception
>> > > occurs in both M3 and snapshot. What I did to create the issue was to
>> > > create a SessionScoped service. This is the CDI part. I then Injected
>> > that
>> > > in to the HomePage.class. The service was simple, has two methods
>> > > increment and getCount. When I call increment inside an overridden
>> event
>> > > method of the WebSocketBehavior such as onMessage, I
>> > > get: org.jboss.weld.context.ContextNotActiveException: WELD-001303: No
>> > > active contexts for scope type javax.enterprise.context.SessionScoped.
>> > >
>> > > This proves the divorce claim, that is an Injected Scope based service
>> > will
>> > > not have an active context of said scope during a WebSocketEvent.
>> > > Just to confirm this I changed the Service scope to ApplicationScoped
>> and
>> > > it works fine. ApplicationScoped does not require a scope (request,
>> > > session, or conversation).
>> > >
>> > > At the surface what I found is that either the wicket-native-websocket
>> > > needs to depend on wicket-cdi-1.1 and setup the contexts during the
>> > > events(not recommended), or wicket-cdi-1.1 needs to somehow intercept
>> the
>> > > websocket events and activate the contexts (not even sure if this is
>> > > plausible), or (my recommendation) is that emond papegaaij and Igor
>> > > Vaynberg
>> > > look into this issue and come up with a proper solution.
>> > >
>> > > When cdi-1.1 was initially introduced into the codebase, the
>> activation
>> > of
>> > > scope was initially solved by requiring a dependency on weld.
>> Ultimately
>> > > Emond simplified this in the current commit of cdi-1.1 and added code
>> > > earlier in the cdi core to allow whatever cdi implementation
>> framework to
>> > > trigger the activation mechanism.
>> > >
>> > > Somehow this same triggering of context activation needs to occur in
>> the
>> > > websockets code so that if a javax.inject annotation service is called
>> > > during a websocket event that the proper context has been activated.
>> > >
>> > > Input is definitely needed from the Wicket team to prevent the two
>> > packages
>> > > from divorcing :)
>> > >
>> > >
>> > > --
>> > > John
>> > >
>> > >
>> > >
>> > >
>> > >
>> > > On Fri, Oct 24, 2014 at 3:02 PM, Sebastien <[email protected]> wrote:
>> > >
>> > > > Hi John, I will give you an example on monday...
>> > > > On Oct 24, 2014 8:46 PM, "John Sarman" <[email protected]>
>> wrote:
>> > > >
>> > > > > This works fine by overriding onMessage in your test class and
>> adding
>> > > > this
>> > > > > crude html to your html.
>> > > > >
>> > > > > java:
>> > > > > protected void onMessage(WebSocketRequestHandler handler,
>> TextMessage
>> > > > > message)
>> > > > > {
>> > > > > LOG.info(message.getText());
>> > > > >
>> > > > > }
>> > > > >
>> > > > > html:
>> > > > >
>> > > > > <button onclick="Wicket.WebSocket.send('message');">PRESS
>> ME</button>
>> > > > >
>> > > > >
>> > > > > Long story longer, I was able to use chrome console to hack a bad
>> > test,
>> > > > > Martin or other contributors, is there an up to date example for
>> the
>> > > > native
>> > > > > websockets, or a simple point me into the right direction to
>> better
>> > > > > understand how to implement a better real world use case, both to
>> > test
>> > > > the
>> > > > > WICKET-5733 and just to learn about the wicket way to implement
>> > native
>> > > > > websocket code.
>> > > > >
>> > > > > Thanks,
>> > > > > John
>> > > > >
>> > > > >
>> > > > > On Fri, Oct 24, 2014 at 2:17 PM, John Sarman <
>> [email protected]>
>> > > > wrote:
>> > > > >
>> > > > > > Yeah, I am just trying to trigger the onMessage at this point
>> > mainly
>> > > > just
>> > > > > > to learn more about the native websocket package at the moment.
>> > So I
>> > > > am
>> > > > > > just adding a button in html and adding some inline js to the
>> > > onclick.
>> > > > > > First attempt I copy pasted var ws = new Wicket.WebSocket();
>> > > > > > ws.send('message'); from the wiki into the onclick and got a
>> > > exception
>> > > > in
>> > > > > > the chrome console stating cannot send message while in the
>> > > connecting
>> > > > > > state. Assuming I need to learn more here, but I will keep
>> probing
>> > > at
>> > > > > this
>> > > > > > mainly as an effort to learn how to use the wicket native
>> > websockets.
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > On Fri, Oct 24, 2014 at 2:11 PM, Sebastien <[email protected]>
>> > wrote:
>> > > > > >
>> > > > > >> Hi John,
>> > > > > >>
>> > > > > >> If you get the "client connected" message and no stacktrace, we
>> > are
>> > > > > good I
>> > > > > >> guess :)
>> > > > > >>
>> > > > > >> I will investigate on my side given your previous answer...
>> > > > > >>
>> > > > > >> Thanks again,
>> > > > > >> Sebastien
>> > > > > >> On Oct 24, 2014 7:58 PM, "John Sarman" <[email protected]>
>> > > wrote:
>> > > > > >>
>> > > > > >> > Sebastian,
>> > > > > >> > Just updated my snapshots, clean built wicket, changed the
>> > > > dependency
>> > > > > to
>> > > > > >> > 7.0.0-SNAPSHOT and all seems well running on Wildfly. Is
>> there
>> > > > > >> something
>> > > > > >> > else I need to do to trigger your error. Currently page
>> comes
>> > up
>> > > > with
>> > > > > >> the
>> > > > > >> > label 7.0.0-SNAPSHOT. I even checked my .m2 repo to verify
>> my
>> > jar
>> > > > > files
>> > > > > >> > had proper timestamp, which they did. I went so far as to
>> clone
>> > > > > wicket,
>> > > > > >> > build, and test app. Is there a way to trigger the websocket
>> > > push,
>> > > > > >> because
>> > > > > >> > currently log file only states:
>> > > > > >> >
>> > > > > >> > 13:40:19,258 ERROR [stderr] (default task-1)
>> > > > > >> >
>> > > ********************************************************************
>> > > > > >> > 13:40:19,259 ERROR [stderr] (default task-1) *** WARNING:
>> Wicket
>> > > is
>> > > > > >> running
>> > > > > >> > in DEVELOPMENT mode. ***
>> > > > > >> > 13:40:19,259 ERROR [stderr] (default task-1) ***
>> > > > > >> > ^^^^^^^^^^^ ***
>> > > > > >> > 13:40:19,259 ERROR [stderr] (default task-1) *** Do NOT
>> deploy
>> > to
>> > > > your
>> > > > > >> live
>> > > > > >> > server(s) without changing this. ***
>> > > > > >> > 13:40:19,259 ERROR [stderr] (default task-1) *** See
>> > > > > >> > Application#getConfigurationType() for more information. ***
>> > > > > >> > 13:40:19,259 ERROR [stderr] (default task-1)
>> > > > > >> >
>> > > ********************************************************************
>> > > > > >> > 13:40:21,592 INFO [com.mycompany.HomePage] (default task-7)
>> > > Client
>> > > > > >> > connected
>> > > > > >> >
>> > > > > >> > where the relevant part is the last log statement stating
>> > > websocket
>> > > > is
>> > > > > >> > connected to client.
>> > > > > >> >
>> > > > > >> > I would like to see if the log message from onPush works
>> fine,
>> > but
>> > > > > >> unsure
>> > > > > >> > how to test this ATM.
>> > > > > >> >
>> > > > > >> > Hope this helps,
>> > > > > >> > John
>> > > > > >> >
>> > > > > >> >
>> > > > > >> >
>> > > > > >> > On Fri, Oct 24, 2014 at 12:03 PM, Sebastien <
>> [email protected]>
>> > > > wrote:
>> > > > > >> >
>> > > > > >> > > Hi,
>> > > > > >> > >
>> > > > > >> > > On Fri, Oct 24, 2014 at 6:00 PM, Martin Grigorov <
>> > > > > >> [email protected]>
>> > > > > >> > > wrote:
>> > > > > >> > >
>> > > > > >> > > > John,
>> > > > > >> > > >
>> > > > > >> > > > The quickstart uses 7.0.0-M3.
>> > > > > >> > > >
>> > > > > >> > >
>> > > > > >> > > Oops, right! I'm really sorry for that! I probably wanted
>> to
>> > > test
>> > > > a
>> > > > > >> last
>> > > > > >> > > time it was not repro in -M3...
>> > > > > >> > >
>> > > > > >> > >
>> > > > > >> > >
>> > > > > >> > > > Sebastien said that the problem is in 7.0.0-SNAPSHOT.
>> > > > > >> > > >
>> > > > > >> > > > Martin Grigorov
>> > > > > >> > > > Wicket Training and Consulting
>> > > > > >> > > > https://twitter.com/mtgrigorov
>> > > > > >> > > >
>> > > > > >> > > > On Fri, Oct 24, 2014 at 6:51 PM, John Sarman <
>> > > > > [email protected]>
>> > > > > >> > > wrote:
>> > > > > >> > > >
>> > > > > >> > > > > Sebastian,
>> > > > > >> > > > > I also deployed your quickstart in wildfly-8.0Final
>> with
>> > no
>> > > > > >> changes,
>> > > > > >> > > and
>> > > > > >> > > > > all seems well. As for the quickstart test, you would
>> > need
>> > > to
>> > > > > use
>> > > > > >> > > > cdi-unit
>> > > > > >> > > > > as a test dependency to enable the cdi aspects for the
>> > > > > >> wicket-tester.
>> > > > > >> > > To
>> > > > > >> > > > > successfully add that please look at the test section
>> in
>> > the
>> > > > > >> > > > wicket-cdi-1.1
>> > > > > >> > > > > code. In there you will find an extended version of
>> > > > > WicketTester
>> > > > > >> > that
>> > > > > >> > > > > properly starts and stops the different contexts.
>> Without
>> > > > > using a
>> > > > > >> > > > package
>> > > > > >> > > > > like cdi-unit or arquillian, I am afraid you will
>> always
>> > > get a
>> > > > > >> > Context
>> > > > > >> > > > > based exception because there is no cdi service
>> provider
>> > > > > >> activated.
>> > > > > >> > > > >
>> > > > > >> > > > > On Fri, Oct 24, 2014 at 11:15 AM, John Sarman <
>> > > > > >> [email protected]>
>> > > > > >> > > > > wrote:
>> > > > > >> > > > >
>> > > > > >> > > > > > I was able to start your test app in Glassfish4
>> without
>> > > > > changes.
>> > > > > >> > > > > >
>> > > > > >> > > > > > For Tomcat I added
>> > > > > >> > > > > > <dependency>
>> > > > > >> > > > > > <groupId>org.jboss.weld</groupId>
>> > > > > >> > > > > > <artifactId>weld-core</artifactId>
>> > > > > >> > > > > > <version>2.1.2.Final</version>
>> > > > > >> > > > > > </dependency>
>> > > > > >> > > > > > <dependency>
>> > > > > >> > > > > > <groupId>org.jboss.weld.servlet</groupId>
>> > > > > >> > > > > > <artifactId>weld-servlet-core</artifactId>
>> > > > > >> > > > > > <version>2.1.2.Final</version>
>> > > > > >> > > > > > </dependency>
>> > > > > >> > > > > > to the pom.
>> > > > > >> > > > > >
>> > > > > >> > > > > > I then added
>> > > > > >> > > > > > <listener>
>> > > > > >> > > > > > <!-- initialize Weld in servlet environment -->
>> > > > > >> > > > > >
>> > > > > >> > > > > >
>> > > > > >> > > > >
>> > > > > >> > > >
>> > > > > >> > >
>> > > > > >> >
>> > > > > >>
>> > > > >
>> > > >
>> > >
>> >
>> <listener-class>org.jboss.weld.environment.servlet.Listener</listener-class>
>> > > > > >> > > > > > </listener>
>> > > > > >> > > > > >
>> > > > > >> > > > > > to web.xml
>> > > > > >> > > > > >
>> > > > > >> > > > > > The homepage then loaded fine as well. If you look
>> in
>> > > > > >> > > wicket-examples
>> > > > > >> > > > > you
>> > > > > >> > > > > > can find these CDI related additions to the pom.xml
>> and
>> > > the
>> > > > > >> > web.xml.
>> > > > > >> > > > > >
>> > > > > >> > > > > > Hope this helps
>> > > > > >> > > > > >
>> > > > > >> > > > > > John Sarman
>> > > > > >> > > > > >
>> > > > > >> > > > > >
>> > > > > >> > > > > >
>> > > > > >> > > > > >
>> > > > > >> > > > > > On Fri, Oct 24, 2014 at 4:12 AM, Sebastien <
>> > > > [email protected]>
>> > > > > >> > wrote:
>> > > > > >> > > > > >
>> > > > > >> > > > > >> Hi Martin,
>> > > > > >> > > > > >>
>> > > > > >> > > > > >> On Thu, Oct 23, 2014 at 8:27 PM, Martin Grigorov <
>> > > > > >> > > > [email protected]>
>> > > > > >> > > > > >> wrote:
>> > > > > >> > > > > >>
>> > > > > >> > > > > >> > Ticket please.
>> > > > > >> > > > > >> > With quickstart will be processed sooner ;-)
>> > > > > >> > > > > >> >
>> > > > > >> > > > > >>
>> > > > > >> > > > > >> Fair :)
>> > > > > >> > > > > >> https://issues.apache.org/jira/browse/WICKET-5733
>> > > > > >> > > > > >>
>> > > > > >> > > > > >> Best regards,
>> > > > > >> > > > > >> Sebastien.
>> > > > > >> > > > > >>
>> > > > > >> > > > > >
>> > > > > >> > > > > >
>> > > > > >> > > > >
>> > > > > >> > > >
>> > > > > >> > >
>> > > > > >> >
>> > > > > >>
>> > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
