From 33551f101dfa19f60bfa0239999f0ddd26b35edd Mon Sep 17 00:00:00 2001 From: Joseph Milazzo Date: Wed, 9 Feb 2022 06:05:51 -0800 Subject: [PATCH] Reading Order Fix (#1050) * Fixed a bug in Get Next/Prev Chapter where chapters were not correctly between volumes and specials. They now behave correctly * Fixed the unit tests and added a lot more edge cases for GetNextChapter/GetPrevChapter --- API.Tests/Services/ReaderServiceTests.cs | 333 +++++++++++++++++++---- API/Services/ReaderService.cs | 50 +++- 2 files changed, 318 insertions(+), 65 deletions(-) diff --git a/API.Tests/Services/ReaderServiceTests.cs b/API.Tests/Services/ReaderServiceTests.cs index 75cfaf73f..2356eaef7 100644 --- a/API.Tests/Services/ReaderServiceTests.cs +++ b/API.Tests/Services/ReaderServiceTests.cs @@ -151,8 +151,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); Assert.Equal(0, await readerService.CapPageToChapter(1, -1)); @@ -197,8 +195,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var successful = await readerService.SaveReadingProgress(new ProgressDto() @@ -248,8 +244,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var successful = await readerService.SaveReadingProgress(new ProgressDto() @@ -320,8 +314,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var volumes = await _unitOfWork.VolumeRepository.GetVolumes(1); @@ -372,8 +364,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var ds = new DirectoryService(Substitute.For>(), fileSystem); var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var volumes = (await _unitOfWork.VolumeRepository.GetVolumes(1)).ToList(); @@ -434,8 +424,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var nextChapter = await readerService.GetNextChapterIdAsync(1, 1, 1, 1); @@ -482,8 +470,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); @@ -493,7 +479,50 @@ public class ReaderServiceTests } [Fact] - public async Task GetNextChapterIdAsync_ShouldNotMoveFromVolumeToSpecial() + public async Task GetNextChapterIdAsync_ShouldRollIntoChaptersFromVolume() + { + await ResetDB(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("0", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + EntityFactory.CreateVolume("1", new List() + { + EntityFactory.CreateChapter("21", false, new List()), + EntityFactory.CreateChapter("22", false, new List()), + }), + } + }); + + _context.AppUser.Add(new AppUser() + { + UserName = "majora2007" + }); + + await _context.SaveChangesAsync(); + + var readerService = new ReaderService(_unitOfWork, Substitute.For>()); + + + var nextChapter = await readerService.GetNextChapterIdAsync(1, 2, 4, 1); + Assert.NotEqual(-1, nextChapter); + var actualChapter = await _unitOfWork.ChapterRepository.GetChapterAsync(nextChapter); + Assert.Equal("1", actualChapter.Range); + } + + [Fact] + public async Task GetNextChapterIdAsync_ShouldFindNoNextChapterFromSpecial() { await ResetDB(); @@ -526,7 +555,41 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); + var readerService = new ReaderService(_unitOfWork, Substitute.For>()); + + + var nextChapter = await readerService.GetNextChapterIdAsync(1, 2, 4, 1); + Assert.Equal(-1, nextChapter); + } + + [Fact] + public async Task GetNextChapterIdAsync_ShouldFindNoNextChapterFromVolume() + { + await ResetDB(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("1", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + } + }); + + _context.AppUser.Add(new AppUser() + { + UserName = "majora2007" + }); + + await _context.SaveChangesAsync(); var readerService = new ReaderService(_unitOfWork, Substitute.For>()); @@ -535,6 +598,90 @@ public class ReaderServiceTests Assert.Equal(-1, nextChapter); } + [Fact] + public async Task GetNextChapterIdAsync_ShouldFindNoNextChapterFromLastChapter() + { + await ResetDB(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("0", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + EntityFactory.CreateVolume("1", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + } + }); + + _context.AppUser.Add(new AppUser() + { + UserName = "majora2007" + }); + + await _context.SaveChangesAsync(); + + var readerService = new ReaderService(_unitOfWork, Substitute.For>()); + + + var nextChapter = await readerService.GetNextChapterIdAsync(1, 1, 2, 1); + Assert.Equal(-1, nextChapter); + } + + [Fact] + public async Task GetNextChapterIdAsync_ShouldMoveFromVolumeToSpecial() + { + await ResetDB(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("1", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + EntityFactory.CreateVolume("0", new List() + { + EntityFactory.CreateChapter("A.cbz", true, new List()), + EntityFactory.CreateChapter("B.cbz", true, new List()), + }), + } + }); + + _context.AppUser.Add(new AppUser() + { + UserName = "majora2007" + }); + + await _context.SaveChangesAsync(); + + var readerService = new ReaderService(_unitOfWork, Substitute.For>()); + + + var nextChapter = await readerService.GetNextChapterIdAsync(1, 1, 2, 1); + Assert.NotEqual(-1, nextChapter); + var actualChapter = await _unitOfWork.ChapterRepository.GetChapterAsync(nextChapter); + Assert.Equal("A.cbz", actualChapter.Range); + } + [Fact] public async Task GetNextChapterIdAsync_ShouldMoveFromSpecialToSpecial() { @@ -569,8 +716,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); @@ -624,8 +769,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var prevChapter = await readerService.GetPrevChapterIdAsync(1, 1, 2, 1); @@ -672,8 +815,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); @@ -716,8 +857,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); @@ -727,6 +866,78 @@ public class ReaderServiceTests Assert.Equal("B.cbz", actualChapter.Range); } + [Fact] + public async Task GetPrevChapterIdAsync_ShouldFindNoPrevChapterFromVolume() + { + await ResetDB(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("1", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + } + }); + + _context.AppUser.Add(new AppUser() + { + UserName = "majora2007" + }); + + await _context.SaveChangesAsync(); + + var readerService = new ReaderService(_unitOfWork, Substitute.For>()); + + + var prevChapter = await readerService.GetPrevChapterIdAsync(1, 1, 1, 1); + Assert.Equal(-1, prevChapter); + } + + [Fact] + public async Task GetPrevChapterIdAsync_ShouldFindNoPrevChapterFromChapter() + { + await ResetDB(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("0", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + } + }); + + _context.AppUser.Add(new AppUser() + { + UserName = "majora2007" + }); + + await _context.SaveChangesAsync(); + + var readerService = new ReaderService(_unitOfWork, Substitute.For>()); + + + var prevChapter = await readerService.GetPrevChapterIdAsync(1, 1, 1, 1); + Assert.Equal(-1, prevChapter); + } + [Fact] public async Task GetPrevChapterIdAsync_ShouldMoveFromSpecialToSpecial() { @@ -761,8 +972,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); @@ -772,6 +981,48 @@ public class ReaderServiceTests Assert.Equal("A.cbz", actualChapter.Range); } + [Fact] + public async Task GetPrevChapterIdAsync_ShouldMoveFromChapterToVolume() + { + await ResetDB(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("0", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + EntityFactory.CreateVolume("1", new List() + { + EntityFactory.CreateChapter("21", false, new List()), + EntityFactory.CreateChapter("22", false, new List()), + }), + } + }); + + _context.AppUser.Add(new AppUser() + { + UserName = "majora2007" + }); + + await _context.SaveChangesAsync(); + + var readerService = new ReaderService(_unitOfWork, Substitute.For>()); + + + var prevChapter = await readerService.GetPrevChapterIdAsync(1, 1, 1, 1); + Assert.NotEqual(-1, prevChapter); + var actualChapter = await _unitOfWork.ChapterRepository.GetChapterAsync(prevChapter); + Assert.Equal("22", actualChapter.Range); + } #endregion #region GetContinuePoint @@ -814,9 +1065,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); // Save progress on first volume chapters and 1st of second volume @@ -888,9 +1136,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); // Save progress on first volume chapters and 1st of second volume @@ -954,10 +1199,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - - - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); // Save progress on first volume chapters and 1st of second volume @@ -1018,10 +1259,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - - - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); // Save progress on first volume chapters and 1st of second volume @@ -1083,10 +1320,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - - - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); // Save progress on first volume chapters and 1st of second volume @@ -1152,10 +1385,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - - - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Progress); @@ -1199,10 +1428,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - - - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Progress); @@ -1247,10 +1472,6 @@ public class ReaderServiceTests await _context.SaveChangesAsync(); - - - var fileSystem = new MockFileSystem(); - var readerService = new ReaderService(_unitOfWork, Substitute.For>()); var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Progress); diff --git a/API/Services/ReaderService.cs b/API/Services/ReaderService.cs index 6af585a0c..36a98317b 100644 --- a/API/Services/ReaderService.cs +++ b/API/Services/ReaderService.cs @@ -216,7 +216,7 @@ public class ReaderService : IReaderService /// Tries to find the next logical Chapter /// /// - /// V1 → V2 → V3 chapter 0 → V3 chapter 10 → SP 01 → SP 02 + /// V1 → V2 → V3 chapter 0 → V3 chapter 10 → V0 chapter 1 -> V0 chapter 2 -> SP 01 → SP 02 /// /// /// @@ -232,7 +232,7 @@ public class ReaderService : IReaderService if (currentVolume.Number == 0) { // Handle specials by sorting on their Filename aka Range - var chapterId = GetNextChapterId(currentVolume.Chapters.OrderByNatural(x => x.Range), currentChapter.Number); + var chapterId = GetNextChapterId(currentVolume.Chapters.OrderByNatural(x => x.Range), currentChapter.Range, dto => dto.Range); if (chapterId > 0) return chapterId; } @@ -242,8 +242,10 @@ public class ReaderService : IReaderService { // Handle Chapters within current Volume // In this case, i need 0 first because 0 represents a full volume file. - var chapterId = GetNextChapterId(currentVolume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparerForInChapterSorting), currentChapter.Number); + var chapterId = GetNextChapterId(currentVolume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparerForInChapterSorting), + currentChapter.Range, dto => dto.Range); if (chapterId > 0) return chapterId; + } if (volume.Number != currentVolume.Number + 1) continue; @@ -257,9 +259,26 @@ public class ReaderService : IReaderService } var firstChapter = chapters.FirstOrDefault(); + if (firstChapter == null) break; + var isSpecial = firstChapter.IsSpecial || currentChapter.IsSpecial; + if (isSpecial) + { + var chapterId = GetNextChapterId(volume.Chapters.OrderByNatural(x => x.Number), + currentChapter.Range, dto => dto.Range); + if (chapterId > 0) return chapterId; + } else if (double.Parse(firstChapter.Number) > double.Parse(currentChapter.Number)) return firstChapter.Id; + } + + // If we are the last volume and we didn't find any next volume, loop back to volume 0 and give the first chapter + // This has an added problem that it will loop up to the beginning always + // Should I change this to Max number? volumes.LastOrDefault()?.Number -> volumes.Max(v => v.Number) + if (currentVolume.Number != 0 && currentVolume.Number == volumes.LastOrDefault()?.Number && volumes.Count > 1) + { + var chapterVolume = volumes.FirstOrDefault(); + if (chapterVolume?.Number != 0) return -1; + var firstChapter = chapterVolume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparer).FirstOrDefault(); if (firstChapter == null) return -1; return firstChapter.Id; - } return -1; @@ -268,7 +287,7 @@ public class ReaderService : IReaderService /// Tries to find the prev logical Chapter /// /// - /// V1 ← V2 ← V3 chapter 0 ← V3 chapter 10 ← SP 01 ← SP 02 + /// V1 ← V2 ← V3 chapter 0 ← V3 chapter 10 ← V0 chapter 1 ← V0 chapter 2 ← SP 01 ← SP 02 /// /// /// @@ -283,7 +302,7 @@ public class ReaderService : IReaderService if (currentVolume.Number == 0) { - var chapterId = GetNextChapterId(currentVolume.Chapters.OrderByNatural(x => x.Range).Reverse(), currentChapter.Number); + var chapterId = GetNextChapterId(currentVolume.Chapters.OrderByNatural(x => x.Range).Reverse(), currentChapter.Number, dto => dto.Number); if (chapterId > 0) return chapterId; } @@ -291,7 +310,8 @@ public class ReaderService : IReaderService { if (volume.Number == currentVolume.Number) { - var chapterId = GetNextChapterId(currentVolume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparerForInChapterSorting).Reverse(), currentChapter.Number); + var chapterId = GetNextChapterId(currentVolume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparerForInChapterSorting).Reverse(), + currentChapter.Number, dto => dto.Number); if (chapterId > 0) return chapterId; } if (volume.Number == currentVolume.Number - 1) @@ -302,6 +322,16 @@ public class ReaderService : IReaderService return lastChapter.Id; } } + + var lastVolume = volumes.OrderBy(v => v.Number).LastOrDefault(); + if (currentVolume.Number == 0 && currentVolume.Number != lastVolume?.Number && lastVolume?.Chapters.Count > 1) + { + var lastChapter = lastVolume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparerForInChapterSorting).LastOrDefault(); + if (lastChapter == null) return -1; + return lastChapter.Id; + } + + return -1; } @@ -331,7 +361,7 @@ public class ReaderService : IReaderService } - private static int GetNextChapterId(IEnumerable chapters, string currentChapterNumber) + private static int GetNextChapterId(IEnumerable chapters, string currentChapterNumber, Func accessor) { var next = false; var chaptersList = chapters.ToList(); @@ -341,7 +371,9 @@ public class ReaderService : IReaderService { return chapter.Id; } - if (currentChapterNumber.Equals(chapter.Number)) next = true; + + var chapterNum = accessor(chapter); + if (currentChapterNumber.Equals(chapterNum)) next = true; } return -1;