Skip to content

Commit

Permalink
Fix Google Photos upload segfault and improve tests (#619)
Browse files Browse the repository at this point in the history 
* Fix filterOnMetadata to handle empty directory case

* Add a test case to track #613

* split e2e tests

* Add end-to-end test for uploading from Google Photos to address issue #613

* Fix nil pointer dereference by checking FromApplication before assigning Albums.

* Refactor GP match functions for improved clarity and performance; add getFileIndex utility

* Refactor matcher tests to improve clarity and update expected results for various cases

* Refactor matchers in Google Photos adapter for improved clarity and organization

* Update end-to-end tests for Google Photos upload to support wildcard zip files and add album name parameter

* Merge branch 'next' into simulot/issue613
  • Loading branch information
@simulot
simulot authored Jan 11, 2025
1 parent 065c8f3 commit 0fab5a8
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 375 deletions.
16 changes: 9 additions & 7 deletions adapters/googlePhotos/googlephotos.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,10 @@ var matchers = []struct {
name string
fn matcherFn
}{
{name: "normalMatch", fn: normalMatch},
// {name: "livePhotoMatch", fn: livePhotoMatch},
{name: "matchWithOneCharOmitted", fn: matchWithOneCharOmitted},
{name: "matchVeryLongNameWithNumber", fn: matchVeryLongNameWithNumber},
{name: "matchDuplicateInYear", fn: matchDuplicateInYear},
{name: "matchEditedName", fn: matchEditedName},
{name: "matchFastTrack", fn: matchFastTrack},
{name: "matchNormal", fn: matchNormal},
{name: "matchForgottenDuplicates", fn: matchForgottenDuplicates},
{name: "matchEditedName", fn: matchEditedName},
}

func (to *Takeout) solvePuzzle(ctx context.Context) error {
Expand Down Expand Up @@ -468,7 +465,9 @@ func (to *Takeout) handleDir(ctx context.Context, dir string, gOut chan *assets.
if to.flags.PartnerSharedAlbum != "" && a.FromPartner {
a.Albums = append(a.Albums, assets.Album{Title: to.flags.PartnerSharedAlbum})
}
a.FromApplication.Albums = a.Albums
if a.FromApplication != nil {
a.FromApplication.Albums = a.Albums
}
}
// If the asset has no GPS information, but the album has, use the album's location
if a.Latitude == 0 && a.Longitude == 0 {
Expand Down Expand Up @@ -580,6 +579,9 @@ func (to *Takeout) filterOnMetadata(ctx context.Context, a *assets.Asset) fileev
if to.flags.ImportFromAlbum != "" {
keep := false
dir := path.Dir(a.File.Name())
if dir == "." {
dir = ""
}
if album, ok := to.albums[dir]; ok {
keep = keep || album.Title == to.flags.ImportFromAlbum
}
Expand Down
206 changes: 59 additions & 147 deletions adapters/googlePhotos/matcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,124 +6,6 @@ import (
"github.com/simulot/immich-go/internal/filetypes"
)

func Test_matchVeryLongNameWithNumber(t *testing.T) {
tests := []struct {
jsonName string
fileName string
want bool
}{
{
jsonName: "Backyard_ceremony_wedding_photography_xxxxxxx_(494).json",
fileName: "Backyard_ceremony_wedding_photography_xxxxxxx_m(494).jpg",
want: true,
},
{
jsonName: "Backyard_ceremony_wedding_photography_xxxxxxx_(494).json",
fileName: "Backyard_ceremony_wedding_photography_xxxxxxx_m(185).jpg",
want: false,
},
}
for _, tt := range tests {
t.Run(tt.fileName, func(t *testing.T) {
if got := matchVeryLongNameWithNumber(tt.jsonName, tt.fileName, filetypes.DefaultSupportedMedia); got != tt.want {
t.Errorf("matchVeryLongNameWithNumber() = %v, want %v", got, tt.want)
}
})
}
}

func Test_matchDuplicateInYear(t *testing.T) {
tests := []struct {
name string
jsonName string
fileName string
want bool
}{
{
name: "match",
jsonName: "IMG_3479.JPG(2).json",
fileName: "IMG_3479(2).JPG",
want: true,
},
{
name: "doesn't match",
jsonName: "IMG_3479.JPG(2).json",
fileName: "IMG_3479(3).JPG",
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := matchDuplicateInYear(tt.jsonName, tt.fileName, filetypes.DefaultSupportedMedia); got != tt.want {
t.Errorf("matchDuplicateInYear() = %v, want %v", got, tt.want)
}
})
}
}

func Test_matchForgottenDuplicates(t *testing.T) {
tests := []struct {
name string
jsonName string
fileName string
want bool
}{
{
name: "match1",
jsonName: "1556189729458-8d2e2d13-bca5-467e-a242-9e4cb238.json",
fileName: "1556189729458-8d2e2d13-bca5-467e-a242-9e4cb238e.jpg",
want: true,
},
{
name: "match2",
jsonName: "1556189729458-8d2e2d13-bca5-467e-a242-9e4cb238.json",
fileName: "1556189729458-8d2e2d13-bca5-467e-a242-9e4cb238e(1).jpg",
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := matchForgottenDuplicates(tt.jsonName, tt.fileName, filetypes.DefaultSupportedMedia); got != tt.want {
t.Errorf("matchDuplicateInYear() = %v, want %v", got, tt.want)
}
})
}
}

/* indexes, but false
goos: linux
goarch: amd64
pkg: github.com/simulot/immich-go/browser/gp
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
Benchmark_matchDuplicateInYear-12 27067428 52.06 ns/op 0 B/op 0 allocs/op
PASS
ok github.com/simulot/immich-go/browser/gp 1.458s
goos: linux
goarch: amd64
pkg: github.com/simulot/immich-go/browser/gp
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
Benchmark_matchDuplicateInYear-12 881652 1491 ns/op 240 B/op 4 allocs/op
PASS
ok github.com/simulot/immich-go/browser/gp 1.332s
goos: linux
goarch: amd64
pkg: github.com/simulot/immich-go/browser/gp
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
Benchmark_matchDuplicateInYear-12 25737067 43.88 ns/op 0 B/op 0 allocs/op
PASS
ok github.com/simulot/immich-go/browser/gp 1.180s
*/

func Benchmark_matchDuplicateInYear(b *testing.B) {
for i := 0; i < b.N; i++ {
matchDuplicateInYear("IMG_3479.JPG(2).json", "IMG_3479(2).JPG", nil)
}
}

func Test_matchers(t *testing.T) {
tests := []struct {
jsonName string
Expand All @@ -133,7 +15,7 @@ func Test_matchers(t *testing.T) {
{
jsonName: "PXL_20211013_220651983.jpg.json",
fileName: "PXL_20211013_220651983.jpg",
want: "normalMatch",
want: "matchFastTrack",
},
{
jsonName: "PXL_20211013_220651983.jpg.json",
Expand All @@ -147,64 +29,80 @@ func Test_matchers(t *testing.T) {
},
{
jsonName: "PXL_20220405_090123740.PORTRAIT.jpg.json",
fileName: "PXL_20220405_100123740.PORTRAIT-modifié.jpg",
fileName: "PXL_20220405_090123741.PORTRAIT-modifié.jpg",
want: "",
},
{
jsonName: "DSC_0238.JPG.json",
fileName: "DSC_0238.JPG",
want: "normalMatch",
jsonName: "DSC_0100.JPG.json",
fileName: "DSC_0100.JPG",
want: "matchFastTrack",
},

{
jsonName: "DSC_0101.JPG(1).json",
fileName: "DSC_0101(1).JPG",
want: "matchNormal",
},
{
jsonName: "DSC_0238.JPG(1).json",
fileName: "DSC_0238(1).JPG",
want: "matchDuplicateInYear",
jsonName: "DSC_0102.JPG(2).json",
fileName: "DSC_0102(1).JPG",
want: "",
},

{
jsonName: "DSC_0103.JPG(1).json",
fileName: "DSC_0103.JPG",
want: "",
},

{
jsonName: "DSC_0238.JPG(1).json",
fileName: "DSC_0238.JPG",
jsonName: "DSC_0104.JPG.json",
fileName: "DSC_0104(1).JPG",
want: "",
},

{
jsonName: "IMG_2710.HEIC(1).json",
fileName: "IMG_2710(1).HEIC",
want: "matchDuplicateInYear",
want: "matchNormal",
},
{
jsonName: "PXL_20231118_035751175.MP.jpg.json",
fileName: "PXL_20231118_035751175.MP.jpg",
want: "normalMatch",
want: "matchFastTrack",
},
// { // 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",
want: "matchWithOneCharOmitted",
want: "matchNormal",
},
{
jsonName: "05yqt21kruxwwlhhgrwrdyb6chhwszi9bqmzu16w0 2.jp.json",
fileName: "05yqt21kruxwwlhhgrwrdyb6chhwszi9bqmzu16w0 2.jpg",
want: "matchWithOneCharOmitted",
want: "matchNormal",
},

{
jsonName: "😀😃😄😁😆😅😂🤣🥲☺️😊😇🙂🙃😉😌😍🥰😘😗😙😚😋.json",
fileName: "😀😃😄😁😆😅😂🤣🥲☺️😊😇🙂🙃😉😌😍🥰😘😗😙😚😋😛.jpg",
want: "matchWithOneCharOmitted",
want: "matchNormal",
},
{
jsonName: "Backyard_ceremony_wedding_photography_xxxxxxx_(494).json",
fileName: "Backyard_ceremony_wedding_photography_xxxxxxx_m(494).jpg",
want: "matchVeryLongNameWithNumber",
want: "matchNormal",
},
{
jsonName: "Backyard_ceremony_wedding_photography_xxxxxxx_(494).json",
fileName: "Backyard_ceremony_wedding_photography_xxxxxxx_m(185).jpg",
want: "",
},
{
jsonName: "original_1d4caa6f-16c6-4c3d-901b-9387de10e528_.json",
fileName: "original_1d4caa6f-16c6-4c3d-901b-9387de10e528_P.jpg",
want: "matchWithOneCharOmitted",
want: "matchNormal",
},

{
jsonName: "original_1d4caa6f-16c6-4c3d-901b-9387de10e528_.json",
fileName: "original_1d4caa6f-16c6-4c3d-901b-9387de10e528_P(1).jpg",
Expand All @@ -213,18 +111,32 @@ func Test_matchers(t *testing.T) {
{ // #405
jsonName: "PXL_20210102_221126856.MP~2.jpg.json",
fileName: "PXL_20210102_221126856.MP~2.jpg",
want: "normalMatch",
want: "matchFastTrack",
},

{ //#613
// 13039_327707840323_537645323_9470255_27214_n.j.json
// 13039_327707840323_537645323_9470255_27214_n.jpg

jsonName: "13039_327707840323_537645323_9470255_27214_n.j(1).json",
fileName: "13039_327707840323_537645323_9470255_27214_n(1).jpg",
want: "matchNormal",
},
// { // #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) {
matcher := ""
for _, m := range matchers {
tmatchers := []struct {
name string
fn matcherFn
}{
{name: "matchFastTrack", fn: matchFastTrack},
{name: "matchNormal", fn: matchNormal},
{name: "matchForgottenDuplicates", fn: matchForgottenDuplicates},
{name: "matchEditedName", fn: matchEditedName},
}

for _, m := range tmatchers {
if m.fn(tt.jsonName, tt.fileName, filetypes.DefaultSupportedMedia) {
matcher = m.name
break
Expand Down
Loading

0 comments on commit 0fab5a8

Please sign in to comment.