diff --git a/download/download.go b/download/download.go index 67aa73021d..06427ead59 100644 --- a/download/download.go +++ b/download/download.go @@ -111,11 +111,16 @@ func (d *Downloader) WithBundlePersistence(persist bool) *Downloader { return d } -// ClearCache resets the etag value on the downloader +// ClearCache is deprecated. Use SetCache instead. func (d *Downloader) ClearCache() { d.etag = "" } +// SetCache sets the given etag value on the downloader. +func (d *Downloader) SetCache(etag string) { + d.etag = etag +} + // Start tells the Downloader to begin downloading bundles. func (d *Downloader) Start(ctx context.Context) { go d.doStart(ctx) diff --git a/download/download_test.go b/download/download_test.go index 042e40ec3f..9e62d2e10a 100644 --- a/download/download_test.go +++ b/download/download_test.go @@ -253,23 +253,13 @@ func TestEtagCachingLifecycle(t *testing.T) { t.Fatalf("Expected empty downloader ETag but got %v", fixture.d.etag) } - // simulate successful bundle activation and check updated etag on the downloader + // simulate downloader error on first bundle download + fixture.server.expCode = 500 fixture.server.expEtag = "some etag value" _, err := fixture.d.oneShot(ctx) - if err != nil { - t.Fatal("Unexpected:", err) - } else if len(fixture.updates) != 1 { - t.Fatal("expected update") - } else if fixture.d.etag != fixture.server.expEtag { - t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag) - } - - // simulate downloader error and check etag is cleared - fixture.server.expCode = 500 - _, err = fixture.d.oneShot(ctx) if err == nil { t.Fatal("Expected error but got nil") - } else if len(fixture.updates) != 2 { + } else if len(fixture.updates) != 1 { t.Fatal("expected update") } else if fixture.d.etag != "" { t.Fatalf("Expected empty downloader ETag but got %v", fixture.d.etag) @@ -277,7 +267,17 @@ func TestEtagCachingLifecycle(t *testing.T) { // simulate successful bundle activation and check updated etag on the downloader fixture.server.expCode = 0 - fixture.server.expEtag = "some new etag value" + _, err = fixture.d.oneShot(ctx) + if err != nil { + t.Fatal("Unexpected:", err) + } else if len(fixture.updates) != 2 { + t.Fatal("expected update") + } else if fixture.d.etag != fixture.server.expEtag { + t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag) + } + + // simulate another successful bundle activation and check updated etag on the downloader + fixture.server.expEtag = "some etag value - 2" _, err = fixture.d.oneShot(ctx) if err != nil { t.Fatal("Unexpected:", err) @@ -287,16 +287,51 @@ func TestEtagCachingLifecycle(t *testing.T) { t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag) } - // simulate bundle activation error and check etag is cleared + // simulate bundle activation error and check etag is set from the last successful activation fixture.mockBundleActivationError = true - fixture.server.expEtag = "some newer etag value" + fixture.server.expEtag = "some newer etag value - 3" _, err = fixture.d.oneShot(ctx) if err != nil { t.Fatal("Unexpected:", err) } else if len(fixture.updates) != 4 { t.Fatal("expected update") - } else if fixture.d.etag != "" { - t.Fatalf("Expected empty downloader ETag but got %v", fixture.d.etag) + } else if fixture.d.etag != "some etag value - 2" { + t.Fatalf("Expected downloader ETag %v but got %v", "some etag value - 2", fixture.d.etag) + } + + // simulate successful bundle activation and check updated etag on the downloader + fixture.server.expCode = 0 + fixture.mockBundleActivationError = false + _, err = fixture.d.oneShot(ctx) + if err != nil { + t.Fatal("Unexpected:", err) + } else if len(fixture.updates) != 5 { + t.Fatal("expected update") + } else if fixture.d.etag != fixture.server.expEtag { + t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag) + } + + // simulate downloader error and check etag is set from the last successful activation + fixture.server.expCode = 500 + _, err = fixture.d.oneShot(ctx) + if err == nil { + t.Fatal("Expected error but got nil") + } else if len(fixture.updates) != 6 { + t.Fatal("expected update") + } else if fixture.d.etag != fixture.server.expEtag { + t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag) + } + + // simulate bundle activation error and check etag is set from the last successful activation + fixture.mockBundleActivationError = true + fixture.server.expCode = 0 + _, err = fixture.d.oneShot(ctx) + if err != nil { + t.Fatal("Unexpected:", err) + } else if len(fixture.updates) != 7 { + t.Fatal("expected update") + } else if fixture.d.etag != fixture.server.expEtag { + t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag) } } @@ -393,6 +428,7 @@ type testFixture struct { server *testServer updates []Update mockBundleActivationError bool + etags map[string]string } func newTestFixture(t *testing.T) testFixture { @@ -443,6 +479,7 @@ func newTestFixture(t *testing.T) testFixture { client: tc, server: &ts, updates: []Update{}, + etags: make(map[string]string), } } @@ -451,16 +488,20 @@ func (t *testFixture) oneShot(ctx context.Context, u Update) { t.updates = append(t.updates, u) if u.Error != nil { - t.d.ClearCache() + etag := t.etags["test/bundle1"] + t.d.SetCache(etag) return } if u.Bundle != nil { if t.mockBundleActivationError { - t.d.ClearCache() + etag := t.etags["test/bundle1"] + t.d.SetCache(etag) return } } + + t.etags["test/bundle1"] = u.ETag } type testServer struct { diff --git a/plugins/bundle/plugin.go b/plugins/bundle/plugin.go index 2ac7622357..cd08bb5040 100644 --- a/plugins/bundle/plugin.go +++ b/plugins/bundle/plugin.go @@ -369,7 +369,8 @@ func (p *Plugin) process(ctx context.Context, name string, u download.Update) { p.log(name).Error("Bundle load failed: %v", u.Error) p.status[name].SetError(u.Error) if !p.stopped { - p.downloaders[name].ClearCache() + etag := p.etags[name] + p.downloaders[name].SetCache(etag) } return } @@ -386,7 +387,8 @@ func (p *Plugin) process(ctx context.Context, name string, u download.Update) { p.log(name).Error("Bundle activation failed: %v", err) p.status[name].SetError(err) if !p.stopped { - p.downloaders[name].ClearCache() + etag := p.etags[name] + p.downloaders[name].SetCache(etag) } return } @@ -399,7 +401,8 @@ func (p *Plugin) process(ctx context.Context, name string, u download.Update) { p.log(name).Error("Persisting bundle to disk failed: %v", err) p.status[name].SetError(err) if !p.stopped { - p.downloaders[name].ClearCache() + etag := p.etags[name] + p.downloaders[name].SetCache(etag) } return } @@ -617,6 +620,7 @@ func (p *Plugin) getBundlePersistPath() (string, error) { type bundleLoader interface { Start(context.Context) Stop(context.Context) + SetCache(string) ClearCache() } @@ -656,3 +660,7 @@ func (*fileLoader) Stop(context.Context) { func (*fileLoader) ClearCache() { } + +func (*fileLoader) SetCache(string) { + +}