Skip to content

Commit b84af20

Browse files
committed
Merge namespace validators into one validateNamespace(allow_root=)
validateScopeNamespace and validateDestroyNamespace differed only in whether the root (length-0) namespace is allowed -- the apparent leniency of the scope validator was illusory (numeric/multi-element namespaces crashed cryptically downstream rather than being permitted). Collapse them into a single validateNamespace(namespace, allow_root) helper: makeScope() passes allow_root = TRUE, the destroy paths use the default. As a side benefit, malformed makeScope() namespaces now get the clean validation error instead of a cryptic fastmap/if-length crash.
1 parent 5617f5b commit b84af20

2 files changed

Lines changed: 16 additions & 24 deletions

File tree

R/mock-session.R

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ MockShinySession <- R6Class(
337337
call. = FALSE
338338
)
339339
}
340-
validateDestroyNamespace(namespace)
340+
validateNamespace(namespace)
341341
self$makeScope(namespace)$destroy()
342342
},
343343

@@ -554,7 +554,7 @@ MockShinySession <- R6Class(
554554
#' @param namespace Character vector indicating a namespace.
555555
#' @return A new session proxy.
556556
makeScope = function(namespace) {
557-
validateScopeNamespace(namespace)
557+
validateNamespace(namespace, allow_root = TRUE)
558558
ns <- NS(namespace)
559559
# The scope's own namespace, captured because the proxy `destroy()` below
560560
# has a `namespace` parameter that would otherwise shadow it.
@@ -586,7 +586,7 @@ MockShinySession <- R6Class(
586586
private$invokeDestroyCallbacks(selfNamespace)
587587
} else {
588588
# Tear down a named child scope.
589-
validateDestroyNamespace(namespace)
589+
validateNamespace(namespace)
590590
self$makeScope(ns(namespace))$destroy()
591591
}
592592
}

R/shiny.R

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -445,38 +445,30 @@ ns.sep <- "-"
445445
# additionally guards against).
446446
destroyNsRoot <- "..root"
447447

448-
# Validate the user-supplied `namespace` passed to `session$destroy(namespace)`.
449-
# Must be a single, non-empty, non-NA string. Checked at the public boundary,
450-
# before `makeScope()` runs (which would otherwise choke on bad input).
451-
validateDestroyNamespace <- function(namespace) {
448+
# Validate a module `namespace`. A namespace must be a single, non-empty,
449+
# non-NA string and may not be the reserved sentinel. When `allow_root = TRUE`
450+
# (e.g. `makeScope()`), the root scope -- a length-0 `NULL` / `character(0)` --
451+
# is also accepted; the destroy entry points pass `allow_root = FALSE` because a
452+
# child scope must be named explicitly.
453+
validateNamespace <- function(namespace, allow_root = FALSE) {
454+
if (allow_root && length(namespace) == 0) {
455+
return(invisible(namespace))
456+
}
452457
if (!is.character(namespace) || length(namespace) != 1L || is.na(namespace) || !nzchar(namespace)) {
453458
stop(
454459
"Invalid `namespace`: ",
455460
encodeString(paste0(format(namespace), collapse = ", "), quote = '"'),
456-
". `namespace` must be a non-empty length 1 character vector.",
461+
". A module namespace must be a non-empty, non-NA string; use `NULL` for the root scope.",
457462
call. = FALSE
458463
)
459464
}
460-
invisible(namespace)
461-
}
462-
463-
# Validate a namespace passed to `makeScope()`. The root (length 0: `NULL` or
464-
# `character(0)`) is allowed; "" / NA are rejected (they can't be a fastmap key,
465-
# and `NS("")` yields a stray `-`); the reserved sentinel is rejected too.
466-
validateScopeNamespace <- function(namespace) {
467465
if (identical(namespace, destroyNsRoot)) {
468466
stop(
469467
"The module namespace '", destroyNsRoot,
470468
"' is reserved for internal use.",
471469
call. = FALSE
472470
)
473471
}
474-
if (length(namespace) == 1L && (is.na(namespace) || !nzchar(namespace))) {
475-
stop(
476-
"A module namespace must be a non-empty, non-NA string; use `NULL` for the root scope.",
477-
call. = FALSE
478-
)
479-
}
480472
invisible(namespace)
481473
}
482474

@@ -1044,7 +1036,7 @@ ShinySession <- R6Class(
10441036
self
10451037
},
10461038
makeScope = function(namespace) {
1047-
validateScopeNamespace(namespace)
1039+
validateNamespace(namespace, allow_root = TRUE)
10481040
ns <- NS(namespace)
10491041
# The scope's own namespace, captured because the proxy `destroy()` below
10501042
# has a `namespace` parameter that would otherwise shadow it.
@@ -1126,7 +1118,7 @@ ShinySession <- R6Class(
11261118
private$invokeDestroyCallbacks(selfNamespace)
11271119
} else {
11281120
# Tear down a named child scope.
1129-
validateDestroyNamespace(namespace)
1121+
validateNamespace(namespace)
11301122
self$makeScope(ns(namespace))$destroy()
11311123
}
11321124
}
@@ -1334,7 +1326,7 @@ ShinySession <- R6Class(
13341326
call. = FALSE
13351327
)
13361328
}
1337-
validateDestroyNamespace(namespace)
1329+
validateNamespace(namespace)
13381330
self$makeScope(namespace)$destroy()
13391331
},
13401332
onInputReceived = function(callback) {

0 commit comments

Comments
 (0)