Commit Graph
5 Commits
Author SHA1 Message Date
john-okeefe e1aef8e85f refactor(services): Modernize Go code style in collection and filters services
Apply Go 1.18+ language features and modern style:

internal/services/collection_service.go:
- Use map[string]any instead of map[string]interface{} (Go 1.18+)
- Use range clause with single variable for iteration-only loops
- Replace if-else chains with switch statements for better readability
- Remove explicit type initialization for zero values

internal/services/filters.go:
- Add Err prefix to custom error variable for error naming convention

internal/router/library.go:
- Use cfg.ProcessingIssuesHandler instead of local processingIssuesHandler variable
- Ensures proper dependency injection through router config

These changes follow current Go best practices and improve code readability.
2026-04-13 09:25:01 -04:00
john-okeefe c3a98fb067 fix: use custom error type for saved filters not found
Fix failing test 'GET /api/saved-filters/:id_with_non-existent_filter_returns_404'
which was returning HTTP 500 instead of HTTP 404 due to string comparison
failure in error handling.

Root Cause:
- Service wrapped database error: fmt.Errorf("filter not found: %w", err)
- Handler checked exact string equality: err.Error() == "filter not found"
- Wrapped error message included database error: "filter not found: no rows in result set"
- String check failed → returned 500 instead of 404

Solution: Use Go error wrapping with custom error type

Changes to internal/services/filters.go:
- Add import: "errors" package
- Add custom error variable: ErrFilterNotFound
- Update GetSavedFilterByID() to return ErrFilterNotFound instead of wrapped error
- Error defined at service layer (domain authority)

Changes to internal/handlers/filters.go:
- Update error check from string comparison to errors.Is(err, services.ErrFilterNotFound)
- Uses Go's standard error wrapping pattern
- Cleaner, more maintainable, type-safe

Architectural Benefits:
-  Service layer owns domain errors (filter not found is a filter concept)
-  Handlers only translate service errors to HTTP status codes
-  Services reusable by any caller (API, WebSocket, CLI)
-  Clean dependency direction: Handlers → Services → Database
-  Follows Go best practices for error handling

Test Results:
- GET /api/saved-filters/:id with non-existent filter now returns 404
- Error message: "filter not found"
- No information leakage about other users' filters

Fixes test failure in TestSavedFilters.
2026-03-21 23:03:43 -04:00
john-okeefe 0960e36f30 feat: add GET /api/saved-filters/:id endpoint with comprehensive tests
Implement missing GET endpoint for retrieving individual saved filters by ID.
This completes the CRUD API for saved filters and enables mobile/SPA clients
to fetch filter details on-demand.

Backend Implementation:
- Add GetSavedFilterByID() handler method (internal/handlers/filters.go)
  - Parse filter ID from URL parameter
  - Validate UUID format, return 400 for invalid IDs
  - Call service layer for business logic + ownership verification
  - Return 404 if filter not found or doesn't belong to user
  - Return 200 with filter object including filters JSONB

- Add GetSavedFilterByID() service method (internal/services/filters.go)
  - Call existing database query GetSavedFilterByID
  - Verify filter exists and belongs to user
  - Return descriptive error: "filter not found or access denied"
  - Reuses existing database query (no new SQL needed)

- Register GET /:id route (internal/router/filters.go)
  - Add route before existing GET "" route
  - Follows RESTful routing conventions

Integration Tests (cmd/server/tests/filters_test.go):
- Test success case: Create filter, retrieve by ID, verify data
- Test error case: Invalid UUID format returns 400
- Test error case: Non-existent filter returns 404
- Test error case: No authentication returns 401
- Test security case: Cross-user access returns 404 (not 403)
  - Admin creates filter, regular user tries to access
  - Uses setup.Token (admin) and setup.RegularToken
  - Verifies information leakage prevention

API Design:
- Endpoint: GET /api/saved-filters/:id
- Authentication: JWT token required
- Response format: SavedFilterResponse with filters as JSON
- Error responses: 400 (invalid ID), 401 (no auth), 404 (not found)
- Security: Returns 404 for cross-user access (hides existence)

Benefits:
- Completes CRUD API for saved filters
- Enables future mobile/SPA clients
- Follows existing handler/service/test patterns
- Comprehensive security testing
- No database changes required (reuses existing queries)

