Bookmark and Reader bugs (#1632)

* Updated swiper and some packages for reported security issues

* Fixed reading lists promotion not working

* Refactor RenameFileForCopy to use iterative recursion, rather than functional.

* Ensured that bookmarks are fetched and ordered by Created date.

* Fixed a bug where bookmarks were coming back in the correct order, but due to filenames, would not sort correctly.

* Default installs to Debug log level given errors users have and Debug not being too noisy

* Added jumpbar to bookmarks page

* Now added jumpbar to bookmarks

* Refactored some code into pipes and added some debug messaging for prefetcher

* Try loading next and prev chapter's first/last page to cache so it renders faster

* Updated GetImage to do a bound check on max page.

Fixed a critical bug in how manga reader updates image elements src to prefetch/load pages. I was not creating a new reference which broke Angular's ability to update DOM on changes.

* Refactored the image setting code to use a single method which tries to use a cached image always.

* Refactored code to use getPage which favors cache and simplifies image creation code
This commit is contained in:
Joe Milazzo 2022-11-02 21:10:19 -04:00 committed by GitHub
parent dab42041d5
commit 38a169818b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
25 changed files with 317 additions and 166 deletions

View file

@ -83,8 +83,9 @@ public class ReaderController : BaseApiController
}
/// <summary>
/// Returns an image for a given chapter. Side effect: This will cache the chapter images for reading.
/// Returns an image for a given chapter. Will perform bounding checks
/// </summary>
/// <remarks>This will cache the chapter images for reading</remarks>
/// <param name="chapterId"></param>
/// <param name="page"></param>
/// <returns></returns>
@ -99,6 +100,7 @@ public class ReaderController : BaseApiController
try
{
// TODO: This code is very generic and repeated, see if we can refactor into a common method
var path = _cacheService.GetCachedPagePath(chapter, page);
if (string.IsNullOrEmpty(path) || !System.IO.File.Exists(path)) return BadRequest($"No such image for page {page}. Try refreshing to allow re-cache.");
var format = Path.GetExtension(path).Replace(".", "");
@ -128,7 +130,6 @@ public class ReaderController : BaseApiController
if (page < 0) page = 0;
var userId = await _unitOfWork.UserRepository.GetUserIdByApiKeyAsync(apiKey);
// NOTE: I'm not sure why I need this flow here
var totalPages = await _cacheService.CacheBookmarkForSeries(userId, seriesId);
if (page > totalPages)
{
@ -139,7 +140,7 @@ public class ReaderController : BaseApiController
{
var path = _cacheService.GetCachedBookmarkPagePath(seriesId, page);
if (string.IsNullOrEmpty(path) || !System.IO.File.Exists(path)) return BadRequest($"No such image for page {page}");
var format = Path.GetExtension(path).Replace(".", "");
var format = Path.GetExtension(path).Replace(".", string.Empty);
return PhysicalFile(path, "image/" + format, Path.GetFileName(path));
}

View file

@ -1,4 +1,5 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using API.Comparators;

View file

@ -1,10 +1,14 @@
namespace API.DTOs.ReadingLists;
using System;
using System.ComponentModel.DataAnnotations;
namespace API.DTOs.ReadingLists;
public class UpdateReadingListDto
{
[Required]
public int ReadingListId { get; set; }
public string Title { get; set; }
public string Summary { get; set; }
public string Title { get; set; } = string.Empty;
public string Summary { get; set; } = string.Empty;
public bool Promoted { get; set; }
public bool CoverImageLocked { get; set; }
}

View file

@ -305,7 +305,7 @@ public class UserRepository : IUserRepository
{
return await _context.AppUserBookmark
.Where(x => x.AppUserId == userId && x.SeriesId == seriesId)
.OrderBy(x => x.Page)
.OrderBy(x => x.Created)
.AsNoTracking()
.ProjectTo<BookmarkDto>(_mapper.ConfigurationProvider)
.ToListAsync();
@ -315,7 +315,7 @@ public class UserRepository : IUserRepository
{
return await _context.AppUserBookmark
.Where(x => x.AppUserId == userId && x.VolumeId == volumeId)
.OrderBy(x => x.Page)
.OrderBy(x => x.Created)
.AsNoTracking()
.ProjectTo<BookmarkDto>(_mapper.ConfigurationProvider)
.ToListAsync();
@ -325,7 +325,7 @@ public class UserRepository : IUserRepository
{
return await _context.AppUserBookmark
.Where(x => x.AppUserId == userId && x.ChapterId == chapterId)
.OrderBy(x => x.Page)
.OrderBy(x => x.Created)
.AsNoTracking()
.ProjectTo<BookmarkDto>(_mapper.ConfigurationProvider)
.ToListAsync();
@ -341,25 +341,27 @@ public class UserRepository : IUserRepository
{
var query = _context.AppUserBookmark
.Where(x => x.AppUserId == userId)
.OrderBy(x => x.Page)
.OrderBy(x => x.Created)
.AsNoTracking();
if (!string.IsNullOrEmpty(filter.SeriesNameQuery))
{
var seriesNameQueryNormalized = Services.Tasks.Scanner.Parser.Parser.Normalize(filter.SeriesNameQuery);
var filterSeriesQuery = query.Join(_context.Series, b => b.SeriesId, s => s.Id, (bookmark, series) => new
{
bookmark,
series
})
.Where(o => EF.Functions.Like(o.series.Name, $"%{filter.SeriesNameQuery}%")
|| EF.Functions.Like(o.series.OriginalName, $"%{filter.SeriesNameQuery}%")
|| EF.Functions.Like(o.series.LocalizedName, $"%{filter.SeriesNameQuery}%")
|| EF.Functions.Like(o.series.NormalizedName, $"%{seriesNameQueryNormalized}%")
);
if (string.IsNullOrEmpty(filter.SeriesNameQuery))
return await query
.ProjectTo<BookmarkDto>(_mapper.ConfigurationProvider)
.ToListAsync();
query = filterSeriesQuery.Select(o => o.bookmark);
}
var seriesNameQueryNormalized = Services.Tasks.Scanner.Parser.Parser.Normalize(filter.SeriesNameQuery);
var filterSeriesQuery = query.Join(_context.Series, b => b.SeriesId, s => s.Id, (bookmark, series) => new
{
bookmark,
series
})
.Where(o => EF.Functions.Like(o.series.Name, $"%{filter.SeriesNameQuery}%")
|| EF.Functions.Like(o.series.OriginalName, $"%{filter.SeriesNameQuery}%")
|| EF.Functions.Like(o.series.LocalizedName, $"%{filter.SeriesNameQuery}%")
|| EF.Functions.Like(o.series.NormalizedName, $"%{seriesNameQueryNormalized}%")
);
query = filterSeriesQuery.Select(o => o.bookmark);
return await query

View file

@ -79,10 +79,7 @@ public static class Seed
{
new() {Key = ServerSettingKey.CacheDirectory, Value = directoryService.CacheDirectory},
new() {Key = ServerSettingKey.TaskScan, Value = "daily"},
new()
{
Key = ServerSettingKey.LoggingLevel, Value = "Information"
}, // Not used from DB, but DB is sync with appSettings.json
new() {Key = ServerSettingKey.LoggingLevel, Value = "Debug"},
new() {Key = ServerSettingKey.TaskBackup, Value = "daily"},
new()
{

View file

@ -1,4 +1,5 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
@ -101,7 +102,7 @@ public class CacheService : ICacheService
var extractPath = GetCachePath(chapterId);
if (_directoryService.Exists(extractPath)) return chapter;
var files = chapter.Files.ToList();
var files = chapter?.Files.ToList();
ExtractChapterFiles(extractPath, files);
return chapter;
@ -223,6 +224,8 @@ public class CacheService : ICacheService
return string.Empty;
}
if (page > files.Length) page = files.Length;
// Since array is 0 based, we need to keep that in account (only affects last image)
return page == files.Length ? files.ElementAt(page - 1) : files.ElementAt(page);
}
@ -234,8 +237,8 @@ public class CacheService : ICacheService
var bookmarkDtos = await _unitOfWork.UserRepository.GetBookmarkDtosForSeries(userId, seriesId);
var files = (await _bookmarkService.GetBookmarkFilesById(bookmarkDtos.Select(b => b.Id))).ToList();
_directoryService.CopyFilesToDirectory(files, destDirectory);
_directoryService.Flatten(destDirectory);
_directoryService.CopyFilesToDirectory(files, destDirectory,
Enumerable.Range(1, files.Count).Select(i => i + string.Empty).ToList());
return files.Count;
}

View file

@ -37,6 +37,7 @@ public interface IDirectoryService
IEnumerable<DirectoryDto> ListDirectory(string rootPath);
Task<byte[]> ReadFileAsync(string path);
bool CopyFilesToDirectory(IEnumerable<string> filePaths, string directoryPath, string prepend = "");
bool CopyFilesToDirectory(IEnumerable<string> filePaths, string directoryPath, IList<string> newFilenames);
bool Exists(string directory);
void CopyFileToDirectory(string fullFilePath, string targetDirectory);
int TraverseTreeParallelForEach(string root, Action<string> action, string searchPattern, ILogger logger);
@ -424,6 +425,46 @@ public class DirectoryService : IDirectoryService
return true;
}
/// <summary>
/// Copies files to a destination directory. If the destination directory doesn't exist, this will create it.
/// </summary>
/// <remarks>If a file already exists in dest, this will rename as (2). It does not support multiple iterations of this. Overwriting is not supported.</remarks>
/// <param name="filePaths"></param>
/// <param name="directoryPath"></param>
/// <param name="newFilenames">A list that matches one to one with filePaths. Each filepath will be renamed to newFilenames</param>
/// <returns></returns>
public bool CopyFilesToDirectory(IEnumerable<string> filePaths, string directoryPath, IList<string> newFilenames)
{
ExistOrCreate(directoryPath);
string currentFile = null;
var index = 0;
try
{
foreach (var file in filePaths)
{
currentFile = file;
if (!FileSystem.File.Exists(file))
{
_logger.LogError("Unable to copy {File} to {DirectoryPath} as it doesn't exist", file, directoryPath);
continue;
}
var fileInfo = FileSystem.FileInfo.FromFileName(file);
var targetFile = FileSystem.FileInfo.FromFileName(RenameFileForCopy(newFilenames[index] + fileInfo.Extension, directoryPath));
fileInfo.CopyTo(FileSystem.Path.Join(directoryPath, targetFile.Name));
index++;
}
}
catch (Exception ex)
{
_logger.LogError(ex, "Unable to copy {File} to {DirectoryPath}", currentFile, directoryPath);
return false;
}
return true;
}
/// <summary>
/// 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
@ -434,30 +475,32 @@ public class DirectoryService : IDirectoryService
/// <returns></returns>
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)
while (true)
{
return targetFile.FullName;
}
var fileInfo = FileSystem.FileInfo.FromFileName(fileToCopy);
var filename = prepend + fileInfo.Name;
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 targetFile = FileSystem.FileInfo.FromFileName(FileSystem.Path.Join(directoryPath, filename));
if (!targetFile.Exists)
{
return targetFile.FullName;
}
var newFilename = prepend + noExtension +
FileSystem.Path.GetExtension(fileInfo.Name);
return RenameFileForCopy(FileSystem.Path.Join(directoryPath, newFilename), directoryPath, prepend);
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);
fileToCopy = FileSystem.Path.Join(directoryPath, newFilename);
}
}
/// <summary>