feat(Provider): Add ResolveSourceIdentity() to the source provider interface (#45)

Author: dmcilvaney

internal/providers/sourceproviders/fedorasourceprovider.go

@@ -221,54 +221,142 @@ func (g *FedoraSourcesProviderImpl) renameSpecIfNeeded(dir, upstreamName, compon
 	return nil
 }
 
-// checkoutTargetCommit determines the appropriate commit to use and checks it out.
-// Priority order:
-//  1. Explicit upstream commit hash - specified per-component via upstream-commit
-//  2. Upstream distro snapshot - snapshot time from the provider's resolved distro
-//  3. Default - use current HEAD (no checkout needed)
+// checkoutTargetCommit resolves the effective commit via [resolveEffectiveCommitHash]
+// and checks it out in the cloned repository.
 func (g *FedoraSourcesProviderImpl) checkoutTargetCommit(
 	ctx context.Context,
 	upstreamCommit string,
 	repoDir string,
 ) error {
+	commitHash, err := g.resolveEffectiveCommitHash(ctx, repoDir, upstreamCommit, slog.LevelInfo)
+	if err != nil {
+		return err
+	}
+
+	if err := g.gitProvider.Checkout(ctx, repoDir, commitHash); err != nil {
+		return fmt.Errorf("failed to checkout commit %#q:\n%w", commitHash, err)
+	}
+
+	return nil
+}
+
+// ResolveIdentity implements [SourceIdentityProvider] by resolving the upstream
+// commit hash for the component. All resolution priority logic is in
+// [resolveEffectiveCommitHash], called via [resolveCommit].
+func (g *FedoraSourcesProviderImpl) ResolveIdentity(
+	ctx context.Context,
+	component components.Component,
+) (string, error) {
+	if component.GetName() == "" {
+		return "", errors.New("component name cannot be empty")
+	}
+
+	upstreamName := component.GetConfig().Spec.UpstreamName
+	if upstreamName == "" {
+		upstreamName = component.GetName()
+	}
+
+	gitRepoURL := strings.ReplaceAll(g.distroGitBaseURI, "$pkg", upstreamName)
+
+	return g.resolveCommit(ctx, gitRepoURL, upstreamName, component.GetConfig().Spec.UpstreamCommit)
+}
+
+// resolveCommit determines the effective commit via [resolveEffectiveCommitHash].
+// For pinned commits (case 1), it returns immediately without cloning. For snapshot
+// and HEAD cases, it performs a metadata-only clone to resolve the commit hash.
+func (g *FedoraSourcesProviderImpl) resolveCommit(
+	ctx context.Context, gitRepoURL string, upstreamName string, upstreamCommit string,
+) (string, error) {
 	// Case 1: Explicit upstream commit hash specified per-component
 	if upstreamCommit != "" {
-		slog.Info("Using explicit upstream commit hash",
-			"commitHash", upstreamCommit)
+		return g.resolveEffectiveCommitHash(ctx, "", upstreamCommit, slog.LevelDebug)
+	}
 
-		if err := g.gitProvider.Checkout(ctx, repoDir, upstreamCommit); err != nil {
-			return fmt.Errorf("failed to checkout upstream commit %#q:\n%w", upstreamCommit, err)
+	// Cases 2 & 3: need a metadata-only clone to resolve snapshot or HEAD commit.
+	tempDir, err := fileutils.MkdirTempInTempDir(g.fs, "azldev-identity-snapshot-")
+	if err != nil {
+		return "", fmt.Errorf("creating temp directory for snapshot clone:\n%w", err)
+	}
+
+	defer func() {
+		if removeErr := g.fs.RemoveAll(tempDir); removeErr != nil {
+			slog.Debug("Failed to clean up snapshot clone temp directory",
+				"path", tempDir, "error", removeErr)
 		}
+	}()
 
-		return nil
+	// Clone a single branch to resolve the snapshot commit. We use a full
+	// (non-shallow) clone because not all git servers support --shallow-since
+	// (e.g., Pagure returns "the remote end hung up unexpectedly").
+	err = retry.Do(ctx, g.retryConfig, func() error {
+		_ = g.fs.RemoveAll(tempDir)
+		_ = fileutils.MkdirAll(g.fs, tempDir)
+
+		return g.gitProvider.Clone(ctx, gitRepoURL, tempDir,
+			git.WithGitBranch(g.distroGitBranch),
+			git.WithMetadataOnly(),
+			git.WithQuiet(),
+		)
+	})
+	if err != nil {
+		return "", fmt.Errorf("partial clone for identity of %#q:\n%w", upstreamName, err)
+	}
+
+	commitHash, err := g.resolveEffectiveCommitHash(ctx, tempDir, "", slog.LevelDebug)
+	if err != nil {
+		return "", fmt.Errorf("resolving commit for %#q:\n%w", upstreamName, err)
+	}
+
+	return commitHash, nil
+}
+
+// resolveEffectiveCommitHash is the single source of truth for which commit a
+// component should use from a cloned repository.
+//
+// Priority:
+//  1. Explicit upstream commit hash (pinned per-component).
+//  2. Snapshot time — commit immediately before the snapshot date.
+//  3. Default — current HEAD.
+func (g *FedoraSourcesProviderImpl) resolveEffectiveCommitHash(
+	ctx context.Context,
+	repoDir string,
+	upstreamCommit string,
+	logLevel slog.Level,
+) (string, error) {
+	// Case 1: Explicit upstream commit hash specified per-component.
+	if upstreamCommit != "" {
+		slog.Log(ctx, logLevel, "Using explicit upstream commit hash", "commitHash", upstreamCommit)
+
+		return upstreamCommit, nil
 	}
 
-	// Case 2: Provider has a snapshot time configured from the resolved distro
+	// Case 2: Provider has a snapshot time configured from the resolved distro.
 	if g.snapshotTime != "" {
 		snapshotDateTime, err := time.Parse(time.RFC3339, g.snapshotTime)
 		if err != nil {
-			return fmt.Errorf("invalid snapshot time %#q:\n%w", g.snapshotTime, err)
+			return "", fmt.Errorf("invalid snapshot time %#q:\n%w", g.snapshotTime, err)
 		}
 
 		commitHash, err := g.gitProvider.GetCommitHashBeforeDate(ctx, repoDir, snapshotDateTime)
 		if err != nil {
-			return fmt.Errorf("failed to get commit hash for snapshot time %s:\n%w",
+			return "", fmt.Errorf("resolving commit for snapshot time %s:\n%w",
 				snapshotDateTime.Format(time.RFC3339), err)
 		}
 
-		slog.Info("Using upstream distro snapshot time",
+		slog.Log(ctx, logLevel, "Using upstream distro snapshot time",
 			"snapshotDateTime", snapshotDateTime.Format(time.RFC3339),
 			"commitHash", commitHash)
 
-		if err := g.gitProvider.Checkout(ctx, repoDir, commitHash); err != nil {
-			return fmt.Errorf("failed to checkout snapshot commit %#q:\n%w", commitHash, err)
-		}
+		return commitHash, nil
+	}
 
-		return nil
+	// Case 3: Default — use current HEAD.
+	commitHash, err := g.gitProvider.GetCurrentCommit(ctx, repoDir)
+	if err != nil {
+		return "", fmt.Errorf("resolving current HEAD commit:\n%w", err)
 	}
 
-	// Case 3: Default - use current HEAD (already checked out by clone)
-	slog.Info("Using current HEAD (no snapshot time configured)")
+	slog.Log(ctx, logLevel, "Using current HEAD", "commitHash", commitHash)
 
-	return nil
+	return commitHash, nil
 }

internal/providers/sourceproviders/fedorasourceprovider_test.go

@@ -169,6 +169,14 @@ func TestGetComponentFromGit(t *testing.T) {
 				return fileutils.WriteFile(env.FS(), sourcesPath, []byte(sourcesContent), fileperms.PublicFile)
 			})
 
+		mockGitProvider.EXPECT().
+			GetCurrentCommit(gomock.Any(), gomock.Any()).
+			Return("head123abc", nil)
+
+		mockGitProvider.EXPECT().
+			Checkout(gomock.Any(), gomock.Any(), "head123abc").
+			Return(nil)
+
 		provider, err := sourceproviders.NewFedoraSourcesProviderImpl(
 			env.FS(),
 			env.DryRunnable,
@@ -255,6 +263,14 @@ func TestGetComponentFromGit(t *testing.T) {
 				return fileutils.WriteFile(env.FS(), sourcesPath, []byte(sourcesContent), fileperms.PublicFile)
 			})
 
+		mockGitProvider.EXPECT().
+			GetCurrentCommit(gomock.Any(), gomock.Any()).
+			Return("head123abc", nil)
+
+		mockGitProvider.EXPECT().
+			Checkout(gomock.Any(), gomock.Any(), "head123abc").
+			Return(nil)
+
 		const testDestDir = "/output-preexisting"
 
 		provider, err := sourceproviders.NewFedoraSourcesProviderImpl(
@@ -323,6 +339,14 @@ func TestGetComponentFromGit(t *testing.T) {
 				return fileutils.WriteFile(env.FS(), specPath, []byte("Name: "+testPackageName), fileperms.PublicFile)
 			})
 
+		mockGitProvider.EXPECT().
+			GetCurrentCommit(gomock.Any(), gomock.Any()).
+			Return("head123abc", nil)
+
+		mockGitProvider.EXPECT().
+			Checkout(gomock.Any(), gomock.Any(), "head123abc").
+			Return(nil)
+
 		provider, err := sourceproviders.NewFedoraSourcesProviderImpl(
 			env.FS(),
 			env.DryRunnable,
@@ -437,6 +461,14 @@ func TestGetComponentFromGit(t *testing.T) {
 			Clone(gomock.Any(), repoURL, gomock.Any(), gomock.Any()).
 			Return(nil)
 
+		mockGitProvider.EXPECT().
+			GetCurrentCommit(gomock.Any(), gomock.Any()).
+			Return("head123abc", nil)
+
+		mockGitProvider.EXPECT().
+			Checkout(gomock.Any(), gomock.Any(), "head123abc").
+			Return(nil)
+
 		// But extractor fails - note it receives the component name, not destDir
 		extractorError := errors.New("extraction failed")
 		mockExtractor.EXPECT().
@@ -490,6 +522,14 @@ func TestGetComponentFromGit(t *testing.T) {
 				)
 			})
 
+		mockGitProvider.EXPECT().
+			GetCurrentCommit(gomock.Any(), gomock.Any()).
+			Return("head123abc", nil)
+
+		mockGitProvider.EXPECT().
+			Checkout(gomock.Any(), gomock.Any(), "head123abc").
+			Return(nil)
+
 		// Extractor succeeds
 		mockExtractor.EXPECT().
 			ExtractSourcesFromRepo(gomock.Any(), gomock.Any(), upstreamName, gomock.Any(), gomock.Any()).
@@ -544,6 +584,14 @@ func TestGetComponentFromGit(t *testing.T) {
 			Clone(gomock.Any(), upstreamRepoURL, gomock.Any(), gomock.Any()).
 			Return(nil)
 
+		mockGitProvider.EXPECT().
+			GetCurrentCommit(gomock.Any(), gomock.Any()).
+			Return("head123abc", nil)
+
+		mockGitProvider.EXPECT().
+			Checkout(gomock.Any(), gomock.Any(), "head123abc").
+			Return(nil)
+
 		// Extractor succeeds
 		mockExtractor.EXPECT().
 			ExtractSourcesFromRepo(gomock.Any(), gomock.Any(), upstreamName, gomock.Any(), gomock.Any()).
@@ -652,7 +700,17 @@ func TestCheckoutTargetCommit(t *testing.T) {
 				return fileutils.WriteFile(env.FS(), specPath, []byte("Name: "+testPackageName), fileperms.PublicFile)
 			})
 
-		// Should NOT call GetCommitHashBeforeDate or Checkout - uses HEAD from clone
+		headCommitHash := "head123abc"
+
+		// GetCurrentCommit is called to resolve HEAD
+		mockGitProvider.EXPECT().
+			GetCurrentCommit(gomock.Any(), gomock.Any()).
+			Return(headCommitHash, nil)
+
+		// Then checkout that commit
+		mockGitProvider.EXPECT().
+			Checkout(gomock.Any(), gomock.Any(), headCommitHash).
+			Return(nil)
 
 		// Extractor succeeds
 		mockExtractor.EXPECT().
@@ -700,7 +758,7 @@ func TestCheckoutTargetCommit(t *testing.T) {
 
 		err = provider.GetComponent(context.Background(), mockComponent, destDir)
 		require.Error(t, err)
-		assert.Contains(t, err.Error(), "failed to get commit hash for snapshot time")
+		assert.Contains(t, err.Error(), "resolving commit for snapshot time")
 		assert.ErrorIs(t, err, hashError)
 	})
 
@@ -747,7 +805,7 @@ func TestCheckoutTargetCommit(t *testing.T) {
 
 		err = provider.GetComponent(context.Background(), mockComponent, destDir)
 		require.Error(t, err)
-		assert.Contains(t, err.Error(), "failed to checkout snapshot commit")
+		assert.Contains(t, err.Error(), "failed to checkout commit")
 		assert.ErrorIs(t, err, checkoutError)
 	})
 
@@ -929,7 +987,7 @@ func TestCheckoutTargetCommit_UpstreamCommit(t *testing.T) {
 
 		err = provider.GetComponent(context.Background(), mockComponent, destDir)
 		require.Error(t, err)
-		assert.Contains(t, err.Error(), "failed to checkout upstream commit")
+		assert.Contains(t, err.Error(), "failed to checkout commit")
 		assert.ErrorIs(t, err, checkoutError)
 	})
 }