From 19678383b214901f29cab079e2928ffc1e072207 Mon Sep 17 00:00:00 2001 From: Joseph Milazzo Date: Sun, 3 Apr 2022 16:11:16 -0500 Subject: [PATCH] Tech Debt + Series Sort bugfix (#1192) * Code cleanup. When copying files, if the target file already exists, append (1), (2), etc onto the file (this is enhancing existing implementation to allow multiple numbers) * Added a ton of null checks to UpdateSeriesMetadata and made the code work on the rare case (not really possible) that SeriesMetadata doesn't exist. * Updated Genre code to use strings to ensure a better, more fault tolerant update experience. * More cleanup on the codebase * Fixed a bug where Series SortName was getting emptied on file scan * Fixed a bad copy * Fixed unit tests --- API.Tests/Parser/ParserTest.cs | 1 - API.Tests/Services/DirectoryServiceTests.cs | 18 ++ API.Tests/Services/SeriesServiceTests.cs | 176 ++++++++++++++++ API/Controllers/DownloadController.cs | 4 +- API/Controllers/OPDSController.cs | 9 +- API/DTOs/SeriesMetadataDto.cs | 5 +- API/Data/DbFactory.cs | 2 +- API/Data/Repositories/SeriesRepository.cs | 2 +- API/Parser/Parser.cs | 9 +- API/Services/DirectoryService.cs | 59 ++++-- API/Services/SeriesService.cs | 192 +++++++++--------- API/Services/Tasks/ScannerService.cs | 9 +- API/Services/Tasks/VersionUpdaterService.cs | 4 +- API/SignalR/EventHub.cs | 3 +- API/SignalR/MessageFactory.cs | 1 - API/SignalR/SignalRMessage.cs | 2 +- .../manage-users/manage-users.component.html | 4 +- 17 files changed, 362 insertions(+), 138 deletions(-) diff --git a/API.Tests/Parser/ParserTest.cs b/API.Tests/Parser/ParserTest.cs index e57f56928..ff1b4dc71 100644 --- a/API.Tests/Parser/ParserTest.cs +++ b/API.Tests/Parser/ParserTest.cs @@ -210,7 +210,6 @@ namespace API.Tests.Parser [InlineData("._Love Hina/Love Hina/", true)] [InlineData("@Recently-Snapshot/Love Hina/", true)] [InlineData("@recycle/Love Hina/", true)] - [InlineData("@recycle/Love Hina/", true)] [InlineData("E:/Test/__MACOSX/Love Hina/", true)] public void HasBlacklistedFolderInPathTest(string inputPath, bool expected) { diff --git a/API.Tests/Services/DirectoryServiceTests.cs b/API.Tests/Services/DirectoryServiceTests.cs index c0d49820b..1ec21efe8 100644 --- a/API.Tests/Services/DirectoryServiceTests.cs +++ b/API.Tests/Services/DirectoryServiceTests.cs @@ -557,6 +557,24 @@ namespace API.Tests.Services Assert.Equal(2, ds.GetFiles("/manga/output/").Count()); } + [Fact] + public void CopyFilesToDirectory_ShouldAppendWhenTargetFileExists() + { + const string testDirectory = "/manga/"; + var fileSystem = new MockFileSystem(); + fileSystem.AddFile($"{testDirectory}file.zip", new MockFileData("")); + fileSystem.AddFile($"/manga/output/file (1).zip", new MockFileData("")); + fileSystem.AddFile($"/manga/output/file (2).zip", new MockFileData("")); + + var ds = new DirectoryService(Substitute.For>(), fileSystem); + ds.CopyFilesToDirectory(new []{$"{testDirectory}file.zip"}, "/manga/output/"); + ds.CopyFilesToDirectory(new []{$"{testDirectory}file.zip"}, "/manga/output/"); + var outputFiles = ds.GetFiles("/manga/output/").Select(API.Parser.Parser.NormalizePath).ToList(); + Assert.Equal(4, outputFiles.Count()); // we have 2 already there and 2 copies + // For some reason, this has C:/ on directory even though everything is emulated + Assert.True(outputFiles.Contains(API.Parser.Parser.NormalizePath("/manga/output/file (3).zip")) || outputFiles.Contains(API.Parser.Parser.NormalizePath("C:/manga/output/file (3).zip"))); + } + #endregion #region ListDirectory diff --git a/API.Tests/Services/SeriesServiceTests.cs b/API.Tests/Services/SeriesServiceTests.cs index 42b586e1f..1b9f5fd3f 100644 --- a/API.Tests/Services/SeriesServiceTests.cs +++ b/API.Tests/Services/SeriesServiceTests.cs @@ -6,8 +6,11 @@ using System.Threading.Tasks; using API.Data; using API.Data.Repositories; using API.DTOs; +using API.DTOs.CollectionTags; +using API.DTOs.Metadata; using API.Entities; using API.Entities.Enums; +using API.Extensions; using API.Helpers; using API.Services; using API.SignalR; @@ -99,6 +102,9 @@ public class SeriesServiceTests { _context.Series.RemoveRange(_context.Series.ToList()); _context.AppUserRating.RemoveRange(_context.AppUserRating.ToList()); + _context.Genre.RemoveRange(_context.Genre.ToList()); + _context.CollectionTag.RemoveRange(_context.CollectionTag.ToList()); + _context.Person.RemoveRange(_context.Person.ToList()); await _context.SaveChangesAsync(); } @@ -569,4 +575,174 @@ public class SeriesServiceTests } #endregion + + #region UpdateSeriesMetadata + + private void SetupUpdateSeriesMetadataDb() + { + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Book, + } + }); + } + + [Fact] + public async Task UpdateSeriesMetadata_ShouldCreateEmptyMetadata_IfDoesntExist() + { + await ResetDb(); + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Book, + } + }); + await _context.SaveChangesAsync(); + + var success = await _seriesService.UpdateSeriesMetadata(new UpdateSeriesMetadataDto() + { + SeriesMetadata = new SeriesMetadataDto() + { + SeriesId = 1, + Genres = new List() {new GenreTagDto() {Id = 0, Title = "New Genre"}} + }, + CollectionTags = new List() + }); + + Assert.True(success); + + var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(1); + Assert.NotNull(series.Metadata); + Assert.True(series.Metadata.Genres.Select(g => g.Title).Contains("New Genre".SentenceCase())); + + } + + [Fact] + public async Task UpdateSeriesMetadata_ShouldCreateNewTags_IfNoneExist() + { + await ResetDb(); + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Book, + } + }); + await _context.SaveChangesAsync(); + + var success = await _seriesService.UpdateSeriesMetadata(new UpdateSeriesMetadataDto() + { + SeriesMetadata = new SeriesMetadataDto() + { + SeriesId = 1, + Genres = new List() {new GenreTagDto() {Id = 0, Title = "New Genre"}}, + Tags = new List() {new TagDto() {Id = 0, Title = "New Tag"}}, + Characters = new List() {new PersonDto() {Id = 0, Name = "Joe Shmo", Role = PersonRole.Character}}, + Colorists = new List() {new PersonDto() {Id = 0, Name = "Joe Shmo", Role = PersonRole.Colorist}}, + Pencillers = new List() {new PersonDto() {Id = 0, Name = "Joe Shmo 2", Role = PersonRole.Penciller}}, + }, + CollectionTags = new List() + { + new CollectionTagDto() {Id = 0, Promoted = false, Summary = string.Empty, CoverImageLocked = false, Title = "New Collection"} + } + }); + + Assert.True(success); + + var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(1); + Assert.NotNull(series.Metadata); + Assert.True(series.Metadata.Genres.Select(g => g.Title).Contains("New Genre".SentenceCase())); + Assert.True(series.Metadata.People.All(g => g.Name is "Joe Shmo" or "Joe Shmo 2")); + Assert.True(series.Metadata.Tags.Select(g => g.Title).Contains("New Tag".SentenceCase())); + Assert.True(series.Metadata.CollectionTags.Select(g => g.Title).Contains("New Collection")); + + } + + [Fact] + public async Task UpdateSeriesMetadata_ShouldRemoveExistingTags() + { + await ResetDb(); + var s = new Series() + { + Name = "Test", + Library = new Library() + { + Name = "Test LIb", + Type = LibraryType.Book, + }, + Metadata = DbFactory.SeriesMetadata(new List()) + }; + var g = DbFactory.Genre("Existing Genre", false); + s.Metadata.Genres = new List() {g}; + _context.Series.Add(s); + + _context.Genre.Add(g); + await _context.SaveChangesAsync(); + + var success = await _seriesService.UpdateSeriesMetadata(new UpdateSeriesMetadataDto() + { + SeriesMetadata = new SeriesMetadataDto() + { + SeriesId = 1, + Genres = new List() {new () {Id = 0, Title = "New Genre"}}, + }, + CollectionTags = new List() + }); + + Assert.True(success); + + var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(1); + Assert.NotNull(series.Metadata); + Assert.True(series.Metadata.Genres.Select(g => g.Title).All(g => g == "New Genre".SentenceCase())); + Assert.False(series.Metadata.GenresLocked); // GenreLocked is false unless the UI Explicitly says it should be locked + } + + [Fact] + public async Task UpdateSeriesMetadata_ShouldLockIfTold() + { + await ResetDb(); + var s = new Series() + { + Name = "Test", + Library = new Library() + { + Name = "Test LIb", + Type = LibraryType.Book, + }, + Metadata = DbFactory.SeriesMetadata(new List()) + }; + var g = DbFactory.Genre("Existing Genre", false); + s.Metadata.Genres = new List() {g}; + s.Metadata.GenresLocked = true; + _context.Series.Add(s); + + _context.Genre.Add(g); + await _context.SaveChangesAsync(); + + var success = await _seriesService.UpdateSeriesMetadata(new UpdateSeriesMetadataDto() + { + SeriesMetadata = new SeriesMetadataDto() + { + SeriesId = 1, + Genres = new List() {new () {Id = 1, Title = "Existing Genre"}}, + GenresLocked = true + }, + CollectionTags = new List() + }); + + Assert.True(success); + + var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(1); + Assert.NotNull(series.Metadata); + Assert.True(series.Metadata.Genres.Select(g => g.Title).All(g => g == "Existing Genre".SentenceCase())); + Assert.True(series.Metadata.GenresLocked); + } + + #endregion } diff --git a/API/Controllers/DownloadController.cs b/API/Controllers/DownloadController.cs index 169c34bd9..b60fae6e8 100644 --- a/API/Controllers/DownloadController.cs +++ b/API/Controllers/DownloadController.cs @@ -26,20 +26,18 @@ namespace API.Controllers private readonly IDirectoryService _directoryService; private readonly IDownloadService _downloadService; private readonly IEventHub _eventHub; - private readonly UserManager _userManager; private readonly ILogger _logger; private readonly IBookmarkService _bookmarkService; private const string DefaultContentType = "application/octet-stream"; public DownloadController(IUnitOfWork unitOfWork, IArchiveService archiveService, IDirectoryService directoryService, - IDownloadService downloadService, IEventHub eventHub, UserManager userManager, ILogger logger, IBookmarkService bookmarkService) + IDownloadService downloadService, IEventHub eventHub, ILogger logger, IBookmarkService bookmarkService) { _unitOfWork = unitOfWork; _archiveService = archiveService; _directoryService = directoryService; _downloadService = downloadService; _eventHub = eventHub; - _userManager = userManager; _logger = logger; _bookmarkService = bookmarkService; } diff --git a/API/Controllers/OPDSController.cs b/API/Controllers/OPDSController.cs index 91607db7d..eaa778121 100644 --- a/API/Controllers/OPDSController.cs +++ b/API/Controllers/OPDSController.cs @@ -782,6 +782,7 @@ public class OpdsController : BaseApiController { CreateLink(FeedLinkRelation.Image, FeedLinkType.Image, $"/api/image/chapter-cover?chapterId={chapterId}"), CreateLink(FeedLinkRelation.Thumbnail, FeedLinkType.Image, $"/api/image/chapter-cover?chapterId={chapterId}"), + // We can't not include acc link in the feed, panels doesn't work with just page streaming option. We have to block download directly accLink, CreatePageStreamLink(seriesId, volumeId, chapterId, mangaFile, apiKey) }, @@ -792,14 +793,6 @@ public class OpdsController : BaseApiController } }; - // We can't not show acc link in the feed, panels wont work like that. We have to block download directly - // var user = await _unitOfWork.UserRepository.GetUserByIdAsync(await GetUser(apiKey)); - // if (await _downloadService.HasDownloadPermission(user)) - // { - // entry.Links.Add(accLink); - // } - - return entry; } diff --git a/API/DTOs/SeriesMetadataDto.cs b/API/DTOs/SeriesMetadataDto.cs index 23e8f4e52..d5ad24610 100644 --- a/API/DTOs/SeriesMetadataDto.cs +++ b/API/DTOs/SeriesMetadataDto.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using API.DTOs.CollectionTags; using API.DTOs.Metadata; using API.Entities.Enums; @@ -8,7 +9,7 @@ namespace API.DTOs public class SeriesMetadataDto { public int Id { get; set; } - public string Summary { get; set; } + public string Summary { get; set; } = string.Empty; /// /// Collections the Series belongs to /// diff --git a/API/Data/DbFactory.cs b/API/Data/DbFactory.cs index 81750fb72..46ebfbf10 100644 --- a/API/Data/DbFactory.cs +++ b/API/Data/DbFactory.cs @@ -18,7 +18,7 @@ namespace API.Data { public static Series Series(string name) { - return new () + return new Series { Name = name, OriginalName = name, diff --git a/API/Data/Repositories/SeriesRepository.cs b/API/Data/Repositories/SeriesRepository.cs index ef97dfc87..d9252f97b 100644 --- a/API/Data/Repositories/SeriesRepository.cs +++ b/API/Data/Repositories/SeriesRepository.cs @@ -378,7 +378,7 @@ public class SeriesRepository : ISeriesRepository /// - /// Returns Volumes, Metadata, and Collection Tags + /// Returns Volumes, Metadata (Incl Genres and People), and Collection Tags /// /// /// diff --git a/API/Parser/Parser.cs b/API/Parser/Parser.cs index cc237eae7..63db72658 100644 --- a/API/Parser/Parser.cs +++ b/API/Parser/Parser.cs @@ -60,6 +60,12 @@ namespace API.Parser private static readonly Regex NormalizeRegex = new Regex(@"[^\p{L}0-9\+]", MatchOptions, RegexTimeout); + /// + /// Recognizes the Special token only + /// + private static readonly Regex SpecialTokenRegex = new Regex(@"SP\d+", + MatchOptions, RegexTimeout); + private static readonly Regex[] MangaVolumeRegex = new[] { @@ -976,9 +982,8 @@ namespace API.Parser /// public static string CleanSpecialTitle(string name) { - // TODO: Optimize this code & Test if (string.IsNullOrEmpty(name)) return name; - var cleaned = new Regex(@"SP\d+").Replace(name.Replace('_', ' '), string.Empty).Trim(); + var cleaned = SpecialTokenRegex.Replace(name.Replace('_', ' '), string.Empty).Trim(); var lastIndex = cleaned.LastIndexOf('.'); if (lastIndex > 0) { diff --git a/API/Services/DirectoryService.cs b/API/Services/DirectoryService.cs index fb7675735..227aa9c29 100644 --- a/API/Services/DirectoryService.cs +++ b/API/Services/DirectoryService.cs @@ -71,6 +71,8 @@ namespace API.Services private static readonly Regex ExcludeDirectories = new Regex( @"@eaDir|\.DS_Store|\.qpkg", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private static readonly Regex FileCopyAppend = new Regex(@"\(\d+\)", + RegexOptions.Compiled | RegexOptions.IgnoreCase); public static readonly string BackupDirectory = Path.Join(Directory.GetCurrentDirectory(), "config", "backups"); public DirectoryService(ILogger logger, IFileSystem fileSystem) @@ -370,24 +372,11 @@ namespace API.Services foreach (var file in filePaths) { currentFile = file; + var fileInfo = FileSystem.FileInfo.FromFileName(file); - if (fileInfo.Exists) - { - // TODO: I need to handle if file already exists and allow either an overwrite or prepend (2) to it - try - { - fileInfo.CopyTo(FileSystem.Path.Join(directoryPath, prepend + fileInfo.Name)); - } - catch (IOException ex) - { - _logger.LogError(ex, "File copy, dest already exists. Appending (2)"); - fileInfo.CopyTo(FileSystem.Path.Join(directoryPath, prepend + FileSystem.Path.GetFileNameWithoutExtension(fileInfo.Name) + " (2)" + FileSystem.Path.GetExtension(fileInfo.Name))); - } - } - else - { - _logger.LogWarning("Tried to copy {File} but it doesn't exist", file); - } + var targetFile = FileSystem.FileInfo.FromFileName(RenameFileForCopy(file, directoryPath, prepend)); + + fileInfo.CopyTo(FileSystem.Path.Join(directoryPath, targetFile.Name)); } } catch (Exception ex) @@ -399,6 +388,42 @@ namespace API.Services return true; } + /// + /// Generates the combined filepath given a prepend (optional), output directory path, and a full input file path. + /// If the output file already exists, will append (1), (2), etc until it can be written out + /// + /// + /// + /// + /// + private string RenameFileForCopy(string fileToCopy, string directoryPath, string prepend = "") + { + var fileInfo = FileSystem.FileInfo.FromFileName(fileToCopy); + var filename = prepend + fileInfo.Name; + + var targetFile = FileSystem.FileInfo.FromFileName(FileSystem.Path.Join(directoryPath, filename)); + if (!targetFile.Exists) + { + return targetFile.FullName; + } + + var noExtension = FileSystem.Path.GetFileNameWithoutExtension(fileInfo.Name); + if (FileCopyAppend.IsMatch(noExtension)) + { + var match = FileCopyAppend.Match(noExtension).Value; + var matchNumber = match.Replace("(", string.Empty).Replace(")", string.Empty); + noExtension = noExtension.Replace(match, $"({int.Parse(matchNumber) + 1})"); + } + else + { + noExtension += " (1)"; + } + + var newFilename = prepend + noExtension + + FileSystem.Path.GetExtension(fileInfo.Name); + return RenameFileForCopy(FileSystem.Path.Join(directoryPath, newFilename), directoryPath, prepend); + } + /// /// Lists all directories in a root path. Will exclude Hidden or System directories. /// diff --git a/API/Services/SeriesService.cs b/API/Services/SeriesService.cs index 3146ce6dc..c1c9386dc 100644 --- a/API/Services/SeriesService.cs +++ b/API/Services/SeriesService.cs @@ -52,103 +52,99 @@ public class SeriesService : ISeriesService var allPeople = (await _unitOfWork.PersonRepository.GetAllPeople()).ToList(); var allTags = (await _unitOfWork.TagRepository.GetAllTagsAsync()).ToList(); - if (series.Metadata == null) + series.Metadata ??= DbFactory.SeriesMetadata(updateSeriesMetadataDto.CollectionTags + .Select(dto => DbFactory.CollectionTag(dto.Id, dto.Title, dto.Summary, dto.Promoted)).ToList()); + + if (series.Metadata.AgeRating != updateSeriesMetadataDto.SeriesMetadata.AgeRating) { - series.Metadata = DbFactory.SeriesMetadata(updateSeriesMetadataDto.CollectionTags - .Select(dto => DbFactory.CollectionTag(dto.Id, dto.Title, dto.Summary, dto.Promoted)).ToList()); + series.Metadata.AgeRating = updateSeriesMetadataDto.SeriesMetadata.AgeRating; + series.Metadata.AgeRatingLocked = true; } - else + + if (series.Metadata.PublicationStatus != updateSeriesMetadataDto.SeriesMetadata.PublicationStatus) { - if (series.Metadata.AgeRating != updateSeriesMetadataDto.SeriesMetadata.AgeRating) - { - series.Metadata.AgeRating = updateSeriesMetadataDto.SeriesMetadata.AgeRating; - series.Metadata.AgeRatingLocked = true; - } - - if (series.Metadata.PublicationStatus != updateSeriesMetadataDto.SeriesMetadata.PublicationStatus) - { - series.Metadata.PublicationStatus = updateSeriesMetadataDto.SeriesMetadata.PublicationStatus; - series.Metadata.PublicationStatusLocked = true; - } - - if (series.Metadata.Summary != updateSeriesMetadataDto.SeriesMetadata.Summary.Trim()) - { - series.Metadata.Summary = updateSeriesMetadataDto.SeriesMetadata?.Summary.Trim(); - series.Metadata.SummaryLocked = true; - } - - if (series.Metadata.Language != updateSeriesMetadataDto.SeriesMetadata.Language) - { - series.Metadata.Language = updateSeriesMetadataDto.SeriesMetadata?.Language; - series.Metadata.LanguageLocked = true; - } - - - series.Metadata.CollectionTags ??= new List(); - UpdateRelatedList(updateSeriesMetadataDto.CollectionTags, series, allCollectionTags, (tag) => - { - series.Metadata.CollectionTags.Add(tag); - }); - - series.Metadata.Genres ??= new List(); - UpdateGenreList(updateSeriesMetadataDto.SeriesMetadata.Genres, series, allGenres, (genre) => - { - series.Metadata.Genres.Add(genre); - }, () => series.Metadata.GenresLocked = true); - - series.Metadata.Tags ??= new List(); - UpdateTagList(updateSeriesMetadataDto.SeriesMetadata.Tags, series, allTags, (tag) => - { - series.Metadata.Tags.Add(tag); - }, () => series.Metadata.TagsLocked = true); - - void HandleAddPerson(Person person) - { - PersonHelper.AddPersonIfNotExists(series.Metadata.People, person); - allPeople.Add(person); - } - - series.Metadata.People ??= new List(); - UpdatePeopleList(PersonRole.Writer, updateSeriesMetadataDto.SeriesMetadata.Writers, series, allPeople, - HandleAddPerson, () => series.Metadata.WriterLocked = true); - UpdatePeopleList(PersonRole.Character, updateSeriesMetadataDto.SeriesMetadata.Characters, series, allPeople, - HandleAddPerson, () => series.Metadata.CharacterLocked = true); - UpdatePeopleList(PersonRole.Colorist, updateSeriesMetadataDto.SeriesMetadata.Colorists, series, allPeople, - HandleAddPerson, () => series.Metadata.ColoristLocked = true); - UpdatePeopleList(PersonRole.Editor, updateSeriesMetadataDto.SeriesMetadata.Editors, series, allPeople, - HandleAddPerson, () => series.Metadata.EditorLocked = true); - UpdatePeopleList(PersonRole.Inker, updateSeriesMetadataDto.SeriesMetadata.Inkers, series, allPeople, - HandleAddPerson, () => series.Metadata.InkerLocked = true); - UpdatePeopleList(PersonRole.Letterer, updateSeriesMetadataDto.SeriesMetadata.Letterers, series, allPeople, - HandleAddPerson, () => series.Metadata.LettererLocked = true); - UpdatePeopleList(PersonRole.Penciller, updateSeriesMetadataDto.SeriesMetadata.Pencillers, series, allPeople, - HandleAddPerson, () => series.Metadata.PencillerLocked = true); - UpdatePeopleList(PersonRole.Publisher, updateSeriesMetadataDto.SeriesMetadata.Publishers, series, allPeople, - HandleAddPerson, () => series.Metadata.PublisherLocked = true); - UpdatePeopleList(PersonRole.Translator, updateSeriesMetadataDto.SeriesMetadata.Translators, series, allPeople, - HandleAddPerson, () => series.Metadata.TranslatorLocked = true); - UpdatePeopleList(PersonRole.CoverArtist, updateSeriesMetadataDto.SeriesMetadata.CoverArtists, series, allPeople, - HandleAddPerson, () => series.Metadata.CoverArtistLocked = true); - - if (!updateSeriesMetadataDto.SeriesMetadata.AgeRatingLocked) series.Metadata.AgeRatingLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.PublicationStatusLocked) series.Metadata.PublicationStatusLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.LanguageLocked) series.Metadata.LanguageLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.GenresLocked) series.Metadata.GenresLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.TagsLocked) series.Metadata.TagsLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.CharacterLocked) series.Metadata.CharacterLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.ColoristLocked) series.Metadata.ColoristLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.EditorLocked) series.Metadata.EditorLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.InkerLocked) series.Metadata.InkerLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.LettererLocked) series.Metadata.LettererLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.PencillerLocked) series.Metadata.PencillerLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.PublisherLocked) series.Metadata.PublisherLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.TranslatorLocked) series.Metadata.TranslatorLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.CoverArtistLocked) series.Metadata.CoverArtistLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.WriterLocked) series.Metadata.WriterLocked = false; - if (!updateSeriesMetadataDto.SeriesMetadata.SummaryLocked) series.Metadata.SummaryLocked = false; - + series.Metadata.PublicationStatus = updateSeriesMetadataDto.SeriesMetadata.PublicationStatus; + series.Metadata.PublicationStatusLocked = true; } + if (series.Metadata.Summary != updateSeriesMetadataDto.SeriesMetadata.Summary.Trim()) + { + series.Metadata.Summary = updateSeriesMetadataDto.SeriesMetadata?.Summary.Trim(); + series.Metadata.SummaryLocked = true; + } + + if (series.Metadata.Language != updateSeriesMetadataDto.SeriesMetadata.Language) + { + series.Metadata.Language = updateSeriesMetadataDto.SeriesMetadata?.Language; + series.Metadata.LanguageLocked = true; + } + + + series.Metadata.CollectionTags ??= new List(); + UpdateRelatedList(updateSeriesMetadataDto.CollectionTags, series, allCollectionTags, (tag) => + { + series.Metadata.CollectionTags.Add(tag); + }); + + series.Metadata.Genres ??= new List(); + UpdateGenreList(updateSeriesMetadataDto.SeriesMetadata.Genres, series, allGenres, (genre) => + { + series.Metadata.Genres.Add(genre); + }, () => series.Metadata.GenresLocked = true); + + series.Metadata.Tags ??= new List(); + UpdateTagList(updateSeriesMetadataDto.SeriesMetadata.Tags, series, allTags, (tag) => + { + series.Metadata.Tags.Add(tag); + }, () => series.Metadata.TagsLocked = true); + + void HandleAddPerson(Person person) + { + PersonHelper.AddPersonIfNotExists(series.Metadata.People, person); + allPeople.Add(person); + } + + series.Metadata.People ??= new List(); + UpdatePeopleList(PersonRole.Writer, updateSeriesMetadataDto.SeriesMetadata.Writers, series, allPeople, + HandleAddPerson, () => series.Metadata.WriterLocked = true); + UpdatePeopleList(PersonRole.Character, updateSeriesMetadataDto.SeriesMetadata.Characters, series, allPeople, + HandleAddPerson, () => series.Metadata.CharacterLocked = true); + UpdatePeopleList(PersonRole.Colorist, updateSeriesMetadataDto.SeriesMetadata.Colorists, series, allPeople, + HandleAddPerson, () => series.Metadata.ColoristLocked = true); + UpdatePeopleList(PersonRole.Editor, updateSeriesMetadataDto.SeriesMetadata.Editors, series, allPeople, + HandleAddPerson, () => series.Metadata.EditorLocked = true); + UpdatePeopleList(PersonRole.Inker, updateSeriesMetadataDto.SeriesMetadata.Inkers, series, allPeople, + HandleAddPerson, () => series.Metadata.InkerLocked = true); + UpdatePeopleList(PersonRole.Letterer, updateSeriesMetadataDto.SeriesMetadata.Letterers, series, allPeople, + HandleAddPerson, () => series.Metadata.LettererLocked = true); + UpdatePeopleList(PersonRole.Penciller, updateSeriesMetadataDto.SeriesMetadata.Pencillers, series, allPeople, + HandleAddPerson, () => series.Metadata.PencillerLocked = true); + UpdatePeopleList(PersonRole.Publisher, updateSeriesMetadataDto.SeriesMetadata.Publishers, series, allPeople, + HandleAddPerson, () => series.Metadata.PublisherLocked = true); + UpdatePeopleList(PersonRole.Translator, updateSeriesMetadataDto.SeriesMetadata.Translators, series, allPeople, + HandleAddPerson, () => series.Metadata.TranslatorLocked = true); + UpdatePeopleList(PersonRole.CoverArtist, updateSeriesMetadataDto.SeriesMetadata.CoverArtists, series, allPeople, + HandleAddPerson, () => series.Metadata.CoverArtistLocked = true); + + if (!updateSeriesMetadataDto.SeriesMetadata.AgeRatingLocked) series.Metadata.AgeRatingLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.PublicationStatusLocked) series.Metadata.PublicationStatusLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.LanguageLocked) series.Metadata.LanguageLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.GenresLocked) series.Metadata.GenresLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.TagsLocked) series.Metadata.TagsLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.CharacterLocked) series.Metadata.CharacterLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.ColoristLocked) series.Metadata.ColoristLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.EditorLocked) series.Metadata.EditorLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.InkerLocked) series.Metadata.InkerLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.LettererLocked) series.Metadata.LettererLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.PencillerLocked) series.Metadata.PencillerLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.PublisherLocked) series.Metadata.PublisherLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.TranslatorLocked) series.Metadata.TranslatorLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.CoverArtistLocked) series.Metadata.CoverArtistLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.WriterLocked) series.Metadata.WriterLocked = false; + if (!updateSeriesMetadataDto.SeriesMetadata.SummaryLocked) series.Metadata.SummaryLocked = false; + + + if (!_unitOfWork.HasChanges()) { return true; @@ -184,6 +180,7 @@ public class SeriesService : ISeriesService private static void UpdateRelatedList(ICollection tags, Series series, IReadOnlyCollection allTags, Action handleAdd) { + if (tags == null) return; // I want a union of these 2 lists. Return only elements that are in both lists, but the list types are different var existingTags = series.Metadata.CollectionTags.ToList(); foreach (var existing in existingTags) @@ -216,11 +213,13 @@ public class SeriesService : ISeriesService private static void UpdateGenreList(ICollection tags, Series series, IReadOnlyCollection allTags, Action handleAdd, Action onModified) { + if (tags == null) return; var isModified = false; // I want a union of these 2 lists. Return only elements that are in both lists, but the list types are different var existingTags = series.Metadata.Genres.ToList(); foreach (var existing in existingTags) { + // NOTE: Why don't I use a NormalizedName here (outside of memory pressure from string creation)? if (tags.SingleOrDefault(t => t.Id == existing.Id) == null) { // Remove tag @@ -232,10 +231,12 @@ public class SeriesService : ISeriesService // At this point, all tags that aren't in dto have been removed. foreach (var tagTitle in tags.Select(t => t.Title)) { - var existingTag = allTags.SingleOrDefault(t => t.Title == tagTitle); + // This should be normalized name + var normalizedTitle = Parser.Parser.Normalize(tagTitle); + var existingTag = allTags.SingleOrDefault(t => t.NormalizedTitle == normalizedTitle); if (existingTag != null) { - if (series.Metadata.Genres.All(t => t.Title != tagTitle)) + if (series.Metadata.Genres.All(t => t.NormalizedTitle != normalizedTitle)) { handleAdd(existingTag); isModified = true; @@ -257,6 +258,8 @@ public class SeriesService : ISeriesService private static void UpdateTagList(ICollection tags, Series series, IReadOnlyCollection allTags, Action handleAdd, Action onModified) { + if (tags == null) return; + var isModified = false; // I want a union of these 2 lists. Return only elements that are in both lists, but the list types are different var existingTags = series.Metadata.Tags.ToList(); @@ -300,6 +303,7 @@ public class SeriesService : ISeriesService private static void UpdatePeopleList(PersonRole role, ICollection tags, Series series, IReadOnlyCollection allTags, Action handleAdd, Action onModified) { + if (tags == null) return; var isModified = false; // I want a union of these 2 lists. Return only elements that are in both lists, but the list types are different var existingTags = series.Metadata.People.Where(p => p.Role == role).ToList(); diff --git a/API/Services/Tasks/ScannerService.cs b/API/Services/Tasks/ScannerService.cs index 08c13e338..6fe4a57c8 100644 --- a/API/Services/Tasks/ScannerService.cs +++ b/API/Services/Tasks/ScannerService.cs @@ -523,7 +523,14 @@ public class ScannerService : IScannerService series.Format = parsedInfos[0].Format; } series.OriginalName ??= parsedInfos[0].Series; - if (!series.SortNameLocked) series.SortName = parsedInfos[0].SeriesSort; + if (!series.SortNameLocked) + { + if (!string.IsNullOrEmpty(parsedInfos[0].SeriesSort)) + { + series.SortName = parsedInfos[0].SeriesSort; + } + series.SortName = series.Name; + } await _eventHub.SendMessageAsync(MessageFactory.NotificationProgress, MessageFactory.LibraryScanProgressEvent(library.Name, ProgressEventType.Ended, series.Name)); diff --git a/API/Services/Tasks/VersionUpdaterService.cs b/API/Services/Tasks/VersionUpdaterService.cs index d72f487b4..5641b0962 100644 --- a/API/Services/Tasks/VersionUpdaterService.cs +++ b/API/Services/Tasks/VersionUpdaterService.cs @@ -54,18 +54,16 @@ public class VersionUpdaterService : IVersionUpdaterService { private readonly ILogger _logger; private readonly IEventHub _eventHub; - private readonly IPresenceTracker _tracker; private readonly Markdown _markdown = new MarkdownDeep.Markdown(); #pragma warning disable S1075 private const string GithubLatestReleasesUrl = "https://api.github.com/repos/Kareadita/Kavita/releases/latest"; private const string GithubAllReleasesUrl = "https://api.github.com/repos/Kareadita/Kavita/releases"; #pragma warning restore S1075 - public VersionUpdaterService(ILogger logger, IEventHub eventHub, IPresenceTracker tracker) + public VersionUpdaterService(ILogger logger, IEventHub eventHub) { _logger = logger; _eventHub = eventHub; - _tracker = tracker; FlurlHttp.ConfigureClient(GithubLatestReleasesUrl, cli => cli.Settings.HttpClientFactory = new UntrustedCertClientFactory()); diff --git a/API/SignalR/EventHub.cs b/API/SignalR/EventHub.cs index fa92d9ec7..50ba20ccf 100644 --- a/API/SignalR/EventHub.cs +++ b/API/SignalR/EventHub.cs @@ -36,9 +36,10 @@ public class EventHub : IEventHub if (onlyAdmins) { var admins = await _presenceTracker.GetOnlineAdmins(); - _messageHub.Clients.Users(admins); + users = _messageHub.Clients.Users(admins); } + await users.SendAsync(method, message); } } diff --git a/API/SignalR/MessageFactory.cs b/API/SignalR/MessageFactory.cs index 485a2fc37..53833e3f3 100644 --- a/API/SignalR/MessageFactory.cs +++ b/API/SignalR/MessageFactory.cs @@ -311,7 +311,6 @@ namespace API.SignalR { Name = CoverUpdate, Title = "Updating Cover", - //SubTitle = series.Name, // TODO: Refactor this Progress = ProgressType.None, Body = new { diff --git a/API/SignalR/SignalRMessage.cs b/API/SignalR/SignalRMessage.cs index b71d0b813..d9564d027 100644 --- a/API/SignalR/SignalRMessage.cs +++ b/API/SignalR/SignalRMessage.cs @@ -34,6 +34,6 @@ namespace API.SignalR /// /// When event took place /// - public DateTime EventTime = DateTime.Now; + public readonly DateTime EventTime = DateTime.Now; } } diff --git a/UI/Web/src/app/admin/manage-users/manage-users.component.html b/UI/Web/src/app/admin/manage-users/manage-users.component.html index c4bbfe553..3fdc66a26 100644 --- a/UI/Web/src/app/admin/manage-users/manage-users.component.html +++ b/UI/Web/src/app/admin/manage-users/manage-users.component.html @@ -12,8 +12,8 @@

{{invite.username | titlecase}}
- - + +