-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(scanner): Always refresh folder image time when adding first image #3764
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(scanner): Always refresh folder image time when adding first image #3764
Conversation
Currently, the `images_updated_at` field is only set to the image modification time. However, in cases where a new image is added _and_ said image is older than the folder mod time, the field is not updated properly. In this the case where `images_updated_at` is null (no images were ever added) and a new images is found, use the folder modification time instead of image modification time. **Note**, this doesn't handle cases such as replacing a newer image with an older one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the proper solution for this woyld be to store in the folder
a hash of all image names and times found in the folder, instead of the most recent image, and use the hash (images_hash?) in reader_artist
instead of the images_last_updated_at
. This would cause any changes in the folder's images to invalidate the cache.
What do you think?
Conceptually it makes sense, but not sure how you would reconcile the external readers requiring date. Not something I would probably be comfortable doing at least. |
Yeah, it requires more work. I'll think about this when implementing the new |
…lder Signed-off-by: Deluan <deluan@navidrome.org>
@kgarner7 do you agree with my commit? Can you try it in your env to be sure it is working as you expect? |
|
Yeah, I didn't want to check if the times array is empty, IMO makes the change less readable, but I can change that.
If the image is not the newest, then yes. But if it is the newest, the |
If you go from one image to no images, then because of the default time being 0, the item won't be invalidated. Maybe that's not an expected use case? |
It will set navidrome/core/artwork/reader_artist.go Lines 46 to 68 in c795bcf
|
Consider the following case in https://github.com/navidrome/navidrome/blob/c795bcfcf7471c244b0735e990fe8ccd0252d0c8/core/artwork/reader_album.go#L47C1-L51C3
I'm less convinced this is possible for artist, but it seems possible for album. |
Currently, the
images_updated_at
field is only set to the image modification time. However, in cases where a new image is added and said image is older than the folder mod time, the field is not updated properly.In this the case where
images_updated_at
is null (no images were ever added) and a new images is found, use the folder modification time instead of image modification time.Note, this doesn't handle cases such as replacing a newer image with an older one.