Misc Bugfixes (#1015)

* Fixed some security issues in dev env

* When deleting folders in bookmark cleanup, delete empty folders correctly.

* When a new library is created and cards are added, cards can have a blank library name. Card library name code is reworked to be much lighter on memory.

* Added a config for github issues to disable blank issues.

* Skip any sort of directory iteration code if we haven't deleted any bookmarks.

* Fixed a bug where some style overrides were duplicating. Now logic is much more targetted, only applying to the correct tags.

* Applied sorting to the filtering apis.

* Reverted one of my changes for a better version Robbie did.

API.Tests/Services/CleanupServiceTests.cs

@@ -400,6 +400,78 @@ public class CleanupServiceTests
 
         await _context.SaveChangesAsync();
 
+        _context.AppUser.Add(new AppUser()
+        {
+            Bookmarks = new List<AppUserBookmark>()
+            {
+                new AppUserBookmark()
+                {
+                    AppUserId = 1,
+                    ChapterId = 1,
+                    Page = 1,
+                    FileName = "1/1/1/0001.jpg",
+                    SeriesId = 1,
+                    VolumeId = 1
+                },
+                new AppUserBookmark()
+                {
+                    AppUserId = 1,
+                    ChapterId = 1,
+                    Page = 2,
+                    FileName = "1/1/1/0002.jpg",
+                    SeriesId = 1,
+                    VolumeId = 1
+                }
+            }
+        });
+
+        await _context.SaveChangesAsync();
+
+
+        var ds = new DirectoryService(Substitute.For<ILogger<DirectoryService>>(), filesystem);
+        var cleanupService = new CleanupService(_logger, _unitOfWork, _messageHub,
+            ds);
+
+        await cleanupService.CleanupBookmarks();
+
+        Assert.Equal(2, ds.GetFiles(BookmarkDirectory, searchOption:SearchOption.AllDirectories).Count());
+
+    }
+
+    [Fact]
+    public async Task CleanupBookmarks_LeavesOneFiles()
+    {
+        var filesystem = CreateFileSystem();
+        filesystem.AddFile($"{BookmarkDirectory}1/1/1/0001.jpg", new MockFileData(""));
+        filesystem.AddFile($"{BookmarkDirectory}1/1/2/0002.jpg", new MockFileData(""));
+
+        // Delete all Series to reset state
+        await ResetDB();
+
+        _context.Series.Add(new Series()
+        {
+            Name = "Test",
+            Library = new Library() {
+                Name = "Test LIb",
+                Type = LibraryType.Manga,
+            },
+            Volumes = new List<Volume>()
+            {
+                new Volume()
+                {
+                    Chapters = new List<Chapter>()
+                    {
+                        new Chapter()
+                        {
+
+                        }
+                    }
+                }
+            }
+        });
+
+        await _context.SaveChangesAsync();
+
         _context.AppUser.Add(new AppUser()
         {
             Bookmarks = new List<AppUserBookmark>()
@@ -426,7 +498,7 @@ public class CleanupServiceTests
         await cleanupService.CleanupBookmarks();
 
         Assert.Equal(1, ds.GetFiles(BookmarkDirectory, searchOption:SearchOption.AllDirectories).Count());
-
+        Assert.Equal(1, ds.FileSystem.Directory.GetDirectories($"{BookmarkDirectory}1/1/").Length);
     }
 
     #endregion

API/Controllers/MetadataController.cs

@@ -111,7 +111,7 @@ public class MetadataController : BaseApiController
         {
             Title = t.ToDescription(),
             Value = t
-        }));
+        }).OrderBy(t => t.Title));
     }
 
     /// <summary>

API/Data/Repositories/GenreRepository.cs

@@ -67,6 +67,7 @@ public class GenreRepository : IGenreRepository
             .Where(s => libraryIds.Contains(s.LibraryId))
             .SelectMany(s => s.Metadata.Genres)
             .Distinct()
+            .OrderBy(p => p.Title)
             .ProjectTo<GenreTagDto>(_mapper.ConfigurationProvider)
             .ToListAsync();
     }

