Skip to content

Commit

Permalink
A few imprpvments to the uploads (#599)
Browse files Browse the repository at this point in the history 
* Add end-to-end test for photo folder upload and implement job waiting logic

* Implement asset replacement functionality and remove redundant asset deletion logic

* Add checksum field to SearchMetadataQuery and implement GetAssetsByHash method

* Set asset ID during replacement in replaceAsset method

* Add GetAssetsByHash and GetAssetsByImageName methods to ImmichClient

* Refactor asset management logic to include asset ID in album handling

* Enhance end-to-end tests for asset upgrades and improve album verification logic

* Add test data for Google Photos upload and enhance bad request handling

* Merge branch 'simulot/issue570' into improve-asset-upgrading

* Write details of HTTP errors in the API-TRACE

* Add test for handling bad requests and improve logging in e2e tests

* Improve API trace logging by adding error check suppression for response encoding

* Disable livePhotoMatch function and update related tests to reflect changes

* Mask decimals part of latitude and longitude when logging to preserve privacy

* Log the number of assets on the server

* Add test with an alternate user
  • Loading branch information
@simulot
simulot authored Jan 4, 2025
1 parent 234b3e3 commit 84cee85
Show file tree
Hide file tree
Showing 33 changed files with 843 additions and 54 deletions.
2 changes: 1 addition & 1 deletion adapters/googlePhotos/googlephotos.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ var matchers = []struct {
fn matcherFn
}{
{name: "normalMatch", fn: normalMatch},
{name: "livePhotoMatch", fn: livePhotoMatch},
// {name: "livePhotoMatch", fn: livePhotoMatch},
{name: "matchWithOneCharOmitted", fn: matchWithOneCharOmitted},
{name: "matchVeryLongNameWithNumber", fn: matchVeryLongNameWithNumber},
{name: "matchDuplicateInYear", fn: matchDuplicateInYear},
Expand Down
31 changes: 23 additions & 8 deletions adapters/googlePhotos/matcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ func Test_matchers(t *testing.T) {
fileName: "PXL_20211013_220651983.jpg",
want: "normalMatch",
},
{
jsonName: "PXL_20211013_220651983.jpg.json",
fileName: "PXL_20211013_220651958.jpg",
want: "",
},
{
jsonName: "PXL_20220405_090123740.PORTRAIT.jpg.json",
fileName: "PXL_20220405_090123740.PORTRAIT-modifié.jpg",
Expand All @@ -155,6 +160,11 @@ func Test_matchers(t *testing.T) {
fileName: "DSC_0238(1).JPG",
want: "matchDuplicateInYear",
},
{
jsonName: "DSC_0238.JPG(1).json",
fileName: "DSC_0238.JPG",
want: "",
},
{
jsonName: "IMG_2710.HEIC(1).json",
fileName: "IMG_2710(1).HEIC",
Expand All @@ -165,11 +175,11 @@ func Test_matchers(t *testing.T) {
fileName: "PXL_20231118_035751175.MP.jpg",
want: "normalMatch",
},
{
jsonName: "PXL_20231118_035751175.MP.jpg.json",
fileName: "PXL_20231118_035751175.MP",
want: "livePhotoMatch",
},
// { // MP are now ignored
// jsonName: "PXL_20231118_035751175.MP.jpg.json",
// fileName: "PXL_20231118_035751175.MP",
// want: "livePhotoMatch",
// },
{
jsonName: "PXL_20230809_203449253.LONG_EXPOSURE-02.ORIGIN.json",
fileName: "PXL_20230809_203449253.LONG_EXPOSURE-02.ORIGINA.jpg",
Expand All @@ -178,7 +188,7 @@ func Test_matchers(t *testing.T) {
{
jsonName: "05yqt21kruxwwlhhgrwrdyb6chhwszi9bqmzu16w0 2.jp.json",
fileName: "05yqt21kruxwwlhhgrwrdyb6chhwszi9bqmzu16w0 2.jpg",
want: "livePhotoMatch",
want: "matchWithOneCharOmitted",
},
{
jsonName: "😀😃😄😁😆😅😂🤣🥲☺️😊😇🙂🙃😉😌😍🥰😘😗😙😚😋.json",
Expand All @@ -202,9 +212,14 @@ func Test_matchers(t *testing.T) {
},
{ // #405
jsonName: "PXL_20210102_221126856.MP~2.jpg.json",
fileName: "PXL_20210102_221126856.MP~2",
want: "livePhotoMatch",
fileName: "PXL_20210102_221126856.MP~2.jpg",
want: "normalMatch",
},
// { // #405
// jsonName: "PXL_20210102_221126856.MP~2.jpg.json",
// fileName: "PXL_20210102_221126856.MP~2",
// want: "livePhotoMatch",
// },
}
for _, tt := range tests {
t.Run(tt.fileName, func(t *testing.T) {
Expand Down
22 changes: 11 additions & 11 deletions adapters/googlePhotos/matchers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ func normalMatch(jsonName string, fileName string, sm filetypes.SupportedMedia)
// PXL_20231118_035751175.MP.jpg.json
// PXL_20231118_035751175.MP.jpg
// PXL_20231118_035751175.MP
func livePhotoMatch(jsonName string, fileName string, sm filetypes.SupportedMedia) bool {
fileExt := path.Ext(fileName)
fileName = strings.TrimSuffix(fileName, fileExt)
base := strings.TrimSuffix(jsonName, path.Ext(jsonName))
base = strings.TrimSuffix(base, path.Ext(base))
if base == fileName {
return true
}
base = strings.TrimSuffix(base, path.Ext(base))
return base == fileName
}
// func livePhotoMatch(jsonName string, fileName string, sm filetypes.SupportedMedia) bool {
// fileExt := path.Ext(fileName)
// fileName = strings.TrimSuffix(fileName, fileExt)
// base := strings.TrimSuffix(jsonName, path.Ext(jsonName))
// base = strings.TrimSuffix(base, path.Ext(base))
// if base == fileName {
// return true
// }
// base = strings.TrimSuffix(base, path.Ext(base))
// return base == fileName
// }

// matchWithOneCharOmitted
//
Expand Down
43 changes: 28 additions & 15 deletions app/cmd/upload/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/simulot/immich-go/internal/assets"
"github.com/simulot/immich-go/internal/fileevent"
"github.com/simulot/immich-go/internal/filters"
"github.com/simulot/immich-go/internal/fshelper"
)

type UpCmd struct {
Expand Down Expand Up @@ -110,6 +111,7 @@ func (upCmd *UpCmd) getImmichAssets(ctx context.Context, updateFn progressUpdate
upCmd.AssetIndex = &AssetIndex{
assets: list,
}
upCmd.app.Log().Info(fmt.Sprintf("Assets on the server: %d", len(list)))
upCmd.AssetIndex.ReIndex()
return nil
}
Expand Down Expand Up @@ -214,11 +216,10 @@ func (upCmd *UpCmd) handleAsset(ctx context.Context, a *assets.Asset) error {

// Manage albums
if len(a.Albums) > 0 {
upCmd.manageAssetAlbums(ctx, a)
upCmd.manageAssetAlbums(ctx, a.File, a.ID, a.Albums)
}
return upCmd.manageAssetTags(ctx, a)
case SmallerOnServer: // Upload, manage albums and delete the server's asset
upCmd.app.Jnl().Record(ctx, fileevent.UploadUpgraded, a, "reason", advice.Message)

// Remember existing asset's albums, if any
for _, al := range advice.ServerAsset.Albums {
Expand All @@ -229,26 +230,22 @@ func (upCmd *UpCmd) handleAsset(ctx context.Context, a *assets.Asset) error {
}

// Upload the superior asset
err = upCmd.uploadAsset(ctx, a)
err = upCmd.replaceAsset(ctx, advice.ServerAsset.ID, a)
if err != nil {
return err
}
upCmd.app.Jnl().Record(ctx, fileevent.UploadUpgraded, a, "reason", advice.Message)

// Manage albums
if len(a.Albums) > 0 {
upCmd.manageAssetAlbums(ctx, a)
upCmd.manageAssetAlbums(ctx, a.File, advice.ServerAsset.ID, a.Albums)
}

err = upCmd.manageAssetTags(ctx, a)
if err != nil {
return err
}

// delete the existing lower quality asset
err = upCmd.app.Client().Immich.DeleteAssets(ctx, []string{advice.ServerAsset.ID}, true)
if err != nil {
upCmd.app.Jnl().Record(ctx, fileevent.Error, nil, "error", err.Error())
}
return err

case SameOnServer:
Expand All @@ -268,7 +265,7 @@ func (upCmd *UpCmd) handleAsset(ctx context.Context, a *assets.Asset) error {

// Manage albums
if len(a.Albums) > 0 {
upCmd.manageAssetAlbums(ctx, a)
upCmd.manageAssetAlbums(ctx, a.File, a.ID, a.Albums)
}

case BetterOnServer: // and manage albums
Expand Down Expand Up @@ -323,30 +320,46 @@ func (upCmd *UpCmd) uploadAsset(ctx context.Context, a *assets.Asset) error {
return nil
}

func (upCmd *UpCmd) replaceAsset(ctx context.Context, ID string, a *assets.Asset) error {
defer upCmd.app.Log().Debug("", "file", a)
ar, err := upCmd.app.Client().Immich.ReplaceAsset(ctx, ID, a)
if err != nil {
upCmd.app.Jnl().Record(ctx, fileevent.UploadServerError, a.File, "error", err.Error())
return err // Must signal the error to the caller
}
if ar.Status == immich.UploadDuplicate {
upCmd.app.Jnl().Record(ctx, fileevent.UploadServerDuplicate, a.File, "reason", "the server has this file")
} else {
upCmd.app.Jnl().Record(ctx, fileevent.UploadUpgraded, a.File)
a.ID = ar.ID
}
return nil
}

// manageAssetAlbums add the assets to the albums listed.
// If an album does not exist, it is created.
// Errors are logged.
func (upCmd *UpCmd) manageAssetAlbums(ctx context.Context, a *assets.Asset) {
for _, album := range a.Albums {
func (upCmd *UpCmd) manageAssetAlbums(ctx context.Context, f fshelper.FSAndName, ID string, albums []assets.Album) {
for _, album := range albums {
title := album.Title
l, exist := upCmd.albums[title]
if !exist {
newAl, err := upCmd.app.Client().Immich.CreateAlbum(ctx, title, album.Description, []string{a.ID})
newAl, err := upCmd.app.Client().Immich.CreateAlbum(ctx, title, album.Description, []string{ID})
if err != nil {
upCmd.app.Jnl().Record(ctx, fileevent.Error, nil, "error", err)
}
upCmd.albums[title] = newAl
l = newAl
} else {
_, err := upCmd.app.Client().Immich.AddAssetToAlbum(ctx, l.ID, []string{a.ID})
_, err := upCmd.app.Client().Immich.AddAssetToAlbum(ctx, l.ID, []string{ID})
if err != nil {
upCmd.app.Jnl().Record(ctx, fileevent.Error, nil, "error", err)
return
}
}

// Log the action
upCmd.app.Jnl().Record(ctx, fileevent.UploadAddToAlbum, a.File, "Album", title)
upCmd.app.Jnl().Record(ctx, fileevent.UploadAddToAlbum, f, "Album", title)
}
}

Expand Down
101 changes: 101 additions & 0 deletions immich/asset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,107 @@ func (ic *ImmichClient) AssetUpload(ctx context.Context, la *assets.Asset) (Asse
return ar, err
}

func (ic *ImmichClient) ReplaceAsset(ctx context.Context, ID string, la *assets.Asset) (AssetResponse, error) {
if ic.dryRun {
return AssetResponse{
ID: uuid.NewString(),
Status: UploadReplaced,
}, nil
}
var ar AssetResponse
ext := path.Ext(la.OriginalFileName)
if strings.TrimSuffix(la.OriginalFileName, ext) == "" {
la.OriginalFileName = "No Name" + ext // fix #88, #128
}

if strings.ToUpper(ext) == ".MP" {
// Should be discarded before calling AssetUpload as MP are useless
ext = ".MP4" // #405
la.OriginalFileName = la.OriginalFileName + ".MP4"
}
mtype := ic.TypeFromExt(ext)
switch mtype {
case "video", "image":
default:
return ar, fmt.Errorf("type file not supported: %s", path.Ext(la.OriginalFileName))
}

f, err := la.OpenFile()
if err != nil {
return ar, (err)
}

body, pw := io.Pipe()
m := multipart.NewWriter(pw)

go func() {
defer func() {
m.Close()
pw.Close()
f.Close()
}()
var s fs.FileInfo
s, err = f.Stat()
if err != nil {
return
}

err = m.WriteField("deviceAssetId", fmt.Sprintf("%s-%d", path.Base(la.OriginalFileName), s.Size()))
if err != nil {
return
}
err = m.WriteField("deviceId", ic.DeviceUUID)
if err != nil {
return
}

if !la.CaptureDate.IsZero() {
err = m.WriteField("fileCreatedAt", la.CaptureDate.Format(TimeFormat))
} else {
err = m.WriteField("fileCreatedAt", s.ModTime().Format(TimeFormat))
}
if err != nil {
return
}
err = m.WriteField("fileModifiedAt", s.ModTime().Format(TimeFormat))
if err != nil {
return
}

h := textproto.MIMEHeader{}
h.Set("Content-Disposition",
fmt.Sprintf(`form-data; name="%s"; filename="%s"`,
escapeQuotes("assetData"), escapeQuotes(path.Base(la.OriginalFileName))))
h.Set("Content-Type", mtype)

var part io.Writer
part, err = m.CreatePart(h)
if err != nil {
return
}
_, err = io.Copy(part, f)
if err != nil {
return
}
}()

var callValues map[string]string
if ic.apiTraceWriter != nil {
callValues = map[string]string{
ctxAssetName: la.File.Name(),
}
// if la.FromSideCar != nil {
// callValues[ctxSideCarName] = la.FromSideCar.File.Name()
// }
}

errCall := ic.newServerCall(ctx, "ReplaceAsset").
do(putRequest("/assets/"+ID+"/original", setContextValue(callValues), setAcceptJSON(), setContentType(m.FormDataContentType()), setBody(body)), responseJSON(&ar))

err = errors.Join(err, errCall)
return ar, err
}

const (
ctxCallValues = "call-values"
ctxAssetName = "asset file name"
Expand Down
29 changes: 23 additions & 6 deletions immich/call.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ type callError struct {
}

type ServerMessage struct {
Error string `json:"error"`
StatusCode string `json:"statusCode"`
Message []string `json:"message"`
Error string `json:"error"`
StatusCode int `json:"statusCode"`
Message []string `json:"message"`
CorrelationID string `json:"correlationId"`
}

func (ce callError) Is(target error) bool {
Expand Down Expand Up @@ -228,13 +229,29 @@ func (sc *serverCall) do(fnRequest requestFunction, opts ...serverResponseOption
if resp.StatusCode >= 300 {
msg := ServerMessage{}
if resp.Body != nil {
defer resp.Body.Close()
if json.NewDecoder(resp.Body).Decode(&msg) == nil {
if sc.ic.apiTraceWriter != nil && sc.endPoint != EndPointGetJobs {
seq := sc.ctx.Value(ctxCallSequenceID)
fmt.Fprintln(
sc.ic.apiTraceWriter,
time.Now().Format(time.RFC3339),
"RESPONSE",
seq,
sc.endPoint,
resp.Request.Method,
resp.Request.URL.String(),
)
fmt.Fprintln(sc.ic.apiTraceWriter, " Status:", resp.Status)
fmt.Fprintln(sc.ic.apiTraceWriter, "-- response body --")
dec := json.NewEncoder(newLimitWriter(sc.ic.apiTraceWriter, 100))
dec.SetIndent("", " ")
_ = dec.Encode(msg) // nolint: errcheck
fmt.Fprint(sc.ic.apiTraceWriter, "-- response body end --\n\n")
}
return sc.Err(req, resp, &msg)
}
}
if resp.Body != nil {
resp.Body.Close()
}
return sc.Err(req, resp, &msg)
}

Expand Down
4 changes: 4 additions & 0 deletions immich/immich.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type ImmichInterface interface {
DownloadAsset(ctx context.Context, id string) (io.ReadCloser, error)

UpdateAsset(ctx context.Context, id string, param UpdAssetField) (*Asset, error)
ReplaceAsset(ctx context.Context, ID string, la *assets.Asset) (AssetResponse, error)
GetAllAssets(ctx context.Context) ([]*Asset, error)
AddAssetToAlbum(context.Context, string, []string) ([]UpdateAlbumResult, error)
UpdateAssets(
Expand All @@ -43,6 +44,9 @@ type ImmichInterface interface {
stackParentID string,
) error
GetAllAssetsWithFilter(context.Context, *SearchMetadataQuery, func(*Asset) error) error
GetAssetsByHash(ctx context.Context, hash string) ([]*Asset, error)
GetAssetsByImageName(ctx context.Context, name string) ([]*Asset, error)

AssetUpload(context.Context, *assets.Asset) (AssetResponse, error)
DeleteAssets(context.Context, []string, bool) error

Expand Down
Loading

0 comments on commit 84cee85

Please sign in to comment.