alex/ephyra
Watch
1
0
Fork
You've already forked ephyra
 0
Code Issues 2 Pull requests Projects Releases Packages 1 Wiki Activity Actions

/UserViews is a view short, and mislabels the one standing where it should be #80

New issue
Closed
opened 2026-08-04 20:21:30 +02:00 by Claude · 3 comments
Claude commented 2026-08-04 20:21:30 +02:00
Collaborator
Copy link

/UserViews is a view short, and mislabels the one standing where it should be.

Measured against a database copied from a running Jellyfin, with both servers
given their own copy.

GET /UserViews
  Items: upstream has 4 entries, ours 3
  Items[2].CollectionType: upstream "playlists", ours "tvshows"
  Items[2].DateCreated:    upstream "2022-05-16T23:00:31.4841257Z",
                           ours     "2021-12-24T17:09:48.0891632Z"

Two things, and the second follows from the first. Upstream serves four views
and we serve three; the missing one is the playlists view. Because it is
missing, index 2 is a different view on each side, which is why
CollectionType and DateCreated "differ" — they belong to different rows.

The library has a PlaylistsFolder row (Emby.Server.Implementations.Playlists. PlaylistsFolder), so this is a view not being derived from a row that is there,
not a row that is absent. /UserViews is the first authenticated request every
client makes, and a client counts the views it got.

Whether playlists are in scope for this project is the prior question — #76
already sets the playlists folder aside as something a scan does not write. If
they are out of scope, the answer is presumably still four views with an empty
one rather than three, since a client branches on the set it is given.

To reproduce.

curl -s -H "Authorization: MediaBrowser Token=\"$TOKEN\"" \
    http://127.0.0.1:8096/UserViews | jq -r '.Items[] | "\(.CollectionType)\t\(.Name)"'
curl -s -H "Authorization: MediaBrowser Token=\"$TOKEN\"" \
    http://127.0.0.1:8097/UserViews | jq -r '.Items[] | "\(.CollectionType)\t\(.Name)"'

Needs a library that has a playlist in it; the committed snapshot has the folder
but nothing under it.

`/UserViews` is a view short, and mislabels the one standing where it should be. Measured against a database copied from a running Jellyfin, with both servers given their own copy. GET /UserViews Items: upstream has 4 entries, ours 3 Items[2].CollectionType: upstream "playlists", ours "tvshows" Items[2].DateCreated: upstream "2022-05-16T23:00:31.4841257Z", ours "2021-12-24T17:09:48.0891632Z" Two things, and the second follows from the first. Upstream serves four views and we serve three; the missing one is the playlists view. Because it is missing, index 2 is a different view on each side, which is why `CollectionType` and `DateCreated` "differ" — they belong to different rows. The library has a `PlaylistsFolder` row (`Emby.Server.Implementations.Playlists. PlaylistsFolder`), so this is a view not being derived from a row that is there, not a row that is absent. `/UserViews` is the first authenticated request every client makes, and a client counts the views it got. Whether playlists are in scope for this project is the prior question — #76 already sets the playlists folder aside as something a scan does not write. If they are out of scope, the answer is presumably still four views with an empty one rather than three, since a client branches on the set it is given. **To reproduce.** curl -s -H "Authorization: MediaBrowser Token=\"$TOKEN\"" \ http://127.0.0.1:8096/UserViews | jq -r '.Items[] | "\(.CollectionType)\t\(.Name)"' curl -s -H "Authorization: MediaBrowser Token=\"$TOKEN\"" \ http://127.0.0.1:8097/UserViews | jq -r '.Items[] | "\(.CollectionType)\t\(.Name)"' Needs a library that has a playlist in it; the committed snapshot has the folder but nothing under it.
Claude commented 2026-08-05 11:05:10 +02:00
Author
Collaborator
Copy link

This gates a benchmark request. In bench run 20260805T083531Z the anchor gate dropped views — GET /UserViews — with 17 differences, and every one of them is downstream of the missing playlists view: Items counts 4 against 3, and then all of Items[2]'s fields differ because index 2 is a different view on each side.

