Merge pull request #1445 from silkeh/fix-db-references

Remove corrupt package references in `db recover`

AUTHORS

@@ -68,3 +68,4 @@ List of contributors, in chronological order:
 * Blake Kostner (https://github.com/btkostner)
 * Leigh London (https://github.com/leighlondon)
 * Gordian Schoenherr (https://github.com/schoenherrg)
+* Silke Hofstra (https://github.com/silkeh)

cmd/db_recover.go

@@ -1,6 +1,9 @@
 package cmd
 
 import (
+	"fmt"
+
+	"github.com/aptly-dev/aptly/deb"
 	"github.com/smira/commander"
 
 	"github.com/aptly-dev/aptly/database/goleveldb"
@@ -16,7 +19,12 @@ func aptlyDBRecover(cmd *commander.Command, args []string) error {
 	}
 
 	context.Progress().Printf("Recovering database...\n")
-	err = goleveldb.RecoverDB(context.DBPath())
+	if err = goleveldb.RecoverDB(context.DBPath()); err != nil {
+		return err
+	}
+
+	context.Progress().Printf("Checking database integrity...\n")
+	err = checkIntegrity()
 
 	return err
 }
@@ -38,3 +46,36 @@ Example:
 
 	return cmd
 }
+
+func checkIntegrity() error {
+	return context.NewCollectionFactory().LocalRepoCollection().ForEach(checkRepo)
+}
+
+func checkRepo(repo *deb.LocalRepo) error {
+	collectionFactory := context.NewCollectionFactory()
+	repos := collectionFactory.LocalRepoCollection()
+
+	err := repos.LoadComplete(repo)
+	if err != nil {
+		return fmt.Errorf("load complete repo %q: %s", repo.Name, err)
+	}
+
+	dangling, err := deb.FindDanglingReferences(repo.RefList(), collectionFactory.PackageCollection())
+	if err != nil {
+		return fmt.Errorf("find dangling references: %w", err)
+	}
+
+	if len(dangling.Refs) > 0 {
+		for _, ref := range dangling.Refs {
+			context.Progress().Printf("Removing dangling database reference %q\n", ref)
+		}
+
+		repo.UpdateRefList(repo.RefList().Subtract(dangling))
+
+		if err = repos.Update(repo); err != nil {
+			return fmt.Errorf("update repo: %w", err)
+		}
+	}
+
+	return nil
+}

deb/find_dangling.go

@@ -0,0 +1,45 @@
+package deb
+
+import (
+	"errors"
+	"fmt"
+
+	"github.com/aptly-dev/aptly/database"
+)
+
+// FindDanglingReferences finds references that exist in the given PackageRefList, but not in the given PackageCollection.
+// It returns all such references, so they can be removed from the database.
+func FindDanglingReferences(reflist *PackageRefList, packages *PackageCollection) (dangling *PackageRefList, err error) {
+	dangling = &PackageRefList{}
+
+	err = reflist.ForEach(func(key []byte) error {
+		ok, err := isDangling(packages, key)
+		if err != nil {
+			return err
+		}
+
+		if ok {
+			dangling.Refs = append(dangling.Refs, key)
+		}
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return dangling, nil
+}
+
+func isDangling(packages *PackageCollection, key []byte) (bool, error) {
+	_, err := packages.ByKey(key)
+	if errors.Is(err, database.ErrNotFound) {
+		return true, nil
+	}
+
+	if err != nil {
+		return false, fmt.Errorf("get reference %q: %w", key, err)
+	}
+
+	return false, nil
+}

deb/find_dangling_test.go

@@ -0,0 +1,46 @@
+package deb_test
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/aptly-dev/aptly/database/goleveldb"
+	"github.com/aptly-dev/aptly/deb"
+)
+
+func TestFindDanglingReferences(t *testing.T) {
+	reflist := deb.NewPackageRefList()
+	reflist.Refs = [][]byte{[]byte("P existing 1.2.3"), []byte("P dangling 1.2.3")}
+
+	db, _ := goleveldb.NewOpenDB(t.TempDir())
+	packages := deb.NewPackageCollection(db)
+
+	if err := packages.Update(&deb.Package{Name: "existing", Version: "1.2.3"}); err != nil {
+		t.Fatal(err)
+	}
+
+	dangling, err := deb.FindDanglingReferences(reflist, packages)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	exp := &deb.PackageRefList{
+		Refs: [][]byte{[]byte("P dangling 1.2.3")},
+	}
+
+	compareRefs(t, exp, dangling)
+}
+
+func compareRefs(t *testing.T, exp, got *deb.PackageRefList) {
+	t.Helper()
+
+	if len(exp.Refs) != len(got.Refs) {
+		t.Fatalf("refs length mismatch: exp %d, got %d", len(exp.Refs), len(got.Refs))
+	}
+
+	for i := range exp.Refs {
+		if !bytes.Equal(exp.Refs[i], got.Refs[i]) {
+			t.Fatalf("refs do not match: exp %q, got %q", exp.Refs[i], got.Refs[i])
+		}
+	}
+}

system/files/corruptdb.go

@@ -0,0 +1,42 @@
+// This utility corrupts a database by deleting matching package entries.
+// Do not use it outside system tests.
+package main
+
+import (
+	"flag"
+	"log"
+
+	"github.com/aptly-dev/aptly/database/goleveldb"
+)
+
+func main() {
+	var dbPath, prefix string
+
+	flag.StringVar(&dbPath, "db", "", "Path to DB to corrupt")
+	flag.StringVar(&prefix, "prefix", "P", "Path to DB to corrupt")
+	flag.Parse()
+
+	db, err := goleveldb.NewOpenDB(dbPath)
+	if err != nil {
+		log.Fatalf("Error opening DB %q: %s", dbPath, err)
+	}
+	defer db.Close()
+
+	keys := db.KeysByPrefix([]byte(prefix))
+	if len(keys) == 0 {
+		keys2 := db.KeysByPrefix([]byte{})
+		for _, key := range keys2 {
+			log.Printf("Have: %q", key)
+		}
+
+		log.Fatal("No keys to delete")
+	}
+
+	for _, key := range keys {
+		log.Printf("Deleting %q", key)
+
+		if err = db.Delete(key); err != nil {
+			log.Fatalf("Error deleting key: %s", err)
+		}
+	}
+}

system/t08_db/RecoverDB1Test_gold

@@ -1 +1,2 @@
 Recovering database...
+Checking database integrity...

system/t08_db/RecoverDB2Test_gold

@@ -1 +1,2 @@
 Recovering database...
+Checking database integrity...

system/t08_db/RecoverDB3Test_cleanup

@@ -0,0 +1,8 @@
+Loading mirrors, local repos, snapshots and published repos...
+Loading list of all packages...
+Deleting unreferenced packages (0)...
+Building list of files referenced by packages...
+Building list of files in package pool...
+Deleting unreferenced files (1)...
+Disk space freed: 12.18 KiB...
+Compacting database...

system/t08_db/RecoverDB3Test_gold

@@ -0,0 +1,3 @@
+Recovering database...
+Checking database integrity...
+Removing dangling database reference "Pamd64 hardlink 0.2.1 daf8fcecbf8210ad"

system/t08_db/recover.py

@@ -1,3 +1,5 @@
+import os
+
 from lib import BaseTest
 
 
@@ -23,3 +25,27 @@ class RecoverDB2Test(BaseTest):
     def check(self):
         self.check_output()
         self.check_cmd_output("aptly mirror list", "mirror_list")
+
+
+class RecoverDB3Test(BaseTest):
+    """
+    recover db: dangling reference
+    """
+    fixtureDB = True
+    fixtureCmds = [
+        "aptly repo create db3test",
+        "aptly repo add db3test changes/hardlink_0.2.1_amd64.deb",
+    ]
+
+    runCmd = "aptly db recover"
+
+    def prepare(self):
+        super(RecoverDB3Test, self).prepare()
+
+        self.run_cmd(["go", "run", "files/corruptdb.go",
+                      "-db", os.path.join(os.environ["HOME"], self.aptlyDir, "db"),
+                      "-prefix", "Pamd64 hardlink 0.2.1"])
+
+    def check(self):
+        self.check_output()
+        self.check_cmd_output("aptly db cleanup", "cleanup")