commit 0e14e23a2a2c5df904cfe93c10ae60229001a9be
parent 4792b18ac7660694e730545e0006e3dd91067aed
Author: David Eisinger <[email protected]>
Date: Tue, 7 Oct 2025 00:42:58 -0400
Handle invalid updates
Diffstat:
3 files changed, 167 insertions(+), 19 deletions(-)
diff --git a/internal/api.go b/internal/api.go
@@ -531,31 +531,50 @@ func NewHandlerWithConfig(db *DB, cfg HandlerConfig) (http.Handler, error) {
http.Error(w, "url and title are required", http.StatusBadRequest)
return
}
+ // Load existing to compare URL and preserve fields.
+ existing, err := db.GetBookmark(r.Context(), id64)
+ if err != nil {
+ log.Printf("get bookmark error: %v", err)
+ http.Error(w, "internal server error", http.StatusInternalServerError)
+ return
+ }
+ if existing == nil {
+ http.NotFound(w, r)
+ return
+ }
desc := req.Description
var pub bool
- // Fetch existing if we need to preserve fields
- if desc == nil || req.Public == nil {
- existing, err := db.GetBookmark(r.Context(), id64)
- if err != nil {
- log.Printf("get bookmark error: %v", err)
- http.Error(w, "internal server error", http.StatusInternalServerError)
- return
- }
- if existing == nil {
- http.NotFound(w, r)
- return
- }
- if desc == nil && existing.Description.Valid {
- d := existing.Description.String
- desc = &d
- }
- if req.Public == nil {
- pub = existing.Public
- }
+ if desc == nil && existing.Description.Valid {
+ d := existing.Description.String
+ desc = &d
+ }
+ if req.Public == nil {
+ pub = existing.Public
}
if req.Public != nil {
pub = *req.Public
}
+
+ // If URL changed, try to refetch archive for the new URL before updating.
+ urlChanged := strings.TrimSpace(req.URL) != existing.URL
+ var newArchive string
+ var shouldArchive bool
+ if urlChanged {
+ ctx, cancel := context.WithTimeout(r.Context(), 45*time.Second)
+ defer cancel()
+ if IsHTMLURL(ctx, req.URL) {
+ shouldArchive = true
+ _, txt, err := TitleAndText(ctx, req.URL)
+ if err != nil {
+ http.Error(w, "failed to archive URL", http.StatusBadGateway)
+ return
+ }
+ newArchive = txt
+ } else {
+ shouldArchive = false
+ }
+ }
+
updated, err := db.UpdateBookmark(r.Context(), id64, req.URL, req.Title, desc, pub)
if err != nil {
log.Printf("update bookmark error: %v", err)
@@ -566,6 +585,22 @@ func NewHandlerWithConfig(db *DB, cfg HandlerConfig) (http.Handler, error) {
http.NotFound(w, r)
return
}
+ // Apply archive changes if URL changed
+ if urlChanged {
+ if shouldArchive {
+ if err := db.SaveArchive(r.Context(), id64, newArchive); err != nil {
+ log.Printf("save archive after URL change error: %v", err)
+ http.Error(w, "internal server error", http.StatusInternalServerError)
+ return
+ }
+ } else {
+ if err := db.DeleteArchive(r.Context(), id64); err != nil {
+ log.Printf("delete archive after URL change error: %v", err)
+ http.Error(w, "internal server error", http.StatusInternalServerError)
+ return
+ }
+ }
+ }
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(toRespWithArchive(r.Context(), db, updated))
case http.MethodDelete:
diff --git a/internal/database.go b/internal/database.go
@@ -409,6 +409,26 @@ func (db *DB) HasArchive(ctx context.Context, id int64) (bool, error) {
return true, nil
}
+// DeleteArchive removes the archive row for a bookmark and clears its FTS
+// content by reindexing the metadata with empty content.
+func (db *DB) DeleteArchive(ctx context.Context, id int64) error {
+ if _, err := db.sql.ExecContext(ctx, `DELETE FROM archives WHERE bookmark_id = ?`, id); err != nil {
+ return fmt.Errorf("delete archive: %w", err)
+ }
+ // Refresh FTS row with empty content
+ b, err := db.GetBookmark(ctx, id)
+ if err != nil {
+ return fmt.Errorf("get bookmark for fts after archive delete: %w", err)
+ }
+ if b == nil {
+ return nil
+ }
+ if err := ftsReplace(ctx, db.sql, id, b.Title, b.URL, nullString(b.Description), ""); err != nil {
+ return fmt.Errorf("fts replace after archive delete: %w", err)
+ }
+ return nil
+}
+
func nullString(ns sql.NullString) any {
if ns.Valid {
return ns.String
diff --git a/internal/update_archive_test.go b/internal/update_archive_test.go
@@ -0,0 +1,93 @@
+package internal
+
+import (
+ "bytes"
+ "encoding/json"
+ "io"
+ "net/http"
+ "net/http/cookiejar"
+ "net/http/httptest"
+ "testing"
+)
+
+func TestUpdateURL_RefetchArchiveAndRejectOnError(t *testing.T) {
+ t.Parallel()
+
+ // HTML servers
+ good := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == http.MethodHead {
+ w.Header().Set("Content-Type", "text/html; charset=utf-8")
+ return
+ }
+ w.Header().Set("Content-Type", "text/html; charset=utf-8")
+ io.WriteString(w, "<!doctype html><html><head><title>T1</title></head><body>alpha</body></html>")
+ }))
+ defer good.Close()
+ changed := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == http.MethodHead {
+ w.Header().Set("Content-Type", "text/html; charset=utf-8")
+ return
+ }
+ w.Header().Set("Content-Type", "text/html; charset=utf-8")
+ io.WriteString(w, "<!doctype html><html><head><title>T2</title></head><body>bravo</body></html>")
+ }))
+ defer changed.Close()
+ failing := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if r.Method == http.MethodHead {
+ w.Header().Set("Content-Type", "text/html; charset=utf-8")
+ return
+ }
+ http.Error(w, "nope", http.StatusNotFound)
+ }))
+ defer failing.Close()
+
+ db, _ := OpenAtPath(t.Context(), t.TempDir()+"/upd.db")
+ t.Cleanup(func() { _ = db.Close() })
+ h, _ := NewHandler(db)
+ srv := httptest.NewServer(h)
+ defer srv.Close()
+
+ jar, _ := cookiejar.New(nil)
+ client := &http.Client{Jar: jar}
+ // signup
+ b, _ := json.Marshal(map[string]string{"username": "upd", "password": "pw"})
+ r, _ := http.NewRequest(http.MethodPost, srv.URL+"/signup", bytes.NewReader(b))
+ r.Header.Set("Content-Type", "application/json")
+ rs, _ := client.Do(r)
+ if rs.StatusCode != 200 {
+ t.Fatalf("signup %d", rs.StatusCode)
+ }
+ rs.Body.Close()
+
+ // create initial
+ cr := postJSONWithClient[map[string]any](t, client, srv.URL+"/bookmarks", map[string]any{"url": good.URL}, 201)
+ id := int(cr["id"].(float64))
+
+ // update URL to changed (should refetch and succeed)
+ putJSONWithClient[map[string]any](t, client, srv.URL+"/bookmarks/"+itoa(id), map[string]any{"url": changed.URL, "title": "First"}, 200)
+ // archive should contain 'bravo'
+ resp := getWithClient(t, client, srv.URL+"/bookmarks/"+itoa(id)+"/archive", 200)
+ body, _ := io.ReadAll(resp.Body)
+ resp.Body.Close()
+ if !bytes.Contains(bytes.ToLower(body), []byte("bravo")) {
+ t.Fatalf("expected new archive to contain bravo, got %q", string(body))
+ }
+
+ // update URL to failing (fetch GET 404) should reject
+ req, _ := http.NewRequest(http.MethodPut, srv.URL+"/bookmarks/"+itoa(id), bytes.NewReader([]byte("{\"url\":\""+failing.URL+"\",\"title\":\"First\"}")))
+ req.Header.Set("Content-Type", "application/json")
+ rs2, _ := client.Do(req)
+ if rs2.StatusCode != http.StatusBadGateway {
+ t.Fatalf("expected 502 on fetch failure, got %d", rs2.StatusCode)
+ }
+ rs2.Body.Close()
+
+ // ensure URL not changed
+ list := getJSONWithClient[[]map[string]any](t, client, srv.URL+"/bookmarks", 200)
+ if len(list) != 1 {
+ t.Fatalf("expected 1 bookmark, got %d", len(list))
+ }
+ if list[0]["url"].(string) != changed.URL {
+ t.Fatalf("expected url to remain %q, got %q", changed.URL, list[0]["url"])
+ }
+}
