bundles: write bundle lists for different URIs

The bundle list for a given 'git-bundle-web-server' is intended to be served
from the route 'http[s]://<host>:<port>/<org>/<repo>', with or without a
trailing slash. The bundles listed are provided as paths relative to
'/<org>/<repo>'. However, these relative paths only work when there's a
trailing slash after '<repo>' in the URI; otherwise '<repo>' is interpreted
as a file in directory '<org>'. For example:

- URI: https://localhost:8080/git/git/, bundle relpath: bundle-1.bundle
  -> https://localhost:8080/git/git/bundle-1.bundle
- URI: https://localhost:8080/git/git, bundle relpath: bundle-2.bundle
  -> https://localhost:8080/git/bundle-2.bundle

This behavior is baked into Git, and is consistent with RFC 1808 [1]. We
still want to be able to serve the bundle list when no trailing slash is
given, though, so write out a second bundle list that prepends '<repo>' to
the bundle filename. In a later commit, we will update the web server to
serve this file's contents when appropriate, fixing the routing bug.

As part of this refactor of 'BundleProvider.WriteBundleList()', switch to
using 'FileSystem.WriteLockFileFunc()' to write lock files. Doing so allows
us to mock filesystem writes and unit test 'WriteBundleList()', which is
done here as well.

[1] https://www.rfc-editor.org/rfc/rfc1808#section-4

Signed-off-by: Victoria Dye <vdye@github.com>

Author: vdye

internal/bundles/bundles.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"path"
 	"path/filepath"
@@ -13,11 +14,18 @@ import (
 	"strings"
 	"time"
 
+	"github.com/github/git-bundle-server/internal/common"
 	"github.com/github/git-bundle-server/internal/core"
 	"github.com/github/git-bundle-server/internal/git"
 	"github.com/github/git-bundle-server/internal/log"
 )
 
