Repository: kopia/kopia
Pull request #2690 (2023-01-12 19:45:15)

fix(snapshots): cached stream file size
When checking if an entry is cached and using StreamingFile, source the file size from the cached entry. StreamingFiles don't know their size until they are read so this fixes the issue where files would have 0 size but still have their full content. Also fixes stats reporting for StreamingFiles

codecov[bot]: # [Codecov](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) Report
Base: **74.41**% // Head: **75.02**% // Increases project coverage by **`+0.60%`** :tada:
> Coverage data is based on head [(`ead6700`)](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) compared to base [(`5227d74`)](https://codecov.io/gh/kopia/kopia/commit/5227d74996b6520f9f96e4203cfe00b832a60d5f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).
> Patch coverage: 61.53% of modified lines in pull request are covered.

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


```diff
@@            Coverage Diff             @@
##           master    #2690      +/-   ##
==========================================
+ Coverage   74.41%   75.02%   +0.60%     
==========================================
  Files         442      457      +15     
  Lines       34974    35820     +846     
==========================================
+ Hits        26026    26873     +847     
  Misses       7051     7051              
+ Partials     1897     1896       -1     
```


| [Impacted Files](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) | Coverage Δ | |
|---|---|---|
| [snapshot/snapshotfs/upload.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-c25hcHNob3Qvc25hcHNob3Rmcy91cGxvYWQuZ28=) | `85.20% <61.53%> (+0.85%)` | :arrow_up: |
| [repo/content/caching\_options.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9jb250ZW50L2NhY2hpbmdfb3B0aW9ucy5nbw==) | `60.00% <0.00%> (-40.00%)` | :arrow_down: |
| [repo/repository.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9yZXBvc2l0b3J5Lmdv) | `80.48% <0.00%> (-2.85%)` | :arrow_down: |
| [...lob/filesystem/filesystem\_storage\_capacity\_unix.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9ibG9iL2ZpbGVzeXN0ZW0vZmlsZXN5c3RlbV9zdG9yYWdlX2NhcGFjaXR5X3VuaXguZ28=) | `70.00% <0.00%> (-2.73%)` | :arrow_down: |
| [repo/blob/retrying/retrying\_storage.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9ibG9iL3JldHJ5aW5nL3JldHJ5aW5nX3N0b3JhZ2UuZ28=) | `79.54% <0.00%> (-2.46%)` | :arrow_down: |
| [repo/api\_server\_repository.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9hcGlfc2VydmVyX3JlcG9zaXRvcnkuZ28=) | `64.65% <0.00%> (-2.34%)` | :arrow_down: |
| [repo/content/content\_manager\_indexes.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9jb250ZW50L2NvbnRlbnRfbWFuYWdlcl9pbmRleGVzLmdv) | `72.72% <0.00%> (-1.93%)` | :arrow_down: |
| [repo/content/committed\_read\_manager.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9jb250ZW50L2NvbW1pdHRlZF9yZWFkX21hbmFnZXIuZ28=) | `69.58% <0.00%> (-1.85%)` | :arrow_down: |
| [internal/cache/persistent\_lru\_cache.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvY2FjaGUvcGVyc2lzdGVudF9scnVfY2FjaGUuZ28=) | `96.06% <0.00%> (-1.74%)` | :arrow_down: |
| [fs/localfs/local\_fs\_pool.go](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-ZnMvbG9jYWxmcy9sb2NhbF9mc19wb29sLmdv) | `81.39% <0.00%> (-0.89%)` | :arrow_down: |
| ... and [92 more](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/kopia/kopia/pull/2690?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](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/upload.go b/snapshot/snapshotfs/upload.go
index e08bcb8e..2b991209 100644
--- a/snapshot/snapshotfs/upload.go
+++ b/snapshot/snapshotfs/upload.go
@@ -463,6 +463,22 @@ func newDirEntry(md fs.Entry, fname string, oid object.ID) (*snapshot.DirEntry,
 	}, nil
 }
 
+// newCachedDirEntry makes DirEntry objects for entries that are also in
+// previous snapshots. It ensures file sizes are populated correctly for
+// StreamingFiles.
+func newCachedDirEntry(md, cached fs.Entry, fname string) (*snapshot.DirEntry, error) {
+	hoid, ok := cached.(object.HasObjectID)
+	if !ok {
+		return nil, errors.New("cached entry does not implement HasObjectID")
+	}
+
+	if _, ok := md.(fs.StreamingFile); ok {
+		return newDirEntry(cached, fname, hoid.ObjectID())
+	}
+
+	return newDirEntry(md, fname, hoid.ObjectID())
+}
+
 // uploadFileWithCheckpointing uploads the specified File to the repository.
 func (u *Uploader) uploadFileWithCheckpointing(ctx context.Context, relativePath string, file fs.File, pol *policy.Policy, sourceInfo snapshot.SourceInfo) (*snapshot.DirEntry, error) {
 	var cp checkpointRegistry
@@ -809,11 +825,11 @@ func (u *Uploader) processSingle(
 		// See if we had this name during either of previous passes.
 		if cachedEntry := u.maybeIgnoreCachedEntry(ctx, findCachedEntry(ctx, entryRelativePath, entry, prevDirs, policyTree)); cachedEntry != nil {
 			atomic.AddInt32(&u.stats.CachedFiles, 1)
-			atomic.AddInt64(&u.stats.TotalFileSize, entry.Size())
-			u.Progress.CachedFile(entryRelativePath, entry.Size())
+			atomic.AddInt64(&u.stats.TotalFileSize, cachedEntry.Size())
+			u.Progress.CachedFile(entryRelativePath, cachedEntry.Size())
+
+			cachedDirEntry, err := newCachedDirEntry(entry, cachedEntry, entry.Name())
 
-			// compute entryResult now, cachedEntry is short-lived
-			cachedDirEntry, err := newDirEntry(entry, entry.Name(), cachedEntry.(object.HasObjectID).ObjectID())
 			u.Progress.FinishedFile(entryRelativePath, err)
 
 			if err != nil {


Test patch:
diff --git a/snapshot/snapshotfs/upload_test.go b/snapshot/snapshotfs/upload_test.go
index 9ef7d562..16fb722e 100644
--- a/snapshot/snapshotfs/upload_test.go
+++ b/snapshot/snapshotfs/upload_test.go
@@ -923,6 +923,7 @@ func TestUpload_VirtualDirectoryWithStreamingFileWithModTime(t *testing.T) {
 			require.Equal(t, int32(1), atomic.LoadInt32(&man1.Stats.NonCachedFiles))
 			require.Equal(t, int32(1), atomic.LoadInt32(&man1.Stats.TotalDirectoryCount))
 			require.Equal(t, int32(1), atomic.LoadInt32(&man1.Stats.TotalFileCount))
+			require.Equal(t, int64(len(content)), atomic.LoadInt64(&man1.Stats.TotalFileSize))
 
 			// wait a little bit to ensure clock moves forward which is not always the case on Windows.
 			time.Sleep(100 * time.Millisecond)
@@ -941,6 +942,7 @@ func TestUpload_VirtualDirectoryWithStreamingFileWithModTime(t *testing.T) {
 			assert.Equal(t, tc.uploadedFiles, atomic.LoadInt32(&man2.Stats.NonCachedFiles))
 			// Cached files don't count towards the total file count.
 			assert.Equal(t, tc.uploadedFiles, atomic.LoadInt32(&man2.Stats.TotalFileCount))
+			require.Equal(t, int64(len(content)), atomic.LoadInt64(&man2.Stats.TotalFileSize))
 		})
 	}
 }