API/Data/Repositories/PersonRepository.cs

@@ -66,6 +66,8 @@ public class PersonRepository : IPersonRepository
             .Where(s => libraryIds.Contains(s.LibraryId))
             .SelectMany(s => s.Metadata.People)
             .Distinct()
+            .OrderBy(p => p.Name)
+            .AsNoTracking()
             .ProjectTo<PersonDto>(_mapper.ConfigurationProvider)
             .ToListAsync();
     }
@@ -74,6 +76,7 @@ public class PersonRepository : IPersonRepository
     public async Task<IList<Person>> GetAllPeople()
     {
         return await _context.Person
+            .OrderBy(p => p.Name)
             .ToListAsync();
     }
 }

API/Data/Repositories/SeriesRepository.cs

@@ -778,6 +778,7 @@ public class SeriesRepository : ISeriesRepository
         var ret = await _context.Series
             .Where(s => libraryIds.Contains(s.LibraryId))
             .Select(s => s.Metadata.Language)
+            .AsNoTracking()
             .Distinct()
             .ToListAsync();
 
@@ -787,7 +788,9 @@ public class SeriesRepository : ISeriesRepository
             {
                 Title = CultureInfo.GetCultureInfo(s).DisplayName,
                 IsoCode = s
-            }).ToList();
+            })
+            .OrderBy(s => s.Title)
+            .ToList();
     }
 
     public async Task<IList<PublicationStatusDto>> GetAllPublicationStatusesDtosForLibrariesAsync(List<int> libraryIds)
@@ -801,6 +804,7 @@ public class SeriesRepository : ISeriesRepository
                 Value = s,
                 Title = s.ToDescription()
             })
+            .OrderBy(s => s.Title)
             .ToListAsync();
     }
 

API/Data/Repositories/TagRepository.cs

@@ -67,6 +67,8 @@ public class TagRepository : ITagRepository
             .Where(s => libraryIds.Contains(s.LibraryId))
             .SelectMany(s => s.Metadata.Tags)
             .Distinct()
+            .OrderBy(t => t.Title)
+            .AsNoTracking()
             .ProjectTo<TagDto>(_mapper.ConfigurationProvider)
             .ToListAsync();
     }
@@ -80,6 +82,7 @@ public class TagRepository : ITagRepository
     {
         return await _context.Tag
             .AsNoTracking()
+            .OrderBy(t => t.Title)
             .ProjectTo<TagDto>(_mapper.ConfigurationProvider)
             .ToListAsync();
     }

API/Services/Tasks/CleanupService.cs

@@ -184,19 +184,23 @@ namespace API.Services.Tasks
 
 
             var filesToDelete = allBookmarkFiles.ToList().Except(bookmarks).ToList();
-            _logger.LogDebug("[Bookmarks] Bookmark cleanup wants to delete {Count} files", filesToDelete.Count());
+            _logger.LogDebug("[Bookmarks] Bookmark cleanup wants to delete {Count} files", filesToDelete.Count);
+
+            if (filesToDelete.Count == 0) return;
 
             _directoryService.DeleteFiles(filesToDelete);
 
             // Clear all empty directories
-            foreach (var directory in _directoryService.FileSystem.Directory.GetDirectories(bookmarkDirectory))
+            foreach (var directory in _directoryService.FileSystem.Directory.GetDirectories(bookmarkDirectory, "", SearchOption.AllDirectories))
             {
-                if (_directoryService.FileSystem.Directory.GetFiles(directory).Length == 0 &&
+                if (_directoryService.FileSystem.Directory.GetFiles(directory, "", SearchOption.AllDirectories).Length == 0 &&
                     _directoryService.FileSystem.Directory.GetDirectories(directory).Length == 0)
                 {
                     _directoryService.FileSystem.Directory.Delete(directory, false);
                 }
             }
+
+
         }
     }
 }

UI/Web/package-lock.json

