fix: close what the style review found, starting with a gate the metadata skipped

The real defect first. RefreshDatabaseMetadata was the one worker of six
that never took the shared lock, and its flag was the one of six missing
from the tab's busy state. It calls MessageCount, which holds the read
lock, so a wipe could start while it was in there -- and the tab would
not have known to grey the button, because it could not see the worker.
Both halves fixed. The pattern is why: seven near-copies of one worker
skeleton, and each copy decided something slightly different.

The clear button failed silently when its thread could not start. The
most destructive control in the plugin, pressed, and nothing happens,
with no way to tell that from a wipe that worked -- while the three
harmless workers beside it do report. Maintenance was the mirror: its
comment promises refusals are said out loud, and then swallowed the
actual failure. Three start-failure paths also bypassed the notify
helper that carries the teardown check, three weeks after it was added
for exactly that.

The database numbers now wait for a real read, like the clear hint
already did. Zero bytes and zero messages read as an empty database, not
as a number nobody has fetched.

SelectionAfterDelete is gone, with its three tests. The accordion has no
selection, so its return value went into a discard -- a function
answering a question the interface does not ask, with green tests
guarding nothing. The project's own self-test README calls that the
anti-pattern of record.

Six new keys replaced by the translated orphans that already said the
same thing. A commit earlier in this cycle is literally called "stop
duplicating a key" and these went past it. The duplicate button also had
the label "Add", which is the one string out of ninety-four that was
never written.

Tests: CleanupDeleteTypes had none, and with the failsafe on -- how a
fresh config ships -- it is the path every cleanup takes. Four now,
including the one that matters: an empty list deletes nothing rather
than everything.

