Skip to content

Commit

Permalink
Bookmark RBS + Dynamic PGO (Kareadita#1503)
Browse files Browse the repository at this point in the history
* Allow .NET to optimize code as it's running.

* Implemented the ability to restrict users Bookmark ability. By default, users will need to now opt-in to get bookmark roles.

* Fixed a tachiyomi progress syncing logic bug
  • Loading branch information
majora2007 authored Sep 2, 2022
1 parent 30500a4 commit 2283ae5
Show file tree
Hide file tree
Showing 14 changed files with 75 additions and 28 deletions.
2 changes: 2 additions & 0 deletions API/API.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
<DockerDefaultTargetOS>Linux</DockerDefaultTargetOS>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<TieredPGO>true</TieredPGO>
<TieredCompilation>true</TieredCompilation>
</PropertyGroup>

<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
Expand Down
6 changes: 5 additions & 1 deletion API/Constants/PolicyConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ public static class PolicyConstants
/// Used to give a user ability to change their own password
/// </summary>
public const string ChangePasswordRole = "Change Password";
/// <summary>
/// Used to give a user ability to bookmark files on the server
/// </summary>
public const string BookmarkRole = "Bookmark";

public static readonly ImmutableArray<string> ValidRoles =
ImmutableArray.Create(AdminRole, PlebRole, DownloadRole, ChangePasswordRole);
ImmutableArray.Create(AdminRole, PlebRole, DownloadRole, ChangePasswordRole, BookmarkRole);
}
}
7 changes: 5 additions & 2 deletions API/Controllers/DownloadController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ public class DownloadController : BaseApiController
private readonly IEventHub _eventHub;
private readonly ILogger<DownloadController> _logger;
private readonly IBookmarkService _bookmarkService;
private readonly IAccountService _accountService;
private const string DefaultContentType = "application/octet-stream";

public DownloadController(IUnitOfWork unitOfWork, IArchiveService archiveService, IDirectoryService directoryService,
IDownloadService downloadService, IEventHub eventHub, ILogger<DownloadController> logger, IBookmarkService bookmarkService)
IDownloadService downloadService, IEventHub eventHub, ILogger<DownloadController> logger, IBookmarkService bookmarkService,
IAccountService accountService)
{
_unitOfWork = unitOfWork;
_archiveService = archiveService;
Expand All @@ -43,6 +45,7 @@ public DownloadController(IUnitOfWork unitOfWork, IArchiveService archiveService
_eventHub = eventHub;
_logger = logger;
_bookmarkService = bookmarkService;
_accountService = accountService;
}

/// <summary>
Expand Down Expand Up @@ -109,7 +112,7 @@ public async Task<ActionResult> DownloadVolume(int volumeId)
private async Task<bool> HasDownloadPermission()
{
var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername());
return await _downloadService.HasDownloadPermission(user);
return await _accountService.HasDownloadPermission(user);
}

private ActionResult GetFirstFileDownload(IEnumerable<MangaFile> files)
Expand Down
7 changes: 5 additions & 2 deletions API/Controllers/OPDSController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class OpdsController : BaseApiController
private readonly ICacheService _cacheService;
private readonly IReaderService _readerService;
private readonly ISeriesService _seriesService;
private readonly IAccountService _accountService;


private readonly XmlSerializer _xmlSerializer;
Expand Down Expand Up @@ -65,14 +66,16 @@ public class OpdsController : BaseApiController

