fix(SourceHydrator): implement new api param for dry hydrated source and reverted to orig protobuff order and used struct as map key (#3)

Signed-off-by: Elias Rami <[email protected]>

Author: Elyytscha

assets/swagger.json

@@ -296,11 +296,18 @@ func (h *Hydrator) getAppsForHydrationKey(hydrationKey types.HydrationQueueKey)
 	return relevantApps, nil
 }
 
+// hydrationDestKey is a struct key for tracking unique hydration destinations.
+// Using a struct instead of string formatting better communicates intent and guarantees no collisions.
+type hydrationDestKey struct {
+	repoURL string
+	path    string
+}
+
 // validateApplications checks that all applications are valid for hydration.
 func (h *Hydrator) validateApplications(apps []*appv1.Application) (map[string]*appv1.AppProject, map[string]error) {
 	projects := make(map[string]*appv1.AppProject)
 	errors := make(map[string]error)
-	uniquePaths := make(map[string]string, len(apps))
+	uniquePaths := make(map[hydrationDestKey]string, len(apps))
 
 	for _, app := range apps {
 		// Get the project for the app and validate if the app is allowed to use the source.
@@ -338,11 +345,11 @@ func (h *Hydrator) validateApplications(apps []*appv1.Application) (map[string]*
 
 		// TODO: test the dupe detection
 		// TODO: normalize the path to avoid "path/.." from being treated as different from "."
-		// Use a combination of repo URL and path for uniqueness since apps can hydrate to different repos
-		destKey := fmt.Sprintf("%s:%s", hydrateToSource.RepoURL, destPath)
+		// Use a struct key for uniqueness since apps can hydrate to different repos
+		destKey := hydrationDestKey{repoURL: hydrateToSource.RepoURL, path: destPath}
 		if appName, ok := uniquePaths[destKey]; ok {
-			errors[app.QualifiedName()] = fmt.Errorf("app %s hydrator use the same destination: %v", appName, destKey)
-			errors[appName] = fmt.Errorf("app %s hydrator use the same destination: %v", app.QualifiedName(), destKey)
+			errors[app.QualifiedName()] = fmt.Errorf("app %s hydrator uses the same destination: repo=%s, path=%s", appName, destKey.repoURL, destKey.path)
+			errors[appName] = fmt.Errorf("app %s hydrator uses the same destination: repo=%s, path=%s", app.QualifiedName(), destKey.repoURL, destKey.path)
 			continue
 		}
 		uniquePaths[destKey] = app.QualifiedName()

controller/hydrator/hydrator.go

@@ -780,8 +780,8 @@ func TestValidateApplications_DuplicateDestination(t *testing.T) {
 	projects, errs := h.validateApplications([]*v1alpha1.Application{app1, app2})
 	require.Nil(t, projects)
 	require.Len(t, errs, 2)
-	require.ErrorContains(t, errs[app1.QualifiedName()], "app default/app2 hydrator use the same destination")
-	require.ErrorContains(t, errs[app2.QualifiedName()], "app default/app1 hydrator use the same destination")
+	require.ErrorContains(t, errs[app1.QualifiedName()], "app default/app2 hydrator uses the same destination")
+	require.ErrorContains(t, errs[app2.QualifiedName()], "app default/app1 hydrator uses the same destination")
 }
 
 func TestValidateApplications_Success(t *testing.T) {
@@ -1224,3 +1224,97 @@ func TestGetSyncSource_FallsBackToDrySourceRepo(t *testing.T) {
 	assert.Equal(t, "app", source.Path)
 	assert.Equal(t, "hydrated", source.TargetRevision)
 }
+
+// TestGetSyncSource_FallsBackToDrySourceRepoAndPath tests that when both RepoURL and Path
+// are empty in SyncSource, it falls back to both DrySource.RepoURL and DrySource.Path.
+func TestGetSyncSource_FallsBackToDrySourceRepoAndPath(t *testing.T) {
+	hydrator := &v1alpha1.SourceHydrator{
+		DrySource: v1alpha1.DrySource{
+			RepoURL:        "https://example.com/dry-repo",
+			TargetRevision: "main",
+			Path:           "base",
+		},
+		SyncSource: v1alpha1.SyncSource{
+			TargetBranch: "hydrated",
+			// RepoURL and Path are empty, should fall back to DrySource
+		},
+	}
+
+	source := hydrator.GetSyncSource()
+	assert.Equal(t, "https://example.com/dry-repo", source.RepoURL)
+	assert.Equal(t, "base", source.Path)
+	assert.Equal(t, "hydrated", source.TargetRevision)
+}
+
+// TestGetHydrateToSource_FallsBackToDrySourceRepo tests that when only RepoURL is empty
+// in SyncSource but Path is set, it only falls back to DrySource.RepoURL (not Path).
+func TestGetHydrateToSource_FallsBackToDrySourceRepo(t *testing.T) {
+	spec := &v1alpha1.ApplicationSpec{
+		SourceHydrator: &v1alpha1.SourceHydrator{
+			DrySource: v1alpha1.DrySource{
+				RepoURL:        "https://example.com/dry-repo",
+				TargetRevision: "main",
+				Path:           "base",
+			},
+			SyncSource: v1alpha1.SyncSource{
+				TargetBranch: "hydrated",
+				Path:         "app",
+				// RepoURL is empty, should fall back to DrySource.RepoURL
+			},
+		},
+	}
+
+	source := spec.GetHydrateToSource()
+	assert.Equal(t, "https://example.com/dry-repo", source.RepoURL)
+	assert.Equal(t, "app", source.Path) // Path should NOT fall back since it's set
+	assert.Equal(t, "hydrated", source.TargetRevision)
+}
+
+// TestGetHydrateToSource_FallsBackToDrySourceRepoAndPath tests that when both RepoURL
+// and Path are empty in SyncSource, it falls back to both DrySource values.
+func TestGetHydrateToSource_FallsBackToDrySourceRepoAndPath(t *testing.T) {
+	spec := &v1alpha1.ApplicationSpec{
+		SourceHydrator: &v1alpha1.SourceHydrator{
+			DrySource: v1alpha1.DrySource{
+				RepoURL:        "https://example.com/dry-repo",
+				TargetRevision: "main",
+				Path:           "base",
+			},
+			SyncSource: v1alpha1.SyncSource{
+				TargetBranch: "hydrated",
+				// RepoURL and Path are empty, should fall back to DrySource
+			},
+		},
+	}
+
+	source := spec.GetHydrateToSource()
+	assert.Equal(t, "https://example.com/dry-repo", source.RepoURL)
+	assert.Equal(t, "base", source.Path)
+	assert.Equal(t, "hydrated", source.TargetRevision)
+}
+
+// TestGetHydrateToSource_UsesHydrateToTargetBranch tests that when HydrateTo is set,
+// its TargetBranch is used instead of SyncSource.TargetBranch.
+func TestGetHydrateToSource_UsesHydrateToTargetBranch(t *testing.T) {
+	spec := &v1alpha1.ApplicationSpec{
+		SourceHydrator: &v1alpha1.SourceHydrator{
+			DrySource: v1alpha1.DrySource{
+				RepoURL:        "https://example.com/dry-repo",
+				TargetRevision: "main",
+				Path:           "base",
+			},
+			SyncSource: v1alpha1.SyncSource{
+				TargetBranch: "hydrated",
+				// RepoURL and Path are empty
+			},
+			HydrateTo: &v1alpha1.HydrateTo{
+				TargetBranch: "staging",
+			},
+		},
+	}
+
+	source := spec.GetHydrateToSource()
+	assert.Equal(t, "https://example.com/dry-repo", source.RepoURL)
+	assert.Equal(t, "base", source.Path)
+	assert.Equal(t, "staging", source.TargetRevision) // Uses HydrateTo.TargetBranch
+}