OneDev migration: fix broken migration caused by various REST API changes in OneDev 7.8.0 and later (#35216)

OneDev migration: fix broken migration caused by various REST API
changes in OneDev 7.8.0 and later

- in REST urls use `~api` instead of `api`
- check minimum required OneDev version before starting migration
- required OneDev version is now 12.0.1
(older versions do not offer necessary API:
https://code.onedev.io/onedev/server/~issues/2491)
- support migrating OneDev subprojects (e.g.
http:/onedev.host/projectA/subProjectB)
- set milestone closed state if milestone is closed in OneDev
- moved memory allocation for milestone JSON decoding into for loop
(which gets 100 milestones per iteration) to fix wrong due dates when
having more than 100 milestones

services/migrations/onedev.go

@@ -6,6 +6,7 @@ package migrations
 import (
 	"context"
 	"fmt"
+	"io"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -16,8 +17,12 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	base "code.gitea.io/gitea/modules/migration"
 	"code.gitea.io/gitea/modules/structs"
+
+	"github.com/hashicorp/go-version"
 )
 
+const OneDevRequiredVersion = "12.0.1"
+
 var (
 	_ base.Downloader        = &OneDevDownloader{}
 	_ base.DownloaderFactory = &OneDevDownloaderFactory{}
@@ -37,23 +42,14 @@ func (f *OneDevDownloaderFactory) New(ctx context.Context, opts base.MigrateOpti
 		return nil, err
 	}
 
-	var repoName string
+	repoPath := strings.Trim(u.Path, "/")
-
-	fields := strings.Split(strings.Trim(u.Path, "/"), "/")
-	if len(fields) == 2 && fields[0] == "projects" {
-		repoName = fields[1]
-	} else if len(fields) == 1 {
-		repoName = fields[0]
-	} else {
-		return nil, fmt.Errorf("invalid path: %s", u.Path)
-	}
 
 	u.Path = ""
 	u.Fragment = ""
 
-	log.Trace("Create onedev downloader. BaseURL: %v RepoName: %s", u, repoName)
+	log.Trace("Create onedev downloader. BaseURL: %v RepoPath: %s", u, repoPath)
 
-	return NewOneDevDownloader(ctx, u, opts.AuthUsername, opts.AuthPassword, repoName), nil
+	return NewOneDevDownloader(ctx, u, opts.AuthUsername, opts.AuthPassword, repoPath), nil
 }
 
 // GitServiceType returns the type of git service
@@ -62,9 +58,9 @@ func (f *OneDevDownloaderFactory) GitServiceType() structs.GitServiceType {
 }
 
 type onedevUser struct {
-	ID    int64  `json:"id"`
+	ID    int64
-	Name  string `json:"name"`
+	Name  string
-	Email string `json:"email"`
+	Email string
 }
 
 // OneDevDownloader implements a Downloader interface to get repository information
@@ -73,7 +69,7 @@ type OneDevDownloader struct {
 	base.NullDownloader
 	client        *http.Client
 	baseURL       *url.URL
-	repoName      string
+	repoPath      string
 	repoID        int64
 	maxIssueIndex int64
 	userMap       map[int64]*onedevUser
@@ -81,10 +77,10 @@ type OneDevDownloader struct {
 }
 
 // NewOneDevDownloader creates a new downloader
-func NewOneDevDownloader(_ context.Context, baseURL *url.URL, username, password, repoName string) *OneDevDownloader {
+func NewOneDevDownloader(_ context.Context, baseURL *url.URL, username, password, repoPath string) *OneDevDownloader {
 	downloader := &OneDevDownloader{
 		baseURL:  baseURL,
-		repoName: repoName,
+		repoPath: repoPath,
 		client: &http.Client{
 			Transport: &http.Transport{
 				Proxy: func(req *http.Request) (*url.URL, error) {
@@ -104,14 +100,14 @@ func NewOneDevDownloader(_ context.Context, baseURL *url.URL, username, password
 
 // String implements Stringer
 func (d *OneDevDownloader) String() string {
-	return fmt.Sprintf("migration from oneDev server %s [%d]/%s", d.baseURL, d.repoID, d.repoName)
+	return fmt.Sprintf("migration from oneDev server %s [%d]/%s", d.baseURL, d.repoID, d.repoPath)
 }
 
 func (d *OneDevDownloader) LogString() string {
 	if d == nil {
 		return "<OneDevDownloader nil>"
 	}
-	return fmt.Sprintf("<OneDevDownloader %s [%d]/%s>", d.baseURL, d.repoID, d.repoName)
+	return fmt.Sprintf("<OneDevDownloader %s [%d]/%s>", d.baseURL, d.repoID, d.repoPath)
 }
 
 func (d *OneDevDownloader) callAPI(ctx context.Context, endpoint string, parameter map[string]string, result any) error {
@@ -139,23 +135,54 @@ func (d *OneDevDownloader) callAPI(ctx context.Context, endpoint string, paramet
 	}
 	defer resp.Body.Close()
 
+	// special case to read OneDev server version, which is not valid JSON
+	if presult, ok := result.(**version.Version); ok {
+		bytes, err := io.ReadAll(resp.Body)
+		if err != nil {
+			return err
+		}
+		vers, err := version.NewVersion(string(bytes))
+		if err != nil {
+			return err
+		}
+		*presult = vers
+		return nil
+	}
+
 	decoder := json.NewDecoder(resp.Body)
 	return decoder.Decode(&result)
 }
 
 // GetRepoInfo returns repository information
 func (d *OneDevDownloader) GetRepoInfo(ctx context.Context) (*base.Repository, error) {
+	// check OneDev server version
+	var serverVersion *version.Version
+	err := d.callAPI(
+		ctx,
+		"/~api/version/server",
+		nil,
+		&serverVersion,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get OneDev server version; OneDev %s or newer required", OneDevRequiredVersion)
+	}
+	requiredVersion, _ := version.NewVersion(OneDevRequiredVersion)
+	if serverVersion.LessThan(requiredVersion) {
+		return nil, fmt.Errorf("OneDev %s or newer required; currently running OneDev %s", OneDevRequiredVersion, serverVersion)
+	}
+
 	info := make([]struct {
 		ID          int64  `json:"id"`
 		Name        string `json:"name"`
+		Path        string `json:"path"`
 		Description string `json:"description"`
 	}, 0, 1)
 
-	err := d.callAPI(
+	err = d.callAPI(
 		ctx,
-		"/api/projects",
+		"/~api/projects",
 		map[string]string{
-			"query":  `"Name" is "` + d.repoName + `"`,
+			"query":  `"Path" is "` + d.repoPath + `"`,
 			"offset": "0",
 			"count":  "1",
 		},
@@ -165,16 +192,12 @@ func (d *OneDevDownloader) GetRepoInfo(ctx context.Context) (*base.Repository, e
 		return nil, err
 	}
 	if len(info) != 1 {
-		return nil, fmt.Errorf("Project %s not found", d.repoName)
+		return nil, fmt.Errorf("Project %s not found", d.repoPath)
 	}
 
 	d.repoID = info[0].ID
 
-	cloneURL, err := d.baseURL.Parse(info[0].Name)
+	cloneURL, err := d.baseURL.Parse(info[0].Path)
-	if err != nil {
-		return nil, err
-	}
-	originalURL, err := d.baseURL.Parse("/projects/" + info[0].Name)
 	if err != nil {
 		return nil, err
 	}
@@ -183,25 +206,25 @@ func (d *OneDevDownloader) GetRepoInfo(ctx context.Context) (*base.Repository, e
 		Name:        info[0].Name,
 		Description: info[0].Description,
 		CloneURL:    cloneURL.String(),
-		OriginalURL: originalURL.String(),
+		OriginalURL: cloneURL.String(),
 	}, nil
 }
 
 // GetMilestones returns milestones
 func (d *OneDevDownloader) GetMilestones(ctx context.Context) ([]*base.Milestone, error) {
-	rawMilestones := make([]struct {
+	endpoint := fmt.Sprintf("/~api/projects/%d/iterations", d.repoID)
-		ID          int64      `json:"id"`
-		Name        string     `json:"name"`
-		Description string     `json:"description"`
-		DueDate     *time.Time `json:"dueDate"`
-		Closed      bool       `json:"closed"`
-	}, 0, 100)
-
-	endpoint := fmt.Sprintf("/api/projects/%d/milestones", d.repoID)
 
 	milestones := make([]*base.Milestone, 0, 100)
 	offset := 0
 	for {
+		rawMilestones := make([]struct {
+			ID          int64  `json:"id"`
+			Name        string `json:"name"`
+			Description string `json:"description"`
+			DueDay      int64  `json:"dueDay"`
+			Closed      bool   `json:"closed"`
+		}, 0, 100)
+
 		err := d.callAPI(
 			ctx,
 			endpoint,
@@ -221,16 +244,26 @@ func (d *OneDevDownloader) GetMilestones(ctx context.Context) ([]*base.Milestone
 
 		for _, milestone := range rawMilestones {
 			d.milestoneMap[milestone.ID] = milestone.Name
-			closed := milestone.DueDate
+
-			if !milestone.Closed {
+			var dueDate *time.Time
-				closed = nil
+			if milestone.DueDay != 0 {
+				d := time.Unix(milestone.DueDay*24*60*60, 0)
+				dueDate = &d
+			}
+
+			var closedDate *time.Time
+			state := "open"
+			if milestone.Closed {
+				closedDate = dueDate
+				state = "closed"
 			}
 
 			milestones = append(milestones, &base.Milestone{
 				Title:       milestone.Name,
 				Description: milestone.Description,
-				Deadline:    milestone.DueDate,
+				Deadline:    dueDate,
-				Closed:      closed,
+				Closed:      closedDate,
+				State:       state,
 			})
 		}
 	}
@@ -273,6 +306,10 @@ type onedevIssueContext struct {
 
 // GetIssues returns issues
 func (d *OneDevDownloader) GetIssues(ctx context.Context, page, perPage int) ([]*base.Issue, bool, error) {
+	type Field struct {
+		Name  string `json:"name"`
+		Value string `json:"value"`
+	}
 	rawIssues := make([]struct {
 		ID          int64     `json:"id"`
 		Number      int64     `json:"number"`
@@ -281,15 +318,17 @@ func (d *OneDevDownloader) GetIssues(ctx context.Context, page, perPage int) ([]
 		Description string    `json:"description"`
 		SubmitterID int64     `json:"submitterId"`
 		SubmitDate  time.Time `json:"submitDate"`
+		Fields      []Field   `json:"fields"`
 	}, 0, perPage)
 
 	err := d.callAPI(
 		ctx,
-		"/api/issues",
+		"/~api/issues",
 		map[string]string{
-			"query":  `"Project" is "` + d.repoName + `"`,
+			"query":      `"Project" is "` + d.repoPath + `"`,
-			"offset": strconv.Itoa((page - 1) * perPage),
+			"offset":     strconv.Itoa((page - 1) * perPage),
-			"count":  strconv.Itoa(perPage),
+			"count":      strconv.Itoa(perPage),
+			"withFields": "true",
 		},
 		&rawIssues,
 	)
@@ -299,22 +338,8 @@ func (d *OneDevDownloader) GetIssues(ctx context.Context, page, perPage int) ([]
 
 	issues := make([]*base.Issue, 0, len(rawIssues))
 	for _, issue := range rawIssues {
-		fields := make([]struct {
-			Name  string `json:"name"`
-			Value string `json:"value"`
-		}, 0, 10)
-		err := d.callAPI(
-			ctx,
-			fmt.Sprintf("/api/issues/%d/fields", issue.ID),
-			nil,
-			&fields,
-		)
-		if err != nil {
-			return nil, false, err
-		}
-
 		var label *base.Label
-		for _, field := range fields {
+		for _, field := range issue.Fields {
 			if field.Name == "Type" {
 				label = &base.Label{Name: field.Value}
 				break
@@ -327,7 +352,7 @@ func (d *OneDevDownloader) GetIssues(ctx context.Context, page, perPage int) ([]
 		}, 0, 10)
 		err = d.callAPI(
 			ctx,
-			fmt.Sprintf("/api/issues/%d/milestones", issue.ID),
+			fmt.Sprintf("/~api/issues/%d/iterations", issue.ID),
 			nil,
 			&milestones,
 		)
@@ -383,9 +408,9 @@ func (d *OneDevDownloader) GetComments(ctx context.Context, commentable base.Com
 
 	var endpoint string
 	if context.IsPullRequest {
-		endpoint = fmt.Sprintf("/api/pull-requests/%d/comments", commentable.GetForeignIndex())
+		endpoint = fmt.Sprintf("/~api/pulls/%d/comments", commentable.GetForeignIndex())
 	} else {
-		endpoint = fmt.Sprintf("/api/issues/%d/comments", commentable.GetForeignIndex())
+		endpoint = fmt.Sprintf("/~api/issues/%d/comments", commentable.GetForeignIndex())
 	}
 
 	err := d.callAPI(
@@ -405,9 +430,9 @@ func (d *OneDevDownloader) GetComments(ctx context.Context, commentable base.Com
 	}, 0, 100)
 
 	if context.IsPullRequest {
-		endpoint = fmt.Sprintf("/api/pull-requests/%d/changes", commentable.GetForeignIndex())
+		endpoint = fmt.Sprintf("/~api/pulls/%d/changes", commentable.GetForeignIndex())
 	} else {
-		endpoint = fmt.Sprintf("/api/issues/%d/changes", commentable.GetForeignIndex())
+		endpoint = fmt.Sprintf("/~api/issues/%d/changes", commentable.GetForeignIndex())
 	}
 
 	err = d.callAPI(
@@ -468,26 +493,24 @@ func (d *OneDevDownloader) GetComments(ctx context.Context, commentable base.Com
 // GetPullRequests returns pull requests
 func (d *OneDevDownloader) GetPullRequests(ctx context.Context, page, perPage int) ([]*base.PullRequest, bool, error) {
 	rawPullRequests := make([]struct {
-		ID             int64     `json:"id"`
+		ID             int64      `json:"id"`
-		Number         int64     `json:"number"`
+		Number         int64      `json:"number"`
-		Title          string    `json:"title"`
+		Title          string     `json:"title"`
-		SubmitterID    int64     `json:"submitterId"`
+		SubmitterID    int64      `json:"submitterId"`
-		SubmitDate     time.Time `json:"submitDate"`
+		SubmitDate     time.Time  `json:"submitDate"`
-		Description    string    `json:"description"`
+		Description    string     `json:"description"`
-		TargetBranch   string    `json:"targetBranch"`
+		TargetBranch   string     `json:"targetBranch"`
-		SourceBranch   string    `json:"sourceBranch"`
+		SourceBranch   string     `json:"sourceBranch"`
-		BaseCommitHash string    `json:"baseCommitHash"`
+		BaseCommitHash string     `json:"baseCommitHash"`
-		CloseInfo      *struct {
+		CloseDate      *time.Time `json:"closeDate"`
-			Date   *time.Time `json:"date"`
+		Status         string     `json:"status"` // Possible values: OPEN, MERGED, DISCARDED
-			Status string     `json:"status"`
-		}
 	}, 0, perPage)
 
 	err := d.callAPI(
 		ctx,
-		"/api/pull-requests",
+		"/~api/pulls",
 		map[string]string{
-			"query":  `"Target Project" is "` + d.repoName + `"`,
+			"query":  `"Target Project" is "` + d.repoPath + `"`,
 			"offset": strconv.Itoa((page - 1) * perPage),
 			"count":  strconv.Itoa(perPage),
 		},
@@ -507,7 +530,7 @@ func (d *OneDevDownloader) GetPullRequests(ctx context.Context, page, perPage in
 		}
 		err := d.callAPI(
 			ctx,
-			fmt.Sprintf("/api/pull-requests/%d/merge-preview", pr.ID),
+			fmt.Sprintf("/~api/pulls/%d/merge-preview", pr.ID),
 			nil,
 			&mergePreview,
 		)
@@ -519,12 +542,12 @@ func (d *OneDevDownloader) GetPullRequests(ctx context.Context, page, perPage in
 		merged := false
 		var closeTime *time.Time
 		var mergedTime *time.Time
-		if pr.CloseInfo != nil {
+		if pr.Status != "OPEN" {
 			state = "closed"
-			closeTime = pr.CloseInfo.Date
+			closeTime = pr.CloseDate
-			if pr.CloseInfo.Status == "MERGED" { // "DISCARDED"
+			if pr.Status == "MERGED" { // "DISCARDED"
 				merged = true
-				mergedTime = pr.CloseInfo.Date
+				mergedTime = pr.CloseDate
 			}
 		}
 		poster := d.tryGetUser(ctx, pr.SubmitterID)
@@ -545,12 +568,12 @@ func (d *OneDevDownloader) GetPullRequests(ctx context.Context, page, perPage in
 			Head: base.PullRequestBranch{
 				Ref:      pr.SourceBranch,
 				SHA:      mergePreview.HeadCommitHash,
-				RepoName: d.repoName,
+				RepoName: d.repoPath,
 			},
 			Base: base.PullRequestBranch{
 				Ref:      pr.TargetBranch,
 				SHA:      mergePreview.TargetHeadCommitHash,
-				RepoName: d.repoName,
+				RepoName: d.repoPath,
 			},
 			ForeignIndex: pr.ID,
 			Context:      onedevIssueContext{IsPullRequest: true},
@@ -566,18 +589,14 @@ func (d *OneDevDownloader) GetPullRequests(ctx context.Context, page, perPage in
 // GetReviews returns pull requests reviews
 func (d *OneDevDownloader) GetReviews(ctx context.Context, reviewable base.Reviewable) ([]*base.Review, error) {
 	rawReviews := make([]struct {
-		ID     int64 `json:"id"`
+		ID     int64  `json:"id"`
-		UserID int64 `json:"userId"`
+		UserID int64  `json:"userId"`
-		Result *struct {
+		Status string `json:"status"` // Possible values: PENDING, APPROVED, REQUESTED_FOR_CHANGES, EXCLUDED
-			Commit   string `json:"commit"`
-			Approved bool   `json:"approved"`
-			Comment  string `json:"comment"`
-		}
 	}, 0, 100)
 
 	err := d.callAPI(
 		ctx,
-		fmt.Sprintf("/api/pull-requests/%d/reviews", reviewable.GetForeignIndex()),
+		fmt.Sprintf("/~api/pulls/%d/reviews", reviewable.GetForeignIndex()),
 		nil,
 		&rawReviews,
 	)
@@ -589,14 +608,11 @@ func (d *OneDevDownloader) GetReviews(ctx context.Context, reviewable base.Revie
 	for _, review := range rawReviews {
 		state := base.ReviewStatePending
 		content := ""
-		if review.Result != nil {
+		switch review.Status {
-			if len(review.Result.Comment) > 0 {
+		case "APPROVED":
-				state = base.ReviewStateCommented
+			state = base.ReviewStateApproved
-				content = review.Result.Comment
+		case "REQUESTED_FOR_CHANGES":
-			}
+			state = base.ReviewStateChangesRequested
-			if review.Result.Approved {
-				state = base.ReviewStateApproved
-			}
 		}
 
 		poster := d.tryGetUser(ctx, review.UserID)
@@ -620,17 +636,52 @@ func (d *OneDevDownloader) GetTopics(_ context.Context) ([]string, error) {
 func (d *OneDevDownloader) tryGetUser(ctx context.Context, userID int64) *onedevUser {
 	user, ok := d.userMap[userID]
 	if !ok {
+		// get user name
+		type RawUser struct {
+			Name string `json:"name"`
+		}
+		var rawUser RawUser
 		err := d.callAPI(
 			ctx,
-			fmt.Sprintf("/api/users/%d", userID),
+			fmt.Sprintf("/~api/users/%d", userID),
 			nil,
-			&user,
+			&rawUser,
 		)
-		if err != nil {
+		var userName string
-			user = &onedevUser{
+		if err == nil {
-				Name: fmt.Sprintf("User %d", userID),
+			userName = rawUser.Name
+		} else {
+			userName = fmt.Sprintf("User %d", userID)
+		}
+
+		// get (primary) user Email address
+		rawEmailAddresses := make([]struct {
+			Value   string `json:"value"`
+			Primary bool   `json:"primary"`
+		}, 0, 10)
+		err = d.callAPI(
+			ctx,
+			fmt.Sprintf("/~api/users/%d/email-addresses", userID),
+			nil,
+			&rawEmailAddresses,
+		)
+		var userEmail string
+		if err == nil {
+			for _, email := range rawEmailAddresses {
+				if userEmail == "" || email.Primary {
+					userEmail = email.Value
+				}
+				if email.Primary {
+					break
+				}
 			}
 		}
+
+		user = &onedevUser{
+			ID:    userID,
+			Name:  userName,
+			Email: userEmail,
+		}
 		d.userMap[userID] = user
 	}