Merge pull request #98 from rhmdnd/add-fk-to-subject-table

Implement a foreign key for subjects

migrations/000003_add_subject_parent_id.down.sql

@@ -0,0 +1,3 @@
+ALTER TABLE subjects DROP CONSTRAINT fk_subjects_parent_id;
+
+ALTER TABLE subjects DROP COLUMN parent_id;

migrations/000003_add_subject_parent_id.up.sql

@@ -0,0 +1,5 @@
+ALTER TABLE subjects
+ADD COLUMN parent_id UUID;
+
+ALTER TABLE subjects
+ADD CONSTRAINT fk_subjects_parent_id FOREIGN KEY (parent_id) REFERENCES subjects (id);

tests/integration_test.go

@@ -1,6 +1,7 @@
 package tests // nolint:testpackage
 
 import (
+	"database/sql"
 	"fmt"
 	"strings"
 	"testing"
@@ -34,6 +35,10 @@ func TestInsertSubjectSucceeds(t *testing.T) { // nolint:paralleltest // databas
 	assert.Equal(t, expectedSubject.ID, subject.ID, "expected %s got %s", expectedSubject.ID, subject.ID)
 	assert.Equal(t, expectedSubject.Name, subject.Name, "expected %s got %s", expectedSubject.Name, subject.Name)
 	assert.Equal(t, expectedSubject.Type, subject.Type, "expected %s got %s", expectedSubject.Type, subject.Type)
+	parentIDValue, err := subject.ParentID.Value()
+	assert.False(t, subject.ParentID.Valid)
+	assert.Nil(t, err)
+	assert.Nil(t, parentIDValue)
 
 	// Drop the database instead of downgrading since we don't need the
 	// data anyway
@@ -178,7 +183,121 @@ func TestInsertMetadataSucceeds(t *testing.T) { // nolint:paralleltest // databa
 	assert.Equal(t, e.UpdatedAt, a.UpdatedAt, "expected %s got %s", e.UpdatedAt, a.UpdatedAt)
 	assert.Equal(t, e.Version, a.Version, "expected %s got %s", e.Version, a.Version)
 	assert.Equal(t, e.Description, a.Description, "expected %s got %s", e.Description, a.Description)
+}
+
+func TestInsertSubjectWithParent(t *testing.T) { // nolint:paralleltest // database tests should run serially
+	m := getMigrationHelper(t)
+	if err := m.Up(); err != nil {
+		t.Fatalf("Unable to upgrade database: %s", err)
+	}
+
+	// Create a subject to act as the parent
+	parentID := getUUIDString()
+	subjectTypeStr := getUUIDString()
+	p := Subject{ID: parentID, Name: clusterName, Type: subjectTypeStr}
+	gormDB := getGormHelper()
+	gormDB.Create(&p)
+
+	// Create another subject referencing the parent
+	id := getUUIDString()
+	subjectTypeStr = getUUIDString()
+	s := Subject{ID: id, Name: clusterName, Type: subjectTypeStr, ParentID: sql.NullString{String: parentID, Valid: true}}
+	gormDB.Create(&s)
+
+	a := Subject{}
+	gormDB.First(&a, "id = ?", id)
+
+	e := Subject{ID: id, Name: clusterName, Type: subjectTypeStr, ParentID: sql.NullString{String: parentID, Valid: true}}
+	assert.Equal(t, e.ID, a.ID, "expected %s got %s", e.ID, a.ID)
+	assert.Equal(t, e.Name, a.Name, "expected %s got %s", e.Name, a.Name)
+	assert.Equal(t, e.Type, a.Type, "expected %s got %s", e.Type, a.Type)
+	assert.True(t, a.ParentID.Valid)
+	assert.Equal(t, e.ParentID.String, a.ParentID.String, "expected %s got %s", e.ParentID.String, a.ParentID.String)
+
+	// Drop the database instead of downgrading since we don't need the
+	// data anyway
+	if err := m.Drop(); err != nil {
+		t.Fatalf("Unable to drop database: %s", err)
+	}
+}
 
+func TestDeleteSubjectWithParent(t *testing.T) { // nolint:paralleltest // database tests should run serially
+	m := getMigrationHelper(t)
+	if err := m.Up(); err != nil {
+		t.Fatalf("Unable to upgrade database: %s", err)
+	}
+
+	// Create a subject to act as the parent
+	parentID := getUUIDString()
+	subjectTypeStr := getUUIDString()
+	p := Subject{ID: parentID, Name: clusterName, Type: subjectTypeStr}
+	gormDB := getGormHelper()
+	err := gormDB.Create(&p).Error
+	assert.Nil(t, err)
+
+	// Create another subject referencing the parent
+	id := getUUIDString()
+	subjectTypeStr = getUUIDString()
+	s := Subject{ID: id, Name: clusterName, Type: subjectTypeStr, ParentID: sql.NullString{String: parentID, Valid: true}}
+	err = gormDB.Create(&s).Error
+	assert.Nil(t, err)
+
+	a := Subject{}
+	gormDB.First(&a, "id = ?", id)
+
+	e := Subject{ID: id, Name: clusterName, Type: subjectTypeStr, ParentID: sql.NullString{String: parentID, Valid: true}}
+	assert.Equal(t, e.ID, a.ID, "expected %s got %s", e.ID, a.ID)
+	assert.Equal(t, e.Name, a.Name, "expected %s got %s", e.Name, a.Name)
+	assert.Equal(t, e.Type, a.Type, "expected %s got %s", e.Type, a.Type)
+	assert.True(t, a.ParentID.Valid)
+	assert.Equal(t, e.ParentID.String, a.ParentID.String, "expected %s got %s", e.ParentID.String, a.ParentID.String)
+
+	err = gormDB.Delete(&p).Error
+	assert.NotEmpty(t, err, "Deleting the parent subject should fail if the child subject still exists")
+
+	// There should still be two subjects in the table
+	var subjects []Subject
+	result := gormDB.Find(&subjects)
+	expectedSubjects := int64(2)
+	assert.Equal(t, expectedSubjects, result.RowsAffected, "expected %d got %d", expectedSubjects, result.RowsAffected)
+
+	// Deleting the child subject first should allow us to delete the
+	// parent subject after since the foreign key isn't violated
+	err = gormDB.Delete(&s).Error
+	assert.Nil(t, err, "Failed to delete the child subject")
+	err = gormDB.Delete(&p).Error
+	assert.Nil(t, err, "Failed to delete the parent subject")
+
+	result = gormDB.Find(&subjects)
+	expectedSubjects = int64(0)
+	assert.Equal(t, expectedSubjects, result.RowsAffected, "expected %d got %d", expectedSubjects, result.RowsAffected)
+
+	// Drop the database instead of downgrading since we don't need the
+	// data anyway
+	if err := m.Drop(); err != nil {
+		t.Fatalf("Unable to drop database: %s", err)
+	}
+}
+
+func TestInsertSubjectWithNonExistentParent(t *testing.T) { // nolint:paralleltest // database tests should run serially
+	m := getMigrationHelper(t)
+	if err := m.Up(); err != nil {
+		t.Fatalf("Unable to upgrade database: %s", err)
+	}
+
+	// Generate a fake parent ID that doesn't actually exist
+	parentID := getUUIDString()
+
+	// Create another subject referencing the parent
+	id := getUUIDString()
+	subjectTypeStr := getUUIDString()
+	s := Subject{ID: id, Name: clusterName, Type: subjectTypeStr, ParentID: sql.NullString{String: parentID, Valid: true}}
+	gormDB := getGormHelper()
+	err := gormDB.Create(&s).Error
+	assert.NotEmpty(t, err, "Shouldn't be able to insert values that violate foreign key constraints")
+
+	// Drop the database instead of downgrading since we don't need the
+	// data anyway
 	if err := m.Drop(); err != nil {
 		t.Fatalf("Unable to drop database: %s", err)
 	}
@@ -202,7 +321,7 @@ func TestMigration(t *testing.T) { // nolint:paralleltest // database tests shou
 	// Upgrade the database and make sure all upgrades apply cleanly.
 	err = m.Up()
 	version, dirty, _ = m.Version()
-	expectedVersion = uint(2)
+	expectedVersion = uint(3)
 	assert.Equal(t, expectedVersion, version, "Database version mismatch: want %d but got %d", expectedVersion, version)
 	assert.Equal(t, false, dirty, "Database state mismatch: want %t but got %t", false, dirty)
 	assert.Equal(t, err, nil, "Error upgrading the database: %s", err)
@@ -215,3 +334,52 @@ func TestMigration(t *testing.T) { // nolint:paralleltest // database tests shou
 	assert.Equal(t, false, dirty, "Database state mismatch: want %t but got %t", false, dirty)
 	assert.Equal(t, err, nil, "Error downgrading the database: %s", err)
 }
+
+func TestSubjectParentIDMigration(t *testing.T) { // nolint:paralleltest // database tests should run serially
+	m := getMigrationHelper(t)
+	gormDB := getGormHelper()
+
+	type subject struct{}
+	tableName := "subjects"
+	columnName := "parent_id"
+
+	if err := m.Migrate(2); err != nil {
+		t.Fatalf("Unable to migrate to version %d: %s", 1, err)
+	}
+	result := gormDB.Migrator().HasTable(tableName)
+	assert.True(t, result, "Table doesn't exist: %s", tableName)
+
+	for _, s := range []string{"id", "name", "type"} {
+		result = gormDB.Migrator().HasColumn(&subject{}, s)
+		assert.True(t, result, "Table doesn't have column: %s", s)
+	}
+	result = gormDB.Migrator().HasColumn(&subject{}, columnName)
+	assert.False(t, result, "Table has column: %s", columnName)
+
+	// Ensure the upgrade adds the parent_id column and the constraint
+	if err := m.Migrate(3); err != nil {
+		t.Fatalf("Unable to migrate to version %d: %s", 2, err)
+	}
+	result = gormDB.Migrator().HasTable(tableName)
+	assert.True(t, result, "Table doesn't exist: %s", tableName)
+
+	for _, s := range []string{"id", "name", "type", "parent_id"} {
+		result = gormDB.Migrator().HasColumn(&subject{}, s)
+		assert.True(t, result, "Table doesn't have column: %s", s)
+	}
+
+	constraintName := "fk_subjects_parent_id"
+	result = gormDB.Migrator().HasConstraint(&subject{}, constraintName)
+	assert.True(t, result, "Table doesn't have constraint: %s", constraintName)
+
+	// Make sure the downgrade removes the parent_id column and the
+	// constraint
+	if err := m.Migrate(2); err != nil {
+		t.Fatalf("Unable to migrate to version %d: %s", 1, err)
+	}
+	result = gormDB.Migrator().HasColumn(&subject{}, columnName)
+	assert.False(t, result, "Table has column: %s", columnName)
+
+	result = gormDB.Migrator().HasConstraint(&subject{}, constraintName)
+	assert.False(t, result, "Table has constraint: %s", constraintName)
+}

tests/utils.go

@@ -24,9 +24,10 @@ type Metadata struct {
 }
 
 type Subject struct {
-	ID   string
-	Name string
-	Type string
+	ID       string
+	Name     string
+	Type     string
+	ParentID sql.NullString
 }
 
 func getDatabaseConnection(t *testing.T) *sql.DB {