Skip to content

Commit

Permalink
More Scan Loop Fixes (#1473)
Browse files Browse the repository at this point in the history
* Added a ToList() to avoid a bug where a person could be removed from a list while iterating over the list.

* When deleting a series, want to read page will now automatically remove that series from the view.

* Fixed a series lookup which was ignoring format

* Ignore XML comment warnings

* Removed a note since it was already working that way

* Fixed unit test
  • Loading branch information
majora2007 authored Aug 25, 2022
1 parent e37d7cf commit f92ef19
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 19 deletions.
10 changes: 7 additions & 3 deletions API.Tests/Repository/SeriesRepositoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,23 +125,27 @@ private async Task SetupSeriesData()
}
};

var s = DbFactory.Series("The Idaten Deities Know Only Peace", "Heion Sedai no Idaten-tachi");
s.Format = MangaFormat.Archive;

library.Series = new List<Series>()
{
DbFactory.Series("The Idaten Deities Know Only Peace", "Heion Sedai no Idaten-tachi"),
s,
};

_unitOfWork.LibraryRepository.Add(library);
await _unitOfWork.CommitAsync();
}


[InlineData("Heion Sedai no Idaten-tachi", "", "The Idaten Deities Know Only Peace")] // Matching on localized name in DB
[InlineData("Heion Sedai no Idaten-tachi", "", MangaFormat.Archive, "The Idaten Deities Know Only Peace")] // Matching on localized name in DB
[InlineData("Heion Sedai no Idaten-tachi", "", MangaFormat.Pdf, null)]
public async Task GetFullSeriesByAnyName_Should(string seriesName, string localizedName, string? expected)
{
var firstSeries = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(1);
var series =
await _unitOfWork.SeriesRepository.GetFullSeriesByAnyName(seriesName, localizedName,
1);
1, MangaFormat.Unknown);
if (expected == null)
{
Assert.Null(series);
Expand Down
6 changes: 6 additions & 0 deletions API/API.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
<NoWarn>1701;1702;1591</NoWarn>
</PropertyGroup>

<!-- Ignore XML comments -->
<PropertyGroup>
<GenerateDocumentationFile>True</GenerateDocumentationFile>
<NoWarn>$(NoWarn);1591</NoWarn>
</PropertyGroup>

<PropertyGroup>
<SatelliteResourceLanguages>en</SatelliteResourceLanguages>
</PropertyGroup>
Expand Down
5 changes: 3 additions & 2 deletions API/Data/Repositories/SeriesRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public interface ISeriesRepository
Task<int> GetSeriesIdByFolder(string folder);
Task<Series> GetSeriesByFolderPath(string folder);
Task<Series> GetFullSeriesByName(string series, int libraryId);
Task<Series> GetFullSeriesByAnyName(string seriesName, string localizedName, int libraryId);
Task<Series> GetFullSeriesByAnyName(string seriesName, string localizedName, int libraryId, MangaFormat format);
Task RemoveSeriesNotInList(IList<ParsedSeries> seenSeries, int libraryId);
Task<IDictionary<string, IList<SeriesModified>>> GetFolderPathMap(int libraryId);
}
Expand Down Expand Up @@ -1218,12 +1218,13 @@ public Task<Series> GetFullSeriesByName(string series, int libraryId)
/// <param name="localizedName"></param>
/// <param name="libraryId"></param>
/// <returns></returns>
public Task<Series> GetFullSeriesByAnyName(string seriesName, string localizedName, int libraryId)
public Task<Series> GetFullSeriesByAnyName(string seriesName, string localizedName, int libraryId, MangaFormat format)
{
var normalizedSeries = Parser.Parser.Normalize(seriesName);
var normalizedLocalized = Parser.Parser.Normalize(localizedName);
var query = _context.Series
.Where(s => s.LibraryId == libraryId)
.Where(s => s.Format == format && format != MangaFormat.Unknown)
.Where(s => s.NormalizedName.Equals(normalizedSeries)
|| (s.NormalizedLocalizedName.Equals(normalizedSeries) && s.NormalizedLocalizedName != string.Empty));
if (!string.IsNullOrEmpty(normalizedLocalized))
Expand Down
3 changes: 2 additions & 1 deletion API/Helpers/PersonHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ public static void KeepOnlySamePeopleBetweenLists(IEnumerable<Person> existingPe
{
foreach (var person in existingPeople)
{
var existingPerson = removeAllExcept.FirstOrDefault(p => p.Role == person.Role && person.NormalizedName.Equals(p.NormalizedName));
var existingPerson = removeAllExcept
.FirstOrDefault(p => p.Role == person.Role && person.NormalizedName.Equals(p.NormalizedName));
if (existingPerson == null)
{
action?.Invoke(person);
Expand Down
4 changes: 2 additions & 2 deletions API/Services/Tasks/Scanner/ProcessSeries.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ await _eventHub.SendMessageAsync(MessageFactory.NotificationProgress,
{
series =
await _unitOfWork.SeriesRepository.GetFullSeriesByAnyName(firstInfo.Series, firstInfo.LocalizedSeries,
library.Id);
library.Id, firstInfo.Format);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -375,7 +375,7 @@ private static void UpdateSeriesMetadata(Series series, LibraryType libraryType)
// NOTE: The issue here is that people is just from chapter, but series metadata might already have some people on it
// I might be able to filter out people that are in locked fields?
var people = chapters.SelectMany(c => c.People).ToList();
PersonHelper.KeepOnlySamePeopleBetweenLists(series.Metadata.People,
PersonHelper.KeepOnlySamePeopleBetweenLists(series.Metadata.People.ToList(),
people, person =>
{
switch (person.Role)
Expand Down
5 changes: 1 addition & 4 deletions API/Services/Tasks/ScannerService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ await _eventHub.SendMessageAsync(MessageFactory.Info,
var shouldUseLibraryScan = !(await _unitOfWork.LibraryRepository.DoAnySeriesFoldersMatch(libraryFolderPaths));
if (!shouldUseLibraryScan)
{
_logger.LogInformation("Library {LibraryName} consists of one ore more Series folders, using series scan", library.Name);
_logger.LogError("Library {LibraryName} consists of one or more Series folders, using series scan", library.Name);
}


Expand All @@ -459,8 +459,6 @@ void TrackFiles(Tuple<bool, IList<ParserInfo>> parsedInfo)
Format = parsedFiles.First().Format
};

// NOTE: Could we check if there are multiple found series (different series) and process each one?

if (skippedScan)
{
seenSeries.AddRange(parsedFiles.Select(pf => new ParsedSeries()
Expand All @@ -485,7 +483,6 @@ void TrackFiles(Tuple<bool, IList<ParserInfo>> parsedInfo)

await Task.WhenAll(processTasks);

//await _eventHub.SendMessageAsync(MessageFactory.NotificationProgress, MessageFactory.LibraryScanProgressEvent(library.Name, ProgressEventType.Ended, string.Empty));
await _eventHub.SendMessageAsync(MessageFactory.NotificationProgress, MessageFactory.FileScanProgressEvent(string.Empty, library.Name, ProgressEventType.Ended));

_logger.LogInformation("[ScannerService] Finished file scan in {ScanAndUpdateTime}. Updating database", scanElapsedTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ <h6 subtitle>{{seriesPagination.totalItems}} Series</h6>
[filterOpen]="filterOpen"
[jumpBarKeys]="jumpbarKeys"
[trackByIdentity]="trackByIdentity"
[refresh]="refresh"
(applyFilter)="updateFilter($event)">
<ng-template #cardItem let-item let-position="idx">
<app-series-card [data]="item" [libraryId]="item.libraryId" (reload)="loadPage()"
<app-series-card [data]="item" [libraryId]="item.libraryId" (reload)="removeSeries(item.id)"
(selection)="bulkSelectionService.handleCardSelection('series', position, series.length, $event)" [selected]="bulkSelectionService.isCardSelected('series', position)" [allowSelection]="true"
></app-series-card>
</ng-template>
Expand Down
35 changes: 29 additions & 6 deletions UI/Web/src/app/want-to-read/want-to-read/want-to-read.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { BulkSelectionService } from 'src/app/cards/bulk-selection.service';
import { FilterSettings } from 'src/app/metadata-filter/filter-settings';
import { FilterUtilitiesService } from 'src/app/shared/_services/filter-utilities.service';
import { UtilityService, KEY_CODES } from 'src/app/shared/_services/utility.service';
import { SeriesRemovedEvent } from 'src/app/_models/events/series-removed-event';
import { JumpKey } from 'src/app/_models/jumpbar/jump-key';
import { Pagination } from 'src/app/_models/pagination';
import { Series } from 'src/app/_models/series';
Expand Down Expand Up @@ -35,6 +36,7 @@ export class WantToReadComponent implements OnInit, OnDestroy {
seriesPagination!: Pagination;
filter: SeriesFilter | undefined = undefined;
filterSettings: FilterSettings = new FilterSettings();
refresh: EventEmitter<void> = new EventEmitter();

filterActiveCheck!: SeriesFilter;
filterActive: boolean = false;
Expand All @@ -43,7 +45,7 @@ export class WantToReadComponent implements OnInit, OnDestroy {

filterOpen: EventEmitter<boolean> = new EventEmitter();

private onDestory: Subject<void> = new Subject<void>();
private onDestroy: Subject<void> = new Subject<void>();
trackByIdentity = (index: number, item: Series) => `${item.name}_${item.localizedName}_${item.pagesRead}`;

bulkActionCallback = (action: Action, data: any) => {
Expand Down Expand Up @@ -77,20 +79,34 @@ export class WantToReadComponent implements OnInit, OnDestroy {
private seriesService: SeriesService, private titleService: Title,
public bulkSelectionService: BulkSelectionService, private actionService: ActionService, private messageHub: MessageHubService,
private filterUtilityService: FilterUtilitiesService, private utilityService: UtilityService, @Inject(DOCUMENT) private document: Document,
private readonly cdRef: ChangeDetectorRef, private scrollService: ScrollService) {
private readonly cdRef: ChangeDetectorRef, private scrollService: ScrollService, private hubService: MessageHubService) {
this.router.routeReuseStrategy.shouldReuseRoute = () => false;
this.titleService.setTitle('Want To Read');

this.seriesPagination = this.filterUtilityService.pagination(this.route.snapshot);
[this.filterSettings.presets, this.filterSettings.openByDefault] = this.filterUtilityService.filterPresetsFromUrl(this.route.snapshot);
this.filterActiveCheck = this.seriesService.createSeriesFilter();
this.cdRef.markForCheck();

this.hubService.messages$.pipe(takeUntil(this.onDestroy)).subscribe((event) => {
if (event.event === EVENTS.SeriesRemoved) {
const seriesRemoved = event.payload as SeriesRemovedEvent;
if (!this.utilityService.deepEqual(this.filter, this.filterActiveCheck)) {
this.loadPage();
return;
}

this.series = this.series.filter(s => s.id != seriesRemoved.seriesId);
this.seriesPagination.totalItems--;
this.cdRef.markForCheck();
this.refresh.emit();
}
});

}

ngOnInit(): void {

this.messageHub.messages$.pipe(takeUntil(this.onDestory), debounceTime(2000)).subscribe(event => {
this.messageHub.messages$.pipe(takeUntil(this.onDestroy), debounceTime(2000)).subscribe(event => {
if (event.event === EVENTS.SeriesRemoved) {
this.loadPage();
}
Expand All @@ -102,8 +118,8 @@ export class WantToReadComponent implements OnInit, OnDestroy {
}

ngOnDestroy() {
this.onDestory.next();
this.onDestory.complete();
this.onDestroy.next();
this.onDestroy.complete();
}

@HostListener('document:keydown.shift', ['$event'])
Expand All @@ -120,6 +136,13 @@ export class WantToReadComponent implements OnInit, OnDestroy {
}
}

removeSeries(seriesId: number) {
this.series = this.series.filter(s => s.id != seriesId);
this.seriesPagination.totalItems--;
this.cdRef.markForCheck();
this.refresh.emit();
}

loadPage() {
this.filterActive = !this.utilityService.deepEqual(this.filter, this.filterActiveCheck);
this.isLoading = true;
Expand Down

0 comments on commit f92ef19

Please sign in to comment.