Define app store interface in client packages (#536)

It makes more sense for the consuming packages to define the interface to the application data store. The SQLite implementation should just export what it exports, and the clients define interfaces over what they need.
mtlynch · Feb 3, 2024 · bf4b1ca · bf4b1ca
1 parent 579cb31
commit bf4b1ca
Show file tree

Hide file tree

Showing 20 changed files with 105 additions and 101 deletions.
diff --git a/cmd/picoshare/main.go b/cmd/picoshare/main.go
@@ -45,7 +45,7 @@ func main() {
 	gc := garbagecollect.NewScheduler(&collector, 7*time.Hour)
 	gc.StartAsync()
 
-	server := handlers.New(authenticator, store, spaceChecker, &collector)
+	server := handlers.New(authenticator, &store, spaceChecker, &collector)
 
 	h := gorilla.LoggingHandler(os.Stdout, server.Router())
 	if os.Getenv("PS_BEHIND_PROXY") != "" {

diff --git a/garbagecollect/collect.go b/garbagecollect/collect.go
@@ -2,26 +2,30 @@ package garbagecollect
 
 import (
 	"sync"
-
-	"github.com/mtlynch/picoshare/v2/store"
 )
 
-type Collector struct {
-	store store.Store
-	mu    sync.Mutex
-}
+type (
+	DatabasePurger interface {
+		Purge() error
+	}
+
+	Collector struct {
+		purger DatabasePurger
+		mu     sync.Mutex
+	}
+)
 
-func NewCollector(store store.Store) Collector {
+func NewCollector(purger DatabasePurger) Collector {
 	return Collector{
-		store: store,
+		purger: purger,
 	}
 }
 
 func (c *Collector) Collect() error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	if err := c.store.Purge(); err != nil {
+	if err := c.purger.Purge(); err != nil {
 		return err
 	}
 

diff --git a/handlers/db_dev.go b/handlers/db_dev.go
@@ -9,7 +9,6 @@ import (
 	"sync"
 
 	"github.com/mtlynch/picoshare/v2/random"
-	"github.com/mtlynch/picoshare/v2/store"
 	"github.com/mtlynch/picoshare/v2/store/test_sqlite"
 )
 
@@ -48,10 +47,10 @@ func (dbs *dbSettings) SetIsolateBySession(isolate bool) {
 
 var (
 	sharedDBSettings dbSettings
-	tokenToDB        map[dbToken]store.Store = map[dbToken]store.Store{}
+	tokenToDB        map[dbToken]Store = map[dbToken]Store{}
 )
 
-func (s Server) getDB(r *http.Request) store.Store {
+func (s Server) getDB(r *http.Request) Store {
 	if !sharedDBSettings.IsolateBySession() {
 		return s.store
 	}
@@ -90,7 +89,8 @@ func assignSessionDB(h http.Handler) http.Handler {
 				token := dbToken(random.String(30, []rune("abcdefghijkmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")))
 				log.Printf("provisioning a new private database with token %s", token)
 				createDBCookie(token, w)
-				tokenToDB[token] = test_sqlite.New()
+				testDb := test_sqlite.New()
+				tokenToDB[token] = &testDb
 			}
 		}
 		h.ServeHTTP(w, r)

diff --git a/handlers/db_prod.go b/handlers/db_prod.go
@@ -4,14 +4,12 @@ package handlers
 
 import (
 	"net/http"
-
-	"github.com/mtlynch/picoshare/v2/store"
 )
 
 func (s *Server) addDevRoutes() {
 	// no-op
 }
 
-func (s Server) getDB(*http.Request) store.Store {
+func (s Server) getDB(*http.Request) Store {
 	return s.store
 }
diff --git a/handlers/delete_test.go b/handlers/delete_test.go
@@ -22,7 +22,7 @@ func TestDeleteExistingFile(t *testing.T) {
 		picoshare.UploadMetadata{
 			ID: picoshare.EntryID("hR87apiUCj"),
 		})
-	s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+	s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 	req, err := http.NewRequest("DELETE", "/api/entry/hR87apiUCj", nil)
 	if err != nil {
@@ -45,7 +45,7 @@ func TestDeleteExistingFile(t *testing.T) {
 
 func TestDeleteNonExistentFile(t *testing.T) {
 	dataStore := test_sqlite.New()
-	s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+	s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 	req, err := http.NewRequest("DELETE", "/api/entry/hR87apiUCj", nil)
 	if err != nil {
@@ -64,7 +64,7 @@ func TestDeleteNonExistentFile(t *testing.T) {
 
 func TestDeleteInvalidEntryID(t *testing.T) {
 	dataStore := test_sqlite.New()
-	s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+	s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 	req, err := http.NewRequest("DELETE", "/api/entry/invalid-entry-id", nil)
 	if err != nil {

diff --git a/handlers/download.go b/handlers/download.go
@@ -64,7 +64,7 @@ func inferContentTypeFromFilename(f picoshare.Filename) (picoshare.ContentType,
 	return picoshare.ContentType(""), errors.New("could not infer content type from filename")
 }
 
-func recordDownload(db store.Store, id picoshare.EntryID, remoteAddr, userAgent string) error {
+func recordDownload(db Store, id picoshare.EntryID, remoteAddr, userAgent string) error {
 	ip, _, err := net.SplitHostPort(remoteAddr)
 	if err != nil {
 		ip = remoteAddr

diff --git a/handlers/download_test.go b/handlers/download_test.go
@@ -116,7 +116,7 @@ func TestEntryGet(t *testing.T) {
 				}
 			}
 
-			s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+			s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 			req, err := http.NewRequest("GET", tt.requestRoute, nil)
 			if err != nil {

diff --git a/handlers/guest_links_test.go b/handlers/guest_links_test.go
@@ -55,7 +55,7 @@ func TestGuestLinksPostAcceptsValidRequest(t *testing.T) {
 	} {
 		t.Run(tt.description, func(t *testing.T) {
 			dataStore := test_sqlite.New()
-			s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+			s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 			req, err := http.NewRequest("POST", "/api/guest-links", strings.NewReader(tt.payload))
 			if err != nil {
@@ -207,7 +207,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
 	} {
 		t.Run(tt.description, func(t *testing.T) {
 			dataStore := test_sqlite.New()
-			s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+			s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 			req, err := http.NewRequest("POST", "/api/guest-links", strings.NewReader(tt.payload))
 			if err != nil {
@@ -242,7 +242,7 @@ func TestDeleteExistingGuestLink(t *testing.T) {
 		Expires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
 	})
 
-	s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+	s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 	req, err := http.NewRequest("DELETE", "/api/guest-links/abcdefgh23456789", nil)
 	if err != nil {
@@ -264,7 +264,7 @@ func TestDeleteExistingGuestLink(t *testing.T) {
 
 func TestDeleteNonExistentGuestLink(t *testing.T) {
 	dataStore := test_sqlite.New()
-	s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+	s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 	req, err := http.NewRequest("DELETE", "/api/guest-links/abcdefgh23456789", nil)
 	if err != nil {
@@ -283,7 +283,7 @@ func TestDeleteNonExistentGuestLink(t *testing.T) {
 
 func TestDeleteInvalidGuestLink(t *testing.T) {
 	dataStore := test_sqlite.New()
-	s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+	s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 	req, err := http.NewRequest("DELETE", "/api/guest-links/i-am-an-invalid-link", nil)
 	if err != nil {

diff --git a/handlers/server.go b/handlers/server.go
@@ -6,7 +6,6 @@ import (
 	"github.com/mtlynch/picoshare/v2/garbagecollect"
 	"github.com/mtlynch/picoshare/v2/handlers/auth"
 	"github.com/mtlynch/picoshare/v2/space"
-	"github.com/mtlynch/picoshare/v2/store"
 )
 
 type (
@@ -17,7 +16,7 @@ type (
 	Server struct {
 		router        *mux.Router
 		authenticator auth.Authenticator
-		store         store.Store
+		store         Store
 		spaceChecker  SpaceChecker
 		collector     *garbagecollect.Collector
 	}
@@ -30,7 +29,7 @@ func (s Server) Router() *mux.Router {
 
 // New creates a new server with all the state it needs to satisfy HTTP
 // requests.
-func New(authenticator auth.Authenticator, store store.Store, spaceChecker SpaceChecker, collector *garbagecollect.Collector) Server {
+func New(authenticator auth.Authenticator, store Store, spaceChecker SpaceChecker, collector *garbagecollect.Collector) Server {
 	s := Server{
 		router:        mux.NewRouter(),
 		authenticator: authenticator,

diff --git a/handlers/settings_test.go b/handlers/settings_test.go
@@ -66,7 +66,7 @@ func TestSettingsPut(t *testing.T) {
 	} {
 		t.Run(tt.description, func(t *testing.T) {
 			dataStore := test_sqlite.New()
-			s := handlers.New(mockAuthenticator{}, dataStore, nilSpaceChecker, nilGarbageCollector)
+			s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
 
 			req, err := http.NewRequest("PUT", "/api/settings", strings.NewReader(tt.payload))
 			if err != nil {

diff --git a/handlers/store.go b/handlers/store.go
@@ -0,0 +1,24 @@
+package handlers
+
+import (
+	"io"
+
+	"github.com/mtlynch/picoshare/v2/picoshare"
+)
+
+type Store interface {
+	GetEntriesMetadata() ([]picoshare.UploadMetadata, error)
+	GetEntry(id picoshare.EntryID) (picoshare.UploadEntry, error)
+	GetEntryMetadata(id picoshare.EntryID) (picoshare.UploadMetadata, error)
+	InsertEntry(reader io.Reader, metadata picoshare.UploadMetadata) error
+	UpdateEntryMetadata(id picoshare.EntryID, metadata picoshare.UploadMetadata) error
+	DeleteEntry(id picoshare.EntryID) error
+	GetGuestLink(picoshare.GuestLinkID) (picoshare.GuestLink, error)
+	GetGuestLinks() ([]picoshare.GuestLink, error)
+	InsertGuestLink(picoshare.GuestLink) error
+	DeleteGuestLink(picoshare.GuestLinkID) error
+	InsertEntryDownload(picoshare.EntryID, picoshare.DownloadRecord) error
+	GetEntryDownloads(id picoshare.EntryID) ([]picoshare.DownloadRecord, error)
+	ReadSettings() (picoshare.Settings, error)
+	UpdateSettings(picoshare.Settings) error
+}
diff --git a/handlers/upload_test.go b/handlers/upload_test.go
@@ -93,7 +93,7 @@ func TestEntryPost(t *testing.T) {
 	} {
 		t.Run(tt.description, func(t *testing.T) {
 			store := test_sqlite.New()
-			s := handlers.New(mockAuthenticator{}, store, nilSpaceChecker, nilGarbageCollector)
+			s := handlers.New(mockAuthenticator{}, &store, nilSpaceChecker, nilGarbageCollector)
 
 			formData, contentType := createMultipartFormBody(tt.filename, tt.note, bytes.NewBuffer([]byte(tt.contents)))
 
@@ -225,7 +225,7 @@ func TestEntryPut(t *testing.T) {
 		t.Run(tt.description, func(t *testing.T) {
 			store := test_sqlite.New()
 			store.InsertEntry(strings.NewReader(("dummy data")), originalEntry)
-			s := handlers.New(mockAuthenticator{}, store, nilSpaceChecker, nilGarbageCollector)
+			s := handlers.New(mockAuthenticator{}, &store, nilSpaceChecker, nilGarbageCollector)
 
 			req, err := http.NewRequest("PUT", "/api/entry/"+tt.targetID, strings.NewReader(tt.payload))
 			if err != nil {
@@ -383,7 +383,7 @@ func TestGuestUpload(t *testing.T) {
 				}
 			}
 
-			s := handlers.New(authenticator, store, nilSpaceChecker, nilGarbageCollector)
+			s := handlers.New(authenticator, &store, nilSpaceChecker, nilGarbageCollector)
 
 			filename := "dummyimage.png"
 			contents := "dummy bytes"

diff --git a/store/sqlite/cleanup.go b/store/sqlite/cleanup.go
@@ -7,23 +7,23 @@ import (
 )
 
 // Purge deletes expired entries and clears orphaned rows from the database.
-func (d DB) Purge() error {
+func (s Store) Purge() error {
 	log.Printf("deleting expired entries and orphaned data from database")
-	if err := d.deleteExpiredEntries(); err != nil {
+	if err := s.deleteExpiredEntries(); err != nil {
 		return err
 	}
 
-	if err := d.deleteOrphanedRows(); err != nil {
+	if err := s.deleteOrphanedRows(); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (d DB) deleteExpiredEntries() error {
+func (s Store) deleteExpiredEntries() error {
 	log.Printf("deleting expired entries from database")
 
-	tx, err := d.ctx.BeginTx(context.Background(), nil)
+	tx, err := s.ctx.BeginTx(context.Background(), nil)
 	if err != nil {
 		return err
 	}
@@ -59,12 +59,12 @@ func (d DB) deleteExpiredEntries() error {
 	return tx.Commit()
 }
 
-func (d DB) deleteOrphanedRows() error {
+func (s Store) deleteOrphanedRows() error {
 	log.Printf("purging orphaned rows from database")
 
 	// Delete rows from entries_data if they don't reference valid rows in
 	// entries. This can happen if the entry insertion fails partway through.
-	if _, err := d.ctx.Exec(`
+	if _, err := s.ctx.Exec(`
 		DELETE FROM
 			entries_data
 		WHERE

diff --git a/store/sqlite/downloads.go b/store/sqlite/downloads.go
@@ -7,9 +7,9 @@ import (
 	"github.com/mtlynch/picoshare/v2/picoshare"
 )
 
-func (d DB) InsertEntryDownload(id picoshare.EntryID, r picoshare.DownloadRecord) error {
+func (s Store) InsertEntryDownload(id picoshare.EntryID, r picoshare.DownloadRecord) error {
 	log.Printf("recording download of file ID %s from client %s", id.String(), r.ClientIP)
-	if _, err := d.ctx.Exec(`
+	if _, err := s.ctx.Exec(`
 	INSERT INTO
 		downloads
 	(
@@ -30,8 +30,8 @@ func (d DB) InsertEntryDownload(id picoshare.EntryID, r picoshare.DownloadRecord
 	return nil
 }
 
-func (d DB) GetEntryDownloads(id picoshare.EntryID) ([]picoshare.DownloadRecord, error) {
-	rows, err := d.ctx.Query(`
+func (s Store) GetEntryDownloads(id picoshare.EntryID) ([]picoshare.DownloadRecord, error) {
+	rows, err := s.ctx.Query(`
 	SELECT
 		download_timestamp,
 		client_ip,