Query and use the repository.branch when defined in spec (#393)

* Query and use the repository.branch when defined in spec

* Remove debug pritning

* Query specific commit and not query branches

* Merge two repository queries

* Fix critical indentation error (and unused type)

* Fix wrong query

* Add changelog entry

Author: mrnugget

CHANGELOG.md

@@ -19,6 +19,8 @@ All notable changes to `src-cli` are documented in this file.
 
 ### Fixed
 
+- The evaluation of the [`repository.branch` attribute](https://docs.sourcegraph.com/campaigns/references/campaign_spec_yaml_reference#on-repository) has been fixed to actually cause the correct version of the repository to be used. [#393](https://github.com/sourcegraph/src-cli/pull/393)
+
 ### Removed
 
 ## 3.22.3

internal/campaigns/archive_fetcher.go

@@ -120,12 +120,11 @@ func fetchRepositoryArchive(ctx context.Context, client api.Client, repo *graphq
 }
 
 func repositoryZipArchivePath(repo *graphql.Repository) string {
-	return path.Join("", repo.Name+"@"+repo.DefaultBranch.Name, "-", "raw")
+	return path.Join("", repo.Name+"@"+repo.BaseRef(), "-", "raw")
 }
 
 func localRepositoryZipArchivePath(dir string, repo *graphql.Repository) string {
-	ref := repo.DefaultBranch.Target.OID
-	return filepath.Join(dir, fmt.Sprintf("%s-%s.zip", repo.Slug(), ref))
+	return filepath.Join(dir, fmt.Sprintf("%s-%s.zip", repo.Slug(), repo.Rev()))
 }
 
 func unzip(zipFile, dest string) error {

internal/campaigns/archive_fetcher_test.go

@@ -30,7 +30,7 @@ func TestWorkspaceCreator_Create(t *testing.T) {
 	repo := &graphql.Repository{
 		ID:            "src-cli",
 		Name:          "github.com/sourcegraph/src-cli",
-		DefaultBranch: &graphql.Branch{Name: "main", Target: struct{ OID string }{OID: "d34db33f"}},
+		DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}},
 	}
 
 	archive := mockRepoArchive{
@@ -156,6 +156,44 @@ func TestWorkspaceCreator_Create(t *testing.T) {
 			t.Fatalf("zip file in temp dir was not cleaned up")
 		}
 	})
+
+	t.Run("non-default branch", func(t *testing.T) {
+		otherBranchOID := "f00b4r"
+		repo := &graphql.Repository{
+			ID:            "src-cli-with-non-main-branch",
+			Name:          "github.com/sourcegraph/src-cli",
+			DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}},
+
+			Commit: graphql.Target{OID: otherBranchOID},
+			Branch: graphql.Branch{Name: "other-branch", Target: graphql.Target{OID: otherBranchOID}},
+		}
+
+		archive := mockRepoArchive{repo: repo, files: map[string]string{}}
+
+		ts := httptest.NewServer(newZipArchivesMux(t, nil, archive))
+		defer ts.Close()
+
+		var clientBuffer bytes.Buffer
+		client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer})
+
+		testTempDir := workspaceTmpDir(t)
+
+		creator := &WorkspaceCreator{dir: testTempDir, client: client}
+
+		_, err := creator.Create(context.Background(), repo)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+
+		wantZipFile := "github.com-sourcegraph-src-cli-" + otherBranchOID + ".zip"
+		ok, err := dirContains(creator.dir, wantZipFile)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !ok {
+			t.Fatalf("temp dir doesnt contain zip file")
+		}
+	})
 }
 
 func TestMkdirAll(t *testing.T) {

internal/campaigns/executor_test.go

@@ -37,14 +37,14 @@ func TestExecutor_Integration(t *testing.T) {
 	srcCLIRepo := &graphql.Repository{
 		ID:            "src-cli",
 		Name:          "github.com/sourcegraph/src-cli",
-		DefaultBranch: &graphql.Branch{Name: "main", Target: struct{ OID string }{OID: "d34db33f"}},
+		DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}},
 	}
 	sourcegraphRepo := &graphql.Repository{
 		ID:   "sourcegraph",
 		Name: "github.com/sourcegraph/sourcegraph",
 		DefaultBranch: &graphql.Branch{
 			Name:   "main",
-			Target: struct{ OID string }{OID: "f00b4r3r"},
+			Target: graphql.Target{OID: "f00b4r3r"},
 		},
 	}
 
