Bladeren bron

fix(fever): always order the items endpoint by entry id

`handleItems` limits the result to 50 entries, but `since_id=0`, `with_ids`
and a request without parameters reached the database without an ORDER BY,
so PostgreSQL returned an arbitrary 50 rows. A client syncing from
`since_id=0` could miss the oldest entries for good.

The sort is set in each branch rather than once before the switch, because
`WithSorting` appends and a global default would take precedence over the
ascending order that `since_id` pagination needs.
Paolo Valletta 4 dagen geleden
bovenliggende
commit
a4c0033162
2 gewijzigde bestanden met toevoegingen van 14 en 14 verwijderingen
  1. 5 6
      internal/fever/README.md
  2. 9 8
      internal/fever/handler.go

+ 5 - 6
internal/fever/README.md

@@ -188,9 +188,9 @@ The handler applies a fixed limit of 50 items.
 
 Supported parameters:
 
-- `since_id`: when greater than `0`, returns entries with `id > since_id`, ordered by `id ASC`
-- `max_id`: when equal to `0`, returns the most recent entries ordered by `id DESC`; when greater than `0`, returns entries with `id < max_id`, ordered by `id DESC`
-- `with_ids`: comma-separated list of entry IDs to fetch
+- `since_id`: returns entries with `id > since_id` when greater than `0`, and the oldest entries otherwise, ordered by `id ASC`
+- `max_id`: returns entries with `id < max_id` when greater than `0`, and the most recent entries otherwise (including `0`), ordered by `id DESC`
+- `with_ids`: comma-separated list of entry IDs to fetch, ordered by `id ASC`
 
 Selector precedence inside `?items` is:
 
@@ -201,9 +201,9 @@ Selector precedence inside `?items` is:
 
 Notes:
 
-- `with_ids` does not enforce the 50-ID maximum mentioned in older Fever documentation
+- `with_ids` does not enforce the 50-ID maximum mentioned in older Fever documentation, but the 50-row limit still applies: a longer list returns its 50 lowest IDs
 - invalid `with_ids` members are parsed as `0` and do not match normal entries
-- when `items` is requested without `since_id`, `max_id`, or `with_ids`, the code applies no explicit `ORDER BY`, so result ordering is not guaranteed by SQL
+- when `items` is requested without `since_id`, `max_id`, or `with_ids`, the 50 oldest entries are returned, ordered by `id ASC`
 - `html` is returned after Miniflux content rewriting and may include media-proxy-rewritten URLs
 
 Example:
@@ -332,7 +332,6 @@ This implementation is Fever-compatible, but it does not match every detail of h
 - `last_refreshed_on_time` is set to the current response time, not the timestamp of the most recently refreshed feed
 - the `Kindling` and `Sparks` super groups are not returned
 - `feeds[].is_spark` is always `0`
-- item ordering without explicit pagination parameters is unspecified
 - `as=saved` and `as=unsaved` toggle the saved flag instead of setting it absolutely
 
 ## Examples

+ 9 - 8
internal/fever/handler.go

@@ -250,23 +250,22 @@ func (h *feverHandler) handleItems(w http.ResponseWriter, r *http.Request) {
 				slog.Int64("since_id", sinceID),
 			)
 			builder = builder.AfterEntryID(sinceID)
-			builder = builder.WithSorting("id", "ASC")
 		}
+		builder = builder.WithSorting("id", "ASC")
 	case request.HasQueryParam(r, "max_id"):
 		maxID := request.QueryInt64Param(r, "max_id", 0)
-		if maxID == 0 {
-			slog.Debug("[Fever] Fetching most recent items",
-				slog.Int64("user_id", userID),
-			)
-			builder = builder.WithSorting("id", "DESC")
-		} else if maxID > 0 {
+		if maxID > 0 {
 			slog.Debug("[Fever] Fetching items before a given item ID",
 				slog.Int64("user_id", userID),
 				slog.Int64("max_id", maxID),
 			)
 			builder = builder.BeforeEntryID(maxID)
-			builder = builder.WithSorting("id", "DESC")
+		} else {
+			slog.Debug("[Fever] Fetching most recent items",
+				slog.Int64("user_id", userID),
+			)
 		}
+		builder = builder.WithSorting("id", "DESC")
 	case request.HasQueryParam(r, "with_ids"):
 		csvItemIDs := request.QueryStringParam(r, "with_ids", "")
 		if csvItemIDs != "" {
@@ -280,10 +279,12 @@ func (h *feverHandler) handleItems(w http.ResponseWriter, r *http.Request) {
 
 			builder = builder.WithEntryIDs(itemIDs...)
 		}
+		builder = builder.WithSorting("id", "ASC")
 	default:
 		slog.Debug("[Fever] Fetching oldest items",
 			slog.Int64("user_id", userID),
 		)
+		builder = builder.WithSorting("id", "ASC")
 	}
 
 	entries, err := builder.GetEntries()