+const (
+	BundleListJsonFilename string = "bundle-list.json"
+	BundleListFilename     string = "bundle-list"
+	RepoBundleListFilename string = "repo-bundle-list"
+)
+
 type BundleHeader struct {
 	Version int64
 
@@ -30,15 +38,21 @@ type BundleHeader struct {
 }
 
 type Bundle struct {
-	URI           string
-	Filename      string
+	// The absolute path to the bundle from the root of the bundle web server,
+	// typically '/org/route/filename'.
+	URI string
+
+	// The absolute path to the bundle on disk
+	Filename string
+
+	// The creation token used in Git's 'creationToken' heuristic
 	CreationToken int64
 }
 
 func NewBundle(repo *core.Repository, timestamp int64) Bundle {
 	bundleName := fmt.Sprintf("bundle-%d.bundle", timestamp)
 	return Bundle{
-		URI:           path.Join(".", bundleName),
+		URI:           path.Join("/", repo.Route, bundleName),
 		Filename:      filepath.Join(repo.WebDir, bundleName),
 		CreationToken: timestamp,
 	}
@@ -76,17 +90,20 @@ type BundleProvider interface {
 }
 
 type bundleProvider struct {
-	logger    log.TraceLogger
-	gitHelper git.GitHelper
+	logger     log.TraceLogger
+	fileSystem common.FileSystem
+	gitHelper  git.GitHelper
 }
 
 func NewBundleProvider(
 	l log.TraceLogger,
+	fs common.FileSystem,
 	g git.GitHelper,
 ) BundleProvider {
 	return &bundleProvider{
-		logger:    l,
-		gitHelper: g,
+		logger:     l,
+		fileSystem: fs,
+		gitHelper:  g,
 	}
 }
 
@@ -115,78 +132,128 @@ func (b *bundleProvider) CreateSingletonList(ctx context.Context, bundle Bundle)
 	return &list
 }
 
-// Given a BundleList
+// Given a BundleList, write the bundle list content to the web directory.
 func (b *bundleProvider) WriteBundleList(ctx context.Context, list *BundleList, repo *core.Repository) error {
 	//lint:ignore SA4006 always override the ctx with the result from 'Region()'
 	ctx, exitRegion := b.logger.Region(ctx, "bundles", "write_bundle_list")
 	defer exitRegion()
 
-	listFile := repo.WebDir + "/bundle-list"
-	jsonFile := repo.RepoDir + "/bundle-list.json"
-
-	// TODO: Formalize lockfile concept.
-	f, err := os.OpenFile(listFile+".lock", os.O_WRONLY|os.O_CREATE, 0o600)
-	if err != nil {
-		return fmt.Errorf("failure to open file: %w", err)
+	var listLockFile, repoListLockFile, jsonLockFile common.LockFile
+	rollbackAll := func() {
+		if listLockFile != nil {
+			listLockFile.Rollback()
+		}
+		if repoListLockFile != nil {
+			repoListLockFile.Rollback()
+		}
+		if jsonLockFile != nil {
+			jsonLockFile.Rollback()
+		}
 	}
 
-	out := bufio.NewWriter(f)
-
-	fmt.Fprintf(
-		out, "[bundle]\n\tversion = %d\n\tmode = %s\n\n",
-		list.Version, list.Mode)
-
+	// Write the bundle list files: one for requests with a trailing slash
+	// (where the relative bundle paths are '<bundlefile>'), one for requests
+	// without a trailing slash (where the relative bundle paths are
+	// '<repo>/<bundlefile>').
 	keys := list.sortedCreationTokens()
+	writeListFile := func(f io.Writer, requestUri string) error {
+		out := bufio.NewWriter(f)
+		defer out.Flush()
 
-	for _, token := range keys {
-		bundle := list.Bundles[token]
 		fmt.Fprintf(
-			out, "[bundle \"%d\"]\n\turi = %s\n\tcreationToken = %d\n\n",
-			token, bundle.URI, token)
+			out, "[bundle]\n\tversion = %d\n\tmode = %s\n\n",
+			list.Version, list.Mode)
+
+		uriBase := path.Dir(requestUri) + "/"
+		for _, token := range keys {
+			bundle := list.Bundles[token]
+
+			// Get the URI relative to the bundle server root
+			uri := strings.TrimPrefix(bundle.URI, uriBase)
+			if uri == bundle.URI {
+				panic("error resolving bundle URI paths")
+			}
+
+			fmt.Fprintf(
+				out, "[bundle \"%d\"]\n\turi = %s\n\tcreationToken = %d\n\n",
+				token, uri, token)
+		}
+		return nil
 	}
 
-	out.Flush()
-	err = f.Close()
+	listLockFile, err := b.fileSystem.WriteLockFileFunc(
+		filepath.Join(repo.WebDir, BundleListFilename),
+		func(f io.Writer) error {
+			return writeListFile(f, path.Join("/", repo.Route)+"/")
+		},
+	)
 	if err != nil {
-		return fmt.Errorf("failed to close lock file: %w", err)
+		rollbackAll()
+		return err
 	}
 
-	f, err = os.OpenFile(jsonFile+".lock", os.O_WRONLY|os.O_CREATE, 0o600)
+	repoListLockFile, err = b.fileSystem.WriteLockFileFunc(
+		filepath.Join(repo.WebDir, RepoBundleListFilename),
+		func(f io.Writer) error {
+			return writeListFile(f, path.Join("/", repo.Route))
+		},
+	)
 	if err != nil {
-		return fmt.Errorf("failed to open JSON file: %w", err)
+		rollbackAll()
+		return err
 	}
 
-	data, jsonErr := json.Marshal(list)
-	if jsonErr != nil {
-		return fmt.Errorf("failed to convert list to JSON: %w", err)
+	// Write the (internal-use) JSON representation of the bundle list
+	jsonLockFile, err = b.fileSystem.WriteLockFileFunc(
+		filepath.Join(repo.RepoDir, BundleListJsonFilename),
+		func(f io.Writer) error {
+			data, err := json.Marshal(list)
+			if err != nil {
+				return fmt.Errorf("failed to convert list to JSON: %w", err)
+			}
+
+			written := 0
+			for written < len(data) {
+				n, writeErr := f.Write(data[written:])
+				if writeErr != nil {
+					return fmt.Errorf("failed to write JSON: %w", err)
+				}
+				written += n
+			}
+
+			return nil
+		},
+	)
+	if err != nil {
+		rollbackAll()
+		return err
 	}
 
-	written := 0
-	for written < len(data) {
-		n, writeErr := f.Write(data[written:])
-		if writeErr != nil {
-			return fmt.Errorf("failed to write JSON: %w", err)
-		}
-		written += n
+	// Commit all lockfiles
+	err = jsonLockFile.Commit()
+	if err != nil {
+		return fmt.Errorf("failed to rename JSON file: %w", err)
 	}
 
-	f.Sync()
-	f.Close()
+	err = listLockFile.Commit()
+	if err != nil {
+		return fmt.Errorf("failed to rename bundle list file: %w", err)
+	}
 
-	renameErr := os.Rename(jsonFile+".lock", jsonFile)
-	if renameErr != nil {
-		return fmt.Errorf("failed to rename JSON file: %w", renameErr)
+	err = repoListLockFile.Commit()
+	if err != nil {
+		return fmt.Errorf("failed to rename repo-level bundle list file: %w", err)
 	}
 
-	return os.Rename(listFile+".lock", listFile)
+	return nil
 }
 
 func (b *bundleProvider) GetBundleList(ctx context.Context, repo *core.Repository) (*BundleList, error) {
 	//lint:ignore SA4006 always override the ctx with the result from 'Region()'
 	ctx, exitRegion := b.logger.Region(ctx, "bundles", "get_bundle_list")
 	defer exitRegion()
 
-	jsonFile := repo.RepoDir + "/bundle-list.json"
+	jsonFile := filepath.Join(repo.RepoDir, BundleListJsonFilename)
 
 	reader, err := os.Open(jsonFile)
 	if err != nil {