From 5bb73a7b4ce6f56930dc3dabc8b790736b3c5477 Mon Sep 17 00:00:00 2001
From: Daniel <dhaavi@users.noreply.github.com>
Date: Wed, 23 Sep 2020 17:10:33 +0200
Subject: [PATCH 1/2] Make shadow deletes conditional

Also, Move maintenance to separate interface.
---
 database/controller.go          | 50 +++++++++++++++++++++++----------
 database/controllers.go         |  4 +--
 database/database.go            | 14 ++++-----
 database/database_test.go       |  8 +++---
 database/registry.go            |  4 +--
 database/storage/bbolt/bbolt.go | 30 +++++++++++++-------
 database/storage/hashmap/map.go |  8 ++++--
 database/storage/injectbase.go  |  2 +-
 database/storage/interface.go   |  8 ++++--
 9 files changed, 83 insertions(+), 45 deletions(-)

diff --git a/database/controller.go b/database/controller.go
index 4f7ae3e..bb9926e 100644
--- a/database/controller.go
+++ b/database/controller.go
@@ -16,7 +16,8 @@ import (
 
 // A Controller takes care of all the extra database logic.
 type Controller struct {
-	storage storage.Interface
+	storage      storage.Interface
+	shadowDelete bool
 
 	hooks         []*RegisteredHook
 	subscriptions []*Subscription
@@ -33,11 +34,12 @@ type Controller struct {
 }
 
 // newController creates a new controller for a storage.
-func newController(storageInt storage.Interface) *Controller {
+func newController(storageInt storage.Interface, shadowDelete bool) *Controller {
 	return &Controller{
-		storage:     storageInt,
-		migrating:   abool.NewBool(false),
-		hibernating: abool.NewBool(false),
+		storage:      storageInt,
+		shadowDelete: shadowDelete,
+		migrating:    abool.NewBool(false),
+		hibernating:  abool.NewBool(false),
 	}
 }
 
@@ -122,12 +124,21 @@ func (c *Controller) Put(r record.Record) (err error) {
 		}
 	}
 
-	r, err = c.storage.Put(r)
-	if err != nil {
-		return err
-	}
-	if r == nil {
-		return errors.New("storage returned nil record after successful put operation")
+	if !c.shadowDelete && r.Meta().IsDeleted() {
+		// Immediate delete.
+		err = c.storage.Delete(r.DatabaseKey())
+		if err != nil {
+			return err
+		}
+	} else {
+		// Put or shadow delete.
+		r, err = c.storage.Put(r)
+		if err != nil {
+			return err
+		}
+		if r == nil {
+			return errors.New("storage returned nil record after successful put operation")
+		}
 	}
 
 	// process subscriptions
@@ -161,7 +172,7 @@ func (c *Controller) PutMany() (chan<- record.Record, <-chan error) {
 	}
 
 	if batcher, ok := c.storage.(storage.Batcher); ok {
-		return batcher.PutMany()
+		return batcher.PutMany(c.shadowDelete)
 	}
 
 	errs := make(chan error, 1)
@@ -236,7 +247,10 @@ func (c *Controller) Maintain(ctx context.Context) error {
 		return nil
 	}
 
-	return c.storage.Maintain(ctx)
+	if maintenance, ok := c.storage.(storage.Maintenance); ok {
+		return maintenance.Maintain(ctx)
+	}
+	return nil
 }
 
 // MaintainThorough runs the MaintainThorough method on the storage.
@@ -248,7 +262,10 @@ func (c *Controller) MaintainThorough(ctx context.Context) error {
 		return nil
 	}
 
-	return c.storage.MaintainThorough(ctx)
+	if maintenance, ok := c.storage.(storage.Maintenance); ok {
+		return maintenance.MaintainThorough(ctx)
+	}
+	return nil
 }
 
 // MaintainRecordStates runs the record state lifecycle maintenance on the storage.
@@ -260,7 +277,10 @@ func (c *Controller) MaintainRecordStates(ctx context.Context, purgeDeletedBefor
 		return nil
 	}
 
-	return c.storage.MaintainRecordStates(ctx, purgeDeletedBefore)
+	if maintenance, ok := c.storage.(storage.Maintenance); ok {
+		return maintenance.MaintainRecordStates(ctx, purgeDeletedBefore)
+	}
+	return nil
 }
 
 // Shutdown shuts down the storage.
diff --git a/database/controllers.go b/database/controllers.go
index ba8c7cd..9b2ac68 100644
--- a/database/controllers.go
+++ b/database/controllers.go
@@ -51,7 +51,7 @@ func getController(name string) (*Controller, error) {
 		return nil, fmt.Errorf(`could not start database %s (type %s): %s`, name, registeredDB.StorageType, err)
 	}
 
-	controller = newController(storageInt)
+	controller = newController(storageInt, registeredDB.ShadowDelete)
 	controllers[name] = controller
 	return controller, nil
 }
@@ -82,7 +82,7 @@ func InjectDatabase(name string, storageInt storage.Interface) (*Controller, err
 		return nil, fmt.Errorf(`database not of type "injected"`)
 	}
 
-	controller := newController(storageInt)
+	controller := newController(storageInt, false)
 	controllers[name] = controller
 	return controller, nil
 }
diff --git a/database/database.go b/database/database.go
index e8e4504..048f35f 100644
--- a/database/database.go
+++ b/database/database.go
@@ -7,13 +7,13 @@ import (
 
 // Database holds information about registered databases
 type Database struct {
-	Name        string
-	Description string
-	StorageType string
-	PrimaryAPI  string
-	Registered  time.Time
-	LastUpdated time.Time
-	LastLoaded  time.Time
+	Name         string
+	Description  string
+	StorageType  string
+	ShadowDelete bool // Whether deleted records should be kept until purged.
+	Registered   time.Time
+	LastUpdated  time.Time
+	LastLoaded   time.Time
 }
 
 // MigrateTo migrates the database to another storage type.
diff --git a/database/database_test.go b/database/database_test.go
index 5749cec..2f3b4a5 100644
--- a/database/database_test.go
+++ b/database/database_test.go
@@ -31,10 +31,10 @@ func testDatabase(t *testing.T, storageType string, testPutMany, testRecordMaint
 		dbName := fmt.Sprintf("testing-%s", storageType)
 		fmt.Println(dbName)
 		_, err := Register(&Database{
-			Name:        dbName,
-			Description: fmt.Sprintf("Unit Test Database for %s", storageType),
-			StorageType: storageType,
-			PrimaryAPI:  "",
+			Name:         dbName,
+			Description:  fmt.Sprintf("Unit Test Database for %s", storageType),
+			StorageType:  storageType,
+			ShadowDelete: true,
 		})
 		if err != nil {
 			t.Fatal(err)
diff --git a/database/registry.go b/database/registry.go
index 8b65f7e..81d1de4 100644
--- a/database/registry.go
+++ b/database/registry.go
@@ -49,8 +49,8 @@ func Register(new *Database) (*Database, error) {
 			registeredDB.Description = new.Description
 			save = true
 		}
-		if registeredDB.PrimaryAPI != new.PrimaryAPI {
-			registeredDB.PrimaryAPI = new.PrimaryAPI
+		if registeredDB.ShadowDelete != new.ShadowDelete {
+			registeredDB.ShadowDelete = new.ShadowDelete
 			save = true
 		}
 	} else {
diff --git a/database/storage/bbolt/bbolt.go b/database/storage/bbolt/bbolt.go
index c512334..654aacb 100644
--- a/database/storage/bbolt/bbolt.go
+++ b/database/storage/bbolt/bbolt.go
@@ -107,7 +107,7 @@ func (b *BBolt) Put(r record.Record) (record.Record, error) {
 }
 
 // PutMany stores many records in the database.
-func (b *BBolt) PutMany() (chan<- record.Record, <-chan error) {
+func (b *BBolt) PutMany(shadowDelete bool) (chan<- record.Record, <-chan error) {
 	batch := make(chan record.Record, 100)
 	errs := make(chan error, 1)
 
@@ -115,16 +115,26 @@ func (b *BBolt) PutMany() (chan<- record.Record, <-chan error) {
 		err := b.db.Batch(func(tx *bbolt.Tx) error {
 			bucket := tx.Bucket(bucketName)
 			for r := range batch {
-				// marshal
-				data, txErr := r.MarshalRecord(r)
-				if txErr != nil {
-					return txErr
-				}
+				if !shadowDelete && r.Meta().IsDeleted() {
+					// Immediate delete.
+					txErr := bucket.Delete([]byte(r.DatabaseKey()))
+					if txErr != nil {
+						return txErr
+					}
+				} else {
+					// Put or shadow delete.
 
-				// put
-				txErr = bucket.Put([]byte(r.DatabaseKey()), data)
-				if txErr != nil {
-					return txErr
+					// marshal
+					data, txErr := r.MarshalRecord(r)
+					if txErr != nil {
+						return txErr
+					}
+
+					// put
+					txErr = bucket.Put([]byte(r.DatabaseKey()), data)
+					if txErr != nil {
+						return txErr
+					}
 				}
 			}
 			return nil
diff --git a/database/storage/hashmap/map.go b/database/storage/hashmap/map.go
index fb1e073..25a3fb2 100644
--- a/database/storage/hashmap/map.go
+++ b/database/storage/hashmap/map.go
@@ -54,7 +54,7 @@ func (hm *HashMap) Put(r record.Record) (record.Record, error) {
 }
 
 // PutMany stores many records in the database.
-func (hm *HashMap) PutMany() (chan<- record.Record, <-chan error) {
+func (hm *HashMap) PutMany(shadowDelete bool) (chan<- record.Record, <-chan error) {
 	hm.dbLock.Lock()
 	defer hm.dbLock.Unlock()
 	// we could lock for every record, but we want to have the same behaviour
@@ -66,7 +66,11 @@ func (hm *HashMap) PutMany() (chan<- record.Record, <-chan error) {
 	// start handler
 	go func() {
 		for r := range batch {
-			hm.db[r.DatabaseKey()] = r
+			if !shadowDelete && r.Meta().IsDeleted() {
+				delete(hm.db, r.DatabaseKey())
+			} else {
+				hm.db[r.DatabaseKey()] = r
+			}
 		}
 		errs <- nil
 	}()
diff --git a/database/storage/injectbase.go b/database/storage/injectbase.go
index 8dda71d..0f6bf25 100644
--- a/database/storage/injectbase.go
+++ b/database/storage/injectbase.go
@@ -28,7 +28,7 @@ func (i *InjectBase) Put(m record.Record) (record.Record, error) {
 }
 
 // PutMany stores many records in the database.
-func (i *InjectBase) PutMany() (batch chan record.Record, err chan error) {
+func (i *InjectBase) PutMany(shadowDelete bool) (batch chan record.Record, err chan error) {
 	batch = make(chan record.Record)
 	err = make(chan error, 1)
 	err <- errNotImplemented
diff --git a/database/storage/interface.go b/database/storage/interface.go
index 4ec5232..c45c2d6 100644
--- a/database/storage/interface.go
+++ b/database/storage/interface.go
@@ -18,13 +18,17 @@ type Interface interface {
 
 	ReadOnly() bool
 	Injected() bool
+	Shutdown() error
+}
+
+// Maintenance defines the database storage API for backends that requ
+type Maintenance interface {
 	Maintain(ctx context.Context) error
 	MaintainThorough(ctx context.Context) error
 	MaintainRecordStates(ctx context.Context, purgeDeletedBefore time.Time) error
-	Shutdown() error
 }
 
 // Batcher defines the database storage API for backends that support batch operations.
 type Batcher interface {
-	PutMany() (batch chan<- record.Record, errs <-chan error)
+	PutMany(shadowDelete bool) (batch chan<- record.Record, errs <-chan error)
 }

From 362539692e510b82ea24c6eec73dd717d5ee5409 Mon Sep 17 00:00:00 2001
From: Daniel <dhaavi@users.noreply.github.com>
Date: Thu, 24 Sep 2020 09:50:38 +0200
Subject: [PATCH 2/2] Improve interfaces and fix more linter errors

---
 config/database.go                     |  1 -
 database/controller.go                 | 13 +++++--------
 database/storage/badger/badger_test.go | 26 +++++++++++++++++++-------
 database/storage/bbolt/bbolt.go        | 10 ----------
 database/storage/bbolt/bbolt_test.go   | 15 +++++++--------
 database/storage/fstree/fstree.go      | 11 +----------
 database/storage/fstree/fstree_test.go |  8 ++++++++
 database/storage/hashmap/map.go        | 10 ----------
 database/storage/hashmap/map_test.go   | 19 ++++++++-----------
 database/storage/interface.go          | 10 +++++++---
 notifications/database.go              |  1 -
 11 files changed, 55 insertions(+), 69 deletions(-)
 create mode 100644 database/storage/fstree/fstree_test.go

diff --git a/config/database.go b/config/database.go
index 109e575..a6419b2 100644
--- a/config/database.go
+++ b/config/database.go
@@ -134,7 +134,6 @@ func registerAsDatabase() error {
 		Name:        "config",
 		Description: "Configuration Manager",
 		StorageType: "injected",
-		PrimaryAPI:  "",
 	})
 	if err != nil {
 		return err
diff --git a/database/controller.go b/database/controller.go
index bb9926e..fd6ddbc 100644
--- a/database/controller.go
+++ b/database/controller.go
@@ -247,8 +247,8 @@ func (c *Controller) Maintain(ctx context.Context) error {
 		return nil
 	}
 
-	if maintenance, ok := c.storage.(storage.Maintenance); ok {
-		return maintenance.Maintain(ctx)
+	if maintainer, ok := c.storage.(storage.Maintainer); ok {
+		return maintainer.Maintain(ctx)
 	}
 	return nil
 }
@@ -262,8 +262,8 @@ func (c *Controller) MaintainThorough(ctx context.Context) error {
 		return nil
 	}
 
-	if maintenance, ok := c.storage.(storage.Maintenance); ok {
-		return maintenance.MaintainThorough(ctx)
+	if maintainer, ok := c.storage.(storage.Maintainer); ok {
+		return maintainer.MaintainThorough(ctx)
 	}
 	return nil
 }
@@ -277,10 +277,7 @@ func (c *Controller) MaintainRecordStates(ctx context.Context, purgeDeletedBefor
 		return nil
 	}
 
-	if maintenance, ok := c.storage.(storage.Maintenance); ok {
-		return maintenance.MaintainRecordStates(ctx, purgeDeletedBefore)
-	}
-	return nil
+	return c.storage.MaintainRecordStates(ctx, purgeDeletedBefore)
 }
 
 // Shutdown shuts down the storage.
diff --git a/database/storage/badger/badger_test.go b/database/storage/badger/badger_test.go
index 6ddb7db..934f664 100644
--- a/database/storage/badger/badger_test.go
+++ b/database/storage/badger/badger_test.go
@@ -11,6 +11,13 @@ import (
 
 	"github.com/safing/portbase/database/query"
 	"github.com/safing/portbase/database/record"
+	"github.com/safing/portbase/database/storage"
+)
+
+var (
+	// Compile time interface checks.
+	_ storage.Interface  = &Badger{}
+	_ storage.Maintainer = &Badger{}
 )
 
 type TestRecord struct {
@@ -117,13 +124,18 @@ func TestBadger(t *testing.T) {
 	}
 
 	// maintenance
-	err = db.Maintain(context.TODO())
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = db.MaintainThorough(context.TODO())
-	if err != nil {
-		t.Fatal(err)
+	maintainer, ok := db.(storage.Maintainer)
+	if ok {
+		err = maintainer.Maintain(context.TODO())
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = maintainer.MaintainThorough(context.TODO())
+		if err != nil {
+			t.Fatal(err)
+		}
+	} else {
+		t.Fatal("should implement Maintainer")
 	}
 
 	// shutdown
diff --git a/database/storage/bbolt/bbolt.go b/database/storage/bbolt/bbolt.go
index 654aacb..06e4d8f 100644
--- a/database/storage/bbolt/bbolt.go
+++ b/database/storage/bbolt/bbolt.go
@@ -245,16 +245,6 @@ func (b *BBolt) Injected() bool {
 	return false
 }
 
-// Maintain runs a light maintenance operation on the database.
-func (b *BBolt) Maintain(_ context.Context) error {
-	return nil
-}
-
-// MaintainThorough runs a thorough maintenance operation on the database.
-func (b *BBolt) MaintainThorough(_ context.Context) error {
-	return nil
-}
-
 // MaintainRecordStates maintains records states in the database.
 func (b *BBolt) MaintainRecordStates(ctx context.Context, purgeDeletedBefore time.Time) error {
 	now := time.Now().Unix()
diff --git a/database/storage/bbolt/bbolt_test.go b/database/storage/bbolt/bbolt_test.go
index 456cd39..783c67f 100644
--- a/database/storage/bbolt/bbolt_test.go
+++ b/database/storage/bbolt/bbolt_test.go
@@ -12,6 +12,13 @@ import (
 
 	"github.com/safing/portbase/database/query"
 	"github.com/safing/portbase/database/record"
+	"github.com/safing/portbase/database/storage"
+)
+
+var (
+	// Compile time interface checks.
+	_ storage.Interface = &BBolt{}
+	_ storage.Batcher   = &BBolt{}
 )
 
 type TestRecord struct {
@@ -146,14 +153,6 @@ func TestBBolt(t *testing.T) {
 	}
 
 	// maintenance
-	err = db.Maintain(context.TODO())
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = db.MaintainThorough(context.TODO())
-	if err != nil {
-		t.Fatal(err)
-	}
 	err = db.MaintainRecordStates(context.TODO(), time.Now())
 	if err != nil {
 		t.Fatal(err)
diff --git a/database/storage/fstree/fstree.go b/database/storage/fstree/fstree.go
index 229eed8..5970864 100644
--- a/database/storage/fstree/fstree.go
+++ b/database/storage/fstree/fstree.go
@@ -255,16 +255,6 @@ func (fst *FSTree) Injected() bool {
 	return false
 }
 
-// Maintain runs a light maintenance operation on the database.
-func (fst *FSTree) Maintain(_ context.Context) error {
-	return nil
-}
-
-// MaintainThorough runs a thorough maintenance operation on the database.
-func (fst *FSTree) MaintainThorough(_ context.Context) error {
-	return nil
-}
-
 // MaintainRecordStates maintains records states in the database.
 func (fst *FSTree) MaintainRecordStates(ctx context.Context, purgeDeletedBefore time.Time) error {
 	// TODO: implement MaintainRecordStates
@@ -279,6 +269,7 @@ func (fst *FSTree) Shutdown() error {
 // writeFile mirrors ioutil.WriteFile, replacing an existing file with the same
 // name atomically. This is not atomic on Windows, but still an improvement.
 // TODO: Replace with github.com/google/renamio.WriteFile as soon as it is fixed on Windows.
+// TODO: This has become a wont-fix. Explore other options.
 // This function is forked from https://github.com/google/renameio/blob/a368f9987532a68a3d676566141654a81aa8100b/writefile.go.
 func writeFile(filename string, data []byte, perm os.FileMode) error {
 	t, err := renameio.TempFile("", filename)
diff --git a/database/storage/fstree/fstree_test.go b/database/storage/fstree/fstree_test.go
new file mode 100644
index 0000000..88af845
--- /dev/null
+++ b/database/storage/fstree/fstree_test.go
@@ -0,0 +1,8 @@
+package fstree
+
+import "github.com/safing/portbase/database/storage"
+
+var (
+	// Compile time interface checks.
+	_ storage.Interface = &FSTree{}
+)
diff --git a/database/storage/hashmap/map.go b/database/storage/hashmap/map.go
index 25a3fb2..4eec7bd 100644
--- a/database/storage/hashmap/map.go
+++ b/database/storage/hashmap/map.go
@@ -150,16 +150,6 @@ func (hm *HashMap) Injected() bool {
 	return false
 }
 
-// Maintain runs a light maintenance operation on the database.
-func (hm *HashMap) Maintain(_ context.Context) error {
-	return nil
-}
-
-// MaintainThorough runs a thorough maintenance operation on the database.
-func (hm *HashMap) MaintainThorough(_ context.Context) error {
-	return nil
-}
-
 // MaintainRecordStates maintains records states in the database.
 func (hm *HashMap) MaintainRecordStates(ctx context.Context, purgeDeletedBefore time.Time) error {
 	hm.dbLock.Lock()
diff --git a/database/storage/hashmap/map_test.go b/database/storage/hashmap/map_test.go
index 36f279b..31df9e1 100644
--- a/database/storage/hashmap/map_test.go
+++ b/database/storage/hashmap/map_test.go
@@ -2,15 +2,22 @@
 package hashmap
 
 import (
-	"context"
 	"reflect"
 	"sync"
 	"testing"
 
+	"github.com/safing/portbase/database/storage"
+
 	"github.com/safing/portbase/database/query"
 	"github.com/safing/portbase/database/record"
 )
 
+var (
+	// Compile time interface checks.
+	_ storage.Interface = &HashMap{}
+	_ storage.Batcher   = &HashMap{}
+)
+
 type TestRecord struct {
 	record.Base
 	sync.Mutex
@@ -130,16 +137,6 @@ func TestHashMap(t *testing.T) {
 		t.Fatal("should fail")
 	}
 
-	// maintenance
-	err = db.Maintain(context.TODO())
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = db.MaintainThorough(context.TODO())
-	if err != nil {
-		t.Fatal(err)
-	}
-
 	// shutdown
 	err = db.Shutdown()
 	if err != nil {
diff --git a/database/storage/interface.go b/database/storage/interface.go
index c45c2d6..432ffd3 100644
--- a/database/storage/interface.go
+++ b/database/storage/interface.go
@@ -11,21 +11,25 @@ import (
 
 // Interface defines the database storage API.
 type Interface interface {
+	// Primary Interface
 	Get(key string) (record.Record, error)
 	Put(m record.Record) (record.Record, error)
 	Delete(key string) error
 	Query(q *query.Query, local, internal bool) (*iterator.Iterator, error)
 
+	// Information and Control
 	ReadOnly() bool
 	Injected() bool
 	Shutdown() error
+
+	// Mandatory Record Maintenance
+	MaintainRecordStates(ctx context.Context, purgeDeletedBefore time.Time) error
 }
 
-// Maintenance defines the database storage API for backends that requ
-type Maintenance interface {
+// Maintainer defines the database storage API for backends that require regular maintenance.
+type Maintainer interface {
 	Maintain(ctx context.Context) error
 	MaintainThorough(ctx context.Context) error
-	MaintainRecordStates(ctx context.Context, purgeDeletedBefore time.Time) error
 }
 
 // Batcher defines the database storage API for backends that support batch operations.
diff --git a/notifications/database.go b/notifications/database.go
index 58b455c..d73f4ca 100644
--- a/notifications/database.go
+++ b/notifications/database.go
@@ -48,7 +48,6 @@ func registerAsDatabase() error {
 		Name:        "notifications",
 		Description: "Notifications",
 		StorageType: "injected",
-		PrimaryAPI:  "",
 	})
 	if err != nil {
 		return err