Skip to content

[3.0] Removes SMF\Lang's dependency on the database - #9581

Merged
Sesquipedalian merged 3 commits into
SimpleMachines:release-3.0from
Sesquipedalian:3.0/lang_no_db
Aug 29, 2026
Merged

[3.0] Removes SMF\Lang's dependency on the database#9581
Sesquipedalian merged 3 commits into
SimpleMachines:release-3.0from
Sesquipedalian:3.0/lang_no_db

Conversation

@Sesquipedalian

Copy link
Copy Markdown
Member

Previously, trying to load any language strings required the theme to be loaded, which in turn required the database to be loaded. Now we simply skip the theme stuff if the database is not available.

Previously, trying to load any language strings required the theme to be loaded, which in turn required the database to be loaded. Now we simply skip the theme stuff if the database is not available.

Signed-off-by: Jon Stovell <jonstovell@gmail.com>
@Sesquipedalian Sesquipedalian added this to the 3.0 Alpha 5 milestone Aug 29, 2026
@Sesquipedalian Sesquipedalian added the Localization Language & internationalization label Aug 29, 2026
@Sesquipedalian

Sesquipedalian commented Aug 29, 2026

Copy link
Copy Markdown
Member Author

@albertlast, this should make it possible to write unit tests for SMF\Lang.

Testing some of the methods in SMF\Lang will still require the tests to add mock content to various properties of SMF\Config, but it should now be possible. (For example, testing SMF\Lang::censortText() will require adding mock content to SMF\Config::$modSettings['censor_vulgar'] and SMF\Config::$modSettings['censor_proper'].)

@albertlast

Copy link
Copy Markdown
Collaborator

Confirmed — I checked this by running both versions against tests/bootstrap.php, which is exactly the "no Settings.php, no database, no request" environment in question.

On release-3.0, in a fresh process:

load(General)                 FAIL  Error: Typed static property SMF\Db\DatabaseApi::$db
                                    must not be accessed before initialization @ Theme.php:1956
get(false)                    FAIL  (same)
getTxt('...', file: 'Themes') FAIL  (same)

On this branch every public method on the class works, and Lang::load('General') brings in 728 strings with no database behind it. Your point about mock content holds and costs little: censorText() needed nothing beyond censor_vulgar and censor_proper in Config::$modSettings, no other state.

One scope note for anyone reading later: most of SMF\Lang was already unit-testable. Running each method in its own process shows only the filesystem-touching ones were blocked — load(), get(), addDirs(), getTxt(…, file:), and txtExists(), which returned false rather than throwing, which is arguably worse. censorText(), sentenceList(), numberFormat(), formatText(), tokenTxtReplace(), setTxt() and getLocaleFromLanguageName() all passed before this PR. So what this unlocks is the file-loading half, and that is the half worth having.

Worth saying how I see the two halves fitting together. For code where a database genuinely cannot be avoided, #9347 (on top of #9345) extends the test system with an integration suite that drives a real installed forum, so that side gets covered rather than staying untestable. But a test that needs no database is far cheaper — the unit suite finishes in under a second, runs on every pull request on 8.4 and 8.5, and needs no Docker and no forum behind it. For a class like Lang, which is mostly string handling and file loading, keeping it on the database-free side is the right way round, so I would rather have this change than test Lang through a live forum. The two are complements.

Checked for fallout as well, all clean: full suite green; the install and upgrade path byte-identical, since under SMF_INSTALLING the Lang::addDirs(Config::$languagesdir) + Lang::get(false) sequence that Tools\Install and Tools\Upgrade use yields the same languages and the same 1032 strings before and after; and a sweep of eleven pages on a running forum returning 200s with an empty log_errors.

I have put the tests up as #9582, on top of this branch — a LangTest covering the file-reading half, and the TimeInterval::localize() cases that TimeIntervalTest carried a note saying belonged to an integration suite precisely because Lang::getTxt() wanted Db::$db. Twelve tests, all of which fail against release-3.0 with the error above and pass on top of this.

Three things I would raise.

1. Admin\Languages::list_getLanguages() loses its override.

Sources/Actions/Admin/Languages.php:1555-1564 temporarily sets actual_theme_dir and base_theme_dir to default_theme_dir so the admin language list only shows languages installed in the default theme. The old Lang::get() read actual_theme_dir; the new one reads Lang::$dirs, which is built from theme_dir — so the override no longer does anything. With a custom theme carrying its own languages/zz_ZZ/:

release-3.0:  Lang::get() under the override sees: en_US
this branch:  Lang::get() under the override sees: en_US, zz_ZZ

It only bites a forum whose theme ships a languages/ directory, and only on a known_languages cache miss, so it is minor — but it is silent. Either point the override at theme_dir as well, or drop it if it is obsolete.

2. isset(Db\DatabaseApi::$db) is not quite "the database is available".

DatabaseApi::load() assigns self::$db = new $class($options) before calling initialize(), and initialize() is what calls displayDbError() when the connection fails. So during a connection failure $db is set and unusable — displayDbError() itself depends on that, at isset(Db::$db) ? @Db::$db->error() : ''. Reproducing that state and calling Lang::load() on this branch goes past the guard and into a query:

isset(Db::$db)   : true
live connection? : no
Lang::load       : FAIL  Error: ... @ MySQL.php:175

Latent today, since nothing calls Lang from there. But it matters because of the next point.

3. The strongest argument for this PR may not be the tests.

ErrorHandlerService::displayDbError() carries // Language files aren't loaded yet :(. and // What to do? Language files haven't and can't be loaded yet..., and hard-codes an English error page and an English email to the admin as a direct result. This change is what would make that translatable — but only with the guard from point 2 testing the connection rather than the object. That seems worth having in the description, since it is a user-visible improvement independent of any test suite.

Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
@Sesquipedalian

Copy link
Copy Markdown
Member Author

1. Admin\Languages::list_getLanguages() loses its override.

Fixed in df4b8dc

  1. isset(Db\DatabaseApi::$db) is not quite "the database is available".

Fixed in c266fd0

3. The strongest argument for this PR may not be the tests.

ErrorHandlerService::displayDbError() carries // Language files aren't loaded yet :(. and // What to do? Language files haven't and can't be loaded yet..., and hard-codes an English error page and an English email to the admin as a direct result. This change is what would make that translatable — but only with the guard from point 2 testing the connection rather than the object. That seems worth having in the description, since it is a user-visible improvement independent of any test suite.

Hm. Yes, that is an added benefit. If I remember correctly, there are also a number of other places where we currently use hard-coded English strings because we couldn't load language files. But I won't try to change all of those in this PR.

@Sesquipedalian
Sesquipedalian merged commit f7a74d2 into SimpleMachines:release-3.0 Aug 29, 2026
7 checks passed
@Sesquipedalian
Sesquipedalian deleted the 3.0/lang_no_db branch August 29, 2026 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Administrative Localization Language & internationalization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants