Repository: kopia/kopia
Pull request #4421 (2025-02-21 02:11:34)

fix(snapshots): fix accounting for symlinks in directory summary
Fix propagation of symlink counts to parent directories. Add tests to check the directory summary has correct counts for symlinks and files.

Before the fix, symlink counts would not be passed to parent directories, resulting in summary data that do not reflect the count of symlinks in the directory tree.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/kopia/kopia/pull/4421?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 76.30%. Comparing base [(`cb455c6`)](https://app.codecov.io/gh/kopia/kopia/commit/cb455c699a04e54cb47e25a676d7d65063ce883e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) to head [(`6cb5ca0`)](https://app.codecov.io/gh/kopia/kopia/commit/6cb5ca0677fde03660c5e2d40fcfe2048cfd5145?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).
> Report is 424 commits behind head on master.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4421      +/-   ##
==========================================
+ Coverage   75.86%   76.30%   +0.43%     
==========================================
  Files         470      527      +57     
  Lines       37301    39934    +2633     
==========================================
+ Hits        28299    30472    +2173     
- Misses       7071     7454     +383     
- Partials     1931     2008      +77     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/kopia/kopia/pull/4421?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).


Patch:
diff --git a/snapshot/snapshotfs/dir_manifest_builder.go b/snapshot/snapshotfs/dir_manifest_builder.go
index 3f1163a8..875e0c25 100644
--- a/snapshot/snapshotfs/dir_manifest_builder.go
+++ b/snapshot/snapshotfs/dir_manifest_builder.go
@@ -54,6 +54,7 @@ func (b *DirManifestBuilder) AddEntry(de *snapshot.DirEntry) {
 			b.summary.TotalFileCount += childSummary.TotalFileCount
 			b.summary.TotalFileSize += childSummary.TotalFileSize
 			b.summary.TotalDirCount += childSummary.TotalDirCount
+			b.summary.TotalSymlinkCount += childSummary.TotalSymlinkCount
 			b.summary.FatalErrorCount += childSummary.FatalErrorCount
 			b.summary.IgnoredErrorCount += childSummary.IgnoredErrorCount
 			b.summary.FailedEntries = append(b.summary.FailedEntries, childSummary.FailedEntries...)


Test patch:
diff --git a/snapshot/snapshotfs/dir_manifest_builder_test.go b/snapshot/snapshotfs/dir_manifest_builder_test.go
new file mode 100644
index 00000000..9edaf884
--- /dev/null
+++ b/snapshot/snapshotfs/dir_manifest_builder_test.go
@@ -0,0 +1,94 @@
+package snapshotfs
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/kopia/kopia/fs"
+	"github.com/kopia/kopia/snapshot"
+)
+
+func TestAddEntry(t *testing.T) {
+	now := time.Date(2020, 1, 2, 3, 4, 5, 6, time.UTC)
+
+	for _, tc := range []struct {
+		desc     string
+		entry    *snapshot.DirEntry
+		expected fs.DirectorySummary
+	}{
+		{
+			desc: "Add file entry",
+			entry: &snapshot.DirEntry{
+				Name:     "file1",
+				Type:     snapshot.EntryTypeFile,
+				FileSize: 100,
+				ModTime:  fs.UTCTimestamp(now.Unix()),
+			},
+			expected: fs.DirectorySummary{
+				TotalFileCount: 1,
+				TotalFileSize:  100,
+				MaxModTime:     fs.UTCTimestamp(now.Unix()),
+			},
+		},
+		{
+			desc: "Add symlink entry",
+			entry: &snapshot.DirEntry{
+				Name:     "symlink1",
+				Type:     snapshot.EntryTypeSymlink,
+				FileSize: 50,
+				ModTime:  fs.UTCTimestamp(now.Add(1 * time.Hour).Unix()),
+			},
+			expected: fs.DirectorySummary{
+				TotalSymlinkCount: 1,
+				TotalFileSize:     50,
+				MaxModTime:        fs.UTCTimestamp(now.Add(1 * time.Hour).Unix()),
+			},
+		},
+		{
+			desc: "Add directory entry with child summary",
+			entry: &snapshot.DirEntry{
+				Name: "dir1",
+				Type: snapshot.EntryTypeDirectory,
+				DirSummary: &fs.DirectorySummary{
+					TotalFileCount:    2,
+					TotalFileSize:     200,
+					TotalDirCount:     1,
+					TotalSymlinkCount: 1,
+					FatalErrorCount:   1,
+					IgnoredErrorCount: 1,
+					MaxModTime:        fs.UTCTimestamp(now.Add(2 * time.Hour).Unix()),
+				},
+				ModTime: fs.UTCTimestamp(now.Add(1 * time.Hour).Unix()),
+			},
+			expected: fs.DirectorySummary{
+				TotalFileCount:    2,
+				TotalFileSize:     200,
+				TotalDirCount:     1,
+				TotalSymlinkCount: 1,
+				FatalErrorCount:   1,
+				IgnoredErrorCount: 1,
+				MaxModTime:        fs.UTCTimestamp(now.Add(2 * time.Hour).Unix()),
+			},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			builder := &DirManifestBuilder{}
+			builder.AddEntry(tc.entry)
+			require.Equal(t, tc.expected.TotalFileCount, builder.summary.TotalFileCount, "TotalFileCount mismatch")
+
+			if tc.entry.Type != snapshot.EntryTypeSymlink {
+				// Note: the file size is not being propagated for symlinks.
+				// For now, don't check the file size for symlinks.
+				require.Equal(t, tc.expected.TotalFileSize, builder.summary.TotalFileSize, "TotalFileSize mismatch")
+			}
+
+			require.Equal(t, tc.expected.TotalSymlinkCount, builder.summary.TotalSymlinkCount, "TotalSymlinkCount mismatch")
+			require.Equal(t, tc.expected.TotalDirCount, builder.summary.TotalDirCount, "TotalDirCount mismatch")
+			require.Equal(t, tc.expected.FatalErrorCount, builder.summary.FatalErrorCount, "FatalErrorCount mismatch")
+			require.Equal(t, tc.expected.IgnoredErrorCount, builder.summary.IgnoredErrorCount, "IgnoredErrorCount mismatch")
+			require.True(t, builder.summary.MaxModTime.Equal(tc.expected.MaxModTime), "MaxModTime mismatch: got %v, want %v", builder.summary.MaxModTime, tc.expected.MaxModTime)
+		})
+	}
+}
diff --git a/snapshot/snapshotfs/upload_test.go b/snapshot/snapshotfs/upload_test.go
index 5f109762..588e53a0 100644
--- a/snapshot/snapshotfs/upload_test.go
+++ b/snapshot/snapshotfs/upload_test.go
@@ -712,6 +712,45 @@ func TestUpload_FinishedFileProgress(t *testing.T) {
 	assert.Equal(t, 2, filesFinished, "FinishedFile calls")
 }
 