@@ -220,7 +220,7 @@ func newZipArchivesMux(t *testing.T, callback http.HandlerFunc, archives ...mock
 
 	for _, archive := range archives {
 		files := archive.files
-		path := fmt.Sprintf("/%s@%s/-/raw", archive.repo.Name, archive.repo.DefaultBranch.Name)
+		path := fmt.Sprintf("/%s@%s/-/raw", archive.repo.Name, archive.repo.BaseRef())
 
 		downloadName := filepath.Base(archive.repo.Name)
 		mediaType := mime.FormatMediaType("Attachment", map[string]string{

internal/campaigns/graphql/repository.go

@@ -19,29 +19,61 @@ fragment repositoryFields on Repository {
             oid
         }
     }
+    commit(rev: $rev) @include(if:$queryCommit) {
+        oid
+    }
 }
 `
 
+type Target struct {
+	OID string
+}
+
 type Branch struct {
 	Name   string
-	Target struct{ OID string }
+	Target Target
 }
 
 type Repository struct {
 	ID                 string
 	Name               string
 	URL                string
 	ExternalRepository struct{ ServiceType string }
-	DefaultBranch      *Branch
+
+	DefaultBranch *Branch
+
+	Commit Target
+	// Branch is populated by resolveRepositoryNameAndBranch with the queried
+	// branch's name and the contents of the Commit property.
+	Branch Branch
 
 	FileMatches map[string]bool
 }
 
+func (r *Repository) HasBranch() bool {
+	return r.DefaultBranch != nil || (r.Commit.OID != "" && r.Branch.Name != "")
+}
+
 func (r *Repository) BaseRef() string {
+	if r.Branch.Name != "" {
+		return ensurePrefix(r.Branch.Name)
+	}
+
 	return r.DefaultBranch.Name
 }
 
+func ensurePrefix(rev string) string {
+	if strings.HasPrefix(rev, "refs/heads/") {
+		return rev
+	}
+	return "refs/heads/" + rev
+}
+
 func (r *Repository) Rev() string {
+	if r.Branch.Target.OID != "" {
+		return r.Branch.Target.OID
+	}
+
 	return r.DefaultBranch.Target.OID
 }
 

internal/campaigns/service.go

@@ -363,8 +363,7 @@ func (svc *Service) ResolveNamespace(ctx context.Context, namespace string) (str
 }
 
 func (svc *Service) ResolveRepositories(ctx context.Context, spec *CampaignSpec) ([]*graphql.Repository, error) {
-	final := []*graphql.Repository{}
-	seen := map[string]struct{}{}
+	seen := map[string]*graphql.Repository{}
 	unsupported := UnsupportedRepoSet{}
 
 	// TODO: this could be trivially parallelised in the future.
@@ -375,11 +374,12 @@ func (svc *Service) ResolveRepositories(ctx context.Context, spec *CampaignSpec)
 		}
 
 		for _, repo := range repos {
-			if _, ok := seen[repo.ID]; !ok {
-				if repo.DefaultBranch == nil {
-					continue
-				}
-				seen[repo.ID] = struct{}{}
+			if !repo.HasBranch() {
+				continue
+			}
+
+			if other, ok := seen[repo.ID]; !ok {
+				seen[repo.ID] = repo
 				switch st := strings.ToLower(repo.ExternalRepository.ServiceType); st {
 				case "github", "gitlab", "bitbucketserver":
 				default:
@@ -388,12 +388,20 @@ func (svc *Service) ResolveRepositories(ctx context.Context, spec *CampaignSpec)
 						continue
 					}
 				}
-
-				final = append(final, repo)
+			} else {
+				// If we've already seen this repository, we overwrite the
+				// Commit/Branch fields with the latest value we have
+				other.Commit = repo.Commit
+				other.Branch = repo.Branch
 			}
 		}
 	}
 
+	final := make([]*graphql.Repository, 0, len(seen))
+	for _, repo := range seen {
+		final = append(final, repo)
+	}
+
 	if unsupported.hasUnsupported() && !svc.allowUnsupported {
 		return final, unsupported
 	}
@@ -404,6 +412,12 @@ func (svc *Service) ResolveRepositories(ctx context.Context, spec *CampaignSpec)
 func (svc *Service) ResolveRepositoriesOn(ctx context.Context, on *OnQueryOrRepository) ([]*graphql.Repository, error) {
 	if on.RepositoriesMatchingQuery != "" {
 		return svc.resolveRepositorySearch(ctx, on.RepositoriesMatchingQuery)
+	} else if on.Repository != "" && on.Branch != "" {
+		repo, err := svc.resolveRepositoryNameAndBranch(ctx, on.Repository, on.Branch)
+		if err != nil {
+			return nil, err
+		}
+		return []*graphql.Repository{repo}, nil
 	} else if on.Repository != "" {
 		repo, err := svc.resolveRepositoryName(ctx, on.Repository)
 		if err != nil {
@@ -418,7 +432,7 @@ func (svc *Service) ResolveRepositoriesOn(ctx context.Context, on *OnQueryOrRepo
 }
 
 const repositoryNameQuery = `
-query Repository($name: String!) {
+query Repository($name: String!, $queryCommit: Boolean!, $rev: String!) {
     repository(name: $name) {
         ...repositoryFields
     }
@@ -428,7 +442,9 @@ query Repository($name: String!) {
 func (svc *Service) resolveRepositoryName(ctx context.Context, name string) (*graphql.Repository, error) {
 	var result struct{ Repository *graphql.Repository }
 	if ok, err := svc.client.NewRequest(repositoryNameQuery, map[string]interface{}{
-		"name": name,
+		"name":        name,
+		"queryCommit": false,
+		"rev":         "",
 	}).Do(ctx, &result); err != nil || !ok {
 		return nil, err
 	}
@@ -438,10 +454,36 @@ func (svc *Service) resolveRepositoryName(ctx context.Context, name string) (*gr
 	return result.Repository, nil
 }
 
+func (svc *Service) resolveRepositoryNameAndBranch(ctx context.Context, name, branch string) (*graphql.Repository, error) {
+	var result struct{ Repository *graphql.Repository }
+	if ok, err := svc.client.NewRequest(repositoryNameQuery, map[string]interface{}{
+		"name":        name,
+		"queryCommit": true,
+		"rev":         branch,
+	}).Do(ctx, &result); err != nil || !ok {
+		return nil, err
+	}
+	if result.Repository == nil {
+		return nil, errors.New("no repository found")
+	}
+	if result.Repository.Commit.OID == "" {
+		return nil, fmt.Errorf("no branch matching %q found for repository %s", branch, name)
+	}
+
+	result.Repository.Branch = graphql.Branch{
+		Name:   branch,
+		Target: result.Repository.Commit,
+	}
+
+	return result.Repository, nil
+}
+
 // TODO: search result alerts.
 const repositorySearchQuery = `
 query ChangesetRepos(
     $query: String!,
+	$queryCommit: Boolean!,
+	$rev: String!,
 ) {
     search(query: $query, version: V2) {
         results {
@@ -471,7 +513,9 @@ func (svc *Service) resolveRepositorySearch(ctx context.Context, query string) (
 		}
 	}
 	if ok, err := svc.client.NewRequest(repositorySearchQuery, map[string]interface{}{
-		"query": setDefaultQueryCount(query),
+		"query":       setDefaultQueryCount(query),
+		"queryCommit": false,
+		"rev":         "",
 	}).Do(ctx, &result); err != nil || !ok {
 		return nil, err
 	}