Skip to content

Commit bff2070

Browse files
committed
Best-effort task scheduling through the session
Currently a no-op change, as the session just passes through to the global scheduleTask implementation. But this allows us to mock the method for testing.
1 parent ed739f9 commit bff2070

File tree

5 files changed

+87
-7
lines changed

5 files changed

+87
-7
lines changed

R/reactives.R

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,14 +1513,16 @@ reactiveTimer <- function(intervalMs=1000, session = getDefaultReactiveDomain())
15131513
# reactId <- nextGlobalReactId()
15141514
# rLog$define(reactId, paste0("timer(", intervalMs, ")"))
15151515

1516+
scheduler <- defineScheduler(session)
1517+
15161518
dependents <- Map$new()
1517-
timerHandle <- scheduleTask(intervalMs, function() {
1519+
timerHandle <- scheduler(intervalMs, function() {
15181520
# Quit if the session is closed
15191521
if (!is.null(session) && session$isClosed()) {
15201522
return(invisible())
15211523
}
15221524

1523-
timerHandle <<- scheduleTask(intervalMs, sys.function())
1525+
timerHandle <<- scheduler(intervalMs, sys.function())
15241526

15251527
doInvalidate <- function() {
15261528
lapply(
@@ -1613,15 +1615,16 @@ reactiveTimer <- function(intervalMs=1000, session = getDefaultReactiveDomain())
16131615
#' }
16141616
#' @export
16151617
invalidateLater <- function(millis, session = getDefaultReactiveDomain()) {
1616-
16171618
force(session)
16181619

16191620
ctx <- getCurrentContext()
16201621
rLog$invalidateLater(ctx$.reactId, ctx$id, millis, session)
16211622

16221623
clear_on_ended_callback <- function() {}
16231624

1624-
timerHandle <- scheduleTask(millis, function() {
1625+
scheduler <- defineScheduler(session)
1626+
1627+
timerHandle <- scheduler(millis, function() {
16251628
if (is.null(session)) {
16261629
ctx$invalidate()
16271630
return(invisible())

R/shiny.R

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,9 @@ ShinySession <- R6Class(
723723
requestFlush = function() {
724724
appsNeedingFlush$set(self$token, self)
725725
},
726+
scheduleTask = function(millis, callback) {
727+
scheduleTask(millis, callback)
728+
},
726729
rootScope = function() {
727730
self
728731
},

R/timer.R

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,15 @@ scheduleTask <- function(millis, callback) {
9696
invisible(timerCallbacks$unschedule(id))
9797
}
9898
}
99+
100+
#' Get a scheduler function for scheduling tasks. Give priority to the
101+
#' session scheduler, but if it doesn't exist, use the global one.
102+
#' @noRd
103+
defineScheduler <- function(session){
104+
if (!is.null(session)){
105+
if (!is.null(session$scheduleTask)){
106+
return(session$scheduleTask)
107+
}
108+
}
109+
scheduleTask
110+
}

tests/testthat/test-reactivity.r

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,3 +1366,59 @@ test_that("reactivePoll doesn't leak observer (#1548)", {
13661366

13671367
expect_equal(i, 3L)
13681368
})
1369+
1370+
test_that("reactivePoll prefers session$scheduleTask", {
1371+
called <- 0
1372+
session <- list(reactlog = function(...){}, onEnded = function(...){}, scheduleTask = function(millis, cb){
1373+
expect_equal(millis, 50)
1374+
called <<- called + 1
1375+
})
1376+
1377+
count <- reactivePoll(50, session, function(){}, function(){})
1378+
observe({
1379+
count()
1380+
})
1381+
1382+
for (i in 1:4) {
1383+
Sys.sleep(0.05)
1384+
shiny:::flushReact()
1385+
}
1386+
expect_gt(called, 0)
1387+
})
1388+
1389+
test_that("invalidateLater prefers session$scheduleTask", {
1390+
called <- 0
1391+
session <- list(reactlog = function(...){}, onEnded = function(...){}, scheduleTask = function(millis, cb){
1392+
expect_equal(millis, 10)
1393+
called <<- called + 1
1394+
})
1395+
1396+
observe({
1397+
invalidateLater(10, session)
1398+
})
1399+
1400+
for (i in 1:4) {
1401+
Sys.sleep(0.05)
1402+
shiny:::flushReact()
1403+
}
1404+
expect_gt(called, 0)
1405+
})
1406+
1407+
test_that("reactiveTimer prefers session$scheduleTask", {
1408+
called <- 0
1409+
session <- list(reactlog = function(...){}, onEnded = function(...){}, scheduleTask = function(millis, cb){
1410+
expect_equal(millis, 10)
1411+
called <<- called + 1
1412+
})
1413+
1414+
rt <- reactiveTimer(10, session)
1415+
observe({
1416+
rt()
1417+
})
1418+
1419+
for (i in 1:4) {
1420+
Sys.sleep(0.05)
1421+
shiny:::flushReact()
1422+
}
1423+
expect_gt(called, 0)
1424+
})

tests/testthat/test-timer.R

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ test_that("Scheduling works", {
2727
test_that("Unscheduling works", {
2828
origTimes <- timerCallbacks$.times
2929
origFuncKeys <- timerCallbacks$.funcs$keys()
30-
30+
3131
taskHandle <- scheduleTask(1000, function() {
3232
message("Whatever")
3333
})
3434
# Unregister
3535
taskHandle()
36-
36+
3737
expect_identical(timerCallbacks$.times, origTimes)
3838
expect_identical(timerCallbacks$.funcs$keys(), origFuncKeys)
3939
})
@@ -42,7 +42,13 @@ test_that("Vectorized unscheduling works", {
4242
key1 <- timerCallbacks$schedule(1000, function() {})
4343
key2 <- timerCallbacks$schedule(1000, function() {})
4444
key3 <- timerCallbacks$schedule(1000, function() {})
45-
45+
4646
expect_identical(timerCallbacks$unschedule(key2), TRUE)
4747
expect_identical(timerCallbacks$unschedule(c(key1, key2, key3)), c(TRUE, FALSE, TRUE))
4848
})
49+
50+
test_that("defineScheduler works", {
51+
expect_identical(defineScheduler(NULL), scheduleTask)
52+
expect_identical(defineScheduler(list()), scheduleTask)
53+
expect_identical(defineScheduler(list(scheduleTask=123)), 123)
54+
})

0 commit comments

Comments
 (0)