dashboard: don't deprecate assets of latest builds

It's bad to deprecate them if there are no related crashes, because they
may appear later.

Check if there are newer builds and only deprecate assets if there are.
Also, use a grace period not just for HTML reports deletion.

Add a test to check the new behavior.

2 files changed, 86 insertions, 8 deletions

diff --git a/dashboard/app/asset_storage.go b/dashboard/app/asset_storage.go
index 379b845cc..5849977ca 100644
--- a/dashboard/app/asset_storage.go
+++ b/dashboard/app/asset_storage.go
@@ -154,8 +154,9 @@ func deprecateCrashAssets(c context.Context) error {
 
 func deprecateNamespaceAssets(c context.Context, ns string) error {
 	ad := buildAssetDeprecator{
-		ns: ns,
-		c:  c,
+		ns:         ns,
+		c:          c,
+		lastBuilds: map[string]*Build{},
 	}
 	const buildBatchSize = 16
 	err := ad.batchProcessBuilds(buildBatchSize)
@@ -170,10 +171,24 @@ type buildAssetDeprecator struct {
 	c            context.Context
 	bugsQueried  bool
 	relevantBugs map[string]bool
+	lastBuilds   map[string]*Build
 }
 
 const keepAssetsForClosedBugs = time.Hour * 24 * 30
 
+func (ad *buildAssetDeprecator) lastBuild(manager string) (*Build, error) {
+	build, ok := ad.lastBuilds[manager]
+	if ok {
+		return build, nil
+	}
+	lastBuild, err := lastManagerBuild(ad.c, ad.ns, manager)
+	if err != nil {
+		return nil, err
+	}
+	ad.lastBuilds[manager] = lastBuild
+	return lastBuild, err
+}
+
 func (ad *buildAssetDeprecator) queryBugs() error {
 	if ad.bugsQueried {
 		return nil
@@ -219,11 +234,6 @@ func (ad *buildAssetDeprecator) queryBugs() error {
 }
 
 func (ad *buildAssetDeprecator) buildArchivePolicy(build *Build, asset *Asset) (bool, error) {
-	// If the asset is reasonably new, we always keep it.
-	const alwaysKeepPeriod = time.Hour * 24 * 14
-	if asset.CreateDate.After(timeNow(ad.c).Add(-alwaysKeepPeriod)) {
-		return true, nil
-	}
 	// Query builds to see whether there's a newer same-type asset on the same week.
 	var builds []*Build
 	_, err := db.NewQuery("Build").
@@ -280,10 +290,20 @@ func (ad *buildAssetDeprecator) buildBugStatusPolicy(build *Build) (bool, error)
 			return true, nil
 		}
 	}
-	return false, nil
+	// If there are no crashes, but it's the latest build, they may still appear.
+	lastBuild, err := ad.lastBuild(build.Manager)
+	if err != nil {
+		return false, nil
+	}
+	return build.ID == lastBuild.ID, nil
 }
 
 func (ad *buildAssetDeprecator) needThisBuildAsset(build *Build, buildAsset *Asset) (bool, error) {
+	// If the asset is reasonably new, we always keep it.
+	const alwaysKeepPeriod = time.Hour * 24 * 14
+	if buildAsset.CreateDate.After(timeNow(ad.c).Add(-alwaysKeepPeriod)) {
+		return true, nil
+	}
 	if buildAsset.Type == dashapi.HTMLCoverageReport {
 		// We want to keep coverage reports forever, not just
 		// while there are any open bugs. But we don't want to
diff --git a/dashboard/app/asset_storage_test.go b/dashboard/app/asset_storage_test.go
index eb75d1c5d..ad972cf8b 100644
--- a/dashboard/app/asset_storage_test.go
+++ b/dashboard/app/asset_storage_test.go
@@ -18,6 +18,7 @@ func TestBuildAssetLifetime(t *testing.T) {
 	defer c.Close()
 
 	build := testBuild(1)
+	build.Manager = "test_manager"
 	// Embed one of the assets right away.
 	build.Assets = []dashapi.NewAsset{
 		{
@@ -27,6 +28,12 @@ func TestBuildAssetLifetime(t *testing.T) {
 	}
 	c.client2.UploadBuild(build)
 
+	// Add one more build, so that the assets of the previous one could be deprecated.
+	c.advanceTime(time.Minute)
+	build2 := testBuild(2)
+	build2.Manager = "test_manager"
+	c.client2.UploadBuild(build2)
+
 	// "Upload" several more assets.
 	c.expectOK(c.client2.AddBuildAssets(&dashapi.AddBuildAssetsReq{
 		BuildID: build.ID,
@@ -258,6 +265,57 @@ func TestCoverReportDeprecation(t *testing.T) {
 	ensureNeeded([]string{weekOneSecond, weekTwoSecond, weekThreeFirst, weekFourFirst})
 }
 
+func TestFreshBuildAssets(t *testing.T) {
+	c := NewCtx(t)
+	defer c.Close()
+
+	ensureNeeded := func(needed []string) {
+		_, err := c.GET("/deprecate_assets")
+		c.expectOK(err)
+		neededResp, err := c.client.NeededAssetsList()
+		c.expectOK(err)
+		sort.Strings(neededResp.DownloadURLs)
+		sort.Strings(needed)
+		c.expectEQ(neededResp.DownloadURLs, needed)
+	}
+
+	build := testBuild(1)
+	build.Manager = "manager"
+	build.Assets = []dashapi.NewAsset{
+		{
+			Type:        dashapi.KernelObject,
+			DownloadURL: "http://google.com/vmlinux",
+		},
+	}
+	c.client.UploadBuild(build)
+
+	// No crashes yet, but it's the latest build, so the assets must be preserved.
+	ensureNeeded([]string{"http://google.com/vmlinux"})
+
+	// Upload one more build for the same manager.
+	c.advanceTime(time.Minute)
+	build2 := testBuild(2)
+	build2.Manager = "manager"
+	build2.Assets = []dashapi.NewAsset{
+		{
+			Type:        dashapi.KernelObject,
+			DownloadURL: "http://google.com/vmlinux2",
+		},
+	}
+	c.client.UploadBuild(build2)
+
+	// The assets of the previous build are reasonably new, so they must be kept.
+	ensureNeeded([]string{"http://google.com/vmlinux", "http://google.com/vmlinux2"})
+
+	// The assets of the first build must be deprecated now.
+	c.advanceTime(time.Hour * 24 * 14)
+	ensureNeeded([]string{"http://google.com/vmlinux2"})
+
+	// But even if a lot of time passes, but there are no new builds, the assets must stay.
+	c.advanceTime(time.Hour * 24 * 365)
+	ensureNeeded([]string{"http://google.com/vmlinux2"})
+}
+
 func TestCrashAssetLifetime(t *testing.T) {
 	c := NewCtx(t)
 	defer c.Close()