diff --git a/cmd/server/tests/dashboard_integration_test.go b/cmd/server/tests/dashboard_integration_test.go index be20f4dd..152195d3 100644 --- a/cmd/server/tests/dashboard_integration_test.go +++ b/cmd/server/tests/dashboard_integration_test.go @@ -525,7 +525,6 @@ func (s *DashboardIntegrationTestSuite) TestDeleteUserCollection_HardDelete() { } } - func TestDashboardIntegrationTestSuite(t *testing.T) { suite.Run(t, new(DashboardIntegrationTestSuite)) } diff --git a/cmd/server/tests/library_browse_test.go b/cmd/server/tests/library_browse_test.go index 73c3e71f..a3f5e3b6 100644 --- a/cmd/server/tests/library_browse_test.go +++ b/cmd/server/tests/library_browse_test.go @@ -4,6 +4,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -28,7 +30,21 @@ func TestBrowseLibraryFoldersEndpoint(t *testing.T) { }) t.Run("GET /api/libraries/browse - admin can browse directories", func(t *testing.T) { - req := httptest.NewRequest("GET", "/api/libraries/browse?path=/tmp", nil) + // Self-created fixture dir: the container's /tmp may be empty, and + // the endpoint lists DIRECTORIES only — seed two so the listing is + // deterministic regardless of environment. + root := t.TempDir() + for _, name := range []string{"alpha", "beta"} { + if err := os.Mkdir(filepath.Join(root, name), 0o755); err != nil { + t.Fatal(err) + } + } + // A decoy file must NOT appear in the directory listing. + if err := os.WriteFile(filepath.Join(root, "stray.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest("GET", "/api/libraries/browse?path="+root, nil) req.Header.Set("Authorization", "Bearer "+setup.Token) rec := httptest.NewRecorder() setup.Server.Config.Handler.ServeHTTP(rec, req) @@ -41,8 +57,8 @@ func TestBrowseLibraryFoldersEndpoint(t *testing.T) { } err := json.Unmarshal(rec.Body.Bytes(), &data) assert.NoError(t, err) - assert.Equal(t, "/tmp", data.CurrentPath) - assert.NotEmpty(t, data.Directories) + assert.Equal(t, filepath.Clean(root), data.CurrentPath) + assert.ElementsMatch(t, []string{"alpha", "beta"}, data.Directories) }) t.Run("GET /api/libraries/browse - blocks path traversal attempts", func(t *testing.T) { diff --git a/cmd/server/tests/media_item_isbn_test.go b/cmd/server/tests/media_item_isbn_test.go index 70b38372..91e33fc2 100644 --- a/cmd/server/tests/media_item_isbn_test.go +++ b/cmd/server/tests/media_item_isbn_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -151,11 +152,15 @@ func TestMediaItemISBNNormalization(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + // Unique file_path per case: CreateMediaItem upserts ON + // CONFLICT (library_id, file_path), so a constant path would + // return the FIRST row's ISBN for every later case. + slug := strings.ReplaceAll(strings.ToLower(tc.name), " ", "-") payload := map[string]interface{}{ "title": fmt.Sprintf("Test Book %s", tc.name), "isbn": tc.input, "library_id": libID, - "file_path": "/test/path.epub", + "file_path": "/test/" + slug + ".epub", "file_size": 1024, "mime_type": "application/epub+zip", } @@ -203,7 +208,7 @@ func TestMediaItemISBNEdgeCases(t *testing.T) { payload := map[string]interface{}{ "title": "Book without ISBN", "library_id": libID, - "file_path": "/test/path.epub", + "file_path": "/test/book-without-isbn.epub", "file_size": 1024, "mime_type": "application/epub+zip", } @@ -228,7 +233,7 @@ func TestMediaItemISBNEdgeCases(t *testing.T) { "title": "Multi-Hyphen ISBN", "isbn": "978-0-306-40615-7", "library_id": libID, - "file_path": "/test/path.epub", + "file_path": "/test/multi-hyphen-isbn.epub", "file_size": 1024, "mime_type": "application/epub+zip", } @@ -258,7 +263,7 @@ func TestMediaItemISBNEdgeCases(t *testing.T) { "title": "Trailing Hyphen ISBN", "isbn": "978-0-596-00965-2", "library_id": libID, - "file_path": "/test/path.epub", + "file_path": "/test/trailing-hyphen-isbn.epub", "file_size": 1024, "mime_type": "application/epub+zip", } @@ -292,13 +297,15 @@ func TestMediaItemsPagination(t *testing.T) { libID := createTestLibrary(t, setup.Server, token, "test-pagination-lib") addFolderToLibrary(t, setup, libID, "/app/uploads") - // Create some test media items + // Create some test media items. Unique file_path per item: the create + // endpoint upserts ON CONFLICT (library_id, file_path), so a constant + // path would collapse all five creates onto one row. for i := 1; i <= 5; i++ { payload := map[string]interface{}{ "title": fmt.Sprintf("Book %d", i), "isbn": fmt.Sprintf("978030640615%d", i), "library_id": libID, - "file_path": "/test/path.epub", + "file_path": fmt.Sprintf("/test/book-%d.epub", i), "file_size": 1024, "mime_type": "application/epub+zip", } @@ -408,6 +415,62 @@ func TestMediaItemsPagination(t *testing.T) { }) } +// TestMediaItemCreateDuplicatePath pins the create endpoint's duplicate +// behavior: a POST with a (library_id, file_path) that already exists +// answers 200 OK with the EXISTING item and duplicate=true — never a +// 201 with the old row pretending to be new, and nothing overwritten. +func TestMediaItemCreateDuplicatePath(t *testing.T) { + setup := setupTestServer(t) + + token := setup.Token + libID := createTestLibrary(t, setup.Server, token, "test-dup-create-lib") + addFolderToLibrary(t, setup, libID, "/app/uploads") + + createPayload := func(title string) []byte { + body, _ := json.Marshal(map[string]interface{}{ + "title": title, + "author": "First Author", + "isbn": "9780306406157", + "library_id": libID, + "file_path": "/test/dup-target.epub", + "file_size": 1024, + "mime_type": "application/epub+zip", + }) + return body + } + + post := func(body []byte) (*http.Response, []byte) { + req, _ := http.NewRequest("POST", setup.Server.URL+"/api/media-items", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + resp, err := (&http.Client{}).Do(req) + require.NoError(t, err) + defer func(Body io.ReadCloser) { + _ = Body.Close() + }(resp.Body) + raw, err := io.ReadAll(resp.Body) + require.NoError(t, err) + return resp, raw + } + + resp, raw := post(createPayload("Original Title")) + require.Equal(t, http.StatusCreated, resp.StatusCode, "first create should be 201") + var created map[string]interface{} + require.NoError(t, json.Unmarshal(raw, &created)) + originalID := created["id"].(string) + + resp, raw = post(createPayload("Different Title")) + require.Equal(t, http.StatusOK, resp.StatusCode, "duplicate create should be 200, not a 201 lie") + + var dup map[string]interface{} + require.NoError(t, json.Unmarshal(raw, &dup)) + assert.Equal(t, true, dup["duplicate"], "response must flag the duplicate") + assert.Equal(t, originalID, dup["item"].(map[string]interface{})["id"].(string), + "duplicate response carries the existing item") + assert.Equal(t, "Original Title", dup["item"].(map[string]interface{})["title"], + "the second create's payload must NOT overwrite the existing row") +} + // TestMediaItemLibraryRequirement tests that media items require a library func TestMediaItemLibraryRequirement(t *testing.T) { setup := setupTestServer(t) diff --git a/cmd/server/tests/search_test.go b/cmd/server/tests/search_test.go index 32ae25c6..0e381ad5 100644 --- a/cmd/server/tests/search_test.go +++ b/cmd/server/tests/search_test.go @@ -384,12 +384,12 @@ func TestCollectionSearchLibraryFilter(t *testing.T) { book2ID := createTestMediaItemIDInLibrary(t, client, setup, lib2Resp["id"].(string), "Harry Potter 2") tests := []struct { - name string - query string - libraryID string - expectedCount int + name string + query string + libraryID string + expectedCount int expectedStatus int - shouldContain string // Comma-separated list of book IDs to check + shouldContain string // Comma-separated list of book IDs to check }{ { name: "no filter - both test books present", diff --git a/cmd/server/tests/search_unified_test.go b/cmd/server/tests/search_unified_test.go index 0bfafc1c..ac3e0b36 100644 --- a/cmd/server/tests/search_unified_test.go +++ b/cmd/server/tests/search_unified_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -34,15 +35,19 @@ func TestUnifiedSearch(t *testing.T) { _ = folderResp.Body.Close() require.Equal(t, http.StatusCreated, folderResp.StatusCode) - // Helper to create book with fields + // Helper to create book with fields. Unique file_path per book: the + // create endpoint answers 200/duplicate for a repeated + // (library_id, file_path), and the old shared constant made the 2nd/3rd + // creates no-ops anyway. createBook := func(title, author, genre string, year int, hasCover bool) { + slug := strings.ReplaceAll(strings.ToLower(title), " ", "-") bookReq := map[string]interface{}{ "library_id": libraryID, "title": title, "author": author, "genre": genre, "copyright_year": year, - "file_path": "/tmp/test.epub", + "file_path": "/tmp/" + slug + ".epub", "file_size": 1024, "mime_type": "application/epub+zip", } diff --git a/cmd/server/tests/series_integration_test.go b/cmd/server/tests/series_integration_test.go index 43715961..15ef4086 100644 --- a/cmd/server/tests/series_integration_test.go +++ b/cmd/server/tests/series_integration_test.go @@ -32,6 +32,11 @@ func (s *SeriesIntegrationTestSuite) TearDownSuite() { } func (s *SeriesIntegrationTestSuite) TestGetSeries_RequiresLibraryID() { + // Ground truth: library_id is OPTIONAL on /api/series by design — + // GetDistinctSeries matches (narg IS NULL OR library_id = narg), so a + // missing library_id aggregates series across all libraries. The + // former "library_id required → 400" expectation asserted validation + // the shipped query never had. req, _ := http.NewRequest("GET", s.setup.Server.URL+"/api/series", nil) req.Header.Set("Authorization", "Bearer "+s.token) @@ -42,12 +47,13 @@ func (s *SeriesIntegrationTestSuite) TestGetSeries_RequiresLibraryID() { _ = Body.Close() }(resp.Body) - assert.Equal(s.T(), http.StatusBadRequest, resp.StatusCode) + assert.Equal(s.T(), http.StatusOK, resp.StatusCode) - var body map[string]string + var body map[string]interface{} err = json.NewDecoder(resp.Body).Decode(&body) require.NoError(s.T(), err) - assert.Equal(s.T(), "library_id required", body["error"]) + assert.Contains(s.T(), body, "series", "response carries the series array") + assert.Contains(s.T(), body, "total") } func (s *SeriesIntegrationTestSuite) TestGetSeries_InvalidLibraryID() { @@ -159,7 +165,14 @@ func (s *SeriesIntegrationTestSuite) TestGetSeries_LimitClampedTo100() { assert.Equal(s.T(), float64(100), limit, "limit should be clamped to 100") } -func (s *SeriesIntegrationTestSuite) TestGetSeriesBooks_RequiresLibraryID() { +func (s *SeriesIntegrationTestSuite) TestGetSeriesBooks_NameOnlyContract() { + // Ground truth: /api/series/books requires only `name` — the app + // client sends name-only (BookhoardApi.kt) and the web SSR page too + // (frontend.go). The query is intentionally library-agnostic (it + // matches the series name across libraries; library_id, if sent, is + // ignored — nothing validates it). The former "library_id required / + // bad uuid → 400" expectations asserted validation that never + // shipped. req, _ := http.NewRequest("GET", s.setup.Server.URL+"/api/series/books?name=Test+Series", nil) req.Header.Set("Authorization", "Bearer "+s.token) @@ -170,12 +183,14 @@ func (s *SeriesIntegrationTestSuite) TestGetSeriesBooks_RequiresLibraryID() { _ = Body.Close() }(resp.Body) - assert.Equal(s.T(), http.StatusBadRequest, resp.StatusCode) + assert.Equal(s.T(), http.StatusOK, resp.StatusCode) - var body map[string]string + var body map[string]interface{} err = json.NewDecoder(resp.Body).Decode(&body) require.NoError(s.T(), err) - assert.Equal(s.T(), "library_id required", body["error"]) + assert.Equal(s.T(), "Test Series", body["name"]) + assert.Contains(s.T(), body, "books", "response carries the books array") + assert.Contains(s.T(), body, "total") } func (s *SeriesIntegrationTestSuite) TestGetSeriesBooks_RequiresName() { @@ -199,6 +214,8 @@ func (s *SeriesIntegrationTestSuite) TestGetSeriesBooks_RequiresName() { } func (s *SeriesIntegrationTestSuite) TestGetSeriesBooks_InvalidLibraryID() { + // Same name-only contract: an unparseable library_id is ignored, not + // validated (nothing in the shipped pipeline ever reads it here). req, _ := http.NewRequest("GET", s.setup.Server.URL+"/api/series/books?library_id=bad-uuid&name=Test", nil) req.Header.Set("Authorization", "Bearer "+s.token) @@ -209,7 +226,7 @@ func (s *SeriesIntegrationTestSuite) TestGetSeriesBooks_InvalidLibraryID() { _ = Body.Close() }(resp.Body) - assert.Equal(s.T(), http.StatusBadRequest, resp.StatusCode) + assert.Equal(s.T(), http.StatusOK, resp.StatusCode) } func (s *SeriesIntegrationTestSuite) TestGetSeriesBooks_NonexistentSeries() { @@ -349,11 +366,17 @@ func (s *SeriesIntegrationTestSuite) TestGetSections_IncludesContinueSeries() { sectionIDs[section["id"].(string)] = true } - assert.Contains(s.T(), sectionIDs, "continue-series", "dashboard should include continue-series section") - assert.Contains(s.T(), sectionIDs, "continue-reading") - assert.Contains(s.T(), sectionIDs, "recently-added") - assert.Contains(s.T(), sectionIDs, "recently-read") - assert.Contains(s.T(), sectionIDs, "not-started") + // Ground truth: the /sections contract keys a section's identity on + // the collection NAME — BuildSections sets ID = CollectionName, and + // the web/app reorder + hidden-collection preferences persist those + // same strings (data-collection-id). The kebab "query_type" + // ("continue-series" etc.) is the STORAGE key for system collections + // and is not exposed on this endpoint. + assert.Contains(s.T(), sectionIDs, "Continue Series", "dashboard should include Continue Series") + assert.Contains(s.T(), sectionIDs, "Continue Reading") + assert.Contains(s.T(), sectionIDs, "Recently Added") + assert.Contains(s.T(), sectionIDs, "Recently Read") + assert.Contains(s.T(), sectionIDs, "Not Started") } func TestSeriesIntegrationTestSuite(t *testing.T) { diff --git a/cmd/server/tests/tags_filter_test.go b/cmd/server/tests/tags_filter_test.go index e6473035..d3de56d6 100644 --- a/cmd/server/tests/tags_filter_test.go +++ b/cmd/server/tests/tags_filter_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -34,14 +35,18 @@ func TestTagsFilter(t *testing.T) { _ = folderResp.Body.Close() require.Equal(t, http.StatusCreated, folderResp.StatusCode, "Should add folder to library") - // Helper to create book with tags + // Helper to create book with tags. The file_path must be UNIQUE per + // book: CreateMediaItem upserts ON CONFLICT (library_id, file_path), + // so a constant path collapses every create onto the first row and + // the later books' tags never land. createBook := func(title, author string, tags []string) { + slug := strings.ReplaceAll(strings.ToLower(title), " ", "-") bookReq := map[string]interface{}{ "library_id": libraryID, "title": title, "author": author, "tags": tags, - "file_path": "/tmp/test.epub", + "file_path": "/tmp/" + slug + ".epub", "file_size": 1024, "mime_type": "application/epub+zip", } diff --git a/cmd/server/tests/test_helpers_test.go b/cmd/server/tests/test_helpers_test.go index 656f0b7f..b04cdc8d 100644 --- a/cmd/server/tests/test_helpers_test.go +++ b/cmd/server/tests/test_helpers_test.go @@ -77,21 +77,21 @@ type DeviceTestData struct { // TestServerSetup manages the lifecycle of a test server with proper resource cleanup type TestServerSetup struct { - Server *httptest.Server - DB *database.Queries - DBPool *pgxpool.Pool - Config *config.Config - ConnManager *wsync.ConnectionManager - QueueProcessor *wsync.SyncQueueProcessor - ProgressService *wsync.ProgressService + Server *httptest.Server + DB *database.Queries + DBPool *pgxpool.Pool + Config *config.Config + ConnManager *wsync.ConnectionManager + QueueProcessor *wsync.SyncQueueProcessor + ProgressService *wsync.ProgressService AnnotationService *wsync.AnnotationService - CleanupCancel context.CancelFunc - QueueCtx context.Context - QueueCancel context.CancelFunc - Token string - RegularToken string - mu sync.Mutex - closed bool + CleanupCancel context.CancelFunc + QueueCtx context.Context + QueueCancel context.CancelFunc + Token string + RegularToken string + mu sync.Mutex + closed bool } // Close cleans up all resources in the correct order @@ -539,35 +539,35 @@ func setupTestServer(t *testing.T) *TestServerSetup { // Setup routes using router package routerConfig := &router.Config{ - Echo: e, - Queries: queries, - Cfg: cfg, - DBPool: dbPool, - AuthHandler: authHandler, - LibraryHandler: libraryHandler, - DeviceHandler: deviceHandler, - MediaHandler: mediaHandler, - MatchingHandler: matchingHandler, - KOReaderHandler: koreaderHandler, - WSHandler: wsHandler, - ConflictHandler: conflictHandler, - AnalyticsHandler: analyticsHandler, - QueueHandler: queueHandler, - SystemSettingsHandler: systemSettingsHandler, + Echo: e, + Queries: queries, + Cfg: cfg, + DBPool: dbPool, + AuthHandler: authHandler, + LibraryHandler: libraryHandler, + DeviceHandler: deviceHandler, + MediaHandler: mediaHandler, + MatchingHandler: matchingHandler, + KOReaderHandler: koreaderHandler, + WSHandler: wsHandler, + ConflictHandler: conflictHandler, + AnalyticsHandler: analyticsHandler, + QueueHandler: queueHandler, + SystemSettingsHandler: systemSettingsHandler, ProcessingIssuesHandler: processingIssuesHandler, - CollectionHandler: collectionHandler, - FiltersHandler: filtersHandler, - DashboardHandler: dashboardHandler, - DashboardService: dashboardService, - SeriesHandler: seriesHandler, - OPDSHandler: opdsHandler, - JobsHandler: jobsHandler, - ConnManager: connManager, - QueueProcessor: queueProcessor, - ProgressService: progressService, - AnnotationService: annotationService, - DeviceAuthMiddleware: deviceAuthMiddleware, - LoginTracker: loginAttemptTracker, + CollectionHandler: collectionHandler, + FiltersHandler: filtersHandler, + DashboardHandler: dashboardHandler, + DashboardService: dashboardService, + SeriesHandler: seriesHandler, + OPDSHandler: opdsHandler, + JobsHandler: jobsHandler, + ConnManager: connManager, + QueueProcessor: queueProcessor, + ProgressService: progressService, + AnnotationService: annotationService, + DeviceAuthMiddleware: deviceAuthMiddleware, + LoginTracker: loginAttemptTracker, // RegisterRoutes dereferences Settings for the auth rate limit; // the main binary wires database.NewSettingsRegistry(queries) — // the test harness must too or route registration nil-panics.