jaliss / securesocial

A module that provides OAuth, OAuth2 and OpenID authentication for Play Framework applications
http://www.securesocial.ws
Apache License 2.0
1.19k stars 510 forks source link

Fix hardcoded execution context #511

Closed danielkza closed 9 years ago

danielkza commented 9 years ago

The default execution context (scala.concurrent.ExecutionContext.global) was hardcoded in multiple places in the code. This is not only inflexible, but problematic for Play applications, since it will cause code inside futures to not use the expected environment, like the custom class loader.

As as solution, introduce an executionContext member to RuntimeEnvironment, defaulting to the global context, that can be overridden by users as desired.

Services that need the contexts have been refactored to receive them once as a constructor parameter in their default implementations, or whenever possible, take them from another parameter that already has one (like another service). The only service that was not linked to any other and needs it's own definition of executionContext is AuthenticatorStore. To mitigate issues, it will get a context from the environment in the default implementation (through an implicit parameter), or default to the global context if not other override exists

danielkza commented 9 years ago

Sorry for the large changeset, but the uses were scattered all round the code and I needed to do a bit of refactoring to properly pass the context from service to service.

jeantil commented 9 years ago

First let me say this is a great idea, this has been bothering me for a long time too :)

I have some comments I will put directly in the changeset.

danielkza commented 9 years ago

I pushed up a new version with some of the suggested fixes. I went with the abstract class solution for AuthenticatorStore, at least temporarily, since it ended up much simpler. I also cleaned up some formatting issues, the compile-time formatter shouldn't need to fix anything anymore.

jeantil commented 9 years ago

cool :) thanks for all the work !

jaliss commented 9 years ago

thanks a lot @danielkza.