mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 22:03:13 -06:00
Merge fix/youtube-never-advances: YouTube items advance, playlists can be cleared, per-display beta opt-in
This commit is contained in:
ScreenTinker
commit
2bf8b4271f
|
|
@ -111,7 +111,7 @@ class PlaylistController(
|
|||
val item = currentItem ?: return
|
||||
val delay = FollowerExit.resumeDelayMs(
|
||||
isRunning = isRunning,
|
||||
isImageOrWidget = item.mimeType.startsWith("image/") || item.isWidget,
|
||||
isImageOrWidget = endsOnTimer(item),
|
||||
slotMs = slotMs(item),
|
||||
elapsedMs = System.currentTimeMillis() - itemStartedAt
|
||||
) ?: return
|
||||
|
|
@ -220,8 +220,17 @@ class PlaylistController(
|
|||
// In solo playback, don't interrupt it: keep it up, stash the new list, and rotate out on the
|
||||
// next natural advance (video end / image duration). Excludes wallFollower + group-sync, whose
|
||||
// advance is driven by their tick, not next() — deferring there would strand the swap.
|
||||
if (isRunning && !wallFollower && hasContentOnScreen && currentlyPlayingId != null &&
|
||||
newItems.none { it.contentId == currentlyPlayingId }) {
|
||||
// An EMPTY new list is never a deferral candidate. Clearing a screen's playlist is an
|
||||
// explicit "stop showing that" from an operator, not an item rotating out — deferring it
|
||||
// meant selecting "no playlist" left the old content up indefinitely, which is the opposite
|
||||
// of what was asked for and looked like the setting had done nothing.
|
||||
if (PendingSwap.shouldDefer(
|
||||
isRunning = isRunning,
|
||||
wallFollower = wallFollower,
|
||||
hasContentOnScreen = hasContentOnScreen,
|
||||
currentlyPlayingId = currentlyPlayingId,
|
||||
newContentIds = newItems.map { it.contentId },
|
||||
)) {
|
||||
var succ: String? = null
|
||||
if (items.isNotEmpty()) {
|
||||
for (k in 1..items.size) {
|
||||
|
|
@ -232,11 +241,13 @@ class PlaylistController(
|
|||
pendingItems = newItems
|
||||
pendingSuccessorId = succ
|
||||
Log.i("PlaylistController", "Current item removed but still live — deferring rotation-out (successor=$succ)")
|
||||
armPendingSwapDeadline()
|
||||
return
|
||||
}
|
||||
// A non-deferred structural update supersedes any pending swap.
|
||||
pendingItems = null
|
||||
pendingSuccessorId = null
|
||||
cancelPendingSwapDeadline()
|
||||
|
||||
items.clear()
|
||||
items.addAll(newItems)
|
||||
|
|
@ -330,6 +341,7 @@ class PlaylistController(
|
|||
isRunning = false
|
||||
cancelAdvance()
|
||||
cancelRetry()
|
||||
cancelPendingSwapDeadline() // else a stopped controller can still fire next()
|
||||
hasContentOnScreen = false
|
||||
pendingItems = null
|
||||
pendingSuccessorId = null
|
||||
|
|
@ -343,6 +355,7 @@ class PlaylistController(
|
|||
// Swap in the stashed list now and continue at the preserved successor (or first playable).
|
||||
pendingItems?.let { p ->
|
||||
pendingItems = null
|
||||
cancelPendingSwapDeadline()
|
||||
val succ = pendingSuccessorId; pendingSuccessorId = null
|
||||
items.clear(); items.addAll(p)
|
||||
if (items.isEmpty()) { currentIndex = -1; cancelAdvance(); onPlaylistEmpty(); return }
|
||||
|
|
@ -373,6 +386,42 @@ class PlaylistController(
|
|||
next()
|
||||
}
|
||||
|
||||
/**
|
||||
* Items whose turn ends on a TIMER rather than a completion callback.
|
||||
*
|
||||
* video/youtube belongs here and did not: it is played by loading an embed into a WebView, which
|
||||
* fires no completion event, so nothing ever advanced past it. playYoutube() even takes a
|
||||
* durationSec and never reads it. A playlist containing a YouTube item simply stopped there.
|
||||
*
|
||||
* That also stranded #157's deferred swap, which waits for "the next natural advance": assigning
|
||||
* a different playlist while a YouTube item was on screen deferred forever, so the change looked
|
||||
* like it had been ignored. Reported as "I assigned Playlist 2 and it kept showing the video".
|
||||
*
|
||||
* Local and remote non-YouTube video stay off this list — ExoPlayer reports STATE_ENDED and
|
||||
* onVideoComplete drives those, and a timer would cut a clip short.
|
||||
*/
|
||||
private var pendingSwapRunnable: Runnable? = null
|
||||
|
||||
/** Apply a deferred swap even if no advance arrives — see PENDING_SWAP_DEADLINE_MS. */
|
||||
private fun armPendingSwapDeadline() {
|
||||
cancelPendingSwapDeadline()
|
||||
pendingSwapRunnable = Runnable {
|
||||
if (pendingItems != null) {
|
||||
Log.w("PlaylistController", "Deferred playlist swap never got an advance — applying it now")
|
||||
next()
|
||||
}
|
||||
}
|
||||
handler.postDelayed(pendingSwapRunnable!!, PendingSwap.DEADLINE_MS)
|
||||
}
|
||||
|
||||
private fun cancelPendingSwapDeadline() {
|
||||
pendingSwapRunnable?.let { handler.removeCallbacks(it) }
|
||||
pendingSwapRunnable = null
|
||||
}
|
||||
|
||||
private fun endsOnTimer(item: PlaylistItem): Boolean =
|
||||
ItemTiming.endsOnTimer(item.mimeType, item.isWidget)
|
||||
|
||||
private fun playCurrentItem() {
|
||||
cancelAdvance()
|
||||
cancelRetry()
|
||||
|
|
@ -395,7 +444,7 @@ class PlaylistController(
|
|||
// For images and widgets, auto-advance after duration. For videos, wait
|
||||
// for the completion callback. Wall followers never auto-advance — the
|
||||
// leader's wall:sync index drives every switch.
|
||||
if (!wallFollower && (item.mimeType.startsWith("image/") || item.isWidget)) {
|
||||
if (!wallFollower && endsOnTimer(item)) {
|
||||
// slotMs() floors a zero/negative duration to 10s (the max(1, duration||10)
|
||||
// contract shared with the web/Tizen players). A raw durationSec*1000 here let a
|
||||
// solo fullscreen widget with duration_sec=0 schedule a 0ms advance -> self-loop.
|
||||
|
|
|
|||
|
|
@ -76,3 +76,62 @@ object PlaybackResume {
|
|||
return savedIndex
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* #157's deferral: when a playlist update drops the item that is CURRENTLY on screen, we let that
|
||||
* item finish its turn instead of yanking it, and apply the new list at the next natural advance.
|
||||
*
|
||||
* The rule needs two guards it did not have, both found from a customer report where a playlist
|
||||
* change appeared to be ignored entirely:
|
||||
*
|
||||
* 1. An EMPTY new list is not a rotation. Clearing a screen's playlist is an operator saying "stop
|
||||
* showing that", so it must take effect now. Deferring it left the old content up forever.
|
||||
* 2. Deferring assumes an advance is coming. A YouTube item never advanced (see endsOnTimer), so
|
||||
* the pending swap was stranded permanently — the caller must pair this with a deadline.
|
||||
*
|
||||
* Pure so the rule can be checked without a device or a WebView.
|
||||
*/
|
||||
object PendingSwap {
|
||||
/**
|
||||
* How long a deferred swap may wait for "the next natural advance" before it is applied anyway.
|
||||
* The deferral assumes an advance is coming; YouTube proved it might not be, and any future item
|
||||
* type that ends on a callback could do the same. Must comfortably clear an ordinary dwell so it
|
||||
* never pre-empts a normal rotation, while still being short enough that an operator watching
|
||||
* the screen sees their change land.
|
||||
*/
|
||||
const val DEADLINE_MS = 60_000L
|
||||
|
||||
/**
|
||||
* Whether a playlist update should wait for the current item to finish.
|
||||
* False means apply it immediately.
|
||||
*/
|
||||
fun shouldDefer(
|
||||
isRunning: Boolean,
|
||||
wallFollower: Boolean,
|
||||
hasContentOnScreen: Boolean,
|
||||
currentlyPlayingId: String?,
|
||||
newContentIds: List<String>,
|
||||
): Boolean {
|
||||
if (!isRunning || wallFollower || !hasContentOnScreen) return false
|
||||
if (currentlyPlayingId == null) return false
|
||||
if (newContentIds.isEmpty()) return false // guard 1: an explicit stop
|
||||
return !newContentIds.contains(currentlyPlayingId)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Which items end on a TIMER versus a completion callback.
|
||||
*
|
||||
* video/youtube was in neither camp and so ended on nothing at all: it is played by loading an embed
|
||||
* into a WebView, which reports no completion, and no advance was ever armed for it. The item's
|
||||
* configured duration was passed to the player and dropped on the floor. A playlist containing a
|
||||
* YouTube item simply stopped there for good, and any pending playlist change stopped with it.
|
||||
*
|
||||
* Local and remote video deliberately stay OFF the timer path — the player reports STATE_ENDED for
|
||||
* those and a timer would cut a clip short at its configured duration.
|
||||
*/
|
||||
object ItemTiming {
|
||||
fun endsOnTimer(mimeType: String, isWidget: Boolean): Boolean =
|
||||
mimeType.startsWith("image/") || isWidget || mimeType == "video/youtube"
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,98 @@
|
|||
package com.remotedisplay.player.player
|
||||
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* A customer assigned a different playlist to a screen and the screen kept showing the old content.
|
||||
* Then they selected "no playlist" — still the old content. Restarting the app showed the new
|
||||
* content instantly, which ruled out downloads, the network and the server payload.
|
||||
*
|
||||
* Two faults met. #157's deferral holds a playlist change until the current item finishes its turn,
|
||||
* and the item on screen was a YouTube video, which never finished: nothing armed an advance for it,
|
||||
* so the pending change waited for an event that could not arrive. And "no playlist" went down the
|
||||
* same deferral path, so the one action that should always take effect immediately did not.
|
||||
*
|
||||
* Invariants pinned here:
|
||||
* - an empty new list is applied at once, never deferred
|
||||
* - a real rotation still defers, because #157's reason for existing has not changed
|
||||
* - an item that ends on a timer is recognised as such, YouTube included
|
||||
*/
|
||||
class PendingSwapTest {
|
||||
|
||||
private val LIVE = "content-on-screen"
|
||||
|
||||
private fun defer(
|
||||
newIds: List<String>,
|
||||
current: String? = LIVE,
|
||||
isRunning: Boolean = true,
|
||||
wallFollower: Boolean = false,
|
||||
hasContent: Boolean = true,
|
||||
) = PendingSwap.shouldDefer(isRunning, wallFollower, hasContent, current, newIds)
|
||||
|
||||
@Test fun THE_BUG_selecting_no_playlist_must_not_be_deferred() {
|
||||
// The decisive observation from the report: "I selected No playlist ... it still showed the
|
||||
// same video." An empty list is an operator saying stop, not an item rotating out.
|
||||
assertFalse(defer(newIds = emptyList()))
|
||||
}
|
||||
|
||||
@Test fun a_genuine_rotation_still_defers_157_must_not_regress() {
|
||||
// The current item is gone from the new list but other items remain: let it finish.
|
||||
assertTrue(defer(newIds = listOf("other-a", "other-b")))
|
||||
}
|
||||
|
||||
@Test fun a_playlist_that_still_contains_the_live_item_never_defers() {
|
||||
assertFalse(defer(newIds = listOf(LIVE, "other-a")))
|
||||
}
|
||||
|
||||
@Test fun nothing_on_screen_yet_means_apply_immediately() {
|
||||
// A first load has nothing to protect, so there is nothing to wait for.
|
||||
assertFalse(defer(newIds = listOf("other-a"), hasContent = false))
|
||||
assertFalse(defer(newIds = listOf("other-a"), current = null))
|
||||
}
|
||||
|
||||
@Test fun a_stopped_controller_does_not_defer() {
|
||||
// Otherwise a swap is parked on an instance that will never advance again.
|
||||
assertFalse(defer(newIds = listOf("other-a"), isRunning = false))
|
||||
}
|
||||
|
||||
@Test fun a_wall_follower_does_not_defer_it_obeys_the_leader() {
|
||||
assertFalse(defer(newIds = listOf("other-a"), wallFollower = true))
|
||||
}
|
||||
|
||||
@Test fun the_deferral_deadline_is_long_enough_for_a_normal_item_and_short_enough_to_notice() {
|
||||
// The deadline is the backstop for "no advance ever arrives". It must clear a typical dwell
|
||||
// comfortably (or it would cut ordinary items short) while still resolving fast enough that
|
||||
// an operator watching the screen sees their change land.
|
||||
val deadline = PendingSwap.DEADLINE_MS
|
||||
assertTrue("deadline must exceed a common 30s dwell", deadline > 30_000L)
|
||||
assertTrue("an operator should not wait minutes", deadline <= 120_000L)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The other half of the same report. A YouTube item ended on nothing: no timer was armed for it and
|
||||
* a WebView embed reports no completion, so it held the screen forever and stranded whatever
|
||||
* playlist change was waiting behind it.
|
||||
*/
|
||||
class ItemTimingTest {
|
||||
|
||||
@Test fun THE_BUG_a_youtube_item_must_end_on_a_timer() {
|
||||
// Nothing else can end it — a WebView embed fires no completion event.
|
||||
assertTrue(ItemTiming.endsOnTimer("video/youtube", isWidget = false))
|
||||
}
|
||||
|
||||
@Test fun images_and_widgets_are_timed_as_they_always_were() {
|
||||
assertTrue(ItemTiming.endsOnTimer("image/jpeg", isWidget = false))
|
||||
assertTrue(ItemTiming.endsOnTimer("image/png", isWidget = false))
|
||||
assertTrue(ItemTiming.endsOnTimer("text/html", isWidget = true))
|
||||
}
|
||||
|
||||
@Test fun real_video_must_NOT_be_timed_or_clips_get_cut_short() {
|
||||
// These end on STATE_ENDED. Arming a timer would truncate a clip at its configured duration,
|
||||
// which is the regression to avoid while fixing the YouTube case.
|
||||
assertFalse(ItemTiming.endsOnTimer("video/mp4", isWidget = false))
|
||||
assertFalse(ItemTiming.endsOnTimer("video/webm", isWidget = false))
|
||||
}
|
||||
}
|
||||
|
|
@ -193,6 +193,7 @@ export const api = {
|
|||
getItemSchedules: (id, itemId) => request(`/playlists/${id}/items/${itemId}/schedules`),
|
||||
setItemSchedules: (id, itemId, blocks) => request(`/playlists/${id}/items/${itemId}/schedules`, { method: 'PUT', body: JSON.stringify({ blocks }) }),
|
||||
assignPlaylistToDevice: (playlistId, device_id) => request(`/playlists/${playlistId}/assign`, { method: 'POST', body: JSON.stringify({ device_id }) }),
|
||||
clearDevicePlaylist: (device_id) => request(`/devices/${device_id}/playlist`, { method: 'DELETE' }),
|
||||
publishPlaylist: (id) => request(`/playlists/${id}/publish`, { method: 'POST' }),
|
||||
discardPlaylistDraft: (id) => request(`/playlists/${id}/discard`, { method: 'POST' }),
|
||||
|
||||
|
|
|
|||
|
|
@ -524,6 +524,8 @@ export default {
|
|||
'device.debug.toggle': 'Debug logging (live)',
|
||||
'device.debug.hint': 'Streams player/zone logs from this device in real time. Turns off on its own when the device reconnects.',
|
||||
'device.ota.toggle': 'Self-update (OTA)',
|
||||
'device.ota.beta': 'Accept pre-release builds',
|
||||
'device.ota.beta_hint': 'Keeps this display on a test build instead of updating it back to the current release. Only affects pre-releases of the version already installed — once a newer release ships, this display updates to it normally.',
|
||||
'device.ota.hint': 'When off, this device is never offered an update — an MDM or operator owns its updates instead. Turn OFF for MDM-managed panels (e.g. Pivot/MAXHUB) so the app never shows a self-install dialog.',
|
||||
'device.reboot_schedule.label': 'Nightly reboot',
|
||||
'device.reboot_schedule.hint': 'Reboot this panel once a day at this device-local time (leave blank for off). A clean nightly reboot clears memory leaks and re-syncs the clock. Silent on device-owner panels; a no-op on panels that can\'t self-reboot.',
|
||||
|
|
|
|||
|
|
@ -446,6 +446,10 @@ async function loadDevice(deviceId, activeTab = null) {
|
|||
<input type="checkbox" id="otaToggle" ${device.ota_enabled === 0 ? '' : 'checked'}> ${t('device.ota.toggle')}
|
||||
</label>
|
||||
<div style="font-size:11px;color:var(--text-muted);margin:4px 0 0 24px">${t('device.ota.hint')}</div>
|
||||
<label style="display:flex;align-items:center;gap:8px;cursor:pointer;font-size:13px;margin-top:8px">
|
||||
<input type="checkbox" id="otaBetaToggle" ${device.ota_beta === 1 ? 'checked' : ''}> ${t('device.ota.beta')}
|
||||
</label>
|
||||
<div style="font-size:11px;color:var(--text-muted);margin:4px 0 0 24px">${t('device.ota.beta_hint')}</div>
|
||||
</div>
|
||||
<div class="form-group" style="max-width:280px">
|
||||
<label>${t('device.reboot_schedule.label')}</label>
|
||||
|
|
@ -977,6 +981,7 @@ function setupActions(device) {
|
|||
orientation: document.getElementById('deviceOrientation').value,
|
||||
default_content_id: document.getElementById('deviceDefaultContent').value || null,
|
||||
ota_enabled: document.getElementById('otaToggle')?.checked ? 1 : 0,
|
||||
ota_beta: document.getElementById('otaBetaToggle')?.checked ? 1 : 0,
|
||||
reboot_schedule: document.getElementById('rebootSchedule')?.value || null,
|
||||
});
|
||||
showToast(t('device.toast.settings_saved'), 'success');
|
||||
|
|
@ -1039,10 +1044,15 @@ function setupActions(device) {
|
|||
|
||||
playlistPicker.addEventListener('change', async () => {
|
||||
const newPlaylistId = playlistPicker.value;
|
||||
if (!newPlaylistId) return; // Don't allow deselecting for now
|
||||
try {
|
||||
await api.assignPlaylistToDevice(newPlaylistId, device.id);
|
||||
device.playlist_id = newPlaylistId;
|
||||
// Empty value is the "No playlist" option. It used to be discarded right here, so the
|
||||
// option was offered, selecting it did nothing, and nothing said so (#234).
|
||||
if (newPlaylistId) {
|
||||
await api.assignPlaylistToDevice(newPlaylistId, device.id);
|
||||
} else {
|
||||
await api.clearDevicePlaylist(device.id);
|
||||
}
|
||||
device.playlist_id = newPlaylistId || null;
|
||||
const assignments = await api.getAssignments(device.id);
|
||||
const pc = document.getElementById('playlistContainer');
|
||||
pc.innerHTML = renderPlaylist(assignments);
|
||||
|
|
|
|||
|
|
@ -299,6 +299,12 @@ const migrations = [
|
|||
// device an update (an MDM/operator owns its updates). Default 1 (self-update on).
|
||||
// UPDATE devices SET ota_enabled = 0 WHERE id = '<device_id>'; (1 to re-enable)
|
||||
"ALTER TABLE devices ADD COLUMN ota_enabled INTEGER NOT NULL DEFAULT 1",
|
||||
// Opt a single display into pre-release builds. Without this, handing someone a test build is a
|
||||
// trap: a prerelease sorts BELOW its own release (1.9.25-fix234d < 1.9.25), so the next OTA check
|
||||
// correctly "upgrades" the device straight back off the build you asked them to test — silently,
|
||||
// within minutes. It cost a reporter on #234 an evening of testing code that had already been
|
||||
// replaced under them. Set this and the display keeps a same-core prerelease.
|
||||
"ALTER TABLE devices ADD COLUMN ota_beta INTEGER NOT NULL DEFAULT 0",
|
||||
// #161: privilege tier reported by the player (0 unprivileged / 1 device-admin / 2 owner-or-
|
||||
// delegated-install) + whether a foreign device owner (MDM) manages it. Drives dashboard gating
|
||||
// of Tier-2 controls (reboot/kiosk/time) — shown only for owned panels.
|
||||
|
|
|
|||
|
|
@ -68,7 +68,7 @@ function isReleased(p) { return p.pre === null || /^patch\d+$/i.test(p.pre); }
|
|||
|
||||
// decide(clientVersion, latestVersion, deviceId?, now?) ->
|
||||
// { update_available, reason, retry_after_seconds?, log? }
|
||||
function decide(clientVersion, latestVersion, deviceId = null, now = Date.now()) {
|
||||
function decide(clientVersion, latestVersion, deviceId = null, now = Date.now(), betaChannel = false) {
|
||||
// ---- PHANTOM / unrecognized guard (immediate, version-based, no rate state) ----
|
||||
if (!clientVersion) return { update_available: false, reason: 'no-version' };
|
||||
const pc = parseVer(clientVersion), pl = parseVer(latestVersion);
|
||||
|
|
@ -76,10 +76,24 @@ function decide(clientVersion, latestVersion, deviceId = null, now = Date.now())
|
|||
const full = cmpParsed(pc, pl);
|
||||
if (full === 0) return { update_available: false, reason: 'up-to-date' };
|
||||
if (full > 0) return { update_available: false, reason: 'client-newer' }; // never offer a downgrade
|
||||
if (!isReleased(pc) && coreCmp(pc, pl) < 0) { // GENUINE superseded old-core prerelease (e.g. 1.9.1-beta4) — a -patchN release is NOT one, so it still gets offered
|
||||
// betaChannel is exempt: this guard would otherwise strand the very displays we hand test
|
||||
// builds to. A tester on 1.9.25-fix234d has an older core than a released 1.9.26, so without
|
||||
// the exemption they are told "superseded" forever and never rejoin the release line — the
|
||||
// opposite of what opting in should mean. Opting in must be reversible by shipping a release.
|
||||
if (!betaChannel && !isReleased(pc) && coreCmp(pc, pl) < 0) { // GENUINE superseded old-core prerelease (e.g. 1.9.1-beta4) — a -patchN release is NOT one, so it still gets offered
|
||||
return { update_available: false, reason: 'superseded-prerelease', log: logOnce(clientVersion, `[ota] superseded prerelease '${clientVersion}' (older core than latest=${latestVersion}) — no offer`) };
|
||||
}
|
||||
|
||||
// A display opted into pre-release builds keeps a prerelease of the CURRENT core. Semver puts
|
||||
// 1.9.25-fix234d below 1.9.25, so without this the only "upgrade" on offer is dropping the very
|
||||
// build we asked this display to run — which is how a test build silently reverts. Scoped to the
|
||||
// same core on purpose: an older-core prerelease is genuinely stale and still gets offered, and
|
||||
// once 1.9.26 ships a 1.9.25-anything device is behind and updates normally. So opting in cannot
|
||||
// strand a display on an abandoned branch.
|
||||
if (betaChannel && !isReleased(pc) && coreCmp(pc, pl) === 0) {
|
||||
return { update_available: false, reason: 'beta-channel' };
|
||||
}
|
||||
|
||||
// ---- offerable (recent real older version) -> RATE breaker, keyed per device / per version ----
|
||||
const key = deviceId ? 'd:' + deviceId : 'v:' + clientVersion;
|
||||
let b = state.get(key);
|
||||
|
|
|
|||
|
|
@ -215,11 +215,44 @@ router.get('/:id/preview-payload', (req, res) => {
|
|||
});
|
||||
|
||||
// Update device
|
||||
// Clear a device's playlist — the "No playlist" option in the dashboard picker.
|
||||
//
|
||||
// There was no way to do this. PUT /devices/:id ignores playlist_id (it always has), and
|
||||
// POST /playlists/:id/assign can only ever SET one, so the picker carried a guard that
|
||||
// silently discarded the selection: `if (!newPlaylistId) return; // Don't allow deselecting`.
|
||||
// The option was offered, selecting it did nothing, and no error said so — reported on #234
|
||||
// as "I selected No playlist and it still showed the same video". It did.
|
||||
//
|
||||
// Device-scoped rather than playlist-scoped because there is no playlist to authorize
|
||||
// against when clearing; ownership is checked the same way every other device mutation
|
||||
// checks it. Clearing an already-clear device is a no-op success, so the button is safe to
|
||||
// press twice.
|
||||
router.delete('/:id/playlist', (req, res) => {
|
||||
const device = checkDeviceOwnership(req, res);
|
||||
if (!device) return;
|
||||
|
||||
db.prepare('UPDATE devices SET playlist_id = NULL, updated_at = ? WHERE id = ?')
|
||||
.run(Math.floor(Date.now() / 1000), req.params.id);
|
||||
|
||||
// Push the now-empty playlist so the screen stops, rather than leaving the old content up
|
||||
// until something else happens to update it.
|
||||
try {
|
||||
const io = req.app.get('io');
|
||||
if (io) {
|
||||
const { buildPlaylistPayload } = require('../ws/deviceSocket');
|
||||
const commandQueue = require('../lib/command-queue');
|
||||
commandQueue.queueOrEmitPlaylistUpdate(io.of('/device'), req.params.id, buildPlaylistPayload);
|
||||
}
|
||||
} catch (e) { /* silent — the DB is the source of truth, the push is best-effort */ }
|
||||
|
||||
res.json({ success: true });
|
||||
});
|
||||
|
||||
router.put('/:id', (req, res) => {
|
||||
const device = checkDeviceOwnership(req, res);
|
||||
if (!device) return;
|
||||
|
||||
const { name, notes, timezone, orientation, default_content_id, layout_id, ota_enabled, reboot_schedule } = req.body;
|
||||
const { name, notes, timezone, orientation, default_content_id, layout_id, ota_enabled, ota_beta, reboot_schedule } = req.body;
|
||||
// #150: validate orientation against the known enum (previously accepted any string, which
|
||||
// let a bad value reach the player -> unknown rotation falls back to landscape silently).
|
||||
if (orientation !== undefined && !deviceSettings.ORIENTATIONS.has(orientation)) {
|
||||
|
|
@ -249,6 +282,11 @@ router.put('/:id', (req, res) => {
|
|||
if (ota_enabled !== undefined) {
|
||||
updates.push('ota_enabled = ?'); values.push(ota_enabled ? 1 : 0);
|
||||
}
|
||||
if (ota_beta !== undefined) {
|
||||
// Per-display pre-release opt-in (#234 follow-up). Stops a test build being reverted by the
|
||||
// next OTA check, which is what a prerelease version sorting below its own release causes.
|
||||
updates.push('ota_beta = ?'); values.push(ota_beta ? 1 : 0);
|
||||
}
|
||||
// #12 scheduled reboot: device-local "HH:MM" (null/'' clears -> off). Reset the
|
||||
// once-per-day guard on any change so a newly-set time can still fire later today.
|
||||
if (reboot_schedule !== undefined) {
|
||||
|
|
|
|||
|
|
@ -704,6 +704,7 @@ app.get('/api/update/check', (req, res) => {
|
|||
const currentVersion = req.query.version;
|
||||
const deviceId = req.query.device_id || null; // #144: optional; beta4+ clients send it for per-device keying
|
||||
const latestVersion = VERSION;
|
||||
let betaChannel = false; // per-display pre-release opt-in, set from the device row below
|
||||
|
||||
// #155/#161: self-update kill switch, enforced SERVER-SIDE so it covers EVERY client
|
||||
// version (not just ones with the client-side stand-down). If OTA is off globally
|
||||
|
|
@ -715,8 +716,12 @@ app.get('/api/update/check', (req, res) => {
|
|||
let otaDeviceOff = false;
|
||||
if (deviceId) {
|
||||
try {
|
||||
const row = require('./db/database').db.prepare('SELECT ota_enabled FROM devices WHERE id = ?').get(deviceId);
|
||||
const row = require('./db/database').db.prepare('SELECT ota_enabled, ota_beta FROM devices WHERE id = ?').get(deviceId);
|
||||
otaDeviceOff = !!row && row.ota_enabled === 0;
|
||||
// #234 follow-up: per-display pre-release opt-in, read from the same row rather than a
|
||||
// second query. Without it, handing someone a test build is a trap — a prerelease sorts
|
||||
// BELOW its own release, so the next check "upgrades" the display straight back off it.
|
||||
betaChannel = !!row && row.ota_beta === 1;
|
||||
} catch (_) { /* device unknown / pre-migration — treat as enabled */ }
|
||||
}
|
||||
if (otaGloballyOff || otaDeviceOff) {
|
||||
|
|
@ -731,7 +736,7 @@ app.get('/api/update/check', (req, res) => {
|
|||
|
||||
// #144: circuit-breaker + phantom-version guard. Keys per device_id when present, else
|
||||
// per reported version (NOT IP — SNAT). Rate-trips a looping client in seconds.
|
||||
const verdict = otaBreaker.decide(currentVersion, latestVersion, deviceId);
|
||||
const verdict = otaBreaker.decide(currentVersion, latestVersion, deviceId, Date.now(), betaChannel);
|
||||
|
||||
// #146 Item C: EARLY-RETURN before any filesystem work when we won't serve
|
||||
// (rate-backoff, up-to-date, phantom, client-newer, …). A looping client that gets
|
||||
|
|
|
|||
107
server/test/clear-device-playlist.test.js
Normal file
107
server/test/clear-device-playlist.test.js
Normal file
|
|
@ -0,0 +1,107 @@
|
|||
'use strict';
|
||||
|
||||
// "No playlist" was an option you could select that did nothing.
|
||||
//
|
||||
// The dashboard picker offered `<option value="">No playlist</option>`, and its change handler
|
||||
// opened with `if (!newPlaylistId) return; // Don't allow deselecting for now` — so choosing it
|
||||
// sent no request, changed nothing, and raised no error. The guard was honest about why: there was
|
||||
// no way to do it. PUT /devices/:id has never read playlist_id (it returns 200 and ignores it), and
|
||||
// POST /playlists/:id/assign can only set one.
|
||||
//
|
||||
// Reported on #234 as "I also selected No playlist ... it still showed the same video". It did.
|
||||
//
|
||||
// The invariant: clearing a display's playlist actually clears it, and only the people allowed to
|
||||
// change that display can do it.
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const path = require('path');
|
||||
const fs = require('fs');
|
||||
const os = require('os');
|
||||
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'st-clearpl-'));
|
||||
process.env.DATA_DIR = tmp;
|
||||
process.env.JWT_SECRET = 'test-secret-clear-playlist';
|
||||
|
||||
const express = require('express');
|
||||
const { db } = require('../db/database');
|
||||
const { requireAuth, generateToken } = require('../middleware/auth');
|
||||
|
||||
// devices -> workspaces -> organizations -> users, FK-enforced, so seed the whole chain.
|
||||
function seed(suffix) {
|
||||
const u = 'u-' + suffix, o = 'o-' + suffix, ws = 'ws-' + suffix;
|
||||
const dev = 'd-' + suffix, pl = 'p-' + suffix;
|
||||
db.prepare("INSERT OR IGNORE INTO users (id, email, password_hash, role) VALUES (?, ?, 'x', 'user')")
|
||||
.run(u, suffix + '@test.local');
|
||||
db.prepare('INSERT OR IGNORE INTO organizations (id, name, owner_user_id) VALUES (?, ?, ?)').run(o, 'org ' + suffix, u);
|
||||
db.prepare('INSERT OR IGNORE INTO workspaces (id, organization_id, name) VALUES (?, ?, ?)').run(ws, o, 'ws ' + suffix);
|
||||
// accessContext resolves through the MEMBERSHIP tables, not organizations.owner_user_id —
|
||||
// seeding only the owner column gets a legitimate owner a 403 and looks like an authz bug.
|
||||
db.prepare("INSERT OR IGNORE INTO organization_members (organization_id, user_id, role) VALUES (?, ?, 'org_owner')").run(o, u);
|
||||
db.prepare("INSERT INTO playlists (id, name, workspace_id, user_id) VALUES (?, 'PL', ?, ?)").run(pl, ws, u);
|
||||
db.prepare(`INSERT INTO devices (id, name, workspace_id, user_id, playlist_id, created_at, updated_at)
|
||||
VALUES (?, 'Screen', ?, ?, ?, strftime('%s','now'), strftime('%s','now'))`).run(dev, ws, u, pl);
|
||||
return { u, ws, dev, pl };
|
||||
}
|
||||
|
||||
const mine = seed('mine');
|
||||
const theirs = seed('theirs');
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use('/api/devices', requireAuth, require('../routes/devices'));
|
||||
const server = app.listen(0);
|
||||
|
||||
const userRow = (id) => db.prepare('SELECT id, email, role FROM users WHERE id = ?').get(id);
|
||||
const tokenFor = (u, ws) => generateToken(userRow(u), ws);
|
||||
|
||||
async function del(deviceId, token) {
|
||||
await new Promise(r => (server.listening ? r() : server.once('listening', r)));
|
||||
const res = await fetch(`http://127.0.0.1:${server.address().port}/api/devices/${deviceId}/playlist`, {
|
||||
method: 'DELETE',
|
||||
headers: token ? { Authorization: `Bearer ${token}` } : {},
|
||||
});
|
||||
return res.status;
|
||||
}
|
||||
|
||||
const playlistOf = (id) => db.prepare('SELECT playlist_id FROM devices WHERE id = ?').get(id).playlist_id;
|
||||
|
||||
test('THE BUG: PUT /devices/:id silently ignores playlist_id, so it could not clear one', async () => {
|
||||
// Pinned so nobody "fixes" the picker by pointing it back at PUT and re-creating the silence.
|
||||
const src = fs.readFileSync(path.join(__dirname, '..', 'routes', 'devices.js'), 'utf8');
|
||||
const put = src.slice(src.indexOf("router.put('/:id'"));
|
||||
const body = put.slice(0, put.indexOf('\nrouter.'));
|
||||
assert.ok(!/playlist_id\s*[,=]/.test(body), 'PUT now touches playlist_id — update this test and the picker');
|
||||
});
|
||||
|
||||
test('THE FIX: clearing a playlist actually clears it', async () => {
|
||||
assert.equal(playlistOf(mine.dev), mine.pl, 'precondition: a playlist is assigned');
|
||||
assert.equal(await del(mine.dev, tokenFor(mine.u, mine.ws)), 200);
|
||||
assert.equal(playlistOf(mine.dev), null, 'the display must end up with no playlist');
|
||||
});
|
||||
|
||||
test('clearing an already-clear display is a harmless no-op', async () => {
|
||||
// The button is in a dropdown a person can pick twice; it must not 404 or 500 on the second go.
|
||||
assert.equal(await del(mine.dev, tokenFor(mine.u, mine.ws)), 200);
|
||||
assert.equal(playlistOf(mine.dev), null);
|
||||
});
|
||||
|
||||
test('someone else\'s display cannot be cleared', async () => {
|
||||
const before = playlistOf(theirs.dev);
|
||||
const status = await del(theirs.dev, tokenFor(mine.u, mine.ws));
|
||||
assert.ok(status === 403 || status === 404, `expected refusal, got ${status}`);
|
||||
assert.equal(playlistOf(theirs.dev), before, 'a refused call must not have changed anything');
|
||||
});
|
||||
|
||||
test('an unauthenticated caller cannot clear a playlist', async () => {
|
||||
const before = playlistOf(theirs.dev);
|
||||
assert.equal(await del(theirs.dev), 401);
|
||||
assert.equal(playlistOf(theirs.dev), before);
|
||||
});
|
||||
|
||||
test('a display that does not exist is refused, not invented', async () => {
|
||||
const status = await del('no-such-device', tokenFor(mine.u, mine.ws));
|
||||
assert.ok(status === 403 || status === 404, `expected refusal, got ${status}`);
|
||||
});
|
||||
|
||||
test.after(() => { server.close(); try { fs.rmSync(tmp, { recursive: true, force: true }); } catch (_) {} });
|
||||
79
server/test/ota-beta-channel.test.js
Normal file
79
server/test/ota-beta-channel.test.js
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
'use strict';
|
||||
|
||||
// Handing someone a test build was a trap.
|
||||
//
|
||||
// A prerelease sorts BELOW its own release: 1.9.25-fix234d < 1.9.25. So a display sideloaded with a
|
||||
// test build asked the server "anything newer?", was correctly told yes — the released 1.9.25 — and
|
||||
// updated itself straight back off the build we had asked someone to test. Same versionCode, so
|
||||
// Android installed it without complaint. Silent, within minutes.
|
||||
//
|
||||
// It happened on #234: the reporter installed the fix, tested for an evening, and reported that
|
||||
// nothing had changed. They were right — their tablet was running the old code again by then.
|
||||
//
|
||||
// The opt-in is per display and deliberately narrow: it holds a prerelease of the CURRENT core
|
||||
// only. An older-core prerelease is genuinely stale and must still be offered an update, and once a
|
||||
// newer release ships the display must rejoin it — otherwise "beta" quietly becomes "abandoned on a
|
||||
// branch nobody maintains".
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const breaker = require('../lib/ota-breaker');
|
||||
|
||||
// decide(client, latest, deviceId, now, betaChannel)
|
||||
const ask = (client, latest, beta) => breaker.decide(client, latest, null, Date.now(), beta);
|
||||
|
||||
test('THE BUG: without the opt-in, a test build is offered its own release and reverts', () => {
|
||||
const v = ask('1.9.25-fix234d', '1.9.25', false);
|
||||
assert.equal(v.update_available, true, 'this is the revert that cost a reporter an evening');
|
||||
assert.equal(v.reason, 'offer');
|
||||
});
|
||||
|
||||
test('THE FIX: an opted-in display keeps a prerelease of the current release', () => {
|
||||
const v = ask('1.9.25-fix234d', '1.9.25', true);
|
||||
assert.equal(v.update_available, false);
|
||||
assert.equal(v.reason, 'beta-channel');
|
||||
});
|
||||
|
||||
test('opting in does NOT strand a display once a newer release ships', () => {
|
||||
// The whole risk of a beta flag is that it becomes permanent. 1.9.26 is a real newer core, so an
|
||||
// opted-in display on any 1.9.25 build must take it.
|
||||
const v = ask('1.9.25-fix234d', '1.9.26', true);
|
||||
assert.equal(v.update_available, true, 'a beta display must rejoin the next real release');
|
||||
});
|
||||
|
||||
test('the superseded-prerelease guard is untouched for displays that did NOT opt in', () => {
|
||||
// #144's phantom protection: a device reporting an ancient beta is not chased with offers.
|
||||
const v = ask('1.9.1-beta4', '1.9.25', false);
|
||||
assert.equal(v.update_available, false);
|
||||
assert.equal(v.reason, 'superseded-prerelease');
|
||||
});
|
||||
|
||||
test('but an opted-in display on an old prerelease IS offered the current release', () => {
|
||||
// This is the escape hatch, and it is the difference between "beta" and "abandoned". Without
|
||||
// it the superseded guard pins a tester on an old test build permanently — they would have to
|
||||
// notice and sideload their way out, which is exactly the trap the opt-in exists to remove.
|
||||
const v = ask('1.9.1-beta4', '1.9.25', true);
|
||||
assert.equal(v.update_available, true, 'opting in must never mean never updating again');
|
||||
});
|
||||
|
||||
test('the opt-in changes nothing for a display on a plain release', () => {
|
||||
assert.equal(ask('1.9.25', '1.9.25', true).reason, 'up-to-date');
|
||||
assert.equal(ask('1.9.24', '1.9.25', true).update_available, true, 'a real upgrade is unaffected');
|
||||
assert.equal(ask('1.9.24', '1.9.25', false).update_available, true);
|
||||
});
|
||||
|
||||
test('a display ahead of the server is never downgraded, opted in or not', () => {
|
||||
assert.equal(ask('1.9.26', '1.9.25', true).reason, 'client-newer');
|
||||
assert.equal(ask('1.9.26', '1.9.25', false).reason, 'client-newer');
|
||||
});
|
||||
|
||||
test('a -patchN build is a release, not a prerelease, so beta does not pin it', () => {
|
||||
// isReleased() treats patchN as released; it must keep being offered real updates.
|
||||
const v = ask('1.9.2-patch3', '1.9.25', true);
|
||||
assert.equal(v.update_available, true, 'a patch release must not be mistaken for a beta build');
|
||||
});
|
||||
|
||||
test('the flag defaults to off, so nothing changes for a fleet that never sets it', () => {
|
||||
const withDefault = breaker.decide('1.9.25-fix234d', '1.9.25', null, Date.now());
|
||||
assert.equal(withDefault.update_available, true, 'default must match pre-existing behaviour');
|
||||
});
|
||||
Loading…
Reference in a new issue