Items: upstream has 4 entries, ours 3
Items[2].Name: upstream "Playlists", ours "Shows"
Items[2].Type: upstream "UserView", ours "CollectionFolder"
TotalRecordCount: upstream 4, ours 3

So this is one bug, not seventeen, and fixing it should take the request into the timed set outright. /UserViews costs 16.5 ms of processor now (see #83) and is unmeasured against upstream.

**This gates a benchmark request.** In bench run `20260805T083531Z` the anchor gate dropped `views` — `GET /UserViews` — with 17 differences, and every one of them is downstream of the missing playlists view: `Items` counts 4 against 3, and then all of `Items[2]`'s fields differ because index 2 is a different view on each side. Items: upstream has 4 entries, ours 3 Items[2].Name: upstream "Playlists", ours "Shows" Items[2].Type: upstream "UserView", ours "CollectionFolder" TotalRecordCount: upstream 4, ours 3 So this is one bug, not seventeen, and fixing it should take the request into the timed set outright. `/UserViews` costs 16.5 ms of processor now (see #83) and is unmeasured against upstream.
Claude commented 2026-08-11 20:28:19 +02:00
Author
Collaborator
Copy link

The runs behind this issue gave the two servers a metadata directory neither of them could use, and that is most of what they measured.

A by-name item's id — person, genre, studio, artist — is the MD5 of <type FullName><metadata path> over UTF-16, lowercased, with the path taken relative to the program data directory when it sits underneath one. A picture's tag is the MD5 of that same path whole. Two things are hashed over one directory, and they answer to different parts of it.

cargo xtask bench copied the anchor's metadata directory to state/metadata — outside both servers' data directories — deliberately, so that both would name one path and agree on picture tags. That satisfies the second rule and breaks the first. Upstream then derives every by-name id from the absolute path, reaches none of the rows the anchor was written with, and:

  • answers People: [] for every item, on a listing and on a by-id read alike;
  • writes a second Person row for each name it touches, with an id of its own;
  • re-derives the user's views instead of finding the ones stored.

Ours joins a person by name and keeps answering, so the pair looks like a port that invents a cast, loses a view, and miscounts.

Measured just now against the anchor scenario, same host, same binaries, only the layout changed:

differences suppressed by the ignore list
metadata outside the data directory 3,025 3,670
metadata inside it, one path for both 1,910 562

The 3,108 suppressions that stopped firing are the People[*].PrimaryImageTag and blurhash entries — the ignore list was absorbing a setup fault, which is exactly what it is not for.

Fixed in f064438: one program data directory, named to both servers, with the one metadata copy linked inside it. crates/jellyfin-harness/README.md now states both rules.

So the numbers in this issue want re-reading, not trusting. What survives the corrected layout is written up per issue.

**The runs behind this issue gave the two servers a metadata directory neither of them could use, and that is most of what they measured.** A by-name item's id — person, genre, studio, artist — is the MD5 of `<type FullName><metadata path>` over UTF-16, lowercased, with the path taken **relative to the program data directory** when it sits underneath one. A picture's tag is the MD5 of that same path *whole*. Two things are hashed over one directory, and they answer to different parts of it. `cargo xtask bench` copied the anchor's metadata directory to `state/metadata` — outside both servers' data directories — deliberately, so that both would name one path and agree on picture tags. That satisfies the second rule and breaks the first. Upstream then derives every by-name id from the absolute path, reaches **none** of the rows the anchor was written with, and: - answers `People: []` for every item, on a listing and on a by-id read alike; - writes a *second* `Person` row for each name it touches, with an id of its own; - re-derives the user's views instead of finding the ones stored. Ours joins a person by name and keeps answering, so the pair looks like a port that invents a cast, loses a view, and miscounts. Measured just now against the anchor scenario, same host, same binaries, only the layout changed: | | differences | suppressed by the ignore list | |---|---|---| | metadata outside the data directory | 3,025 | 3,670 | | metadata inside it, one path for both | **1,910** | **562** | The 3,108 suppressions that stopped firing are the `People[*].PrimaryImageTag` and blurhash entries — the ignore list was absorbing a setup fault, which is exactly what it is not for. Fixed in `f064438`: one program data directory, named to both servers, with the one metadata copy linked inside it. `crates/jellyfin-harness/README.md` now states both rules. **So the numbers in this issue want re-reading, not trusting.** What survives the corrected layout is written up per issue.
Claude commented 2026-08-11 22:01:03 +02:00
Author
Collaborator
Copy link

These are one bug, both numbers in the issue are right, and the reason they looked unreproducible is that the measurement used the wrong account.

Fixed on main in 468c50d, ffa3436, 05a0592 and the two commits beside them. Closing #79 and #80 together.

Why they seemed to vanish

/Items/Counts and a recursive /Items scope to the top parents of the user's views, so a missing view is uncounted rows: the two are the same bug, and the two missing rows are the playlists folder and the playlist under it.

Upstream serves a playlists view only to someone who can open a playlist — GetUserViews gates it on items.Any(item => item.IsVisible(user)), and a playlist is visible to its owner, to anyone it is shared with, or if it is open access. The anchor holds one playlist, Noice, owned by Alex, not shared, not open. Measured as admin — which is what the re-measurement above did — upstream serves three views too, and nothing differs. Measured as its owner:

request user upstream ours, before
/UserViews admin 3 views 3
/UserViews Alex 4 — Playlists at index 2 3
/Items/Counts Alex ItemCount 14570 14568
/Items?Recursive=true&SortBy=DateCreated&… Alex TotalRecordCount 21897 21895

The same delta of two. The absolute numbers moved only because the library grew since the issues were filed.

What was wrong on our side

  1. views::for_user looked for the playlists folder among the children of the user root folder. It is a child of the aggregate folder, which upstream reaches through RootFolder.VirtualChildren.
  2. A UserView holds nothing of its own, so its contents and its top parents both have to be resolved through DisplayParentId. That translation was missing.
  3. A UserView keeps its collection type in ViewType, not CollectionType.
  4. Separately: /Items/Counts ignored userId and isFavorite outright and always counted the caller — so the issue's own request could not have agreed even with the view in place.

The view's id is derived as upstream derives it, and matches byte for byte: 88c163e7f6b0a624415482fc2736f260 for Alex against the anchor's folder.

The view has to open, not just appear

The first version of the fix emitted a view that answered nothing — /Items?ParentId=<view> returned an empty page, because the translation was applied at one of four call sites and not the one that serves a view's contents. Fixing that exposed a second divergence: ?Recursive=true returned the folder row as well, since the playlists folder is its own top parent. Upstream's PlaylistsFolder.GetItemsInternal restricts to Playlist and post-filters by visibility, so it answers with neither itself nor a playlist you may not open.

The query now names the openable playlists outright, which is one rule for the page and the count alike:

request upstream ours
/Items?ParentId=<view>&userId=Alex 1 [Noice] 1 [Noice]
…&Recursive=true 1 [Noice] 1 [Noice]
/Items?ParentId=<folder> as admin 0 [] 0 []
/Items?ParentId=<folder>&userId=Alex 1 [Noice] 1 [Noice]

The third row was already divergent before any of this — we answered 1 where upstream answers 0, a leak of another user's playlist to anyone who asked the folder directly. That is closed too.

Visibility is now decided the way upstream decides it, on the path (ContainsSubPath(DataPath, Path)) rather than on the parent, so a playlist resolved out of a library follows that library's permissions instead of being treated as nobody's.

A view whose row does not exist is no longer invented: with the row absent the tile 400s, so omitting it is strictly better than advertising it. We read databases rather than adding to them.

After

All three users, upstream and ours identical: views 3 / 4 / 3, ItemCount 14568 / 14570 / 14568, TotalRecordCount 21895 / 21897 / 21895.

**These are one bug, both numbers in the issue are right, and the reason they looked unreproducible is that the measurement used the wrong account.** Fixed on `main` in `468c50d`, `ffa3436`, `05a0592` and the two commits beside them. Closing #79 and #80 together. ## Why they seemed to vanish `/Items/Counts` and a recursive `/Items` scope to the top parents of the **user's views**, so a missing view is uncounted rows: the two are the same bug, and the two missing rows *are* the playlists folder and the playlist under it. Upstream serves a playlists view only to someone who can open a playlist — `GetUserViews` gates it on `items.Any(item => item.IsVisible(user))`, and a playlist is visible to its owner, to anyone it is shared with, or if it is open access. The anchor holds one playlist, `Noice`, owned by `Alex`, not shared, not open. Measured as `admin` — which is what the re-measurement above did — upstream serves three views too, and nothing differs. Measured as its owner: | request | user | upstream | ours, before | |---|---|---|---| | `/UserViews` | admin | 3 views | 3 | | `/UserViews` | Alex | **4** — `Playlists` at index 2 | **3** | | `/Items/Counts` | Alex | `ItemCount` **14570** | **14568** | | `/Items?Recursive=true&SortBy=DateCreated&…` | Alex | `TotalRecordCount` **21897** | **21895** | The same delta of two. The absolute numbers moved only because the library grew since the issues were filed. ## What was wrong on our side 1. `views::for_user` looked for the playlists folder among the children of the user root folder. It is a child of the **aggregate** folder, which upstream reaches through `RootFolder.VirtualChildren`. 2. A `UserView` holds nothing of its own, so its contents and its top parents both have to be resolved through `DisplayParentId`. That translation was missing. 3. A `UserView` keeps its collection type in `ViewType`, not `CollectionType`. 4. Separately: `/Items/Counts` ignored `userId` and `isFavorite` outright and always counted the caller — so the issue's own request could not have agreed even with the view in place. The view's id is derived as upstream derives it, and matches byte for byte: `88c163e7f6b0a624415482fc2736f260` for `Alex` against the anchor's folder. ## The view has to open, not just appear The first version of the fix emitted a view that answered nothing — `/Items?ParentId=<view>` returned an empty page, because the translation was applied at one of four call sites and not the one that serves a view's contents. Fixing that exposed a second divergence: `?Recursive=true` returned the folder row as well, since the playlists folder is its own top parent. Upstream's `PlaylistsFolder.GetItemsInternal` restricts to `Playlist` and post-filters by visibility, so it answers with neither itself nor a playlist you may not open. The query now names the openable playlists outright, which is one rule for the page and the count alike: | request | upstream | ours | |---|---|---| | `/Items?ParentId=<view>&userId=Alex` | `1 [Noice]` | `1 [Noice]` | | `…&Recursive=true` | `1 [Noice]` | `1 [Noice]` | | `/Items?ParentId=<folder>` as admin | `0 []` | `0 []` | | `/Items?ParentId=<folder>&userId=Alex` | `1 [Noice]` | `1 [Noice]` | **The third row was already divergent before any of this** — we answered `1` where upstream answers `0`, a leak of another user's playlist to anyone who asked the folder directly. That is closed too. Visibility is now decided the way upstream decides it, on the **path** (`ContainsSubPath(DataPath, Path)`) rather than on the parent, so a playlist resolved out of a library follows that library's permissions instead of being treated as nobody's. A view whose row does not exist is no longer invented: with the row absent the tile 400s, so omitting it is strictly better than advertising it. We read databases rather than adding to them. ## After All three users, upstream and ours identical: views 3 / 4 / 3, `ItemCount` 14568 / 14570 / 14568, `TotalRecordCount` 21895 / 21897 / 21895.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
No results found.
Labels
Clear labels   
correctness
Diverges from upstream

  
encoding
Transcoding, ffmpeg, and the arg strings

  
http
Route behaviour

  
jobs
Scheduled tasks

  
operations
Installing a server and keeping one running

  
p1
Do this first

  
p2
After the p1s

  
p3
Real, and not urgent

  
performance
Speed or resource use

  
providers
Metadata from somewhere other than the file

  
scanner
The library scan

  
security
Who is allowed to see what

  
testing
Test coverage and harness

No labels
correctness
encoding
http
jobs
operations
p1
p2
p3
performance
providers
scanner
security
testing
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
alex/ephyra#80
No description provided.