Skip to content

Commit

Permalink
Improve performance when testing if user owns an album
Browse files Browse the repository at this point in the history
This is done by querying parent albums instead of sub-albums,
which changes the runtime from exponential to linear time.

This fixes #388
  • Loading branch information
viktorstrate committed Aug 30, 2021
1 parent 27bcbdc commit e142784
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 36 deletions.
24 changes: 24 additions & 0 deletions api/graphql/models/album.go
Expand Up @@ -54,3 +54,27 @@ func GetChildrenFromAlbums(db *gorm.DB, filter func(*gorm.DB) *gorm.DB, albumIDs

return children, err
}

func (a *Album) GetParents(db *gorm.DB, filter func(*gorm.DB) *gorm.DB) (parents []*Album, err error) {
return GetParentsFromAlbums(db, filter, a.ID)
}

func GetParentsFromAlbums(db *gorm.DB, filter func(*gorm.DB) *gorm.DB, albumID int) (parents []*Album, err error) {
query := db.Model(&Album{}).Table("super_albums")

if filter != nil {
query = filter(query)
}

err = db.Raw(`
WITH recursive super_albums AS (
SELECT * FROM albums AS leaf WHERE id = ?
UNION ALL
SELECT parent.* from albums AS parent JOIN super_albums ON parent.id = super_albums.parent_album_id
)
?
`, albumID, query).Find(&parents).Error

return parents, err
}
96 changes: 63 additions & 33 deletions api/graphql/models/album_test.go
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/stretchr/testify/assert"
)

func TestAlbumGetChildren(t *testing.T) {
func TestAlbumGetChildrenAndParents(t *testing.T) {
db := test_utils.DatabaseTest(t)

rootAlbum := models.Album{
Expand Down Expand Up @@ -51,43 +51,73 @@ func TestAlbumGetChildren(t *testing.T) {
return
}

root_children, err := rootAlbum.GetChildren(db, nil)
if !assert.NoError(t, err) {
return
}
verifyResult := func(t *testing.T, expected_albums []*models.Album, result []*models.Album) {
assert.Equal(t, len(expected_albums), len(result))

expected_children := []*models.Album{
{
Title: "root",
Path: "/photos",
},
{
Title: "child1",
Path: "/photos/child1",
},
{
Title: "child2",
Path: "/photos/child2",
},
{
Title: "subchild",
Path: "/photos/child1/subchild",
},
for _, expected := range expected_albums {
found_expected := false
for _, item := range result {
if item.Title == expected.Title && item.Path == expected.Path {
found_expected = true
break
}
}
if !found_expected {
assert.Failf(t, "albums did not match", "expected to find item: %v", expected)
}
}
}

assert.Equal(t, len(expected_children), len(root_children))
t.Run("Album get children", func(t *testing.T) {
root_children, err := rootAlbum.GetChildren(db, nil)
if !assert.NoError(t, err) {
return
}

for _, expected := range expected_children {
found_expected := false
for _, item := range root_children {
if item.Title == expected.Title && item.Path == expected.Path {
found_expected = true
break
}
expected_children := []*models.Album{
{
Title: "root",
Path: "/photos",
},
{
Title: "child1",
Path: "/photos/child1",
},
{
Title: "child2",
Path: "/photos/child2",
},
{
Title: "subchild",
Path: "/photos/child1/subchild",
},
}

verifyResult(t, expected_children, root_children)
})

t.Run("Album get parents", func(t *testing.T) {
parents, err := sub_child.GetParents(db, nil)
if !assert.NoError(t, err) {
return
}
if !found_expected {
assert.Failf(t, "root children did not match", "expected to find item: %v", expected)

expected_parents := []*models.Album{
{
Title: "root",
Path: "/photos",
},
{
Title: "child1",
Path: "/photos/child1",
},
{
Title: "subchild",
Path: "/photos/child1/subchild",
},
}
}

verifyResult(t, expected_parents, parents)
})

}
6 changes: 3 additions & 3 deletions api/graphql/models/user.go
Expand Up @@ -175,13 +175,13 @@ func (user *User) OwnsAlbum(db *gorm.DB, album *Album) (bool, error) {
}

filter := func(query *gorm.DB) *gorm.DB {
return query.Where("id = ?", album.ID)
return query.Where("id IN (?)", albumIDs)
}

ownedAlbum, err := GetChildrenFromAlbums(db, filter, albumIDs)
ownedParents, err := album.GetParents(db, filter)
if err != nil {
return false, err
}

return len(ownedAlbum) > 0, nil
return len(ownedParents) > 0, nil
}

0 comments on commit e142784

Please sign in to comment.