+func TestUpload_SymlinkStats(t *testing.T) {
+	ctx := testlogging.Context(t)
+	th := newUploadTestHarness(ctx, t)
+
+	root := mockfs.NewDirectory()
+	root.AddFile("f1", []byte{1, 2, 3}, defaultPermissions)
+	root.AddDir("d1", defaultPermissions)
+	root.AddDir("d1/d1", defaultPermissions)
+	root.AddFile("d1/d1/f1", []byte{1, 2, 3}, defaultPermissions)
+	root.AddSymlink("s1", "d1/d1/f1", defaultPermissions)
+	root.AddSymlink("s2", "f1", defaultPermissions)
+	root.AddSymlink("s3", "d1", defaultPermissions)
+
+	u := NewUploader(th.repo)
+	policyTree := policy.BuildTree(nil, policy.DefaultPolicy)
+
+	// First upload of the root directory.
+	man1, err := u.Upload(ctx, root, policyTree, snapshot.SourceInfo{})
+	require.NoError(t, err)
+
+	// Expect the directory summary to have the correct breakdown of files and symlinks.
+	require.Equal(t, int64(3), man1.RootEntry.DirSummary.TotalSymlinkCount, "Directory summary TotalSymlinkCount")
+	require.Equal(t, int64(2), man1.RootEntry.DirSummary.TotalFileCount, "Directory summary TotalSymlinkCount")
+
+	// Expect the directory summary total file size to match the stats total file size.
+	require.Equal(t, atomic.LoadInt64(&man1.Stats.TotalFileSize), man1.RootEntry.DirSummary.TotalFileSize, "Total file size")
+
+	// Upload a second time to check the stats from cached files.
+	man2, err := u.Upload(ctx, root, policyTree, snapshot.SourceInfo{}, man1)
+	require.NoError(t, err)
+
+	// Expect total file count for the second upload to be zero - all files are cached.
+	require.Equal(t, int32(0), atomic.LoadInt32(&man2.Stats.TotalFileCount), "Directory summary TotalFileCount")
+
+	// Expect the directory summary to have the correct breakdown of files and symlinks.
+	require.Equal(t, int64(3), man2.RootEntry.DirSummary.TotalSymlinkCount, "Directory summary TotalSymlinkCount")
+	require.Equal(t, int64(2), man2.RootEntry.DirSummary.TotalFileCount, "Directory summary TotalSymlinkCount")
+}
+
 func TestUploadWithCheckpointing(t *testing.T) {
 	ctx := testlogging.Context(t)
 	th := newUploadTestHarness(ctx, t)