plan 9: keyword cache keys on extension, max-read and Unicode tables, trims removed keywords

3 files changed, 91 insertions, 15 deletions

diff --git a/internal/kwcache/fuzz_test.go b/internal/kwcache/fuzz_test.go
index 0b0eced..8ac6c95 100644
--- a/internal/kwcache/fuzz_test.go
+++ b/internal/kwcache/fuzz_test.go
@@ -15,7 +15,7 @@ func FuzzLoad(f *testing.F) {
 	seed := New("fp")
 	seed.Store(a, map[string]bool{"k:acme": true, "k:x": false})
 	path := filepath.Join(f.TempDir(), "seed.cache")
-	if err := seed.Save(path, []ID{a}); err != nil {
+	if err := seed.Save(path, []ID{a}, allKeys); err != nil {
 		f.Fatal(err)
 	}
 	good, err := os.ReadFile(path)
@@ -43,7 +43,7 @@ func FuzzLoad(f *testing.F) {
 		if err != nil && ok {
 			t.Fatalf("a cache that failed to load (%v) answered", err)
 		}
-		if err := c.Save(filepath.Join(t.TempDir(), "out.cache"), []ID{a, b}); err != nil {
+		if err := c.Save(filepath.Join(t.TempDir(), "out.cache"), []ID{a, b}, allKeys); err != nil {
 			t.Fatal(err)
 		}
 	})
diff --git a/internal/kwcache/kwcache.go b/internal/kwcache/kwcache.go
index b61cfb1..c9af7b8 100644
--- a/internal/kwcache/kwcache.go
+++ b/internal/kwcache/kwcache.go
@@ -19,15 +19,18 @@ import (
 )
 
 // version is the on-disk format; a file of any other version loads empty.
-const version = 1
+// 2: the extension is part of a file's ID.
+const version = 2
 
 // ID is how a file is recognised: the same inode with the same size and
-// modification time is taken to hold the same content. A move or rename
-// within one filesystem keeps it.
+// modification time is taken to hold the same content, and the same
+// extension is read by the same extractor. A move within one filesystem
+// keeps it; a rename that changes the extension does not (review M6).
 type ID struct {
 	Dev, Ino uint64
 	Size     int64
-	MTime    int64 // Unix nanoseconds
+	MTime    int64  // Unix nanoseconds
+	Ext      string // lower case, with its dot; "" for none
 }
 
 // entry is what is known about one file: every keyword it was checked
@@ -58,6 +61,7 @@ type diskFile struct {
 	Ino   uint64 `json:"ino"`
 	Size  int64  `json:"size"`
 	MTime int64  `json:"mtime"`
+	Ext   string `json:"ext"`
 	Set   int    `json:"keywords"` // index into diskCache.Keywords
 	Hits  []int  `json:"hits"`     // indices into that keyword list
 }
@@ -106,7 +110,7 @@ func Load(path, fingerprint string) (*Cache, error) {
 			}
 			hits[set[h]] = true
 		}
-		c.old[ID{Dev: f.Dev, Ino: f.Ino, Size: f.Size, MTime: f.MTime}] = entry{checked: set, hits: hits}
+		c.old[ID{Dev: f.Dev, Ino: f.Ino, Size: f.Size, MTime: f.MTime, Ext: f.Ext}] = entry{checked: set, hits: hits}
 	}
 	c.existed = true
 	return c, nil
