Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to configure Broadcaster cache with web.xml when using AtmosphereHandlerService Annotation #1871

Open
Darrelk opened this issue Feb 23, 2015 · 5 comments

Comments

@Darrelk
Copy link

Darrelk commented Feb 23, 2015

I am trying to add the UUIDBroadcasterCache to my application. In the documentation (https://github.com/Atmosphere/atmosphere/wiki/Understanding-BroadcasterCache) it says that I just need to add the following to the web.xml:

 <init-param>
   <param-name>org.atmosphere.cpr.broadcasterCacheClass</param-name>
   <param-value>org.atmosphere.cache.UUIDBroadcasterCache</param-value>
 </init-param>

So I added the above to my web.xml, but still the following message gets printed when I start the application:

 WARN  o.atmosphere.cpr.AtmosphereFramework - No BroadcasterCache configured. Broadcasted message between client reconnection will be LOST. It is recommended to configure the org.atmosphere.cache.UUIDBroadcasterCache

I did some debugging and discovered the following:

  • The framework first sets the Cache class from the web.xml , but does not use that.
  • It then goes trough the Handler annotations (AtmosphereHandlerServiceProcessor), then overrides the cache class with what ever is in there, so if there is nothing, it uses the "org.atmosphere.cache.DefaultBroadcasterCache"
@jfarcand jfarcand added the 2.3.0 label Feb 25, 2015
@jfarcand
Copy link
Member

@Darrelk The idea here is to add aboolean in AtmosphereFramework and use that flag in the xxxProcessor to discard the annoation's value. Just in case you want to contribute :-)

@elakito
Copy link
Contributor

elakito commented Apr 22, 2015

@jfarcand
While relabeling my own 2.3.0 issue to 2.3.1, I saw this 2.3.0 issue.
I haven't looked at the code, so I don't know how it looks inside, but
I'm wondering whether the code can be simply changed to not override the cache that is already explicitly set via web.xml. In that way, we wouldn't need an extra boolean flag.

@elakito elakito added 2.3.1 and removed 2.3.0 labels Apr 22, 2015
@jfarcand
Copy link
Member

@elakito I didn't wanted to touch that code for 2.3.0 as it can have side effets...but yes it should be simple and should be done in 2.3.1.

@jfarcand jfarcand removed the 2.3.1 label Jun 17, 2015
@Darrelk
Copy link
Author

Darrelk commented Sep 11, 2015

@jfarcand So to fix this issue, Which of the following solutions do you prefer?

  1. Add a new config setting like "org.atmosphere.cache.forceConfig":
    • When this config is true, it will use the setting from the web.xml file
    • When this setting is false or not specified, it will use the class specified in the annotation.
  2. Only use setting in web.xml if no annotation specified:
    • If "org.atmosphere.cpr.broadcasterCacheClass" is specified in the web.xml and no annotation class specified, then use the web.xml class.
    • If no web.xml class specified and no annotated class, use default.

I will implement and do a pull request when done.

@jfarcand
Copy link
Member

@Darrelk (2) is the best IMO. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants