From e5f3c16587483daa18fa1db8bbc111452d9b864a Mon Sep 17 00:00:00 2001
From: Chai-Shi <changchaishi@gmail.com>
Date: Fri, 10 Jan 2025 13:29:55 +0800
Subject: [PATCH] Fix sync fork for consistency (#33147)

Fixes #33145

An integration test could be added.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
---
 modules/structs/repo_branch.go                |   8 ++
 routers/api/v1/api.go                         |   1 +
 routers/api/v1/repo/branch.go                 |  45 +++++++
 routers/api/v1/swagger/repo.go                |  12 ++
 services/repository/merge_upstream.go         |  49 +++++--
 .../repo/code/upstream_diverging_info.tmpl    |   6 +-
 templates/swagger/v1_json.tmpl                |  78 +++++++++++
 tests/integration/repo_merge_upstream_test.go | 122 ++++++++++++++++++
 8 files changed, 308 insertions(+), 13 deletions(-)
 create mode 100644 tests/integration/repo_merge_upstream_test.go

diff --git a/modules/structs/repo_branch.go b/modules/structs/repo_branch.go
index a9aa1d330a..55c98d60b9 100644
--- a/modules/structs/repo_branch.go
+++ b/modules/structs/repo_branch.go
@@ -133,3 +133,11 @@ type EditBranchProtectionOption struct {
 type UpdateBranchProtectionPriories struct {
 	IDs []int64 `json:"ids"`
 }
+
+type MergeUpstreamRequest struct {
+	Branch string `json:"branch"`
+}
+
+type MergeUpstreamResponse struct {
+	MergeStyle string `json:"merge_type"`
+}
diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index 2f943d306c..b1a42a85e6 100644
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -1190,6 +1190,7 @@ func Routes() *web.Router {
 				m.Get("/archive/*", reqRepoReader(unit.TypeCode), repo.GetArchive)
 				m.Combo("/forks").Get(repo.ListForks).
 					Post(reqToken(), reqRepoReader(unit.TypeCode), bind(api.CreateForkOption{}), repo.CreateFork)
+				m.Post("/merge-upstream", reqToken(), mustNotBeArchived, reqRepoWriter(unit.TypeCode), bind(api.MergeUpstreamRequest{}), repo.MergeUpstream)
 				m.Group("/branches", func() {
 					m.Get("", repo.ListBranches)
 					m.Get("/*", repo.GetBranch)
diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go
index aa0ab70ce8..592879235c 100644
--- a/routers/api/v1/repo/branch.go
+++ b/routers/api/v1/repo/branch.go
@@ -18,6 +18,7 @@ import (
 	"code.gitea.io/gitea/modules/optional"
 	repo_module "code.gitea.io/gitea/modules/repository"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/routers/api/v1/utils"
 	"code.gitea.io/gitea/services/context"
@@ -1186,3 +1187,47 @@ func UpdateBranchProtectionPriories(ctx *context.APIContext) {
 
 	ctx.Status(http.StatusNoContent)
 }
+
+func MergeUpstream(ctx *context.APIContext) {
+	// swagger:operation POST /repos/{owner}/{repo}/merge-upstream repository repoMergeUpstream
+	// ---
+	// summary: Merge a branch from upstream
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: owner
+	//   in: path
+	//   description: owner of the repo
+	//   type: string
+	//   required: true
+	// - name: repo
+	//   in: path
+	//   description: name of the repo
+	//   type: string
+	//   required: true
+	// - name: body
+	//   in: body
+	//   schema:
+	//     "$ref": "#/definitions/MergeUpstreamRequest"
+	// responses:
+	//   "200":
+	//     "$ref": "#/responses/MergeUpstreamResponse"
+	//   "400":
+	//     "$ref": "#/responses/error"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
+	form := web.GetForm(ctx).(*api.MergeUpstreamRequest)
+	mergeStyle, err := repo_service.MergeUpstream(ctx, ctx.Doer, ctx.Repo.Repository, form.Branch)
+	if err != nil {
+		if errors.Is(err, util.ErrInvalidArgument) {
+			ctx.Error(http.StatusBadRequest, "MergeUpstream", err)
+			return
+		} else if errors.Is(err, util.ErrNotExist) {
+			ctx.Error(http.StatusNotFound, "MergeUpstream", err)
+			return
+		}
+		ctx.Error(http.StatusInternalServerError, "MergeUpstream", err)
+		return
+	}
+	ctx.JSON(http.StatusOK, &api.MergeUpstreamResponse{MergeStyle: mergeStyle})
+}
diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go
index b9d2a0217c..f754c80a5b 100644
--- a/routers/api/v1/swagger/repo.go
+++ b/routers/api/v1/swagger/repo.go
@@ -448,3 +448,15 @@ type swaggerCompare struct {
 	// in:body
 	Body api.Compare `json:"body"`
 }
+
+// swagger:response MergeUpstreamRequest
+type swaggerMergeUpstreamRequest struct {
+	// in:body
+	Body api.MergeUpstreamRequest `json:"body"`
+}
+
+// swagger:response MergeUpstreamResponse
+type swaggerMergeUpstreamResponse struct {
+	// in:body
+	Body api.MergeUpstreamResponse `json:"body"`
+}
diff --git a/services/repository/merge_upstream.go b/services/repository/merge_upstream.go
index 85ca8f7e31..ef161889c0 100644
--- a/services/repository/merge_upstream.go
+++ b/services/repository/merge_upstream.go
@@ -12,17 +12,20 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/gitrepo"
 	repo_module "code.gitea.io/gitea/modules/repository"
+	"code.gitea.io/gitea/modules/reqctx"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/pull"
 )
 
 type UpstreamDivergingInfo struct {
-	BaseIsNewer   bool
-	CommitsBehind int
-	CommitsAhead  int
+	BaseHasNewCommits bool
+	CommitsBehind     int
+	CommitsAhead      int
 }
 
+// MergeUpstream merges the base repository's default branch into the fork repository's current branch.
 func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, branch string) (mergeStyle string, err error) {
 	if err = repo.MustNotBeArchived(); err != nil {
 		return "", err
@@ -32,7 +35,7 @@ func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model.
 	}
 	err = git.Push(ctx, repo.BaseRepo.RepoPath(), git.PushOptions{
 		Remote: repo.RepoPath(),
-		Branch: fmt.Sprintf("%s:%s", branch, branch),
+		Branch: fmt.Sprintf("%s:%s", repo.BaseRepo.DefaultBranch, branch),
 		Env:    repo_module.PushingEnvironment(doer, repo),
 	})
 	if err == nil {
@@ -64,7 +67,7 @@ func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model.
 		BaseRepoID: repo.BaseRepo.ID,
 		BaseRepo:   repo.BaseRepo,
 		HeadBranch: branch, // maybe HeadCommitID is not needed
-		BaseBranch: branch,
+		BaseBranch: repo.BaseRepo.DefaultBranch,
 	}
 	fakeIssue.PullRequest = fakePR
 	err = pull.Update(ctx, fakePR, doer, "merge upstream", false)
@@ -74,7 +77,8 @@ func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model.
 	return "merge", nil
 }
 
-func GetUpstreamDivergingInfo(ctx context.Context, repo *repo_model.Repository, branch string) (*UpstreamDivergingInfo, error) {
+// GetUpstreamDivergingInfo returns the information about the divergence between the fork repository's branch and the base repository's default branch.
+func GetUpstreamDivergingInfo(ctx reqctx.RequestContext, repo *repo_model.Repository, branch string) (*UpstreamDivergingInfo, error) {
 	if !repo.IsFork {
 		return nil, util.NewInvalidArgumentErrorf("repo is not a fork")
 	}
@@ -92,7 +96,7 @@ func GetUpstreamDivergingInfo(ctx context.Context, repo *repo_model.Repository,
 		return nil, err
 	}
 
-	baseBranch, err := git_model.GetBranch(ctx, repo.BaseRepo.ID, branch)
+	baseBranch, err := git_model.GetBranch(ctx, repo.BaseRepo.ID, repo.BaseRepo.DefaultBranch)
 	if err != nil {
 		return nil, err
 	}
@@ -102,14 +106,39 @@ func GetUpstreamDivergingInfo(ctx context.Context, repo *repo_model.Repository,
 		return info, nil
 	}
 
-	// TODO: if the fork repo has new commits, this call will fail:
+	// if the fork repo has new commits, this call will fail because they are not in the base repo
 	// exit status 128 - fatal: Invalid symmetric difference expression aaaaaaaaaaaa...bbbbbbbbbbbb
-	// so at the moment, we are not able to handle this case, should be improved in the future
+	// so at the moment, we first check the update time, then check whether the fork branch has base's head
 	diff, err := git.GetDivergingCommits(ctx, repo.BaseRepo.RepoPath(), baseBranch.CommitID, forkBranch.CommitID)
 	if err != nil {
-		info.BaseIsNewer = baseBranch.UpdatedUnix > forkBranch.UpdatedUnix
+		info.BaseHasNewCommits = baseBranch.UpdatedUnix > forkBranch.UpdatedUnix
+		if info.BaseHasNewCommits {
+			return info, nil
+		}
+
+		// if the base's update time is before the fork, check whether the base's head is in the fork
+		baseGitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo.BaseRepo)
+		if err != nil {
+			return nil, err
+		}
+		headGitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo)
+		if err != nil {
+			return nil, err
+		}
+
+		baseCommitID, err := baseGitRepo.ConvertToGitID(baseBranch.CommitID)
+		if err != nil {
+			return nil, err
+		}
+		headCommit, err := headGitRepo.GetCommit(forkBranch.CommitID)
+		if err != nil {
+			return nil, err
+		}
+		hasPreviousCommit, _ := headCommit.HasPreviousCommit(baseCommitID)
+		info.BaseHasNewCommits = !hasPreviousCommit
 		return info, nil
 	}
+
 	info.CommitsBehind, info.CommitsAhead = diff.Behind, diff.Ahead
 	return info, nil
 }
diff --git a/templates/repo/code/upstream_diverging_info.tmpl b/templates/repo/code/upstream_diverging_info.tmpl
index 51402598f9..bdcd99a7f7 100644
--- a/templates/repo/code/upstream_diverging_info.tmpl
+++ b/templates/repo/code/upstream_diverging_info.tmpl
@@ -1,8 +1,8 @@
-{{if and .UpstreamDivergingInfo (or .UpstreamDivergingInfo.BaseIsNewer .UpstreamDivergingInfo.CommitsBehind)}}
+{{if and .UpstreamDivergingInfo (or .UpstreamDivergingInfo.BaseHasNewCommits .UpstreamDivergingInfo.CommitsBehind)}}
 <div class="ui message flex-text-block">
 	<div class="tw-flex-1">
-		{{$upstreamLink := printf "%s/src/branch/%s" .Repository.BaseRepo.Link (.BranchName|PathEscapeSegments)}}
-		{{$upstreamHtml := HTMLFormat `<a href="%s">%s:%s</a>` $upstreamLink .Repository.BaseRepo.FullName .BranchName}}
+		{{$upstreamLink := printf "%s/src/branch/%s" .Repository.BaseRepo.Link (.Repository.BaseRepo.DefaultBranch|PathEscapeSegments)}}
+		{{$upstreamHtml := HTMLFormat `<a href="%s">%s:%s</a>` $upstreamLink .Repository.BaseRepo.FullName .Repository.BaseRepo.DefaultBranch}}
 		{{if .UpstreamDivergingInfo.CommitsBehind}}
 			{{ctx.Locale.TrN .UpstreamDivergingInfo.CommitsBehind "repo.pulls.upstream_diverging_prompt_behind_1" "repo.pulls.upstream_diverging_prompt_behind_n" .UpstreamDivergingInfo.CommitsBehind $upstreamHtml}}
 		{{else}}
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index 82a301da2f..fb37d45ce8 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -10867,6 +10867,52 @@
         }
       }
     },
