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
This commit is contained in:
Joseph Milazzo 2022-02-09 06:05:51 -08:00 committed by GitHub
parent a63412e3f2
commit 33551f101d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 318 additions and 65 deletions

View File

@ -151,8 +151,6 @@ public class ReaderServiceTests
await _context.SaveChangesAsync();
var fileSystem = new MockFileSystem();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
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<ILogger<DirectoryService>>(), fileSystem);
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
@ -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<Volume>()
{
EntityFactory.CreateVolume("0", new List<Chapter>()
{
EntityFactory.CreateChapter("1", false, new List<MangaFile>()),
EntityFactory.CreateChapter("2", false, new List<MangaFile>()),
}),
EntityFactory.CreateVolume("1", new List<Chapter>()
{
EntityFactory.CreateChapter("21", false, new List<MangaFile>()),
EntityFactory.CreateChapter("22", false, new List<MangaFile>()),
}),
}
});
_context.AppUser.Add(new AppUser()
{
UserName = "majora2007"
});
await _context.SaveChangesAsync();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
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<Volume>()
{
EntityFactory.CreateVolume("1", new List<Chapter>()
{
EntityFactory.CreateChapter("1", false, new List<MangaFile>()),
EntityFactory.CreateChapter("2", false, new List<MangaFile>()),
}),
}
});
_context.AppUser.Add(new AppUser()
{
UserName = "majora2007"
});
await _context.SaveChangesAsync();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
@ -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<Volume>()
{
EntityFactory.CreateVolume("0", new List<Chapter>()
{
EntityFactory.CreateChapter("1", false, new List<MangaFile>()),
EntityFactory.CreateChapter("2", false, new List<MangaFile>()),
}),
EntityFactory.CreateVolume("1", new List<Chapter>()
{
EntityFactory.CreateChapter("1", false, new List<MangaFile>()),
EntityFactory.CreateChapter("2", false, new List<MangaFile>()),
}),
}
});
_context.AppUser.Add(new AppUser()
{
UserName = "majora2007"
});
await _context.SaveChangesAsync();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<Volume>()
{
EntityFactory.CreateVolume("1", new List<Chapter>()
{
EntityFactory.CreateChapter("1", false, new List<MangaFile>()),
EntityFactory.CreateChapter("2", false, new List<MangaFile>()),
}),
EntityFactory.CreateVolume("0", new List<Chapter>()
{
EntityFactory.CreateChapter("A.cbz", true, new List<MangaFile>()),
EntityFactory.CreateChapter("B.cbz", true, new List<MangaFile>()),
}),
}
});
_context.AppUser.Add(new AppUser()
{
UserName = "majora2007"
});
await _context.SaveChangesAsync();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
@ -624,8 +769,6 @@ public class ReaderServiceTests
await _context.SaveChangesAsync();
var fileSystem = new MockFileSystem();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
@ -716,8 +857,6 @@ public class ReaderServiceTests
await _context.SaveChangesAsync();
var fileSystem = new MockFileSystem();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
@ -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<Volume>()
{
EntityFactory.CreateVolume("1", new List<Chapter>()
{
EntityFactory.CreateChapter("1", false, new List<MangaFile>()),
EntityFactory.CreateChapter("2", false, new List<MangaFile>()),
}),
}
});
_context.AppUser.Add(new AppUser()
{
UserName = "majora2007"
});
await _context.SaveChangesAsync();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<Volume>()
{
EntityFactory.CreateVolume("0", new List<Chapter>()
{
EntityFactory.CreateChapter("1", false, new List<MangaFile>()),
EntityFactory.CreateChapter("2", false, new List<MangaFile>()),
}),
}
});
_context.AppUser.Add(new AppUser()
{
UserName = "majora2007"
});
await _context.SaveChangesAsync();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
@ -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<Volume>()
{
EntityFactory.CreateVolume("0", new List<Chapter>()
{
EntityFactory.CreateChapter("1", false, new List<MangaFile>()),
EntityFactory.CreateChapter("2", false, new List<MangaFile>()),
}),
EntityFactory.CreateVolume("1", new List<Chapter>()
{
EntityFactory.CreateChapter("21", false, new List<MangaFile>()),
EntityFactory.CreateChapter("22", false, new List<MangaFile>()),
}),
}
});
_context.AppUser.Add(new AppUser()
{
UserName = "majora2007"
});
await _context.SaveChangesAsync();
var readerService = new ReaderService(_unitOfWork, Substitute.For<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
// 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<ILogger<ReaderService>>());
// 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<ILogger<ReaderService>>());
// 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<ILogger<ReaderService>>());
// 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<ILogger<ReaderService>>());
// 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<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
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<ILogger<ReaderService>>());
var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Progress);

View File

@ -216,7 +216,7 @@ public class ReaderService : IReaderService
/// Tries to find the next logical Chapter
/// </summary>
/// <example>
/// 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
/// </example>
/// <param name="seriesId"></param>
/// <param name="volumeId"></param>
@ -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
/// </summary>
/// <example>
/// 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
/// </example>
/// <param name="seriesId"></param>
/// <param name="volumeId"></param>
@ -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<ChapterDto> chapters, string currentChapterNumber)
private static int GetNextChapterId(IEnumerable<ChapterDto> chapters, string currentChapterNumber, Func<ChapterDto, string> 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;