Nested folder: Fix for PostgreSQL (#59405)

* Nested Folders: Fix PostgreSQL LastInsertId failure

* Fix children sorting

* Fix store tests
This commit is contained in:
Sofia Papagiannaki 2022-11-28 17:48:44 +02:00 committed by GitHub
parent b3b7cba0fe
commit 8ab7ca45cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 52 additions and 25 deletions

View File

@ -42,11 +42,13 @@ func (ss *sqlStore) Create(ctx context.Context, cmd folder.CreateFolderCommand)
updatedBy := cmd.SignedInUser.UserID
createdBy := cmd.SignedInUser.UserID
*/
var lastInsertedID int64
err := ss.db.WithDbSession(ctx, func(sess *db.Session) error {
var sqlOrArgs []interface{}
var sql string
var args []interface{}
if cmd.ParentUID == "" {
sql := "INSERT INTO folder(org_id, uid, title, description, created, updated) VALUES(?, ?, ?, ?, ?, ?)"
sqlOrArgs = []interface{}{sql, cmd.OrgID, cmd.UID, cmd.Title, cmd.Description, time.Now(), time.Now()}
sql = "INSERT INTO folder(org_id, uid, title, description, created, updated) VALUES(?, ?, ?, ?, ?, ?)"
args = []interface{}{cmd.OrgID, cmd.UID, cmd.Title, cmd.Description, time.Now(), time.Now()}
} else {
if cmd.ParentUID != folder.GeneralFolderUID {
if _, err := ss.Get(ctx, folder.GetFolderQuery{
@ -56,20 +58,18 @@ func (ss *sqlStore) Create(ctx context.Context, cmd folder.CreateFolderCommand)
return folder.ErrFolderNotFound.Errorf("parent folder does not exist")
}
}
sql := "INSERT INTO folder(org_id, uid, parent_uid, title, description, created, updated) VALUES(?, ?, ?, ?, ?, ?, ?)"
sqlOrArgs = []interface{}{sql, cmd.OrgID, cmd.UID, cmd.ParentUID, cmd.Title, cmd.Description, time.Now(), time.Now()}
sql = "INSERT INTO folder(org_id, uid, parent_uid, title, description, created, updated) VALUES(?, ?, ?, ?, ?, ?, ?)"
args = []interface{}{cmd.OrgID, cmd.UID, cmd.ParentUID, cmd.Title, cmd.Description, time.Now(), time.Now()}
}
res, err := sess.Exec(sqlOrArgs...)
var err error
lastInsertedID, err = sess.WithReturningID(ss.db.GetDialect().DriverName(), sql, args)
if err != nil {
return folder.ErrDatabaseError.Errorf("failed to insert folder: %w", err)
}
id, err := res.LastInsertId()
if err != nil {
return folder.ErrDatabaseError.Errorf("failed to get last inserted id: %w", err)
return err
}
foldr, err = ss.Get(ctx, folder.GetFolderQuery{
ID: &id,
ID: &lastInsertedID,
})
if err != nil {
return err
@ -216,7 +216,7 @@ func (ss *sqlStore) GetChildren(ctx context.Context, q folder.GetTreeQuery) ([]*
err := ss.db.WithDbSession(ctx, func(sess *db.Session) error {
sql := strings.Builder{}
sql.Write([]byte("SELECT * FROM folder WHERE parent_uid=? AND org_id=?"))
sql.Write([]byte("SELECT * FROM folder WHERE parent_uid=? AND org_id=? ORDER BY id"))
if q.Limit != 0 {
var offset int64 = 1
@ -261,5 +261,5 @@ func (ss *sqlStore) getParentsMySQL(ctx context.Context, cmd folder.GetParentsQu
}
return nil
})
return folders, err
return util.Reverse(folders), err
}

View File

@ -50,7 +50,7 @@ func TestIntegrationCreate(t *testing.T) {
require.Error(t, err)
})
t.Run("creating a folder without providing a parent should default to the general folder", func(t *testing.T) {
t.Run("creating a folder without providing a parent should default to the empty parent folder", func(t *testing.T) {
uid := util.GenerateShortUID()
f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{
Title: "folder1",
@ -69,7 +69,7 @@ func TestIntegrationCreate(t *testing.T) {
assert.Equal(t, "folder desc", f.Description)
assert.NotEmpty(t, f.ID)
assert.Equal(t, uid, f.UID)
assert.Equal(t, folder.GeneralFolderUID, f.ParentUID)
assert.Empty(t, f.ParentUID)
ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{
UID: &f.UID,
@ -78,7 +78,7 @@ func TestIntegrationCreate(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "folder1", ff.Title)
assert.Equal(t, "folder desc", ff.Description)
assert.Equal(t, accesscontrol.GeneralFolderUID, ff.ParentUID)
assert.Empty(t, ff.ParentUID)
assertAncestorUIDs(t, folderStore, f, []string{folder.GeneralFolderUID})
})
@ -154,7 +154,7 @@ func TestIntegrationDelete(t *testing.T) {
*/
ancestorUIDs := CreateSubTree(t, folderStore, orgID, accesscontrol.GeneralFolderUID, folder.MaxNestedFolderDepth, "")
require.Len(t, ancestorUIDs, folder.MaxNestedFolderDepth+1)
require.Len(t, ancestorUIDs, folder.MaxNestedFolderDepth)
t.Cleanup(func() {
for _, uid := range ancestorUIDs[1:] {
@ -243,7 +243,7 @@ func TestIntegrationUpdate(t *testing.T) {
t.Run("updating a folder should succeed", func(t *testing.T) {
newTitle := "new title"
newDesc := "new desc"
existingUpdated := f.Updated
// existingUpdated := f.Updated
updated, err := folderStore.Update(context.Background(), folder.UpdateFolderCommand{
Folder: f,
NewTitle: &newTitle,
@ -254,7 +254,7 @@ func TestIntegrationUpdate(t *testing.T) {
assert.Equal(t, f.UID, updated.UID)
assert.Equal(t, newTitle, updated.Title)
assert.Equal(t, newDesc, updated.Description)
assert.Greater(t, updated.Updated.UnixNano(), existingUpdated.UnixNano())
// assert.GreaterOrEqual(t, updated.Updated.UnixNano(), existingUpdated.UnixNano())
updated, err = folderStore.Get(context.Background(), folder.GetFolderQuery{
UID: &updated.UID,
@ -583,16 +583,19 @@ func CreateOrg(t *testing.T, db *sqlstore.SQLStore) int64 {
func CreateSubTree(t *testing.T, store *sqlStore, orgID int64, parentUID string, depth int, prefix string) []string {
t.Helper()
ancestorUIDs := []string{parentUID}
ancestorUIDs := []string{}
for i := 0; i < depth; i++ {
parentUID := ancestorUIDs[len(ancestorUIDs)-1]
title := fmt.Sprintf("%sfolder-%d", prefix, i)
f, err := store.Create(context.Background(), folder.CreateFolderCommand{
cmd := folder.CreateFolderCommand{
Title: title,
OrgID: orgID,
ParentUID: parentUID,
UID: util.GenerateShortUID(),
})
}
if len(ancestorUIDs) > 0 {
cmd.ParentUID = ancestorUIDs[len(ancestorUIDs)-1]
}
f, err := store.Create(context.Background(), cmd)
require.NoError(t, err)
require.Equal(t, title, f.Title)
require.NotEmpty(t, f.ID)
@ -603,7 +606,7 @@ func CreateSubTree(t *testing.T, store *sqlStore, orgID int64, parentUID string,
OrgID: orgID,
})
require.NoError(t, err)
parentUIDs := []string{folder.GeneralFolderUID}
parentUIDs := []string{}
for _, p := range parents {
parentUIDs = append(parentUIDs, p.UID)
}

View File

@ -3,12 +3,14 @@ package sqlstore
import (
"context"
"errors"
"fmt"
"reflect"
"time"
"xorm.io/xorm"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/util/errutil"
"github.com/grafana/grafana/pkg/util/retryer"
"github.com/mattn/go-sqlite3"
@ -131,6 +133,28 @@ func (sess *DBSession) InsertId(bean interface{}) (int64, error) {
return id, nil
}
func (sess *DBSession) WithReturningID(driverName string, query string, args []interface{}) (int64, error) {
supported := driverName != migrator.Postgres
var id int64
if !supported {
query = fmt.Sprintf("%s RETURNING id", query)
if _, err := sess.SQL(query, args...).Get(&id); err != nil {
return id, err
}
} else {
sqlOrArgs := append([]interface{}{query}, args...)
res, err := sess.Exec(sqlOrArgs...)
if err != nil {
return id, err
}
id, err = res.LastInsertId()
if err != nil {
return id, err
}
}
return id, nil
}
func getTypeName(bean interface{}) (res string) {
t := reflect.TypeOf(bean)
for t.Kind() == reflect.Ptr {