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

Remove global state #11838

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -48,12 +48,6 @@ final case class GuiceApplicationBuilder(
// extra constructor for creating from Java
def this() = this(environment = Environment.simple())

/**
* Sets the configuration key to enable/disable global application state
*/
def globalApp(enabled: Boolean): GuiceApplicationBuilder =
configure(Play.GlobalAppConfigKey -> enabled)

/**
* Set the initial configuration loader.
* Overrides the default or any previously configured values.
Expand Down
Expand Up @@ -16,7 +16,7 @@ public class DependencyInjectedRoutingDslTest extends AbstractRoutingDslTest {

@BeforeClass
public static void startApp() {
app = new GuiceApplicationBuilder().configure("play.allowGlobalApplication", true).build();
app = new GuiceApplicationBuilder().build();
Helpers.start(app);
}

Expand Down
Expand Up @@ -54,10 +54,9 @@ class ScalaResultsSpec extends PlaySpecification {
playSession.data must_== Map("user" -> "kiki", "langs" -> "fr:en:de")
}

"bake cookies should not depends on global state" in withApplication("play.allowGlobalApplication" -> false) {
implicit app =>
Ok.bakeCookies(cookieHeaderEncoding, sessionBaker, flashBaker) must
not(beNull) // we are interested just that it executes without global state
"bake cookies should not depends on global state" in withApplication() { implicit app =>
Ok.bakeCookies(cookieHeaderEncoding, sessionBaker, flashBaker) must
not(beNull) // we are interested just that it executes without global state
}

"support a custom application context" in {
Expand Down
Expand Up @@ -21,19 +21,19 @@ class AnyContentBodyParserSpec extends PlaySpecification {
await(parsers.anyContent(maxLength).apply(request).run(Source.single(body)))
}

"parse text bodies for DELETE requests" in new WithApplication(_.globalApp(false)) {
"parse text bodies for DELETE requests" in new WithApplication() {
override def running() = {
parse("DELETE", Some("text/plain"), ByteString("bar")) must beRight(AnyContentAsText("bar"))
}
}

"parse text bodies for GET requests" in new WithApplication(_.globalApp(false)) {
"parse text bodies for GET requests" in new WithApplication() {
override def running() = {
parse("GET", Some("text/plain"), ByteString("bar")) must beRight(AnyContentAsText("bar"))
}
}

"parse empty bodies as raw for GET requests" in new WithApplication(_.globalApp(false)) {
"parse empty bodies as raw for GET requests" in new WithApplication() {
override def running() = {
parse("PUT", None, ByteString.empty) must beRight.like {
case AnyContentAsRaw(rawBuffer) =>
Expand All @@ -44,39 +44,39 @@ class AnyContentBodyParserSpec extends PlaySpecification {
}
}

"parse text bodies for HEAD requests" in new WithApplication(_.globalApp(false)) {
"parse text bodies for HEAD requests" in new WithApplication() {
override def running() = {
parse("HEAD", Some("text/plain"), ByteString("bar")) must beRight(AnyContentAsText("bar"))
}
}

"parse text bodies for OPTIONS requests" in new WithApplication(_.globalApp(false)) {
"parse text bodies for OPTIONS requests" in new WithApplication() {
override def running() = {
parse("OPTIONS", Some("text/plain"), ByteString("bar")) must beRight(AnyContentAsText("bar"))
}
}

"parse XML bodies for PATCH requests" in new WithApplication(_.globalApp(false)) {
"parse XML bodies for PATCH requests" in new WithApplication() {
override def running() = {
parse("POST", Some("text/xml"), ByteString("<bar></bar>")) must beRight(AnyContentAsXml(<bar></bar>))
}
}

"parse text bodies for POST requests" in new WithApplication(_.globalApp(false)) {
"parse text bodies for POST requests" in new WithApplication() {
override def running() = {
parse("POST", Some("text/plain"), ByteString("bar")) must beRight(AnyContentAsText("bar"))
}
}

"parse JSON bodies for PUT requests" in new WithApplication(_.globalApp(false)) {
"parse JSON bodies for PUT requests" in new WithApplication() {
override def running() = {
parse("PUT", Some("application/json"), ByteString("""{"foo":"bar"}""")) must beRight.like {
case AnyContentAsJson(json) => (json \ "foo").as[String] must_== "bar"
}
}
}

"parse unknown bodies as raw for PUT requests" in new WithApplication(_.globalApp(false)) {
"parse unknown bodies as raw for PUT requests" in new WithApplication() {
override def running() = {
parse("PUT", None, ByteString.empty) must beRight.like {
case AnyContentAsRaw(rawBuffer) =>
Expand All @@ -87,7 +87,7 @@ class AnyContentBodyParserSpec extends PlaySpecification {
}
}

"accept greater than 2G bytes. not Int overflow" in new WithApplication(_.globalApp(false)) {
"accept greater than 2G bytes. not Int overflow" in new WithApplication() {
override def running() = {
parse("POST", Some("text/plain"), ByteString("bar"), maxLength = Some(Int.MaxValue.toLong + 2L)) must beRight(
AnyContentAsText("bar")
Expand Down
3 changes: 0 additions & 3 deletions core/play/src/main/resources/reference.conf
Expand Up @@ -7,9 +7,6 @@
promise.akka.actor.typed.timeout=5s

play {
# Defines whether the global application is allowed
# Set this to true if you need to use Play.application, or any deprecated global helpers.
allowGlobalApplication = false

logger {
# This is a boolean configuration.
Expand Down
9 changes: 0 additions & 9 deletions core/play/src/main/scala/play/api/Application.scala
Expand Up @@ -134,15 +134,6 @@ trait Application {
* @return The injector.
*/
def injector: Injector = NewInstanceInjector

/**
* Returns true if the global application is enabled for this app. If set to false, this changes the behavior of
* Play.start to disallow access to the global application instance,
* also affecting the deprecated Play APIs that use these.
*/
lazy val globalApplicationEnabled: Boolean = {
configuration.getOptional[Boolean](Play.GlobalAppConfigKey).getOrElse(true)
}
}

