-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
fix(server): don't reimport files more than once #16375
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
Conversation
23b6e31 to
4334c2a
Compare
| localDateTime, | ||
| fileCreatedAt: exifData.dateTimeOriginal ?? undefined, | ||
| fileModifiedAt: exifData.modifyDate ?? undefined, | ||
| fileModifiedAt: asset.fileModifiedAt, |
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'd prefer if you used a fileModifiedAt variable instead of assigning to asset.
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'd prefer if you used a
fileModifiedAtvariable instead of assigning toasset.
This is much less trivial than might first seen. It must be assigned to asset in order for getDates to work, and assigning a new variable only makes it more complex.
Rewriting the metadata extraction feature is on my list as it's getting rather messy, but that's not in scope for this PR.
4334c2a to
bf53e5a
Compare
| asset.fileModifiedAt = stats.mtime; | ||
| } | ||
|
|
||
| const fileModifiedAt = asset.fileModifiedAt; |
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.
No, I mean that instead of asset.fileModifiedAt = stats.mtime; followed by const fileModifiedAt = asset.fileModifiedAt;, I'd prefer const fileModifiedAt = stats.mtime; (or inlining to fileModifiedAt: stats.mtime)
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.
There is no other example in the metadata service from what I can see where the asset is modified rather than creating a variable.
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.
Ah, I see. I inlined it as per your suggestion, let me know what you think.
bf53e5a to
5fc4318
Compare
|
Is this something we can use Medium Tests for? |
I haven't played with them enough to know yet, but it's high on my todo list to migrate as many library tests as possible to that. Let's get this fix in 1.127.1 first. |
|
Can you help fix the failed tests? |
Sure, I didn't know they failed before I left home. I'll do that in a few hours when I'm back at the computer. If you are short on time the fix should be easy, just modify the mocks and asserts a little |
* fix(server) don't reimport files more than once * fix: test --------- Co-authored-by: Alex Tran <[email protected]>
|
I think the adaption is not sufficient... I'm currently on v1.128 I've changed access rights and owner with chmod / chown. (File Inode Change Date/Time)
although nothing about the picture itself changed, each asset is being re-imported. Solution proposal |
This is by design. You changed the files, and the mtime was updated. Then it will be reimported. |
|
@etnoy @alextran1502 do you think it's worth opening a Feature Request for my solution proposal to make re-importing more efficent? Solution proposal
If a file change is being identified compare if image / video part hash has changed compared to the changed file Goal: prevent unnecessary processing. if no changes happen to content |
No, I don't think so. By design, we want to allow multiple files with identical hashes with different paths within an external library |
Whenever a library asset is changed it will be reimported during the next scan. However, the metadata extractor puts the incorrect date in fileModifiedAt, so that the file will be reimported on the next scan, and the next, and the next, no matter if it was changed or not.
This fix puts the correct date in fileModifiedAt so that this won't happen. fileCreatedAt should be derived from the file, and not exif data.
Yes, this is a bug from my previous PR #16225...hope this is correct now.