Refactor: Extract Shared Default Branch Setup Logic

[pedrogaudencio]

Extract duplicated default branch setup logic from ArticleView() handler and RepoRefByType middleware into a reusable helper function to improve code maintainability and reduce duplication.

---

Problem Description

During the implementation of article versioning in PR #54, a new handler ArticleView() was added that needed to set up the default branch context. This logic was duplicated from the existing RepoRefByType middleware because the article route requires conditional behavior (version parameter vs. no version parameter) that cannot be handled by middleware alone.

Current Duplication:

• File 1: routers/web/repo/commit.go (lines 494-528) - ArticleView() function
• File 2: services/context/repo.go (lines 867-903) - RepoRefByType middleware

Both locations contain nearly identical logic for:

1. Handling empty repositories
2. Determining the default branch (with fallback to first available branch)
3. Setting up repository context (BranchName, RefFullName, Commit, CommitID, CommitsCount)
4. Handling broken repositories with specific error string matching
5. Setting template context data

---

Current Behavior

Location 1: ArticleView() in routers/web/repo/commit.go (lines 494-528)

// Set up branch reference (default branch)
if ctx.Repo.Repository.IsEmpty {
    ctx.Repo.BranchName = ctx.Repo.Repository.DefaultBranch
    ctx.Repo.RefFullName = git.RefNameFromBranch(ctx.Repo.BranchName)
    ctx.Data["BranchName"] = ctx.Repo.BranchName
    ctx.Data["TreePath"] = ""
} else {
    refShortName := ctx.Repo.Repository.DefaultBranch
    if !gitrepo.IsBranchExist(ctx, ctx.Repo.Repository, refShortName) {
        brs, _, err := ctx.Repo.GitRepo.GetBranchNames(0, 1)
        if err == nil && len(brs) != 0 {
            refShortName = brs[0]
        }
    }
    ctx.Repo.RefFullName = git.RefNameFromBranch(refShortName)
    ctx.Repo.BranchName = refShortName
    ctx.Repo.TreePath = ""

    var err error
    ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refShortName)
    if err == nil {
        ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
        ctx.Repo.CommitsCount, _ = ctx.Repo.GetCommitsCount()
    } else if !strings.Contains(err.Error(), "fatal: not a git repository") && !strings.Contains(err.Error(), "object does not exist") {
        ctx.ServerError("GetBranchCommit", err)
        return
    }

    // Set all required context data for templates
    ctx.Data["RefFullName"] = ctx.Repo.RefFullName
    ctx.Data["BranchName"] = ctx.Repo.BranchName
    ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount
    ctx.Data["TreePath"] = ""
    ctx.Data["CommitID"] = ctx.Repo.CommitID
    ctx.Data["RefTypeNameSubURL"] = ctx.Repo.RefTypeNameSubURL()
}

Location 2: RepoRefByType in services/context/repo.go (lines 867-903)

if ctx.Repo.Repository.IsEmpty {
    // assume the user is viewing the (non-existent) default branch
    ctx.Repo.BranchName = ctx.Repo.Repository.DefaultBranch
    ctx.Repo.RefFullName = git.RefNameFromBranch(ctx.Repo.BranchName)
    // these variables are used by the template to "add/upload" new files
    ctx.Data["BranchName"] = ctx.Repo.BranchName
    ctx.Data["TreePath"] = ""
    return
}

