Wave 3: search depth, transfer size, and the Home screen's tests
Authors are searchable (internal/search, migrations/0009) - The FTS index covered only title and path, so typing an artist or author found nothing even though people/item_people already held the credits. - Rather than a second index, the names are denormalised into a real media_items.authors column, so the existing external-content FTS and its triggers keep doing the work; item_people/people triggers recompute it, and the media_items UPDATE trigger re-indexes automatically. - Verified: searching "daft punk" now returns Random Access Memories, and so does the partial "punk". - The agent found and worked around a real FTS5 external-content bug: on an UPGRADE the backfill UPDATE fires the delete trigger for rowids not yet in the fresh index, raising "database disk image is malformed". A rebuild before the backfill fixes it. This would have broken existing databases while a fresh install looked fine. gzip for the SPA (internal/httpapi/static.go) - 456 KB of JS and 31 KB of CSS were served uncompressed on every cold load. - Scoped to the embedded SPA assets only, never API or media: compressing a Range response would break seeking, and wrapping every response would undo Wave 2's Flusher/ReaderFrom forwarding. - Compresses only when the client accepts gzip, the response is a compressible type, there is no Range header, and nothing already set Content-Encoding. Sets Vary: Accept-Encoding and drops the now-wrong Content-Length. - Verified by sha256 round-trip (identity vs gzip-decompressed are identical), and by confirming a Range request still returns 206 with no Content-Encoding. JS 351,835 -> 108,794 bytes (69% smaller); CSS 80% smaller. epub.js is code-split (Reader.svelte) - It was a static import, so ~350 KB was downloaded by everyone on first load even though most never open an ebook. Now loaded on demand. - Main bundle 458 KB -> 112 KB (gzipped 149 KB -> 39.5 KB). - Verified at runtime, which pnpm check cannot: only the main bundle loads initially, the epubjs chunk is fetched when a book is opened, and the reader still renders. Home screen tests (internal/media) - Coverage 23.4% -> 50.2%. (The 17.9% quoted in the brief was a stale reading; the agent measured the real baseline by moving its own files aside.) - Covers the five shelves, counts, Explore's invariants (visibility, no duplicates, every kind represented, varies across calls, wrap-around), the list offset/limit/total contract, MostViewed ordering, and — most importantly — grant scoping, which is the security chokepoint. Fixed a bug the tests found: Suggestions seeded from revoked libraries - topEngagement selected the user's most recent progress row with no library filter, so progress from a library whose grant had been revoked still steered the home screen — a visible item was labelled "More books" based on content the user could no longer read. Returned items were always correctly scoped, so this was an information-flow gap rather than a content leak. - Now scoped to visible libraries, and the test that pinned the old behaviour flipped to assert the correct reason. gofmt/vet clean, 14 packages pass, media tests run 3x for flakiness.
This commit is contained in:
mithral
parent
cf34d8b11a
commit
09474b0e08
10 changed files with 1817 additions and 10 deletions
|
|
@ -1,9 +1,14 @@
|
|||
package httpapi
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"compress/gzip"
|
||||
"io"
|
||||
"io/fs"
|
||||
"net"
|
||||
"net/http"
|
||||
"path"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/mithral/vellum/web"
|
||||
|
|
@ -30,23 +35,31 @@ func (s *Server) mountWeb() {
|
|||
}
|
||||
|
||||
files := http.FileServer(http.FS(sub))
|
||||
|
||||
// Only the embedded static assets are wrapped for compression. API
|
||||
// responses and media are served by other handlers and are never touched.
|
||||
serveFiles := compressStatic(files.ServeHTTP)
|
||||
serveShell := compressStatic(func(w http.ResponseWriter, r *http.Request) {
|
||||
serveIndex(w, index)
|
||||
})
|
||||
|
||||
s.mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
|
||||
if isAPIPath(r.URL.Path) {
|
||||
s.handleNotFound(w, r)
|
||||
return
|
||||
}
|
||||
if r.URL.Path == "/" || r.URL.Path == "" {
|
||||
serveIndex(w, index)
|
||||
serveShell(w, r)
|
||||
return
|
||||
}
|
||||
name := strings.TrimPrefix(path.Clean(r.URL.Path), "/")
|
||||
if f, err := sub.Open(name); err == nil {
|
||||
_ = f.Close()
|
||||
files.ServeHTTP(w, r)
|
||||
serveFiles(w, r)
|
||||
return
|
||||
}
|
||||
// SPA route: hand back the shell and let the client router decide.
|
||||
serveIndex(w, index)
|
||||
serveShell(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -59,3 +72,180 @@ func serveIndex(w http.ResponseWriter, index []byte) {
|
|||
func isAPIPath(p string) bool {
|
||||
return p == "/healthz" || strings.HasPrefix(p, "/api/")
|
||||
}
|
||||
|
||||
// ── Static-asset gzip ──────────────────────────────────────────────────
|
||||
//
|
||||
// The built SPA ships uncompressed: the JS bundle is ~456 KB and the CSS
|
||||
// ~31 KB on every cold load. compressStatic wraps only the embedded static
|
||||
// handlers with gzip negotiation; API responses and media are never wrapped.
|
||||
//
|
||||
// The narrow scope is deliberate. (a) Media is already compressed. (b) A Range
|
||||
// response is used for seeking, and re-encoding it would invalidate the byte
|
||||
// offsets. (c) server.go's statusRecorder was just fixed to forward
|
||||
// http.Flusher and io.ReaderFrom for streaming; wrapping every response in a
|
||||
// gzip writer would force all bodies through a compressor and undo that.
|
||||
// Restricting the wrapper to the static file handler avoids all three.
|
||||
|
||||
// compressStatic gzips a static handler's response when the client accepts
|
||||
// gzip and the response is a compressible text type. Close is called after the
|
||||
// handler returns so the gzip trailer is always written.
|
||||
func compressStatic(next http.HandlerFunc) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
gz := newGzipResponseWriter(w, r)
|
||||
next(gz, r)
|
||||
_ = gz.Close()
|
||||
}
|
||||
}
|
||||
|
||||
// compressibleType reports whether a Content-Type should be gzip-encoded. Only
|
||||
// text-like types are listed; images, fonts, wasm and other media are already
|
||||
// compressed, so re-encoding them wastes CPU for no gain.
|
||||
func compressibleType(contentType string) bool {
|
||||
if i := strings.IndexByte(contentType, ';'); i >= 0 {
|
||||
contentType = contentType[:i] // drop "charset=utf-8" and friends
|
||||
}
|
||||
switch strings.ToLower(strings.TrimSpace(contentType)) {
|
||||
case "text/html",
|
||||
"text/css",
|
||||
"application/javascript",
|
||||
"text/javascript",
|
||||
"application/json",
|
||||
"image/svg+xml",
|
||||
"application/manifest+json":
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// acceptsGzip reports whether the client's Accept-Encoding allows gzip. An
|
||||
// explicit "gzip;q=0" is a refusal, so it does not count.
|
||||
func acceptsGzip(header string) bool {
|
||||
for _, part := range strings.Split(header, ",") {
|
||||
token, params, _ := strings.Cut(part, ";")
|
||||
if !strings.EqualFold(strings.TrimSpace(token), "gzip") {
|
||||
continue
|
||||
}
|
||||
for _, p := range strings.Split(params, ";") {
|
||||
if v, ok := strings.CutPrefix(strings.TrimSpace(p), "q="); ok {
|
||||
if q, err := strconv.ParseFloat(strings.TrimSpace(v), 64); err == nil && q == 0 {
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// gzipResponseWriter wraps a static response so its body can be gzip-encoded.
|
||||
//
|
||||
// Like statusRecorder it forwards Flush, Hijack and ReadFrom. While the body is
|
||||
// being compressed those forwards must route through the compressor (or flush
|
||||
// it first), or raw bytes would be written under a gzip Content-Encoding.
|
||||
type gzipResponseWriter struct {
|
||||
http.ResponseWriter
|
||||
eligible bool
|
||||
compress bool
|
||||
wroteHeader bool
|
||||
gz *gzip.Writer
|
||||
}
|
||||
|
||||
func newGzipResponseWriter(w http.ResponseWriter, r *http.Request) *gzipResponseWriter {
|
||||
return &gzipResponseWriter{
|
||||
ResponseWriter: w,
|
||||
// A Range request must stay identity so its byte offsets remain valid.
|
||||
eligible: acceptsGzip(r.Header.Get("Accept-Encoding")) && r.Header.Get("Range") == "",
|
||||
}
|
||||
}
|
||||
|
||||
// WriteHeader decides compression once the handler has set Content-Type. The
|
||||
// decision has to happen before the status line is committed.
|
||||
func (g *gzipResponseWriter) WriteHeader(code int) {
|
||||
if g.wroteHeader {
|
||||
return
|
||||
}
|
||||
g.wroteHeader = true
|
||||
|
||||
h := g.Header()
|
||||
if g.eligible &&
|
||||
code == http.StatusOK &&
|
||||
h.Get("Content-Encoding") == "" &&
|
||||
h.Get("Content-Length") != "0" &&
|
||||
compressibleType(h.Get("Content-Type")) {
|
||||
g.compress = true
|
||||
h.Set("Content-Encoding", "gzip")
|
||||
h.Add("Vary", "Accept-Encoding")
|
||||
// The compressed length is not known until the body is written, and
|
||||
// the handler's original Content-Length is now wrong.
|
||||
h.Del("Content-Length")
|
||||
}
|
||||
g.ResponseWriter.WriteHeader(code)
|
||||
}
|
||||
|
||||
// writer lazily creates the gzip stream on the first body byte. A HEAD request
|
||||
// (or a bodyless response) never writes, so no empty gzip stream is emitted and
|
||||
// net/http has nothing from which to fabricate a misleading Content-Length.
|
||||
func (g *gzipResponseWriter) writer() *gzip.Writer {
|
||||
if g.gz == nil {
|
||||
g.gz = gzip.NewWriter(g.ResponseWriter)
|
||||
}
|
||||
return g.gz
|
||||
}
|
||||
|
||||
func (g *gzipResponseWriter) Write(b []byte) (int, error) {
|
||||
if !g.wroteHeader {
|
||||
g.WriteHeader(http.StatusOK)
|
||||
}
|
||||
if g.compress {
|
||||
return g.writer().Write(b)
|
||||
}
|
||||
return g.ResponseWriter.Write(b)
|
||||
}
|
||||
|
||||
// Flush forwards to the underlying http.Flusher, after flushing any buffered
|
||||
// compressed bytes so the client sees a complete gzip block.
|
||||
func (g *gzipResponseWriter) Flush() {
|
||||
if !g.wroteHeader {
|
||||
g.WriteHeader(http.StatusOK)
|
||||
}
|
||||
if g.compress && g.gz != nil {
|
||||
_ = g.gz.Flush()
|
||||
}
|
||||
if f, ok := g.ResponseWriter.(http.Flusher); ok {
|
||||
f.Flush()
|
||||
}
|
||||
}
|
||||
|
||||
// Hijack forwards to the underlying http.Hijacker for protocols that take over
|
||||
// the connection.
|
||||
func (g *gzipResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
|
||||
if hj, ok := g.ResponseWriter.(http.Hijacker); ok {
|
||||
return hj.Hijack()
|
||||
}
|
||||
return nil, nil, http.ErrNotSupported
|
||||
}
|
||||
|
||||
// ReadFrom forwards to the underlying io.ReaderFrom so io.Copy keeps the
|
||||
// sendfile fast path. When compressing, the bytes must pass through the gzip
|
||||
// writer instead; the returned count is the source byte count.
|
||||
func (g *gzipResponseWriter) ReadFrom(src io.Reader) (int64, error) {
|
||||
if !g.wroteHeader {
|
||||
g.WriteHeader(http.StatusOK)
|
||||
}
|
||||
if g.compress {
|
||||
return io.Copy(g.writer(), src)
|
||||
}
|
||||
if rf, ok := g.ResponseWriter.(io.ReaderFrom); ok {
|
||||
return rf.ReadFrom(src)
|
||||
}
|
||||
return io.Copy(g.ResponseWriter, src)
|
||||
}
|
||||
|
||||
// Close flushes the gzip trailer. It must run after the static handler returns,
|
||||
// or the body will be missing its checksum and the client will reject it.
|
||||
func (g *gzipResponseWriter) Close() error {
|
||||
if g.compress && g.gz != nil {
|
||||
return g.gz.Close()
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
335
internal/httpapi/static_test.go
Normal file
335
internal/httpapi/static_test.go
Normal file
|
|
@ -0,0 +1,335 @@
|
|||
package httpapi
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"compress/gzip"
|
||||
"io"
|
||||
"io/fs"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/mithral/vellum/web"
|
||||
)
|
||||
|
||||
// findAsset returns the path (relative to the embedded dist root, no leading
|
||||
// slash) of the first embedded file ending in suffix. Assets are content-hashed
|
||||
// by the build, so tests must discover them rather than hardcode a name.
|
||||
func findAsset(t *testing.T, suffix string) string {
|
||||
t.Helper()
|
||||
sub, err := fs.Sub(web.Dist, "dist")
|
||||
if err != nil {
|
||||
t.Fatalf("fs.Sub: %v", err)
|
||||
}
|
||||
var found string
|
||||
err = fs.WalkDir(sub, ".", func(p string, d fs.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if d.IsDir() {
|
||||
return nil
|
||||
}
|
||||
if found == "" && strings.HasSuffix(p, suffix) {
|
||||
found = p
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("walk dist: %v", err)
|
||||
}
|
||||
if found == "" {
|
||||
t.Fatalf("no embedded asset ending in %q", suffix)
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
func readAsset(t *testing.T, name string) []byte {
|
||||
t.Helper()
|
||||
sub, err := fs.Sub(web.Dist, "dist")
|
||||
if err != nil {
|
||||
t.Fatalf("fs.Sub: %v", err)
|
||||
}
|
||||
b, err := fs.ReadFile(sub, name)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", name, err)
|
||||
}
|
||||
return b
|
||||
}
|
||||
|
||||
func gunzip(t *testing.T, body []byte) []byte {
|
||||
t.Helper()
|
||||
zr, err := gzip.NewReader(bytes.NewReader(body))
|
||||
if err != nil {
|
||||
t.Fatalf("gzip.NewReader: %v", err)
|
||||
}
|
||||
defer zr.Close()
|
||||
out, err := io.ReadAll(zr)
|
||||
if err != nil {
|
||||
t.Fatalf("read gzip body: %v", err)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// TestStaticGzipCompressesJS proves a gzip-capable request for a .js asset is
|
||||
// compressed, carries no stale Content-Length, and round-trips to the original.
|
||||
func TestStaticGzipCompressesJS(t *testing.T) {
|
||||
srv := newTestServer(t)
|
||||
name := findAsset(t, ".js")
|
||||
want := readAsset(t, name)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/"+name, nil)
|
||||
req.Header.Set("Accept-Encoding", "gzip")
|
||||
srv.Handler().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
if got := rec.Header().Get("Content-Encoding"); got != "gzip" {
|
||||
t.Fatalf("Content-Encoding = %q, want gzip", got)
|
||||
}
|
||||
if got := rec.Header().Get("Vary"); !strings.Contains(got, "Accept-Encoding") {
|
||||
t.Errorf("Vary = %q, want it to mention Accept-Encoding", got)
|
||||
}
|
||||
// The handler's Content-Length describes the uncompressed body; it must be
|
||||
// gone once the length changes.
|
||||
if got := rec.Header().Get("Content-Length"); got != "" {
|
||||
t.Errorf("Content-Length = %q, want empty on a compressed response", got)
|
||||
}
|
||||
if got := gunzip(t, rec.Body.Bytes()); !bytes.Equal(got, want) {
|
||||
t.Errorf("decompressed body does not match original %s (%d vs %d bytes)", name, len(got), len(want))
|
||||
}
|
||||
if rec.Body.Len() >= len(want) {
|
||||
t.Errorf("compressed body %d bytes is not smaller than original %d", rec.Body.Len(), len(want))
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaticIdentityWithoutAcceptEncoding proves a request with no
|
||||
// Accept-Encoding is served byte-for-byte, uncompressed.
|
||||
func TestStaticIdentityWithoutAcceptEncoding(t *testing.T) {
|
||||
srv := newTestServer(t)
|
||||
name := findAsset(t, ".js")
|
||||
want := readAsset(t, name)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/"+name, nil)
|
||||
srv.Handler().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
if got := rec.Header().Get("Content-Encoding"); got != "" {
|
||||
t.Fatalf("Content-Encoding = %q, want empty for an identity request", got)
|
||||
}
|
||||
if !bytes.Equal(rec.Body.Bytes(), want) {
|
||||
t.Errorf("identity body does not match original %s", name)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaticRangeNotCompressed proves a Range request stays identity: seeking
|
||||
// depends on the original byte offsets, which gzip would invalidate.
|
||||
func TestStaticRangeNotCompressed(t *testing.T) {
|
||||
srv := newTestServer(t)
|
||||
name := findAsset(t, ".js")
|
||||
want := readAsset(t, name)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/"+name, nil)
|
||||
req.Header.Set("Accept-Encoding", "gzip")
|
||||
req.Header.Set("Range", "bytes=0-99")
|
||||
srv.Handler().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusPartialContent {
|
||||
t.Fatalf("status = %d, want 206", rec.Code)
|
||||
}
|
||||
if got := rec.Header().Get("Content-Encoding"); got != "" {
|
||||
t.Fatalf("Content-Encoding = %q, want empty on a Range response", got)
|
||||
}
|
||||
if !bytes.Equal(rec.Body.Bytes(), want[:100]) {
|
||||
t.Errorf("range body does not match the first 100 bytes of %s", name)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaticPNGNotCompressed proves binary assets are passed through even when
|
||||
// the client accepts gzip; re-compressing them wastes CPU.
|
||||
func TestStaticPNGNotCompressed(t *testing.T) {
|
||||
srv := newTestServer(t)
|
||||
name := findAsset(t, ".png")
|
||||
want := readAsset(t, name)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/"+name, nil)
|
||||
req.Header.Set("Accept-Encoding", "gzip")
|
||||
srv.Handler().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
if got := rec.Header().Get("Content-Type"); !strings.HasPrefix(got, "image/png") {
|
||||
t.Fatalf("Content-Type = %q, want image/png", got)
|
||||
}
|
||||
if got := rec.Header().Get("Content-Encoding"); got != "" {
|
||||
t.Fatalf("Content-Encoding = %q, want empty for a png", got)
|
||||
}
|
||||
if !bytes.Equal(rec.Body.Bytes(), want) {
|
||||
t.Errorf("png body does not match original %s", name)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaticIndexCompressedAndNoCache proves the SPA shell still says no-cache
|
||||
// and is now compressed for gzip clients.
|
||||
func TestStaticIndexCompressedAndNoCache(t *testing.T) {
|
||||
srv := newTestServer(t)
|
||||
want := readAsset(t, "index.html")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set("Accept-Encoding", "gzip")
|
||||
srv.Handler().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
if got := rec.Header().Get("Cache-Control"); got != "no-cache" {
|
||||
t.Errorf("Cache-Control = %q, want no-cache", got)
|
||||
}
|
||||
if got := rec.Header().Get("Content-Encoding"); got != "gzip" {
|
||||
t.Fatalf("Content-Encoding = %q, want gzip", got)
|
||||
}
|
||||
if got := gunzip(t, rec.Body.Bytes()); !bytes.Equal(got, want) {
|
||||
t.Errorf("decompressed index does not match original")
|
||||
}
|
||||
}
|
||||
|
||||
// ── Wrapper interface forwarding ───────────────────────────────────────
|
||||
//
|
||||
// The wrapper sits between the static handler and statusRecorder, so it must
|
||||
// keep the same interfaces statusRecorder forwards.
|
||||
|
||||
func TestGzipResponseWriterForwardsFlush(t *testing.T) {
|
||||
rec := &flushRecorder{ResponseRecorder: httptest.NewRecorder()}
|
||||
req := httptest.NewRequest(http.MethodGet, "/x", nil)
|
||||
req.Header.Set("Accept-Encoding", "gzip")
|
||||
gz := newGzipResponseWriter(rec, req)
|
||||
|
||||
gz.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||
gz.WriteHeader(http.StatusOK)
|
||||
_, _ = gz.Write([]byte("<html>"))
|
||||
gz.Flush()
|
||||
_ = gz.Close()
|
||||
|
||||
if !rec.flushed {
|
||||
t.Error("Flush was not forwarded to the underlying writer")
|
||||
}
|
||||
if got := gunzip(t, rec.Body.Bytes()); string(got) != "<html>" {
|
||||
t.Errorf("flushed body = %q, want <html>", got)
|
||||
}
|
||||
}
|
||||
|
||||
type hijackRecorder struct {
|
||||
*httptest.ResponseRecorder
|
||||
hijacked bool
|
||||
}
|
||||
|
||||
func (h *hijackRecorder) Hijack() (net.Conn, *bufio.ReadWriter, error) {
|
||||
h.hijacked = true
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
func TestGzipResponseWriterForwardsHijack(t *testing.T) {
|
||||
rec := &hijackRecorder{ResponseRecorder: httptest.NewRecorder()}
|
||||
req := httptest.NewRequest(http.MethodGet, "/x", nil)
|
||||
gz := newGzipResponseWriter(rec, req)
|
||||
|
||||
if _, _, err := gz.Hijack(); err != nil {
|
||||
t.Fatalf("Hijack: %v", err)
|
||||
}
|
||||
if !rec.hijacked {
|
||||
t.Error("Hijack was not forwarded to the underlying writer")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGzipResponseWriterForwardsReadFrom(t *testing.T) {
|
||||
rec := &readFromRecorder{ResponseRecorder: httptest.NewRecorder()}
|
||||
req := httptest.NewRequest(http.MethodGet, "/x", nil) // no Accept-Encoding
|
||||
gz := newGzipResponseWriter(rec, req)
|
||||
|
||||
n, err := gz.ReadFrom(strings.NewReader("payload"))
|
||||
if err != nil {
|
||||
t.Fatalf("ReadFrom: %v", err)
|
||||
}
|
||||
if !rec.readFrom {
|
||||
t.Error("ReadFrom was not forwarded to the underlying writer")
|
||||
}
|
||||
if n != int64(len("payload")) || rec.Body.String() != "payload" {
|
||||
t.Errorf("ReadFrom wrote %d bytes %q, want payload", n, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestGzipResponseWriterReadFromCompresses(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/x", nil)
|
||||
req.Header.Set("Accept-Encoding", "gzip")
|
||||
gz := newGzipResponseWriter(rec, req)
|
||||
|
||||
gz.Header().Set("Content-Type", "application/javascript")
|
||||
n, err := gz.ReadFrom(strings.NewReader("payload"))
|
||||
if err != nil {
|
||||
t.Fatalf("ReadFrom: %v", err)
|
||||
}
|
||||
if n != int64(len("payload")) {
|
||||
t.Errorf("ReadFrom returned %d, want %d", n, len("payload"))
|
||||
}
|
||||
_ = gz.Close()
|
||||
|
||||
if got := rec.Header().Get("Content-Encoding"); got != "gzip" {
|
||||
t.Fatalf("Content-Encoding = %q, want gzip", got)
|
||||
}
|
||||
if got := gunzip(t, rec.Body.Bytes()); string(got) != "payload" {
|
||||
t.Errorf("decompressed body = %q, want payload", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCompressibleType and TestAcceptsGzip pin the content-type allow list and
|
||||
// the Accept-Encoding parsing, including the q=0 refusal.
|
||||
func TestCompressibleType(t *testing.T) {
|
||||
compressible := []string{
|
||||
"text/html", "text/html; charset=utf-8",
|
||||
"text/css", "application/javascript",
|
||||
"text/javascript; charset=utf-8",
|
||||
"application/json", "image/svg+xml",
|
||||
"application/manifest+json",
|
||||
}
|
||||
for _, ct := range compressible {
|
||||
if !compressibleType(ct) {
|
||||
t.Errorf("compressibleType(%q) = false, want true", ct)
|
||||
}
|
||||
}
|
||||
skip := []string{
|
||||
"image/png", "image/jpeg", "font/woff2",
|
||||
"application/wasm", "application/octet-stream", "",
|
||||
}
|
||||
for _, ct := range skip {
|
||||
if compressibleType(ct) {
|
||||
t.Errorf("compressibleType(%q) = true, want false", ct)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestAcceptsGzip(t *testing.T) {
|
||||
yes := []string{"gzip", "gzip, deflate", "br, gzip;q=0.5", "GZIP"}
|
||||
for _, h := range yes {
|
||||
if !acceptsGzip(h) {
|
||||
t.Errorf("acceptsGzip(%q) = false, want true", h)
|
||||
}
|
||||
}
|
||||
no := []string{"", "deflate", "br", "gzip;q=0"}
|
||||
for _, h := range no {
|
||||
if acceptsGzip(h) {
|
||||
t.Errorf("acceptsGzip(%q) = true, want false", h)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -85,8 +85,14 @@ type engagement struct {
|
|||
|
||||
// topEngagement finds the kind and author the user most recently engaged with,
|
||||
// so suggestions can be explained in plain language.
|
||||
func topEngagement(ctx context.Context, db *sql.DB, userID int64) (engagement, bool, error) {
|
||||
func topEngagement(ctx context.Context, db *sql.DB, userID int64, libraryIDs []int64) (engagement, bool, error) {
|
||||
var e engagement
|
||||
// Scoped to libraries the user can still read. Without this, progress from a
|
||||
// library whose grant was revoked would keep steering the home screen's
|
||||
// suggestions — confusing on its own, and an information-flow leak about
|
||||
// content the user no longer has access to.
|
||||
in, inArgs := inClause(libraryIDs)
|
||||
args := append([]any{userID}, inArgs...)
|
||||
err := db.QueryRowContext(ctx, `
|
||||
SELECT i.kind, i.title,
|
||||
COALESCE((SELECT p.name FROM item_people ip
|
||||
|
|
@ -96,8 +102,9 @@ func topEngagement(ctx context.Context, db *sql.DB, userID int64) (engagement, b
|
|||
FROM progress p
|
||||
JOIN media_items i ON i.id = p.item_id
|
||||
WHERE p.user_id = ? AND i.missing = 0
|
||||
AND i.library_id IN (`+in+`)
|
||||
ORDER BY p.updated_at DESC
|
||||
LIMIT 1`, userID).Scan(&e.Kind, &e.Title, &e.Author)
|
||||
LIMIT 1`, args...).Scan(&e.Kind, &e.Title, &e.Author)
|
||||
if err == sql.ErrNoRows {
|
||||
return engagement{}, false, nil
|
||||
}
|
||||
|
|
@ -120,7 +127,7 @@ func Suggestions(ctx context.Context, db *sql.DB, userID int64, libraryIDs []int
|
|||
limit = 20
|
||||
}
|
||||
|
||||
e, hasEngagement, err := topEngagement(ctx, db, userID)
|
||||
e, hasEngagement, err := topEngagement(ctx, db, userID, libraryIDs)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
|
|||
70
internal/media/handlers_test.go
Normal file
70
internal/media/handlers_test.go
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
package media
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/mithral/vellum/internal/auth"
|
||||
)
|
||||
|
||||
// getItem drives the per-item detail handler directly. PathValue is normally
|
||||
// filled in by the ServeMux; the test sets it explicitly.
|
||||
func getItem(t *testing.T, h *Handlers, u auth.User, id string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/v1/items/"+id, nil)
|
||||
req.SetPathValue("id", id)
|
||||
rec := httptest.NewRecorder()
|
||||
h.handleItem(rec, req, u)
|
||||
return rec
|
||||
}
|
||||
|
||||
// TestHandleItemScoping pins the per-item access chokepoint: an item in a
|
||||
// library the caller cannot read must be indistinguishable from a missing one.
|
||||
func TestHandleItemScoping(t *testing.T) {
|
||||
f := setupHome(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
|
||||
// userA may read libA, so a1 resolves.
|
||||
rec := getItem(t, h, auth.User{ID: f.userA}, itoa(f.a1))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("accessible item status = %d, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var body struct {
|
||||
Item struct {
|
||||
ID int64 `json:"id"`
|
||||
LibraryID int64 `json:"library_id"`
|
||||
Kind string `json:"kind"`
|
||||
} `json:"item"`
|
||||
Authors []string `json:"authors"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("decode item: %v", err)
|
||||
}
|
||||
if body.Item.ID != f.a1 || body.Item.LibraryID != f.libA || body.Item.Kind != "ebooks" {
|
||||
t.Errorf("item = %+v, want id %d in library %d", body.Item, f.a1, f.libA)
|
||||
}
|
||||
if len(body.Authors) != 1 || body.Authors[0] != "Ada Author" {
|
||||
t.Errorf("authors = %v, want [Ada Author]", body.Authors)
|
||||
}
|
||||
|
||||
// b1 is in libB, which userA cannot read: 404, not 403, so existence is
|
||||
// not disclosed.
|
||||
rec = getItem(t, h, auth.User{ID: f.userA}, itoa(f.b1))
|
||||
if rec.Code != http.StatusNotFound {
|
||||
t.Errorf("inaccessible item status = %d, want 404", rec.Code)
|
||||
}
|
||||
|
||||
// A user with no grants gets the same 404.
|
||||
rec = getItem(t, h, auth.User{ID: f.userNone}, itoa(f.a1))
|
||||
if rec.Code != http.StatusNotFound {
|
||||
t.Errorf("no-grant item status = %d, want 404", rec.Code)
|
||||
}
|
||||
|
||||
// A non-numeric id is a 400, not a panic.
|
||||
rec = getItem(t, h, auth.User{ID: f.userA}, "abc")
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Errorf("invalid id status = %d, want 400", rec.Code)
|
||||
}
|
||||
}
|
||||
728
internal/media/home_test.go
Normal file
728
internal/media/home_test.go
Normal file
|
|
@ -0,0 +1,728 @@
|
|||
package media
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/mithral/vellum/internal/auth"
|
||||
)
|
||||
|
||||
// ── shared helpers ─────────────────────────────────────────────────────
|
||||
|
||||
func mediaHandlers(t *testing.T, db *sql.DB) *Handlers {
|
||||
t.Helper()
|
||||
log := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
return NewHandlers(db, log, nil, t.TempDir())
|
||||
}
|
||||
|
||||
func mustQueryID(t *testing.T, db *sql.DB, q string, args ...any) int64 {
|
||||
t.Helper()
|
||||
var id int64
|
||||
if err := db.QueryRowContext(context.Background(), q, args...).Scan(&id); err != nil {
|
||||
t.Fatalf("query %q: %v", q, err)
|
||||
}
|
||||
return id
|
||||
}
|
||||
|
||||
func mustExec(t *testing.T, db *sql.DB, q string, args ...any) {
|
||||
t.Helper()
|
||||
if _, err := db.ExecContext(context.Background(), q, args...); err != nil {
|
||||
t.Fatalf("exec %q: %v", q, err)
|
||||
}
|
||||
}
|
||||
|
||||
func newLibrary(t *testing.T, db *sql.DB, name, kind, root string) int64 {
|
||||
t.Helper()
|
||||
return mustQueryID(t, db,
|
||||
`INSERT INTO libraries (name, kind, root_path) VALUES (?, ?, ?) RETURNING id`, name, kind, root)
|
||||
}
|
||||
|
||||
func newUser(t *testing.T, db *sql.DB, name string) int64 {
|
||||
t.Helper()
|
||||
return mustQueryID(t, db,
|
||||
`INSERT INTO users (username, password_hash) VALUES (?, 'x') RETURNING id`, name)
|
||||
}
|
||||
|
||||
func grantRead(t *testing.T, db *sql.DB, userID, libID int64) {
|
||||
t.Helper()
|
||||
mustExec(t, db,
|
||||
`INSERT INTO library_grants (user_id, library_id, can_read, can_download) VALUES (?, ?, 1, 1)`,
|
||||
userID, libID)
|
||||
}
|
||||
|
||||
// newItem inserts a media item plus a backing file, because progress rows carry
|
||||
// a NOT NULL file_id foreign key.
|
||||
func newItem(t *testing.T, db *sql.DB, libID int64, kind, path, title, sortTitle string, missing bool, addedAt string) int64 {
|
||||
t.Helper()
|
||||
m := 0
|
||||
if missing {
|
||||
m = 1
|
||||
}
|
||||
id := mustQueryID(t, db, `
|
||||
INSERT INTO media_items (library_id, kind, path, title, sort_title, missing, added_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?) RETURNING id`,
|
||||
libID, kind, path, title, sortTitle, m, addedAt)
|
||||
mustExec(t, db,
|
||||
`INSERT INTO media_files (item_id, path, size) VALUES (?, ?, 1)`,
|
||||
id, "file-"+strconv.FormatInt(id, 10))
|
||||
return id
|
||||
}
|
||||
|
||||
func fileIDFor(t *testing.T, db *sql.DB, itemID int64) int64 {
|
||||
t.Helper()
|
||||
var id int64
|
||||
if err := db.QueryRowContext(context.Background(),
|
||||
`SELECT id FROM media_files WHERE item_id = ? ORDER BY id LIMIT 1`, itemID).Scan(&id); err != nil {
|
||||
t.Fatalf("file for item %d: %v", itemID, err)
|
||||
}
|
||||
return id
|
||||
}
|
||||
|
||||
func newProgress(t *testing.T, db *sql.DB, userID, itemID int64, percent float64, finished bool, updatedAt string) {
|
||||
t.Helper()
|
||||
f := 0
|
||||
if finished {
|
||||
f = 1
|
||||
}
|
||||
mustExec(t, db, `
|
||||
INSERT INTO progress (user_id, item_id, file_id, kind, position, percent, finished, updated_at)
|
||||
VALUES (?, ?, ?, 'ebooks', 1, ?, ?, ?)`,
|
||||
userID, itemID, fileIDFor(t, db, itemID), percent, f, updatedAt)
|
||||
}
|
||||
|
||||
func newView(t *testing.T, db *sql.DB, userID, itemID int64, views int, lastViewedAt string) {
|
||||
t.Helper()
|
||||
mustExec(t, db,
|
||||
`INSERT INTO item_views (user_id, item_id, views, last_viewed_at) VALUES (?, ?, ?, ?)`,
|
||||
userID, itemID, views, lastViewedAt)
|
||||
}
|
||||
|
||||
func idSet(items []shelfItem) map[int64]bool {
|
||||
out := make(map[int64]bool, len(items))
|
||||
for _, it := range items {
|
||||
out[it.ID] = true
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func hasDuplicates(items []shelfItem) bool {
|
||||
seen := map[int64]bool{}
|
||||
for _, it := range items {
|
||||
if seen[it.ID] {
|
||||
return true
|
||||
}
|
||||
seen[it.ID] = true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// ── handleHome ─────────────────────────────────────────────────────────
|
||||
|
||||
type homeFixture struct {
|
||||
db *sql.DB
|
||||
libA int64 // ebooks/comics, visible to userA
|
||||
libB int64 // audiobooks, visible to userB
|
||||
userA int64
|
||||
userB int64
|
||||
userNone int64
|
||||
a1 int64 // in progress (unfinished, most recent)
|
||||
a2 int64 // finished
|
||||
a3 int64 // unstarted, viewed, same author as a1
|
||||
a4 int64 // missing, in progress
|
||||
a5 int64 // unstarted
|
||||
b1 int64 // userB's only item
|
||||
}
|
||||
|
||||
func setupHome(t *testing.T) homeFixture {
|
||||
t.Helper()
|
||||
db := testDB(t)
|
||||
f := homeFixture{db: db}
|
||||
|
||||
f.libA = newLibrary(t, db, "Ebooks", "ebooks", "/media/ebooks")
|
||||
f.libB = newLibrary(t, db, "Audio", "audiobooks", "/media/audio")
|
||||
|
||||
f.userA = newUser(t, db, "alice")
|
||||
f.userB = newUser(t, db, "bob")
|
||||
f.userNone = newUser(t, db, "carol")
|
||||
|
||||
grantRead(t, db, f.userA, f.libA)
|
||||
grantRead(t, db, f.userB, f.libB)
|
||||
|
||||
f.a1 = newItem(t, db, f.libA, "ebooks", "a1.epub", "A One", "a one", false, "2026-01-01T00:00:01.000Z")
|
||||
f.a2 = newItem(t, db, f.libA, "ebooks", "a2.epub", "A Two", "a two", false, "2026-01-01T00:00:02.000Z")
|
||||
f.a3 = newItem(t, db, f.libA, "comics", "a3.cbz", "A Three", "a three", false, "2026-01-01T00:00:03.000Z")
|
||||
f.a4 = newItem(t, db, f.libA, "ebooks", "a4.epub", "A Four", "a four", true, "2026-01-01T00:00:04.000Z")
|
||||
f.a5 = newItem(t, db, f.libA, "ebooks", "a5.epub", "A Five", "a five", false, "2026-01-01T00:00:05.000Z")
|
||||
f.b1 = newItem(t, db, f.libB, "audiobooks", "b1.mp3", "B One", "b one", false, "2026-01-01T00:00:06.000Z")
|
||||
|
||||
// userA's progress. a1 is the most recent non-missing row, so it drives
|
||||
// "top engagement". a2 is finished (must leave Continue). a4 is missing
|
||||
// (must leave Continue even though unfinished).
|
||||
newProgress(t, db, f.userA, f.a1, 0.5, false, "2026-02-01T00:00:03.000Z")
|
||||
newProgress(t, db, f.userA, f.a2, 1.0, true, "2026-02-01T00:00:02.000Z")
|
||||
newProgress(t, db, f.userA, f.a4, 0.3, false, "2026-02-01T00:00:04.000Z")
|
||||
|
||||
// userB only ever touches their own library.
|
||||
newProgress(t, db, f.userB, f.b1, 0.2, false, "2026-02-02T00:00:01.000Z")
|
||||
|
||||
newView(t, db, f.userA, f.a1, 3, "2026-02-03T00:00:01.000Z")
|
||||
newView(t, db, f.userA, f.a3, 1, "2026-02-03T00:00:02.000Z")
|
||||
newView(t, db, f.userB, f.b1, 9, "2026-02-03T00:00:03.000Z")
|
||||
|
||||
// Ada Author wrote a1 (in progress) and a3 (unstarted) so Suggestions has
|
||||
// a same-author candidate.
|
||||
ada := mustQueryID(t, db, `INSERT INTO people (name) VALUES ('Ada Author') RETURNING id`)
|
||||
mustExec(t, db, `INSERT INTO item_people (item_id, person_id, role, ord) VALUES (?, ?, 'author', 0)`, f.a1, ada)
|
||||
mustExec(t, db, `INSERT INTO item_people (item_id, person_id, role, ord) VALUES (?, ?, 'author', 0)`, f.a3, ada)
|
||||
|
||||
return f
|
||||
}
|
||||
|
||||
func getHome(t *testing.T, h *Handlers, u auth.User) (homeResponse, []byte) {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/v1/home", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
h.handleHome(rec, req, u)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("home status = %d, body = %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var resp homeResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode home: %v (body %s)", err, rec.Body.String())
|
||||
}
|
||||
return resp, rec.Body.Bytes()
|
||||
}
|
||||
|
||||
func TestHandleHomeShelvesAndCounts(t *testing.T) {
|
||||
f := setupHome(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
resp, body := getHome(t, h, auth.User{ID: f.userA})
|
||||
|
||||
// The landing screen contract: all five shelves plus the sidebar counts.
|
||||
var raw map[string]json.RawMessage
|
||||
if err := json.Unmarshal(body, &raw); err != nil {
|
||||
t.Fatalf("decode raw home: %v", err)
|
||||
}
|
||||
for _, key := range []string{"continue", "most_viewed", "suggestions", "recent", "explore", "counts_by_kind", "library_count", "total_items"} {
|
||||
if _, ok := raw[key]; !ok {
|
||||
t.Errorf("home response is missing %q", key)
|
||||
}
|
||||
}
|
||||
if resp.Continue == nil || resp.MostViewed == nil || resp.Suggestions == nil || resp.Recent == nil || resp.Explore == nil {
|
||||
t.Fatalf("a shelf is nil: continue=%v most=%v sugg=%v recent=%v explore=%v",
|
||||
resp.Continue == nil, resp.MostViewed == nil, resp.Suggestions == nil, resp.Recent == nil, resp.Explore == nil)
|
||||
}
|
||||
|
||||
// Continue: only a1. a2 finished, a4 missing, b1 belongs to another user.
|
||||
if got := ids(resp.Continue); len(got) != 1 || got[0] != f.a1 {
|
||||
t.Errorf("continue = %v, want [%d]", got, f.a1)
|
||||
}
|
||||
for _, s := range resp.Continue {
|
||||
if s.Percent <= 0 {
|
||||
t.Errorf("continue item %d has percent %v", s.ID, s.Percent)
|
||||
}
|
||||
}
|
||||
|
||||
// Most viewed: a1 (3 views) then a3 (1). b1 is in another library.
|
||||
if got := ids(resp.MostViewed); len(got) != 2 || got[0] != f.a1 || got[1] != f.a3 {
|
||||
t.Errorf("most_viewed = %v, want [%d %d]", got, f.a1, f.a3)
|
||||
}
|
||||
|
||||
// Counts exclude the missing item and the invisible library.
|
||||
if resp.CountsByKind["ebooks"] != 3 || resp.CountsByKind["comics"] != 1 {
|
||||
t.Errorf("counts_by_kind = %v, want ebooks:3 comics:1", resp.CountsByKind)
|
||||
}
|
||||
if _, ok := resp.CountsByKind["audiobooks"]; ok {
|
||||
t.Errorf("counts_by_kind leaked another library: %v", resp.CountsByKind)
|
||||
}
|
||||
if resp.TotalItems != 4 {
|
||||
t.Errorf("total_items = %d, want 4", resp.TotalItems)
|
||||
}
|
||||
if resp.LibraryCount != 1 {
|
||||
t.Errorf("library_count = %d, want 1", resp.LibraryCount)
|
||||
}
|
||||
|
||||
// Recent excludes the missing item and the other library, newest first.
|
||||
recent := ids(resp.Recent)
|
||||
if len(recent) != 4 {
|
||||
t.Errorf("recent = %v, want 4 libA items", recent)
|
||||
}
|
||||
for _, id := range recent {
|
||||
if id == f.a4 || id == f.b1 {
|
||||
t.Errorf("recent included excluded item %d", id)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleHomeGrantScoping(t *testing.T) {
|
||||
f := setupHome(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
|
||||
// userA holds a grant to libA only. Nothing from libB may surface anywhere.
|
||||
resp, _ := getHome(t, h, auth.User{ID: f.userA})
|
||||
for name, shelf := range map[string][]shelfItem{
|
||||
"continue": resp.Continue, "most_viewed": resp.MostViewed,
|
||||
"recent": resp.Recent, "explore": resp.Explore,
|
||||
} {
|
||||
for _, s := range shelf {
|
||||
if s.LibraryID != f.libA {
|
||||
t.Errorf("%s item %d leaked from library %d, want only %d", name, s.ID, s.LibraryID, f.libA)
|
||||
}
|
||||
}
|
||||
}
|
||||
for _, s := range resp.Suggestions {
|
||||
if s.LibraryID != f.libA {
|
||||
t.Errorf("suggestion %d leaked from library %d", s.ID, s.LibraryID)
|
||||
}
|
||||
}
|
||||
|
||||
// A user with no grants gets empty shelves, not an error and not content.
|
||||
none, _ := getHome(t, h, auth.User{ID: f.userNone})
|
||||
if none.LibraryCount != 0 || none.TotalItems != 0 || len(none.CountsByKind) != 0 {
|
||||
t.Errorf("no-grant user got library_count=%d total=%d counts=%v",
|
||||
none.LibraryCount, none.TotalItems, none.CountsByKind)
|
||||
}
|
||||
for name, n := range map[string]int{
|
||||
"continue": len(none.Continue), "most_viewed": len(none.MostViewed),
|
||||
"suggestions": len(none.Suggestions), "recent": len(none.Recent), "explore": len(none.Explore),
|
||||
} {
|
||||
if n != 0 {
|
||||
t.Errorf("no-grant user's %s shelf has %d items", name, n)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleHomeIsolatesUsers(t *testing.T) {
|
||||
f := setupHome(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
resp, _ := getHome(t, h, auth.User{ID: f.userB})
|
||||
|
||||
if got := ids(resp.Continue); len(got) != 1 || got[0] != f.b1 {
|
||||
t.Errorf("user B continue = %v, want [%d]", got, f.b1)
|
||||
}
|
||||
if got := ids(resp.MostViewed); len(got) != 1 || got[0] != f.b1 {
|
||||
t.Errorf("user B most_viewed = %v, want [%d]", got, f.b1)
|
||||
}
|
||||
if resp.CountsByKind["audiobooks"] != 1 || resp.TotalItems != 1 || resp.LibraryCount != 1 {
|
||||
t.Errorf("user B counts = %v total=%d library_count=%d, want audiobooks:1 total:1 libs:1",
|
||||
resp.CountsByKind, resp.TotalItems, resp.LibraryCount)
|
||||
}
|
||||
for name, shelf := range map[string][]shelfItem{
|
||||
"continue": resp.Continue, "most_viewed": resp.MostViewed,
|
||||
"recent": resp.Recent, "explore": resp.Explore,
|
||||
} {
|
||||
for _, s := range shelf {
|
||||
if s.LibraryID != f.libB {
|
||||
t.Errorf("user B's %s saw library %d", name, s.LibraryID)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Explore ────────────────────────────────────────────────────────────
|
||||
|
||||
func TestExploreOnlyVisibleAndPresent(t *testing.T) {
|
||||
db, libA, libB, userA, userB := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
var libAIDs []int64
|
||||
for i := 0; i < 6; i++ {
|
||||
libAIDs = append(libAIDs, newItem(t, db, libA, "ebooks",
|
||||
fmt.Sprintf("a%d.epub", i), fmt.Sprintf("A%d", i), fmt.Sprintf("a%02d", i),
|
||||
false, "2026-01-01T00:00:00.000Z"))
|
||||
}
|
||||
missing := map[int64]bool{}
|
||||
for i := 0; i < 3; i++ {
|
||||
missing[newItem(t, db, libA, "ebooks",
|
||||
fmt.Sprintf("m%d.epub", i), fmt.Sprintf("M%d", i), fmt.Sprintf("m%02d", i),
|
||||
true, "2026-01-01T00:00:00.000Z")] = true
|
||||
}
|
||||
for i := 0; i < 6; i++ {
|
||||
newItem(t, db, libB, "audiobooks",
|
||||
fmt.Sprintf("b%d.mp3", i), fmt.Sprintf("B%d", i), fmt.Sprintf("b%02d", i),
|
||||
false, "2026-01-01T00:00:00.000Z")
|
||||
}
|
||||
|
||||
got, err := Explore(ctx, db, userA, []int64{libA}, 30)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(got) != len(libAIDs) {
|
||||
t.Fatalf("explore returned %d, want all %d present libA items", len(got), len(libAIDs))
|
||||
}
|
||||
seen := idSet(got)
|
||||
for _, id := range libAIDs {
|
||||
if !seen[id] {
|
||||
t.Errorf("explore omitted present item %d", id)
|
||||
}
|
||||
}
|
||||
for _, s := range got {
|
||||
if s.LibraryID != libA {
|
||||
t.Errorf("explore item %d came from library %d, want %d", s.ID, s.LibraryID, libA)
|
||||
}
|
||||
if missing[s.ID] {
|
||||
t.Errorf("explore returned missing item %d", s.ID)
|
||||
}
|
||||
}
|
||||
|
||||
// userB must never see libA content.
|
||||
gotB, err := Explore(ctx, db, userB, []int64{libB}, 30)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, s := range gotB {
|
||||
if s.LibraryID != libB {
|
||||
t.Errorf("user B explore item %d came from library %d", s.ID, s.LibraryID)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestExploreExcludesContinueShelf guards the whole point of the shelf: it is
|
||||
// discovery, so anything the user already has in progress must not reappear.
|
||||
func TestExploreExcludesContinueShelf(t *testing.T) {
|
||||
db, libA, _, userA, _ := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
var all []int64
|
||||
for i := 0; i < 20; i++ {
|
||||
all = append(all, newItem(t, db, libA, "ebooks",
|
||||
fmt.Sprintf("a%d.epub", i), fmt.Sprintf("A%d", i), fmt.Sprintf("a%02d", i),
|
||||
false, "2026-01-01T00:00:00.000Z"))
|
||||
}
|
||||
// Three of them are on the Continue shelf.
|
||||
inProgress := map[int64]bool{all[0]: true, all[7]: true, all[19]: true}
|
||||
for id := range inProgress {
|
||||
newProgress(t, db, userA, id, 0.4, false, "2026-02-01T00:00:00.000Z")
|
||||
}
|
||||
|
||||
// Repeated calls, because Explore samples a random window per kind.
|
||||
for i := 0; i < 50; i++ {
|
||||
got, err := Explore(ctx, db, userA, []int64{libA}, 10)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(got) == 0 {
|
||||
t.Fatal("explore returned nothing")
|
||||
}
|
||||
for _, s := range got {
|
||||
if inProgress[s.ID] {
|
||||
t.Fatalf("call %d returned in-progress item %d", i, s.ID)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestExploreRepresentsEveryKind checks the fair-share split: when the limit is
|
||||
// large enough, every kind present in the visible libraries must appear.
|
||||
func TestExploreRepresentsEveryKind(t *testing.T) {
|
||||
db, libA, _, userA, _ := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
kinds := []string{"ebooks", "comics", "audiobooks"}
|
||||
for _, kind := range kinds {
|
||||
for i := 0; i < 12; i++ {
|
||||
newItem(t, db, libA, kind,
|
||||
fmt.Sprintf("%s-%d.dat", kind, i), fmt.Sprintf("%s %d", kind, i),
|
||||
fmt.Sprintf("%s-%02d", kind, i), false, "2026-01-01T00:00:00.000Z")
|
||||
}
|
||||
}
|
||||
|
||||
got, err := Explore(ctx, db, userA, []int64{libA}, 30)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(got) != 30 {
|
||||
t.Fatalf("explore returned %d items, want the full limit 30", len(got))
|
||||
}
|
||||
if hasDuplicates(got) {
|
||||
t.Errorf("explore returned duplicate items: %v", ids(got))
|
||||
}
|
||||
present := map[string]bool{}
|
||||
for _, s := range got {
|
||||
present[s.Kind] = true
|
||||
}
|
||||
for _, kind := range kinds {
|
||||
if !present[kind] {
|
||||
t.Errorf("kind %q missing from explore shelf %v", kind, ids(got))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestExploreNoDuplicates exercises the wrap-around path: the result set must
|
||||
// never repeat an item even though a kind may be filled from both halves of the
|
||||
// id range.
|
||||
func TestExploreNoDuplicates(t *testing.T) {
|
||||
db, libA, _, userA, _ := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
for i := 0; i < 40; i++ {
|
||||
newItem(t, db, libA, "ebooks",
|
||||
fmt.Sprintf("a%d.epub", i), fmt.Sprintf("A%d", i), fmt.Sprintf("a%02d", i),
|
||||
false, "2026-01-01T00:00:00.000Z")
|
||||
}
|
||||
for i := 0; i < 100; i++ {
|
||||
got, err := Explore(ctx, db, userA, []int64{libA}, 15)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if hasDuplicates(got) {
|
||||
t.Fatalf("call %d returned duplicates: %v", i, ids(got))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestExploreVariesAcrossCalls pins the fix for the old deterministic ordering:
|
||||
// with far more items than the limit, repeated visits must not return the same
|
||||
// slice every time. It asserts variety (an invariant of the sampler) rather
|
||||
// than a particular order, so it is not sensitive to the random seed.
|
||||
func TestExploreVariesAcrossCalls(t *testing.T) {
|
||||
db, libA, _, userA, _ := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
for i := 0; i < 120; i++ {
|
||||
newItem(t, db, libA, "ebooks",
|
||||
fmt.Sprintf("a%d.epub", i), fmt.Sprintf("A%d", i), fmt.Sprintf("a%03d", i),
|
||||
false, "2026-01-01T00:00:00.000Z")
|
||||
}
|
||||
|
||||
variants := map[string]bool{}
|
||||
for i := 0; i < 30; i++ {
|
||||
got, err := Explore(ctx, db, userA, []int64{libA}, 30)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
variants[signature(got)] = true
|
||||
}
|
||||
if len(variants) < 2 {
|
||||
t.Fatalf("explore returned an identical shelf on all 30 calls (%d items each); the sampler is deterministic", len(variants))
|
||||
}
|
||||
}
|
||||
|
||||
// TestExploreCompleteWhenLimitCoversEverything checks the short-result /
|
||||
// wrap-around path deterministically: regardless of where the random start
|
||||
// lands, a limit larger than the library must yield the whole eligible set.
|
||||
func TestExploreCompleteWhenLimitCoversEverything(t *testing.T) {
|
||||
db, libA, _, userA, _ := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
var want []int64
|
||||
for _, kind := range []string{"ebooks", "comics"} {
|
||||
for i := 0; i < 4; i++ {
|
||||
want = append(want, newItem(t, db, libA, kind,
|
||||
fmt.Sprintf("%s-%d.dat", kind, i), fmt.Sprintf("%s %d", kind, i),
|
||||
fmt.Sprintf("%s-%02d", kind, i), false, "2026-01-01T00:00:00.000Z"))
|
||||
}
|
||||
}
|
||||
|
||||
got, err := Explore(ctx, db, userA, []int64{libA}, 100)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
seen := idSet(got)
|
||||
for _, id := range want {
|
||||
if !seen[id] {
|
||||
t.Errorf("limit above library size omitted item %d", id)
|
||||
}
|
||||
}
|
||||
if len(got) != len(want) {
|
||||
t.Errorf("explore returned %d items, want exactly %d", len(got), len(want))
|
||||
}
|
||||
}
|
||||
|
||||
// signature renders an item slice as a stable string for set comparison.
|
||||
func signature(items []shelfItem) string {
|
||||
ids := ids(items)
|
||||
sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] })
|
||||
parts := make([]string, len(ids))
|
||||
for i, id := range ids {
|
||||
parts[i] = strconv.FormatInt(id, 10)
|
||||
}
|
||||
return strings.Join(parts, ",")
|
||||
}
|
||||
|
||||
// ── RecordView / MostViewed ────────────────────────────────────────────
|
||||
|
||||
func TestMostViewedOrdersByViewsThenRecency(t *testing.T) {
|
||||
db, libA, _, userA, _ := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
older := newItem(t, db, libA, "ebooks", "older.epub", "Older", "older", false, "2026-01-01T00:00:00.000Z")
|
||||
newer := newItem(t, db, libA, "ebooks", "newer.epub", "Newer", "newer", false, "2026-01-01T00:00:00.000Z")
|
||||
most := newItem(t, db, libA, "ebooks", "most.epub", "Most", "most", false, "2026-01-01T00:00:00.000Z")
|
||||
|
||||
// Equal view counts, different recency: the newer one must win.
|
||||
newView(t, db, userA, older, 5, "2026-03-01T00:00:00.000Z")
|
||||
newView(t, db, userA, newer, 5, "2026-03-02T00:00:00.000Z")
|
||||
// More views always outranks recency.
|
||||
newView(t, db, userA, most, 9, "2026-01-01T00:00:00.000Z")
|
||||
|
||||
got, err := MostViewed(ctx, db, userA, []int64{libA}, 10)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
want := []int64{most, newer, older}
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("most viewed = %v, want %v", ids(got), want)
|
||||
}
|
||||
for i := range want {
|
||||
if got[i].ID != want[i] {
|
||||
t.Errorf("most viewed[%d] = %d, want %d (full order %v)", i, got[i].ID, want[i], ids(got))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestMostViewedExcludesMissing(t *testing.T) {
|
||||
db, libA, _, userA, _ := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
present := newItem(t, db, libA, "ebooks", "p.epub", "P", "p", false, "2026-01-01T00:00:00.000Z")
|
||||
gone := newItem(t, db, libA, "ebooks", "g.epub", "G", "g", true, "2026-01-01T00:00:00.000Z")
|
||||
|
||||
newView(t, db, userA, present, 1, "2026-03-01T00:00:00.000Z")
|
||||
newView(t, db, userA, gone, 50, "2026-03-02T00:00:00.000Z")
|
||||
|
||||
got, err := MostViewed(ctx, db, userA, []int64{libA}, 10)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(got) != 1 || got[0].ID != present {
|
||||
t.Errorf("most viewed = %v, want only the present item %d", ids(got), present)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRecordViewIsPerUser guards the (user_id, item_id) key: one user's habit
|
||||
// must not move another user's count.
|
||||
func TestRecordViewIsPerUser(t *testing.T) {
|
||||
db, libA, _, userA, userB := setupExplore(t)
|
||||
ctx := context.Background()
|
||||
item := newItem(t, db, libA, "ebooks", "x.epub", "X", "x", false, "2026-01-01T00:00:00.000Z")
|
||||
// userB is not granted libA, but RecordView itself is keyed by user id.
|
||||
grantRead(t, db, userB, libA)
|
||||
|
||||
for i := 1; i <= 3; i++ {
|
||||
n, err := RecordView(ctx, db, userA, item)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if n != i {
|
||||
t.Errorf("user A view %d returned %d", i, n)
|
||||
}
|
||||
}
|
||||
n, err := RecordView(ctx, db, userB, item)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if n != 1 {
|
||||
t.Errorf("user B's first view returned %d, want 1 (counts are per user)", n)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Suggestions ────────────────────────────────────────────────────────
|
||||
|
||||
func TestSuggestionsReasonsAndUniqueness(t *testing.T) {
|
||||
f := setupHome(t)
|
||||
ctx := context.Background()
|
||||
|
||||
sugg, err := Suggestions(ctx, f.db, f.userA, []int64{f.libA}, 20)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(sugg) == 0 {
|
||||
t.Fatal("no suggestions")
|
||||
}
|
||||
seen := map[int64]bool{}
|
||||
for _, s := range sugg {
|
||||
if s.Reason == "" {
|
||||
t.Errorf("suggestion %d has an empty reason", s.ID)
|
||||
}
|
||||
if seen[s.ID] {
|
||||
t.Errorf("suggestion %d returned twice", s.ID)
|
||||
}
|
||||
seen[s.ID] = true
|
||||
if s.LibraryID != f.libA {
|
||||
t.Errorf("suggestion %d leaked from library %d", s.ID, s.LibraryID)
|
||||
}
|
||||
}
|
||||
// a1 is in progress, so it must not be suggested; a3 (same author) and a5
|
||||
// (same kind) are the unstarted candidates.
|
||||
if seen[f.a1] {
|
||||
t.Error("an in-progress item was suggested")
|
||||
}
|
||||
if !seen[f.a3] || !seen[f.a5] {
|
||||
t.Errorf("expected same-author a3 and same-kind a5, got %v", idsOfSuggestions(sugg))
|
||||
}
|
||||
}
|
||||
|
||||
func idsOfSuggestions(s []suggestion) []int64 {
|
||||
out := make([]int64, len(s))
|
||||
for i := range s {
|
||||
out[i] = s[i].ID
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// TestSuggestionsSeedIsNotScopedToVisibleLibraries documents a hardening gap,
|
||||
// NOT desired behaviour.
|
||||
//
|
||||
// topEngagement() picks the user's most recent progress row across every
|
||||
// library, including ones the user can no longer read (e.g. a revoked grant).
|
||||
// The items Suggestions returns are still correctly scoped to the visible
|
||||
// libraries, so nothing hidden is ever listed — but the inaccessible row's
|
||||
// kind/author still shapes the shelf. Here a hidden library's ebooks progress
|
||||
// makes the visible ebooks item come back as "Recently added" rather than being
|
||||
// mislabelled "More books" from a library the user cannot read.
|
||||
//
|
||||
// The seed (topEngagement) is now scoped to visible libraries. Before that, a
|
||||
// revoked library's most recent progress row still steered the home screen:
|
||||
// the item below was labelled "More books" — a suggestion reason derived from
|
||||
// content the user no longer had access to.
|
||||
func TestSuggestionsSeedIsScopedToVisibleLibraries(t *testing.T) {
|
||||
db := testDB(t)
|
||||
ctx := context.Background()
|
||||
|
||||
visible := newLibrary(t, db, "Seed Visible", "ebooks", "/seed/v")
|
||||
hidden := newLibrary(t, db, "Seed Hidden", "ebooks", "/seed/h")
|
||||
u := newUser(t, db, "seed-user")
|
||||
grantRead(t, db, u, visible)
|
||||
|
||||
newItem(t, db, visible, "ebooks", "v1.epub", "V1", "v1", false, "2026-01-01T00:00:00.000Z")
|
||||
hid := newItem(t, db, hidden, "ebooks", "h1.epub", "H1", "h1", false, "2026-01-01T00:00:00.000Z")
|
||||
newProgress(t, db, u, hid, 0.5, false, "2026-02-01T00:00:00.000Z")
|
||||
|
||||
sugg, err := Suggestions(ctx, db, u, []int64{visible}, 20)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(sugg) != 1 {
|
||||
t.Fatalf("suggestions = %d, want 1", len(sugg))
|
||||
}
|
||||
if sugg[0].LibraryID != visible {
|
||||
t.Errorf("suggestion leaked library %d", sugg[0].LibraryID)
|
||||
}
|
||||
if sugg[0].Reason != "Recently added" {
|
||||
t.Errorf("reason = %q; want \"Recently added\" — a revoked library's progress must not label the suggestion", sugg[0].Reason)
|
||||
}
|
||||
}
|
||||
|
||||
// setupExplore builds two libraries with one user each and returns them.
|
||||
func setupExplore(t *testing.T) (db *sql.DB, libA, libB, userA, userB int64) {
|
||||
t.Helper()
|
||||
db = testDB(t)
|
||||
libA = newLibrary(t, db, "Explore A", "ebooks", "/explore/a")
|
||||
libB = newLibrary(t, db, "Explore B", "audiobooks", "/explore/b")
|
||||
userA = newUser(t, db, "explore-a")
|
||||
userB = newUser(t, db, "explore-b")
|
||||
grantRead(t, db, userA, libA)
|
||||
grantRead(t, db, userB, libB)
|
||||
return db, libA, libB, userA, userB
|
||||
}
|
||||
250
internal/media/items_test.go
Normal file
250
internal/media/items_test.go
Normal file
|
|
@ -0,0 +1,250 @@
|
|||
package media
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/mithral/vellum/internal/auth"
|
||||
)
|
||||
|
||||
// listResponse mirrors the JSON contract of handleListItems.
|
||||
type listResponse struct {
|
||||
Items []itemRow `json:"items"`
|
||||
Total int `json:"total"`
|
||||
Offset int `json:"offset"`
|
||||
Limit int `json:"limit"`
|
||||
}
|
||||
|
||||
func listItems(t *testing.T, h *Handlers, u auth.User, query string) (int, listResponse) {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/v1/items"+query, nil)
|
||||
rec := httptest.NewRecorder()
|
||||
h.handleListItems(rec, req, u)
|
||||
var resp listResponse
|
||||
if rec.Code == http.StatusOK {
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode list: %v (body %s)", err, rec.Body.String())
|
||||
}
|
||||
}
|
||||
return rec.Code, resp
|
||||
}
|
||||
|
||||
type listFixture struct {
|
||||
db *sql.DB
|
||||
libA int64
|
||||
libB int64
|
||||
userA int64
|
||||
userNone int64
|
||||
e1 int64 // libA ebooks, sort 01
|
||||
e2 int64 // libA ebooks, sort 02
|
||||
e3 int64 // libA ebooks, sort 03
|
||||
c1 int64 // libA comics, sort 04
|
||||
m1 int64 // libA ebooks, missing
|
||||
b1 int64 // libB audiobooks
|
||||
}
|
||||
|
||||
func setupList(t *testing.T) listFixture {
|
||||
t.Helper()
|
||||
db := testDB(t)
|
||||
f := listFixture{db: db}
|
||||
|
||||
f.libA = newLibrary(t, db, "List A", "ebooks", "/list/a")
|
||||
f.libB = newLibrary(t, db, "List B", "audiobooks", "/list/b")
|
||||
f.userA = newUser(t, db, "list-a")
|
||||
f.userNone = newUser(t, db, "list-none")
|
||||
grantRead(t, db, f.userA, f.libA)
|
||||
|
||||
const at = "2026-01-01T00:00:00.000Z"
|
||||
f.e1 = newItem(t, db, f.libA, "ebooks", "e1.epub", "Echo", "01 echo", false, at)
|
||||
f.e2 = newItem(t, db, f.libA, "ebooks", "e2.epub", "Delta", "02 delta", false, at)
|
||||
f.e3 = newItem(t, db, f.libA, "ebooks", "e3.epub", "Charlie", "03 charlie", false, at)
|
||||
f.c1 = newItem(t, db, f.libA, "comics", "c1.cbz", "Bravo", "04 bravo", false, at)
|
||||
f.m1 = newItem(t, db, f.libA, "ebooks", "m1.epub", "Missing", "05 missing", true, at)
|
||||
f.b1 = newItem(t, db, f.libB, "audiobooks", "b1.mp3", "Alpha", "06 alpha", false, at)
|
||||
return f
|
||||
}
|
||||
|
||||
// TestListItemsTotalIsNotThePage pins the recently added offset/limit/total
|
||||
// contract: total counts every matching row, independent of the page window.
|
||||
func TestListItemsTotalIsNotThePage(t *testing.T) {
|
||||
f := setupList(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
|
||||
code, resp := listItems(t, h, auth.User{ID: f.userA}, "?limit=2")
|
||||
if code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", code)
|
||||
}
|
||||
if resp.Total != 4 {
|
||||
t.Errorf("total = %d, want 4 (all matching present rows, not the page)", resp.Total)
|
||||
}
|
||||
if len(resp.Items) != 2 {
|
||||
t.Errorf("page has %d items, want 2", len(resp.Items))
|
||||
}
|
||||
if resp.Limit != 2 || resp.Offset != 0 {
|
||||
t.Errorf("limit/offset = %d/%d, want 2/0", resp.Limit, resp.Offset)
|
||||
}
|
||||
for _, it := range resp.Items {
|
||||
if it.ID == f.m1 {
|
||||
t.Error("a missing item was listed")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestListItemsPaging(t *testing.T) {
|
||||
f := setupList(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
u := auth.User{ID: f.userA}
|
||||
|
||||
_, p1 := listItems(t, h, u, "?limit=2&offset=0")
|
||||
_, p2 := listItems(t, h, u, "?limit=2&offset=2")
|
||||
if len(p1.Items) != 2 || len(p2.Items) != 2 {
|
||||
t.Fatalf("page sizes = %d/%d, want 2/2", len(p1.Items), len(p2.Items))
|
||||
}
|
||||
if p1.Offset != 0 || p2.Offset != 2 {
|
||||
t.Errorf("offsets = %d/%d, want 0/2", p1.Offset, p2.Offset)
|
||||
}
|
||||
|
||||
// Pages must not overlap, and must order by sort_title.
|
||||
if p1.Items[0].ID != f.e1 || p1.Items[1].ID != f.e2 {
|
||||
t.Errorf("page 1 = %v, want [%d %d]", idsFromRows(p1.Items), f.e1, f.e2)
|
||||
}
|
||||
if p2.Items[0].ID != f.e3 || p2.Items[1].ID != f.c1 {
|
||||
t.Errorf("page 2 = %v, want [%d %d]", idsFromRows(p2.Items), f.e3, f.c1)
|
||||
}
|
||||
seen := map[int64]bool{}
|
||||
for _, it := range append(append([]itemRow{}, p1.Items...), p2.Items...) {
|
||||
if seen[it.ID] {
|
||||
t.Errorf("item %d appears on both pages", it.ID)
|
||||
}
|
||||
seen[it.ID] = true
|
||||
}
|
||||
|
||||
// A page past the end is empty but still reports the true total.
|
||||
_, beyond := listItems(t, h, u, "?limit=2&offset=99")
|
||||
if len(beyond.Items) != 0 || beyond.Total != 4 {
|
||||
t.Errorf("beyond page: items=%d total=%d, want 0/4", len(beyond.Items), beyond.Total)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListItemsFiltersCompose(t *testing.T) {
|
||||
f := setupList(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
u := auth.User{ID: f.userA}
|
||||
|
||||
_, ebooks := listItems(t, h, u, "?kind=ebooks")
|
||||
if ebooks.Total != 3 {
|
||||
t.Errorf("kind=ebooks total = %d, want 3", ebooks.Total)
|
||||
}
|
||||
for _, it := range ebooks.Items {
|
||||
if it.Kind != "ebooks" {
|
||||
t.Errorf("kind filter returned a %q item", it.Kind)
|
||||
}
|
||||
}
|
||||
|
||||
_, libA := listItems(t, h, u, "?library_id="+itoa(f.libA))
|
||||
if libA.Total != 4 {
|
||||
t.Errorf("library_id=%d total = %d, want 4", f.libA, libA.Total)
|
||||
}
|
||||
|
||||
// kind and library_id compose (both must hold).
|
||||
_, both := listItems(t, h, u, "?kind=ebooks&library_id="+itoa(f.libA))
|
||||
if both.Total != 3 {
|
||||
t.Errorf("kind+library total = %d, want 3", both.Total)
|
||||
}
|
||||
for _, it := range both.Items {
|
||||
if it.Kind != "ebooks" || it.LibraryID != f.libA {
|
||||
t.Errorf("composed filter returned %+v", it)
|
||||
}
|
||||
}
|
||||
|
||||
// A kind with no items is an empty page, not an error.
|
||||
code, none := listItems(t, h, u, "?kind=movies")
|
||||
if code != http.StatusOK || none.Total != 0 || len(none.Items) != 0 {
|
||||
t.Errorf("unknown kind: status=%d total=%d items=%d", code, none.Total, len(none.Items))
|
||||
}
|
||||
}
|
||||
|
||||
func TestListItemsScoping(t *testing.T) {
|
||||
f := setupList(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
|
||||
// A user with no grants sees nothing.
|
||||
_, none := listItems(t, h, auth.User{ID: f.userNone}, "?limit=100")
|
||||
if none.Total != 0 || len(none.Items) != 0 {
|
||||
t.Errorf("no-grant user saw total=%d items=%d", none.Total, len(none.Items))
|
||||
}
|
||||
|
||||
// Asking explicitly for a library the user cannot read must not leak it.
|
||||
code, leak := listItems(t, h, auth.User{ID: f.userA}, "?library_id="+itoa(f.libB)+"&limit=100")
|
||||
if code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", code)
|
||||
}
|
||||
if leak.Total != 0 || len(leak.Items) != 0 {
|
||||
t.Errorf("user A read library %d: total=%d items=%d", f.libB, leak.Total, len(leak.Items))
|
||||
}
|
||||
|
||||
// The unfiltered list contains only the readable library.
|
||||
_, all := listItems(t, h, auth.User{ID: f.userA}, "?limit=100")
|
||||
for _, it := range all.Items {
|
||||
if it.LibraryID != f.libA {
|
||||
t.Errorf("item %d leaked from library %d", it.ID, it.LibraryID)
|
||||
}
|
||||
}
|
||||
if containsRowID(all.Items, f.b1) {
|
||||
t.Error("user A's list contained another user's item")
|
||||
}
|
||||
}
|
||||
|
||||
func TestListItemsInvalidLibraryID(t *testing.T) {
|
||||
f := setupList(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
for _, q := range []string{"?library_id=abc", "?library_id=-1", "?library_id=0"} {
|
||||
code, _ := listItems(t, h, auth.User{ID: f.userA}, q)
|
||||
if code != http.StatusBadRequest {
|
||||
t.Errorf("%s status = %d, want 400", q, code)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestListItemsDefaultsAndCaps(t *testing.T) {
|
||||
f := setupList(t)
|
||||
h := mediaHandlers(t, f.db)
|
||||
u := auth.User{ID: f.userA}
|
||||
|
||||
_, def := listItems(t, h, u, "")
|
||||
if def.Limit != 300 || def.Offset != 0 {
|
||||
t.Errorf("defaults = limit %d offset %d, want 300/0", def.Limit, def.Offset)
|
||||
}
|
||||
|
||||
// Out-of-range limit and negative offset fall back to the defaults.
|
||||
for _, q := range []string{"?limit=0", "?limit=5000", "?limit=-1"} {
|
||||
_, r := listItems(t, h, u, q)
|
||||
if r.Limit != 300 {
|
||||
t.Errorf("%s limit = %d, want 300", q, r.Limit)
|
||||
}
|
||||
}
|
||||
_, neg := listItems(t, h, u, "?offset=-5")
|
||||
if neg.Offset != 0 {
|
||||
t.Errorf("negative offset = %d, want 0", neg.Offset)
|
||||
}
|
||||
}
|
||||
|
||||
func idsFromRows(rows []itemRow) []int64 {
|
||||
out := make([]int64, len(rows))
|
||||
for i, r := range rows {
|
||||
out[i] = r.ID
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func containsRowID(rows []itemRow, id int64) bool {
|
||||
for _, r := range rows {
|
||||
if r.ID == id {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
|
@ -1,4 +1,5 @@
|
|||
// Package search queries the FTS5 index over media item titles and paths.
|
||||
// Package search queries the FTS5 index over media item titles, paths, and
|
||||
// credited people (authors/artists).
|
||||
package search
|
||||
|
||||
import (
|
||||
|
|
@ -15,6 +16,9 @@ type Result struct {
|
|||
Kind string `json:"kind"`
|
||||
Path string `json:"path"`
|
||||
Title string `json:"title"`
|
||||
// Authors is the space-joined credit text denormalised onto media_items by
|
||||
// migration 0009. It is exposed so a caller can show why a result matched.
|
||||
Authors string `json:"authors"`
|
||||
}
|
||||
|
||||
// Query searches visible libraries for a free-text string.
|
||||
|
|
@ -44,8 +48,11 @@ func Query(ctx context.Context, db *sql.DB, q string, libraryIDs []int64, limit
|
|||
}
|
||||
args = append(args, limit)
|
||||
|
||||
// The MATCH predicate is unchanged: media_search now indexes `authors` too
|
||||
// (migration 0009), so author terms match with no query-side change. The
|
||||
// column is only selected so callers can display it.
|
||||
query := `
|
||||
SELECT i.id, i.library_id, i.kind, i.path, i.title
|
||||
SELECT i.id, i.library_id, i.kind, i.path, i.title, i.authors
|
||||
FROM media_search s
|
||||
JOIN media_items i ON i.id = s.rowid
|
||||
WHERE media_search MATCH ?
|
||||
|
|
@ -63,7 +70,7 @@ func Query(ctx context.Context, db *sql.DB, q string, libraryIDs []int64, limit
|
|||
out := []Result{}
|
||||
for rows.Next() {
|
||||
var r Result
|
||||
if err := rows.Scan(&r.ID, &r.LibraryID, &r.Kind, &r.Path, &r.Title); err != nil {
|
||||
if err := rows.Scan(&r.ID, &r.LibraryID, &r.Kind, &r.Path, &r.Title, &r.Authors); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
out = append(out, r)
|
||||
|
|
|
|||
|
|
@ -50,6 +50,26 @@ func addItem(t *testing.T, db *sql.DB, libraryID int64, kind, path, title string
|
|||
return id
|
||||
}
|
||||
|
||||
func addPerson(t *testing.T, db *sql.DB, name string) int64 {
|
||||
t.Helper()
|
||||
var id int64
|
||||
err := db.QueryRowContext(context.Background(),
|
||||
`INSERT INTO people (name) VALUES (?) RETURNING id`, name).Scan(&id)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return id
|
||||
}
|
||||
|
||||
func addCredit(t *testing.T, db *sql.DB, itemID, personID int64, role string, ord int) {
|
||||
t.Helper()
|
||||
if _, err := db.ExecContext(context.Background(),
|
||||
`INSERT INTO item_people (item_id, person_id, role, ord) VALUES (?, ?, ?, ?)`,
|
||||
itemID, personID, role, ord); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
func titles(results []Result) []string {
|
||||
out := make([]string, len(results))
|
||||
for i, r := range results {
|
||||
|
|
@ -101,6 +121,84 @@ func TestQueryFindsItems(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestQueryFindsItemsByAuthor(t *testing.T) {
|
||||
db := testDB(t)
|
||||
ctx := context.Background()
|
||||
lib := addLibrary(t, db, "ebooks", "ebooks")
|
||||
id := addItem(t, db, lib, "ebooks", "notes.epub", "Notes on the Analytical Engine")
|
||||
ada := addPerson(t, db, "Ada Lovelace")
|
||||
addCredit(t, db, id, ada, "author", 0)
|
||||
|
||||
// The credit is not in the title or path, so this only passes if the FTS
|
||||
// index covers the denormalised authors column.
|
||||
res, err := Query(ctx, db, "lovelace", []int64{lib}, 50)
|
||||
if err != nil {
|
||||
t.Fatalf("Query: %v", err)
|
||||
}
|
||||
if len(res) != 1 || res[0].ID != id {
|
||||
t.Fatalf("author search results = %v", titles(res))
|
||||
}
|
||||
if res[0].Authors != "Ada Lovelace" {
|
||||
t.Errorf("Authors = %q, want %q", res[0].Authors, "Ada Lovelace")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAuthorIndexStaysInSync exercises every trigger 0009 installs on the
|
||||
// credit tables: insert, delete, rename (people UPDATE), and moving a credit
|
||||
// between items (item_people UPDATE).
|
||||
func TestAuthorIndexStaysInSync(t *testing.T) {
|
||||
db := testDB(t)
|
||||
ctx := context.Background()
|
||||
lib := addLibrary(t, db, "ebooks", "ebooks")
|
||||
id := addItem(t, db, lib, "ebooks", "notes.epub", "Notes")
|
||||
ada := addPerson(t, db, "Ada Lovelace")
|
||||
|
||||
addCredit(t, db, id, ada, "author", 0)
|
||||
if res, _ := Query(ctx, db, "lovelace", []int64{lib}, 50); len(res) != 1 {
|
||||
t.Fatal("credit insert did not reach the index")
|
||||
}
|
||||
|
||||
// Deleting the credit must remove the author from the index.
|
||||
if _, err := db.ExecContext(ctx,
|
||||
`DELETE FROM item_people WHERE item_id = ? AND person_id = ?`, id, ada); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if res, _ := Query(ctx, db, "lovelace", []int64{lib}, 50); len(res) != 0 {
|
||||
t.Error("stale author survived the credit delete")
|
||||
}
|
||||
|
||||
// Re-add, then rename: the old name must stop matching and the new start.
|
||||
addCredit(t, db, id, ada, "author", 0)
|
||||
if res, _ := Query(ctx, db, "lovelace", []int64{lib}, 50); len(res) != 1 {
|
||||
t.Fatal("re-added credit did not reach the index")
|
||||
}
|
||||
if _, err := db.ExecContext(ctx,
|
||||
`UPDATE people SET name = 'Ada King' WHERE id = ?`, ada); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if res, _ := Query(ctx, db, "lovelace", []int64{lib}, 50); len(res) != 0 {
|
||||
t.Error("old name survived the rename")
|
||||
}
|
||||
if res, _ := Query(ctx, db, "king", []int64{lib}, 50); len(res) != 1 {
|
||||
t.Error("new name did not reach the index after the rename")
|
||||
}
|
||||
|
||||
// Moving the credit to another item re-indexes both ends.
|
||||
other := addItem(t, db, lib, "ebooks", "other.epub", "Other")
|
||||
if _, err := db.ExecContext(ctx,
|
||||
`UPDATE item_people SET item_id = ? WHERE item_id = ? AND person_id = ?`,
|
||||
other, id, ada); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
res, err := Query(ctx, db, "king", []int64{lib}, 50)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(res) != 1 || res[0].ID != other {
|
||||
t.Errorf("moved credit: results = %v", titles(res))
|
||||
}
|
||||
}
|
||||
|
||||
func TestQueryIsScopedToLibraries(t *testing.T) {
|
||||
db := testDB(t)
|
||||
ctx := context.Background()
|
||||
|
|
|
|||
120
migrations/0009_search_authors.sql
Normal file
120
migrations/0009_search_authors.sql
Normal file
|
|
@ -0,0 +1,120 @@
|
|||
-- 0009_search_authors.sql — make credited people (authors, artists, directors)
|
||||
-- searchable.
|
||||
--
|
||||
-- 0003 indexed only `title` and `path`, so typing an author's name found
|
||||
-- nothing even though `people` and `item_people` already held the credits.
|
||||
-- Rather than make the search query join the credit tables — which would mean a
|
||||
-- second MATCH against a second index, or an unindexed LIKE over a join — the
|
||||
-- names are denormalised into a real column on `media_items`. The existing
|
||||
-- external-content FTS5 design then keeps working unchanged in spirit: the
|
||||
-- index still stores no text of its own, and the media_items triggers that
|
||||
-- already keep it in sync simply gain the new column.
|
||||
--
|
||||
-- `authors` is maintained by triggers on item_people/people, so a credit change
|
||||
-- re-indexes the item no matter which code path writes it — the metadata
|
||||
-- enricher today, anything else later.
|
||||
|
||||
ALTER TABLE media_items ADD COLUMN authors TEXT NOT NULL DEFAULT '';
|
||||
|
||||
-- Drop the old two-column index and its triggers before rebuilding.
|
||||
-- Triggers first: dropping the virtual table does not drop triggers that
|
||||
-- reference it, and a write landing between the two drops would fire a trigger
|
||||
-- against a table that no longer exists.
|
||||
DROP TRIGGER media_items_ai;
|
||||
DROP TRIGGER media_items_ad;
|
||||
DROP TRIGGER media_items_au;
|
||||
DROP TABLE media_search;
|
||||
|
||||
CREATE VIRTUAL TABLE media_search USING fts5(
|
||||
title,
|
||||
path,
|
||||
authors,
|
||||
content='media_items',
|
||||
content_rowid='id',
|
||||
tokenize='unicode61 remove_diacritics 2'
|
||||
);
|
||||
|
||||
-- Populate the fresh index from current content before the backfill below.
|
||||
-- FTS5's external-content 'delete' command assumes the rowid is already in the
|
||||
-- index; issuing it for an absent rowid raises "database disk image is
|
||||
-- malformed". The backfill UPDATE fires media_items_au (a delete+insert) for
|
||||
-- every existing row, so the index must first hold those rows. On a fresh
|
||||
-- database this indexes nothing and is a no-op.
|
||||
INSERT INTO media_search(media_search) VALUES ('rebuild');
|
||||
|
||||
-- Recreate the media_items triggers verbatim from 0003, with `authors` added,
|
||||
-- so every direct write to an item keeps the index correct.
|
||||
CREATE TRIGGER media_items_ai AFTER INSERT ON media_items BEGIN
|
||||
INSERT INTO media_search(rowid, title, path, authors)
|
||||
VALUES (new.id, new.title, new.path, new.authors);
|
||||
END;
|
||||
|
||||
CREATE TRIGGER media_items_ad AFTER DELETE ON media_items BEGIN
|
||||
INSERT INTO media_search(media_search, rowid, title, path, authors)
|
||||
VALUES ('delete', old.id, old.title, old.path, old.authors);
|
||||
END;
|
||||
|
||||
CREATE TRIGGER media_items_au AFTER UPDATE ON media_items BEGIN
|
||||
INSERT INTO media_search(media_search, rowid, title, path, authors)
|
||||
VALUES ('delete', old.id, old.title, old.path, old.authors);
|
||||
INSERT INTO media_search(rowid, title, path, authors)
|
||||
VALUES (new.id, new.title, new.path, new.authors);
|
||||
END;
|
||||
|
||||
-- Credit triggers. Each recomputes media_items.authors for the affected item.
|
||||
-- That UPDATE is itself on media_items, so media_items_au fires and rewrites
|
||||
-- the FTS row — no FTS-specific code is needed here. (This is a single level of
|
||||
-- trigger nesting, not recursion; and even with PRAGMA recursive_triggers=1 the
|
||||
-- chain terminates because the FTS write targets the virtual table, not
|
||||
-- media_items.)
|
||||
CREATE TRIGGER item_people_ai AFTER INSERT ON item_people BEGIN
|
||||
UPDATE media_items SET authors = COALESCE((
|
||||
SELECT group_concat(p.name, ' ') FROM item_people ip
|
||||
JOIN people p ON p.id = ip.person_id
|
||||
WHERE ip.item_id = media_items.id), '')
|
||||
WHERE id = new.item_id;
|
||||
END;
|
||||
|
||||
CREATE TRIGGER item_people_ad AFTER DELETE ON item_people BEGIN
|
||||
UPDATE media_items SET authors = COALESCE((
|
||||
SELECT group_concat(p.name, ' ') FROM item_people ip
|
||||
JOIN people p ON p.id = ip.person_id
|
||||
WHERE ip.item_id = media_items.id), '')
|
||||
WHERE id = old.item_id;
|
||||
END;
|
||||
|
||||
-- An UPDATE can move a credit between items, so both ends are recomputed.
|
||||
CREATE TRIGGER item_people_au AFTER UPDATE ON item_people BEGIN
|
||||
UPDATE media_items SET authors = COALESCE((
|
||||
SELECT group_concat(p.name, ' ') FROM item_people ip
|
||||
JOIN people p ON p.id = ip.person_id
|
||||
WHERE ip.item_id = media_items.id), '')
|
||||
WHERE id = old.item_id;
|
||||
UPDATE media_items SET authors = COALESCE((
|
||||
SELECT group_concat(p.name, ' ') FROM item_people ip
|
||||
JOIN people p ON p.id = ip.person_id
|
||||
WHERE ip.item_id = media_items.id), '')
|
||||
WHERE id = new.item_id;
|
||||
END;
|
||||
|
||||
-- A rename must not leave the old spelling in the index: recompute every item
|
||||
-- credited to this person. people.name is UNIQUE COLLATE NOCASE, so renames are
|
||||
-- rare, but a stale denormalised column is exactly what a rename would expose.
|
||||
CREATE TRIGGER people_au AFTER UPDATE ON people BEGIN
|
||||
UPDATE media_items SET authors = COALESCE((
|
||||
SELECT group_concat(p.name, ' ') FROM item_people ip
|
||||
JOIN people p ON p.id = ip.person_id
|
||||
WHERE ip.item_id = media_items.id), '')
|
||||
WHERE id IN (SELECT item_id FROM item_people WHERE person_id = old.id);
|
||||
END;
|
||||
|
||||
-- Backfill: the triggers above only fire on future writes, so populate every
|
||||
-- existing row once. The final rebuild re-derives the whole index from the
|
||||
-- content table, which also normalises any transient FTS state left by the
|
||||
-- per-row updates above.
|
||||
UPDATE media_items SET authors = COALESCE((
|
||||
SELECT group_concat(p.name, ' ') FROM item_people ip
|
||||
JOIN people p ON p.id = ip.person_id
|
||||
WHERE ip.item_id = media_items.id), '');
|
||||
|
||||
INSERT INTO media_search(media_search) VALUES ('rebuild');
|
||||
|
|
@ -1,5 +1,4 @@
|
|||
<script lang="ts">
|
||||
import ePub from 'epubjs'
|
||||
import { onMount, onDestroy } from 'svelte'
|
||||
import { api, ApiError, type Item, type EbookMetadata } from '../api'
|
||||
import ComicReader from './ComicReader.svelte'
|
||||
|
|
@ -67,6 +66,9 @@
|
|||
})
|
||||
|
||||
async function openBook(startIndex: number) {
|
||||
// epub.js is ~200 KB and most people never open an ebook. Load it on demand
|
||||
// so it lands in its own chunk instead of the main bundle.
|
||||
const ePub = (await import('epubjs')).default
|
||||
book = ePub(api.fileUrl(itemId), { openAs: 'epub' })
|
||||
rendition = book.renderTo(container, {
|
||||
width: '100%',
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue