Auto Collection Bugfixes (#1769)

* SeriesGroup tag can now have comma separated value to allow a series to be a part of multiple collections.

* Added a missing unit test

* Refactored how collection tags are created to work in the scan loop reliably.

* Added a unit test for RemoveTagsWithoutSeries

* Fixed a bug in reading list title generation to avoid Volume 0 if the underlying file had a title set. Fixed a misconfigured unit test.
This commit is contained in:
Joe Milazzo 2023-02-01 08:22:02 -08:00 committed by GitHub
parent e86694ea9a
commit a76770b240
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 234 additions and 106 deletions

View File

@ -61,14 +61,6 @@ public static class EntityFactory
};
}
public static SeriesMetadata CreateSeriesMetadata(ICollection<CollectionTag> collectionTags)
{
return new SeriesMetadata()
{
CollectionTags = collectionTags
};
}
public static CollectionTag CreateCollectionTag(int id, string title, string summary, bool promoted)
{
return DbFactory.CollectionTag(id, title, summary, promoted);

View File

@ -103,7 +103,7 @@ public class PersonHelperTests
DbFactory.Person("Joe Shmo", PersonRole.CoverArtist)
};
var peopleRemoved = new List<Person>();
PersonHelper.RemovePeople(existingPeople, Array.Empty<string>(), PersonRole.Writer, person =>
PersonHelper.RemovePeople(existingPeople, new List<string>(), PersonRole.Writer, person =>
{
peopleRemoved.Add(person);
});

View File

@ -110,4 +110,46 @@ public class CollectionTagServiceTests : AbstractDbTest
Assert.Empty(metadatas.First().CollectionTags);
Assert.NotEmpty(await _unitOfWork.SeriesRepository.GetSeriesMetadataForIdsAsync(new[] {2}));
}
[Fact]
public async Task GetTagOrCreate_ShouldReturnNewTag()
{
await SeedSeries();
var tag = await _service.GetTagOrCreate(0, "GetTagOrCreate_ShouldReturnNewTag");
Assert.NotNull(tag);
Assert.NotSame(0, tag.Id);
}
[Fact]
public async Task GetTagOrCreate_ShouldReturnExistingTag()
{
await SeedSeries();
var tag = await _service.GetTagOrCreate(1, string.Empty);
Assert.NotNull(tag);
Assert.NotSame(1, tag.Id);
}
[Fact]
public async Task RemoveTagsWithoutSeries_ShouldRemoveAbandonedEntries()
{
await SeedSeries();
// Setup a tag with one series
var tag = await _service.GetTagOrCreate(0, "Tag with a series");
await _unitOfWork.CommitAsync();
var metadatas = await _unitOfWork.SeriesRepository.GetSeriesMetadataForIdsAsync(new[] {1});
tag.SeriesMetadatas.Add(metadatas.First());
var tagId = tag.Id;
await _unitOfWork.CommitAsync();
// Validate it doesn't remove tags it shouldn't
await _service.RemoveTagsWithoutSeries();
Assert.NotNull(await _unitOfWork.CollectionTagRepository.GetTagAsync(tagId));
await _service.RemoveTagFromSeries(tag, new[] {1});
// Validate it does remove tags it should
await _service.RemoveTagsWithoutSeries();
Assert.Null(await _unitOfWork.CollectionTagRepository.GetTagAsync(tagId));
}
}

View File

