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
feat(server, web): API and UI to replace asset data without changing record id #8480
base: main
Are you sure you want to change the base?
Conversation
This adds a new api:
It accepts the following properties, declared in a new
If an existing asset has NoteNothing is deleted in any case. A new asset is created as a backup using the original photo data, Related artifacts such as: stack information, faces, smart search data, favorite status, etc are not modified by the replace action. This information is also NOT copied to the backup asset. This means you can not perfectly undo this action if you "restore" the backup copy. The backup is really just a safety measure. You'd be better off downloading the backup copy, and replacing the original again. Job behavior changes:A new enum value was added to the The Jobs execution pattern for a new asset is as follows: An update of the existing record will perform exactly the same steps, so that the existing record has new thumbnails, faces, indexes, etc.
For the backup (the clone), since its being trashed, we can skip some of these steps, so for the cloned asset, it looks like this:
UI changes:A new option "Replace photo" was added to menu on asset-viewer-nav-bar The file uploader was modified to accept a new parameter, A cache-buster was added to all thumbnail, video-thumbnail urls, which uses the checksum of the image as query param. When the client receives and event, it will regen the url with a new query param - this causes the browser to treat this like a new image, and bypass the cache. In photo-viewer and video-viewer, it doesn't use QuestionsI have noticed the |
The cachebuster might fix #6955 |
The key query param is used for shared link authentication. So routes that require authentication even when you are not logged in. |
const _getExistingAsset = { | ||
..._getAsset_1(), | ||
duration: null, | ||
type: AssetType.IMAGE, | ||
checksum: Buffer.from('_getExistingAsset', 'utf8'), | ||
libraryId: 'libraryId', | ||
}; | ||
const _getExistingAssetWithSideCar = { | ||
..._getExistingAsset, | ||
sidecarPath: 'sidecar-path', | ||
checksum: Buffer.from('_getExistingAssetWithSideCar', 'utf8'), | ||
}; | ||
const _getClonedAsset = { | ||
id: 'cloned-copy', | ||
originalPath: 'cloned-path', | ||
}; | ||
const _getExistingLivePhotoMotionAsset = { | ||
...assetStub.livePhotoMotionAsset, | ||
checksum: Buffer.from('_getExistingLivePhotoMotionAsset', 'utf8'), | ||
libraryId: 'libraryId', | ||
}; | ||
const _getExistingLivePhotoStillAsset = { | ||
..._getExistingAsset, | ||
livePhotoVideoId: _getExistingLivePhotoMotionAsset.id, | ||
}; | ||
const _getClonedLivePhotoMotionAsset = { | ||
id: 'cloned-livephotomotion', | ||
originalPath: 'cloned-livephotomotion-path', | ||
checksum: Buffer.from('cloned-livephotomotion', 'utf8'), | ||
}; |
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.
Since those aren't methods I find _get
misleading here. IMO it's just the existingAsset
for instance
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.
Can we rename these to just cloneLivePhotoMotionAsset
, etc.?
} | ||
interface FileUploadWithAlbum extends FileUploadParams { | ||
albumId?: string; | ||
assetId?: never; |
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.
Why is that type never
?
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 this makes it possible to access item.assetId when it is this type of above without needing to cast it.
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.
Code looks good. A bunch of it is just stuff moving around, so it wasn't actually that bad. Can you resolve the conflicts and then add some e2e tests for the new methods in the asset media controller?
@alextran1502 - do you mind helping me test this PR after the conflicts with main are resolved?
A new API to update an existing asset:
PUT /api/assets/:id/upload
The asset will continue to have the same
id
and anyalbum
memberships. Only the asset data will be updated. The existing asset data will be copied into a new asset (with a new id) and immediately put into the trash. Reused the same parameters as POST /api/asset that pertain to the asset binary itself.This is to support the Lightroom workflow using the Immich Publisher plugin: (https://github.com/midzelis/mi.Immich.Publisher) Lightroom is a non-destructive editor. If you make edits to an image that has been published to immich, lightroom will attempt to send the new updates to the destination. Immich didn't support updating existing pictures until now.
Since the asset thumbnails are served with a very long cache, updating the asset in this way needs a way to tell the client to ignore the cache. Easiest way to do this is with a cache-buster query param. This optional parameter is set to the asset checksum, which changes when the asset it updated.
Algorithm:
Retrieve the existing asset
Retrieve the existing livePhotoAsset (if present)
Update or Create the existing livePhotoAsset with new livePhotoAsset (if present)
Update the existing asset with uploaded file path, and new livePhotoAsset (if present)
"Clone" the existing asset (copy all properties to a new asset, but with a new
id
), run the standard jobs, except don't inform client of a new successful upload, and then immediately trash it.