Add a default expiration time setting (#364)

* Add a default expiration time setting

* Move settings.go

* work in progress

* Temp

* work in progress

* work in progress

* More progress

* work in progress

* work in progress

* work in progress

* work in progress

* Sync access to settings

* work in progress

* Parse file lifetime more rigorously

* Fix SQL indentation

* work in progress

* Add e2e tests

* Tweak UI

* Remove unnecessary comments

* Refactor constructors for file lifetime

cmd/picoshare/main.go

@@ -38,7 +38,12 @@ func main() {
 	gc := garbagecollect.NewScheduler(&collector, 7*time.Hour)
 	gc.StartAsync()
 
-	h := gorilla.LoggingHandler(os.Stdout, handlers.New(authenticator, store, &collector).Router())
+	server, err := handlers.New(authenticator, store, &collector)
+	if err != nil {
+		panic(err)
+	}
+
+	h := gorilla.LoggingHandler(os.Stdout, server.Router())
 	if os.Getenv("PS_BEHIND_PROXY") != "" {
 		h = gorilla.ProxyIPHeadersHandler(h)
 	}

e2e/auth.spec.ts

@@ -10,7 +10,7 @@ test("logs in and logs out", async ({ page }) => {
   await page.locator("form input[type='submit']").click();
 
   await expect(page).toHaveURL("/");
-  await page.locator(".navbar-end .navbar-item.is-hoverable").hover();
+  await page.locator("data-test-id=system-dropdown").hover();
   await page.locator("#navbar-log-out").click();
 
   await expect(

e2e/settings.spec.ts

@@ -0,0 +1,89 @@
+import { test, expect } from "@playwright/test";
+import { login } from "./helpers/login.js";
+
+test("default file expiration is 30 days", async ({ page }) => {
+  await login(page);
+
+  await page.locator("data-test-id=system-dropdown").hover();
+  await page.locator("a[href='/settings']").click();
+  await expect(page).toHaveURL("/settings");
+
+  await expect(page.locator("#default-expiration")).toHaveValue("30");
+  await expect(page.locator("#time-unit")).toHaveValue("days");
+
+  await page.locator("data-test-id=upload-btn").click();
+  await expect(page).toHaveURL("/");
+
+  await expect(page.locator("#expiration-select option[selected]")).toHaveText(
+    "30 days"
+  );
+});
+
+test("changes default file expiration to 5 days", async ({ page }) => {
+  await login(page);
+
+  await page.locator("data-test-id=system-dropdown").hover();
+  await page.locator("a[href='/settings']").click();
+  await expect(page).toHaveURL("/settings");
+
+  await page.locator("#default-expiration").fill("5");
+  await page.locator("#settings-form button[type='submit']").click();
+
+  await page.locator("data-test-id=upload-btn").click();
+  await expect(page).toHaveURL("/");
+
+  await expect(page.locator("#expiration-select option[selected]")).toHaveText(
+    "5 days"
+  );
+});
+
+test("changes default file expiration to 1 year", async ({ page }) => {
+  await login(page);
+
+  await page.locator("data-test-id=system-dropdown").hover();
+  await page.locator("a[href='/settings']").click();
+  await expect(page).toHaveURL("/settings");
+
+  await page.locator("#default-expiration").fill("1");
+  await page.locator("#time-unit").selectOption("years");
+  await page.locator("#settings-form button[type='submit']").click();
+
+  await page.locator("data-test-id=upload-btn").click();
+  await expect(page).toHaveURL("/");
+
+  await expect(page.locator("#expiration-select option[selected]")).toHaveText(
+    "1 year"
+  );
+
+  // Because 1 year is one of the built-in defaults, we shouldn't see any
+  // additional items in the options list.
+  const expirationOptions = await page
+    .locator("#expiration-select option")
+    .allInnerTexts();
+  expect(expirationOptions[0]).toEqual("1 day");
+  expect(expirationOptions[1]).toEqual("7 days");
+  expect(expirationOptions[2]).toEqual("30 days");
+  expect(expirationOptions[3]).toEqual("1 year");
+  expect(expirationOptions[4]).toEqual("Never");
+  expect(expirationOptions[5]).toEqual("Custom");
+});
+
+test("changes default file expiration to 10 years", async ({ page }) => {
+  await login(page);
+
+  await page.locator("data-test-id=system-dropdown").hover();
+  await page.locator("a[href='/settings']").click();
+  await expect(page).toHaveURL("/settings");
+
+  // Change default expiration to 10 years.
+  await page.locator("#default-expiration").fill("10");
+  await page.locator("#time-unit").selectOption("years");
+  await page.locator("#settings-form button[type='submit']").click();
+
+  await page.locator("data-test-id=upload-btn").click();
+  await expect(page).toHaveURL("/");
+
+  await expect(page.locator("#expiration-select option[selected]")).toHaveText(
+    "10 years"
+  );
+});

handlers/delete_test.go

@@ -22,7 +22,10 @@ func TestDeleteExistingFile(t *testing.T) {
 			ID: picoshare.EntryID("hR87apiUCj"),
 		})
 
-	s := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	s, err := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	req, err := http.NewRequest("DELETE", "/api/entry/hR87apiUCj", nil)
 	if err != nil {
@@ -46,7 +49,10 @@ func TestDeleteExistingFile(t *testing.T) {
 func TestDeleteNonExistentFile(t *testing.T) {
 	dataStore := test_sqlite.New()
 
-	s := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	s, err := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	req, err := http.NewRequest("DELETE", "/api/entry/hR87apiUCj", nil)
 	if err != nil {
@@ -66,7 +72,10 @@ func TestDeleteNonExistentFile(t *testing.T) {
 func TestDeleteInvalidEntryID(t *testing.T) {
 	dataStore := test_sqlite.New()
 
-	s := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	s, err := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	req, err := http.NewRequest("DELETE", "/api/entry/invalid-entry-id", nil)
 	if err != nil {

handlers/guest_links_test.go

@@ -56,7 +56,10 @@ func TestGuestLinksPostAcceptsValidRequest(t *testing.T) {
 		t.Run(tt.description, func(t *testing.T) {
 			dataStore := test_sqlite.New()
 
-			s := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+			s, err := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+			if err != nil {
+				t.Fatal(err)
+			}
 
 			req, err := http.NewRequest("POST", "/api/guest-links", strings.NewReader(tt.payload))
 			if err != nil {
@@ -209,7 +212,10 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
 		t.Run(tt.description, func(t *testing.T) {
 			dataStore := test_sqlite.New()
 
-			s := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+			s, err := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+			if err != nil {
+				t.Fatal(err)
+			}
 
 			req, err := http.NewRequest("POST", "/api/guest-links", strings.NewReader(tt.payload))
 			if err != nil {
@@ -244,7 +250,10 @@ func TestDeleteExistingGuestLink(t *testing.T) {
 		Expires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
 	})
 
-	s := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	s, err := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	req, err := http.NewRequest("DELETE", "/api/guest-links/abcdefgh23456789", nil)
 	if err != nil {
@@ -267,7 +276,10 @@ func TestDeleteExistingGuestLink(t *testing.T) {
 func TestDeleteNonExistentGuestLink(t *testing.T) {
 	dataStore := test_sqlite.New()
 
-	s := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	s, err := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	req, err := http.NewRequest("DELETE", "/api/guest-links/abcdefgh23456789", nil)
 	if err != nil {
@@ -287,7 +299,10 @@ func TestDeleteNonExistentGuestLink(t *testing.T) {
 func TestDeleteInvalidGuestLink(t *testing.T) {
 	dataStore := test_sqlite.New()
 
-	s := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	s, err := handlers.New(mockAuthenticator{}, dataStore, nilGarbageCollector)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	req, err := http.NewRequest("DELETE", "/api/guest-links/i-am-an-invalid-link", nil)
 	if err != nil {

handlers/parse/file_lifetime.go

@@ -0,0 +1,29 @@
+package parse
+
+import (
+	"fmt"
+
+	"github.com/mtlynch/picoshare/v2/picoshare"
+)
+
+const minFileLifetimeInDays = 1
+const maxFileLifetimeInYears = 10
+
+// This is imprecise, but it's okay because file lifetimes are not exact
+// measures of time.
+const daysPerYear = 365
+
+var (
+	ErrFileLifetimeTooShort = fmt.Errorf("file lifetime must be at least %d days", minFileLifetimeInDays)
+	ErrFileLifetimeTooLong  = fmt.Errorf("file lifetime must be at most %d years", maxFileLifetimeInYears)
+)
+
+func FileLifetime(lifetimeInDays uint16) (picoshare.FileLifetime, error) {
+	if lifetimeInDays < minFileLifetimeInDays {
+		return picoshare.FileLifetime{}, ErrFileLifetimeTooShort
+	}
+	if lifetimeInDays > (maxFileLifetimeInYears * daysPerYear) {
+		return picoshare.FileLifetime{}, ErrFileLifetimeTooLong
+	}
+	return picoshare.NewFileLifetimeInDays(lifetimeInDays), nil
+}

handlers/parse/file_lifetime_test.go

@@ -0,0 +1,58 @@
+package parse_test
+
+import (
+	"testing"
+
+	"github.com/mtlynch/picoshare/v2/handlers/parse"
+	"github.com/mtlynch/picoshare/v2/picoshare"
+)
+
+func TestFileLifetime(t *testing.T) {
+	for _, tt := range []struct {
+		description string
+		input       uint16
+		output      picoshare.FileLifetime
+		err         error
+	}{
+		{
+			description: "valid lifetime",
+			input:       7,
+			output:      picoshare.NewFileLifetimeInDays(7),
+			err:         nil,
+		},
+		{
+			description: "accepts the minimum valid lifetime",
+			input:       1,
+			output:      picoshare.NewFileLifetimeInDays(1),
+			err:         nil,
+		},
+		{
+			description: "accepts the maximum valid lifetime",
+			input:       365 * 10,
+			output:      picoshare.NewFileLifetimeInYears(10),
+			err:         nil,
+		},
+		{
+			description: "rejects too short a lifetime",
+			input:       0,
+			output:      picoshare.FileLifetime{},
+			err:         parse.ErrFileLifetimeTooShort,
+		},
+		{
+			description: "rejects too long a lifetime",
+			input:       3651,
+			output:      picoshare.FileLifetime{},
+			err:         parse.ErrFileLifetimeTooLong,
+		},
+	} {
+		t.Run(tt.description, func(t *testing.T) {
+			lt, err := parse.FileLifetime(tt.input)
+			if got, want := err, tt.err; got != want {
+				t.Fatalf("err=%v, want=%v", got, want)
+			}
+			if got, want := lt, tt.output; got != want {
+				t.Errorf("lifetime=%s, want=%s", got.FriendlyName(), want.FriendlyName())
+			}
+		})
+	}
+}

handlers/routes.go

@@ -15,6 +15,7 @@ func (s *Server) routes() {
 	authenticatedApis.HandleFunc("/entry/{id}", s.entryDelete()).Methods(http.MethodDelete)
 	authenticatedApis.HandleFunc("/guest-links", s.guestLinksPost()).Methods(http.MethodPost)
 	authenticatedApis.HandleFunc("/guest-links/{id}", s.guestLinksDelete()).Methods(http.MethodDelete)
+	authenticatedApis.HandleFunc("/settings", s.settingsPut()).Methods(http.MethodPut)
 
 	publicApis := s.router.PathPrefix("/api").Subrouter()
 	publicApis.HandleFunc("/guest/{guestLinkID}", s.guestEntryPost()).Methods(http.MethodPost)
@@ -48,6 +49,7 @@ func (s *Server) routes() {
 	authenticatedViews.HandleFunc("/files/{id}/confirm-delete", s.fileConfirmDeleteGet()).Methods(http.MethodGet)
 	authenticatedViews.HandleFunc("/guest-links", s.guestLinkIndexGet()).Methods(http.MethodGet)
 	authenticatedViews.HandleFunc("/guest-links/new", s.guestLinksNewGet()).Methods(http.MethodGet)
+	authenticatedViews.HandleFunc("/settings", s.settingsGet()).Methods(http.MethodGet)
 
 	views := s.router.PathPrefix("/").Subrouter()
 	views.Use(upgradeToHttps)

handlers/server.go

@@ -1,19 +1,30 @@
 package handlers
 
 import (
+	"sync"
+
 	"github.com/gorilla/mux"
 
 	"github.com/mtlynch/picoshare/v2/garbagecollect"
 	"github.com/mtlynch/picoshare/v2/handlers/auth"
+	"github.com/mtlynch/picoshare/v2/picoshare"
 	"github.com/mtlynch/picoshare/v2/store"
 )
 
-type Server struct {
-	router        *mux.Router
-	authenticator auth.Authenticator
-	store         store.Store
-	collector     *garbagecollect.Collector
-}
+type (
+	syncedSettings struct {
+		settings picoshare.Settings
+		mu       *sync.RWMutex
+	}
+
+	Server struct {
+		router        *mux.Router
+		authenticator auth.Authenticator
+		store         store.Store
+		collector     *garbagecollect.Collector
+		settings      *syncedSettings
+	}
+)
 
 // Router returns the underlying router interface for the server.
 func (s Server) Router() *mux.Router {
@@ -22,14 +33,34 @@ 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, collector *garbagecollect.Collector) Server {
+func New(authenticator auth.Authenticator, store store.Store, collector *garbagecollect.Collector) (Server, error) {
+	settings, err := store.ReadSettings()
+	if err != nil {
+		return Server{}, err
+	}
 	s := Server{
 		router:        mux.NewRouter(),
 		authenticator: authenticator,
 		store:         store,
 		collector:     collector,
+		settings: &syncedSettings{
+			settings: settings,
+			mu:       &sync.RWMutex{},
+		},
 	}
 
 	s.routes()
-	return s
+	return s, nil
+}
+
+func (ss syncedSettings) Get() picoshare.Settings {
+	ss.mu.RLock()
+	defer ss.mu.RUnlock()
+	return ss.settings
+}
+
+func (ss *syncedSettings) Update(s picoshare.Settings) {
+	ss.mu.Lock()
+	ss.settings = s
+	ss.mu.Unlock()
 }