Allow DI to instanciate xxxFactory

[jfarcand]

A default constructor instead of the current one will do the work, with class creation delagated to AtmosphereObjectFactory

[slovdahl]

Commit 9371be0 makes it harder (or even impossible?) to inject AtmosphereResourceFactory with Guice. Below are the Guice module provider method I use with Atmosphere 2.2.3:

@Provides @Singleton
AtmosphereResourceFactory provideAtmosphereResourceFactory(AtmosphereGuiceServlet atmosphereGuiceServlet) {
    return atmosphereGuiceServlet.framework().atmosphereFactory();
}

The atmosphere-guice extension is configured to use the same injector as my application, which means that my provider method will be invoked when Atmosphere's GuiceObjectFactory tries to create an AtmosphereResourceFactory instance. And calling atmosphereGuiceServlet.framework().atmosphereFactory() will result in Guice invoking my provider method again and again. Voilá, a StackOverflowError occurs.

Removing the provider method makes Guice create a new AtmosphereResourceFactory instance every time an instance is injected, which obviously isn't desired.

I think there are two ways to work around this problem:

1. Make AtmosphereResourceFactory abstract and implement it in eg. DefaultAtmosphereResourceFactory. In that case AtmosphereFramework line 2988 could ask for an instance of DefaultAtmosphereResourceFactory. This is how BroadcasterFactory is implemented, and the reason why I'm able to inject BroadcasterFactory with Guice.
2. Annotate AtmosphereResourceFactory with the javax.inject.Singleton annotation. That way only one instance would ever be created by Guice.

Thoughts on this, @jfarcand?

[jfarcand]

DefaultAtmosphereResourceFactory has been added recently, as well as https://github.com/Atmosphere/atmosphere/blob/master/modules/cpr/src/main/java/org/atmosphere/cpr/AtmosphereFramework.java#L3028 . We cannot use Singleton has it will make a strong dependency on that library which I want to avoid.

[slovdahl]

Heh, didn't check the master branch. ;) Sorry for that. So can I backport the interface and DefaultAtmosphereResourceFactory in order to resolve my issue? Without the new Async API I guess?

And yes, I agree with you about Singletonas well!

[jfarcand]

Yes you can backport except the new API if that fixes Guice. Let me know once done as I'm planning to cut the release today (will test once your changes are in).

[slovdahl]

Of course it's not so easy..

1. Having a AtmosphereResourceFactory interface and a DefaultAtmosphereResourceFactory implementation makes it impossible to keep the AtmosphereResourceFactory.getDefault() method, and hence breaks backward compatibility.
2. Having an abstract AtmosphereResourceFactory class and a DefaultAtmosphereResourceFactory implementation would force DefaultAtmosphereResourceFactory to update a static field in AtmosphereResourceFactory every time a new DefaultAtmosphereResourceFactory is created.
3. Revert the changes made for this issue on the atmosphere-2.2.x branch.

I don't like breaking backward compatibility, so I suggest either solution 2 or 3. What do you think?

[jfarcand]

True 1 shoudn't be done. Go with 2 IMO as it keep backward compatibility.

[slovdahl]

What do you think about slovdahl@e0f8852? It's in my private fork, I'll push it if it looks okay.

[jfarcand]

+1