@ -432,7 +432,6 @@ public class ReadingListServiceTests
#endregion
#region CalculateAgeRating
[Fact]
@ -502,6 +501,29 @@ public class ReadingListServiceTests
public async Task CalculateAgeRating_ShouldUpdateToMax()
{
await ResetDb();
var s = new Series()
{
Name = "Test",
Metadata = DbFactory.SeriesMetadata(new List<CollectionTag>()),
Volumes = new List<Volume>()
{
new Volume()
{
Name = "0",
Chapters = new List<Chapter>()
{
new Chapter()
{
Number = "1",
},
new Chapter()
{
Number = "2",
}
}
}
}
};
_context.AppUser.Add(new AppUser()
{
UserName = "majora2007",
@ -514,34 +536,14 @@ public class ReadingListServiceTests
Type = LibraryType.Book,
Series = new List<Series>()
{
new Series()
{
Name = "Test",
Metadata = DbFactory.SeriesMetadata(new List<CollectionTag>()),
Volumes = new List<Volume>()
{
new Volume()
{
Name = "0",
Chapters = new List<Chapter>()
{
new Chapter()
{
Number = "1",
},
new Chapter()
{
Number = "2",
}
}
}
}
}
s
}
},
}
});
s.Metadata.AgeRating = AgeRating.G;
await _context.SaveChangesAsync();
var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.ReadingLists);
@ -558,7 +560,66 @@ public class ReadingListServiceTests
await _unitOfWork.CommitAsync();
await _readingListService.CalculateReadingListAgeRating(readingList);
Assert.Equal(AgeRating.Unknown, readingList.AgeRating);
Assert.Equal(AgeRating.G, readingList.AgeRating);
}
#endregion
#region FormatTitle
[Fact]
public void FormatTitle_ShouldFormatCorrectly()
{
// Manga Library & Archive
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Manga, "1")));
Assert.Equal("Chapter 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Manga, "1", "1")));
Assert.Equal("Chapter 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Manga, "1", "1", "The Title")));
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Manga, "1", chapterTitleName: "The Title")));
Assert.Equal("The Title", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Manga, chapterTitleName: "The Title")));
// Comic Library & Archive
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Comic, "1")));
Assert.Equal("Issue #1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Comic, "1", "1")));
Assert.Equal("Issue #1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Comic, "1", "1", "The Title")));
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Comic, "1", chapterTitleName: "The Title")));
Assert.Equal("The Title", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Comic, chapterTitleName: "The Title")));
// Book Library & Archive
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Book, "1")));
Assert.Equal("Book 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Book, "1", "1")));
Assert.Equal("Book 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Book, "1", "1", "The Title")));
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Book, "1", chapterTitleName: "The Title")));
Assert.Equal("The Title", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Archive, LibraryType.Book, chapterTitleName: "The Title")));
// Manga Library & EPUB
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Manga, "1")));
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Manga, "1", "1")));
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Manga, "1", "1", "The Title")));
Assert.Equal("The Title", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Manga, "1", chapterTitleName: "The Title")));
Assert.Equal("The Title", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Manga, chapterTitleName: "The Title")));
// Book Library & EPUB
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Book, "1")));
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Book, "1", "1")));
Assert.Equal("Volume 1", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Book, "1", "1", "The Title")));
Assert.Equal("The Title", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Book, "1", chapterTitleName: "The Title")));
Assert.Equal("The Title", ReadingListService.FormatTitle(CreateListItemDto(MangaFormat.Epub, LibraryType.Book, chapterTitleName: "The Title")));
}
private static ReadingListItemDto CreateListItemDto(MangaFormat seriesFormat, LibraryType libraryType,
string volumeNumber = API.Services.Tasks.Scanner.Parser.Parser.DefaultVolume,
string chapterNumber = API.Services.Tasks.Scanner.Parser.Parser.DefaultChapter,
string chapterTitleName = "")
{
return new ReadingListItemDto()
{
SeriesFormat = seriesFormat,
LibraryType = libraryType,
VolumeNumber = volumeNumber,
ChapterNumber = chapterNumber,
ChapterTitleName = chapterTitleName
};
}
#endregion

View File

@ -27,7 +27,7 @@ public static class DbFactory
NormalizedLocalizedName = Services.Tasks.Scanner.Parser.Parser.Normalize(name),
SortName = name,
Volumes = new List<Volume>(),
Metadata = SeriesMetadata(Array.Empty<CollectionTag>())
Metadata = SeriesMetadata(new List<CollectionTag>())
};
}
@ -46,7 +46,7 @@ public static class DbFactory
NormalizedLocalizedName = Services.Tasks.Scanner.Parser.Parser.Normalize(localizedName),
SortName = name,
Volumes = new List<Volume>(),
Metadata = SeriesMetadata(Array.Empty<CollectionTag>())
Metadata = SeriesMetadata(new List<CollectionTag>())
};
}
@ -76,11 +76,6 @@ public static class DbFactory
};
}
public static SeriesMetadata SeriesMetadata(ComicInfo info)
{
return SeriesMetadata(Array.Empty<CollectionTag>());
}
public static SeriesMetadata SeriesMetadata(ICollection<CollectionTag> collectionTags)
{
return new SeriesMetadata()
@ -98,7 +93,8 @@ public static class DbFactory
NormalizedTitle = Services.Tasks.Scanner.Parser.Parser.Normalize(title?.Trim()),
Title = title?.Trim(),
Summary = summary?.Trim(),
Promoted = promoted
Promoted = promoted,
SeriesMetadatas = new List<SeriesMetadata>()
};
}

