Mirrored_Repos/yattee
1
0
Fork 0
mirror of https://github.com/yattee/yattee.git synced 2026-11-06 11:57:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Aim a removal at the row the server counts

Browse Source 
Remote playlist entries carried the display row number as their server
position, so a row the app could not parse shifted every removal after it
by one, and rows were paired back to entries by video id, which cannot
tell two copies of the same video apart.

- Piped entry positions are now the raw stream index; entries carry a
  separate displayRow for the row they are drawn as, filled in by both
  snapshot builders and used by the detail view's row map
- an emptied description clears a server-side playlist instead of being
  read as "leave it alone"
- a delete that reported failure forgets a playlist the refetch 404s on,
  and drops it from the account's cached list
- the browse view discards account-scoped content when a status change
  moves which account the instance resolves to
- every mutation re-reads the credential immediately before it is sent,
  which matters most after Piped's verify-before-remove fetch
- Handoff carries the owning account id, so a Piped playlist keeps its
  ownership on the receiving device
This commit is contained in:
Arkadiusz Fal
2026-09-02 23:56:59 +02:00
parent b3c4216013
commit 24b43ac866
11 changed files with 430 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
Yattee/Models/RemotePlaylistEntry.swift
View File

@@ -21,7 +21,9 @@ import Foundation
/// - ``invidious(playlistID:indexID:)`` wraps the server's `indexId` token,
/// which names the same row across refetches even as videos above it are
/// removed.
/// - ``piped(playlistID:position:)`` is **snapshot-scoped only**. Piped
/// - ``piped(playlistID:position:)`` is **snapshot-scoped only**, and the
/// position is the server's own index over every row it sent — not the row
/// number the app draws. Piped
/// removes by position, so an id minted from one fetch is meaningless once
/// the playlist changes. This is safe because the caching layer refetches
/// the whole playlist after every mutation — never persist these, and never
@@ -47,13 +49,26 @@ struct RemotePlaylistEntry: Identifiable, Hashable, Sendable {
/// The video shown for this row.
let video: Video
/// Zero-based position within the playlist as the snapshot saw it.
/// Zero-based position within the playlist **as the server counts it** —
/// over every row it sent, including the ones the app could not parse and
/// does not display. This is the number a removal is addressed by, so it
/// must never be renumbered over the filtered display list.
let position: Int
init(id: RemotePlaylistEntryID, video: Video, position: Int) {
/// Index of this entry in ``RemotePlaylistSnapshot/playlist``'s `videos`.
///
/// Separate from ``position`` on purpose: rows the app cannot display are
/// dropped from `playlist.videos` but still occupy a server position, so the
/// two numbers diverge the moment a playlist holds one. Carried explicitly
/// rather than recovered by matching video ids, which cannot tell two rows
/// of the same video apart.
let displayRow: Int
init(id: RemotePlaylistEntryID, video: Video, position: Int, displayRow: Int) {
self.id = id
self.video = video
self.position = position
self.displayRow = displayRow
}
}
@@ -61,6 +76,10 @@ struct RemotePlaylistEntry: Identifiable, Hashable, Sendable {
///
/// `entries` may be shorter than `playlist.videos`: a video the server did not
/// hand an identity to is still displayed, it just cannot be a mutation target.
/// It can also be shorter than what the server sent: a row that failed to parse
/// is neither displayed nor addressable, but it still holds a server position,
/// which is why ``RemotePlaylistEntry/position`` and
/// ``RemotePlaylistEntry/displayRow`` are two different numbers.
struct RemotePlaylistSnapshot: Hashable, Sendable {
/// The playlist for display.
let playlist: Playlist

29
Yattee/Services/API/InvidiousAPI.swift
View File

@@ -327,16 +327,27 @@ actor InvidiousAPI: InstanceAPI {
// and `index` places it. Rows missing either are still displayed —
// they stay in `playlist.videos` — they just never appear in `entries`,
// so no removal can be aimed at a row the server refuses to name.
let entries: [RemotePlaylistEntry] = allVideos.compactMap { item in
guard case .video(let video) = item,
let indexID = video.indexId,
let index = video.index
else { return nil }
//
// `displayRow` is counted over exactly what `toPlaylist` keeps, so an
// entry says which visible row it belongs to without anyone having to
// match video ids — which could not tell two rows of the same video
// apart anyway.
var displayRow = 0
var entries: [RemotePlaylistEntry] = []
for item in allVideos {
guard case .video(let video) = item else { continue }
let row = displayRow
displayRow += 1
return RemotePlaylistEntry(
id: .invidious(playlistID: playlistID, indexID: indexID),
video: video.toVideo(baseURL: baseURL),
position: index
guard let indexID = video.indexId, let index = video.index else { continue }
entries.append(
RemotePlaylistEntry(
id: .invidious(playlistID: playlistID, indexID: indexID),
video: video.toVideo(baseURL: baseURL),
position: index,
displayRow: row
)
)
}

26
Yattee/Services/API/PipedAPI.swift
View File

@@ -444,16 +444,26 @@ actor PipedAPI: InstanceAPI {
nextpage: nil
).toPlaylist(instanceURL: instanceURL, playlistID: playlistID)
// Positions run 0-based across every page in fetch order and are taken
// over the same list the playlist displays, so `entries[n]` and
// `playlist.videos[n]` are always the same row. Piped removes by
// Positions run 0-based across every page in fetch order and are counted
// over the *raw* stream list, malformed rows included: that is the index
// `POST /user/playlists/remove` addresses, and renumbering over the
// filtered display list would aim a removal one row off for every row
// the app could not parse. `displayRow` is the separate number that says
// where the entry appears in `playlist.videos`. Piped removes by
// position, so these ids die with this snapshot.
let entries = playlist.videos.enumerated().map { offset, video in
RemotePlaylistEntry(
id: .piped(playlistID: playlistID, position: offset),
video: video,
position: offset
var displayRow = 0
var entries: [RemotePlaylistEntry] = []
for (position, item) in allStreams.enumerated() {
guard case .video(let video) = item else { continue }
entries.append(
RemotePlaylistEntry(
id: .piped(playlistID: playlistID, position: position),
video: video.toVideo(instanceURL: instanceURL),
position: position,
displayRow: displayRow
)
)
displayRow += 1
}
return RemotePlaylistSnapshot(playlist: playlist, entries: entries)

23
Yattee/Services/AccountService.swift
View File

@@ -146,6 +146,7 @@ final class AccountService {
let (instance, credential) = try resolve(account, requiring: \.supportsPlaylistWrites)
let privacy = supportedPrivacy(privacy, for: account)
try requireStillCurrent(credential, for: account)
return try await tracking(account, credential: credential) {
try await transport.createPlaylist(
title: title,
@@ -168,6 +169,7 @@ final class AccountService {
let (instance, credential) = try resolve(account, requiring: \.supportsPlaylistWrites)
let privacy = supportedPrivacy(privacy, for: account)
try requireStillCurrent(credential, for: account)
try await tracking(account, credential: credential) {
try await transport.updatePlaylist(
id: id,
@@ -184,6 +186,7 @@ final class AccountService {
func deletePlaylist(id: String, for account: Account) async throws {
let (instance, credential) = try resolve(account, requiring: \.supportsPlaylistWrites)
try requireStillCurrent(credential, for: account)
try await tracking(account, credential: credential) {
try await transport.deletePlaylist(id: id, instance: instance, credential: credential)
}
@@ -193,6 +196,7 @@ final class AccountService {
func addVideo(videoID: String, toPlaylist id: String, for account: Account) async throws {
let (instance, credential) = try resolve(account, requiring: \.supportsPlaylistWrites)
try requireStillCurrent(credential, for: account)
try await tracking(account, credential: credential) {
try await transport.addVideoToPlaylist(
playlistID: id,
@@ -212,6 +216,10 @@ final class AccountService {
/// and the removal the playlist can still change, and only the caller's
/// refetch afterwards can show what actually happened. Invidious needs none
/// of this: its `indexId` names the row itself, whatever moved around it.
///
/// The preflight is also where the credential can change out from under the
/// removal — it is the one place here that awaits a server before writing —
/// so the credential is read again before the removal is sent.
func removeEntry(
_ entry: RemotePlaylistEntry,
fromPlaylist id: String,
@@ -237,6 +245,7 @@ final class AccountService {
}
}
try requireStillCurrent(credential, for: account)
try await tracking(account, credential: credential) {
try await transport.removeEntry(entry.id, instance: instance, credential: credential)
}
@@ -274,6 +283,20 @@ final class AccountService {
return (instance, credential)
}
/// Refuses to send a write with a credential that is no longer the
/// account's.
///
/// Resolving a credential and sending the request are not the same instant:
/// a sign-out and a sign-in as somebody else can both happen in between —
/// Piped's removals in particular await a whole playlist fetch first. A
/// stale *read* is merely wasted; a stale *write* changes the previous
/// user's data, so every mutation checks again immediately before it goes.
private func requireStillCurrent(_ credential: String, for account: Account) throws {
guard credentialReader(account) == credential else {
throw AccountServiceError.notAuthenticated
}
}
/// The privacy to send, or `nil` where the backend has no such setting.
private func supportedPrivacy(_ privacy: PlaylistPrivacy?, for account: Account) -> PlaylistPrivacy? {
guard accountsManager.capabilities(for: account)?.supportsPlaylistPrivacy == true else { return nil }

35
Yattee/Services/Handoff/HandoffManager.swift
View File

@@ -32,6 +32,7 @@ final class HandoffManager {
static let channelSource = "channelSource"
static let playlistID = "playlistID"
static let playlistSource = "playlistSource"
static let playlistAccountID = "playlistAccountID"
static let localPlaylistUUID = "localPlaylistUUID"
static let searchQuery = "searchQuery"
static let playbackTime = "playbackTime"
@@ -164,18 +165,23 @@ final class HandoffManager {
userInfo[UserInfoKey.destinationType] = DestinationType.localPlaylist.rawValue
userInfo[UserInfoKey.localPlaylistUUID] = uuid.uuidString
activity.title = "Playlist"
// The owning account is deliberately left out of the payload. The
// userInfo is assembled key by key rather than encoded from the
// enum, so an extra associated value changes nothing about the
// coding shape either way — but an account is only usable on a
// device that holds its credential, and the receiving device
// resolves ownership for itself from the instance it lands on.
case .remote(let playlistID, _, _, _):
// The owning account travels with the playlist. Account ids are the
// same on every device the login is synced to, so the receiver can
// resolve one — and without it a Piped playlist arrives as nobody's:
// its id carries no hint of ownership the way an Invidious `IVPL`
// one does, so a private playlist would be loaded through the public
// endpoint and fail. A receiver that does not know the account is no
// worse off than before: the key is simply absent, which is also
// what an older payload looks like.
case .remote(let playlistID, _, _, let accountID):
userInfo[UserInfoKey.destinationType] = DestinationType.playlist.rawValue
userInfo[UserInfoKey.playlistID] = playlistID.playlistID
if let contentSource = playlistID.source {
userInfo[UserInfoKey.playlistSource] = encodeSource(contentSource)
}
if let accountID {
userInfo[UserInfoKey.playlistAccountID] = accountID.uuidString
}
activity.title = "Playlist"
}
@@ -312,7 +318,20 @@ final class HandoffManager {
} else {
source = nil
}
return (.playlist(.remote(PlaylistID(source: source, playlistID: playlistID), instance: nil)), nil)
// An account this device does not have resolves to nil in the
// detail view, which falls back to the ownership it can infer —
// exactly what a payload without the key does.
let accountID = (userInfo[UserInfoKey.playlistAccountID] as? String).flatMap(UUID.init(uuidString:))
return (
.playlist(
.remote(
PlaylistID(source: source, playlistID: playlistID),
instance: nil,
accountID: accountID
)
),
nil
)
case .localPlaylist:
guard let uuidString = userInfo[UserInfoKey.localPlaylistUUID] as? String,

44
Yattee/Services/RemotePlaylistsCache.swift
View File

@@ -163,8 +163,9 @@ final class RemotePlaylistsCache {
} catch {
// The delete may have landed even though the answer did not, so the
// snapshot is refetched rather than assumed: whichever way it went,
// the server's answer replaces it.
await reconcileSnapshot(id: id, for: account)
// the server's answer replaces it — and a server that no longer has
// the playlist is answering that the delete did land.
await reconcileSnapshotAfterDelete(id: id, for: account)
await reconcileList(for: account)
throw error
}
@@ -334,11 +335,50 @@ final class RemotePlaylistsCache {
try? await fetchSnapshot(id: id, for: account)
}
/// The same refetch, after a delete that reported failure — where the
/// refetch is also the verdict.
///
/// A playlist the server says it does not have is gone, whatever the delete
/// said, so the stored snapshot is dropped rather than left to be shown by
/// a detail view that is still open on it. Every other failure is swallowed
/// like any reconciling refetch: not reaching the server says nothing about
/// whether the playlist still exists.
private func reconcileSnapshotAfterDelete(id: String, for account: Account) async {
do {
try await fetchSnapshot(id: id, for: account)
} catch let error as APIError where Self.saysPlaylistIsGone(error) {
forgetSnapshot(id: id, for: account.id)
forgetListEntry(id: id, for: account.id)
} catch {
// Swallowed: see the doc comment on `reconcileList(for:)`.
}
}
private static func saysPlaylistIsGone(_ error: APIError) -> Bool {
switch error {
case .notFound:
return true
case .httpError(let statusCode, _):
return statusCode == 404
default:
return false
}
}
private func forgetSnapshot(id: String, for accountID: UUID) {
snapshots[accountID]?[id] = nil
snapshotFetchDates[accountID]?[id] = nil
}
/// Takes a playlist out of the account's last-fetched list.
///
/// The list is refetched right after, which normally says the same thing —
/// but a refetch that cannot reach the server would otherwise leave a
/// playlist that is known to be gone sitting in it.
private func forgetListEntry(id: String, for accountID: UUID) {
lists[accountID]?.removeAll { $0.id.playlistID == id }
}
// MARK: - Fences
private func isCurrent(_ generation: Int, for accountID: UUID) -> Bool {

34
Yattee/Views/Instances/InstanceBrowseView.swift
View File

@@ -29,6 +29,9 @@ struct InstanceBrowseView: View {
/// Which login the account-scoped content belongs to, so a page still in
/// flight cannot be appended to the next one's feed.
@State private var accountContentGeneration = 0
/// Which account the displayed — or in-flight — account-scoped content is
/// for, so a change of effective account can be noticed at all.
@State private var accountContentAccountID: UUID?
@State private var feedVideos: [Video] = []
@State private var feedSubscriptions: [Channel] = []
@@ -443,9 +446,16 @@ struct InstanceBrowseView: View {
}
// A credential can arrive over iCloud Keychain, or an account be added,
// while this stays on screen — the Feed tab depends on it and would
// otherwise stay missing until the view was rebuilt.
// otherwise stay missing until the view was rebuilt. A status change can
// also move *which* account is effective here, since the resolution
// falls back to a sibling login on the same instance, so the content
// loaded for the previous one has to go with it.
.onReceive(NotificationCenter.default.publisher(for: .accountStatusesDidChange)) { _ in
let accountChanged = discardAccountScopedContentIfAccountChanged()
refreshLoginState()
if accountChanged {
Task { await startContentLoad() }
}
}
.onReceive(NotificationCenter.default.publisher(for: .accountsDidChange)) { _ in
refreshLoginState()
@@ -1228,6 +1238,21 @@ struct InstanceBrowseView: View {
// would have to exceed a page it never loaded before it could ask for
// a second.
feedLoadedVideoCount = 0
accountContentAccountID = nil
}
/// Drops account-scoped content when the account it belongs to is no longer
/// the one this instance resolves to.
///
/// - Returns: whether anything was discarded, so the caller can reload.
@discardableResult
private func discardAccountScopedContentIfAccountChanged() -> Bool {
guard let loadedFor = accountContentAccountID, loadedFor != playlistsAccount?.id else {
return false
}
discardAccountScopedContent()
return true
}
private func refreshLoginState() {
@@ -1280,6 +1305,13 @@ struct InstanceBrowseView: View {
// are unauthenticated and deliberately not fenced.
let generation = accountContentGeneration
// Recorded before the request goes out, not after it lands: an account
// change while the fetch is in flight has to be able to see whose fetch
// it was.
if selectedTab == .feed || selectedTab == .playlists {
accountContentAccountID = playlistsAccount?.id
}
do {
switch selectedTab {
case .popular:

17
Yattee/Views/Playlist/PlaylistFormSheet.swift
View File

@@ -299,9 +299,24 @@ struct PlaylistFormSheet: View {
let trimmedTitle = title.trimmingCharacters(in: .whitespacesAndNewlines)
let trimmedDescription = descriptionText.trimmingCharacters(in: .whitespacesAndNewlines)
onSave(trimmedTitle, trimmedDescription.isEmpty ? nil : trimmedDescription, privacy)
onSave(trimmedTitle, savedDescription(trimmedDescription), privacy)
dismiss()
}
/// What an emptied description field means to the caller.
///
/// Editing a server-side playlist is the one case where the difference
/// matters: `nil` means "leave whatever is there" all the way down to the
/// write layer, so an emptied field has to travel as `""` or clearing a
/// description would silently keep it. Everywhere else — creating anything,
/// editing a local playlist — an empty description *is* no description.
private func savedDescription(_ trimmed: String) -> String? {
guard !trimmed.isEmpty else {
if case .editRemote = mode { return "" }
return nil
}
return trimmed
}
}
// MARK: - Preview

23
Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift
View File

@@ -945,23 +945,18 @@ struct UnifiedPlaylistDetailView: View {
/// Pairs each displayed row with the entry that can name it to the server.
///
/// `entries` is a *subsequence* of the playlist's videos — a row the server
/// refused to hand an identity to is still displayed, it just cannot be a
/// target — so the two lists are walked together rather than indexed
/// against each other. Matching on the video id alone would not do either:
/// a playlist may hold the same video twice, and each copy is its own row.
/// The correspondence is the one the snapshot builder recorded, not one
/// recovered here: `entries` is a *subsequence* of the playlist's videos —
/// a row the server refused to hand an identity to is still displayed, it
/// just cannot be a target — and matching the two lists back up by video id
/// cannot tell two rows of the same video apart, so a duplicate row without
/// an identity would steal its twin's entry and remove the wrong one.
private static func rowEntries(for snapshot: RemotePlaylistSnapshot) -> [Int: RemotePlaylistEntry] {
let videos = snapshot.playlist.videos
let rowCount = snapshot.playlist.videos.count
var map: [Int: RemotePlaylistEntry] = [:]
var row = 0
for entry in snapshot.entries {
while row < videos.count, videos[row].id != entry.video.id {
row += 1
}
guard row < videos.count else { break }
map[row] = entry
row += 1
for entry in snapshot.entries where entry.displayRow >= 0 && entry.displayRow < rowCount {
map[entry.displayRow] = entry
}
return map