Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define app store interface in client packages #536

Merged
merged 3 commits into from
Feb 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/picoshare/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") != "" {
Expand Down
22 changes: 13 additions & 9 deletions garbagecollect/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 4 additions & 4 deletions handlers/db_dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions handlers/db_prod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 3 additions & 3 deletions handlers/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion handlers/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion handlers/download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions handlers/guest_links_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions handlers/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -17,7 +16,7 @@ type (
Server struct {
router *mux.Router
authenticator auth.Authenticator
store store.Store
store Store
spaceChecker SpaceChecker
collector *garbagecollect.Collector
}
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion handlers/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
24 changes: 24 additions & 0 deletions handlers/store.go
Original file line number Diff line number Diff line change
@@ -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
}
6 changes: 3 additions & 3 deletions handlers/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"
Expand Down
14 changes: 7 additions & 7 deletions store/sqlite/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions store/sqlite/downloads.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
(
Expand All @@ -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,
Expand Down
Loading
Loading