fix: Fix OneDev migration with OneDev 16.8 and newer (#40017)

OneDev 16.8 moved the server version endpoint from
`/~api/version/server` to `/~api/server/version`. Both are tried, the
new one first, and both errors are reported when neither works.

---------

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>

models/unittest/mock_http.go

@@ -12,6 +12,7 @@ import (
 	"net/url"
 	"os"
 	"slices"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -103,6 +104,10 @@ func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveM
 			require.NoError(t, err, "creating fixture %s", fixturePath)
 			defer out.Close()
 
+			if resp.StatusCode != http.StatusOK {
+				_, err = fmt.Fprintf(out, "HTTP/1.1 %d %s\n", resp.StatusCode, http.StatusText(resp.StatusCode))
+				require.NoError(t, err, "writing fixture header for %s", liveURL)
+			}
 			for _, name := range slices.Sorted(maps.Keys(resp.Header)) {
 				if slices.Contains(ignoredHeaders, strings.ToLower(name)) {
 					continue
@@ -125,14 +130,21 @@ func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveM
 
 		replayed := strings.ReplaceAll(string(raw), liveServerBaseURL, mockURL)
 		headers, body, _ := strings.Cut(replayed, "\n\n")
+		code := http.StatusOK
 		for line := range strings.SplitSeq(headers, "\n") {
+			if strings.HasPrefix(line, "HTTP/") {
+				parts := strings.SplitN(line, " ", 3)
+				require.Len(t, parts, 3)
+				code, _ = strconv.Atoi(parts[1])
+				continue
+			}
 			name, value, ok := strings.Cut(line, ": ")
 			if !ok || strings.EqualFold(name, "Content-Length") {
 				continue
 			}
 			w.Header().Set(name, value)
 		}
-		w.WriteHeader(http.StatusOK)
+		w.WriteHeader(code)
 		_, err = w.Write([]byte(body))
 		require.NoError(t, err)
 	})

services/migrations/_mock_data/TestOneDevDownloadRepo/GET_%2F~api%2Fserver%2Fversion

@@ -0,0 +1 @@
+HTTP/1.1 404 Not Found

services/migrations/onedev.go

@@ -5,6 +5,7 @@ package migrations
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -77,6 +78,8 @@ type OneDevDownloader struct {
 	maxIssueIndex int64
 	userMap       map[int64]*onedevUser
 	milestoneMap  map[int64]string
+
+	cachedServerVersion *version.Version
 }
 
 // NewOneDevDownloader creates a new downloader
@@ -103,10 +106,10 @@ func (d *OneDevDownloader) LogString() string {
 	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 {
+func (d *OneDevDownloader) makeRequest(ctx context.Context, endpoint string, parameter map[string]string) (*http.Response, error) {
 	u, err := d.baseURL.Parse(endpoint)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	if parameter != nil {
@@ -119,52 +122,75 @@ func (d *OneDevDownloader) callAPI(ctx context.Context, endpoint string, paramet
 
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
 	if err != nil {
-		return err
+		return nil, err
 	}
+	return d.client.Do(req)
+}
 
-	resp, err := d.client.Do(req)
+func (d *OneDevDownloader) callAPI(ctx context.Context, endpoint string, parameter map[string]string, result any) error {
+	resp, err := d.makeRequest(ctx, endpoint, parameter)
 	if err != nil {
 		return err
 	}
 	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(io.LimitReader(resp.Body, maxOneDevVersionResponseSize+1))
-		if err != nil {
-			return err
-		}
-		if len(bytes) > maxOneDevVersionResponseSize {
-			return fmt.Errorf("OneDev server version response exceeds %d bytes", maxOneDevVersionResponseSize)
-		}
-		vers, err := version.NewVersion(string(bytes))
-		if err != nil {
-			return err
-		}
-		*presult = vers
-		return nil
-	}
-
 	decoder := json.NewDecoder(resp.Body)
 	return decoder.Decode(&result)
 }
 
+func (d *OneDevDownloader) getServerVersionByEndpoint(ctx context.Context, endpoint string) (*version.Version, error) {
+	resp, err := d.makeRequest(ctx, endpoint, nil)
+	if err != nil {
+		return nil, err
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("OneDev server version request failed: %s", resp.Status)
+	}
+	// This patch doesn't change the situation, "decoder.Decode(&result)" in callAPI still reads the entire body
+	bytes, err := io.ReadAll(io.LimitReader(resp.Body, maxOneDevVersionResponseSize+1))
+	if err != nil {
+		return nil, err
+	}
+	if len(bytes) > maxOneDevVersionResponseSize {
+		return nil, fmt.Errorf("OneDev server version response exceeds %d bytes", maxOneDevVersionResponseSize)
+	}
+	ver, err := version.NewVersion(string(bytes))
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse OneDev server version %q: %w", string(bytes), err)
+	}
+	return ver, nil
+}
+
+// getServerVersion reads the OneDev server version.
+func (d *OneDevDownloader) getServerVersion(ctx context.Context) (_ *version.Version, retErr error) {
+	if d.cachedServerVersion != nil {
+		return d.cachedServerVersion, nil
+	}
+	// OneDev moved the endpoint from /~api/version/server to /~api/server/version in 16.8, so both are tried, the new first.
+	oneDevVersionEndpoints := []string{"/~api/server/version", "/~api/version/server"}
+	for _, endpoint := range oneDevVersionEndpoints {
+		ver, err := d.getServerVersionByEndpoint(ctx, endpoint)
+		if err == nil {
+			d.cachedServerVersion = ver
+			return ver, nil
+		}
+		retErr = errors.Join(err, fmt.Errorf("failed to get OneDev server version via %s: %w", endpoint, err))
+	}
+	return nil, retErr
+}
+
 // 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,
-	)
+	serverVersion, err := d.getServerVersion(ctx)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get OneDev server version; OneDev %s or newer required", OneDevRequiredVersion)
+		return nil, fmt.Errorf("OneDev %s or newer is required: %w", OneDevRequiredVersion, err)
 	}
 	requiredVersion, _ := version.NewVersion(OneDevRequiredVersion)
 	if serverVersion.LessThan(requiredVersion) {
-		return nil, fmt.Errorf("OneDev %s or newer required; currently running OneDev %s", OneDevRequiredVersion, serverVersion)
+		return nil, fmt.Errorf("OneDev version %s is too old, %s or newer is required", serverVersion, OneDevRequiredVersion)
 	}
 
 	info := make([]struct {

services/migrations/onedev_test.go

@@ -4,35 +4,72 @@
 package migrations
 
 import (
-	"bytes"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"gitea.dev/models/unittest"
 	base "gitea.dev/modules/migration"
+	"gitea.dev/modules/util"
 
-	"github.com/hashicorp/go-version"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestOneDevVersionResponseSizeLimit(t *testing.T) {
+func TestOneDevServerVersion(t *testing.T) {
+	type respMapType map[string]struct {
+		code int
+		body string
+	}
+	var respMap atomic.Pointer[respMapType]
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		_, _ = w.Write(bytes.Repeat([]byte("1"), maxOneDevVersionResponseSize+1))
+		resp, ok := (*respMap.Load())[r.URL.Path]
+		w.WriteHeader(util.Iif(ok, resp.code, http.StatusNotFound))
+		_, _ = io.WriteString(w, resp.body)
 	}))
 	defer server.Close()
 
 	baseURL, err := url.Parse(server.URL)
 	assert.NoError(t, err)
-	download := &OneDevDownloader{baseURL: baseURL, client: server.Client()}
-	var version *version.Version
-	err = download.callAPI(t.Context(), "/~api/version/server", nil, &version)
-	assert.Error(t, err)
+	downloader := func() *OneDevDownloader { return &OneDevDownloader{baseURL: baseURL, client: server.Client()} }
+
+	t.Run("LargeResponse", func(t *testing.T) {
+		respMap.Store(&respMapType{"/~api/version/server": {code: http.StatusOK, body: strings.Repeat("x", maxOneDevVersionResponseSize+1)}})
+		_, err := downloader().getServerVersion(t.Context())
+		assert.ErrorContains(t, err, "server version response exceeds")
+	})
+
+	t.Run("LegacyEndpoint", func(t *testing.T) {
+		respMap.Store(&respMapType{"/~api/version/server": {code: http.StatusOK, body: "12.0.1"}})
+		d := downloader()
+		v, err := d.getServerVersion(t.Context())
+		assert.NoError(t, err)
+		assert.Equal(t, "12.0.1", v.String())
+		respMap.Store(&respMapType{}) // remove the legacy endpoint, next call should still succeed by using cached version
+		v, err = d.getServerVersion(t.Context())
+		assert.NoError(t, err)
+		assert.Equal(t, "12.0.1", v.String())
+	})
+
+	t.Run("NewEndpoint", func(t *testing.T) {
+		respMap.Store(&respMapType{"/~api/server/version": {code: http.StatusOK, body: "16.9.7"}})
+		v, err := downloader().getServerVersion(t.Context())
+		assert.NoError(t, err)
+		assert.Equal(t, "16.9.7", v.String())
+	})
+
+	t.Run("NoEndpoint", func(t *testing.T) {
+		respMap.Store(&respMapType{})
+		_, err := downloader().getServerVersion(t.Context())
+		assert.ErrorContains(t, err, "server version request failed")
+	})
 }
 
 func TestOneDevDownloadRepo(t *testing.T) {