From e9bb805f025e62f04c86fddc4891ef63e68983ad Mon Sep 17 00:00:00 2001 From: Tim Gels Date: Fri, 6 Mar 2026 21:26:40 +0100 Subject: [PATCH 1/4] refactor(core,vm): introduce per-file track values model and fix suppress flag reset - Extract FileTrackValues as a dedicated per-file track data model - Replace TrackConfiguration reuse in per-file collections with FileTrackValues - Add ILanguageProvider.Resolve() for centralized language code resolution - Fix _suppressBatchConfigUpdate not being reset on early return in ApplyTrackProperties (try/finally) - Only log SelectedLanguage null warning when not in internal sync path - Update MkvPropeditArgumentsGenerator remark to reflect that per-file values are updated by the user - Add MkvPropeditArgumentsGenerator logging partial class - Correct 'iso639_3' to 'ISO 639-3' in test comment - Update tests for new FileTrackValues model and refactored APIs Refs: #85 --- .../Models/FileTrackConfiguration.cs | 17 +-- .../Models/FileTrackValues.cs | 66 ++++++++ .../Services/BatchConfiguration.cs | 4 +- .../BatchTrackConfigurationInitializer.cs | 18 ++- .../Services/IBatchConfiguration.cs | 14 +- .../Services/ILanguageProvider.cs | 7 + .../Services/LanguageProvider.cs | 17 +++ .../MkvPropeditArgumentsGenerator.Logging.cs | 14 ++ .../Services/MkvPropeditArgumentsGenerator.cs | 52 +++++-- .../Services/TrackConfigurationFactory.cs | 27 +--- .../Presentation/AudioViewModel.cs | 4 +- .../Presentation/SubtitleViewModel.cs | 4 +- .../TrackViewModelBase.Logging.cs | 10 ++ .../Presentation/TrackViewModelBase.cs | 144 ++++++++++++------ .../Presentation/VideoViewModel.cs | 4 +- .../Services/BatchConfigurationTests.cs | 61 +++++--- ...BatchTrackConfigurationInitializerTests.cs | 83 +++++++++- .../Services/LanguageProviderTests.cs | 144 ++++++++++++++++++ .../TrackConfigurationFactoryTests.cs | 32 ++-- .../TrackModificationIntegrationTests.cs | 7 +- .../Presentation/AudioViewModelTests.cs | 19 ++- .../Presentation/SubtitleViewModelTests.cs | 19 ++- .../Presentation/TrackViewModelBaseTests.cs | 47 +++--- .../Presentation/VideoViewModelTests.cs | 19 ++- 24 files changed, 630 insertions(+), 203 deletions(-) create mode 100644 src/MatroskaBatchFlow.Core/Models/FileTrackValues.cs create mode 100644 src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.Logging.cs create mode 100644 src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.Logging.cs diff --git a/src/MatroskaBatchFlow.Core/Models/FileTrackConfiguration.cs b/src/MatroskaBatchFlow.Core/Models/FileTrackConfiguration.cs index 25771ae..fe53338 100644 --- a/src/MatroskaBatchFlow.Core/Models/FileTrackConfiguration.cs +++ b/src/MatroskaBatchFlow.Core/Models/FileTrackConfiguration.cs @@ -1,6 +1,5 @@ using System.Collections.ObjectModel; using MatroskaBatchFlow.Core.Enums; -using MatroskaBatchFlow.Core.Services; namespace MatroskaBatchFlow.Core.Models; @@ -15,26 +14,26 @@ public sealed class FileTrackConfiguration public string FilePath { get; set; } = string.Empty; /// - /// Audio track configurations for this file. + /// Audio track values for this file. /// - public ObservableCollection AudioTracks { get; set; } = []; + public ObservableCollection AudioTracks { get; set; } = []; /// - /// Video track configurations for this file. + /// Video track values for this file. /// - public ObservableCollection VideoTracks { get; set; } = []; + public ObservableCollection VideoTracks { get; set; } = []; /// - /// Subtitle track configurations for this file. + /// Subtitle track values for this file. /// - public ObservableCollection SubtitleTracks { get; set; } = []; + public ObservableCollection SubtitleTracks { get; set; } = []; /// /// Gets the track list for a specific track type. /// /// The track type to retrieve. - /// Observable collection of track configurations for the specified type. - public ObservableCollection GetTrackListForType(TrackType trackType) + /// Observable collection of track values for the specified type. + public ObservableCollection GetTrackListForType(TrackType trackType) { return trackType switch { diff --git a/src/MatroskaBatchFlow.Core/Models/FileTrackValues.cs b/src/MatroskaBatchFlow.Core/Models/FileTrackValues.cs new file mode 100644 index 0000000..6b8af60 --- /dev/null +++ b/src/MatroskaBatchFlow.Core/Models/FileTrackValues.cs @@ -0,0 +1,66 @@ +using MatroskaBatchFlow.Core.Enums; + +namespace MatroskaBatchFlow.Core.Models; + +/// +/// Stores per-file track values that are initially populated from the MediaInfo scan and +/// subsequently updated when the user changes settings in the UI. +/// The original scanned data is always available via . +/// +/// +/// The batch configuration uses a dual-model approach for tracks: +/// +/// +/// - one per track index in the global collection. +/// Carries modification intent (ShouldModify* flags) and is shared across all files. +/// +/// +/// - one per track per file in . +/// Holds the actual values (Name, Language, flags) to write for that specific file. +/// +/// +/// During command generation, reads +/// ShouldModify* from the global track and the values to write from the per-file track. +/// +public sealed class FileTrackValues +{ + /// + /// The raw track information as returned by MediaInfo. + /// + public required MediaInfoResult.MediaInfo.TrackInfo ScannedTrackInfo { get; init; } + + /// + /// The type of this track (Audio, Video, Text). + /// + public TrackType Type { get; init; } + + /// + /// Zero-based index of this track within its type (e.g. 0 = first audio track). + /// + public int Index { get; init; } + + /// + /// The track name as scanned from the file. + /// + public string Name { get; set; } = string.Empty; + + /// + /// The track language as scanned from the file. + /// + public MatroskaLanguageOption Language { get; set; } = MatroskaLanguageOption.Undetermined; + + /// + /// Whether this track has the default flag set. + /// + public bool Default { get; set; } + + /// + /// Whether this track has the forced flag set. + /// + public bool Forced { get; set; } + + /// + /// Whether this track is enabled (corresponds to the Matroska FlagEnabled element). + /// + public bool Enabled { get; set; } +} diff --git a/src/MatroskaBatchFlow.Core/Services/BatchConfiguration.cs b/src/MatroskaBatchFlow.Core/Services/BatchConfiguration.cs index f144db7..e032a6c 100644 --- a/src/MatroskaBatchFlow.Core/Services/BatchConfiguration.cs +++ b/src/MatroskaBatchFlow.Core/Services/BatchConfiguration.cs @@ -321,7 +321,7 @@ public IList GetTrackListForType(TrackType trackType) /// /// Thrown if no per-file track configuration exists for the specified file ID. - public IList GetTrackListForFile(Guid fileId, TrackType trackType) + public IList GetTrackListForFile(Guid fileId, TrackType trackType) { // Prefer per-file configuration. If none exists, fail fast by throwing an exception. if (FileConfigurations.TryGetValue(fileId, out var fileConfig)) @@ -656,7 +656,7 @@ public bool ShouldModifyEnabledFlag } } - protected void OnPropertyChanged(string propertyName) + private void OnPropertyChanged(string propertyName) { PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); } diff --git a/src/MatroskaBatchFlow.Core/Services/BatchTrackConfigurationInitializer.cs b/src/MatroskaBatchFlow.Core/Services/BatchTrackConfigurationInitializer.cs index 6406f63..ecbebab 100644 --- a/src/MatroskaBatchFlow.Core/Services/BatchTrackConfigurationInitializer.cs +++ b/src/MatroskaBatchFlow.Core/Services/BatchTrackConfigurationInitializer.cs @@ -16,9 +16,11 @@ namespace MatroskaBatchFlow.Core.Services; /// /// The batch configuration to be modified. /// The factory for creating track configurations. +/// The language provider for resolving track language codes. public class BatchTrackConfigurationInitializer( IBatchConfiguration batchConfig, - ITrackConfigurationFactory trackConfigFactory) : IBatchTrackConfigurationInitializer + ITrackConfigurationFactory trackConfigFactory, + ILanguageProvider languageProvider) : IBatchTrackConfigurationInitializer { /// public void Initialize(ScannedFileInfo scannedFile, params TrackType[] trackTypes) @@ -36,7 +38,7 @@ public void Initialize(ScannedFileInfo scannedFile, params TrackType[] trackType batchConfig.FileConfigurations.Add(scannedFile.Id, fileConfig); } - // Populate file-specific track configurations based on what this file has + // Populate file-specific track values based on what this file has foreach (var trackType in trackTypes) { // Ordering tracks by StreamKindID as it represents the track order in the file @@ -52,7 +54,17 @@ public void Initialize(ScannedFileInfo scannedFile, params TrackType[] trackType for (int i = existingCount; i < scannedCount; i++) { - fileTracks.Add(trackConfigFactory.Create(scannedTracks[i], trackType, i)); + var scannedTrack = scannedTracks[i]; + fileTracks.Add(new FileTrackValues + { + ScannedTrackInfo = scannedTrack, + Type = trackType, + Index = i, + Name = scannedTrack.Title ?? string.Empty, + Language = languageProvider.Resolve(scannedTrack.Language), + Default = scannedTrack.Default, + Forced = scannedTrack.Forced, + }); } } diff --git a/src/MatroskaBatchFlow.Core/Services/IBatchConfiguration.cs b/src/MatroskaBatchFlow.Core/Services/IBatchConfiguration.cs index e497798..96155fe 100644 --- a/src/MatroskaBatchFlow.Core/Services/IBatchConfiguration.cs +++ b/src/MatroskaBatchFlow.Core/Services/IBatchConfiguration.cs @@ -9,8 +9,9 @@ namespace MatroskaBatchFlow.Core.Services; /// /// Defines the contract for batch configuration of media files. ///
-/// Note: The TrackConfiguration items in the collections also implement INotifyPropertyChanged, -/// so property changes within tracks can be observed. +/// Note: The TrackConfiguration items in the global collections implement INotifyPropertyChanged, +/// so property changes within global tracks can be observed. +/// Per-file track values are stored as which carry no modification intent. /// public interface IBatchConfiguration : INotifyPropertyChanged { @@ -51,13 +52,14 @@ public interface IBatchConfiguration : INotifyPropertyChanged public IList GetTrackListForType(TrackType trackType); /// - /// Gets track configuration for a specific file and track type. - /// Always uses per-file configurations. Falls back to global if file config not found. + /// Gets the per-file track values for a specific file and track type. + /// These carry only scanned values (Name, Language, flags) — no modification intent. + /// Modification flags (ShouldModify*) live on the global objects returned by . /// /// The ScannedFileInfo.Id (Guid) of the file. /// Type of track to retrieve. - /// List of track configurations for the specified file and track type. - public IList GetTrackListForFile(Guid fileId, TrackType trackType); + /// List of per-file track values for the specified file and track type. + public IList GetTrackListForFile(Guid fileId, TrackType trackType); /// /// Migrates file configuration from an old file ID to a new file ID. diff --git a/src/MatroskaBatchFlow.Core/Services/ILanguageProvider.cs b/src/MatroskaBatchFlow.Core/Services/ILanguageProvider.cs index 9deb1a3..1d387f5 100644 --- a/src/MatroskaBatchFlow.Core/Services/ILanguageProvider.cs +++ b/src/MatroskaBatchFlow.Core/Services/ILanguageProvider.cs @@ -8,4 +8,11 @@ public interface ILanguageProvider public ImmutableList Languages { get; } public void LoadLanguages(); + + /// + /// Resolves a raw language string from MediaInfo into a typed . + /// + /// The language code (e.g., "en", "eng", or "English"). + /// The matching language option, or if no match found. + MatroskaLanguageOption Resolve(string? languageCode); } diff --git a/src/MatroskaBatchFlow.Core/Services/LanguageProvider.cs b/src/MatroskaBatchFlow.Core/Services/LanguageProvider.cs index dd7ec3e..7fc4fef 100644 --- a/src/MatroskaBatchFlow.Core/Services/LanguageProvider.cs +++ b/src/MatroskaBatchFlow.Core/Services/LanguageProvider.cs @@ -66,4 +66,21 @@ public void LoadLanguages() Languages = []; } } + + /// + public MatroskaLanguageOption Resolve(string? languageCode) + { + if (string.IsNullOrWhiteSpace(languageCode)) + return MatroskaLanguageOption.Undetermined; + + var matchedLanguage = Languages.FirstOrDefault(lang => + string.Equals(lang.Iso639_2_b, languageCode, StringComparison.OrdinalIgnoreCase) || + string.Equals(lang.Iso639_2_t, languageCode, StringComparison.OrdinalIgnoreCase) || + string.Equals(lang.Iso639_1, languageCode, StringComparison.OrdinalIgnoreCase) || + string.Equals(lang.Iso639_3, languageCode, StringComparison.OrdinalIgnoreCase) || + string.Equals(lang.Name, languageCode, StringComparison.OrdinalIgnoreCase) || + string.Equals(lang.Code, languageCode, StringComparison.OrdinalIgnoreCase)); + + return matchedLanguage ?? MatroskaLanguageOption.Undetermined; + } } diff --git a/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.Logging.cs b/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.Logging.cs new file mode 100644 index 0000000..6c9e6f5 --- /dev/null +++ b/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.Logging.cs @@ -0,0 +1,14 @@ +using MatroskaBatchFlow.Core.Enums; +using Microsoft.Extensions.Logging; + +namespace MatroskaBatchFlow.Core.Services; + +/// +/// LoggerMessage definitions for . +/// +public sealed partial class MkvPropeditArgumentsGenerator +{ + [LoggerMessage(Level = LogLevel.Warning, + Message = "Per-file {TrackType} track index {TrackIndex} exceeds global track count {GlobalTrackCount} for file: {FilePath}")] + private partial void LogPerFileTrackExceedsGlobalCount(string filePath, TrackType trackType, int trackIndex, int globalTrackCount); +} diff --git a/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.cs b/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.cs index c136e8b..d3c7d46 100644 --- a/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.cs +++ b/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.cs @@ -1,13 +1,14 @@ using MatroskaBatchFlow.Core.Builders.MkvPropeditArguments; using MatroskaBatchFlow.Core.Enums; using MatroskaBatchFlow.Core.Models; +using Microsoft.Extensions.Logging; namespace MatroskaBatchFlow.Core.Services; /// /// Provides reusable logic for generating mkvpropedit command-line arguments from an . /// -public sealed class MkvPropeditArgumentsGenerator : IMkvPropeditArgumentsGenerator +public sealed partial class MkvPropeditArgumentsGenerator(ILogger logger) : IMkvPropeditArgumentsGenerator { /// public string[] BuildBatchArguments(IBatchConfiguration batchConfiguration) @@ -42,7 +43,7 @@ public string BuildFileArgumentString(ScannedFileInfo file, IBatchConfiguration /// Contains global title settings and per-file track configurations. /// Token array suitable for joining. Returns an empty array if no modifications are requested /// (to signal "no-op"). - private static string[] BuildFileArgumentTokens(ScannedFileInfo file, IBatchConfiguration batchConfiguration) + private string[] BuildFileArgumentTokens(ScannedFileInfo file, IBatchConfiguration batchConfiguration) { var builder = new MkvPropeditArgumentsBuilder(); @@ -84,11 +85,17 @@ private static string[] BuildFileArgumentTokens(ScannedFileInfo file, IBatchConf /// Adds track-specific modifications to the builder for a specific file, filtering out tracks that have /// no requested changes or don't exist in the file. /// + /// + /// Modification intent (ShouldModify*) is read from the global at + /// the matching index. The actual values to write (Name, Language, flags) are read from the per-file + /// , which are initially populated from the MediaInfo scan and subsequently + /// updated when the user changes settings in the UI. + /// /// The accumulating mkvpropedit argument builder. /// The file being processed. /// The track type (must map to a Matroska track element). /// The batch configuration containing track availability data. - private static void AddTracksForFile( + private void AddTracksForFile( MkvPropeditArgumentsBuilder builder, ScannedFileInfo file, TrackType type, @@ -99,17 +106,30 @@ private static void AddTracksForFile( return; } - // Get file-specific track configuration. - var tracks = batchConfig.GetTrackListForFile(file.Id, type); + // Per-file values (Name, Language, flags as scanned from this file). + var perFileTracks = batchConfig.GetTrackListForFile(file.Id, type); - foreach (var track in tracks) + // Global tracks carry the modification intent (ShouldModify* flags). + var globalTracks = batchConfig.GetTrackListForType(type); + + foreach (var track in perFileTracks) { + // Defensive: the initializer always expands global tracks to the maximum + // count, so this should not be true under normal operation. + if (track.Index >= globalTracks.Count) + { + LogPerFileTrackExceedsGlobalCount(file.Path, type, track.Index, globalTracks.Count); + continue; + } + + var globalTrack = globalTracks[track.Index]; + // Skip inert tracks (no requested modifications). - if (!(track.ShouldModifyLanguage || - track.ShouldModifyName || - track.ShouldModifyDefaultFlag || - track.ShouldModifyForcedFlag || - track.ShouldModifyEnabledFlag)) + if (!(globalTrack.ShouldModifyLanguage || + globalTrack.ShouldModifyName || + globalTrack.ShouldModifyDefaultFlag || + globalTrack.ShouldModifyForcedFlag || + globalTrack.ShouldModifyEnabledFlag)) { continue; } @@ -126,27 +146,27 @@ private static void AddTracksForFile( // Track ID converted to 1-based indexing for mkvpropedit conventions. tb.SetTrackId(track.Index + 1).SetTrackType(type); - if (track.ShouldModifyLanguage) + if (globalTrack.ShouldModifyLanguage) { tb.WithLanguage(track.Language.Code); } - if (track.ShouldModifyName) + if (globalTrack.ShouldModifyName) { tb.WithName(track.Name); } - if (track.ShouldModifyDefaultFlag) + if (globalTrack.ShouldModifyDefaultFlag) { tb.WithIsDefault(track.Default); } - if (track.ShouldModifyForcedFlag) + if (globalTrack.ShouldModifyForcedFlag) { tb.WithIsForced(track.Forced); } - if (track.ShouldModifyEnabledFlag) + if (globalTrack.ShouldModifyEnabledFlag) { tb.WithIsEnabled(track.Enabled); } diff --git a/src/MatroskaBatchFlow.Core/Services/TrackConfigurationFactory.cs b/src/MatroskaBatchFlow.Core/Services/TrackConfigurationFactory.cs index f7512a7..977cf42 100644 --- a/src/MatroskaBatchFlow.Core/Services/TrackConfigurationFactory.cs +++ b/src/MatroskaBatchFlow.Core/Services/TrackConfigurationFactory.cs @@ -22,34 +22,9 @@ public TrackConfiguration Create( Type = trackType, Index = index, Name = scannedTrackInfo.Title ?? string.Empty, - Language = ParseLanguageFromCode(scannedTrackInfo.Language), + Language = languageProvider.Resolve(scannedTrackInfo.Language), Default = scannedTrackInfo.Default, Forced = scannedTrackInfo.Forced }; } - - /// - /// Parses a language code from scanned track info and returns the matching . - /// - /// - /// Performs case-insensitive comparison against ISO 639-1, ISO 639-2 (both bibliographic and terminologic), - /// ISO 639-3 codes, the language name, and a custom code field. - /// - /// The language code from MediaInfo (e.g., "en", "eng", "jpn", or "English"). - /// The matching language option, or if no match found. - private MatroskaLanguageOption ParseLanguageFromCode(string? languageCode) - { - if (string.IsNullOrWhiteSpace(languageCode)) - return MatroskaLanguageOption.Undetermined; - - var matchedLanguage = languageProvider.Languages.FirstOrDefault(lang => - string.Equals(lang.Iso639_2_b, languageCode, StringComparison.OrdinalIgnoreCase) || - string.Equals(lang.Iso639_2_t, languageCode, StringComparison.OrdinalIgnoreCase) || - string.Equals(lang.Iso639_1, languageCode, StringComparison.OrdinalIgnoreCase) || - string.Equals(lang.Iso639_3, languageCode, StringComparison.OrdinalIgnoreCase) || - string.Equals(lang.Name, languageCode, StringComparison.OrdinalIgnoreCase) || - string.Equals(lang.Code, languageCode, StringComparison.OrdinalIgnoreCase)); - - return matchedLanguage ?? MatroskaLanguageOption.Undetermined; - } } diff --git a/src/MatroskaBatchFlow.Uno/Presentation/AudioViewModel.cs b/src/MatroskaBatchFlow.Uno/Presentation/AudioViewModel.cs index e9e7eeb..721fc07 100644 --- a/src/MatroskaBatchFlow.Uno/Presentation/AudioViewModel.cs +++ b/src/MatroskaBatchFlow.Uno/Presentation/AudioViewModel.cs @@ -28,8 +28,8 @@ public ObservableCollection AudioTracks /// public bool IsFileListPopulated => _batchConfiguration.FileList.Count > 0; - public AudioViewModel(ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences) - : base(languageProvider, batchConfiguration, uiPreferences) + public AudioViewModel(ILogger logger, ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences) + : base(logger, languageProvider, batchConfiguration, uiPreferences) { AudioTracks = [.. _batchConfiguration.AudioTracks]; diff --git a/src/MatroskaBatchFlow.Uno/Presentation/SubtitleViewModel.cs b/src/MatroskaBatchFlow.Uno/Presentation/SubtitleViewModel.cs index 54395b9..bce831b 100644 --- a/src/MatroskaBatchFlow.Uno/Presentation/SubtitleViewModel.cs +++ b/src/MatroskaBatchFlow.Uno/Presentation/SubtitleViewModel.cs @@ -28,8 +28,8 @@ public ObservableCollection SubtitleTracks /// public bool IsFileListPopulated => _batchConfiguration.FileList.Count > 0; - public SubtitleViewModel(ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences) - : base(languageProvider, batchConfiguration, uiPreferences) + public SubtitleViewModel(ILogger logger, ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences) + : base(logger, languageProvider, batchConfiguration, uiPreferences) { SubtitleTracks = [.. _batchConfiguration.SubtitleTracks]; diff --git a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.Logging.cs b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.Logging.cs new file mode 100644 index 0000000..bf92959 --- /dev/null +++ b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.Logging.cs @@ -0,0 +1,10 @@ +namespace MatroskaBatchFlow.Uno.Presentation; + +/// +/// LoggerMessage definitions for . +/// +public abstract partial class TrackViewModelBase +{ + [LoggerMessage(Level = LogLevel.Warning, Message = "SelectedLanguage was set to null; falling back to Undetermined. This is unexpected during normal UI operation.")] + private partial void LogSelectedLanguageNullFallback(); +} diff --git a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs index 89aa3bc..3392e5c 100644 --- a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs +++ b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs @@ -8,6 +8,7 @@ namespace MatroskaBatchFlow.Uno.Presentation; public abstract partial class TrackViewModelBase : ObservableObject { + private readonly ILogger _logger; protected bool _suppressBatchConfigUpdate = false; protected ObservableCollection _tracks = []; @@ -53,7 +54,7 @@ public bool IsDefaultTrack { _isDefaultTrack = value; OnPropertyChanged(nameof(IsDefaultTrack)); - UpdateBatchConfigTrackProperty(tc => tc.Default = value); + UpdateBatchConfigTrackProperty(tc => tc.Default = value, ftv => ftv.Default = value); } } } @@ -69,7 +70,7 @@ public bool IsDefaultFlagModificationEnabled { _isDefaultFlagModificationEnabled = value; OnPropertyChanged(nameof(IsDefaultFlagModificationEnabled)); - UpdateBatchConfigTrackProperty(tc => tc.ShouldModifyDefaultFlag = value); + UpdateGlobalModificationFlag(tc => tc.ShouldModifyDefaultFlag = value); } } } @@ -85,7 +86,7 @@ public bool IsEnabledTrack { _isEnabledTrack = value; OnPropertyChanged(nameof(IsEnabledTrack)); - UpdateBatchConfigTrackProperty(tc => tc.Enabled = value); + UpdateBatchConfigTrackProperty(tc => tc.Enabled = value, ftv => ftv.Enabled = value); } } } @@ -101,7 +102,7 @@ public bool IsEnabledFlagModificationEnabled { _isEnabledFlagModificationEnabled = value; OnPropertyChanged(nameof(IsEnabledFlagModificationEnabled)); - UpdateBatchConfigTrackProperty(tc => tc.ShouldModifyEnabledFlag = value); + UpdateGlobalModificationFlag(tc => tc.ShouldModifyEnabledFlag = value); } } } @@ -117,7 +118,7 @@ public bool IsForcedTrack { _isForcedTrack = value; OnPropertyChanged(nameof(IsForcedTrack)); - UpdateBatchConfigTrackProperty(tc => tc.Forced = value); + UpdateBatchConfigTrackProperty(tc => tc.Forced = value, ftv => ftv.Forced = value); } } } @@ -133,7 +134,7 @@ public bool IsForcedFlagModificationEnabled { _isForcedFlagModificationEnabled = value; OnPropertyChanged(nameof(IsForcedFlagModificationEnabled)); - UpdateBatchConfigTrackProperty(tc => tc.ShouldModifyForcedFlag = value); + UpdateGlobalModificationFlag(tc => tc.ShouldModifyForcedFlag = value); } } } @@ -149,7 +150,7 @@ public string TrackName { _trackName = value; OnPropertyChanged(nameof(TrackName)); - UpdateBatchConfigTrackProperty(tc => tc.Name = value); + UpdateBatchConfigTrackProperty(tc => tc.Name = value, ftv => ftv.Name = value); } } } @@ -165,7 +166,7 @@ public bool IsTrackNameModificationEnabled { _isTrackNameModificationEnabled = value; OnPropertyChanged(nameof(IsTrackNameModificationEnabled)); - UpdateBatchConfigTrackProperty(tc => tc.ShouldModifyName = value); + UpdateGlobalModificationFlag(tc => tc.ShouldModifyName = value); } } } @@ -181,7 +182,16 @@ public MatroskaLanguageOption? SelectedLanguage { _selectedLanguage = value; OnPropertyChanged(nameof(SelectedLanguage)); - UpdateBatchConfigTrackProperty(tc => tc.Language = value); + + // Only warn when null comes from outside the internal sync path (e.g., ComboBox clearing its selection). + // During internal resets (ApplyTrackProperties), _suppressBatchConfigUpdate is true and null is expected. + if (value is null && !_suppressBatchConfigUpdate) + { + LogSelectedLanguageNullFallback(); + } + + MatroskaLanguageOption language = value ?? MatroskaLanguageOption.Undetermined; + UpdateBatchConfigTrackProperty(tc => tc.Language = language, ftv => ftv.Language = language); } } } @@ -197,7 +207,7 @@ public bool IsSelectedLanguageModificationEnabled { _isSelectedLanguageModificationEnabled = value; OnPropertyChanged(nameof(IsSelectedLanguageModificationEnabled)); - UpdateBatchConfigTrackProperty(tc => tc.ShouldModifyLanguage = value); + UpdateGlobalModificationFlag(tc => tc.ShouldModifyLanguage = value); } } } @@ -209,12 +219,13 @@ public bool IsSelectedLanguageModificationEnabled public bool ShowTrackAvailabilityText => _uiPreferences.ShowTrackAvailabilityText; - protected TrackViewModelBase(ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences) + protected TrackViewModelBase(ILogger logger, ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences) { + _logger = logger; _batchConfiguration = batchConfiguration; _uiPreferences = uiPreferences; _languages = languageProvider.Languages; - + // Subscribe to property changes on the service _uiPreferences.PropertyChanged += OnUIPreferencesChanged; } @@ -270,10 +281,10 @@ public string GetTrackAvailabilityText(int trackIndex) { int available = GetTrackAvailabilityCount(trackIndex); int total = TotalFileCount; - + if (total == 0) return "0/0"; - + return $"{available}/{total}"; } @@ -358,18 +369,20 @@ protected virtual void OnTrackPropertyChanged(object? sender, PropertyChangedEve /// Updates the properties of the currently selected track in the batch configuration using the provided update action. /// /// - /// This method applies the provided update action to both: + /// This method applies the provided update actions to both: /// /// The global track collection (used for UI display) - triggers PropertyChanged which fires StateChanged - /// All per-file track configurations (used for command generation) - updated silently + /// All per-file track value collections (used for command generation) - updated silently /// /// The global track update will trigger the StateChanged event through its PropertyChanged handler, /// ensuring the UI and command generation stay synchronized. /// If updates are suppressed or no track is selected, the method performs no operation. /// - /// An delegate that defines the update to apply to the selected track's - /// configuration. - protected virtual void UpdateBatchConfigTrackProperty(Action updateAction) + /// An delegate that defines the update to apply to the global track configuration. + /// An delegate that defines the update to apply to each per-file track value. + protected virtual void UpdateBatchConfigTrackProperty( + Action globalUpdateAction, + Action perFileUpdateAction) { // If suppressing updates, do nothing to avoid (potential) recursion. if (_suppressBatchConfigUpdate) @@ -382,24 +395,50 @@ protected virtual void UpdateBatchConfigTrackProperty(Action if (index < 0 || index >= tracks.Count) return; - // First, update all per-file configurations silently (without triggering events) + // First, update all per-file value collections silently (without triggering events) // This ensures command generation uses the updated values var trackType = SelectedTrack.Type; foreach (var kvp in _batchConfiguration.FileConfigurations) { var fileConfig = kvp.Value; var fileTracks = fileConfig.GetTrackListForType(trackType); - + // Only update if this file actually has this track if (index >= 0 && index < fileTracks.Count) { - updateAction(fileTracks[index]); + perFileUpdateAction(fileTracks[index]); } } // Finally, apply the update action to the global track configuration // This will trigger PropertyChanged -> TrackConfiguration_PropertyChanged -> StateChanged // which updates CanProcessBatch and regenerates commands + globalUpdateAction(tracks[index]); + } + + /// + /// Updates only the global for a modification flag (e.g. ShouldModify*). + /// Per-file track values do not carry modification flags, so only the global track is updated. + /// + /// + /// If updates are suppressed or no track is selected, the method performs no operation. + /// + /// An delegate that sets the modification flag on the global track configuration. + protected virtual void UpdateGlobalModificationFlag(Action updateAction) + { + // If suppressing updates, do nothing to avoid (potential) recursion. + if (_suppressBatchConfigUpdate) + return; + if (SelectedTrack == null || GetTracks() == null) + return; + + int index = SelectedTrack.Index; + var tracks = GetTracks(); + if (index < 0 || index >= tracks.Count) + return; + + // Apply the update action to the global track configuration only + // This will trigger PropertyChanged -> TrackConfiguration_PropertyChanged -> StateChanged updateAction(tracks[index]); } @@ -422,38 +461,43 @@ protected virtual void OnSelectedTrackChanged(TrackConfiguration? newSelectedTra /// name="track"/> is , all track-related properties are reset to default values. private void ApplyTrackProperties(TrackConfiguration? track) { - // If suppressing updates, do nothing to avoid (potential) recursion. + // Suppress batch config updates while synchronizing properties to avoid (potential) recursion. _suppressBatchConfigUpdate = true; - // TODO: Need a better way to reset properties when no track is provided. - if (track is null) + try { - IsDefaultTrack = false; - IsEnabledTrack = true; - IsForcedTrack = false; - TrackName = string.Empty; - SelectedLanguage = null; - IsDefaultFlagModificationEnabled = false; - IsEnabledFlagModificationEnabled = false; - IsForcedFlagModificationEnabled = false; - IsTrackNameModificationEnabled = false; - IsSelectedLanguageModificationEnabled = false; + // TODO: Need a better way to reset properties when no track is provided. + if (track is null) + { + IsDefaultTrack = false; + IsEnabledTrack = true; + IsForcedTrack = false; + TrackName = string.Empty; + SelectedLanguage = null; + IsDefaultFlagModificationEnabled = false; + IsEnabledFlagModificationEnabled = false; + IsForcedFlagModificationEnabled = false; + IsTrackNameModificationEnabled = false; + IsSelectedLanguageModificationEnabled = false; + + return; + } - return; + // Synchronize properties with the selected track. + IsDefaultTrack = track.Default; + IsEnabledTrack = track.Enabled; + IsForcedTrack = track.Forced; + TrackName = track.Name; + SelectedLanguage = track.Language; + IsDefaultFlagModificationEnabled = track.ShouldModifyDefaultFlag; + IsEnabledFlagModificationEnabled = track.ShouldModifyEnabledFlag; + IsForcedFlagModificationEnabled = track.ShouldModifyForcedFlag; + IsTrackNameModificationEnabled = track.ShouldModifyName; + IsSelectedLanguageModificationEnabled = track.ShouldModifyLanguage; + } + finally + { + _suppressBatchConfigUpdate = false; } - - // Synchronize properties with the selected track. - IsDefaultTrack = track.Default; - IsEnabledTrack = track.Enabled; - IsForcedTrack = track.Forced; - TrackName = track.Name; - SelectedLanguage = track.Language; - IsDefaultFlagModificationEnabled = track.ShouldModifyDefaultFlag; - IsEnabledFlagModificationEnabled = track.ShouldModifyEnabledFlag; - IsForcedFlagModificationEnabled = track.ShouldModifyForcedFlag; - IsTrackNameModificationEnabled = track.ShouldModifyName; - IsSelectedLanguageModificationEnabled = track.ShouldModifyLanguage; - - _suppressBatchConfigUpdate = false; } } diff --git a/src/MatroskaBatchFlow.Uno/Presentation/VideoViewModel.cs b/src/MatroskaBatchFlow.Uno/Presentation/VideoViewModel.cs index 300a030..ebe4a16 100644 --- a/src/MatroskaBatchFlow.Uno/Presentation/VideoViewModel.cs +++ b/src/MatroskaBatchFlow.Uno/Presentation/VideoViewModel.cs @@ -28,8 +28,8 @@ public ObservableCollection VideoTracks /// public bool IsFileListPopulated => _batchConfiguration.FileList.Count > 0; - public VideoViewModel(ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences) - : base(languageProvider, batchConfiguration, uiPreferences) + public VideoViewModel(ILogger logger, ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences) + : base(logger, languageProvider, batchConfiguration, uiPreferences) { VideoTracks = [.. _batchConfiguration.VideoTracks]; diff --git a/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchConfigurationTests.cs b/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchConfigurationTests.cs index 164376a..956ee09 100644 --- a/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchConfigurationTests.cs +++ b/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchConfigurationTests.cs @@ -263,15 +263,39 @@ public void MigrateFileConfiguration_PreservesAllTrackTypes() var newFileId = Guid.NewGuid(); // Create file configuration with all track types - var audioTrack = new TrackConfiguration(new MediaInfoResultBuilder() - .AddTrackOfType(TrackType.Audio) - .Build().Media.Track.First(t => t.Type == TrackType.Audio)); - var videoTrack = new TrackConfiguration(new MediaInfoResultBuilder() - .AddTrackOfType(TrackType.Video) - .Build().Media.Track.First(t => t.Type == TrackType.Video)); - var subtitleTrack = new TrackConfiguration(new MediaInfoResultBuilder() - .AddTrackOfType(TrackType.Text) - .Build().Media.Track.First(t => t.Type == TrackType.Text)); + var audioTrack = new FileTrackValues + { + ScannedTrackInfo = new MediaInfoResultBuilder() + .AddTrackOfType(TrackType.Audio) + .Build().Media.Track.First(t => t.Type == TrackType.Audio), + Type = TrackType.Audio, + Index = 0, + Default = false, + Forced = false, + Enabled = false + }; + var videoTrack = new FileTrackValues + { + ScannedTrackInfo = new MediaInfoResultBuilder() + .AddTrackOfType(TrackType.Video) + .Build().Media.Track.First(t => t.Type == TrackType.Video), + Type = TrackType.Video, + Index = 0, + Default = false, + Forced = false, + Enabled = false + }; + var subtitleTrack = new FileTrackValues + { + ScannedTrackInfo = new MediaInfoResultBuilder() + .AddTrackOfType(TrackType.Text) + .Build().Media.Track.First(t => t.Type == TrackType.Text), + Type = TrackType.Text, + Index = 0, + Default = false, + Forced = false, + Enabled = false + }; var fileConfig = new FileTrackConfiguration { @@ -303,15 +327,18 @@ public void MigrateFileConfiguration_PreservesUserModifications() var oldFileId = Guid.NewGuid(); var newFileId = Guid.NewGuid(); - // Create audio track with user modifications - var audioTrack = new TrackConfiguration(new MediaInfoResultBuilder() - .AddTrackOfType(TrackType.Audio) - .Build().Media.Track.First(t => t.Type == TrackType.Audio)) + // Create audio track with user-modified values + var audioTrack = new FileTrackValues { + ScannedTrackInfo = new MediaInfoResultBuilder() + .AddTrackOfType(TrackType.Audio) + .Build().Media.Track.First(t => t.Type == TrackType.Audio), + Type = TrackType.Audio, + Index = 0, Name = "Modified Track Name", Default = true, - ShouldModifyName = true, - ShouldModifyDefaultFlag = true + Forced = false, + Enabled = false }; var fileConfig = new FileTrackConfiguration @@ -324,13 +351,11 @@ public void MigrateFileConfiguration_PreservesUserModifications() // Act config.MigrateFileConfiguration(oldFileId, newFileId); - // Assert - user modifications should be preserved + // Assert - user-modified values should be preserved after migration var migratedConfig = config.FileConfigurations[newFileId]; var migratedTrack = migratedConfig.AudioTracks[0]; Assert.Equal("Modified Track Name", migratedTrack.Name); Assert.True(migratedTrack.Default); - Assert.True(migratedTrack.ShouldModifyName); - Assert.True(migratedTrack.ShouldModifyDefaultFlag); } } diff --git a/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchTrackConfigurationInitializerTests.cs b/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchTrackConfigurationInitializerTests.cs index 61bd984..ebdd492 100644 --- a/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchTrackConfigurationInitializerTests.cs +++ b/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchTrackConfigurationInitializerTests.cs @@ -54,7 +54,7 @@ private static BatchTrackConfigurationInitializer CreateInitializer( { languageProvider ??= CreateMockLanguageProvider(); var trackConfigFactory = new TrackConfigurationFactory(languageProvider); - return new BatchTrackConfigurationInitializer(batchConfig, trackConfigFactory); + return new BatchTrackConfigurationInitializer(batchConfig, trackConfigFactory, languageProvider); } [Fact] @@ -233,4 +233,85 @@ public void Initialize_EmptyMediaInfo_CreatesConfigWithoutTracks() Assert.True(fileConfigs.ContainsKey(scannedFile.Id)); Assert.Empty(fileConfigs[scannedFile.Id].AudioTracks); } + + [Fact] + public void Initialize_PerFileTracksAreFileTrackValuesWithScannedData() + { + // Arrange + var (mockConfig, fileConfigs, _, _, _) = CreateMockConfig(); + var initializer = CreateInitializer(mockConfig); + + var mediaInfoResult = new MediaInfoResultBuilder() + .WithCreatingLibrary() + .AddTrackOfType(TrackType.Audio) + .AddTrackOfType(TrackType.Audio) + .Build(); + var scannedFile = new ScannedFileInfo(mediaInfoResult, "file.mkv"); + mockConfig.FileList.Add(scannedFile); + + // Act + initializer.Initialize(scannedFile, TrackType.Audio); + + // Assert - per-file tracks are FileTrackValues initialized from the scan + var fileConfig = fileConfigs[scannedFile.Id]; + Assert.Equal(2, fileConfig.AudioTracks.Count); + + Assert.Equal(TrackType.Audio, fileConfig.AudioTracks[0].Type); + Assert.Equal(0, fileConfig.AudioTracks[0].Index); + Assert.NotNull(fileConfig.AudioTracks[0].ScannedTrackInfo); + + Assert.Equal(TrackType.Audio, fileConfig.AudioTracks[1].Type); + Assert.Equal(1, fileConfig.AudioTracks[1].Index); + Assert.NotNull(fileConfig.AudioTracks[1].ScannedTrackInfo); + } + + [Fact] + public void Initialize_FilesAddedAfterUserConfigure_ReceiveFileTrackValues() + { + // Regression test for issue #85: files added after processing were skipped because + // per-file TrackConfiguration objects always had ShouldModify* = false. + // Now per-file tracks are FileTrackValues with no ShouldModify* fields, + // so the argument generator reads ShouldModify* from global tracks for every file. + // Arrange + var (mockConfig, fileConfigs, audioTracks, _, _) = CreateMockConfig(); + var initializer = CreateInitializer(mockConfig); + + var firstFile = new ScannedFileInfo( + new MediaInfoResultBuilder() + .WithCreatingLibrary() + .AddTrackOfType(TrackType.Audio) + .Build(), + "file1.mkv" + ); + mockConfig.FileList.Add(firstFile); + initializer.Initialize(firstFile, TrackType.Audio); + + // Simulate user enabling modifications on the global track + audioTracks[0].ShouldModifyLanguage = true; + audioTracks[0].ShouldModifyName = true; + + // Add a second file after modifications were configured (this was the bug scenario) + var secondFile = new ScannedFileInfo( + new MediaInfoResultBuilder() + .WithCreatingLibrary() + .AddTrackOfType(TrackType.Audio) + .Build(), + "file2.mkv" + ); + mockConfig.FileList.Add(secondFile); + + // Act + initializer.Initialize(secondFile, TrackType.Audio); + + // Assert - second file receives FileTrackValues (not TrackConfiguration with ShouldModify* = false) + Assert.True(fileConfigs.ContainsKey(secondFile.Id)); + var secondFileConfig = fileConfigs[secondFile.Id]; + Assert.Single(secondFileConfig.AudioTracks); + Assert.Equal(TrackType.Audio, secondFileConfig.AudioTracks[0].Type); + Assert.Equal(0, secondFileConfig.AudioTracks[0].Index); + + // Global tracks still hold the ShouldModify* flags set by the user + Assert.True(audioTracks[0].ShouldModifyLanguage); + Assert.True(audioTracks[0].ShouldModifyName); + } } diff --git a/tests/MatroskaBatchFlow.Core.UnitTests/Services/LanguageProviderTests.cs b/tests/MatroskaBatchFlow.Core.UnitTests/Services/LanguageProviderTests.cs index 9a05872..06b924b 100644 --- a/tests/MatroskaBatchFlow.Core.UnitTests/Services/LanguageProviderTests.cs +++ b/tests/MatroskaBatchFlow.Core.UnitTests/Services/LanguageProviderTests.cs @@ -194,6 +194,150 @@ public void Languages_IsImmutable() Assert.IsAssignableFrom>(provider.Languages); } + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Resolve_ReturnsUndetermined_WhenLanguageCodeIsNullOrWhitespace(string? languageCode) + { + // Arrange + var provider = CreateProviderWithTestLanguages(); + + // Act + var result = provider.Resolve(languageCode); + + // Assert + Assert.Equal(MatroskaLanguageOption.Undetermined, result); + } + + [Fact] + public void Resolve_ReturnsUndetermined_WhenNoMatchFound() + { + // Arrange + var provider = CreateProviderWithTestLanguages(); + + // Act + var result = provider.Resolve("nonexistent"); + + // Assert + Assert.Equal(MatroskaLanguageOption.Undetermined, result); + } + + [Fact] + public void Resolve_MatchesByIso639_2_b() + { + // Arrange — French has iso639_2_b "fre" (distinct from iso639_2_t "fra") + var provider = CreateProviderWithTestLanguages(); + + // Act + var result = provider.Resolve("fre"); + + // Assert + Assert.Equal("French", result.Name); + } + + [Fact] + public void Resolve_MatchesByIso639_2_t() + { + // Arrange — French has iso639_2_t "fra" (distinct from iso639_2_b "fre") + var provider = CreateProviderWithTestLanguages(); + + // Act + var result = provider.Resolve("fra"); + + // Assert + Assert.Equal("French", result.Name); + } + + [Fact] + public void Resolve_MatchesByIso639_1() + { + // Arrange + var provider = CreateProviderWithTestLanguages(); + + // Act + var result = provider.Resolve("en"); + + // Assert + Assert.Equal("English", result.Name); + } + + [Fact] + public void Resolve_MatchesByIso639_3() + { + // Arrange — "cmn" only appears in the ISO 639-3 field, so this isolates that match path. + var provider = CreateProviderWithLanguageWithoutIso639_1(); + + // Act + var result = provider.Resolve("cmn"); + + // Assert + Assert.Equal("Chinese", result.Name); + } + + [Fact] + public void Resolve_MatchesByName() + { + // Arrange + var provider = CreateProviderWithTestLanguages(); + + // Act + var result = provider.Resolve("French"); + + // Assert + Assert.Equal("French", result.Name); + } + + [Fact] + public void Resolve_MatchesByCode() + { + // Arrange — Code is derived: iso639_1 ?? iso639_2_b, so "fr" for French. + // "fr" also matches iso639_1 directly. Use a language with null iso639_1 + // to isolate the Code path. + var provider = CreateProviderWithLanguageWithoutIso639_1(); + + // Act — Code falls back to iso639_2_b "zho" when iso639_1 is null + var result = provider.Resolve("zho"); + + // Assert + Assert.Equal("Chinese", result.Name); + } + + [Fact] + public void Resolve_IsCaseInsensitive() + { + // Arrange + var provider = CreateProviderWithTestLanguages(); + + // Act + var result = provider.Resolve("ENG"); + + // Assert + Assert.Equal("English", result.Name); + } + + private LanguageProvider CreateProviderWithTestLanguages() + { + var testFile = CreateTestLanguageFile("resolve_test.json"); + var options = CreateOptions(testFile); + return new LanguageProvider(options, _logger); + } + + private LanguageProvider CreateProviderWithLanguageWithoutIso639_1() + { + var testFile = Path.Combine(_testFilesDirectory, "resolve_no_iso1.json"); + var json = """ + { + "languages": [ + { "name": "Chinese", "iso639_1": null, "iso639_2_b": "zho", "iso639_2_t": "chi", "iso639_3": "cmn" } + ] + } + """; + File.WriteAllText(testFile, json); + var options = CreateOptions(testFile); + return new LanguageProvider(options, _logger); + } + private static IOptions CreateOptions(string filePath) { var options = Options.Create(new LanguageOptions { FilePath = filePath }); diff --git a/tests/MatroskaBatchFlow.Core.UnitTests/Services/TrackConfigurationFactoryTests.cs b/tests/MatroskaBatchFlow.Core.UnitTests/Services/TrackConfigurationFactoryTests.cs index 9505027..c0274f3 100644 --- a/tests/MatroskaBatchFlow.Core.UnitTests/Services/TrackConfigurationFactoryTests.cs +++ b/tests/MatroskaBatchFlow.Core.UnitTests/Services/TrackConfigurationFactoryTests.cs @@ -1,4 +1,3 @@ -using System.Collections.Immutable; using MatroskaBatchFlow.Core.Enums; using MatroskaBatchFlow.Core.Models; using MatroskaBatchFlow.Core.Services; @@ -13,18 +12,19 @@ namespace MatroskaBatchFlow.Core.UnitTests.Services; /// public class TrackConfigurationFactoryTests { - private static ILanguageProvider CreateMockLanguageProvider(IEnumerable? languages = null) + private readonly ILanguageProvider _mockLanguageProvider = Substitute.For(); + + public TrackConfigurationFactoryTests() { - var mockLanguageProvider = Substitute.For(); - mockLanguageProvider.Languages.Returns(languages?.ToImmutableList() ?? ImmutableList.Empty); - return mockLanguageProvider; + // Default: any unmatched code resolves to Undetermined. + _mockLanguageProvider.Resolve(Arg.Any()).Returns(MatroskaLanguageOption.Undetermined); } [Fact] public void Create_SetsBasicProperties() { // Arrange - var factory = new TrackConfigurationFactory(CreateMockLanguageProvider()); + var factory = new TrackConfigurationFactory(_mockLanguageProvider); var trackInfo = new TrackInfoBuilder() .WithType(TrackType.Audio) .WithTitle("English Commentary") @@ -49,7 +49,7 @@ public void Create_SetsBasicProperties() public void Create_SetsEmptyNameWhenTitleIsNull() { // Arrange - var factory = new TrackConfigurationFactory(CreateMockLanguageProvider()); + var factory = new TrackConfigurationFactory(_mockLanguageProvider); var trackInfo = new TrackInfoBuilder() .WithType(TrackType.Video) .WithStreamKindID(0) @@ -63,7 +63,7 @@ public void Create_SetsEmptyNameWhenTitleIsNull() } [Fact] - public void Create_ResolvesLanguageFromIso639_2_b() + public void Create_DelegatesToResolveAndUsesResult() { // Arrange var englishLanguage = new MatroskaLanguageOption( @@ -72,7 +72,8 @@ public void Create_ResolvesLanguageFromIso639_2_b() iso639_2_b: "eng", iso639_2_t: "eng", iso639_3: "eng"); - var factory = new TrackConfigurationFactory(CreateMockLanguageProvider([englishLanguage])); + _mockLanguageProvider.Resolve("eng").Returns(englishLanguage); + var factory = new TrackConfigurationFactory(_mockLanguageProvider); var trackInfo = new TrackInfoBuilder() .WithType(TrackType.Audio) .WithLanguage("eng") @@ -87,7 +88,7 @@ public void Create_ResolvesLanguageFromIso639_2_b() } [Fact] - public void Create_ResolvesLanguageFromIso639_1() + public void Create_PassesLanguageCodeToResolve() { // Arrange var japaneseLanguage = new MatroskaLanguageOption( @@ -96,7 +97,8 @@ public void Create_ResolvesLanguageFromIso639_1() iso639_2_b: "jpn", iso639_2_t: "jpn", iso639_3: "jpn"); - var factory = new TrackConfigurationFactory(CreateMockLanguageProvider([japaneseLanguage])); + _mockLanguageProvider.Resolve("ja").Returns(japaneseLanguage); + var factory = new TrackConfigurationFactory(_mockLanguageProvider); var trackInfo = new TrackInfoBuilder() .WithType(TrackType.Audio) .WithLanguage("ja") @@ -114,7 +116,7 @@ public void Create_ResolvesLanguageFromIso639_1() public void Create_ReturnsUndeterminedForUnknownLanguage() { // Arrange - var factory = new TrackConfigurationFactory(CreateMockLanguageProvider()); + var factory = new TrackConfigurationFactory(_mockLanguageProvider); var trackInfo = new TrackInfoBuilder() .WithType(TrackType.Audio) .WithLanguage("xyz") @@ -132,7 +134,7 @@ public void Create_ReturnsUndeterminedForUnknownLanguage() public void Create_ReturnsUndeterminedForEmptyLanguage() { // Arrange - var factory = new TrackConfigurationFactory(CreateMockLanguageProvider()); + var factory = new TrackConfigurationFactory(_mockLanguageProvider); var trackInfo = new TrackInfoBuilder() .WithType(TrackType.Audio) .WithLanguage(string.Empty) @@ -150,7 +152,7 @@ public void Create_ReturnsUndeterminedForEmptyLanguage() public void Create_ReturnsUndeterminedForWhitespaceLanguage() { // Arrange - var factory = new TrackConfigurationFactory(CreateMockLanguageProvider()); + var factory = new TrackConfigurationFactory(_mockLanguageProvider); var trackInfo = new TrackInfoBuilder() .WithType(TrackType.Audio) .WithLanguage(" ") @@ -168,7 +170,7 @@ public void Create_ReturnsUndeterminedForWhitespaceLanguage() public void Create_ThrowsForNullTrackInfo() { // Arrange - var factory = new TrackConfigurationFactory(CreateMockLanguageProvider()); + var factory = new TrackConfigurationFactory(_mockLanguageProvider); // Act & Assert Assert.Throws(() => factory.Create(null!, TrackType.Audio, 0)); diff --git a/tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs b/tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs index b86e957..fe678c5 100644 --- a/tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs +++ b/tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs @@ -61,7 +61,7 @@ public async Task IntegrationTest_EditingTrackOnlyInSecondFile_TriggersStateChan // Initialize per-file configurations var trackConfigFactory = new TrackConfigurationFactory(mockLanguageProvider); - var initializer = new BatchTrackConfigurationInitializer(batchConfig, trackConfigFactory); + var initializer = new BatchTrackConfigurationInitializer(batchConfig, trackConfigFactory, mockLanguageProvider); initializer.Initialize(file1, TrackType.Text); initializer.Initialize(file2, TrackType.Text); @@ -70,7 +70,7 @@ public async Task IntegrationTest_EditingTrackOnlyInSecondFile_TriggersStateChan batchConfig.StateChanged += (_, __) => tcs.TrySetResult(true); // Create SubtitleViewModel using real BatchConfiguration - var subtitleViewModel = new SubtitleViewModel(mockLanguageProvider, batchConfig, mockUIPreferences); + var subtitleViewModel = new SubtitleViewModel(Substitute.For>(), mockLanguageProvider, batchConfig, mockUIPreferences); // Select track 17 (index 16) - only exists in file2 Assert.Equal(17, batchConfig.SubtitleTracks.Count); @@ -104,10 +104,9 @@ public async Task IntegrationTest_EditingTrackOnlyInSecondFile_TriggersStateChan var file2Config = batchConfig.FileConfigurations[file2.Id]; Assert.Equal(17, file2Config.SubtitleTracks.Count); Assert.Equal("Track17Modified", file2Config.SubtitleTracks[16].Name); - Assert.True(file2Config.SubtitleTracks[16].ShouldModifyName); // Verify command generation works correctly - var argumentsGenerator = new MkvPropeditArgumentsGenerator(); + var argumentsGenerator = new MkvPropeditArgumentsGenerator(Substitute.For>()); var commands = argumentsGenerator.BuildBatchArguments(batchConfig); Assert.Single(commands); diff --git a/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/AudioViewModelTests.cs b/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/AudioViewModelTests.cs index 2b20719..0fc4f09 100644 --- a/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/AudioViewModelTests.cs +++ b/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/AudioViewModelTests.cs @@ -7,6 +7,7 @@ using MatroskaBatchFlow.Core.Utilities; using MatroskaBatchFlow.Uno.Contracts.Services; using MatroskaBatchFlow.Uno.Presentation; +using Microsoft.Extensions.Logging; using NSubstitute; namespace MatroskaBatchFlow.Uno.UnitTests.Presentation; @@ -16,12 +17,14 @@ public class AudioViewModelTests private readonly ILanguageProvider _languageProvider; private readonly IBatchConfiguration _batchConfiguration; private readonly IUIPreferencesService _uiPreferences; + private readonly ILogger _logger; public AudioViewModelTests() { _languageProvider = Substitute.For(); _batchConfiguration = Substitute.For(); _uiPreferences = Substitute.For(); + _logger = Substitute.For>(); _languageProvider.Languages.Returns([]); _batchConfiguration.AudioTracks.Returns(new ObservableCollection()); @@ -46,7 +49,7 @@ public void Constructor_InitializesAudioTracksFromBatchConfiguration() _batchConfiguration.AudioTracks.Returns(audioTracks); // Act - var viewModel = new AudioViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new AudioViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.Single(viewModel.AudioTracks); @@ -70,7 +73,7 @@ public void Constructor_SetsSelectedTrackToFirstTrack() _batchConfiguration.AudioTracks.Returns(audioTracks); // Act - var viewModel = new AudioViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new AudioViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.NotNull(viewModel.SelectedTrack); @@ -91,7 +94,7 @@ public void IsFileListPopulated_ReturnsTrueWhenFilesExist() _batchConfiguration.FileList.Returns(fileList); // Act - var viewModel = new AudioViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new AudioViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.True(viewModel.IsFileListPopulated); @@ -103,7 +106,7 @@ public void IsFileListPopulated_ReturnsFalseWhenNoFilesExist() // Arrange - fileList is empty by default // Act - var viewModel = new AudioViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new AudioViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.False(viewModel.IsFileListPopulated); @@ -116,7 +119,7 @@ public void OnFileListChanged_UpdatesIsFileListPopulated() var fileList = new UniqueObservableCollection(Substitute.For()); _batchConfiguration.FileList.Returns(fileList); - var viewModel = new AudioViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new AudioViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); Assert.False(viewModel.IsFileListPopulated); var mediaInfoResult = new MediaInfoResultBuilder() @@ -145,7 +148,7 @@ public void OnBatchConfigurationAudioTracksChanged_UpdatesAudioTracks() var audioTracks = new ObservableCollection(); _batchConfiguration.AudioTracks.Returns(audioTracks); - var viewModel = new AudioViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new AudioViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); Assert.Empty(viewModel.AudioTracks); var mediaInfoResult = new MediaInfoResultBuilder() @@ -169,7 +172,7 @@ public void OnBatchConfigurationChanged_UpdatesAudioTracksWhenAudioTracksPropert var initialTracks = new ObservableCollection(); _batchConfiguration.AudioTracks.Returns(initialTracks); - var viewModel = new AudioViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new AudioViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); var mediaInfoResult = new MediaInfoResultBuilder() .WithCreatingLibrary() @@ -199,7 +202,7 @@ public void OnBatchConfigurationChanged_DoesNotUpdateWhenOtherPropertiesChange() var audioTracks = new ObservableCollection(); _batchConfiguration.AudioTracks.Returns(audioTracks); - var viewModel = new AudioViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new AudioViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); int audioTracksChangedCount = 0; viewModel.PropertyChanged += (s, e) => { diff --git a/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/SubtitleViewModelTests.cs b/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/SubtitleViewModelTests.cs index 54b0273..ec7effd 100644 --- a/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/SubtitleViewModelTests.cs +++ b/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/SubtitleViewModelTests.cs @@ -7,6 +7,7 @@ using MatroskaBatchFlow.Core.Utilities; using MatroskaBatchFlow.Uno.Contracts.Services; using MatroskaBatchFlow.Uno.Presentation; +using Microsoft.Extensions.Logging; using NSubstitute; namespace MatroskaBatchFlow.Uno.UnitTests.Presentation; @@ -16,12 +17,14 @@ public class SubtitleViewModelTests private readonly ILanguageProvider _languageProvider; private readonly IBatchConfiguration _batchConfiguration; private readonly IUIPreferencesService _uiPreferences; + private readonly ILogger _logger; public SubtitleViewModelTests() { _languageProvider = Substitute.For(); _batchConfiguration = Substitute.For(); _uiPreferences = Substitute.For(); + _logger = Substitute.For>(); _languageProvider.Languages.Returns([]); _batchConfiguration.SubtitleTracks.Returns(new ObservableCollection()); @@ -46,7 +49,7 @@ public void Constructor_InitializesSubtitleTracksFromBatchConfiguration() _batchConfiguration.SubtitleTracks.Returns(subtitleTracks); // Act - var viewModel = new SubtitleViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new SubtitleViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.Single(viewModel.SubtitleTracks); @@ -70,7 +73,7 @@ public void Constructor_SetsSelectedTrackToFirstTrack() _batchConfiguration.SubtitleTracks.Returns(subtitleTracks); // Act - var viewModel = new SubtitleViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new SubtitleViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.NotNull(viewModel.SelectedTrack); @@ -91,7 +94,7 @@ public void IsFileListPopulated_ReturnsTrueWhenFilesExist() _batchConfiguration.FileList.Returns(fileList); // Act - var viewModel = new SubtitleViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new SubtitleViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.True(viewModel.IsFileListPopulated); @@ -103,7 +106,7 @@ public void IsFileListPopulated_ReturnsFalseWhenNoFilesExist() // Arrange - fileList is empty by default // Act - var viewModel = new SubtitleViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new SubtitleViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.False(viewModel.IsFileListPopulated); @@ -116,7 +119,7 @@ public void OnFileListChanged_UpdatesIsFileListPopulated() var fileList = new UniqueObservableCollection(Substitute.For()); _batchConfiguration.FileList.Returns(fileList); - var viewModel = new SubtitleViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new SubtitleViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); Assert.False(viewModel.IsFileListPopulated); var mediaInfoResult = new MediaInfoResultBuilder() @@ -145,7 +148,7 @@ public void OnBatchConfigurationSubtitleTracksChanged_UpdatesSubtitleTracks() var subtitleTracks = new ObservableCollection(); _batchConfiguration.SubtitleTracks.Returns(subtitleTracks); - var viewModel = new SubtitleViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new SubtitleViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); Assert.Empty(viewModel.SubtitleTracks); var mediaInfoResult = new MediaInfoResultBuilder() @@ -169,7 +172,7 @@ public void OnBatchConfigurationChanged_UpdatesSubtitleTracksWhenSubtitleTracksP var initialTracks = new ObservableCollection(); _batchConfiguration.SubtitleTracks.Returns(initialTracks); - var viewModel = new SubtitleViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new SubtitleViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); var mediaInfoResult = new MediaInfoResultBuilder() .WithCreatingLibrary() @@ -199,7 +202,7 @@ public void OnBatchConfigurationChanged_DoesNotUpdateWhenOtherPropertiesChange() var subtitleTracks = new ObservableCollection(); _batchConfiguration.SubtitleTracks.Returns(subtitleTracks); - var viewModel = new SubtitleViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new SubtitleViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); int subtitleTracksChangedCount = 0; viewModel.PropertyChanged += (s, e) => { diff --git a/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/TrackViewModelBaseTests.cs b/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/TrackViewModelBaseTests.cs index 7a91793..07b55a3 100644 --- a/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/TrackViewModelBaseTests.cs +++ b/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/TrackViewModelBaseTests.cs @@ -6,6 +6,7 @@ using MatroskaBatchFlow.Core.Utilities; using MatroskaBatchFlow.Uno.Contracts.Services; using MatroskaBatchFlow.Uno.Presentation; +using Microsoft.Extensions.Logging; using NSubstitute; namespace MatroskaBatchFlow.Uno.UnitTests.Presentation; @@ -19,6 +20,8 @@ namespace MatroskaBatchFlow.Uno.UnitTests.Presentation; /// facilitate testing scenarios. public class TrackViewModelBaseTests { + private readonly ILogger _mockLogger = Substitute.For(); + /// /// Test implementation of TrackViewModelBase for testing purposes /// @@ -26,8 +29,8 @@ private class TestTrackViewModel : TrackViewModelBase { private readonly IList _testTracks; - public TestTrackViewModel(ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences, IList tracks) - : base(languageProvider, batchConfiguration, uiPreferences) + public TestTrackViewModel(ILogger logger, ILanguageProvider languageProvider, IBatchConfiguration batchConfiguration, IUIPreferencesService uiPreferences, IList tracks) + : base(logger, languageProvider, batchConfiguration, uiPreferences) { _testTracks = tracks; SetupEventHandlers(); @@ -74,10 +77,10 @@ public void UpdateBatchConfigTrackProperty_UpdatesGlobalAndPerFileConfigurations var file2 = new ScannedFileInfo(mediaInfoResult, "file2.mkv"); var file1Config = new FileTrackConfiguration { FilePath = file1.Path }; - file1Config.SubtitleTracks.Add(new TrackConfiguration(trackInfo) { Type = TrackType.Text, Index = 0, Name = "Original" }); + file1Config.SubtitleTracks.Add(new FileTrackValues { ScannedTrackInfo = trackInfo, Type = TrackType.Text, Index = 0, Name = "Original", Default = false, Forced = false, Enabled = false }); var file2Config = new FileTrackConfiguration { FilePath = file2.Path }; - file2Config.SubtitleTracks.Add(new TrackConfiguration(trackInfo) { Type = TrackType.Text, Index = 0, Name = "Original" }); + file2Config.SubtitleTracks.Add(new FileTrackValues { ScannedTrackInfo = trackInfo, Type = TrackType.Text, Index = 0, Name = "Original", Default = false, Forced = false, Enabled = false }); var fileConfigurations = new Dictionary { @@ -88,7 +91,7 @@ public void UpdateBatchConfigTrackProperty_UpdatesGlobalAndPerFileConfigurations mockBatchConfig.FileConfigurations.Returns(fileConfigurations); // Create view model - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); // Set the selected track viewModel.SelectedTrack = globalTracks[0]; @@ -103,10 +106,8 @@ public void UpdateBatchConfigTrackProperty_UpdatesGlobalAndPerFileConfigurations // Assert - verify per-file configurations were also updated Assert.Equal("Updated Name", file1Config.SubtitleTracks[0].Name); - Assert.True(file1Config.SubtitleTracks[0].ShouldModifyName); Assert.Equal("Updated Name", file2Config.SubtitleTracks[0].Name); - Assert.True(file2Config.SubtitleTracks[0].ShouldModifyName); } [Fact] @@ -132,7 +133,7 @@ public void UpdateBatchConfigTrackProperty_SkipsFilesWithoutTrack() // File1 has track 0 var file1 = new ScannedFileInfo(mediaInfoResult, "file1.mkv"); var file1Config = new FileTrackConfiguration { FilePath = file1.Path }; - file1Config.SubtitleTracks.Add(new TrackConfiguration(trackInfo) { Type = TrackType.Text, Index = 0, Name = "Original" }); + file1Config.SubtitleTracks.Add(new FileTrackValues { ScannedTrackInfo = trackInfo, Type = TrackType.Text, Index = 0, Name = "Original", Default = false, Forced = false, Enabled = false }); // File2 has NO tracks (different track count) var file2 = new ScannedFileInfo(mediaInfoResult, "file2.mkv"); @@ -147,7 +148,7 @@ public void UpdateBatchConfigTrackProperty_SkipsFilesWithoutTrack() mockBatchConfig.FileConfigurations.Returns(fileConfigurations); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); viewModel.SelectedTrack = globalTracks[0]; // Act - enable modification and update track name @@ -182,7 +183,7 @@ public void SelectedTrack_WhenSetToNull_ResetsAllPropertiesToDefault() mockBatchConfig.FileConfigurations.Returns(new Dictionary()); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); viewModel.SelectedTrack = globalTracks[0]; // Act - Set selected track to null @@ -222,7 +223,7 @@ public void IsTrackSelected_ReturnsTrueWhenTrackIsSelected() mockBatchConfig.FileConfigurations.Returns(new Dictionary()); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); // Act viewModel.SelectedTrack = globalTracks[0]; @@ -247,7 +248,7 @@ public void IsTrackSelected_ReturnsFalseWhenNoTrackIsSelected() var globalTracks = new List(); mockBatchConfig.FileConfigurations.Returns(new Dictionary()); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); // Act & Assert Assert.False(viewModel.IsTrackSelected); @@ -274,7 +275,7 @@ public void IsDefaultTrack_UpdatesBatchConfiguration_WhenChanged() var file1 = new ScannedFileInfo(mediaInfoResult, "file1.mkv"); var file1Config = new FileTrackConfiguration { FilePath = file1.Path }; - file1Config.SubtitleTracks.Add(new TrackConfiguration(trackInfo) { Type = TrackType.Text, Index = 0, Default = false }); + file1Config.SubtitleTracks.Add(new FileTrackValues { ScannedTrackInfo = trackInfo, Type = TrackType.Text, Index = 0, Default = false, Forced = false, Enabled = false }); var fileConfigurations = new Dictionary { @@ -283,7 +284,7 @@ public void IsDefaultTrack_UpdatesBatchConfiguration_WhenChanged() mockBatchConfig.FileConfigurations.Returns(fileConfigurations); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); viewModel.SelectedTrack = globalTracks[0]; // Act @@ -315,7 +316,7 @@ public void IsForcedTrack_UpdatesBatchConfiguration_WhenChanged() var file1 = new ScannedFileInfo(mediaInfoResult, "file1.mkv"); var file1Config = new FileTrackConfiguration { FilePath = file1.Path }; - file1Config.SubtitleTracks.Add(new TrackConfiguration(trackInfo) { Type = TrackType.Text, Index = 0, Forced = false }); + file1Config.SubtitleTracks.Add(new FileTrackValues { ScannedTrackInfo = trackInfo, Type = TrackType.Text, Index = 0, Default = false, Forced = false, Enabled = false }); var fileConfigurations = new Dictionary { @@ -324,7 +325,7 @@ public void IsForcedTrack_UpdatesBatchConfiguration_WhenChanged() mockBatchConfig.FileConfigurations.Returns(fileConfigurations); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); viewModel.SelectedTrack = globalTracks[0]; // Act @@ -356,7 +357,7 @@ public void SelectedLanguage_UpdatesBatchConfiguration_WhenChanged() var file1 = new ScannedFileInfo(mediaInfoResult, "file1.mkv"); var file1Config = new FileTrackConfiguration { FilePath = file1.Path }; - file1Config.SubtitleTracks.Add(new TrackConfiguration(trackInfo) { Type = TrackType.Text, Index = 0, Language = MatroskaLanguageOption.Undetermined }); + file1Config.SubtitleTracks.Add(new FileTrackValues { ScannedTrackInfo = trackInfo, Type = TrackType.Text, Index = 0, Language = MatroskaLanguageOption.Undetermined, Default = false, Forced = false, Enabled = false }); var fileConfigurations = new Dictionary { @@ -365,7 +366,7 @@ public void SelectedLanguage_UpdatesBatchConfiguration_WhenChanged() mockBatchConfig.FileConfigurations.Returns(fileConfigurations); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, globalTracks); viewModel.SelectedTrack = globalTracks[0]; var newLanguage = new MatroskaLanguageOption("English", "en", "eng", "eng", "eng"); @@ -407,7 +408,7 @@ public void GetTrackAvailabilityCount_ReturnsCorrectCount() mockBatchConfig.FileList.Returns(fileList); mockBatchConfig.FileConfigurations.Returns(new Dictionary()); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); // Act int count0 = viewModel.GetTrackAvailabilityCount(0); @@ -439,7 +440,7 @@ public void GetTrackAvailabilityText_ReturnsFormattedString() mockBatchConfig.FileList.Returns(fileList); mockBatchConfig.FileConfigurations.Returns(new Dictionary()); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); // Act string result = viewModel.GetTrackAvailabilityText(0); @@ -460,7 +461,7 @@ public void TotalFileCount_ReturnsCorrectCount() mockBatchConfig.FileList.Returns(fileList); mockBatchConfig.FileConfigurations.Returns(new Dictionary()); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); // Act & Assert Assert.Equal(0, viewModel.TotalFileCount); @@ -478,7 +479,7 @@ public void ShowTrackAvailabilityText_ReflectsUIPreferencesValue() mockBatchConfig.FileConfigurations.Returns(new Dictionary()); // Act - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); // Assert Assert.True(viewModel.ShowTrackAvailabilityText); @@ -495,7 +496,7 @@ public void UIPreferences_PropertyChanged_UpdatesShowTrackAvailabilityText() mockUIPreferences.ShowTrackAvailabilityText.Returns(false); mockBatchConfig.FileConfigurations.Returns(new Dictionary()); - var viewModel = new TestTrackViewModel(mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); + var viewModel = new TestTrackViewModel(_mockLogger, mockLanguageProvider, mockBatchConfig, mockUIPreferences, new List()); Assert.False(viewModel.ShowTrackAvailabilityText); // Act diff --git a/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/VideoViewModelTests.cs b/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/VideoViewModelTests.cs index 9042330..70eba8f 100644 --- a/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/VideoViewModelTests.cs +++ b/tests/MatroskaBatchFlow.Uno.UnitTests/Presentation/VideoViewModelTests.cs @@ -7,6 +7,7 @@ using MatroskaBatchFlow.Core.Utilities; using MatroskaBatchFlow.Uno.Contracts.Services; using MatroskaBatchFlow.Uno.Presentation; +using Microsoft.Extensions.Logging; using NSubstitute; namespace MatroskaBatchFlow.Uno.UnitTests.Presentation; @@ -16,12 +17,14 @@ public class VideoViewModelTests private readonly ILanguageProvider _languageProvider; private readonly IBatchConfiguration _batchConfiguration; private readonly IUIPreferencesService _uiPreferences; + private readonly ILogger _logger; public VideoViewModelTests() { _languageProvider = Substitute.For(); _batchConfiguration = Substitute.For(); _uiPreferences = Substitute.For(); + _logger = Substitute.For>(); _languageProvider.Languages.Returns([]); _batchConfiguration.VideoTracks.Returns(new ObservableCollection()); @@ -46,7 +49,7 @@ public void Constructor_InitializesVideoTracksFromBatchConfiguration() _batchConfiguration.VideoTracks.Returns(videoTracks); // Act - var viewModel = new VideoViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new VideoViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.Single(viewModel.VideoTracks); @@ -70,7 +73,7 @@ public void Constructor_SetsSelectedTrackToFirstTrack() _batchConfiguration.VideoTracks.Returns(videoTracks); // Act - var viewModel = new VideoViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new VideoViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.NotNull(viewModel.SelectedTrack); @@ -91,7 +94,7 @@ public void IsFileListPopulated_ReturnsTrueWhenFilesExist() _batchConfiguration.FileList.Returns(fileList); // Act - var viewModel = new VideoViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new VideoViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.True(viewModel.IsFileListPopulated); @@ -103,7 +106,7 @@ public void IsFileListPopulated_ReturnsFalseWhenNoFilesExist() // Arrange - fileList is empty by default // Act - var viewModel = new VideoViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new VideoViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); // Assert Assert.False(viewModel.IsFileListPopulated); @@ -116,7 +119,7 @@ public void OnFileListChanged_UpdatesIsFileListPopulated() var fileList = new UniqueObservableCollection(Substitute.For()); _batchConfiguration.FileList.Returns(fileList); - var viewModel = new VideoViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new VideoViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); Assert.False(viewModel.IsFileListPopulated); var mediaInfoResult = new MediaInfoResultBuilder() @@ -145,7 +148,7 @@ public void OnBatchConfigurationVideoTracksChanged_UpdatesVideoTracks() var videoTracks = new ObservableCollection(); _batchConfiguration.VideoTracks.Returns(videoTracks); - var viewModel = new VideoViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new VideoViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); Assert.Empty(viewModel.VideoTracks); var mediaInfoResult = new MediaInfoResultBuilder() @@ -169,7 +172,7 @@ public void OnBatchConfigurationChanged_UpdatesVideoTracksWhenVideoTracksPropert var initialTracks = new ObservableCollection(); _batchConfiguration.VideoTracks.Returns(initialTracks); - var viewModel = new VideoViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new VideoViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); var mediaInfoResult = new MediaInfoResultBuilder() .WithCreatingLibrary() @@ -199,7 +202,7 @@ public void OnBatchConfigurationChanged_DoesNotUpdateWhenOtherPropertiesChange() var videoTracks = new ObservableCollection(); _batchConfiguration.VideoTracks.Returns(videoTracks); - var viewModel = new VideoViewModel(_languageProvider, _batchConfiguration, _uiPreferences); + var viewModel = new VideoViewModel(_logger, _languageProvider, _batchConfiguration, _uiPreferences); int videoTracksChangedCount = 0; viewModel.PropertyChanged += (s, e) => { From adf7ba7a6f192008a20384211759a57b40db4200 Mon Sep 17 00:00:00 2001 From: Tim Gels Date: Fri, 6 Mar 2026 21:42:10 +0100 Subject: [PATCH 2/4] refactor(vm): improve SelectedLanguage null warning log message - Rename LogSelectedLanguageNullFallback to LogSelectedLanguageReceivedNull - Simplify message to state the fact without assuming the source --- .../Presentation/TrackViewModelBase.Logging.cs | 4 ++-- src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.Logging.cs b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.Logging.cs index bf92959..23312e4 100644 --- a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.Logging.cs +++ b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.Logging.cs @@ -5,6 +5,6 @@ namespace MatroskaBatchFlow.Uno.Presentation; /// public abstract partial class TrackViewModelBase { - [LoggerMessage(Level = LogLevel.Warning, Message = "SelectedLanguage was set to null; falling back to Undetermined. This is unexpected during normal UI operation.")] - private partial void LogSelectedLanguageNullFallback(); + [LoggerMessage(Level = LogLevel.Warning, Message = "SelectedLanguage was set to null; falling back to Undetermined.")] + private partial void LogSelectedLanguageReceivedNull(); } diff --git a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs index 3392e5c..1a38db3 100644 --- a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs +++ b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs @@ -187,7 +187,7 @@ public MatroskaLanguageOption? SelectedLanguage // During internal resets (ApplyTrackProperties), _suppressBatchConfigUpdate is true and null is expected. if (value is null && !_suppressBatchConfigUpdate) { - LogSelectedLanguageNullFallback(); + LogSelectedLanguageReceivedNull(); } MatroskaLanguageOption language = value ?? MatroskaLanguageOption.Undetermined; From b172688f3873cd28c6bbec52842d63343c347305 Mon Sep 17 00:00:00 2001 From: Tim Gels Date: Sat, 7 Mar 2026 18:07:14 +0100 Subject: [PATCH 3/4] fix(processing): read track values from global config instead of per-file - Change AddTracksForFile to read Name, Language, Default, Forced, and Enabled values from the global TrackConfiguration instead of per-file FileTrackValues - Fixes newly added files being processed with stale scanned values instead of user-configured values - Fixes toggle-off-then-on scenario leaving new files inconsistent --- .../BatchTrackConfigurationInitializer.cs | 8 ++++---- .../Services/MkvPropeditArgumentsGenerator.cs | 17 ++++++++--------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/MatroskaBatchFlow.Core/Services/BatchTrackConfigurationInitializer.cs b/src/MatroskaBatchFlow.Core/Services/BatchTrackConfigurationInitializer.cs index ecbebab..e21b06b 100644 --- a/src/MatroskaBatchFlow.Core/Services/BatchTrackConfigurationInitializer.cs +++ b/src/MatroskaBatchFlow.Core/Services/BatchTrackConfigurationInitializer.cs @@ -7,15 +7,15 @@ namespace MatroskaBatchFlow.Core.Services; /// Initializes per-file track configurations based on scanned file information. /// /// -/// This class orchestrates the creation of individual track configurations for each file: +/// This class orchestrates track initialization for each file: /// -/// Delegates track configuration creation to +/// Creates per-file directly from scanned track metadata +/// Delegates global creation to when expanding to match maximum track counts /// Populates file-specific track lists in -/// Updates global track collections to reflect maximum track counts for UI display /// /// /// The batch configuration to be modified. -/// The factory for creating track configurations. +/// The factory for creating global track configurations. /// The language provider for resolving track language codes. public class BatchTrackConfigurationInitializer( IBatchConfiguration batchConfig, diff --git a/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.cs b/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.cs index d3c7d46..f6a038e 100644 --- a/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.cs +++ b/src/MatroskaBatchFlow.Core/Services/MkvPropeditArgumentsGenerator.cs @@ -86,10 +86,9 @@ private string[] BuildFileArgumentTokens(ScannedFileInfo file, IBatchConfigurati /// no requested changes or don't exist in the file. /// /// - /// Modification intent (ShouldModify*) is read from the global at - /// the matching index. The actual values to write (Name, Language, flags) are read from the per-file - /// , which are initially populated from the MediaInfo scan and subsequently - /// updated when the user changes settings in the UI. + /// Both the modification intent (ShouldModify*) and the actual values to write (Name, Language, + /// flags) are read from the global at the matching index. Per-file + /// are used only to determine which tracks exist in each file. /// /// The accumulating mkvpropedit argument builder. /// The file being processed. @@ -148,27 +147,27 @@ private void AddTracksForFile( if (globalTrack.ShouldModifyLanguage) { - tb.WithLanguage(track.Language.Code); + tb.WithLanguage(globalTrack.Language.Code); } if (globalTrack.ShouldModifyName) { - tb.WithName(track.Name); + tb.WithName(globalTrack.Name); } if (globalTrack.ShouldModifyDefaultFlag) { - tb.WithIsDefault(track.Default); + tb.WithIsDefault(globalTrack.Default); } if (globalTrack.ShouldModifyForcedFlag) { - tb.WithIsForced(track.Forced); + tb.WithIsForced(globalTrack.Forced); } if (globalTrack.ShouldModifyEnabledFlag) { - tb.WithIsEnabled(track.Enabled); + tb.WithIsEnabled(globalTrack.Enabled); } return tb; From 3db11a647051b3d18abbc08561f6c7fe45c011dc Mon Sep 17 00:00:00 2001 From: Tim Gels Date: Sat, 7 Mar 2026 18:35:47 +0100 Subject: [PATCH 4/4] docs(core,vm): update comments to reflect global-value-read pattern - Update FileTrackValues XML docs to clarify global TrackConfiguration is the source of truth for write-values during command generation - Update TrackViewModelBase comment to clarify per-file sync is for consistency, not correctness - Configure ILanguageProvider.Resolve default in test mocks to prevent null Language values --- .../Models/FileTrackValues.cs | 14 ++++++++------ .../Presentation/TrackViewModelBase.cs | 2 +- .../BatchTrackConfigurationInitializerTests.cs | 1 + .../TrackModificationIntegrationTests.cs | 1 + 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/MatroskaBatchFlow.Core/Models/FileTrackValues.cs b/src/MatroskaBatchFlow.Core/Models/FileTrackValues.cs index 6b8af60..90a6fd1 100644 --- a/src/MatroskaBatchFlow.Core/Models/FileTrackValues.cs +++ b/src/MatroskaBatchFlow.Core/Models/FileTrackValues.cs @@ -3,8 +3,7 @@ namespace MatroskaBatchFlow.Core.Models; /// -/// Stores per-file track values that are initially populated from the MediaInfo scan and -/// subsequently updated when the user changes settings in the UI. +/// Stores per-file track values that are initially populated from the MediaInfo scan. /// The original scanned data is always available via . /// /// @@ -12,15 +11,18 @@ namespace MatroskaBatchFlow.Core.Models; /// /// /// - one per track index in the global collection. -/// Carries modification intent (ShouldModify* flags) and is shared across all files. +/// Carries modification intent (ShouldModify* flags) and the effective values +/// (Name, Language, flags) that should be written, and is shared across all files. /// /// /// - one per track per file in . -/// Holds the actual values (Name, Language, flags) to write for that specific file. +/// Represents the scanned/current per-file values and indicates whether a given file actually +/// has a track at a particular index. /// /// -/// During command generation, reads -/// ShouldModify* from the global track and the values to write from the per-file track. +/// During command generation, reads both +/// ShouldModify* and the values to write from the global track configuration, and uses +/// only to determine per-file track existence and indexing. /// public sealed class FileTrackValues { diff --git a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs index 1a38db3..6bdf566 100644 --- a/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs +++ b/src/MatroskaBatchFlow.Uno/Presentation/TrackViewModelBase.cs @@ -396,7 +396,7 @@ protected virtual void UpdateBatchConfigTrackProperty( return; // First, update all per-file value collections silently (without triggering events) - // This ensures command generation uses the updated values + // This keeps per-file configurations in sync with the global track settings used for command generation var trackType = SelectedTrack.Type; foreach (var kvp in _batchConfiguration.FileConfigurations) { diff --git a/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchTrackConfigurationInitializerTests.cs b/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchTrackConfigurationInitializerTests.cs index ebdd492..b81fbe7 100644 --- a/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchTrackConfigurationInitializerTests.cs +++ b/tests/MatroskaBatchFlow.Core.UnitTests/Services/BatchTrackConfigurationInitializerTests.cs @@ -19,6 +19,7 @@ private static ILanguageProvider CreateMockLanguageProvider() { var mockLanguageProvider = Substitute.For(); mockLanguageProvider.Languages.Returns(ImmutableList.Empty); + mockLanguageProvider.Resolve(Arg.Any()).Returns(MatroskaLanguageOption.Undetermined); return mockLanguageProvider; } diff --git a/tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs b/tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs index fe678c5..f180cf2 100644 --- a/tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs +++ b/tests/MatroskaBatchFlow.Uno.IntegrationTests/TrackModificationIntegrationTests.cs @@ -33,6 +33,7 @@ public async Task IntegrationTest_EditingTrackOnlyInSecondFile_TriggersStateChan // Mock only external dependencies (language data, UI preferences) var mockLanguageProvider = Substitute.For(); mockLanguageProvider.Languages.Returns(ImmutableList.Empty); + mockLanguageProvider.Resolve(Arg.Any()).Returns(MatroskaLanguageOption.Undetermined); var mockUIPreferences = Substitute.For(); var file1Path = "file1.mkv";