object Application {
Expand Down
62 changes: 1 addition & 61 deletions core/play/src/main/scala/play/api/Play.scala
Expand Up @@ -29,8 +29,6 @@ import play.utils.Threads
object Play {
private val logger = Logger(Play.getClass)

private[play] val GlobalAppConfigKey = "play.allowGlobalApplication"

private[play] lazy val xercesSaxParserFactory = {
val saxParserFactory = SAXParserFactory.newInstance()
saxParserFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.EXTERNAL_GENERAL_ENTITIES_FEATURE, false)
Expand All @@ -45,31 +43,6 @@ object Play {
*/
private[play] def XML = scala.xml.XML.withSAXParser(xercesSaxParserFactory.newSAXParser())

private[play] def privateMaybeApplication: Try[Application] = {
if (_currentApp.get != null) {
Success(_currentApp.get)
} else {
Failure(
new RuntimeException(
s"""
|The global application reference is disabled. Play's global state is deprecated and will
|be removed in a future release. You should use dependency injection instead. To enable
|the global application anyway, set $GlobalAppConfigKey = true.
""".stripMargin
)
)
}
}

/* Used by the routes compiler to resolve an application for the injector. Treat as private. */
def routesCompilerMaybeApplication: Option[Application] = privateMaybeApplication.toOption

// _currentApp is an AtomicReference so that `start()` can invoke `stop()`
// without causing a deadlock. That potential deadlock (and this derived complexity)
// was introduced when using CoordinatedShutdown because `unsetGlobalApp(app)`
// may run from a different thread.
private val _currentApp: AtomicReference[Application] = new AtomicReference[Application]()

/**
* Sets the global application instance.
*
Expand All @@ -79,38 +52,10 @@ object Play {
* @param app the application to start
*/
def start(app: Application): Unit = synchronized {
val globalApp = app.globalApplicationEnabled

// Stop the current app if the new app needs to replace the current app instance
if (globalApp && _currentApp.get != null) {
logger.info("Stopping current application")
stop(_currentApp.get())
}

app.mode match {
case Mode.Test =>
case mode =>
logger.info(s"Application started ($mode)${if (!globalApp) " (no global state)" else ""}")
}

// Set the current app if the global application is enabled
// Also set it if the current app is null, in order to display more useful errors if we try to use the app
if (globalApp) {
logger.warn(
s"""
|You are using the deprecated global state to set and access the current running application. If you
|need an instance of Application, set $GlobalAppConfigKey = false and use Dependency Injection instead.
""".stripMargin
)
_currentApp.set(app)

// It's possible to stop the Application using Coordinated Shutdown, when that happens the Application
// should no longer be considered the current App
app.coordinatedShutdown.addTask(CoordinatedShutdown.PhaseBeforeActorSystemTerminate, "unregister-global-app") {
() =>
unsetGlobalApp(app)
Future.successful(Done)
}
logger.info(s"Application started ($mode)")
}
}

Expand All @@ -127,11 +72,6 @@ object Play {
}
}

private def unsetGlobalApp(app: Application) = {
// Don't bother un-setting the current app unless it's our app
_currentApp.compareAndSet(app, null)
}

/**
* Returns the name of the cookie that can be used to permanently set the user's language.
*/
Expand Down
Expand Up @@ -168,8 +168,7 @@ case class ActionCompositionConfiguration(
case class FileMimeTypesConfiguration(mimeTypes: Map[String, String] = Map.empty)

object HttpConfiguration {
private val logger = LoggerFactory.getLogger(classOf[HttpConfiguration])
private val httpConfigurationCache = Application.instanceCache[HttpConfiguration]
private val logger = LoggerFactory.getLogger(classOf[HttpConfiguration])

def parseSameSite(config: Configuration, key: String): Option[SameSite] = {
config.get[Option[String]](key).flatMap { value =>
Expand Down Expand Up @@ -303,10 +302,7 @@ object HttpConfiguration {
/**
* Don't use this - only exists for transition from global state
*/
private[play] def current: HttpConfiguration = Play.privateMaybeApplication match {
case Success(app) => httpConfigurationCache(app)
case Failure(_) => HttpConfiguration()
}
private[play] def current: HttpConfiguration = HttpConfiguration()

/**
* For calling from Java.
Expand Down
66 changes: 5 additions & 61 deletions core/play/src/test/scala/play/api/PlayGlobalAppSpec.scala
Expand Up @@ -9,95 +9,39 @@ import org.specs2.mutable.Specification
class PlayGlobalAppSpec extends Specification {
sequential

def testApp(allowGlobalApp: Boolean): PlayCoreTestApplication =
def testApp(): PlayCoreTestApplication =
PlayCoreTestApplication(
Map(
"play.allowGlobalApplication" -> allowGlobalApp,
"play.akka.actor-system" -> "global-app-spec",
"akka.coordinated-shutdown.phases.actor-system-terminate.timeout" -> "90 second",
"akka.coordinated-shutdown.exit-jvm" -> "off"
)
)

"play.api.Play" should {
"start apps with global state enabled" in {
val app = testApp(true)
Play.start(app)
Play.privateMaybeApplication must beSuccessfulTry.withValue(app)
Play.stop(app)
success
}
"start apps with global state disabled" in {
val app = testApp(false)
val app = testApp()
Play.start(app)
Play.privateMaybeApplication must beFailedTry
Play.stop(app)
success
}
"shut down the first app when starting a second app with global state enabled" in {
val app1 = testApp(true)
Play.start(app1)
val app2 = testApp(true)
Play.start(app2)
app1.isTerminated must beTrue
app2.isTerminated must beFalse
Play.privateMaybeApplication must beSuccessfulTry.withValue(app2)
Play.stop(app1)
Play.stop(app2)
success
}
"start one app with global state after starting another without global state" in {
val app1 = testApp(false)
Play.start(app1)
val app2 = testApp(true)
Play.start(app2)
app1.isTerminated must beFalse
app2.isTerminated must beFalse
Play.privateMaybeApplication must beSuccessfulTry.withValue(app2)
Play.stop(app1)
Play.stop(app2)
success
}
"start one app without global state after starting another with global state" in {
val app1 = testApp(true)
Play.start(app1)
val app2 = testApp(false)
Play.start(app2)
app1.isTerminated must beFalse
app2.isTerminated must beFalse
Play.privateMaybeApplication must beSuccessfulTry.withValue(app1)
Play.stop(app1)
Play.stop(app2)
success
}
"start multiple apps with global state disabled" in {
val app1 = testApp(false)
val app1 = testApp()
Play.start(app1)
val app2 = testApp(false)
val app2 = testApp()
Play.start(app2)
app1.isTerminated must beFalse
app2.isTerminated must beFalse
Play.privateMaybeApplication must beFailedTry
Play.stop(app1)
Play.stop(app2)
success
}
"should stop an app with global state disabled" in {
val app = testApp(false)
Play.start(app)
Play.privateMaybeApplication must beFailedTry

Play.stop(app)
app.isTerminated must beTrue
}
"should unset current app when stopping with global state enabled" in {
val app = testApp(true)
val app = testApp()
Play.start(app)
Play.privateMaybeApplication must beSuccessfulTry.withValue(app)

Play.stop(app)
app.isTerminated must beTrue
Play.privateMaybeApplication must beFailedTry
}
}
}
Expand Up @@ -134,7 +134,7 @@ package object templates {
* The code to statically get the Play injector
*/
val Injector =
"play.api.Play.routesCompilerMaybeApplication.map(_.injector).getOrElse(play.api.inject.NewInstanceInjector)"
"play.api.inject.NewInstanceInjector"

val scalaReservedWords = List(
"as",
Expand Down
6 changes: 6 additions & 0 deletions project/BuildSettings.scala
Expand Up @@ -444,6 +444,12 @@ object BuildSettings {
ProblemFilters.exclude[IncompatibleResultTypeProblem]("play.db.jpa.DefaultJPAApi.em"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("play.db.jpa.JPAApi.em"),
ProblemFilters.exclude[ReversedMissingMethodProblem]("play.db.jpa.JPAApi.em"),
// Remove global state
ProblemFilters.exclude[DirectMissingMethodProblem]("play.api.Application.globalApplicationEnabled"),
ProblemFilters.exclude[DirectMissingMethodProblem]("play.api.DefaultApplication.globalApplicationEnabled"),
ProblemFilters.exclude[DirectMissingMethodProblem]("play.api.inject.guice.GuiceApplicationBuilder.globalApp"),
ProblemFilters.exclude[DirectMissingMethodProblem]("play.api.Play.routesCompilerMaybeApplication"),
ProblemFilters.exclude[DirectMissingMethodProblem]("play.api.test.DefaultTestServerFactory.optionalGlobalLock"),
),
(Compile / unmanagedSourceDirectories) += {
val suffix = CrossVersion.partialVersion(scalaVersion.value) match {
Expand Down
Expand Up @@ -39,8 +39,7 @@ import play.api.routing.Router
)(createRouter: BuiltInComponents => Router): ApplicationFactory = withComponents {
val context = ApplicationLoader.Context.create(
environment = Environment.simple(),
initialSettings = Map[String, AnyRef](Play.GlobalAppConfigKey -> java.lang.Boolean.FALSE) ++ extraConfig
.asInstanceOf[Map[String, AnyRef]]
initialSettings = extraConfig.asInstanceOf[Map[String, AnyRef]]
)
new BuiltInComponentsFromContext(context) with NoHttpFiltersComponents {
override lazy val router: Router = createRouter(this)
Expand Down