public OpdsController(IUnitOfWork unitOfWork, IDownloadService downloadService,
IDirectoryService directoryService, ICacheService cacheService,
IReaderService readerService, ISeriesService seriesService)
IReaderService readerService, ISeriesService seriesService,
IAccountService accountService)
{
_unitOfWork = unitOfWork;
_downloadService = downloadService;
_directoryService = directoryService;
_cacheService = cacheService;
_readerService = readerService;
_seriesService = seriesService;
_accountService = accountService;

_xmlSerializer = new XmlSerializer(typeof(Feed));
_xmlOpenSearchSerializer = new XmlSerializer(typeof(OpenSearchDescription));
Expand Down Expand Up @@ -619,7 +622,7 @@ public async Task<ActionResult> DownloadFile(string apiKey, int seriesId, int vo
if (!(await _unitOfWork.SettingsRepository.GetSettingsDtoAsync()).EnableOpds)
return BadRequest("OPDS is not enabled on this server");
var user = await _unitOfWork.UserRepository.GetUserByIdAsync(await GetUser(apiKey));
if (!await _downloadService.HasDownloadPermission(user))
if (!await _accountService.HasDownloadPermission(user))
{
return BadRequest("User does not have download permissions");
}
Expand Down
16 changes: 14 additions & 2 deletions API/Controllers/ReaderController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,20 @@ public class ReaderController : BaseApiController
private readonly ILogger<ReaderController> _logger;
private readonly IReaderService _readerService;
private readonly IBookmarkService _bookmarkService;
private readonly IAccountService _accountService;

/// <inheritdoc />
public ReaderController(ICacheService cacheService,
IUnitOfWork unitOfWork, ILogger<ReaderController> logger,
IReaderService readerService, IBookmarkService bookmarkService)
IReaderService readerService, IBookmarkService bookmarkService,
IAccountService accountService)
{
_cacheService = cacheService;
_unitOfWork = unitOfWork;
_logger = logger;
_readerService = readerService;
_bookmarkService = bookmarkService;
_accountService = accountService;
}

/// <summary>
Expand Down Expand Up @@ -657,8 +660,13 @@ public async Task<ActionResult<IEnumerable<BookmarkDto>>> GetBookmarksForSeries(
public async Task<ActionResult> BookmarkPage(BookmarkDto bookmarkDto)
{
// Don't let user save past total pages.
bookmarkDto.Page = await _readerService.CapPageToChapter(bookmarkDto.ChapterId, bookmarkDto.Page);
var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername(), AppUserIncludes.Bookmarks);
if (user == null) return new UnauthorizedResult();

if (!await _accountService.HasBookmarkPermission(user))
return BadRequest("You do not have permission to bookmark");

bookmarkDto.Page = await _readerService.CapPageToChapter(bookmarkDto.ChapterId, bookmarkDto.Page);
var chapter = await _cacheService.Ensure(bookmarkDto.ChapterId);
if (chapter == null) return BadRequest("Could not find cached image. Reload and try again.");
var path = _cacheService.GetCachedPagePath(chapter, bookmarkDto.Page);
Expand All @@ -681,8 +689,12 @@ public async Task<ActionResult> BookmarkPage(BookmarkDto bookmarkDto)
public async Task<ActionResult> UnBookmarkPage(BookmarkDto bookmarkDto)
{
var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername(), AppUserIncludes.Bookmarks);
if (user == null) return new UnauthorizedResult();
if (user.Bookmarks == null) return Ok();

if (!await _accountService.HasBookmarkPermission(user))
return BadRequest("You do not have permission to unbookmark");

if (await _bookmarkService.RemoveBookmarkPage(user, bookmarkDto))
{
BackgroundJob.Enqueue(() => _cacheService.CleanupBookmarkCache(bookmarkDto.SeriesId));
Expand Down
2 changes: 1 addition & 1 deletion API/Controllers/TachiyomiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public async Task<ActionResult<ChapterDto>> GetLatestChapter(int seriesId)
if (prevChapterId == -1)
{
var series = await _unitOfWork.SeriesRepository.GetSeriesDtoByIdAsync(seriesId, userId);
var userHasProgress = series.PagesRead != 0 && series.PagesRead < series.Pages;
var userHasProgress = series.PagesRead != 0 && series.PagesRead <= series.Pages;

// If the user doesn't have progress, then return null, which the extension will catch as 204 (no content) and report nothing as read
if (!userHasProgress) return null;
Expand Down
26 changes: 26 additions & 0 deletions API/Services/AccountService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using API.Constants;
using API.Data;
using API.Entities;
using API.Errors;
Expand All @@ -17,6 +18,8 @@ public interface IAccountService
Task<IEnumerable<ApiException>> ValidatePassword(AppUser user, string password);
Task<IEnumerable<ApiException>> ValidateUsername(string username);
Task<IEnumerable<ApiException>> ValidateEmail(string email);
Task<bool> HasBookmarkPermission(AppUser user);
Task<bool> HasDownloadPermission(AppUser appuser);
}

public class AccountService : IAccountService
Expand Down Expand Up @@ -92,5 +95,28 @@ public async Task<IEnumerable<ApiException>> ValidateEmail(string email)
new ApiException(400, "Email is already registered")
};
}

/// <summary>
/// Does the user have the Bookmark permission or admin rights
/// </summary>
/// <param name="user"></param>
/// <returns></returns>
public async Task<bool> HasBookmarkPermission(AppUser user)
{
var roles = await _userManager.GetRolesAsync(user);
return roles.Contains(PolicyConstants.BookmarkRole) || roles.Contains(PolicyConstants.AdminRole);
}

/// <summary>
/// Does the user have the Download permission or admin rights
/// </summary>
/// <param name="user"></param>
/// <returns></returns>
public async Task<bool> HasDownloadPermission(AppUser user)
{
var roles = await _userManager.GetRolesAsync(user);
return roles.Contains(PolicyConstants.DownloadRole) || roles.Contains(PolicyConstants.AdminRole);
}

}
}
1 change: 1 addition & 0 deletions API/Services/BookmarkService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using API.Constants;
using API.Data;
using API.DTOs.Reader;
using API.Entities;
Expand Down
14 changes: 2 additions & 12 deletions API/Services/DownloadService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using API.Constants;
using API.Entities;
using Microsoft.AspNetCore.Identity;
using Microsoft.AspNetCore.StaticFiles;
Expand All @@ -14,17 +13,12 @@ public interface IDownloadService
{
Tuple<string, string, string> GetFirstFileDownload(IEnumerable<MangaFile> files);
string GetContentTypeFromFile(string filepath);
Task<bool> HasDownloadPermission(AppUser user);
}
public class DownloadService : IDownloadService
{
private readonly UserManager<AppUser> _userManager;
private readonly FileExtensionContentTypeProvider _fileTypeProvider = new FileExtensionContentTypeProvider();

public DownloadService(UserManager<AppUser> userManager)
{
_userManager = userManager;
}
public DownloadService() { }

/// <summary>
/// Downloads the first file in the file enumerable for download
Expand Down Expand Up @@ -62,9 +56,5 @@ public string GetContentTypeFromFile(string filepath)
return contentType;
}

