Bookmark RBS + Dynamic PGO (#1503)

* 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

API/API.csproj

@@ -6,6 +6,8 @@
     <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
     <DockerDefaultTargetOS>Linux</DockerDefaultTargetOS>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
+    <TieredPGO>true</TieredPGO>
+    <TieredCompilation>true</TieredCompilation>
   </PropertyGroup>
 
   <PropertyGroup Condition=" '$(Configuration)' == 'Release' ">

API/Constants/PolicyConstants.cs

@@ -23,8 +23,12 @@ namespace API.Constants
         /// 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);
     }
 }

API/Controllers/DownloadController.cs

@@ -31,10 +31,12 @@ namespace API.Controllers
         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;
@@ -43,6 +45,7 @@ namespace API.Controllers
             _eventHub = eventHub;
             _logger = logger;
             _bookmarkService = bookmarkService;
+            _accountService = accountService;
         }
 
         /// <summary>
@@ -109,7 +112,7 @@ namespace API.Controllers
         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)

API/Controllers/OPDSController.cs

@@ -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;
@@ -65,7 +66,8 @@ 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;
@@ -73,6 +75,7 @@ public class OpdsController : BaseApiController
         _cacheService = cacheService;
         _readerService = readerService;
         _seriesService = seriesService;
+        _accountService = accountService;
 
         _xmlSerializer = new XmlSerializer(typeof(Feed));
         _xmlOpenSearchSerializer = new XmlSerializer(typeof(OpenSearchDescription));
@@ -619,7 +622,7 @@ public class OpdsController : BaseApiController
         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");
         }

API/Controllers/ReaderController.cs

@@ -29,17 +29,20 @@ namespace API.Controllers
         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>
@@ -657,8 +660,13 @@ namespace API.Controllers
         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);
@@ -681,8 +689,12 @@ namespace API.Controllers
         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));

API/Controllers/TachiyomiController.cs

@@ -50,7 +50,7 @@ public class TachiyomiController : BaseApiController
         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;

API/Services/AccountService.cs

@@ -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;
@@ -17,6 +18,8 @@ namespace API.Services
         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
@@ -92,5 +95,28 @@ namespace API.Services
                 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);
+        }
+
     }
 }

API/Services/BookmarkService.cs

@@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
+using API.Constants;
 using API.Data;
 using API.DTOs.Reader;
 using API.Entities;

API/Services/DownloadService.cs

@@ -3,7 +3,6 @@ using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
-using API.Constants;
 using API.Entities;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.AspNetCore.StaticFiles;
@@ -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)
+    public DownloadService() { }
-    {
-        _userManager = userManager;
-    }
 
     /// <summary>
     /// Downloads the first file in the file enumerable for download
@@ -62,9 +56,5 @@ public class DownloadService : IDownloadService
         return contentType;
     }
 
-    public async Task<bool> HasDownloadPermission(AppUser user)
+
-    {
-        var roles = await _userManager.GetRolesAsync(user);
-        return roles.Contains(PolicyConstants.DownloadRole) || roles.Contains(PolicyConstants.AdminRole);
-    }
 }

Kavita.Common/Kavita.Common.csproj

@@ -6,6 +6,7 @@
         <Product>Kavita</Product>
         <AssemblyVersion>0.5.6.1</AssemblyVersion>
         <NeutralLanguage>en</NeutralLanguage>
+        <TieredPGO>true</TieredPGO>
     </PropertyGroup>
 
     <ItemGroup>

UI/Web/src/app/_services/account.service.ts

@@ -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');
   }

UI/Web/src/app/admin/edit-user/edit-user.component.html

@@ -22,7 +22,7 @@
             <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

UI/Web/src/app/manga-reader/manga-reader.component.html

@@ -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 && hasBookmarkRights" class="btn btn-icon btn-small" role="checkbox" [attr.aria-checked]="CurrentPageBookmarked"
-                <button *ngIf="!bookmarkMode" 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>

UI/Web/src/app/manga-reader/manga-reader.component.ts

@@ -122,6 +122,7 @@ export class MangaReaderComponent implements OnInit, AfterViewInit, OnDestroy {
   layoutModes = layoutModes;
 
   isLoading = true;
+  hasBookmarkRights: boolean = false;
 
   private ctx!: CanvasRenderingContext2D;
   /**
@@ -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.
@@ -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.
    */
@@ -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;