Alter source tests to cover entire prefetch algorithm and make it pass

dnscrypt-proxy/sources.go

@@ -26,7 +26,8 @@ const (
 )
 
 const (
-	MinSourcesUpdateDelay = time.Duration(24) * time.Hour
+	DefaultPrefetchDelay    time.Duration = 24 * time.Hour
+	MinimumPrefetchInterval time.Duration = 10 * time.Minute
 )
 
 type Source struct {
@@ -50,8 +51,8 @@ var timeNow = time.Now
 
 func fetchFromCache(cacheFile string, refreshDelay time.Duration) (bin []byte, delayTillNextUpdate time.Duration, err error) {
 	delayTillNextUpdate = time.Duration(0)
-	if refreshDelay < MinSourcesUpdateDelay {
+	if refreshDelay < DefaultPrefetchDelay {
-		refreshDelay = MinSourcesUpdateDelay
+		refreshDelay = DefaultPrefetchDelay
 	}
 	var fi os.FileInfo
 	if fi, err = os.Stat(cacheFile); err != nil {
@@ -62,7 +63,7 @@ func fetchFromCache(cacheFile string, refreshDelay time.Duration) (bin []byte, d
 	}
 	if elapsed := timeNow().Sub(fi.ModTime()); elapsed < refreshDelay {
 		dlog.Debugf("Cache file [%s] is still fresh", cacheFile)
-		delayTillNextUpdate = MinSourcesUpdateDelay - elapsed
+		delayTillNextUpdate = DefaultPrefetchDelay - elapsed
 	} else {
 		dlog.Debugf("Cache file [%s] needs to be refreshed", cacheFile)
 	}
@@ -105,7 +106,7 @@ func fetchWithCache(xTransport *XTransport, urlStr string, cacheFile string, ref
 			dlog.Warnf("%s: %s", absPath, err)
 		}
 	}
-	delayTillNextUpdate = MinSourcesUpdateDelay
+	delayTillNextUpdate = DefaultPrefetchDelay
 	return
 }
 
@@ -178,6 +179,7 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac
 
 func PrefetchSources(xTransport *XTransport, sources []*Source) time.Duration {
 	now := timeNow()
+	interval := MinimumPrefetchInterval
 	for _, source := range sources {
 		for _, urlToPrefetch := range source.prefetch {
 			if now.After(urlToPrefetch.when) {
@@ -186,11 +188,15 @@ func PrefetchSources(xTransport *XTransport, sources []*Source) time.Duration {
 					dlog.Debugf("Prefetching [%s] failed: %s", urlToPrefetch.url, err)
 				} else {
 					dlog.Debugf("Prefetching [%s] succeeded. Next refresh scheduled for %v", urlToPrefetch.url, urlToPrefetch.when)
+					delay := urlToPrefetch.when.Sub(now)
+					if delay >= MinimumPrefetchInterval && (interval == MinimumPrefetchInterval || interval > delay) {
+						interval = delay
 					}
 				}
 			}
 		}
-	return 60 * time.Second
+	}
+	return interval
 }
 
 func (source *Source) Parse(prefix string) ([]RegisteredServer, error) {
@@ -268,7 +274,7 @@ PartsLoop:
 }
 
 func PrefetchSourceURL(xTransport *XTransport, urlToPrefetch *URLToPrefetch) error {
-	_, delayTillNextUpdate, err := fetchWithCache(xTransport, urlToPrefetch.url, urlToPrefetch.cacheFile, MinSourcesUpdateDelay)
+	_, delayTillNextUpdate, err := fetchWithCache(xTransport, urlToPrefetch.url, urlToPrefetch.cacheFile, DefaultPrefetchDelay)
 	urlToPrefetch.when = timeNow().Add(delayTillNextUpdate)
 	return err
 }

dnscrypt-proxy/sources_test.go

@@ -228,8 +228,8 @@ func setupSourceTest(t *testing.T) (func(), *SourceTestData) {
 	}
 	d.xTransport.rebuildTransport()
 	d.timeNow = time.Now().AddDate(0, 0, 0)
-	d.timeOld = d.timeNow.Add(MinSourcesUpdateDelay * -4)
+	d.timeOld = d.timeNow.Add(DefaultPrefetchDelay * -4)
-	d.timeUpd = d.timeNow.Add(MinSourcesUpdateDelay)
+	d.timeUpd = d.timeNow.Add(DefaultPrefetchDelay)
 	timeNow = func() time.Time { return d.timeNow } // originally defined in sources.go, replaced during testing to ensure consistent results
 	makeTempDir(t, d)
 	makeTestServer(t, d)
@@ -267,7 +267,7 @@ func prepSourceTestDownload(t *testing.T, d *SourceTestData, e *SourceTestExpect
 				switch state {
 				case TestStateCorrect:
 					e.cache = []SourceFixture{d.fixtures[state][source], d.fixtures[state][source+".minisig"]}
-					e.Source.in, e.success, e.refresh = e.cache[0].content, true, d.timeUpd
+					e.Source.in, e.success, e.download, e.refresh = e.cache[0].content, true, true, d.timeUpd
 					fallthrough
 				case TestStateMissingSig, TestStatePartial, TestStatePartialSig, TestStateReadSigErr:
 					d.reqExpect[path+".minisig"]++
@@ -321,7 +321,7 @@ func setupSourceTestCase(t *testing.T, d *SourceTestData, i int,
 func TestNewSource(t *testing.T) {
 	teardown, d := setupSourceTest(t)
 	defer teardown()
-	doTest := func(t *testing.T, e *SourceTestExpect, got *Source, err error) {
+	checkResult := func(t *testing.T, e *SourceTestExpect, got *Source, err error) {
 		c := check.T(t)
 		if len(e.err) > 0 {
 			c.Match(err, e.err, "Unexpected error")
@@ -338,14 +338,14 @@ func TestNewSource(t *testing.T) {
 		refresh      time.Duration
 		e            *SourceTestExpect
 	}{
-		{"old format", d.keyStr, "v1", MinSourcesUpdateDelay * 3, &SourceTestExpect{
+		{"old format", d.keyStr, "v1", DefaultPrefetchDelay * 3, &SourceTestExpect{
 			Source: &Source{}, err: "Unsupported source format"}},
-		{"invalid public key", "", "v2", MinSourcesUpdateDelay * 3, &SourceTestExpect{
+		{"invalid public key", "", "v2", DefaultPrefetchDelay * 3, &SourceTestExpect{
 			Source: &Source{}, err: "Invalid encoded public key"}},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := NewSource(d.xTransport, tt.e.Source.urls, tt.key, tt.e.cachePath, tt.v, tt.refresh)
-			doTest(t, tt.e, got, err)
+			checkResult(t, tt.e, got, err)
 		})
 	}
 	for cacheTestName, cacheTest := range d.cacheTests {
@@ -354,28 +354,28 @@ func TestNewSource(t *testing.T) {
 			for i := range d.sources {
 				id, e := setupSourceTestCase(t, d, i, &cacheTest, downloadTest)
 				t.Run("cache "+cacheTestName+", download "+downloadTestName+"/"+id, func(t *testing.T) {
-					got, err := NewSource(d.xTransport, e.Source.urls, d.keyStr, e.cachePath, "v2", MinSourcesUpdateDelay*3)
+					got, err := NewSource(d.xTransport, e.Source.urls, d.keyStr, e.cachePath, "v2", DefaultPrefetchDelay*3)
-					doTest(t, e, got, err)
+					checkResult(t, e, got, err)
 				})
 			}
 		}
 	}
 }
 
-func TestPrefetchSourceURL(t *testing.T) {
+func TestPrefetchSources(t *testing.T) {
 	teardown, d := setupSourceTest(t)
 	defer teardown()
-	doTest := func(t *testing.T, expects []*SourceTestExpect) {
+	checkResult := func(t *testing.T, expects []*SourceTestExpect, got time.Duration) {
 		c := check.T(t)
+		expectDelay := MinimumPrefetchInterval
 		for _, e := range expects {
-			for _, url := range e.Source.urls {
+			if e.refresh.After(d.timeNow) {
-				for _, suffix := range []string{"", ".minisig"} {
+				expectDelay = e.refresh.Sub(d.timeNow)
-					pf := &URLToPrefetch{url + suffix, e.cachePath + suffix, d.timeOld}
+			} else if e.download {
-					PrefetchSourceURL(d.xTransport, pf)
+				expectDelay = DefaultPrefetchDelay
-					c.InDelta(pf.when, e.refresh, time.Second, "Unexpected prefetch refresh time")
-				}
 			}
 		}
+		c.InDelta(got, expectDelay, time.Second, "Unexpected return")
 		checkTestServer(c, d)
 		for _, e := range expects {
 			checkSourceCache(c, e.cachePath, e.cache)
@@ -383,13 +383,22 @@ func TestPrefetchSourceURL(t *testing.T) {
 	}
 	for downloadTestName, downloadTest := range d.downloadTests {
 		d.n++
+		sources := []*Source{}
 		expects := []*SourceTestExpect{}
 		for i := range d.sources {
 			_, e := setupSourceTestCase(t, d, i, nil, downloadTest)
+			sources = append(sources, e.Source)
 			expects = append(expects, e)
+			for _, pf := range e.Source.prefetch {
+				pf.when = d.timeOld
+			}
+			if e.download {
+				e.refresh = d.timeUpd
+			}
 		}
 		t.Run("download "+downloadTestName, func(t *testing.T) {
-			doTest(t, expects)
+			got := PrefetchSources(d.xTransport, sources)
+			checkResult(t, expects, got)
 		})
 	}
 }