Remove AUTOINITIALIZE env variable (#1824)

Since importing app no longer automatically initializes anything, we are able to clean up this conditional code.

src/palace/manager/api/app.py

@@ -1,5 +1,4 @@
 import logging
-import os
 
 import flask_babel
 from flask import request
@@ -17,7 +16,7 @@
     PalaceXrayProfiler,
 )
 from palace.manager.core.app_server import ErrorHandler
-from palace.manager.service.container import Services, container_instance
+from palace.manager.service.container import container_instance
 from palace.manager.sqlalchemy.flask_sqlalchemy_session import flask_scoped_session
 from palace.manager.sqlalchemy.model.key import Key, KeyType
 from palace.manager.sqlalchemy.session import SessionManager
@@ -66,24 +65,19 @@ def initialize_admin(_db: Session | None = None):
     ).value
 
 
-def initialize_circulation_manager(container: Services):
-    if os.environ.get("AUTOINITIALIZE") == "False":
-        # It's the responsibility of the importing code to set app.manager
-        # appropriately.
-        pass
-    else:
-        if getattr(app, "manager", None) is None:
-            try:
-                app.manager = CirculationManager(app._db)
-            except Exception:
-                logging.exception("Error instantiating circulation manager!")
-                raise
-            # Make sure that any changes to the database (as might happen
-            # on initial setup) are committed before continuing.
-            app.manager._db.commit()
+def initialize_circulation_manager():
+    if getattr(app, "manager", None) is None:
+        try:
+            app.manager = CirculationManager(app._db)
+        except Exception:
+            logging.exception("Error instantiating circulation manager!")
+            raise
+        # Make sure that any changes to the database (as might happen
+        # on initial setup) are committed before continuing.
+        app.manager._db.commit()
 
-            # setup the cache data object
-            CachedData.initialize(app._db)
+        # setup the cache data object
+        CachedData.initialize(app._db)
 
 
 def initialize_database():
@@ -113,6 +107,6 @@ def initialize_application() -> PalaceFlask:
         app.register_error_handler(Exception, error_handler.handle)
 
         # Initialize the circulation manager
-        initialize_circulation_manager(container)
+        initialize_circulation_manager()
         initialize_admin()
     return app

tests/manager/api/test_app.py

@@ -1,6 +1,11 @@
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
-from palace.manager.api.app import initialize_application
+import pytest
+
+from palace.manager.api.app import (
+    initialize_application,
+    initialize_circulation_manager,
+)
 from palace.manager.util.http import HTTP
 from tests.fixtures.database import DatabaseTransactionFixture
 from tests.fixtures.services import ServicesFixture
@@ -23,3 +28,30 @@ def test_initialize_application_http(
 
     # Make sure that the HTTP configuration was set
     mock_set_quick_failure_settings.assert_called_once()
+
+
+def test_initialize_circulation_manager(caplog: pytest.LogCaptureFixture):
+    with (
+        patch("palace.manager.api.app.app") as mock_app,
+        patch("palace.manager.api.app.CirculationManager") as mock_circulation_manager,
+        patch("palace.manager.api.app.CachedData") as mock_cached_data,
+    ):
+        # If app is already initialized, it should not be re-initialized
+        mock_app.manager = MagicMock()
+        initialize_circulation_manager()
+        mock_circulation_manager.assert_not_called()
+        mock_cached_data.initialize.assert_not_called()
+
+        # If app is not initialized, it should be initialized
+        mock_app.manager = None
+        initialize_circulation_manager()
+        mock_circulation_manager.assert_called_once()
+        mock_cached_data.initialize.assert_called_once()
+        assert mock_app.manager == mock_circulation_manager.return_value
+
+        # If an exception is raised, it should be logged
+        mock_app.manager = None
+        mock_circulation_manager.side_effect = Exception("Test exception")
+        with pytest.raises(Exception):
+            initialize_circulation_manager()
+        assert "Error instantiating circulation manager!" in caplog.text

tests/manager/api/test_authenticator.py

@@ -5,7 +5,6 @@
 
 import datetime
 import json
-import os
 import re
 from collections.abc import Callable
 from decimal import Decimal
@@ -1020,12 +1019,8 @@ def _geographic_areas(cls, library):
 
         # We're about to call url_for, so we must create an
         # application context.
-        os.environ["AUTOINITIALIZE"] = "False"
         from palace.manager.api.app import app
 
-        self.app = app
-        del os.environ["AUTOINITIALIZE"]
-
         # Set up configuration settings for links.
         library_settings.terms_of_service = "http://terms.com"  # type: ignore[assignment]
         library_settings.privacy_policy = "http://privacy.com"  # type: ignore[assignment]
@@ -1091,7 +1086,7 @@ def _geographic_areas(cls, library):
             finish=announcement_fixture.yesterday,
         )
 
-        with self.app.test_request_context("/"):
+        with app.test_request_context("/"):
             url = authenticator.authentication_document_url()
             assert url.endswith("/%s/authentication_document" % library.short_name)