+    "/repos/{owner}/{repo}/merge-upstream": {
+      "post": {
+        "produces": [
+          "application/json"
+        ],
+        "tags": [
+          "repository"
+        ],
+        "summary": "Merge a branch from upstream",
+        "operationId": "repoMergeUpstream",
+        "parameters": [
+          {
+            "type": "string",
+            "description": "owner of the repo",
+            "name": "owner",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "string",
+            "description": "name of the repo",
+            "name": "repo",
+            "in": "path",
+            "required": true
+          },
+          {
+            "name": "body",
+            "in": "body",
+            "schema": {
+              "$ref": "#/definitions/MergeUpstreamRequest"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "$ref": "#/responses/MergeUpstreamResponse"
+          },
+          "400": {
+            "$ref": "#/responses/error"
+          },
+          "404": {
+            "$ref": "#/responses/notFound"
+          }
+        }
+      }
+    },
     "/repos/{owner}/{repo}/milestones": {
       "get": {
         "produces": [
@@ -22827,6 +22873,26 @@
       "x-go-name": "MergePullRequestForm",
       "x-go-package": "code.gitea.io/gitea/services/forms"
     },
+    "MergeUpstreamRequest": {
+      "type": "object",
+      "properties": {
+        "branch": {
+          "type": "string",
+          "x-go-name": "Branch"
+        }
+      },
+      "x-go-package": "code.gitea.io/gitea/modules/structs"
+    },
+    "MergeUpstreamResponse": {
+      "type": "object",
+      "properties": {
+        "merge_type": {
+          "type": "string",
+          "x-go-name": "MergeStyle"
+        }
+      },
+      "x-go-package": "code.gitea.io/gitea/modules/structs"
+    },
     "MigrateRepoOptions": {
       "description": "MigrateRepoOptions options for migrating repository's\nthis is used to interact with api v1",
       "type": "object",
@@ -26008,6 +26074,18 @@
         "type": "string"
       }
     },
+    "MergeUpstreamRequest": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/MergeUpstreamRequest"
+      }
+    },
+    "MergeUpstreamResponse": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/MergeUpstreamResponse"
+      }
+    },
     "Milestone": {
       "description": "Milestone",
       "schema": {
diff --git a/tests/integration/repo_merge_upstream_test.go b/tests/integration/repo_merge_upstream_test.go
new file mode 100644
index 0000000000..e3e423c51d
--- /dev/null
+++ b/tests/integration/repo_merge_upstream_test.go
@@ -0,0 +1,122 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+	"strings"
+	"testing"
+	"time"
+
+	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/models/db"
+	git_model "code.gitea.io/gitea/models/git"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRepoMergeUpstream(t *testing.T) {
+	onGiteaRun(t, func(*testing.T, *url.URL) {
+		forkUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
+
+		baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+		baseUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: baseRepo.OwnerID})
+
+		checkFileContent := func(branch, exp string) {
+			req := NewRequest(t, "GET", fmt.Sprintf("/%s/test-repo-fork/raw/branch/%s/new-file.txt", forkUser.Name, branch))
+			resp := MakeRequest(t, req, http.StatusOK)
+			require.Equal(t, exp, resp.Body.String())
+		}
+
+		session := loginUser(t, forkUser.Name)
+		token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+		// create a fork
+		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/forks", baseUser.Name, baseRepo.Name), &api.CreateForkOption{
+			Name: util.ToPointer("test-repo-fork"),
+		}).AddTokenAuth(token)
+		MakeRequest(t, req, http.StatusAccepted)
+		forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: forkUser.ID, Name: "test-repo-fork"})
+
+		// create fork-branch
+		req = NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/test-repo-fork/branches/_new/branch/master", forkUser.Name), map[string]string{
+			"_csrf":           GetUserCSRFToken(t, session),
+			"new_branch_name": "fork-branch",
+		})
+		session.MakeRequest(t, req, http.StatusSeeOther)
+
+		queryMergeUpstreamButtonLink := func(htmlDoc *HTMLDoc) string {
+			return htmlDoc.Find(`button[data-url*="merge-upstream"]`).AttrOr("data-url", "")
+		}
+
+		t.Run("HeadBeforeBase", func(t *testing.T) {
+			// add a file in base repo
+			require.NoError(t, createOrReplaceFileInBranch(baseUser, baseRepo, "new-file.txt", "master", "test-content-1"))
+
+			// the repo shows a prompt to "sync fork"
+			var mergeUpstreamLink string
+			require.Eventually(t, func() bool {
+				resp := session.MakeRequest(t, NewRequestf(t, "GET", "/%s/test-repo-fork/src/branch/fork-branch", forkUser.Name), http.StatusOK)
+				htmlDoc := NewHTMLParser(t, resp.Body)
+				mergeUpstreamLink = queryMergeUpstreamButtonLink(htmlDoc)
+				if mergeUpstreamLink == "" {
+					return false
+				}
+				respMsg, _ := htmlDoc.Find(".ui.message:not(.positive)").Html()
+				return strings.Contains(respMsg, `This branch is 1 commit behind <a href="/user2/repo1/src/branch/master">user2/repo1:master</a>`)
+			}, 5*time.Second, 100*time.Millisecond)
+
+			// click the "sync fork" button
+			req = NewRequestWithValues(t, "POST", mergeUpstreamLink, map[string]string{"_csrf": GetUserCSRFToken(t, session)})
+			session.MakeRequest(t, req, http.StatusOK)
+			checkFileContent("fork-branch", "test-content-1")
+		})
+
+		t.Run("BaseChangeAfterHeadChange", func(t *testing.T) {
+			// update the files: base first, head later, and check the prompt
+			require.NoError(t, createOrReplaceFileInBranch(baseUser, baseRepo, "new-file.txt", "master", "test-content-2"))
+			require.NoError(t, createOrReplaceFileInBranch(forkUser, forkRepo, "new-file-other.txt", "fork-branch", "test-content-other"))
+
+			// make sure the base branch's update time is before the fork, to make it test the complete logic
+			baseBranch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: baseRepo.ID, Name: "master"})
+			forkBranch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: forkRepo.ID, Name: "fork-branch"})
+			_, err := db.GetEngine(db.DefaultContext).ID(forkBranch.ID).Update(&git_model.Branch{UpdatedUnix: baseBranch.UpdatedUnix + 1})
+			require.NoError(t, err)
+
+			// the repo shows a prompt to "sync fork"
+			require.Eventually(t, func() bool {
+				resp := session.MakeRequest(t, NewRequestf(t, "GET", "/%s/test-repo-fork/src/branch/fork-branch", forkUser.Name), http.StatusOK)
+				htmlDoc := NewHTMLParser(t, resp.Body)
+				respMsg, _ := htmlDoc.Find(".ui.message:not(.positive)").Html()
+				return strings.Contains(respMsg, `The base branch <a href="/user2/repo1/src/branch/master">user2/repo1:master</a> has new changes`)
+			}, 5*time.Second, 100*time.Millisecond)
+
+			// and do the merge-upstream by API
+			req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/test-repo-fork/merge-upstream", forkUser.Name), &api.MergeUpstreamRequest{
+				Branch: "fork-branch",
+			}).AddTokenAuth(token)
+			resp := MakeRequest(t, req, http.StatusOK)
+			checkFileContent("fork-branch", "test-content-2")
+
+			var mergeResp api.MergeUpstreamResponse
+			DecodeJSON(t, resp, &mergeResp)
+			assert.Equal(t, "merge", mergeResp.MergeStyle)
+
+			// after merge, there should be no "sync fork" button anymore
+			require.Eventually(t, func() bool {
+				resp := session.MakeRequest(t, NewRequestf(t, "GET", "/%s/test-repo-fork/src/branch/fork-branch", forkUser.Name), http.StatusOK)
+				htmlDoc := NewHTMLParser(t, resp.Body)
+				return queryMergeUpstreamButtonLink(htmlDoc) == ""
+			}, 5*time.Second, 100*time.Millisecond)
+		})
+	})
+}