SongMetadata.Artists and .AlbumArtists are nullable EF primitive collections that FallbackMetadataProvider.GetSongMetadata never assigns, so untagged songs persist them as NULL. SongVideoGenerator dereferenced both unguarded (metadata.Artists.Count, string.Join, AlbumArtists.Filter(...Artists.Contains...)), throwing NRE/ANE during song-video generation on the playback path. Rather than enumerating and guarding each read site (the same mistake that left these unswept after #671), add backing fields to the two properties whose getters coalesce null to an empty list. EF Core's default PreferField access mode reads/writes the raw backing field during materialization and change-tracking (confirmed by running the full ErsatzTV.Tests suite, including SongMetadata round-trip tests, unchanged), while every other caller -- SongVideoGenerator, MediaCollectionRepository's rerun-collection artist grouping, and any future reader -- goes through the property getter and always sees a non-null list. This subsumes the ad hoc `metadata.Artists ??= []` guards already hand-applied in LuceneSearchIndex/ElasticSearchIndex and the `?? []` in LibraryBrowseItemMapper, which remain but are now redundant. Adds SongVideoGeneratorTests covering an untagged song (null Artists/ AlbumArtists) through GenerateSongVideo; verified RED (NRE at SongMetadata.cs's Artists getter) by reverting only the `??= []` clause, not the file. Strips the pre-existing UTF-8 BOM from SongMetadata.cs per the #311 formatting gate (touching a legacy-BOM file makes stripping it ours to do). Refs #691 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
130 lines
4.3 KiB
C#
130 lines
4.3 KiB
C#
using ErsatzTV.Core.Domain;
|
|
using ErsatzTV.Core.FFmpeg;
|
|
using ErsatzTV.Core.Interfaces.FFmpeg;
|
|
using ErsatzTV.Core.Interfaces.Images;
|
|
using ErsatzTV.Core.Interfaces.Metadata;
|
|
using ErsatzTV.FFmpeg.State;
|
|
using NSubstitute;
|
|
using NUnit.Framework;
|
|
using Shouldly;
|
|
|
|
namespace ErsatzTV.Core.Tests.FFmpeg;
|
|
|
|
[TestFixture]
|
|
public class SongVideoGeneratorTests
|
|
{
|
|
private ITempFilePool _tempFilePool;
|
|
private IImageCache _imageCache;
|
|
private IFFmpegProcessService _ffmpegProcessService;
|
|
private ILocalFileSystem _localFileSystem;
|
|
private SongVideoGenerator _songVideoGenerator;
|
|
private string _tempSubtitleFile;
|
|
|
|
[SetUp]
|
|
public void SetUp()
|
|
{
|
|
_tempSubtitleFile = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid()}.ass");
|
|
|
|
_tempFilePool = Substitute.For<ITempFilePool>();
|
|
_tempFilePool.GetNextTempFile(Arg.Any<TempFileCategory>()).Returns(_tempSubtitleFile);
|
|
|
|
_imageCache = Substitute.For<IImageCache>();
|
|
_imageCache.GetPathForImage(Arg.Any<string>(), Arg.Any<ArtworkKind>(), Arg.Any<Option<int>>())
|
|
.Returns("/fake/watermark.png");
|
|
|
|
_ffmpegProcessService = Substitute.For<IFFmpegProcessService>();
|
|
_ffmpegProcessService.GenerateSongImage(
|
|
Arg.Any<string>(),
|
|
Arg.Any<string>(),
|
|
Arg.Any<Option<string>>(),
|
|
Arg.Any<Channel>(),
|
|
Arg.Any<MediaVersion>(),
|
|
Arg.Any<string>(),
|
|
Arg.Any<bool>(),
|
|
Arg.Any<Option<string>>(),
|
|
Arg.Any<WatermarkLocation>(),
|
|
Arg.Any<int>(),
|
|
Arg.Any<int>(),
|
|
Arg.Any<int>(),
|
|
Arg.Any<CancellationToken>())
|
|
.Returns(Either<BaseError, string>.Right("/fake/song-image.png"));
|
|
|
|
_localFileSystem = Substitute.For<ILocalFileSystem>();
|
|
_localFileSystem.GetCustomOrDefaultFile(Arg.Any<string>(), Arg.Any<string>())
|
|
.Returns("/fake/background.png");
|
|
|
|
_songVideoGenerator = new SongVideoGenerator(
|
|
_tempFilePool,
|
|
_imageCache,
|
|
_ffmpegProcessService,
|
|
_localFileSystem);
|
|
}
|
|
|
|
[TearDown]
|
|
public void TearDown()
|
|
{
|
|
if (_tempSubtitleFile is not null && File.Exists(_tempSubtitleFile))
|
|
{
|
|
File.Delete(_tempSubtitleFile);
|
|
}
|
|
}
|
|
|
|
private static Channel BuildChannel()
|
|
{
|
|
var resolution = new Resolution { Width = 1920, Height = 1080 };
|
|
FFmpegProfile ffmpegProfile = FFmpegProfile.New("test", resolution);
|
|
|
|
return new Channel(Guid.NewGuid())
|
|
{
|
|
Number = "1",
|
|
Name = "Test Channel",
|
|
FFmpegProfile = ffmpegProfile,
|
|
SongVideoMode = ChannelSongVideoMode.Default
|
|
};
|
|
}
|
|
|
|
private static Song BuildUntaggedSong()
|
|
{
|
|
// an untagged song: FallbackMetadataProvider.GetSongMetadata never assigns
|
|
// Artists/AlbumArtists, so they persist (and materialize) as null (ersatztv#691)
|
|
var metadata = new SongMetadata
|
|
{
|
|
MetadataKind = MetadataKind.Fallback,
|
|
Title = "Untagged Song",
|
|
Artwork = [],
|
|
Artists = null,
|
|
AlbumArtists = null
|
|
};
|
|
|
|
return new Song
|
|
{
|
|
SongMetadata = [metadata],
|
|
MediaVersions = []
|
|
};
|
|
}
|
|
|
|
[Test]
|
|
public async Task GenerateSongVideo_should_not_throw_when_artists_and_album_artists_are_null()
|
|
{
|
|
Song song = BuildUntaggedSong();
|
|
Channel channel = BuildChannel();
|
|
|
|
// SongVideoGenerator randomly picks between two rendering styles (and dereferences
|
|
// metadata.Artists/AlbumArtists differently in each); loop enough times that both
|
|
// branches -- including the AlbumArtists.Filter(... Artists.Contains ...) branch --
|
|
// are exercised with overwhelming probability, so the null guard is proven on both.
|
|
for (var i = 0; i < 25; i++)
|
|
{
|
|
Tuple<string, MediaVersion> result = await _songVideoGenerator.GenerateSongVideo(
|
|
song,
|
|
channel,
|
|
"/usr/bin/ffmpeg",
|
|
"/usr/bin/ffprobe",
|
|
CancellationToken.None);
|
|
|
|
result.ShouldNotBeNull();
|
|
result.Item1.ShouldBe("/fake/song-image.png");
|
|
}
|
|
}
|
|
}
|