// Get default branch.
var refShortName string
reqPath := ctx.PathParam("*")
if reqPath == "" {
    refShortName = ctx.Repo.Repository.DefaultBranch
    if !gitrepo.IsBranchExist(ctx, ctx.Repo.Repository, refShortName) {
        brs, _, err := ctx.Repo.GitRepo.GetBranchNames(0, 1)
        if err == nil && len(brs) != 0 {
            refShortName = brs[0]
        } else if len(brs) == 0 {
            log.Error("No branches in non-empty repository %s", ctx.Repo.GitRepo.Path)
        } else {
            log.Error("GetBranches error: %v", err)
        }
    }
    ctx.Repo.RefFullName = git.RefNameFromBranch(refShortName)
    ctx.Repo.BranchName = refShortName
    ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refShortName)
    if err == nil {
        ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
    } else if strings.Contains(err.Error(), "fatal: not a git repository") || strings.Contains(err.Error(), "object does not exist") {
        // if the repository is broken, we can continue to the handler code, to show "Settings -> Delete Repository" for end users
        log.Error("GetBranchCommit: %v", err)
    } else {
        ctx.ServerError("GetBranchCommit", err)
        return
    }

Key Differences:

1. RepoRefByType has additional error logging for edge cases (no branches, GetBranches error)
2. ArticleView sets CommitsCount immediately, while RepoRefByType sets it later in the function
3. ArticleView sets template data immediately, while RepoRefByType sets it at the end of the middleware
4. Error handling logic is inverted (negation in ArticleView vs. positive check in RepoRefByType)

---

Proposed Solution

Step 1: Create Helper Function

Add a new helper function in services/context/repo.go:

// SetupDefaultBranchContext sets up the repository context for the default branch.
// This is used when no specific ref is requested (e.g., viewing repository home).
// It handles empty repositories, missing default branches, and broken repositories.
//
// Returns true if setup was successful, false if an error was written to the response.
func (r *Repository) SetupDefaultBranchContext(ctx *Context) bool {
    // Handle empty repositories
    if r.Repository.IsEmpty {
        r.BranchName = r.Repository.DefaultBranch
        r.RefFullName = git.RefNameFromBranch(r.BranchName)
        ctx.Data["BranchName"] = r.BranchName
        ctx.Data["TreePath"] = ""
        return true
    }

    // Determine the default branch (with fallback to first available branch)
    refShortName := r.Repository.DefaultBranch
    if !gitrepo.IsBranchExist(ctx, r.Repository, refShortName) {
        brs, _, err := r.GitRepo.GetBranchNames(0, 1)
        if err == nil && len(brs) != 0 {
            refShortName = brs[0]
        } else if len(brs) == 0 {
            log.Error("No branches in non-empty repository %s", r.GitRepo.Path)
        } else {
            log.Error("GetBranches error: %v", err)
        }
    }

    // Set up branch context
    r.RefFullName = git.RefNameFromBranch(refShortName)
    r.BranchName = refShortName
    r.TreePath = ""

    // Get the branch commit
    var err error
    r.Commit, err = r.GitRepo.GetBranchCommit(refShortName)
    if err == nil {
        r.CommitID = r.Commit.ID.String()
        r.CommitsCount, _ = r.GetCommitsCount()
    } else if strings.Contains(err.Error(), "fatal: not a git repository") || strings.Contains(err.Error(), "object does not exist") {
        // If the repository is broken, we can continue to the handler code,
        // to show "Settings -> Delete Repository" for end users
        log.Error("GetBranchCommit: %v", err)
    } else {
        ctx.ServerError("GetBranchCommit", err)
        return false
    }

    // Set template context data
    ctx.Data["RefFullName"] = r.RefFullName
    ctx.Data["BranchName"] = r.BranchName
    ctx.Data["CommitsCount"] = r.CommitsCount
    ctx.Data["TreePath"] = r.TreePath
    ctx.Data["CommitID"] = r.CommitID
    ctx.Data["RefTypeNameSubURL"] = r.RefTypeNameSubURL()

    return true
}

Step 2: Update RepoRefByType Middleware

Replace lines 867-903 in services/context/repo.go:

// Get default branch.
var refShortName string
reqPath := ctx.PathParam("*")
if reqPath == "" {
    if !ctx.Repo.SetupDefaultBranchContext(ctx) {
        return // error already written to response
    }
} else {
    // ... existing logic for handling paths ...
}

Step 3: Update ArticleView Handler

Replace lines 494-528 in routers/web/repo/commit.go:

