✨ Generate Primary Keys. (#635)

Generate primary keys instead of GORM.
This fixes the issue of GORM reusing the highest key after the model
with that ID is deleted.
When the PK is 0, GORM assigns the next (highest) ID.

This approach is to assign the ID ahead of time using a pool managed by
tackle.

---------

Signed-off-by: Jeff Ortel <jortel@redhat.com>

api/migrationwave.go

@@ -118,6 +118,7 @@ func (h MigrationWaveHandler) Create(ctx *gin.Context) {
 		_ = ctx.Error(err)
 		return
 	}
+
 	r.With(m)
 
 	h.Respond(ctx, http.StatusCreated, r)

cmd/main.go

@@ -17,6 +17,7 @@ import (
 	crd "github.com/konveyor/tackle2-hub/k8s/api"
 	"github.com/konveyor/tackle2-hub/metrics"
 	"github.com/konveyor/tackle2-hub/migration"
+	"github.com/konveyor/tackle2-hub/model"
 	"github.com/konveyor/tackle2-hub/reaper"
 	"github.com/konveyor/tackle2-hub/seed"
 	"github.com/konveyor/tackle2-hub/settings"
@@ -53,6 +54,10 @@ func Setup() (db *gorm.DB, err error) {
 	if err != nil {
 		return
 	}
+	err = database.PK.Load(db, model.ALL)
+	if err != nil {
+		return
+	}
 	return
 }
 

database/db_test.go

@@ -69,3 +69,56 @@ func TestConcurrent(t *testing.T) {
 		fmt.Printf("Done %d\n", id)
 	}
 }
+
+func TestKeyGen(t *testing.T) {
+	pid := os.Getpid()
+	Settings.DB.Path = fmt.Sprintf("/tmp/keygen-%d.db", pid)
+	defer func() {
+		_ = os.Remove(Settings.DB.Path)
+	}()
+	db, err := Open(true)
+	if err != nil {
+		panic(err)
+	}
+	// ids 1-7 created.
+	N = 8
+	for n := 1; n < N; n++ {
+		m := &model.Setting{Key: fmt.Sprintf("key-%d", n), Value: n}
+		err := db.Create(m).Error
+		if err != nil {
+			panic(err)
+		}
+		fmt.Printf("CREATED: %d/%d\n", m.ID, n)
+		if uint(n) != m.ID {
+			t.Errorf("id:%d but expected: %d", m.ID, n)
+			return
+		}
+	}
+	// delete ids=2,4,7.
+	err = db.Delete(&model.Setting{}, []uint{2, 4, 7}).Error
+	if err != nil {
+		panic(err)
+	}
+
+	var count int64
+	err = db.Model(&model.Setting{}).Where([]uint{2, 4, 7}).Count(&count).Error
+	if err != nil {
+		panic(err)
+	}
+	if count > 0 {
+		t.Errorf("DELETED ids: 2,4,7 found.")
+		return
+	}
+	// id=8 (next) created.
+	next := N
+	m := &model.Setting{Key: fmt.Sprintf("key-%d", next), Value: next}
+	err = db.Create(m).Error
+	if err != nil {
+		panic(err)
+	}
+	fmt.Printf("CREATED: %d/%d (next)\n", m.ID, next)
+	if uint(N) != m.ID {
+		t.Errorf("id:%d but expected: %d", m.ID, next)
+		return
+	}
+}

database/pk.go

@@ -0,0 +1,160 @@
+package database
+
+import (
+	"errors"
+	"reflect"
+	"strings"
+	"sync"
+
+	"github.com/konveyor/tackle2-hub/model"
+	"gorm.io/gorm"
+	"gorm.io/gorm/clause"
+	"gorm.io/gorm/logger"
+)
+
+// PK singleton pk sequence.
+var PK PkSequence
+
+// PkSequence provides a primary key sequence.
+type PkSequence struct {
+	mutex sync.Mutex
+}
+
+// Load highest key for all models.
+func (r *PkSequence) Load(db *gorm.DB, models []any) (err error) {
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
+	for _, m := range models {
+		mt := reflect.TypeOf(m)
+		if mt.Kind() == reflect.Ptr {
+			mt = mt.Elem()
+		}
+		kind := strings.ToUpper(mt.Name())
+		db = r.session(db)
+		q := db.Table(kind)
+		q = q.Select("MAX(ID) id")
+		cursor, err := q.Rows()
+		if err != nil || !cursor.Next() {
+			// not a table with id.
+			// discarded.
+			continue
+		}
+		id := int64(0)
+		err = cursor.Scan(&id)
+		_ = cursor.Close()
+		if err != nil {
+			r.add(db, kind, uint(0))
+		} else {
+			r.add(db, kind, uint(id))
+		}
+	}
+	return
+}
+
+// Next returns the next primary key.
+func (r *PkSequence) Next(db *gorm.DB) (id uint) {
+	r.mutex.Lock()
+	defer r.mutex.Unlock()
+	kind := strings.ToUpper(db.Statement.Table)
+	m := &model.PK{}
+	db = r.session(db)
+	err := db.First(m, "Kind", kind).Error
+	if err != nil {
+		return
+	}
+	m.LastID++
+	id = m.LastID
+	err = db.Save(m).Error
+	if err != nil {
+		panic(err)
+	}
+	return
+}
+
+// session returns a new DB with a new session.
+func (r *PkSequence) session(in *gorm.DB) (out *gorm.DB) {
+	out = &gorm.DB{
+		Config: in.Config,
+	}
+	out.Config.Logger.LogMode(logger.Warn)
+	out.Statement = &gorm.Statement{
+		DB:       out,
+		ConnPool: in.Statement.ConnPool,
+		Context:  in.Statement.Context,
+		Clauses:  map[string]clause.Clause{},
+		Vars:     make([]interface{}, 0, 8),
+	}
+	return
+}
+
+// add the last (higher) id for the kind.
+func (r *PkSequence) add(db *gorm.DB, kind string, id uint) {
+	m := &model.PK{Kind: kind}
+	db = r.session(db)
+	err := db.First(m).Error
+	if err != nil {
+		if !errors.Is(err, gorm.ErrRecordNotFound) {
+			panic(err)
+		}
+	}
+	if m.LastID > id {
+		return
+	}
+	m.LastID = id
+	db = r.session(db)
+	err = db.Save(m).Error
+	if err != nil {
+		panic(err)
+	}
+}
+
+// assignPk assigns PK as needed.
+func assignPk(db *gorm.DB) {
+	statement := db.Statement
+	schema := statement.Schema
+	if schema == nil {
+		return
+	}
+	switch statement.ReflectValue.Kind() {
+	case reflect.Slice,
+		reflect.Array:
+		for i := 0; i < statement.ReflectValue.Len(); i++ {
+			for _, f := range schema.Fields {
+				if f.Name != "ID" {
+					continue
+				}
+				_, isZero := f.ValueOf(
+					statement.Context,
+					statement.ReflectValue.Index(i))
+				if isZero {
+					id := PK.Next(db)
+					_ = f.Set(
+						statement.Context,
+						statement.ReflectValue.Index(i),
+						id)
+
+				}
+				break
+			}
+		}
+	case reflect.Struct:
+		for _, f := range schema.Fields {
+			if f.Name != "ID" {
+				continue
+			}
+			_, isZero := f.ValueOf(
+				statement.Context,
+				statement.ReflectValue)
+			if isZero {
+				id := PK.Next(db)
+				_ = f.Set(
+					statement.Context,
+					statement.ReflectValue,
+					id)
+			}
+			break
+		}
+	default:
+		log.Info("[WARN] assignPk: unknown kind.")
+	}
+}

database/pkg.go

@@ -51,7 +51,16 @@ func Open(enforceFKs bool) (db *gorm.DB, err error) {
 		err = liberr.Wrap(err)
 		return
 	}
-	err = db.AutoMigrate(model.Setting{})
+	err = db.AutoMigrate(model.PK{}, model.Setting{})
+	if err != nil {
+		err = liberr.Wrap(err)
+		return
+	}
+	err = PK.Load(db, []any{model.Setting{}})
+	if err != nil {
+		return
+	}
+	err = db.Callback().Create().Before("gorm:before_create").Register("assign-pk", assignPk)
 	if err != nil {
 		err = liberr.Wrap(err)
 		return

migration/v14/model/core.go

@@ -20,6 +20,13 @@ type Model struct {
 	UpdateUser string
 }
 
+// PK sequence.
+type PK struct {
+	Kind   string `gorm:"<-:create;primaryKey"`
+	LastID uint
+}
+
+// Setting hub settings.
 type Setting struct {
 	Model
 	Key   string `gorm:"<-:create;uniqueIndex"`

migration/v14/model/pkg.go

@@ -32,6 +32,7 @@ func All() []any {
 		ImportTag{},
 		JobFunction{},
 		MigrationWave{},
+		PK{},
 		Proxy{},
 		Review{},
 		Setting{},

model/pkg.go

@@ -8,6 +8,8 @@ import (
 // Field (data) types.
 type JSON = model.JSON
 
+var ALL = model.All()
+
 // Models
 type Model = model.Model
 type Application = model.Application
@@ -29,6 +31,7 @@ type ImportSummary = model.ImportSummary
 type ImportTag = model.ImportTag
 type JobFunction = model.JobFunction
 type MigrationWave = model.MigrationWave
+type PK = model.PK
 type Proxy = model.Proxy
 type Questionnaire = model.Questionnaire
 type Review = model.Review

test/api/migrationwave/api_test.go

@@ -18,6 +18,7 @@ func TestMigrationWaveCRUD(t *testing.T) {
 			}
 			assert.Must(t, Application.Create(&expectedApp))
 			createdApps = append(createdApps, expectedApp)
+			r.Applications[0].ID = expectedApp.ID
 		}
 
 		createdStakeholders := []api.Stakeholder{}
@@ -28,6 +29,7 @@ func TestMigrationWaveCRUD(t *testing.T) {
 			}
 			assert.Must(t, Stakeholder.Create(&expectedStakeholder))
 			createdStakeholders = append(createdStakeholders, expectedStakeholder)
+			r.Stakeholders[0].ID = expectedStakeholder.ID
 		}
 
 		createdStakeholderGroups := []api.StakeholderGroup{}
@@ -38,6 +40,7 @@ func TestMigrationWaveCRUD(t *testing.T) {
 			}
 			assert.Must(t, StakeholderGroup.Create(&expectedStakeholderGroup))
 			createdStakeholderGroups = append(createdStakeholderGroups, expectedStakeholderGroup)
+			r.StakeholderGroups[0].ID = expectedStakeholderGroup.ID
 		}
 
 		assert.Must(t, MigrationWave.Create(&r))
@@ -102,6 +105,7 @@ func TestMigrationWaveList(t *testing.T) {
 			}
 			assert.Must(t, Application.Create(&expectedApp))
 			createdApps = append(createdApps, expectedApp)
+			r.Applications[0].ID = expectedApp.ID
 		}
 
 		for _, stakeholder := range r.Stakeholders {
@@ -111,6 +115,7 @@ func TestMigrationWaveList(t *testing.T) {
 			}
 			assert.Must(t, Stakeholder.Create(&expectedStakeholder))
 			createdStakeholders = append(createdStakeholders, expectedStakeholder)
+			r.Stakeholders[0].ID = expectedStakeholder.ID
 		}
 
 		for _, stakeholderGroup := range r.StakeholderGroups {
@@ -120,6 +125,7 @@ func TestMigrationWaveList(t *testing.T) {
 			}
 			assert.Must(t, StakeholderGroup.Create(&expectedStakeholderGroup))
 			createdStakeholderGroups = append(createdStakeholderGroups, expectedStakeholderGroup)
+			r.StakeholderGroups[0].ID = expectedStakeholderGroup.ID
 		}
 		assert.Must(t, MigrationWave.Create(&r))
 		createdMigrationWaves = append(createdMigrationWaves, r)

test/api/migrationwave/samples.go

@@ -13,19 +13,16 @@ var Samples = []api.MigrationWave{
 		EndDate:   time.Date(time.Now().Year(), time.Now().Month(), time.Now().Day(), 0, 0, 0, 0, time.Local).Add(30 * time.Minute),
 		Applications: []api.Ref{
 			{
-				ID:   1,
 				Name: "Sample Application",
 			},
 		},
 		Stakeholders: []api.Ref{
 			{
-				ID:   1,
 				Name: "Sample Stakeholders",
 			},
 		},
 		StakeholderGroups: []api.Ref{
 			{
-				ID:   1,
 				Name: "Sample Stakeholders Groups",
 			},
 		},

test/api/review/api_test.go

@@ -143,7 +143,7 @@ func TestReviewList(t *testing.T) {
 
 	// Delete related reviews and applications.
 	for _, review := range createdReviews {
-		assert.Must(t, Application.Delete(review.ID))
+		assert.Must(t, Application.Delete(review.Application.ID))
 		assert.Must(t, Review.Delete(review.ID))
 	}
 }

test/api/review/samples.go

@@ -12,7 +12,6 @@ var Samples = []api.Review{
 		WorkPriority:        1,
 		Comments:            "nil",
 		Application: &api.Ref{
-			ID:   1,
 			Name: "Sample Review 1",
 		},
 	},
@@ -23,7 +22,6 @@ var Samples = []api.Review{
 		WorkPriority:        2,
 		Comments:            "nil",
 		Application: &api.Ref{
-			ID:   2,
 			Name: "Sample Review 2",
 		},
 	},