@@ -10645,9 +10645,33 @@
       "dev": true
     },
     "node-fetch": {
-      "version": "2.6.1",
+      "version": "2.6.7",
-      "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz",
+      "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.7.tgz",
-      "integrity": "sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw=="
+      "integrity": "sha512-ZjMPFEfVx5j+y2yF35Kzx5sF7kDzxuDj6ziH4FFbOp87zKDZNx8yExJIb05OGF4Nlt9IHFIMBkRl41VdvcNdbQ==",
+      "requires": {
+        "whatwg-url": "^5.0.0"
+      },
+      "dependencies": {
+        "tr46": {
+          "version": "0.0.3",
+          "resolved": "https://registry.npmjs.org/tr46/-/tr46-0.0.3.tgz",
+          "integrity": "sha1-gYT9NH2snNwYWZLzpmIuFLnZq2o="
+        },
+        "webidl-conversions": {
+          "version": "3.0.1",
+          "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz",
+          "integrity": "sha1-JFNCdeKnvGvnvIZhHMFq4KVlSHE="
+        },
+        "whatwg-url": {
+          "version": "5.0.0",
+          "resolved": "https://registry.npmjs.org/whatwg-url/-/whatwg-url-5.0.0.tgz",
+          "integrity": "sha1-lmRU6HZUYuN2RNNib2dCzotwll0=",
+          "requires": {
+            "tr46": "~0.0.3",
+            "webidl-conversions": "^3.0.0"
+          }
+        }
+      }
     },
     "node-forge": {
       "version": "0.10.0",
@@ -12384,8 +12408,7 @@
           "dependencies": {
             "ansi-regex": {
               "version": "5.0.0",
-              "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.0.tgz",
+              "resolved": "",
-              "integrity": "sha512-bY6fj56OUQ0hU1KjFNDQuJFezqKdrAyFdIevADiqrWHwSlbmBNMHp5ak2f40Pm8JTFyM2mqxkG6ngkHO11f/lg==",
               "dev": true
             },
             "strip-ansi": {
@@ -12590,8 +12613,7 @@
           "dependencies": {
             "ansi-regex": {
               "version": "5.0.0",
-              "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.0.tgz",
+              "resolved": "",
-              "integrity": "sha512-bY6fj56OUQ0hU1KjFNDQuJFezqKdrAyFdIevADiqrWHwSlbmBNMHp5ak2f40Pm8JTFyM2mqxkG6ngkHO11f/lg==",
               "dev": true
             },
             "ansi-styles": {

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

@@ -34,6 +34,21 @@ export class LibraryService {
     }));
   }
 
+  getLibraryName(libraryId: number) {
+    if (this.libraryNames != undefined && this.libraryNames.hasOwnProperty(libraryId)) {
+      return of(this.libraryNames[libraryId]);
+    }
+    return this.httpClient.get<Library[]>(this.baseUrl + 'library').pipe(map(l => {
+      this.libraryNames = {};
+      l.forEach(lib => {
+        if (this.libraryNames !== undefined) {
+          this.libraryNames[lib.id] = lib.name;
+        }        
+      });
+      return this.libraryNames[libraryId];
+    }));
+  }
+
   listDirectories(rootPath: string) {
     let query = '';
     if (rootPath !== undefined && rootPath.length > 0) {

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

@@ -43,6 +43,15 @@ const TOP_OFFSET = -50 * 1.5; // px the sticky header takes up
 const CHAPTER_ID_NOT_FETCHED = -2;
 const CHAPTER_ID_DOESNT_EXIST = -1;
 
+/**
+ * Styles that should be applied on the top level book-content tag
+ */
+const pageLevelStyles = ['margin-left', 'margin-right', 'font-size'];
+/**
+ * Styles that should be applied on every element within book-content tag
+ */
+const elementLevelStyles = ['line-height', 'font-family'];
+
 @Component({
   selector: 'app-book-reader',
   templateUrl: './book-reader.component.html',
@@ -680,17 +689,10 @@ export class BookReaderComponent implements OnInit, AfterViewInit, OnDestroy {
       page = 0;
     }
 
-    // BUG: Last page is not counting as read
     if (!(page === 0 || page === this.maxPages - 1)) {
       page -= 1;
     }
 
-    // // Due to the fact that we start at image 0, but page 1, we need the last page to have progress as page + 1 to be completed
-    // let tempPageNum = this.pageNum;
-    // if (this.pageNum == this.maxPages - 1) {
-    //   tempPageNum = this.pageNum + 1;
-    // }
-
     this.pageNum = page;
     this.loadPage();
 
@@ -903,31 +905,41 @@ export class BookReaderComponent implements OnInit, AfterViewInit, OnDestroy {
     this.updateReaderStyles();
   }
 
+  /**
+   * Applies styles onto the html of the book page
+   */
   updateReaderStyles() {
-    if (this.readingHtml != undefined && this.readingHtml.nativeElement) {
+    if (this.readingHtml === undefined || !this.readingHtml.nativeElement)  return;
-      Object.entries(this.pageStyles).forEach(item => {
-        if (item[1] == '100%' || item[1] == '0px' || item[1] == 'inherit') {
-          // Remove the style or skip
-          this.renderer.removeStyle(this.readingHtml.nativeElement, item[0]);
-          return;
-        }
-        this.renderer.setStyle(this.readingHtml.nativeElement, item[0], item[1], RendererStyleFlags2.Important);
-      });
 
-      for(let i = 0; i < this.readingHtml.nativeElement.children.length; i++) {
+    // Line Height must be placed on each element in the page
-        const elem = this.readingHtml.nativeElement.children.item(i);
+    
-        if (elem?.tagName === 'STYLE') continue;
+    // Apply page level overrides
-          Object.entries(this.pageStyles).forEach(item => {
+    Object.entries(this.pageStyles).forEach(item => {
-            if (item[1] == '100%' || item[1] == '0px' || item[1] == 'inherit') {
+      if (item[1] == '100%' || item[1] == '0px' || item[1] == 'inherit') {
-              // Remove the style or skip
+        // Remove the style or skip
-              this.renderer.removeStyle(elem, item[0]);
+        this.renderer.removeStyle(this.readingHtml.nativeElement, item[0]);
-              return;
+        return;
-            }
-            this.renderer.setStyle(elem, item[0], item[1], RendererStyleFlags2.Important);
-          });
-        
       }
+      if (pageLevelStyles.includes(item[0])) {
+        this.renderer.setStyle(this.readingHtml.nativeElement, item[0], item[1], RendererStyleFlags2.Important);
+      }
+    });
+
+    const individualElementStyles = Object.entries(this.pageStyles).filter(item => elementLevelStyles.includes(item[0]));
+    for(let i = 0; i < this.readingHtml.nativeElement.children.length; i++) {
+      const elem = this.readingHtml.nativeElement.children.item(i);
+      if (elem?.tagName === 'STYLE') continue;
+      individualElementStyles.forEach(item => {
+          if (item[1] == '100%' || item[1] == '0px' || item[1] == 'inherit') {
+            // Remove the style or skip
+            this.renderer.removeStyle(elem, item[0]);
+            return;
+          }
+          this.renderer.setStyle(elem, item[0], item[1], RendererStyleFlags2.Important);
+        });
+      
     }
+
   }
 
 

UI/Web/src/app/cards/card-item/card-item.component.ts

@@ -115,12 +115,15 @@ export class CardItemComponent implements OnInit, OnDestroy {
     }
 
     if (this.supressLibraryLink === false) {
-      this.libraryService.getLibraryNames().pipe(takeUntil(this.onDestroy)).subscribe(names => {
+      if (this.entity !== undefined && this.entity.hasOwnProperty('libraryId')) {
-        if (this.entity !== undefined && this.entity.hasOwnProperty('libraryId')) {
+        this.libraryId = (this.entity as Series).libraryId;
-          this.libraryId = (this.entity as Series).libraryId;
+      }
-          this.libraryName = names[this.libraryId];
+
-        }
+      if (this.libraryId !== undefined && this.libraryId > 0) {
-      });
+        this.libraryService.getLibraryName(this.libraryId).pipe(takeUntil(this.onDestroy)).subscribe(name => {
+          this.libraryName = name;
+        });
+      }
     }
     this.format = (this.entity as Series).format;