Fix stack creation error when uploading assets with same name (#752)

* discard the ID obtained via replaceAsset, and set the current ID

* refactor: better use of DeviceAssetID to manage images with the same name fix: #749

adapters/googlePhotos/googlephotos.go

@@ -268,6 +268,8 @@ func (to *Takeout) passOneFsWalk(ctx context.Context, w fs.FS) error {
 					baseName: base,
 					size:     finfo.Size(),
 				}
+
+				// TODO: remove debugging code
 				tracking, _ := to.fileTracker.Load(key) // tracking := to.fileTracker[key]
 				tracking.paths = append(tracking.paths, dir)
 				tracking.count++
@@ -537,9 +539,11 @@ func (to *Takeout) makeAsset(_ context.Context, dir string, f *assetFile, md *as
 
 	// get the original file name from metadata
 	if md != nil && md.FileName != "" {
-		a.OriginalFileName = md.FileName
-
 		title := md.FileName
+
+		// a.OriginalFileName = md.FileName
+		// title := md.FileName
+
 		// trim superfluous extensions
 		titleExt := path.Ext(title)
 		fileExt := path.Ext(file)

app/cmd/upload/advice.go

@@ -98,16 +98,16 @@ func (ai *AssetIndex) adviceNotOnServer() *Advice {
 //
 // The server may have different assets with the same name. This happens with photos produced by digital cameras.
 // The server may have the asset, but in lower resolution. Compare the taken date and resolution
+//
+// la - local asset
+// la.File.Name() is the full path to the file as it is on the source
+// la.OriginalFileName is the name of the file as it was on the device before it was uploaded to the server
 
 func (ai *AssetIndex) ShouldUpload(la *assets.Asset) (*Advice, error) {
-	filename := la.OriginalFileName
-	if path.Ext(filename) == "" {
-		filename += path.Ext(la.File.Name())
-	}
-
-	ID := la.DeviceAssetID()
+	filename := la.File.Name()
+	DeviceAssetID := fmt.Sprintf("%s-%d", path.Base(filename), la.FileSize)
 
-	sa := ai.byID[ID]
+	sa := ai.byDeviceAssetID[DeviceAssetID]
 	if sa != nil {
 		// the same ID exist on the server
 		return ai.adviceSameOnServer(sa), nil

app/cmd/upload/assets.go

@@ -1,7 +1,6 @@
 package upload
 
 import (
-	"fmt"
 	"path"
 	"strings"
 
@@ -10,20 +9,20 @@ import (
 )
 
 type AssetIndex struct {
-	assets []*immich.Asset
-	byHash map[string][]*immich.Asset
-	byName map[string][]*immich.Asset
-	byID   map[string]*immich.Asset
+	assets          []*immich.Asset
+	byHash          map[string][]*immich.Asset
+	byName          map[string][]*immich.Asset
+	byDeviceAssetID map[string]*immich.Asset
 	// albums []immich.AlbumSimplified
 }
 
 func (ai *AssetIndex) ReIndex() {
 	ai.byHash = map[string][]*immich.Asset{}
 	ai.byName = map[string][]*immich.Asset{}
-	ai.byID = map[string]*immich.Asset{}
+	ai.byDeviceAssetID = map[string]*immich.Asset{}
 
 	for _, a := range ai.assets {
-		ID := fmt.Sprintf("%s-%d", a.OriginalFileName, a.ExifInfo.FileSizeInByte)
+		ID := a.DeviceAssetID
 		l := ai.byHash[a.Checksum]
 		l = append(l, a)
 		ai.byHash[a.Checksum] = l
@@ -32,7 +31,7 @@ func (ai *AssetIndex) ReIndex() {
 		l = ai.byName[n]
 		l = append(l, a)
 		ai.byName[n] = l
-		ai.byID[ID] = a
+		ai.byDeviceAssetID[ID] = a
 	}
 }
 
@@ -43,7 +42,7 @@ func (ai *AssetIndex) Len() int {
 func (ai *AssetIndex) AddLocalAsset(la *assets.Asset, immichID string) {
 	sa := &immich.Asset{
 		ID:               immichID,
-		DeviceAssetID:    la.DeviceAssetID(),
+		DeviceAssetID:    la.DeviceAssetID,
 		OriginalFileName: strings.TrimSuffix(path.Base(la.OriginalFileName), path.Ext(la.OriginalFileName)),
 		ExifInfo: immich.ExifInfo{
 			FileSizeInByte:   int64(la.FileSize),
@@ -53,7 +52,7 @@ func (ai *AssetIndex) AddLocalAsset(la *assets.Asset, immichID string) {
 		},
 	}
 	ai.assets = append(ai.assets, sa)
-	ai.byID[sa.DeviceAssetID] = sa
+	ai.byDeviceAssetID[sa.DeviceAssetID] = sa
 	l := ai.byName[sa.OriginalFileName]
 	l = append(l, sa)
 	ai.byName[sa.OriginalFileName] = l

app/cmd/upload/run.go

@@ -220,27 +220,6 @@ func (upCmd *UpCmd) handleAsset(ctx context.Context, a *assets.Asset) error {
 		return err
 	}
 
-	// If the asset exists on the server, at full size, or smaller, we should get its tags and not tag it again.
-	if advice.ServerAsset != nil {
-		serverAsset, err := upCmd.app.Client().Immich.GetAssetInfo(ctx, advice.ServerAsset.ID)
-		if err == nil {
-			newList := []assets.Tag{}
-			for _, t := range a.Tags {
-				keepMe := true
-				for _, st := range serverAsset.Tags {
-					if t.Name == st.Name {
-						keepMe = false
-						break
-					}
-				}
-				if keepMe {
-					newList = append(newList, t)
-				}
-			}
-			a.Tags = newList
-		}
-	}
-
 	switch advice.Advice {
 	case NotOnServer: // Upload and manage albums
 		err = upCmd.uploadAsset(ctx, a)
@@ -343,6 +322,7 @@ func (upCmd *UpCmd) replaceAsset(ctx context.Context, ID string, a *assets.Asset
 	if ar.Status == immich.UploadDuplicate {
 		upCmd.app.Jnl().Record(ctx, fileevent.UploadServerDuplicate, a.File, "reason", "the server has this file")
 	} else {
+		a.ID = ID
 		upCmd.app.Jnl().Record(ctx, fileevent.UploadUpgraded, a.File)
 	}
 	return nil

internal/assets/asset.go

@@ -25,9 +25,10 @@ import (
 
 type Asset struct {
 	// File system and file name
-	File     fshelper.FSAndName
-	FileDate time.Time // File creation date
-	ID       string    // Immich ID after upload
+	File          fshelper.FSAndName
+	FileDate      time.Time // File creation date
+	ID            string    // Immich ID after upload
+	DeviceAssetID string    // file name on the input (as delivered in the takeout) + file size
 
 	// Common fields
 	OriginalFileName string // File name as delivered to Immich/Google

internal/assets/assetFile.go

@@ -1,16 +1,14 @@
 package assets
 
 import (
-	"fmt"
-
 	"github.com/simulot/immich-go/internal/fshelper/cachereader"
 	"github.com/simulot/immich-go/internal/fshelper/debugfiles"
 	"github.com/simulot/immich-go/internal/fshelper/osfs"
 )
 
-func (a *Asset) DeviceAssetID() string {
-	return fmt.Sprintf("%s-%d", a.OriginalFileName, a.FileSize)
-}
+// func (a *Asset) DeviceAssetID() string {
+// 	return fmt.Sprintf("%s-%d", path.Base(a.File.Name()), a.FileSize)
+// }
 
 // OpenFile return an os.File whatever the type of source reader is.
 // It can be called several times for the same asset.