Merge pull request 'Backport #2139 to v1.21' (#2142) from algernon/forgejo:b/v1.21/missing-branch-makes-server-sad into v1.21/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2142
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

services/repository/branch.go

@@ -100,7 +100,13 @@ func LoadBranches(ctx context.Context, repo *repo_model.Repository, gitRepo *git
 	for i := range dbBranches {
 		branch, err := loadOneBranch(ctx, repo, dbBranches[i], &rules, repoIDToRepo, repoIDToGitRepo)
 		if err != nil {
-			return nil, nil, 0, fmt.Errorf("loadOneBranch: %v", err)
+			log.Error("loadOneBranch() on repo #%d, branch '%s' failed: %v", repo.ID, dbBranches[i].Name, err)
+
+			// TODO: Ideally, we would only do this if the branch doesn't exist
+			// anymore. That is not practical to check here currently, so we do
+			// this for all kinds of errors.
+			totalNumOfBranches--
+			continue
 		}
 
 		branches = append(branches, branch)
@@ -136,7 +142,7 @@ func loadOneBranch(ctx context.Context, repo *repo_model.Repository, dbBranch *g
 		var err error
 		divergence, err = files_service.CountDivergingCommits(ctx, repo, git.BranchPrefix+branchName)
 		if err != nil {
-			log.Error("CountDivergingCommits: %v", err)
+			return nil, fmt.Errorf("CountDivergingCommits: %v", err)
 		}
 	}
 

tests/integration/repo_branch_test.go

@@ -1,4 +1,5 @@
 // Copyright 2017 The Gitea Authors. All rights reserved.
+// Copyright 2024 The Forgejo Authors c/o Codeberg e.V.. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package integration
@@ -7,14 +8,21 @@ import (
 	"net/http"
 	"net/url"
 	"path"
+	"strconv"
 	"strings"
 	"testing"
 
+	"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"
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/modules/translation"
+	repo_service "code.gitea.io/gitea/services/repository"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -159,3 +167,49 @@ func TestCreateBranchInvalidCSRF(t *testing.T) {
 		strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
 	)
 }
+
+func TestDatabaseMissingABranch(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, URL *url.URL) {
+		adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{IsAdmin: true})
+		session := loginUser(t, "user2")
+
+		// Create two branches
+		testCreateBranch(t, session, "user2", "repo1", "branch/master", "will-be-present", http.StatusSeeOther)
+		testCreateBranch(t, session, "user2", "repo1", "branch/master", "will-be-missing", http.StatusSeeOther)
+
+		// Run the repo branch sync, to ensure the db and git agree.
+		err2 := repo_service.AddAllRepoBranchesToSyncQueue(graceful.GetManager().ShutdownContext(), adminUser.ID)
+		assert.NoError(t, err2)
+
+		// Delete one branch from git only, leaving it in the database
+		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+		cmd := git.NewCommand(db.DefaultContext, "branch", "-D").AddDynamicArguments("will-be-missing")
+		_, _, err := cmd.RunStdString(&git.RunOpts{Dir: repo.RepoPath()})
+		assert.NoError(t, err)
+
+		// Verify that loading the repo's branches page works still, and that it
+		// reports at least three branches (master, will-be-present, and
+		// will-be-missing).
+		req := NewRequest(t, "GET", "/user2/repo1/branches")
+		resp := session.MakeRequest(t, req, http.StatusOK)
+		doc := NewHTMLParser(t, resp.Body)
+		firstBranchCount, _ := strconv.Atoi(doc.Find(".repository-menu a[href*='/branches'] b").Text())
+		assert.GreaterOrEqual(t, firstBranchCount, 3)
+
+		// Run the repo branch sync again
+		err2 = repo_service.AddAllRepoBranchesToSyncQueue(graceful.GetManager().ShutdownContext(), adminUser.ID)
+		assert.NoError(t, err2)
+
+		// Verify that loading the repo's branches page works still, and that it
+		// reports one branch less than the first time.
+		//
+		// NOTE: This assumes that the branch counter on the web UI is out of
+		// date before the sync. If that problem gets resolved, we'll have to
+		// find another way to test that the syncing works.
+		req = NewRequest(t, "GET", "/user2/repo1/branches")
+		resp = session.MakeRequest(t, req, http.StatusOK)
+		doc = NewHTMLParser(t, resp.Body)
+		secondBranchCount, _ := strconv.Atoi(doc.Find(".repository-menu a[href*='/branches'] b").Text())
+		assert.Equal(t, firstBranchCount-1, secondBranchCount)
+	})
+}