View File

@ -31,7 +31,7 @@ public interface ICollectionTagRepository
Task<CollectionTag> GetFullTagAsync(int tagId, CollectionTagIncludes includes = CollectionTagIncludes.SeriesMetadata);
void Update(CollectionTag tag);
Task<int> RemoveTagsWithoutSeries();
Task<IEnumerable<CollectionTag>> GetAllTagsAsync();
Task<IEnumerable<CollectionTag>> GetAllTagsAsync(CollectionTagIncludes includes = CollectionTagIncludes.None);
Task<IList<string>> GetAllCoverImagesAsync();
Task<bool> TagExists(string title);
}
@ -66,7 +66,6 @@ public class CollectionTagRepository : ICollectionTagRepository
/// </summary>
public async Task<int> RemoveTagsWithoutSeries()
{
// TODO: Write a Unit test to validate this works
var tagsToDelete = await _context.CollectionTag
.Include(c => c.SeriesMetadatas)
.Where(c => c.SeriesMetadatas.Count == 0)
@ -77,10 +76,11 @@ public class CollectionTagRepository : ICollectionTagRepository
return await _context.SaveChangesAsync();
}
public async Task<IEnumerable<CollectionTag>> GetAllTagsAsync()
public async Task<IEnumerable<CollectionTag>> GetAllTagsAsync(CollectionTagIncludes includes = CollectionTagIncludes.None)
{
return await _context.CollectionTag
.OrderBy(c => c.NormalizedTitle)
.Includes(includes)
.ToListAsync();
}

View File

@ -5,6 +5,7 @@ using API.DTOs.ReadingLists;
using API.Entities;
using API.Entities.Enums;
using API.Helpers;
using API.Services;
using AutoMapper;
using AutoMapper.QueryableExtensions;
using Microsoft.EntityFrameworkCore;
@ -145,7 +146,7 @@ public class ReadingListRepository : IReadingListRepository
{
TotalPages = chapter.Pages,
ChapterNumber = chapter.Range,
ReleaseDate = chapter.ReleaseDate,
chapter.ReleaseDate,
ReadingListItem = data,
ChapterTitleName = chapter.TitleName,
@ -201,7 +202,7 @@ public class ReadingListRepository : IReadingListRepository
foreach (var item in items)
{
item.Title = ReadingListHelper.FormatTitle(item);
item.Title = ReadingListService.FormatTitle(item);
}
// Attach progress information

View File

@ -199,6 +199,9 @@ public class SeriesRepository : ISeriesRepository
var query = _context.Series
.Where(s => s.LibraryId == libraryId)
.Include(s => s.Metadata)
.ThenInclude(m => m.CollectionTags)
.Include(s => s.Metadata)
.ThenInclude(m => m.People)

View File

@ -1,49 +0,0 @@
using System;
using System.Text.RegularExpressions;
using API.DTOs.ReadingLists;
using API.Entities;
using API.Entities.Enums;
using API.Services;
namespace API.Helpers;
public static class ReadingListHelper
{
private static readonly Regex JustNumbers = new Regex(@"^\d+$", RegexOptions.Compiled | RegexOptions.IgnoreCase,
Services.Tasks.Scanner.Parser.Parser.RegexTimeout);
public static string FormatTitle(ReadingListItemDto item)
{
var title = string.Empty;
if (item.ChapterNumber == Services.Tasks.Scanner.Parser.Parser.DefaultChapter) {
title = $"Volume {item.VolumeNumber}";
}
if (item.SeriesFormat == MangaFormat.Epub) {
var specialTitle = Services.Tasks.Scanner.Parser.Parser.CleanSpecialTitle(item.ChapterNumber);
if (specialTitle == Services.Tasks.Scanner.Parser.Parser.DefaultChapter)
{
if (!string.IsNullOrEmpty(item.ChapterTitleName))
{
title = item.ChapterTitleName;
}
else
{
title = $"Volume {Services.Tasks.Scanner.Parser.Parser.CleanSpecialTitle(item.VolumeNumber)}";
}
} else {
title = $"Volume {specialTitle}";
}
}
var chapterNum = item.ChapterNumber;
if (!string.IsNullOrEmpty(chapterNum) && !JustNumbers.Match(item.ChapterNumber).Success) {
chapterNum = Services.Tasks.Scanner.Parser.Parser.CleanSpecialTitle(item.ChapterNumber);
}
if (title == string.Empty) {
title = ReaderService.FormatChapterName(item.LibraryType, true, true) + chapterNum;
}
return title;
}
}

View File

@ -21,6 +21,8 @@ public interface ICollectionTagService
Task<bool> RemoveTagFromSeries(CollectionTag tag, IEnumerable<int> seriesIds);
Task<CollectionTag> GetTagOrCreate(int tagId, string title);
void AddTagToSeriesMetadata(CollectionTag tag, SeriesMetadata metadata);
CollectionTag CreateTag(string title);
Task<bool> RemoveTagsWithoutSeries();
}
@ -113,10 +115,13 @@ public class CollectionTagService : ICollectionTagService
public void AddTagToSeriesMetadata(CollectionTag tag, SeriesMetadata metadata)
{
metadata.CollectionTags ??= new List<CollectionTag>();
if (metadata.CollectionTags.Any(t => t.Title.Equals(tag.Title, StringComparison.InvariantCulture))) return;
if (metadata.CollectionTags.Any(t => t.NormalizedTitle.Equals(tag.NormalizedTitle, StringComparison.InvariantCulture))) return;
metadata.CollectionTags.Add(tag);
_unitOfWork.SeriesMetadataRepository.Update(metadata);
if (metadata.Id != 0)
{
_unitOfWork.SeriesMetadataRepository.Update(metadata);
}
}
public async Task<bool> RemoveTagFromSeries(CollectionTag tag, IEnumerable<int> seriesIds)
@ -148,10 +153,26 @@ public class CollectionTagService : ICollectionTagService
var tag = await _unitOfWork.CollectionTagRepository.GetFullTagAsync(tagId);
if (tag == null)
{
tag = DbFactory.CollectionTag(0, title, string.Empty, false);
_unitOfWork.CollectionTagRepository.Add(tag);
tag = CreateTag(title);
}
return tag;
}
/// <summary>
/// This just creates the entity and adds to tracking. Use <see cref="GetTagOrCreate"/> for checks of duplication.
/// </summary>
/// <param name="title"></param>
/// <returns></returns>
public CollectionTag CreateTag(string title)
{
var tag = DbFactory.CollectionTag(0, title, string.Empty, false);
_unitOfWork.CollectionTagRepository.Add(tag);
return tag;
}
public async Task<bool> RemoveTagsWithoutSeries()
{
return await _unitOfWork.CollectionTagRepository.RemoveTagsWithoutSeries() > 0;
}
}