@@ -154,14 +158,22 @@ func (c *Cache) Store(id ID, answers map[string]bool) {
 
 // Save writes the entries of the files in present, from this run or loaded,
 // to path, and drops every other: the cache only ever describes files
-// still in the directory. The directory is created 0700 and the file
-// written 0600 under a temporary name, then renamed into place. A cache
-// with nothing to write and no file on disk writes nothing.
-func (c *Cache) Save(path string, present []ID) error {
+// still in the directory. Each entry keeps only the keywords in keywords -
+// the directory's current ones - so a keyword removed from the
+// configuration leaves the cache too, and an entry left with none is
+// dropped. The directory is made private (0700, tightened if it already
+// existed) and the file written 0600 under a temporary name, then renamed
+// into place. A cache with nothing to write and no file on disk writes
+// nothing.
+func (c *Cache) Save(path string, present []ID, keywords []string) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
 	d := diskCache{Version: version, Fingerprint: c.fingerprint, Keywords: [][]string{}, Files: []diskFile{}}
+	current := make(map[string]bool, len(keywords))
+	for _, k := range keywords {
+		current[k] = true
+	}
 	sets := map[string]int{}
 	seen := map[ID]bool{}
 	for _, id := range present {
@@ -176,6 +188,10 @@ func (c *Cache) Save(path string, present []ID) error {
 		if !ok {
 			continue
 		}
+		e = trimmed(e, current)
+		if len(e.checked) == 0 {
+			continue
+		}
 		key := strings.Join(e.checked, "\x00")
 		set, ok := sets[key]
 		if !ok {
@@ -183,7 +199,7 @@ func (c *Cache) Save(path string, present []ID) error {
 			sets[key] = set
 			d.Keywords = append(d.Keywords, e.checked)
 		}
-		f := diskFile{Dev: id.Dev, Ino: id.Ino, Size: id.Size, MTime: id.MTime, Set: set, Hits: []int{}}
+		f := diskFile{Dev: id.Dev, Ino: id.Ino, Size: id.Size, MTime: id.MTime, Ext: id.Ext, Set: set, Hits: []int{}}
 		for i, k := range e.checked {
 			if e.hits[k] {
 				f.Hits = append(f.Hits, i)
@@ -209,6 +225,9 @@ func (c *Cache) Save(path string, present []ID) error {
 	if err := os.MkdirAll(dir, 0o700); err != nil {
 		return err
 	}
+	if err := os.Chmod(dir, 0o700); err != nil {
+		return err
+	}
 	tmp, err := os.CreateTemp(dir, ".kwcache-*")
 	if err != nil {
 		return err
@@ -229,3 +248,17 @@ func (c *Cache) Save(path string, present []ID) error {
 	c.existed = true
 	return nil
 }
+
+// trimmed returns e keeping only the keywords in current.
+func trimmed(e entry, current map[string]bool) entry {
+	out := entry{hits: map[string]bool{}}
+	for _, k := range e.checked {
+		if current[k] {
+			out.checked = append(out.checked, k)
+			if e.hits[k] {
+				out.hits[k] = true
+			}
+		}
+	}
+	return out
+}
diff --git a/internal/kwcache/kwcache_test.go b/internal/kwcache/kwcache_test.go
index 8395b2d..077ecd3 100644
--- a/internal/kwcache/kwcache_test.go
+++ b/internal/kwcache/kwcache_test.go
@@ -6,9 +6,13 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 )
 
+// allKeys is every keyword the tests store, for Save calls that keep them all.
+var allKeys = []string{"k:acme", "k:faktura", "k:nip", "k:new", "k:x"}
+
 var (
 	a = ID{Dev: 1, Ino: 10, Size: 100, MTime: 1000}
 	b = ID{Dev: 1, Ino: 11, Size: 200, MTime: 2000}
@@ -17,7 +21,7 @@ var (
 func saved(t *testing.T, c *Cache, present ...ID) string {
 	t.Helper()
 	path := filepath.Join(t.TempDir(), "sub", "dl.cache")
-	if err := c.Save(path, present); err != nil {
+	if err := c.Save(path, present, allKeys); err != nil {
 		t.Fatal(err)
 	}
 	return path
@@ -94,7 +98,7 @@ func TestSaveKeepsOnlyPresentFiles(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := next.Save(path, []ID{b}); err != nil {
+	if err := next.Save(path, []ID{b}, allKeys); err != nil {
 		t.Fatal(err)
 	}
 	last, err := Load(path, "fp")
@@ -171,10 +175,49 @@ func TestLoadMissingAndCorrupt(t *testing.T) {
 // has no file yet, creates none.
 func TestSaveWithNothingWritesNothing(t *testing.T) {
 	path := filepath.Join(t.TempDir(), "sub", "dl.cache")
-	if err := New("fp").Save(path, []ID{a}); err != nil {
+	if err := New("fp").Save(path, []ID{a}, allKeys); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := os.Stat(filepath.Dir(path)); !os.IsNotExist(err) {
 		t.Errorf("Save created %s for an empty cache", filepath.Dir(path))
 	}
 }
+
+// TestSaveTrimsToCurrentKeywords: a keyword no longer in the configuration
+// is not kept in the cache file (review cache F6).
+func TestSaveTrimsToCurrentKeywords(t *testing.T) {
+	c := New("fp")
+	c.Store(a, map[string]bool{"k:keep": true, "k:gone client": true})
+	path := filepath.Join(t.TempDir(), "dl.cache")
+	if err := c.Save(path, []ID{a}, []string{"k:keep"}); err != nil {
+		t.Fatal(err)
+	}
+	b, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(b), "gone client") {
+		t.Errorf("a removed keyword is still stored:\n%s", b)
+	}
+	got, _ := Load(path, "fp")
+	if hits, ok := got.Lookup(a, []string{"k:keep"}); !ok || !hits[0] {
+		t.Errorf("the kept keyword's answer was lost: %v %v", hits, ok)
+	}
+}
+
+// TestSaveTightensAnExistingDirectory: the cache directory is private even
+// when it already existed with wider permissions (review cache F7).
+func TestSaveTightensAnExistingDirectory(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "krino")
+	if err := os.Mkdir(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	c := New("fp")
+	c.Store(a, map[string]bool{"k:acme": true})
+	if err := c.Save(filepath.Join(dir, "dl.cache"), []ID{a}, []string{"k:acme"}); err != nil {
+		t.Fatal(err)
+	}
+	if fi, err := os.Stat(dir); err != nil || fi.Mode().Perm() != 0o700 {
+		t.Errorf("directory mode %v, %v; want 0700", fi.Mode().Perm(), err)
+	}
+}