#3290 better code structure and batch minor improvements

3 files changed, 117 insertions, 78 deletions

diff --git a/models/error.go b/models/error.go
index a6c54ab088..1778701ef6 100644
--- a/models/error.go
+++ b/models/error.go
@@ -5,7 +5,6 @@
 package models
 
 import (
-	"bytes"
 	"fmt"
 )
 
@@ -35,30 +34,6 @@ func (err ErrNamePatternNotAllowed) Error() string {
 	return fmt.Sprintf("name pattern is not allowed [pattern: %s]", err.Pattern)
 }
 
-type ErrMultipleErrors struct {
-	Errors []error
-}
-
-func IsErrMultipleErrors(err error) bool {
-	_, ok := err.(ErrMultipleErrors)
-	return ok
-}
-
-func (err ErrMultipleErrors) Error() string {
-	var message bytes.Buffer
-
-	message.WriteString("Multiple errors encountered: ")
-
-	for i := range err.Errors {
-		message.WriteString(err.Errors[i].Error())
-		if i < len(err.Errors)-1 {
-			message.WriteString("; ")
-		}
-	}
-
-	return message.String()
-}
-
 //  ____ ___
 // |    |   \______ ___________
 // |    |   /  ___// __ \_  __ \
@@ -558,7 +533,8 @@ func (err ErrCommentNotExist) Error() string {
 //         \/    \/    \/     \/
 
 type ErrLabelNotExist struct {
-	ID int64
+	LabelID int64
+	RepoID  int64
 }
 
 func IsErrLabelNotExist(err error) bool {
@@ -567,21 +543,7 @@ func IsErrLabelNotExist(err error) bool {
 }
 
 func (err ErrLabelNotExist) Error() string {
-	return fmt.Sprintf("label does not exist [id: %d]", err.ID)
-}
-
-type ErrLabelNotValidForRepository struct {
-	ID     int64
-	RepoID int64
-}
-
-func IsErrLabelNotValidForRepository(err error) bool {
-	_, ok := err.(ErrLabelNotValidForRepository)
-	return ok
-}
-
-func (err ErrLabelNotValidForRepository) Error() string {
-	return fmt.Sprintf("label is not valid for repository [label_id: %d, repo_id: %d]", err.ID, err.RepoID)
+	return fmt.Sprintf("label does not exist [label_id: %d, repo_id: %d]", err.LabelID, err.RepoID)
 }
 
 //    _____  .__.__                   __
diff --git a/models/issue.go b/models/issue.go
index 9d0f58186e..4a6f9b278a 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -181,7 +181,7 @@ func (i *Issue) addLabel(e *xorm.Session, label *Label) error {
 	return newIssueLabel(e, i, label)
 }
 
-// AddLabel adds new label to issue by given ID.
+// AddLabel adds a new label to the issue.
 func (i *Issue) AddLabel(label *Label) (err error) {
 	sess := x.NewSession()
 	defer sessionRelease(sess)
@@ -196,52 +196,76 @@ func (i *Issue) AddLabel(label *Label) (err error) {
 	return sess.Commit()
 }
 
-func (i *Issue) getLabels(e Engine) (err error) {
-	if len(i.Labels) > 0 {
+func (issue *Issue) addLabels(e *xorm.Session, labels []*Label) error {
+	return newIssueLabels(e, issue, labels)
+}
+
+// AddLabels adds a list of new labels to the issue.
+func (issue *Issue) AddLabels(labels []*Label) error {
+	return NewIssueLabels(issue, labels)
+}
+
+func (issue *Issue) getLabels(e Engine) (err error) {
+	if len(issue.Labels) > 0 {
 		return nil
 	}
 
-	i.Labels, err = getLabelsByIssueID(e, i.ID)
+	issue.Labels, err = getLabelsByIssueID(e, issue.ID)
 	if err != nil {
 		return fmt.Errorf("getLabelsByIssueID: %v", err)
 	}
 	return nil
 }
 
-func (i *Issue) removeLabel(e *xorm.Session, label *Label) error {
-	return deleteIssueLabel(e, i, label)
+func (issue *Issue) removeLabel(e *xorm.Session, label *Label) error {
+	return deleteIssueLabel(e, issue, label)
 }
 
 // RemoveLabel removes a label from issue by given ID.
-func (i *Issue) RemoveLabel(label *Label) (err error) {
+func (issue *Issue) RemoveLabel(label *Label) (err error) {
+	return DeleteIssueLabel(issue, label)
+}
+
+func (issue *Issue) clearLabels(e *xorm.Session) (err error) {
+	if err = issue.getLabels(e); err != nil {
+		return fmt.Errorf("getLabels: %v", err)
+	}
+
+	for i := range issue.Labels {
+		if err = issue.removeLabel(e, issue.Labels[i]); err != nil {
+			return fmt.Errorf("removeLabel: %v", err)
+		}
+	}
+
+	return nil
+}
+
+func (issue *Issue) ClearLabels() (err error) {
 	sess := x.NewSession()
 	defer sessionRelease(sess)
 	if err = sess.Begin(); err != nil {
 		return err
 	}
 
-	if err = i.removeLabel(sess, label); err != nil {
+	if err = issue.clearLabels(sess); err != nil {
 		return err
 	}
 
 	return sess.Commit()
 }
 
-func (i *Issue) ClearLabels() (err error) {
+// ReplaceLabels removes all current labels and add new labels to the issue.
+func (issue *Issue) ReplaceLabels(labels []*Label) (err error) {
 	sess := x.NewSession()
 	defer sessionRelease(sess)
 	if err = sess.Begin(); err != nil {
 		return err
 	}
 
-	if err = i.getLabels(sess); err != nil {
-		return err
-	}
-
-	for idx := range i.Labels {
-		if err = i.removeLabel(sess, i.Labels[idx]); err != nil {
-			return err
-		}
+	if err = issue.clearLabels(sess); err != nil {
+		return fmt.Errorf("clearLabels: %v", err)
+	} else if err = issue.addLabels(sess, labels); err != nil {
+		return fmt.Errorf("addLabels: %v", err)
 	}
 
 	return sess.Commit()
diff --git a/models/issue_label.go b/models/issue_label.go
index 4f814821f8..ed9b616011 100644
--- a/models/issue_label.go
+++ b/models/issue_label.go
@@ -11,6 +11,8 @@ import (
 	"strings"
 
 	"github.com/go-xorm/xorm"
+
+	"github.com/gogits/gogs/modules/base"
 )
 
 // Label represents a label of repository for issues.
@@ -56,48 +58,65 @@ func NewLabel(l *Label) error {
 	return err
 }
 
-func getLabelByID(e Engine, id int64) (*Label, error) {
-	if id <= 0 {
-		return nil, ErrLabelNotExist{id}
+// getLabelInRepoByID returns a label by ID in given repository.
+// If pass repoID as 0, then ORM will ignore limitation of repository
+// and can return arbitrary label with any valid ID.
+func getLabelInRepoByID(e Engine, repoID, labelID int64) (*Label, error) {
+	if labelID <= 0 {
+		return nil, ErrLabelNotExist{labelID, repoID}
 	}
 
-	l := &Label{ID: id}
+	l := &Label{
+		ID:     labelID,
+		RepoID: repoID,
+	}
 	has, err := x.Get(l)
 	if err != nil {
 		return nil, err
 	} else if !has {
-		return nil, ErrLabelNotExist{l.ID}
+		return nil, ErrLabelNotExist{l.ID, l.RepoID}
 	}
 	return l, nil
 }
 
 // GetLabelByID returns a label by given ID.
 func GetLabelByID(id int64) (*Label, error) {
-	return getLabelByID(x, id)
+	return getLabelInRepoByID(x, 0, id)
+}
+
+// GetLabelInRepoByID returns a label by ID in given repository.
+func GetLabelInRepoByID(repoID, labelID int64) (*Label, error) {
+	return getLabelInRepoByID(x, repoID, labelID)
+}
+
+// GetLabelsInRepoByIDs returns a list of labels by IDs in given repository,
+// it silently ignores label IDs that are not belong to the repository.
+func GetLabelsInRepoByIDs(repoID int64, labelIDs []int64) ([]*Label, error) {
+	labels := make([]*Label, 0, len(labelIDs))
+	return labels, x.Where("repo_id = ?", repoID).In("id", base.Int64sToStrings(labelIDs)).Find(&labels)
 }
 
 // GetLabelsByRepoID returns all labels that belong to given repository by ID.
 func GetLabelsByRepoID(repoID int64) ([]*Label, error) {
 	labels := make([]*Label, 0, 10)
-	return labels, x.Where("repo_id=?", repoID).Find(&labels)
+	return labels, x.Where("repo_id = ?", repoID).Find(&labels)
 }
 
 func getLabelsByIssueID(e Engine, issueID int64) ([]*Label, error) {
 	issueLabels, err := getIssueLabels(e, issueID)
 	if err != nil {
 		return nil, fmt.Errorf("getIssueLabels: %v", err)
+	} else if len(issueLabels) == 0 {
+		return []*Label{}, nil
 	}
 
-	var label *Label
-	labels := make([]*Label, 0, len(issueLabels))
-	for idx := range issueLabels {
-		label, err = getLabelByID(e, issueLabels[idx].LabelID)
-		if err != nil && !IsErrLabelNotExist(err) {
-			return nil, fmt.Errorf("getLabelByID: %v", err)
-		}
-		labels = append(labels, label)
+	labelIDs := make([]int64, len(issueLabels))
+	for i := range issueLabels {
+		labelIDs[i] = issueLabels[i].LabelID
 	}
-	return labels, nil
+
+	labels := make([]*Label, 0, len(labelIDs))
+	return labels, e.Where("id > 0").In("id", base.Int64sToStrings(labelIDs)).Find(&labels)
 }
 
 // GetLabelsByIssueID returns all labels that belong to given issue by ID.
@@ -117,7 +136,7 @@ func UpdateLabel(l *Label) error {
 
 // DeleteLabel delete a label of given repository.
 func DeleteLabel(repoID, labelID int64) error {
-	l, err := GetLabelByID(labelID)
+	_, err := GetLabelInRepoByID(repoID, labelID)
 	if err != nil {
 		if IsErrLabelNotExist(err) {
 			return nil
@@ -131,11 +150,12 @@ func DeleteLabel(repoID, labelID int64) error {
 		return err
 	}
 
-	if _, err = x.Where("label_id=?", labelID).Delete(new(IssueLabel)); err != nil {
+	if _, err = sess.Id(labelID).Delete(new(Label)); err != nil {
 		return err
-	} else if _, err = sess.Delete(l); err != nil {
+	} else if _, err = sess.Where("label_id = ?", labelID).Delete(new(IssueLabel)); err != nil {
 		return err
 	}
+
 	return sess.Commit()
 }
 
@@ -154,7 +174,7 @@ type IssueLabel struct {
 }
 
 func hasIssueLabel(e Engine, issueID, labelID int64) bool {
-	has, _ := e.Where("issue_id=? AND label_id=?", issueID, labelID).Get(new(IssueLabel))
+	has, _ := e.Where("issue_id = ? AND label_id = ?", issueID, labelID).Get(new(IssueLabel))
 	return has
 }
 
@@ -180,6 +200,10 @@ func newIssueLabel(e *xorm.Session, issue *Issue, label *Label) (err error) {
 
 // NewIssueLabel creates a new issue-label relation.
 func NewIssueLabel(issue *Issue, label *Label) (err error) {
+	if HasIssueLabel(issue.ID, label.ID) {
+		return nil
+	}
+
 	sess := x.NewSession()
 	defer sessionRelease(sess)
 	if err = sess.Begin(); err != nil {
@@ -193,6 +217,35 @@ func NewIssueLabel(issue *Issue, label *Label) (err error) {
 	return sess.Commit()
 }
 
+func newIssueLabels(e *xorm.Session, issue *Issue, labels []*Label) (err error) {
+	for i := range labels {
+		if hasIssueLabel(e, issue.ID, labels[i].ID) {
+			continue
+		}
+
+		if err = newIssueLabel(e, issue, labels[i]); err != nil {
+			return fmt.Errorf("newIssueLabel: %v", err)
+		}
+	}
+
+	return nil
+}
+
+// NewIssueLabels creates a list of issue-label relations.
+func NewIssueLabels(issue *Issue, labels []*Label) (err error) {
+	sess := x.NewSession()
+	defer sessionRelease(sess)
+	if err = sess.Begin(); err != nil {
+		return err
+	}
+
+	if err = newIssueLabels(sess, issue, labels); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}
+
 func getIssueLabels(e Engine, issueID int64) ([]*IssueLabel, error) {
 	issueLabels := make([]*IssueLabel, 0, 10)
 	return issueLabels, e.Where("issue_id=?", issueID).Asc("label_id").Find(&issueLabels)