diff --git a/lib/core/theme/theme_controller.dart b/lib/core/theme/theme_controller.dart index a156a99d..148c40af 100644 --- a/lib/core/theme/theme_controller.dart +++ b/lib/core/theme/theme_controller.dart @@ -227,7 +227,8 @@ class ThemeController extends ChangeNotifier { } _themeFolderWatcher ??= ThemeFolderWatcher( themesDirectory: ExtensionPaths.extensionsDirectory, - onThemesChanged: loadAvailableThemes, + onThemesChanged: ({required bool structuralChange}) => + loadAvailableThemes(reloadExtensions: structuralChange), ); await _themeFolderWatcher!.start(); } @@ -296,14 +297,16 @@ class ThemeController extends ChangeNotifier { _notifyThemeChanged(); } - Future loadAvailableThemes() async { + Future loadAvailableThemes({bool reloadExtensions = true}) async { if (_isLoadingAvailableThemes) return; _isLoadingAvailableThemes = true; notifyListeners(); try { - final scanned = await _registryService.loadThemeDefinitions(); + final scanned = await _registryService.loadThemeDefinitions( + reloadExtensions: reloadExtensions, + ); _availableThemes = _mergeBuiltinThemes(scanned); _syncSelectedThemeAfterRefresh(); } on Object { diff --git a/lib/core/theme/theme_folder_watcher.dart b/lib/core/theme/theme_folder_watcher.dart index 5eeae28d..123bb158 100644 --- a/lib/core/theme/theme_folder_watcher.dart +++ b/lib/core/theme/theme_folder_watcher.dart @@ -8,19 +8,22 @@ import 'package:path/path.dart' as p; class ThemeFolderWatcher { ThemeFolderWatcher({ required Future Function() themesDirectory, - required Future Function() onThemesChanged, + required Future Function({required bool structuralChange}) + onThemesChanged, this.debounce = const Duration(milliseconds: 400), }) : _themesDirectory = themesDirectory, _onThemesChanged = onThemesChanged; final Future Function() _themesDirectory; - final Future Function() _onThemesChanged; + final Future Function({required bool structuralChange}) + _onThemesChanged; final Duration debounce; StreamSubscription? _subscription; Timer? _debounceTimer; bool _started = false; bool _refreshInFlight = false; + bool _pendingStructural = false; bool get isStarted => _started; @@ -71,23 +74,32 @@ class ThemeFolderWatcher { _subscription = null; _started = false; _refreshInFlight = false; + _pendingStructural = false; await Future.delayed(const Duration(milliseconds: 150)); } void _onFilesystemEvent(FileSystemEvent event) { if (!_isRelevantEvent(event)) return; + if (event is FileSystemCreateEvent || + event is FileSystemDeleteEvent || + event is FileSystemMoveEvent) { + _pendingStructural = true; + } + _debounceTimer?.cancel(); _debounceTimer = Timer(debounce, () { - unawaited(_triggerRefresh()); + final structural = _pendingStructural; + _pendingStructural = false; + unawaited(_triggerRefresh(structuralChange: structural)); }); } - Future _triggerRefresh() async { + Future _triggerRefresh({required bool structuralChange}) async { if (_refreshInFlight) return; _refreshInFlight = true; try { - await _onThemesChanged(); + await _onThemesChanged(structuralChange: structuralChange); } on Object catch (error) { debugPrint('ThemeFolderWatcher: refresh failed ($error)'); } finally { diff --git a/lib/core/theme/theme_registry_service.dart b/lib/core/theme/theme_registry_service.dart index 1d66f21b..9fbdcc26 100644 --- a/lib/core/theme/theme_registry_service.dart +++ b/lib/core/theme/theme_registry_service.dart @@ -48,19 +48,31 @@ class ThemeRegistryService { final List _bundledThemeAssetFiles; final _ThemeLruCache _themeCache; int _themeParseCount = 0; + int _themeFileReadCount = 0; bool _hasMigratedThemes = false; + final Map _definitionScanCache = {}; + List? _cachedBuiltinDefinitions; /// Number of cache misses that performed a full theme parse. @visibleForTesting int get themeParseCount => _themeParseCount; + /// Number of theme files read from disk during definition scans. + @visibleForTesting + int get themeFileReadCount => _themeFileReadCount; + /// Clears parsed theme cache and parse counter. void clearCache() { _themeCache.clear(); _themeParseCount = 0; + _themeFileReadCount = 0; + _definitionScanCache.clear(); + _cachedBuiltinDefinitions = null; } - Future> loadThemeDefinitions() async { + Future> loadThemeDefinitions({ + bool reloadExtensions = true, + }) async { final definitions = []; await _loadBuiltinAssetDefinitions(definitions); @@ -68,9 +80,13 @@ class ThemeRegistryService { if (!_hasMigratedThemes) { await _migrateLegacyThemesToExtensions(); _hasMigratedThemes = true; + } else if (reloadExtensions) { + await LocalExtensionRegistry.instance.reload(); + } else { + await LocalExtensionRegistry.instance.load(); } - await LocalExtensionRegistry.instance.reload(); + final seenPaths = {}; for (final manifest in LocalExtensionRegistry.instance.manifests) { if (manifest.type != ExtensionType.theme) continue; final installPath = manifest.installPath; @@ -84,9 +100,16 @@ class ThemeRegistryService { ? ThemeSource.imported : ThemeSource.filesystem; - final definition = await _definitionFromFile(file, source, extensionId: manifest.id); + final definition = await _definitionFromFile( + file, + source, + extensionId: manifest.id, + ); if (definition != null) { definitions.add(definition); + if (definition.path != null) { + seenPaths.add(definition.path!); + } } } @@ -98,8 +121,15 @@ class ThemeRegistryService { definition.source != ThemeSource.legacyImported, ); definitions.add(legacy); + if (legacy.path != null) { + seenPaths.add(legacy.path!); + } } + _definitionScanCache.removeWhere( + (path, _) => !_isAssetPath(path) && !seenPaths.contains(path), + ); + definitions.sort( (a, b) => a.name.toLowerCase().compareTo(b.name.toLowerCase()), ); @@ -492,6 +522,12 @@ class ThemeRegistryService { } Future _loadBuiltinAssetDefinitions(List out) async { + if (_cachedBuiltinDefinitions != null) { + out.addAll(_cachedBuiltinDefinitions!); + return; + } + + final builtins = []; for (final fileName in _bundledThemeAssetFiles) { final assetPath = BuiltinThemeAssets.assetPath(fileName); try { @@ -511,12 +547,15 @@ class ThemeRegistryService { contentHash: hash, ); if (definition != null) { - out.add(definition); + builtins.add(definition); + _definitionScanCache[assetPath] = definition; } } on Object catch (e) { _logScanError(assetPath, e); } } + _cachedBuiltinDefinitions = List.unmodifiable(builtins); + out.addAll(_cachedBuiltinDefinitions!); } Future _readThemeRaw(ThemeDefinition definition) async { @@ -547,28 +586,25 @@ class ThemeRegistryService { }) async { try { final stat = await file.stat(); + final cached = _definitionScanCache[file.path]; + if (cached != null && + cached.lastModified != null && + cached.lastModified == stat.modified && + (extensionId == null || cached.id == extensionId)) { + return cached; + } + + _themeFileReadCount++; final raw = await file.readAsString(); final hash = _contentHash(raw); final json = _decodeRoot(raw); if (json == null) { _logScanError(file.path, 'Invalid JSON'); + _definitionScanCache.remove(file.path); return null; } - final schema = json['schema']?.toString(); - if (schema == queryaThemeSchemaV1) { - return _definitionFromRaw( - json: json, - path: file.path, - fileBaseName: p.basenameWithoutExtension(file.path), - source: source, - contentHash: hash, - lastModified: stat.modified, - extensionId: extensionId, - ); - } - - return _definitionFromRaw( + final definition = _definitionFromRaw( json: json, path: file.path, fileBaseName: p.basenameWithoutExtension(file.path), @@ -577,6 +613,12 @@ class ThemeRegistryService { lastModified: stat.modified, extensionId: extensionId, ); + if (definition != null) { + _definitionScanCache[file.path] = definition; + } else { + _definitionScanCache.remove(file.path); + } + return definition; } on Object catch (e) { _logScanError(file.path, e); return null; diff --git a/test/core/theme/theme_controller_test.dart b/test/core/theme/theme_controller_test.dart index 6cea1a06..0be27a8f 100644 --- a/test/core/theme/theme_controller_test.dart +++ b/test/core/theme/theme_controller_test.dart @@ -46,9 +46,11 @@ class _GatedRegistryService extends ThemeRegistryService { final gate = Completer(); @override - Future> loadThemeDefinitions() async { + Future> loadThemeDefinitions({ + bool reloadExtensions = true, + }) async { await gate.future; - return super.loadThemeDefinitions(); + return super.loadThemeDefinitions(reloadExtensions: reloadExtensions); } } diff --git a/test/core/theme/theme_folder_watcher_test.dart b/test/core/theme/theme_folder_watcher_test.dart index bf127861..76084642 100644 --- a/test/core/theme/theme_folder_watcher_test.dart +++ b/test/core/theme/theme_folder_watcher_test.dart @@ -80,7 +80,7 @@ void main() { var refreshCount = 0; final watcher = ThemeFolderWatcher( themesDirectory: () async => themesDir, - onThemesChanged: () async { + onThemesChanged: ({required bool structuralChange}) async { refreshCount++; }, debounce: const Duration(milliseconds: 80), @@ -104,7 +104,7 @@ void main() { var refreshCount = 0; final watcher = ThemeFolderWatcher( themesDirectory: () async => themesDir, - onThemesChanged: () async { + onThemesChanged: ({required bool structuralChange}) async { refreshCount++; if (!refreshGate.isCompleted) { refreshGate.complete(); @@ -132,7 +132,7 @@ void main() { var refreshCount = 0; final watcher = ThemeFolderWatcher( themesDirectory: () async => themesDir, - onThemesChanged: () async { + onThemesChanged: ({required bool structuralChange}) async { refreshCount++; }, debounce: const Duration(milliseconds: 80), diff --git a/test/core/theme/theme_registry_cache_test.dart b/test/core/theme/theme_registry_cache_test.dart index 4a4d7078..63cf85e9 100644 --- a/test/core/theme/theme_registry_cache_test.dart +++ b/test/core/theme/theme_registry_cache_test.dart @@ -73,6 +73,25 @@ void main() { }); group('ThemeRegistryService cache', () { + test('skips theme file read when mtime unchanged on refresh', () async { + await _copyFixture( + 'querya_custom_dark.json', + File(p.join(themesDir.path, 'querya_custom_dark.json')), + ); + + final first = await registry.loadThemeDefinitions(); + expect(first, hasLength(1)); + final readsAfterCold = registry.themeFileReadCount; + expect(readsAfterCold, greaterThan(0)); + + final second = await registry.loadThemeDefinitions( + reloadExtensions: false, + ); + expect(second, hasLength(1)); + expect(second.single.id, first.single.id); + expect(registry.themeFileReadCount, readsAfterCold); + }); + test('loads same definition twice with a single parse', () async { await _copyFixture( 'querya_custom_dark.json',