// Set up branch reference (default branch)
if !ctx.Repo.SetupDefaultBranchContext(ctx) {
    return // error already written to response
}

---

Benefits

1. Reduced Code Duplication: Eliminates ~35 lines of duplicated logic
2. Single Source of Truth: Branch setup logic is defined in one place
3. Easier Maintenance: Bug fixes and improvements only need to be made once
4. Consistency: Ensures identical behavior across all routes that need default branch setup
5. Better Testability: Helper function can be unit tested independently
6. Improved Readability: Handler code becomes more concise and focused on business logic

---

Scope and Impact

Files to Modify

1. services/context/repo.go:

   • Add new SetupDefaultBranchContext() method (~50 lines)
   • Refactor RepoRefByType middleware to use the new helper (~5 lines changed)

2. routers/web/repo/commit.go:

   • Refactor ArticleView handler to use the new helper (~35 lines removed, 3 lines added)

Testing Requirements

Critical: This change affects core middleware used across dozens of routes throughout Gitea:

• Repository home page (/{owner}/{repo})
• File browser (/{owner}/{repo}/src/branch/{branch})
• Commits page (/{owner}/{repo}/commits)
• Releases page (/{owner}/{repo}/releases)
• And many more...

Required Tests:

1. Unit Tests: Test SetupDefaultBranchContext() with:

   • Empty repositories
   • Repositories with default branch
   • Repositories with missing default branch (fallback to first branch)
   • Broken repositories (error handling)

2. Integration Tests: Verify existing routes still work:

   • Repository home page loads correctly
   • Article view (with and without version parameter) works
   • Branch selector shows correct default branch
   • Broken repository shows "Delete Repository" option

3. Edge Cases:

   • Repository with no branches (empty but initialized)
   • Repository with renamed default branch
   • Repository with broken git directory

---

Related Context

• PR Add versioning if param is present #54: article versioning feature
• Identified this duplication during PR review in Add versioning if param is present #54 (comment)
• Decision: Deferred from PR Add versioning if param is present #54 to avoid scope creep and ensure focused review
• Original Discussion: This refactoring was deemed valuable but requires careful testing due to impact on core middleware

---

Implementation Notes

Technical Considerations

1. Error Handling Pattern: The helper function returns bool to indicate success/failure, following Gitea's pattern for context setup functions that may write errors to the response.

2. String Matching for Errors: The current implementation uses strings.Contains() to check for specific error messages. This is intentional and matches Gitea's existing patterns (see services/context/repo.go:897). While typed errors would be better, Gitea doesn't currently provide typed errors for "not a git repository" errors.

3. CommitsCount Calculation: The helper function calls GetCommitsCount() which uses caching. This is important for performance on large repositories.

4. Template Data: The helper function sets all required template context data. Callers don't need to set this data separately.

5. Backward Compatibility: This refactoring should not change any external behavior - it's purely an internal code organization improvement.

Implementation Steps

1. Add the SetupDefaultBranchContext() method to services/context/repo.go
2. Add unit tests for the new method
3. Update RepoRefByType middleware to use the new helper
4. Update ArticleView handler to use the new helper
5. Run full test suite to ensure no regressions
6. Manual testing of key routes (repository home, article view, etc.)
7. Code review focusing on error handling and edge cases

Potential Future Improvements

Once this refactoring is complete, consider:

• Adding typed errors for "not a git repository" errors (requires changes to modules/git/error.go)
• Extracting other duplicated repository context setup patterns
• Adding more comprehensive unit tests for repository context setup

---

Acceptance Criteria

• New SetupDefaultBranchContext() helper function added to services/context/repo.go
• RepoRefByType middleware refactored to use the helper
• ArticleView handler refactored to use the helper
• Unit tests added for the new helper function
• All existing integration tests pass
• Manual testing confirms no behavioral changes
• Code review approved
• Documentation updated (if needed)

---

Risk Level: Medium (affects core middleware, requires thorough testing)