Check primary keys for all tables and drop ForeignReference (#21721)

Some dbs require that all tables have primary keys, see
- #16802
- #21086

We can add a test to keep it from being broken again.

Edit:

~Added missing primary key for `ForeignReference`~ Dropped the
`ForeignReference` table to satisfy the check, so it closes #21086.

More context can be found in comments.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: zeripath <art27@cantab.net>

2 files changed, 5 insertions, 89 deletions

diff --git a/models/issues/issue.go b/models/issues/issue.go
index fc93fcf454..f45e635c0e 100644
--- a/models/issues/issue.go
+++ b/models/issues/issue.go
@@ -9,11 +9,9 @@ import (
 	"fmt"
 	"regexp"
 	"sort"
-	"strconv"
 	"strings"
 
 	"code.gitea.io/gitea/models/db"
-	"code.gitea.io/gitea/models/foreignreference"
 	"code.gitea.io/gitea/models/organization"
 	"code.gitea.io/gitea/models/perm"
 	access_model "code.gitea.io/gitea/models/perm/access"
@@ -136,12 +134,11 @@ type Issue struct {
 	UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
 	ClosedUnix  timeutil.TimeStamp `xorm:"INDEX"`
 
-	Attachments      []*repo_model.Attachment           `xorm:"-"`
-	Comments         []*Comment                         `xorm:"-"`
-	Reactions        ReactionList                       `xorm:"-"`
-	TotalTrackedTime int64                              `xorm:"-"`
-	Assignees        []*user_model.User                 `xorm:"-"`
-	ForeignReference *foreignreference.ForeignReference `xorm:"-"`
+	Attachments      []*repo_model.Attachment `xorm:"-"`
+	Comments         []*Comment               `xorm:"-"`
+	Reactions        ReactionList             `xorm:"-"`
+	TotalTrackedTime int64                    `xorm:"-"`
+	Assignees        []*user_model.User       `xorm:"-"`
 
 	// IsLocked limits commenting abilities to users on an issue
 	// with write access
@@ -321,29 +318,6 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) {
 	return nil
 }
 
-func (issue *Issue) loadForeignReference(ctx context.Context) (err error) {
-	if issue.ForeignReference != nil {
-		return nil
-	}
-	reference := &foreignreference.ForeignReference{
-		RepoID:     issue.RepoID,
-		LocalIndex: issue.Index,
-		Type:       foreignreference.TypeIssue,
-	}
-	has, err := db.GetEngine(ctx).Get(reference)
-	if err != nil {
-		return err
-	} else if !has {
-		return foreignreference.ErrForeignIndexNotExist{
-			RepoID:     issue.RepoID,
-			LocalIndex: issue.Index,
-			Type:       foreignreference.TypeIssue,
-		}
-	}
-	issue.ForeignReference = reference
-	return nil
-}
-
 // LoadMilestone load milestone of this issue.
 func (issue *Issue) LoadMilestone(ctx context.Context) (err error) {
 	if (issue.Milestone == nil || issue.Milestone.ID != issue.MilestoneID) && issue.MilestoneID > 0 {
@@ -406,10 +380,6 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
 		}
 	}
 
-	if err = issue.loadForeignReference(ctx); err != nil && !foreignreference.IsErrForeignIndexNotExist(err) {
-		return err
-	}
-
 	return issue.loadReactions(ctx)
 }
 
@@ -1097,26 +1067,6 @@ func GetIssueByIndex(repoID, index int64) (*Issue, error) {
 	return issue, nil
 }
 
-// GetIssueByForeignIndex returns raw issue by foreign ID
-func GetIssueByForeignIndex(ctx context.Context, repoID, foreignIndex int64) (*Issue, error) {
-	reference := &foreignreference.ForeignReference{
-		RepoID:       repoID,
-		ForeignIndex: strconv.FormatInt(foreignIndex, 10),
-		Type:         foreignreference.TypeIssue,
-	}
-	has, err := db.GetEngine(ctx).Get(reference)
-	if err != nil {
-		return nil, err
-	} else if !has {
-		return nil, foreignreference.ErrLocalIndexNotExist{
-			RepoID:       repoID,
-			ForeignIndex: foreignIndex,
-			Type:         foreignreference.TypeIssue,
-		}
-	}
-	return GetIssueByIndex(repoID, reference.LocalIndex)
-}
-
 // GetIssueWithAttrsByIndex returns issue by index in a repository.
 func GetIssueWithAttrsByIndex(repoID, index int64) (*Issue, error) {
 	issue, err := GetIssueByIndex(repoID, index)
diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go
index 6764a9e626..de1da19ab9 100644
--- a/models/issues/issue_test.go
+++ b/models/issues/issue_test.go
@@ -7,13 +7,11 @@ import (
 	"context"
 	"fmt"
 	"sort"
-	"strconv"
 	"sync"
 	"testing"
 	"time"
 
 	"code.gitea.io/gitea/models/db"
-	"code.gitea.io/gitea/models/foreignreference"
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/organization"
 	repo_model "code.gitea.io/gitea/models/repo"
@@ -501,38 +499,6 @@ func TestCorrectIssueStats(t *testing.T) {
 	assert.EqualValues(t, issueStats.OpenCount, issueAmount)
 }
 
-func TestIssueForeignReference(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 4})
-	assert.NotEqualValues(t, issue.Index, issue.ID) // make sure they are different to avoid false positive
-
-	// it is fine for an issue to not have a foreign reference
-	err := issue.LoadAttributes(db.DefaultContext)
-	assert.NoError(t, err)
-	assert.Nil(t, issue.ForeignReference)
-
-	var foreignIndex int64 = 12345
-	_, err = issues_model.GetIssueByForeignIndex(context.Background(), issue.RepoID, foreignIndex)
-	assert.True(t, foreignreference.IsErrLocalIndexNotExist(err))
-
-	err = db.Insert(db.DefaultContext, &foreignreference.ForeignReference{
-		LocalIndex:   issue.Index,
-		ForeignIndex: strconv.FormatInt(foreignIndex, 10),
-		RepoID:       issue.RepoID,
-		Type:         foreignreference.TypeIssue,
-	})
-	assert.NoError(t, err)
-
-	err = issue.LoadAttributes(db.DefaultContext)
-	assert.NoError(t, err)
-
-	assert.EqualValues(t, issue.ForeignReference.ForeignIndex, strconv.FormatInt(foreignIndex, 10))
-
-	found, err := issues_model.GetIssueByForeignIndex(context.Background(), issue.RepoID, foreignIndex)
-	assert.NoError(t, err)
-	assert.EqualValues(t, found.Index, issue.Index)
-}
-
 func TestMilestoneList_LoadTotalTrackedTimes(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	miles := issues_model.MilestoneList{