ShokoAnime / ShokoServer

Repository for Shoko Server.
http://shokoanime.com/shoko-server/
MIT License
374 stars 74 forks source link

Feature regression relating to the OnFileMatched event #1115

Closed fearnlj01 closed 2 months ago

fearnlj01 commented 3 months ago

Current Version

When Shoko first 'discovers' an anime, the OnFileMatched event fired has an empty collection for EpisodeInfo (& therefore also for AnimeInfo & GroupInfo).

Whilst this is arguably the correct behaviour, this information has previously been available at this point. (With January's builds being the last time I could say this for certain).

I'm also not sure as to if this is solely limited to just when Shoko first discovers an anime, however I would assume that once Shoko has gotten episode info for a series, this would prevent an XRef not matching an actual episode in future matched files.

Disclaimer

I do acknowledge this is more of a "me" issue than a universal feature regression...

I assume that I could just work around this for my plugin by listening for the OnFileMoved event as well, at which point I would suspect we'd have to have the episode info, and matching that event's VideoLocal_Place up against what I'm currently tracking, however this would then only work for setups where file moving is enabled on import.

Background

Historically, my setup has had no issues when using the OnFileMatched event that's fired. Even when it is the first time Shoko has 'discovered' anything from the series, the anime/episode information has been populated as expected by the time this event was fired.

This changed after updating my plugin to be compatible with the changes introduced in 3be0802... Being lazy, I ignored the fact that it was possible for the FileEventArg's AnimeInfo & EpisodeInfo fields to be empty, which ended up causing a fatal crash when trying to access a non-existent index...

Before updating my plugin, simply grabbing the FirstOrDefault value from the aforementioned fields has always been 'good enough to function as expected. Just as a sanity check, I've gone back to January and looked through the historical webhooks sent by my setup, and I can see evidence that the OnFileMatched event must have contain this information on episode 1's that were matched at the time.

Reproduction Steps

Logfile

Attached should be two trace log files, split into two parts (as the file needed adding to AniDB) to show what happens when the reproduction steps are followed. 00-ImportingUnrecognised.txt 01-ImportingRecognised.txt

revam commented 3 months ago

This is likely related to the issues with importing anime in the new queue. The cross-references are already made when it's matched, and the episodes should be available at this point.

@da3dsoul can correct me if I'm wrong.

da3dsoul commented 3 months ago

It's possible. I cannot confirm or deny without experimenting

fearnlj01 commented 2 months ago

Just quickly confirming... I continue to get this behaviour in the most recent build, 6f910d9.

A trimmed log is available as per the below (with the errors thrown by my own plugin being initially triggered by the OnFileMatched event). example.log

revam commented 2 months ago

@fearnlj01 starting in ecc1917 you can use the IReadOnlyList<IVideoCrossReference> CrossReferences on the IVideo to check if the cross-references has been made, and also get the episode ids to look out for in the episode updated event (when filtering the UpdateReason Reason to UpdateReason.Added or UpdateReason.Updated on the event), if the episodes are empty on the file matched event data. I don't think we can guarantee that the episode/anime metadata is available when the matched event is dispatched, but now you can at least react to it.

Alternatively since IReadOnlyList<IVideoCrossReference> CrossReferences has also been added to the IEpisode interface then you can store the video ED2K hashes that didn't have the episode events, then later in the episode updated event look for them to "resume" what you were going to do once the cross-references were available.

Either way, it should be even easier to handle when the cross-references are made but before the episodes are added now compared to before.

fearnlj01 commented 2 months ago

I don't think we can guarantee that the episode/anime metadata is available when the matched event is dispatched

In hindsight, it's more surprising that this information was seemingly always available given when you would logically expect a file match event to be fired.

The alterations as you've described should be more than enough to suit my needs.

Unless there were any plans on deferring the OnFileMatched event to a point in time after cross references are acquired (which wouldn't really make sense...) then I can't see any reason that this issue would remain open, so I'll close this as a happy resolution.

Thanks for the changes!