public async Task<bool> HasDownloadPermission(AppUser user)
{
var roles = await _userManager.GetRolesAsync(user);
return roles.Contains(PolicyConstants.DownloadRole) || roles.Contains(PolicyConstants.AdminRole);
}

}
3 changes: 2 additions & 1 deletion Kavita.Common/Kavita.Common.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<Product>Kavita</Product>
<AssemblyVersion>0.5.6.1</AssemblyVersion>
<NeutralLanguage>en</NeutralLanguage>
<TieredPGO>true</TieredPGO>
</PropertyGroup>

<ItemGroup>
Expand All @@ -20,4 +21,4 @@
</ItemGroup>


</Project>
</Project>
4 changes: 4 additions & 0 deletions UI/Web/src/app/_services/account.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ export class AccountService implements OnDestroy {
return user && user.roles.includes('Download');
}

hasBookmarkRole(user: User) {
return user && user.roles.includes('Bookmark');
}

getRoles() {
return this.httpClient.get<string[]>(this.baseUrl + 'account/roles');
}
Expand Down
2 changes: 1 addition & 1 deletion UI/Web/src/app/admin/edit-user/edit-user.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ <h4 class="modal-title" id="modal-basic-title">Edit {{member.username | sentence
<div class="col-md-6 col-sm-12">
<div class="mb-3" style="width:100%">
<label for="email" class="form-label">Email</label>
<input class="form-control" type="email" id="email" formControlName="email" [disabled]="true">
<input class="form-control" type="email" id="email" formControlName="email">
<div id="inviteForm-validations" class="invalid-feedback" *ngIf="userForm.dirty || userForm.touched">
<div *ngIf="userForm.get('email')?.errors?.required">
This field is required
Expand Down
3 changes: 1 addition & 2 deletions UI/Web/src/app/manga-reader/manga-reader.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
<i class="fa-regular fa-rectangle-list" aria-hidden="true"></i>
<span class="visually-hidden">Keyboard Shortcuts Modal</span>
</button>

<button *ngIf="!bookmarkMode" class="btn btn-icon btn-small" role="checkbox" [attr.aria-checked]="CurrentPageBookmarked"
<button *ngIf="!bookmarkMode && hasBookmarkRights" class="btn btn-icon btn-small" role="checkbox" [attr.aria-checked]="CurrentPageBookmarked"
title="{{CurrentPageBookmarked ? 'Unbookmark Page' : 'Bookmark Page'}}" (click)="bookmarkPage()">
<i class="{{CurrentPageBookmarked ? 'fa' : 'far'}} fa-bookmark" aria-hidden="true"></i>
<span class="visually-hidden">{{CurrentPageBookmarked ? 'Unbookmark Page' : 'Bookmark Page'}}</span>
Expand Down
10 changes: 6 additions & 4 deletions UI/Web/src/app/manga-reader/manga-reader.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
layoutModes = layoutModes;

isLoading = true;
hasBookmarkRights: boolean = false;

private ctx!: CanvasRenderingContext2D;
/**
Expand Down Expand Up @@ -153,7 +154,7 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
* Responsible to hold current page -2 2. Used to know if we should render
* @remarks Used solely for LayoutMode.DoubleReverse rendering.
*/
canvasImageBehindBy2 = new Image();
canvasImageBehindBy2 = new Image();
/**
* Dictates if we use render with canvas or with image.
* @remarks This is only for Splitting.
Expand All @@ -174,16 +175,16 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
/**
* An event emitter when a page change occurs. Used solely by the webtoon reader.
*/
goToPageEvent!: BehaviorSubject<number>;
goToPageEvent!: BehaviorSubject<number>;

/**
* An event emitter when a bookmark on a page change occurs. Used solely by the webtoon reader.
*/
showBookmarkEffectEvent: ReplaySubject<number> = new ReplaySubject<number>();
showBookmarkEffectEvent: ReplaySubject<number> = new ReplaySubject<number>();
/**
* An event emitter when fullscreen mode is toggled. Used solely by the webtoon reader.
*/
fullscreenEvent: ReplaySubject<boolean> = new ReplaySubject<boolean>();
fullscreenEvent: ReplaySubject<boolean> = new ReplaySubject<boolean>();
/**
* If the menu is open/visible.
*/
Expand Down Expand Up @@ -467,6 +468,7 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
}

this.user = user;
this.hasBookmarkRights = this.accountService.hasBookmarkRole(user);
this.readingDirection = this.user.preferences.readingDirection;
this.scalingOption = this.user.preferences.scalingOption;
this.pageSplitOption = this.user.preferences.pageSplitOption;
Expand Down

0 comments on commit 2283ae5

Please sign in to comment.