Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: Remove CommitStatus.Index field. #14160

Closed
wants to merge 6 commits into from
Closed

WIP: Remove CommitStatus.Index field. #14160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
51fbccd
remove CommitStatus.Index field, and update db index to use ID instea…
Dec 27, 2020
2e1d9cd
remove the fetching and incrementing of the index field
Dec 27, 2020
edea3e1
make the order by use ID instead of Index
Dec 27, 2020
44c0d98
make the API ID use ID instead of Index
Dec 27, 2020
96023c2
remove commit_status.index from fixtures
Dec 27, 2020
35d9a74
Merge branch 'master' into fix_commit_status_index
6543 Dec 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 7 additions & 26 deletions models/commit_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ import (

// CommitStatus holds a single Status of a single Commit
type CommitStatus struct {
ID int64 `xorm:"pk autoincr"`
Index int64 `xorm:"INDEX UNIQUE(repo_sha_index)"`
RepoID int64 `xorm:"INDEX UNIQUE(repo_sha_index)"`
ID int64 `xorm:"pk autoincr INDEX UNIQUE(repo_sha_id)"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no point in adding Id to unique index as all records will be unique anyway, PK already guarantees that. It needs to be rechecked why unique index was set. (Maybe context needs to be added to that uq?)

RepoID int64 `xorm:"INDEX UNIQUE(repo_sha_id)"`
Repo *Repository `xorm:"-"`
State api.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"`
SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_sha_index)"`
SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_sha_id)"`
TargetURL string `xorm:"TEXT"`
Description string `xorm:"TEXT"`
ContextHash string `xorm:"char(40) index"`
Expand Down Expand Up @@ -130,9 +129,9 @@ func sortCommitStatusesSession(sess *xorm.Session, sortType string) {
case "leastupdate":
sess.Asc("updated_unix")
case "leastindex":
sess.Desc("index")
sess.Desc("id")
case "highestindex":
sess.Asc("index")
sess.Asc("id")
default:
sess.Desc("created_unix")
}
Expand Down Expand Up @@ -217,30 +216,12 @@ func NewCommitStatus(opts NewCommitStatusOptions) error {
opts.CommitStatus.CreatorID = opts.Creator.ID
opts.CommitStatus.RepoID = opts.Repo.ID

// Get the next Status Index
var nextIndex int64
lastCommitStatus := &CommitStatus{
SHA: opts.SHA,
RepoID: opts.Repo.ID,
}
has, err := sess.Desc("index").Limit(1).Get(lastCommitStatus)
if err != nil {
if err := sess.Rollback(); err != nil {
log.Error("NewCommitStatus: sess.Rollback: %v", err)
}
return fmt.Errorf("NewCommitStatus[%s, %s]: %v", repoPath, opts.SHA, err)
}
if has {
log.Debug("NewCommitStatus[%s, %s]: found", repoPath, opts.SHA)
nextIndex = lastCommitStatus.Index
}
opts.CommitStatus.Index = nextIndex + 1
log.Debug("NewCommitStatus[%s, %s]: %d", repoPath, opts.SHA, opts.CommitStatus.Index)
log.Debug("NewCommitStatus[%s, %s]", repoPath, opts.SHA)

opts.CommitStatus.ContextHash = hashCommitStatusContext(opts.CommitStatus.Context)

// Insert new CommitStatus
if _, err = sess.Insert(opts.CommitStatus); err != nil {
if _, err := sess.Insert(opts.CommitStatus); err != nil {
if err := sess.Rollback(); err != nil {
log.Error("Insert CommitStatus: sess.Rollback: %v", err)
}
Expand Down
5 changes: 0 additions & 5 deletions models/fixtures/commit_status.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
-
id: 1
index: 1
repo_id: 1
state: "pending"
sha: "1234123412341234123412341234123412341234"
Expand All @@ -11,7 +10,6 @@

-
id: 2
index: 2
repo_id: 1
state: "warning"
sha: "1234123412341234123412341234123412341234"
Expand All @@ -22,7 +20,6 @@

-
id: 3
index: 3
repo_id: 1
state: "success"
sha: "1234123412341234123412341234123412341234"
Expand All @@ -33,7 +30,6 @@

-
id: 4
index: 4
repo_id: 1
state: "failure"
sha: "1234123412341234123412341234123412341234"
Expand All @@ -44,7 +40,6 @@

-
id: 5
index: 5
repo_id: 1
state: "error"
sha: "1234123412341234123412341234123412341234"
Expand Down
2 changes: 1 addition & 1 deletion modules/convert/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func ToCommitStatus(status *models.CommitStatus) *api.CommitStatus {
State: status.State,
TargetURL: status.TargetURL,
Description: status.Description,
ID: status.Index,
ID: status.ID,
URL: status.APIURL(),
Context: status.Context,
}
Expand Down