Possible ClassCastException when resolving AtmosphereResource

[gdrouet]

As far as I understand the code, this can produce a ClassCastException if the user set an ObjectFactory which does not return an AtmosphereResourceImpl

[jfarcand]

As ugly as it look, that's true except right now only one implementation of AtmosphereResource exists...legacy code, welcome to fix that mess :-)

[gdrouet]

Two options for us:

• we expose specific impl features on the interface
• we consider that no other implementation will be created in the future. IMHO, it will be the case for several reasons. I hunt the interfaces with only one implementation. We can remove the AtmosphereResource interface and rename AtmosphereResourceImpl to AtmosphereResource.

[jfarcand]

Ya but the issue with no interface is Spring will complains/not work. So we need to be careful IMO, but I'm all for it as it will greatly improve code readability.

[gdrouet]

Exactly, if user enables spring extension we will delegate instantiation to an ObjectFactory that uses an application context to create bean.

Any reason to create an AtmosphereResource using spring ? If not we can mark the class as a "reserved" class. The AtmosphereFramework will intercept any marked class and instantiate it itself instead of delegating this task to the ObjectFactory. Maybe other classes only managed by the framework are good candidate for it.

[jfarcand]

The AtmosphereResourceImpl contains extra method that are used by the framework itself and not exposed to an application. So we need to keep those hidden, hence I think we need to keep the Interface => Impl, but we should introduce a new interface for method that aren't exposed to the AtmosphereResource, but the framework could look at the "ControlInterface" and cast as needed. So what is really needed is a new interface, keep both AtmosphereResource and AtmosphereResourceImpl, but AtmosphereResourceImpl should implements the new interface. Thay way the code will be cleaner and all those downcast to AtmosphereResourceImpl removed. If an external framework decide to remove replace AtmosphereResourceImpl, then at least it will work and we can display warning in the log about the need to implement the ControlInterface as well. What do you think?

[gdrouet]

Was very strange the first time I read your comment but I finally understood what you want to do! Yes it's a very good idea :-)

[jfarcand]

Won't fix this issue as it is too risky for a 2.x changes. In 3.0 those issues are fixed.