I've applied the improvement. Martin Grigorov Wicket Training and Consulting https://twitter.com/mtgrigorov
On Tue, Oct 28, 2014 at 12:05 PM, Martin Grigorov <[email protected]> wrote: > Same for org.apache.wicket.atmosphere.AtmosphereRequestHandler > > Martin Grigorov > Wicket Training and Consulting > https://twitter.com/mtgrigorov > > On Tue, Oct 28, 2014 at 11:59 AM, Martin Grigorov <[email protected]> > wrote: > >> 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. >>>> > > > > >> > > > > >> >>>> > > > > >> > > > > > >>>> > > > > >> > > > > > >>>> > > > > >> > > > > >>>> > > > > >> > > > >>>> > > > > >> > > >>>> > > > > >> > >>>> > > > > >> >>>> > > > > > >>>> > > > > > >>>> > > > > >>>> > > > >>>> > > >>>> > >>>> >>> >> >