And a self-test for the gate wiring, which is what would have caught the
metadata worker. The unit tests prove the gate works; nothing proved the
workers use it.
This commit is contained in:
2026-08-19 07:09:54 +02:00
parent e7b76fb21b
commit 9ea9e96145
37 changed files with 310 additions and 529 deletions
@@ -171,7 +171,12 @@ internal sealed class TabEditor
// already has its sixty channels picked, and Tab.Clone has been ready
// for it since v1.8.0.
ImGui.SameLine();
if (ImGuiUtil.IconButton(FontAwesomeIcon.Copy, tooltip: Language.Options_Tabs_Add))
if (
ImGuiUtil.IconButton(
FontAwesomeIcon.Copy,
tooltip: HellionStrings.Settings_Tabs_Duplicate
)
)
{
var copy = tab.Clone();
copy.Identifier = Guid.NewGuid();
@@ -191,7 +196,6 @@ internal sealed class TabEditor
if (_editing == tab.Identifier)
ClearWorking();
_ = TabLifecycleHelpers.SelectionAfterDelete(tabs, index);
_plugin.SaveConfig();
RequestRefilter();
}
@@ -69,6 +69,11 @@ internal sealed class DataPrivacyTab
// refusal the user has to trigger to discover is not an answer.
private DbOperation CurrentOperation => _plugin.DbOperations.Current;
// The gate-wiring self-test asserts that this sees a held gate. It is the
// one thing about this state that a unit test cannot reach: the flags are
// instance state on a DI singleton and the gate is another.
internal bool AnythingRunningForSelfTest => AnythingRunning;
private bool AnythingRunning =>
_exportRunning
|| _exportDialogOpen
@@ -76,6 +81,7 @@ internal sealed class DataPrivacyTab
|| _cleanupRunning
|| _clearRunning
|| _maintenanceRunning
|| _dbRefreshRunning
|| _plugin.RetentionSweepRunning
|| CurrentOperation != DbOperation.None;
@@ -380,21 +386,27 @@ internal sealed class DataPrivacyTab
ImGuiUtil.Tooltip(Language.Options_Database_Metadata_CopyConfigPath);
}
ImGuiUtil.HelpText(
string.Format(
Language.Options_Database_Metadata_Size,
StringUtil.BytesToString(_dbSize)
)
);
ImGuiUtil.HelpText(
string.Format(
Language.Options_Database_Metadata_LogSize,
StringUtil.BytesToString(_dbLogSize)
)
);
ImGuiUtil.HelpText(
string.Format(Language.Options_Database_Metadata_MessageCount, _dbMessageCount)
);
// Same reason the clear hint below waits: the fields start at zero, and
// "0 B / 0 messages" reads as an empty database rather than as a number
// that has not been fetched yet.
if (_dbEverRefreshed)
{
ImGuiUtil.HelpText(
string.Format(
Language.Options_Database_Metadata_Size,
StringUtil.BytesToString(_dbSize)
)
);
ImGuiUtil.HelpText(
string.Format(
Language.Options_Database_Metadata_LogSize,
StringUtil.BytesToString(_dbLogSize)
)
);
ImGuiUtil.HelpText(
string.Format(Language.Options_Database_Metadata_MessageCount, _dbMessageCount)
);
}
ImGui.Spacing();
@@ -458,11 +470,25 @@ internal sealed class DataPrivacyTab
{
try
{
_dbSize = _plugin.MessageManager.Store.DatabaseSize();
_dbLogSize = _plugin.MessageManager.Store.DatabaseLogSize();
_dbMessageCount = _plugin.MessageManager.Store.MessageCount();
_dbRefreshedAt = Environment.TickCount64;
_dbEverRefreshed = true;
// Takes the gate like every other reader of the store.
// MessageCount holds _readLock, and a VACUUM starting under it
// is exactly the collision this gate was written for -- being a
// read rather than a write does not exempt it.
if (!_plugin.DbOperations.TryBegin(DbOperation.Cleanup))
return;
try
{
_dbSize = _plugin.MessageManager.Store.DatabaseSize();
_dbLogSize = _plugin.MessageManager.Store.DatabaseLogSize();
_dbMessageCount = _plugin.MessageManager.Store.MessageCount();
_dbRefreshedAt = Environment.TickCount64;
_dbEverRefreshed = true;
}
finally
{
_plugin.DbOperations.End(DbOperation.Cleanup);
}
}
catch (Exception e)
{
@@ -561,6 +587,11 @@ internal sealed class DataPrivacyTab
{
_clearRunning = false;
_logger.LogError(e, "Could not start the clear thread");
// The most destructive button in the plugin. Silence here means the
// user pressed it and nothing happened, with no way to tell that
// from a wipe that worked.
Notify(HellionStrings.Settings_Database_ClearError, NotificationType.Error);
}
}
@@ -682,6 +713,11 @@ internal sealed class DataPrivacyTab
catch (Exception e)
{
_logger.LogError(e, "Manual maintenance failed");
// The comment above promises refusals are said out loud. A
// failure that is not is the same silence wearing a different
// hat.
Notify(HellionStrings.Settings_Database_ClearError, NotificationType.Error);
}
finally
{
@@ -1047,7 +1083,7 @@ internal sealed class DataPrivacyTab
{
_cleanupRunning = false;
_logger.LogError(e, "Could not start the cleanup thread");
WrapperUtil.AddNotification(HellionStrings.Cleanup_Error, NotificationType.Error);
Notify(HellionStrings.Cleanup_Error, NotificationType.Error);
}
}
@@ -1106,9 +1142,9 @@ internal sealed class DataPrivacyTab
);
}
// Whole groups rather than sixty individual channels. Nobody makes a
// sixty-way choice, and the eight groups are the same ones the privacy
// wizard already uses, so the two screens describe the world the same way.
// Whole groups rather than eighty-nine individual channels. Nobody makes an
// eighty-nine-way choice, and the same eight groups carry the persist grid
// below, so the two screens describe channels the same way.
private void DrawExportChannels()
{
ImGui.Spacing();
@@ -1334,7 +1370,7 @@ internal sealed class DataPrivacyTab
// The thread never ran, so nothing will clear the flag for us.
_exportRunning = false;
_logger.LogError(e, "Could not start the export thread");
WrapperUtil.AddNotification(HellionStrings.Export_Error, NotificationType.Error);
Notify(HellionStrings.Export_Error, NotificationType.Error);
}
}
@@ -44,7 +44,7 @@ internal sealed class GeneralTab
_w.ToggleRow(
ImGui.GetID("general.behaviour.reducemotion"u8),
HellionStrings.Settings_ThemeAndLayout_ReduceMotion_Name,
HellionStrings.Settings_General_ReduceMotion_Description,
HellionStrings.Settings_ThemeAndLayout_ReduceMotion_Description,
() => Plugin.Config.ReduceMotion,
v => Plugin.Config.ReduceMotion = v
);
@@ -80,8 +80,8 @@ internal sealed class GeneralTab
// is typing does not see it. On by default and never reachable.
_w.ToggleRow(
ImGui.GetID("general.notifications.failedtell"u8),
HellionStrings.Settings_NotifyFailedTell_Name,
HellionStrings.Settings_NotifyFailedTell_Description,
HellionStrings.Settings_Chat_NotifyFailedTell_Name,
HellionStrings.Settings_Chat_NotifyFailedTell_Description,
() => Plugin.Config.NotifyFailedTell,
v => Plugin.Config.NotifyFailedTell = v
);
@@ -93,8 +93,8 @@ internal sealed class WindowTab
);
_w.SliderFloatRow(
ImGui.GetID("window.opacity.inactive"u8),
HellionStrings.Settings_Window_InactiveOpacity_Name,
HellionStrings.Settings_Window_InactiveOpacity_Description,
HellionStrings.Settings_ThemeAndLayout_WindowOpacityInactive_Name,
HellionStrings.Settings_ThemeAndLayout_WindowOpacityInactive_Description,
() => Plugin.Config.WindowOpacityInactive,
v => Plugin.Config.WindowOpacityInactive = v,
0.1f,
@@ -39,7 +39,7 @@ internal sealed class ThemeImportExportRow
}
ImGui.SameLine();
if (ImGui.Button(HellionStrings.Settings_Theme_ExportActive))
if (ImGui.Button(HellionStrings.Settings_Themes_ExportActive))
{
ExportActive();
}