Follows PROJECT_GUIDELINES.md service layer architecture and testing patterns.
2026-03-21 22:37:19 -04:00
john-okeefe 85964ec932 fix(api): enforce user isolation on saved filters delete operation
Fix critical security issue where admin users could delete other users'
saved filters due to incorrect error handling in DELETE query.

Database Schema Changes:
- Change DeleteSavedFilter from :exec to :one (queries.sql:1747-1750)
- Add RETURNING * to return deleted row for proper error detection
- Regenerate querier.go and queries.sql.go with updated signature

Service Layer (internal/services/filters.go):
- Update DeleteSavedFilter to capture returned row (using _ to discard)
- Properly propagate pgx.ErrNoRows when no rows are deleted
- Error wrapping preserves original error for handler detection

Handler Layer (internal/handlers/filters.go):
- Add errors.Is() check for pgx.ErrNoRows (line 148)
- Return 404 Not Found when filter doesn't exist or belongs to different user
- Return 500 Internal Server Error for other database errors
- Add "errors" import (line 8)

Security Fix Details:
Before: Admin could delete user's filter → 204 No Content (SUCCESS)
After:  Admin tries to delete user's filter → 404 Not Found (DENIED)

The DELETE query uses WHERE id = @id AND user_id = @user_id, which matches
0 rows when attempting to delete another user's filter. The old :exec query
didn't return row count, so 0 affected rows looked like success. The new :one
query with RETURNING * returns pgx.ErrNoRows when no rows match, allowing
the handler to return proper 404 error.

Test Impact:
- TestSavedFilters/User_cannot_access_another_user's_filter now passes
- All 6 integration tests pass with proper user isolation enforcement

Pattern Consistency:
- Matches DeleteLibraryFolder pattern (line 99 in queries.sql)
- Uses same error handling as media handlers (errors.Is + pgx.ErrNoRows)
- Follows user-scoping pattern used throughout codebase

Related: Saved filters implementation user isolation
Security: Prevents unauthorized deletion of user data
2026-03-21 01:24:15 -04:00
john-okeefe ddfc832b68 feat(api): implement saved filters backend service and handlers
Add complete backend implementation for saved filters CRUD operations
with proper service layer architecture and RESTful API endpoints.

Service Layer (internal/services/filters.go):
- NewFiltersService() constructor following project patterns
- GetSavedFilters(): Retrieve all filters for user + resource type
- CreateSavedFilter(): Create filter with duplicate name validation
- UpdateSavedFilter(): Update filter with ownership verification
- DeleteSavedFilter(): Delete filter with user scoping

Business Logic:
- Filter name uniqueness enforced per user + resource type
- User ownership validation on all operations (JWT user_id)
- JSONB marshaling/unmarshaling for flexible filter storage
- Proper error wrapping with context messages

Handler Layer (internal/handlers/filters.go):
- NewFiltersHandler() constructor (receives db.Queries)
- GetSavedFilters: GET /api/saved-filters?resource_type=X
- CreateSavedFilter: POST /api/saved-filters
- UpdateSavedFilter: PUT /api/saved-filters/:id
- DeleteSavedFilter: DELETE /api/saved-filters/:id

Content Negotiation:
- Supports both JSON (API clients) and HTML (HTMX) responses
- wantsHTML() helper checks Accept header
- HX-Redirect header for HTMX form submissions
- Proper status codes (200, 201, 204, 400, 401, 404, 409)

Router Configuration:
- registerFiltersRoutes() function in internal/router/filters.go
- JWT middleware protection on all endpoints
- RESTful route structure: /api/saved-filters
- Registered in main router.go RegisterRoutes() function
- Added FiltersHandler to router.Config struct

Test Infrastructure:
- Added FiltersHandler to test server setup (test_helpers_test.go)
- FiltersHandler initialized in setupTestServer() function
- Router.Config includes FiltersHandler for integration tests

Code Quality:
- Follows PROJECT_GUIDELINES.md service layer patterns
- Uses database models (not custom domain models)
- JSONB returned as []byte (matches collections pattern)
- All errors wrapped with context using fmt.Errorf
- Handlers create services internally (not dependency injection)

Part of: Saved Filters Implementation (Phase 2: Backend)
Related: #saved-filters-feature
2026-03-21 00:16:10 -04:00