View File

@ -1,11 +1,13 @@
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using API.Comparators;
using API.Data;
using API.Data.Repositories;
using API.DTOs.ReadingLists;
using API.Entities;
using API.Entities.Enums;
using Microsoft.Extensions.Logging;
namespace API.Services;
@ -31,6 +33,8 @@ public class ReadingListService : IReadingListService
private readonly IUnitOfWork _unitOfWork;
private readonly ILogger<ReadingListService> _logger;
private readonly ChapterSortComparerZeroFirst _chapterSortComparerForInChapterSorting = new ChapterSortComparerZeroFirst();
private static readonly Regex JustNumbers = new Regex(@"^\d+$", RegexOptions.Compiled | RegexOptions.IgnoreCase,
Tasks.Scanner.Parser.Parser.RegexTimeout);
public ReadingListService(IUnitOfWork unitOfWork, ILogger<ReadingListService> logger)
{
@ -38,6 +42,48 @@ public class ReadingListService : IReadingListService
_logger = logger;
}
public static string FormatTitle(ReadingListItemDto item)
{
var title = string.Empty;
if (item.ChapterNumber == Tasks.Scanner.Parser.Parser.DefaultChapter && item.VolumeNumber != Tasks.Scanner.Parser.Parser.DefaultVolume) {
title = $"Volume {item.VolumeNumber}";
}
if (item.SeriesFormat == MangaFormat.Epub) {
var specialTitle = Tasks.Scanner.Parser.Parser.CleanSpecialTitle(item.ChapterNumber);
if (specialTitle == Tasks.Scanner.Parser.Parser.DefaultChapter)
{
if (!string.IsNullOrEmpty(item.ChapterTitleName))
{
title = item.ChapterTitleName;
}
else
{
title = $"Volume {Tasks.Scanner.Parser.Parser.CleanSpecialTitle(item.VolumeNumber)}";
}
} else {
title = $"Volume {specialTitle}";
}
}
var chapterNum = item.ChapterNumber;
if (!string.IsNullOrEmpty(chapterNum) && !JustNumbers.Match(item.ChapterNumber).Success) {
chapterNum = Tasks.Scanner.Parser.Parser.CleanSpecialTitle(item.ChapterNumber);
}
if (title != string.Empty) return title;
if (item.ChapterNumber == Tasks.Scanner.Parser.Parser.DefaultChapter &&
!string.IsNullOrEmpty(item.ChapterTitleName))
{
title = item.ChapterTitleName;
}
else
{
title = ReaderService.FormatChapterName(item.LibraryType, true, true) + chapterNum;
}
return title;
}
/// <summary>

View File

@ -7,6 +7,7 @@ using System.Threading;
using System.Threading.Tasks;
using API.Data;
using API.Data.Metadata;
using API.Data.Repositories;
using API.Entities;
using API.Entities.Enums;
using API.Extensions;
@ -50,6 +51,7 @@ public class ProcessSeries : IProcessSeries
private IList<Genre> _genres;
private IList<Person> _people;
private IList<Tag> _tags;
private Dictionary<string, CollectionTag> _collectionTags;
public ProcessSeries(IUnitOfWork unitOfWork, ILogger<ProcessSeries> logger, IEventHub eventHub,
IDirectoryService directoryService, ICacheHelper cacheHelper, IReadingItemService readingItemService,
@ -76,6 +78,9 @@ public class ProcessSeries : IProcessSeries
_genres = await _unitOfWork.GenreRepository.GetAllGenresAsync();
_people = await _unitOfWork.PersonRepository.GetAllPeople();
_tags = await _unitOfWork.TagRepository.GetAllTagsAsync();
_collectionTags = (await _unitOfWork.CollectionTagRepository.GetAllTagsAsync(CollectionTagIncludes.SeriesMetadata))
.ToDictionary(t => t.NormalizedTitle);
}
public async Task ProcessSeriesAsync(IList<ParserInfo> parsedInfos, Library library, bool forceUpdate = false)
@ -154,7 +159,7 @@ public class ProcessSeries : IProcessSeries
series.NormalizedLocalizedName = Parser.Parser.Normalize(series.LocalizedName);
}
await UpdateSeriesMetadata(series, library);
UpdateSeriesMetadata(series, library);
// Update series FolderPath here
await UpdateSeriesFolderPath(parsedInfos, library, series);
@ -226,7 +231,7 @@ public class ProcessSeries : IProcessSeries
BackgroundJob.Enqueue(() => _wordCountAnalyzerService.ScanSeries(libraryId, seriesId, forceUpdate));
}
private async Task UpdateSeriesMetadata(Series series, Library library)
private void UpdateSeriesMetadata(Series series, Library library)
{
series.Metadata ??= DbFactory.SeriesMetadata(new List<CollectionTag>());
var isBook = library.Type == LibraryType.Book;
@ -283,10 +288,19 @@ public class ProcessSeries : IProcessSeries
if (!string.IsNullOrEmpty(firstChapter.SeriesGroup) && library.ManageCollections)
{
_logger.LogDebug("Collection tag found for {SeriesName}", series.Name);
_logger.LogDebug("Collection tag(s) found for {SeriesName}, updating collections", series.Name);
var tag = await _collectionTagService.GetTagOrCreate(0, firstChapter.SeriesGroup);
_collectionTagService.AddTagToSeriesMetadata(tag, series.Metadata);
foreach (var collection in firstChapter.SeriesGroup.Split(','))
{
var normalizedName = Parser.Parser.Normalize(collection);
if (!_collectionTags.TryGetValue(normalizedName, out var tag))
{
tag = _collectionTagService.CreateTag(collection);
_collectionTags.Add(normalizedName, tag);
}
_collectionTagService.AddTagToSeriesMetadata(tag, series.Metadata);
}
}
// Handle People
@ -519,6 +533,7 @@ public class ProcessSeries : IProcessSeries
series.Volumes = nonDeletedVolumes;
}
// DO I need this anymore?
_logger.LogDebug("[ScannerService] Updated {SeriesName} volumes from count of {StartingVolumeCount} to {VolumeCount}",
series.Name, startingVolumeCount, series.Volumes.Count);
}

View File

@ -7,7 +7,7 @@
"name": "GPL-3.0",
"url": "https://github.com/Kareadita/Kavita/blob/develop/LICENSE"
},
"version": "0.6.1.32"
"version": "0.6.1.33"
},
"servers": [
{