Skip to content

Conversation

@jdarwood007
Copy link
Member

Fixes #7512

@Sesquipedalian, I ran into an issue here and worked around it, but I know it could be better.

The config class doesn't allow me to do "custom" variables well. It would be nice if I could specify a Config::sqlite_wal variable. The set method does support $custom attributes, but there is no support for the getter. It looks like its expected that mod authors would extend this with a hook.
I didn't think it was necessary to update the Config class just to add a single boolean.

Another idea I had was to make a Config::$cache_opts = []; data set, and we would store various cache options in there, including the memcache stuff, etc. Seemed like an idea that would ensure caching, which needs to know its settings before connecting to the database, has a way to be extensible.

@Sesquipedalian
Copy link
Member

Sesquipedalian commented Aug 25, 2025

The config class doesn't allow me to do "custom" variables well. It would be nice if I could specify a Config::sqlite_wal variable. The set method does support $custom attributes, but there is no support for the getter. It looks like its expected that mod authors would extend this with a hook.

Everything is static in SMF\Config, so there are no object properties at all, and therefore no setter or getter methods.

Instead, anything setting that isn't part of SMF's standard configuration settings exists in SMF\Config::$custom and should be read from there directly.

For example, if Settings.php contains the following custom setting:

$foo = 'bar';

... then any mod that wants to read that custom setting can access it as Config::$custom['foo'], like so:

if (Config::$custom['foo'] === 'bar') {
	// do something
} elseif (Config::$custom['foo'] === 'baz') {
	// do something else
} else {
	// do a different thing
}

No hooks are needed or expected.


I didn't think it was necessary to update the Config class just to add a single boolean.

Adding your setting as a named static property of the Config class would be the best approach, actually. The Config class is intended to provide all of SMF's standard settings as named static properties of the class, so adding your new setting as a property of Config makes perfectly good sense.

First, you should add something like this in the Config::$settings_defs array:

		'cache_sqlite_wal' => [
			'text' => <<<'END'
				/**
				 * @var bool
				 *
				 * This is only used for the SQLite3 cache system.
				 * Whether to enable Write-Ahead Logging.
				 */
				END,
			'default' => false,
			'type' => 'boolean',
		],

Then you can add this at the appropriate location in Config's list of static properties:

	/**
	 * @var bool
	 *
	 * This is only used for the SQLite3 cache system.
	 * Whether to enable Write-Ahead Logging.
	 */
	public static bool $cache_sqlite_wal;

The result will be that Config::$cache_sqlite_wal will contain the boolean value that you need.

Now, in theory you could skip the second step and everything would still work, except that instead of Config::$cache_sqlite_wal you would need to check Config::$custom['cache_sqlite_wal']. But the lead developer, being the insufferably picky fellow that he is, would almost certainly tell you to do that second step anyway in order to keep everything aligned with the intended design of the class.

@Sesquipedalian
Copy link
Member

Another idea I had was to make a Config::$cache_opts = []; data set, and we would store various cache options in there, including the memcache stuff, etc. Seemed like an idea that would ensure caching, which needs to know its settings before connecting to the database, has a way to be extensible.

That's a good idea in theory, but it would require a bunch of extra pain and complication because we would need to handle both the old and new ways of recording that information in Settings.php. Doing so would involve both correctly interpreting both formats when reading Settings.php in Config::set() and—far more difficult—correctly finding both possible formats during Config::updateSettingsFile() and correctly replacing them with the new format in all cases.

@jdarwood007
Copy link
Member Author

jdarwood007 commented Sep 8, 2025

Another note to add to my recent commit. There is a way to better determine the sqlite file is in readonly mode.

https://www.sqlite.org/fileformat.html#file_format_version_numbers

The file format write version and file format read version at offsets 18 and 19 are intended to allow for enhancements of the file format in future versions of SQLite. In current versions of SQLite, both of these values are 1 for rollback journalling modes and 2 for WAL journalling mode. If a version of SQLite coded to the current file format specification encounters a database file where the read version is 1 or 2 but the write version is greater than 2, then the database file must be treated as read-only. If a database file with a read version greater than 2 is encountered, then that database cannot be read or written.

I could have read bytes 18/19 to see the file was in wol mode and that we had it off. This just seems simpler.

PHP offers no way to determine that, so sniff for the file to guess.

Additionally to fix issues with errors occurring, enabled exceptions for sqlite3 and used that to handle silently logging the error.
@jdarwood007 jdarwood007 added this to the 3.0 Alpha 6 milestone Oct 26, 2025
@Sesquipedalian Sesquipedalian merged commit d81420f into SimpleMachines:release-3.0 Nov 2, 2025
7 checks passed
@jdarwood007 jdarwood007 deleted the 3.0/feature7512 branch November 2, 2025 05:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQLite3 WAL mode

2 participants