From b6054ce351e7ebeef31fcce5396383b1140cfd6d Mon Sep 17 00:00:00 2001 From: wlandau Date: Mon, 5 Nov 2018 20:41:24 -0500 Subject: [PATCH 01/11] Sketch custom mangler --- .Rbuildignore | 2 ++ R/driver_rds.R | 85 +++++++++++++++++++++++++++++++++++++++++++---- R/driver_remote.R | 3 +- R/utils.R | 5 +++ 4 files changed, 87 insertions(+), 8 deletions(-) diff --git a/.Rbuildignore b/.Rbuildignore index 54336f4..44aa68b 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -16,3 +16,5 @@ Rprof\.out ^scripts$ ^vignettes_src$ ^appveyor\.yml$ +^.*\.Rproj$ +^\.Rproj\.user$ diff --git a/R/driver_rds.R b/R/driver_rds.R index bbcbda7..a72b037 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -163,7 +163,8 @@ R6_driver_rds <- R6::R6Class( driver_rds_config_file(path, "version")) if (!is.null(mangle_key)) { - assert_scalar_logical(mangle_key) + assert_scalar(mangle_key) + assert_is(mangle_key, c("logical", "character")) } self$mangle_key <- driver_rds_config(path, "mangle_key", mangle_key, FALSE, TRUE) @@ -245,7 +246,7 @@ R6_driver_rds <- R6::R6Class( path <- file.path(self$path, "keys", namespace) files <- dir(path) if (self$mangle_key) { - ret <- decode64(files, error = FALSE) + ret <- self$decode(files, error = FALSE) if (anyNA(ret)) { message_corrupted_rds_keys(namespace, path, files[is.na(ret)]) ret <- ret[!is.na(ret)] @@ -268,7 +269,7 @@ R6_driver_rds <- R6::R6Class( if (self$mangle_key) { path <- file.path(self$path, "keys", namespace) files <- dir(path) - i <- is.na(decode64(files, error = FALSE)) + i <- is.na(self$decode(files, error = FALSE)) if (any(i)) { res <- file.remove(file.path(path, files[i])) message(sprintf("Removed %d of %d corrupt %s", @@ -286,10 +287,46 @@ R6_driver_rds <- R6::R6Class( }, name_key = function(key, namespace) { - if (self$mangle_key) { - key <- encode64(key, pad = self$mangle_key_pad) - } + key <- self$encode(key, pad = self$mangle_key_pad) file.path(self$path, "keys", namespace, key) + }, + + encode = function(x, pad) { + if (identical(self$mangle_key, FALSE)) { + return(x) + } + if (identical(self$mangle_key, "none")) { + return(x) + } + if (identical(self$mangle_key, TRUE)) { + return(encode64(x, pad = pad)) + } + if (identical(self$mangle_key, "base64")) { + return(encode64(x, pad = pad)) + } + mangler <- getOption("storr_mangler") + assert_list(mangler) + assert_identical(mangler, mangler$name) + mangler$encode(x) + }, + + decode = function(x, error) { + if (identical(self$mangle_key, FALSE)) { + return(x) + } + if (identical(self$mangle_key, "none")) { + return(x) + } + if (identical(self$mangle_key, TRUE)) { + return(decode64(x, error = error)) + } + if (identical(self$mangle_key, "base64")) { + return(decode64(x, error = error)) + } + mangler <- getOption("storr_mangler") + assert_list(mangler) + assert_identical(mangler, mangler$name) + mangler$decode(x) } )) @@ -447,3 +484,39 @@ See 'Corrupt keys' within ?storr_rds for how to proceed" -> fmt message(sprintf(fmt, length(files), namespace, path, files)) corrupt_notices[[path]] <- now } + +storr_rds_decode <- function(x, error) { + if (identical(self$mangle_key, FALSE)) { + return(x) + } + if (identical(self$mangle_key, "none")) { + return(x) + } + if (identical(self$mangle_key, TRUE)) { + return(decode64(x, error = error)) + } + if (identical(self$mangle_key, "base64")) { + return(decode64(x, error = error)) + } + mangler <- getOption("storr_mangler") + assert_mangler(mangler) + assert_identical(mangler, mangler$name) + mangler$decode(x) +} + +#' @title Register a key mangler +#' @description Define custom functinons for mangling \code{storr_rds()} keys. +#' @export +#' @return nothing +#' @param name character scalar, name of the mangler +#' @param encode function to encode keys. Must have arguments \code{x} +#' and \code{pad}. +#' @param decode function to decode keys. Must have arguments \code{x} +#' and \code{error} +register_mangler <- function(name, encode, decode, overwrite = FALSE) { + current <- getOption("storr_mangler") + if (is.list(current) && !overwrite){ + return() + } + options(storr_mangler = list(name = name, encode = encode, decode = decode)) +} diff --git a/R/driver_remote.R b/R/driver_remote.R index 403e0d2..c385889 100644 --- a/R/driver_remote.R +++ b/R/driver_remote.R @@ -134,7 +134,7 @@ R6_driver_remote <- R6::R6Class( return(character(0)) } ret <- self$ops$list_dir(path) - if (self$rds$mangle_key) decode64(ret, TRUE) else ret + self$rds$decode(ret, TRUE) }, ## These functions could be done better if driver_rds takes a @@ -152,7 +152,6 @@ R6_driver_remote <- R6::R6Class( basename(p)) })) - ## It would be really nice to do this as a single operation but that ## probably can't be easily done generally. Quite possibly it would ## be possible to get/fetch an entire directory though. diff --git a/R/utils.R b/R/utils.R index 81e93e2..4c41735 100644 --- a/R/utils.R +++ b/R/utils.R @@ -26,6 +26,11 @@ vcapply <- function(X, FUN, ...) { vapply(X, FUN, character(1), ...) } +assert_identical <- function(x, y) { + if (!identical(x, y)) { + stop(sprintf("'%s' and '%s' must be identical", name), call. = FALSE) + } +} assert_scalar <- function(x, name = deparse(substitute(x))) { if (length(x) != 1) { From a76269846dd824eb595d0a2bcfc6022b8df5edde Mon Sep 17 00:00:00 2001 From: wlandau Date: Mon, 5 Nov 2018 21:11:55 -0500 Subject: [PATCH 02/11] Store manglers --- R/driver_rds.R | 29 +++++++++++++++++++---------- R/utils.R | 25 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/R/driver_rds.R b/R/driver_rds.R index a72b037..1281642 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -187,6 +187,8 @@ R6_driver_rds <- R6::R6Class( } self$hash_algorithm <- driver_rds_config(path, "hash_algorithm", hash_algorithm, "md5", TRUE) + + self$set_mangler(path, mangle_key, is_new) }, type = function() { @@ -304,10 +306,7 @@ R6_driver_rds <- R6::R6Class( if (identical(self$mangle_key, "base64")) { return(encode64(x, pad = pad)) } - mangler <- getOption("storr_mangler") - assert_list(mangler) - assert_identical(mangler, mangler$name) - mangler$encode(x) + self$mangler$encode(x, pad = pad) }, decode = function(x, error) { @@ -323,12 +322,22 @@ R6_driver_rds <- R6::R6Class( if (identical(self$mangle_key, "base64")) { return(decode64(x, error = error)) } - mangler <- getOption("storr_mangler") - assert_list(mangler) - assert_identical(mangler, mangler$name) - mangler$decode(x) - } - )) + self$mangler$decode(x, error = error) + }, + + set_mangler = function(path, mangle_key, is_new){ + if (chosen_default_mangler(mangle_key)) { + return() + } + if (is_new) { + self$mangler <- getOption("storr_mangler") + assert_mangler(self$mangler) + saveRDS(file.path(path, "config", "mangler.rds"), mangler) + } else { + self$mangler <- readRDS(file.path(path, "config", "mangler.rds")) + assert_custom_mangler(self$mangler, mangle_key) + } + })) ## This attempts to check that we are connecting to a storr of diff --git a/R/utils.R b/R/utils.R index 4c41735..eb54ff2 100644 --- a/R/utils.R +++ b/R/utils.R @@ -121,6 +121,25 @@ assert_probably_storr_driver <- function(x, name = deparse(substitute(x))) { invisible(x) } +assert_mangler <- function(mangler) { + if (is.null(mangler)) { + stop("Set a key mangler with register_mangler().", call. = FALSE) + } +} + +assert_custom_mangler <- function(mangler, key) { + assert_mangler(mangler) + if (identical(mangler$name, key)) { + stop( + sprintf( + "New key mangler '%s' disagrees with old mangler '%s'", + mangler$name, mangle_key + ), + call. = FALSE + ) + } +} + match_value <- function(x, choices, name = deparse(substitute(x))) { assert_scalar_character(x, name) @@ -184,3 +203,9 @@ file_size <- function(...) { prompt_ask_yes_no <- function(reason) { utils::menu(c("no", "yes"), FALSE, title = reason) == 2 # nocov } + +chosen_default_mangler <- function(mangle_key) { + is.null(mangle_key) || + is.logical(mangle_key) || + mangle_key %in% c("none", "base64") +} From 222bfcc219505385d9e6b33f34588bee698d7167 Mon Sep 17 00:00:00 2001 From: wlandau Date: Mon, 5 Nov 2018 21:15:00 -0500 Subject: [PATCH 03/11] Change a function name --- R/driver_rds.R | 2 +- R/utils.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/driver_rds.R b/R/driver_rds.R index 1281642..69dd3d4 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -326,7 +326,7 @@ R6_driver_rds <- R6::R6Class( }, set_mangler = function(path, mangle_key, is_new){ - if (chosen_default_mangler(mangle_key)) { + if (chose_default_mangler(mangle_key)) { return() } if (is_new) { diff --git a/R/utils.R b/R/utils.R index eb54ff2..65c48dc 100644 --- a/R/utils.R +++ b/R/utils.R @@ -204,7 +204,7 @@ prompt_ask_yes_no <- function(reason) { utils::menu(c("no", "yes"), FALSE, title = reason) == 2 # nocov } -chosen_default_mangler <- function(mangle_key) { +chose_default_mangler <- function(mangle_key) { is.null(mangle_key) || is.logical(mangle_key) || mangle_key %in% c("none", "base64") From f2c7a262d9711807127c564cf86dc4c4a56b9c82 Mon Sep 17 00:00:00 2001 From: wlandau Date: Mon, 5 Nov 2018 21:15:59 -0500 Subject: [PATCH 04/11] Make sure register_mangler() returns nothing --- DESCRIPTION | 2 +- NAMESPACE | 1 + R/driver_rds.R | 1 + man/storr_rds.Rd | 5 +++-- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 4d7b8ef..a3a6179 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -27,5 +27,5 @@ Suggests: rbenchmark, testthat (>= 1.0.0) VignetteBuilder: knitr -RoxygenNote: 6.0.1 +RoxygenNote: 6.1.0 Encoding: UTF-8 diff --git a/NAMESPACE b/NAMESPACE index 88661fb..6e9d7c7 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -10,6 +10,7 @@ export(driver_remote) export(encode64) export(fetch_hook_read) export(join_key_namespace) +export(register_mangler) export(storr) export(storr_dbi) export(storr_environment) diff --git a/R/driver_rds.R b/R/driver_rds.R index 69dd3d4..57252a7 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -528,4 +528,5 @@ register_mangler <- function(name, encode, decode, overwrite = FALSE) { return() } options(storr_mangler = list(name = name, encode = encode, decode = decode)) + invisible() } diff --git a/man/storr_rds.Rd b/man/storr_rds.Rd index 4e46782..a5c4e23 100644 --- a/man/storr_rds.Rd +++ b/man/storr_rds.Rd @@ -5,8 +5,9 @@ \alias{driver_rds} \title{rds object cache driver} \usage{ -storr_rds(path, compress = NULL, mangle_key = NULL, mangle_key_pad = NULL, - hash_algorithm = NULL, default_namespace = "objects") +storr_rds(path, compress = NULL, mangle_key = NULL, + mangle_key_pad = NULL, hash_algorithm = NULL, + default_namespace = "objects") driver_rds(path, compress = NULL, mangle_key = NULL, mangle_key_pad = NULL, hash_algorithm = NULL) From 8c8775a1fa18a814ce2ee86b7e63628d54098ff9 Mon Sep 17 00:00:00 2001 From: wlandau Date: Tue, 6 Nov 2018 05:31:17 -0500 Subject: [PATCH 05/11] Avoid checking the mangler too often --- R/driver_rds.R | 91 ++++++++++++----------------------------- R/driver_remote.R | 2 +- R/utils.R | 26 +++++++----- man/register_mangler.Rd | 23 +++++++++++ 4 files changed, 66 insertions(+), 76 deletions(-) create mode 100644 man/register_mangler.Rd diff --git a/R/driver_rds.R b/R/driver_rds.R index 57252a7..05198c2 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -132,6 +132,7 @@ R6_driver_rds <- R6::R6Class( compress = NULL, mangle_key = NULL, mangle_key_pad = NULL, + mangler = NULL, hash_algorithm = NULL, traits = list(accept = "raw"), @@ -188,7 +189,7 @@ R6_driver_rds <- R6::R6Class( self$hash_algorithm <- driver_rds_config(path, "hash_algorithm", hash_algorithm, "md5", TRUE) - self$set_mangler(path, mangle_key, is_new) + self$set_mangler(path, is_new) }, type = function() { @@ -247,14 +248,12 @@ R6_driver_rds <- R6::R6Class( list_keys = function(namespace) { path <- file.path(self$path, "keys", namespace) files <- dir(path) + ret <- self$mangler$decode(x = files, error = FALSE) if (self$mangle_key) { - ret <- self$decode(files, error = FALSE) if (anyNA(ret)) { message_corrupted_rds_keys(namespace, path, files[is.na(ret)]) ret <- ret[!is.na(ret)] } - } else { - ret <- files } ret }, @@ -271,7 +270,7 @@ R6_driver_rds <- R6::R6Class( if (self$mangle_key) { path <- file.path(self$path, "keys", namespace) files <- dir(path) - i <- is.na(self$decode(files, error = FALSE)) + i <- is.na(self$mangler$decode(files, error = FALSE)) if (any(i)) { res <- file.remove(file.path(path, files[i])) message(sprintf("Removed %d of %d corrupt %s", @@ -289,53 +288,34 @@ R6_driver_rds <- R6::R6Class( }, name_key = function(key, namespace) { - key <- self$encode(key, pad = self$mangle_key_pad) + key <- self$mangler$encode(x = key, pad = self$mangle_key_pad) file.path(self$path, "keys", namespace, key) }, - encode = function(x, pad) { - if (identical(self$mangle_key, FALSE)) { - return(x) - } - if (identical(self$mangle_key, "none")) { - return(x) - } - if (identical(self$mangle_key, TRUE)) { - return(encode64(x, pad = pad)) - } - if (identical(self$mangle_key, "base64")) { - return(encode64(x, pad = pad)) - } - self$mangler$encode(x, pad = pad) - }, - - decode = function(x, error) { - if (identical(self$mangle_key, FALSE)) { - return(x) - } - if (identical(self$mangle_key, "none")) { - return(x) - } - if (identical(self$mangle_key, TRUE)) { - return(decode64(x, error = error)) - } - if (identical(self$mangle_key, "base64")) { - return(decode64(x, error = error)) - } - self$mangler$decode(x, error = error) - }, - - set_mangler = function(path, mangle_key, is_new){ - if (chose_default_mangler(mangle_key)) { - return() - } - if (is_new) { + set_mangler = function(path, is_new) { + if (use_no_mangler(self$mangle_key)) { + self$mangler <- list( + name = "none", + encode = function(x, pad) { + x + }, + decode = function(x, error) { + x + } + ) + } else if (use_base64_mangler(self$mangle_key)) { + self$mangler <- list( + name = "base64", + encode = encode64, + decode = decode64 + ) + } else if (is_new){ self$mangler <- getOption("storr_mangler") - assert_mangler(self$mangler) - saveRDS(file.path(path, "config", "mangler.rds"), mangler) + assert_custom_mangler(self$mangler, self$mangle_key) + saveRDS(file.path(path, "config", "mangler.rds"), self$mangler) } else { self$mangler <- readRDS(file.path(path, "config", "mangler.rds")) - assert_custom_mangler(self$mangler, mangle_key) + assert_custom_mangler(self$mangler, self$mangle_key) } })) @@ -494,25 +474,6 @@ See 'Corrupt keys' within ?storr_rds for how to proceed" -> fmt corrupt_notices[[path]] <- now } -storr_rds_decode <- function(x, error) { - if (identical(self$mangle_key, FALSE)) { - return(x) - } - if (identical(self$mangle_key, "none")) { - return(x) - } - if (identical(self$mangle_key, TRUE)) { - return(decode64(x, error = error)) - } - if (identical(self$mangle_key, "base64")) { - return(decode64(x, error = error)) - } - mangler <- getOption("storr_mangler") - assert_mangler(mangler) - assert_identical(mangler, mangler$name) - mangler$decode(x) -} - #' @title Register a key mangler #' @description Define custom functinons for mangling \code{storr_rds()} keys. #' @export diff --git a/R/driver_remote.R b/R/driver_remote.R index c385889..c5da9c9 100644 --- a/R/driver_remote.R +++ b/R/driver_remote.R @@ -134,7 +134,7 @@ R6_driver_remote <- R6::R6Class( return(character(0)) } ret <- self$ops$list_dir(path) - self$rds$decode(ret, TRUE) + self$rds$mangler$decode(x = ret, error = TRUE) }, ## These functions could be done better if driver_rds takes a diff --git a/R/utils.R b/R/utils.R index 65c48dc..88e7018 100644 --- a/R/utils.R +++ b/R/utils.R @@ -121,15 +121,15 @@ assert_probably_storr_driver <- function(x, name = deparse(substitute(x))) { invisible(x) } -assert_mangler <- function(mangler) { +assert_custom_mangler <- function(mangler, mangle_key) { if (is.null(mangler)) { - stop("Set a key mangler with register_mangler().", call. = FALSE) + stop( + sprintf("No mangler set. Expected '%s'.", mangle_key), + "Set a key mangler with register_mangler().", + call. = FALSE + ) } -} - -assert_custom_mangler <- function(mangler, key) { - assert_mangler(mangler) - if (identical(mangler$name, key)) { + if (identical(mangler$name, mangle_key)) { stop( sprintf( "New key mangler '%s' disagrees with old mangler '%s'", @@ -204,8 +204,14 @@ prompt_ask_yes_no <- function(reason) { utils::menu(c("no", "yes"), FALSE, title = reason) == 2 # nocov } -chose_default_mangler <- function(mangle_key) { +use_no_mangler <- function(mangle_key) { is.null(mangle_key) || - is.logical(mangle_key) || - mangle_key %in% c("none", "base64") + identical(mangle_key, FALSE) || + identical(mangle_key, "none") +} + +use_base64_mangler <- function(mangle_key) { + identical(mangle_key, TRUE) || + identical(mangle_key, "base64") } + diff --git a/man/register_mangler.Rd b/man/register_mangler.Rd new file mode 100644 index 0000000..e3723f4 --- /dev/null +++ b/man/register_mangler.Rd @@ -0,0 +1,23 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/driver_rds.R +\name{register_mangler} +\alias{register_mangler} +\title{Register a key mangler} +\usage{ +register_mangler(name, encode, decode, overwrite = FALSE) +} +\arguments{ +\item{name}{character scalar, name of the mangler} + +\item{encode}{function to encode keys. Must have arguments \code{x} +and \code{pad}.} + +\item{decode}{function to decode keys. Must have arguments \code{x} +and \code{error}} +} +\value{ +nothing +} +\description{ +Define custom functinons for mangling \code{storr_rds()} keys. +} From 2644b2536202a092e15577ffeddbeadc072f3457 Mon Sep 17 00:00:00 2001 From: wlandau Date: Tue, 6 Nov 2018 06:53:59 -0500 Subject: [PATCH 06/11] Test custom key mangling... and avoid excessive checking inside the mangler itself. --- R/driver_rds.R | 20 +++++++--- R/utils.R | 8 ++-- tests/testthat/helper-storr.R | 24 ++++++++++++ tests/testthat/test-custom-manglers.R | 54 +++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 tests/testthat/test-custom-manglers.R diff --git a/R/driver_rds.R b/R/driver_rds.R index 05198c2..9621c7e 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -205,7 +205,7 @@ R6_driver_rds <- R6::R6Class( }, set_hash = function(key, namespace, hash) { - dir_create(self$name_key("", namespace)) + dir_create(self$key_path("", namespace)) write_lines(hash, self$name_key(key, namespace), scratch_dir = self$path_scratch) }, @@ -249,7 +249,7 @@ R6_driver_rds <- R6::R6Class( path <- file.path(self$path, "keys", namespace) files <- dir(path) ret <- self$mangler$decode(x = files, error = FALSE) - if (self$mangle_key) { + if (use_base64_mangler(self$mangle_key)) { if (anyNA(ret)) { message_corrupted_rds_keys(namespace, path, files[is.na(ret)]) ret <- ret[!is.na(ret)] @@ -267,7 +267,7 @@ R6_driver_rds <- R6::R6Class( }, purge_corrupt_keys = function(namespace) { - if (self$mangle_key) { + if (use_base64_mangler(self$mangle_key)) { path <- file.path(self$path, "keys", namespace) files <- dir(path) i <- is.na(self$mangler$decode(files, error = FALSE)) @@ -289,6 +289,10 @@ R6_driver_rds <- R6::R6Class( name_key = function(key, namespace) { key <- self$mangler$encode(x = key, pad = self$mangle_key_pad) + self$key_path(key, namespace) + }, + + key_path = function(key, namespace) { file.path(self$path, "keys", namespace, key) }, @@ -312,7 +316,7 @@ R6_driver_rds <- R6::R6Class( } else if (is_new){ self$mangler <- getOption("storr_mangler") assert_custom_mangler(self$mangler, self$mangle_key) - saveRDS(file.path(path, "config", "mangler.rds"), self$mangler) + saveRDS(self$mangler, file.path(self$path, "config", "mangler.rds")) } else { self$mangler <- readRDS(file.path(path, "config", "mangler.rds")) assert_custom_mangler(self$mangler, self$mangle_key) @@ -334,8 +338,12 @@ driver_rds_config <- function(path, name, value, default, must_agree) { load_value <- function() { if (file.exists(path_opt)) { - value <- readLines(path_opt) - storage.mode(value) <- storage.mode(default) + value_prev <- tmp <- readLines(path_opt) + storage.mode(tmp) <- storage.mode(default) + if (is.na(tmp)) { + stop(ConfigError(name, value_prev, value)) + } + value <- tmp } else { value <- default } diff --git a/R/utils.R b/R/utils.R index 88e7018..44f82a7 100644 --- a/R/utils.R +++ b/R/utils.R @@ -124,15 +124,15 @@ assert_probably_storr_driver <- function(x, name = deparse(substitute(x))) { assert_custom_mangler <- function(mangler, mangle_key) { if (is.null(mangler)) { stop( - sprintf("No mangler set. Expected '%s'.", mangle_key), - "Set a key mangler with register_mangler().", + sprintf("Mangler '%s' not registered. ", mangle_key), + "Registered it with register_mangler().", call. = FALSE ) } - if (identical(mangler$name, mangle_key)) { + if (!identical(mangler$name, mangle_key)) { stop( sprintf( - "New key mangler '%s' disagrees with old mangler '%s'", + "Registered key mangler '%s' contradicts mangle_key ('%s')", mangler$name, mangle_key ), call. = FALSE diff --git a/tests/testthat/helper-storr.R b/tests/testthat/helper-storr.R index 02516ee..68c15f1 100644 --- a/tests/testthat/helper-storr.R +++ b/tests/testthat/helper-storr.R @@ -45,3 +45,27 @@ with_options <- function(opts, code) { on.exit(options(oo)) force(code) } + +test_mangler <- list( + name = "test_mangler", + encode = function(x, pad) { + paste0("test_mangler_", x) + }, + decode = function(x, error) { + gsub(pattern = "^test_mangler_", replacement = "", x = x) + } +) + +test_key <- function(s, raw, encoded) { + key_file <- file.path( + s$driver$path, + "keys", + s$default_namespace, + encoded + ) + testthat::expect_false(file.exists(key_file)) + s$set(raw, "x") + testthat::expect_true(file.exists(key_file)) + testthat::expect_equal(s$get(raw), "x") + testthat::expect_equal(s$list(), raw) +} diff --git a/tests/testthat/test-custom-manglers.R b/tests/testthat/test-custom-manglers.R new file mode 100644 index 0000000..c8ad943 --- /dev/null +++ b/tests/testthat/test-custom-manglers.R @@ -0,0 +1,54 @@ +context("test-custom-manglers") + +test_that("ignore registered mangler on a new storr", { + on.exit(options(storr_mangler = NULL)) + do.call(register_mangler, test_mangler) + s <- storr_rds(tempfile(), mangle_key = "none") + test_key(s, "a", "a") +}) + +test_that("use registered mangler on a new storr", { + on.exit(options(storr_mangler = NULL)) + do.call(register_mangler, test_mangler) + s <- storr_rds(tempfile(), mangle_key = "test_mangler") + test_key(s, "a", "test_mangler_a") +}) + +test_that("unregistered mangler and new storr", { + expect_null(getOption("storr_mangler")) + expect_error( + storr_rds(tempfile(), mangle_key = "test_mangler"), + regex = "not registered" + ) +}) + +test_that("new mangler registry contradicts old mangler", { + on.exit(options(storr_mangler = NULL)) + do.call(register_mangler, test_mangler) + s <- storr_rds(tempfile(), mangle_key = "test_mangler") + do.call( + register_mangler, + list(name = "test_mangler_2", encode = I, decode = I) + ) + test_key(s, "a", "test_mangler_a") +}) + +test_that("new custom mangler conflicts with old preset", { + on.exit(options(storr_mangler = NULL)) + for (mangle_key in c("none", "base64", "test_mangler")) { + if (identical(mangle_key, "test_mangler")) { + do.call(register_mangler, test_mangler) + } + s <- storr_rds(tempfile(), mangle_key = mangle_key) + with_options(list(storr_mangler = test_mangler), { + expect_error( + s <- storr_rds(s$driver$path, mangle_key = "test_mangler_2"), + regex = "Incompatible value for mangle_key" + ) + }) + } + expect_error( + s <- storr_rds(s$driver$path, mangle_key = TRUE), + regex = "Incompatible value for mangle_key" + ) +}) From efd5287e60003f0587754dcedc8824cd260d33ad Mon Sep 17 00:00:00 2001 From: wlandau Date: Tue, 6 Nov 2018 07:19:43 -0500 Subject: [PATCH 07/11] Ensure we can recover old manglers --- R/driver_rds.R | 21 ++++++++++----------- tests/testthat/helper-storr.R | 3 ++- tests/testthat/test-custom-manglers.R | 16 +++++++++++++++- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/R/driver_rds.R b/R/driver_rds.R index 9621c7e..dec6c59 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -168,8 +168,11 @@ R6_driver_rds <- R6::R6Class( assert_is(mangle_key, c("logical", "character")) } self$mangle_key <- driver_rds_config(path, "mangle_key", mangle_key, - FALSE, TRUE) - + "FALSE", TRUE) + if (self$mangle_key %in% c("TRUE", "FALSE")){ + self$mangle_key <- as.logical(self$mangle_key) + } + if (!is.null(mangle_key_pad)) { assert_scalar_logical(mangle_key_pad) } @@ -335,21 +338,17 @@ R6_driver_rds <- R6::R6Class( ## from the existing storr's mangledness. driver_rds_config <- function(path, name, value, default, must_agree) { path_opt <- driver_rds_config_file(path, name) - + load_value <- function() { if (file.exists(path_opt)) { - value_prev <- tmp <- readLines(path_opt) - storage.mode(tmp) <- storage.mode(default) - if (is.na(tmp)) { - stop(ConfigError(name, value_prev, value)) - } - value <- tmp + value <- readLines(path_opt) + storage.mode(value) <- storage.mode(default) } else { value <- default } value } - + if (is.null(value)) { value <- load_value() } else if (must_agree && file.exists(path_opt)) { @@ -361,7 +360,7 @@ driver_rds_config <- function(path, name, value, default, must_agree) { if (!file.exists(path_opt)) { writeLines(as.character(value), path_opt) } - + value } diff --git a/tests/testthat/helper-storr.R b/tests/testthat/helper-storr.R index 68c15f1..895f627 100644 --- a/tests/testthat/helper-storr.R +++ b/tests/testthat/helper-storr.R @@ -67,5 +67,6 @@ test_key <- function(s, raw, encoded) { s$set(raw, "x") testthat::expect_true(file.exists(key_file)) testthat::expect_equal(s$get(raw), "x") - testthat::expect_equal(s$list(), raw) + testthat::expect_true(s$exists(raw)) + testthat::expect_true(raw %in% s$list()) } diff --git a/tests/testthat/test-custom-manglers.R b/tests/testthat/test-custom-manglers.R index c8ad943..685062b 100644 --- a/tests/testthat/test-custom-manglers.R +++ b/tests/testthat/test-custom-manglers.R @@ -14,7 +14,21 @@ test_that("use registered mangler on a new storr", { test_key(s, "a", "test_mangler_a") }) -test_that("unregistered mangler and new storr", { +test_that("recover an old custom mangler", { + on.exit(options(storr_mangler = NULL)) + do.call(register_mangler, test_mangler) + s <- storr_rds(tempfile(), mangle_key = "test_mangler") + test_key(s, "a", "test_mangler_a") + s <- storr_rds(s$driver$path, mangle_key = "test_mangler") + expect_equal(s$get("a"), "x") + test_key(s, "b", "test_mangler_b") + s <- storr_rds(s$driver$path) + expect_equal(s$get("a"), "x") + expect_equal(s$get("b"), "x") + test_key(s, "c", "test_mangler_c") +}) + +test_that("unregistered mangler and a new storr", { expect_null(getOption("storr_mangler")) expect_error( storr_rds(tempfile(), mangle_key = "test_mangler"), From de4b8e4f1379768eb82ec26245359fffb631f652 Mon Sep 17 00:00:00 2001 From: wlandau Date: Tue, 6 Nov 2018 07:34:11 -0500 Subject: [PATCH 08/11] Add a warning when overwriting the key mangler option --- R/driver_rds.R | 17 ++++++++++---- R/utils.R | 11 ++------- tests/testthat/test-custom-manglers.R | 34 +++++++++++++++++++++++++-- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/R/driver_rds.R b/R/driver_rds.R index dec6c59..85f16b8 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -172,7 +172,7 @@ R6_driver_rds <- R6::R6Class( if (self$mangle_key %in% c("TRUE", "FALSE")){ self$mangle_key <- as.logical(self$mangle_key) } - + if (!is.null(mangle_key_pad)) { assert_scalar_logical(mangle_key_pad) } @@ -338,7 +338,7 @@ R6_driver_rds <- R6::R6Class( ## from the existing storr's mangledness. driver_rds_config <- function(path, name, value, default, must_agree) { path_opt <- driver_rds_config_file(path, name) - + load_value <- function() { if (file.exists(path_opt)) { value <- readLines(path_opt) @@ -348,7 +348,7 @@ driver_rds_config <- function(path, name, value, default, must_agree) { } value } - + if (is.null(value)) { value <- load_value() } else if (must_agree && file.exists(path_opt)) { @@ -360,7 +360,7 @@ driver_rds_config <- function(path, name, value, default, must_agree) { if (!file.exists(path_opt)) { writeLines(as.character(value), path_opt) } - + value } @@ -495,6 +495,15 @@ register_mangler <- function(name, encode, decode, overwrite = FALSE) { if (is.list(current) && !overwrite){ return() } + if (overwrite) { + warning( + sprintf( + "Overwriting existing key mangler '%s' with '%s'", + current$name, name + ), + call. = FALSE + ) + } options(storr_mangler = list(name = name, encode = encode, decode = decode)) invisible() } diff --git a/R/utils.R b/R/utils.R index 44f82a7..3d6a54e 100644 --- a/R/utils.R +++ b/R/utils.R @@ -26,12 +26,6 @@ vcapply <- function(X, FUN, ...) { vapply(X, FUN, character(1), ...) } -assert_identical <- function(x, y) { - if (!identical(x, y)) { - stop(sprintf("'%s' and '%s' must be identical", name), call. = FALSE) - } -} - assert_scalar <- function(x, name = deparse(substitute(x))) { if (length(x) != 1) { stop(sprintf("'%s' must be a scalar", name), call. = FALSE) @@ -132,12 +126,12 @@ assert_custom_mangler <- function(mangler, mangle_key) { if (!identical(mangler$name, mangle_key)) { stop( sprintf( - "Registered key mangler '%s' contradicts mangle_key ('%s')", + "Registered key mangler '%s' conflicts with mangle_key ('%s')", mangler$name, mangle_key ), call. = FALSE ) - } + } } @@ -214,4 +208,3 @@ use_base64_mangler <- function(mangle_key) { identical(mangle_key, TRUE) || identical(mangle_key, "base64") } - diff --git a/tests/testthat/test-custom-manglers.R b/tests/testthat/test-custom-manglers.R index 685062b..c21e58f 100644 --- a/tests/testthat/test-custom-manglers.R +++ b/tests/testthat/test-custom-manglers.R @@ -36,7 +36,37 @@ test_that("unregistered mangler and a new storr", { ) }) -test_that("new mangler registry contradicts old mangler", { +test_that("mangle conflicts with mangle_key a new storr", { + on.exit(options(storr_mangler = NULL)) + do.call(register_mangler, test_mangler) + expect_error( + storr_rds(tempfile(), mangle_key = "test_mangler_2"), + regex = "conflicts with mangle_key" + ) +}) + +test_that("overwriting registered manglers", { + on.exit(options(storr_mangler = NULL)) + do.call(register_mangler, test_mangler) + test_mangler_2 <- test_mangler + test_mangler_2$name <- "test_mangler_2" + do.call(register_mangler, test_mangler_2) + s <- storr_rds(tempfile(), mangle_key = "test_mangler") + test_key(s, "a", "test_mangler_a") + test_mangler_2$overwrite = TRUE + expect_warning( + do.call(register_mangler, test_mangler_2), + regex = "Overwriting existing key mangler" + ) + expect_error( + storr_rds(tempfile(), mangle_key = "test_mangler"), + regex = "conflicts with mangle_key" + ) + s <- storr_rds(tempfile(), mangle_key = "test_mangler_2") + test_key(s, "a", "test_mangler_a") +}) + +test_that("new mangler global option conflicts with old stored mangler", { on.exit(options(storr_mangler = NULL)) do.call(register_mangler, test_mangler) s <- storr_rds(tempfile(), mangle_key = "test_mangler") @@ -47,7 +77,7 @@ test_that("new mangler registry contradicts old mangler", { test_key(s, "a", "test_mangler_a") }) -test_that("new custom mangler conflicts with old preset", { +test_that("new custom mangler conflicts with old stored one", { on.exit(options(storr_mangler = NULL)) for (mangle_key in c("none", "base64", "test_mangler")) { if (identical(mangle_key, "test_mangler")) { From 00c6818cf1deff092d2648b65769ae9e2c8e17bd Mon Sep 17 00:00:00 2001 From: wlandau Date: Tue, 6 Nov 2018 07:36:04 -0500 Subject: [PATCH 09/11] Document register_mangler(overwrite) --- R/driver_rds.R | 2 ++ man/register_mangler.Rd | 3 +++ 2 files changed, 5 insertions(+) diff --git a/R/driver_rds.R b/R/driver_rds.R index 85f16b8..ee30ff8 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -490,6 +490,8 @@ See 'Corrupt keys' within ?storr_rds for how to proceed" -> fmt #' and \code{pad}. #' @param decode function to decode keys. Must have arguments \code{x} #' and \code{error} +#' @param overwrite logical, whether to overwrite a previously +#' registered mangler. register_mangler <- function(name, encode, decode, overwrite = FALSE) { current <- getOption("storr_mangler") if (is.list(current) && !overwrite){ diff --git a/man/register_mangler.Rd b/man/register_mangler.Rd index e3723f4..f85a0e5 100644 --- a/man/register_mangler.Rd +++ b/man/register_mangler.Rd @@ -14,6 +14,9 @@ and \code{pad}.} \item{decode}{function to decode keys. Must have arguments \code{x} and \code{error}} + +\item{overwrite}{logical, whether to overwrite a previously +registered mangler.} } \value{ nothing From 5dc76a5bf3f882f6047f6414900e03444fe9b9b5 Mon Sep 17 00:00:00 2001 From: wlandau Date: Tue, 6 Nov 2018 08:08:35 -0500 Subject: [PATCH 10/11] Document custom mangling Details in the `storr_rds()` help file. Examples in the `register_mangler()` help file. --- R/driver_rds.R | 72 +++++++++++++++++++++++++++++++---------- man/register_mangler.Rd | 27 ++++++++++++++++ man/storr_rds.Rd | 21 +++++++++--- 3 files changed, 99 insertions(+), 21 deletions(-) diff --git a/R/driver_rds.R b/R/driver_rds.R index ee30ff8..68852f4 100644 --- a/R/driver_rds.R +++ b/R/driver_rds.R @@ -21,14 +21,24 @@ ##' is set. Using \code{mangle_key = NULL} uses whatever mangledness ##' exists (or no mangledness if creating a new storr). ##' +##' If base64 encoding does not meet your use case +##' (e.g. if it is too slow) then you can supply custom functions +##' to encode and decode the keys. +##' First, register your encoder and decoder functions +##' with \code{\link{register_mangler}()}. +##' Next, set \code{mangle_key} equal to the \code{name} +##' you gave \code{\link{register_mangler}()}. +##' ##' @section Corrupt keys: ##' ##' Some file synchronisation utilities like dropbox can create file ##' that confuse an rds storr (e.g., ##' \code{"myobject (Someone's conflicted copy)"}. If -##' \code{mangle_key} is \code{FALSE} these cannot be detected but at +##' \code{mangle_key} is \code{"none"}, +##' these cannot be detected but at ##' the same time are not a real problem for storr. However, if -##' \code{mangle_key} is \code{TRUE} and keys are base64 encoded then +##' \code{mangle_key} is \code{"base64"} +##' and keys are base64 encoded then ##' these conflicted copies can break parts of storr. ##' ##' If you see a warning asking you to deal with these files, please @@ -57,8 +67,11 @@ ##' @param compress Compress the generated file? This saves a small ##' amount of space for a reasonable amount of time. ##' -##' @param mangle_key Mangle keys? If TRUE, then the key is encoded -##' using base64 before saving to the filesystem. See Details. +##' @param mangle_key Mangle keys? If \code{"base64"}, +##' then the key is encoded using base64 before saving to the filesystem. +##' If \code{"none"}, then keys are not encoded. If some other string, +##' \code{storr} will use a custom key mangler set up with +##' \code{\link{register_mangler}()}. See Details. ##' ##' @param mangle_key_pad Logical indicating if the filenames created ##' when using \code{mangle_key} should also be "padded" with the @@ -252,7 +265,7 @@ R6_driver_rds <- R6::R6Class( path <- file.path(self$path, "keys", namespace) files <- dir(path) ret <- self$mangler$decode(x = files, error = FALSE) - if (use_base64_mangler(self$mangle_key)) { + if (!use_no_mangler(self$mangle_key)) { if (anyNA(ret)) { message_corrupted_rds_keys(namespace, path, files[is.na(ret)]) ret <- ret[!is.na(ret)] @@ -270,7 +283,7 @@ R6_driver_rds <- R6::R6Class( }, purge_corrupt_keys = function(namespace) { - if (use_base64_mangler(self$mangle_key)) { + if (!use_no_mangler(self$mangle_key)) { path <- file.path(self$path, "keys", namespace) files <- dir(path) i <- is.na(self$mangler$decode(files, error = FALSE)) @@ -481,17 +494,42 @@ See 'Corrupt keys' within ?storr_rds for how to proceed" -> fmt corrupt_notices[[path]] <- now } -#' @title Register a key mangler -#' @description Define custom functinons for mangling \code{storr_rds()} keys. -#' @export -#' @return nothing -#' @param name character scalar, name of the mangler -#' @param encode function to encode keys. Must have arguments \code{x} -#' and \code{pad}. -#' @param decode function to decode keys. Must have arguments \code{x} -#' and \code{error} -#' @param overwrite logical, whether to overwrite a previously -#' registered mangler. +##' @title Register a key mangler +##' @description Define custom functinons for mangling \code{storr_rds()} keys. +##' @details +##' \code{storr_rds(mangle_key = "base64")} encodes keys using base64 +##' encoding. This precaution ensures that the names of key files +##' do not have illegal character names such as ":". However, +##' base 64 encoding can be slow for some applications, +##' so you have the option of setting your own custom key mangler. +##' First, create functions to encode and decode keys and register them +##' with \code{\link{register_mangler}()}. +##' Next, set \code{mangle_key} equal to the \code{name} +##' you gave \code{\link{register_mangler}()}. +##' @export +##' @return nothing +##' @param name character scalar, name of the mangler +##' @param encode function to encode keys. Must have arguments \code{x} +##' and \code{pad}. +##' @param decode function to decode keys. Must have arguments \code{x} +##' and \code{error} +##' @param overwrite logical, whether to overwrite a previously +##' registered mangler. +##' @examples +##' register_mangler( +##' "test_mangler", +##' encode = function (x, pad) { +##' paste0("x_", x) +##' }, +##' decode = function(x, error) { +##' substr(x, start = 3, stop = 1e7) +##' } +##' ) +##' s <- storr_rds(tempfile(), mangle_key = "test_mangler") +##' s$set("a", 1) +##' s$get("a") +##' list.files(file.path(s$driver$path, "keys", s$default_namespace)) +##' options(storr_mangler = NULL) register_mangler <- function(name, encode, decode, overwrite = FALSE) { current <- getOption("storr_mangler") if (is.list(current) && !overwrite){ diff --git a/man/register_mangler.Rd b/man/register_mangler.Rd index f85a0e5..fd6fd44 100644 --- a/man/register_mangler.Rd +++ b/man/register_mangler.Rd @@ -24,3 +24,30 @@ nothing \description{ Define custom functinons for mangling \code{storr_rds()} keys. } +\details{ +\code{storr_rds(mangle_key = "base64")} encodes keys using base64 +encoding. This precaution ensures that the names of key files +do not have illegal character names such as ":". However, +base 64 encoding can be slow for some applications, +so you have the option of setting your own custom key mangler. +First, create functions to encode and decode keys and register them +with \code{\link{register_mangler}()}. +Next, set \code{mangle_key} equal to the \code{name} +you gave \code{\link{register_mangler}()}. +} +\examples{ +register_mangler( + "test_mangler", + encode = function (x, pad) { + paste0("x_", x) + }, + decode = function(x, error) { + substr(x, start = 3, stop = 1e7) + } +) +s <- storr_rds(tempfile(), mangle_key = "test_mangler") +s$set("a", 1) +s$get("a") +list.files(file.path(s$driver$path, "keys", s$default_namespace)) +options(storr_mangler = NULL) +} diff --git a/man/storr_rds.Rd b/man/storr_rds.Rd index a5c4e23..738137f 100644 --- a/man/storr_rds.Rd +++ b/man/storr_rds.Rd @@ -20,8 +20,11 @@ might be nice for persistent application data.} \item{compress}{Compress the generated file? This saves a small amount of space for a reasonable amount of time.} -\item{mangle_key}{Mangle keys? If TRUE, then the key is encoded -using base64 before saving to the filesystem. See Details.} +\item{mangle_key}{Mangle keys? If \code{"base64"}, +then the key is encoded using base64 before saving to the filesystem. +If \code{"none"}, then keys are not encoded. If some other string, +\code{storr} will use a custom key mangler set up with +\code{\link{register_mangler}()}. See Details.} \item{mangle_key_pad}{Logical indicating if the filenames created when using \code{mangle_key} should also be "padded" with the @@ -64,6 +67,14 @@ not safe to use the same path for a storr with and without mangling. So once an rds storr has been created its "mangledness" is set. Using \code{mangle_key = NULL} uses whatever mangledness exists (or no mangledness if creating a new storr). + +If base64 encoding does not meet your use case +(e.g. if it is too slow) then you can supply custom functions +to encode and decode the keys. +First, register your encoder and decoder functions +with \code{\link{register_mangler}()}. +Next, set \code{mangle_key} equal to the \code{name} +you gave \code{\link{register_mangler}()}. } \section{Corrupt keys}{ @@ -71,9 +82,11 @@ exists (or no mangledness if creating a new storr). Some file synchronisation utilities like dropbox can create file that confuse an rds storr (e.g., \code{"myobject (Someone's conflicted copy)"}. If -\code{mangle_key} is \code{FALSE} these cannot be detected but at +\code{mangle_key} is \code{"none"}, +these cannot be detected but at the same time are not a real problem for storr. However, if -\code{mangle_key} is \code{TRUE} and keys are base64 encoded then +\code{mangle_key} is \code{"base64"} +and keys are base64 encoded then these conflicted copies can break parts of storr. If you see a warning asking you to deal with these files, please From d0bb797463a0422845a866c13bef69e95eab077b Mon Sep 17 00:00:00 2001 From: wlandau Date: Tue, 6 Nov 2018 08:12:35 -0500 Subject: [PATCH 11/11] Update news --- NEWS.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 320ed08..f4299aa 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,8 @@ -## storr 1.2.1 (2018-??-??) +## storr 1.2.2 (????-??-??) + +- Supply custom key manglers with `register_mangler()` (#88, @wlandau). + +## storr 1.2.1 (2018-10-18) * Avoid a race condition in writing to rds storrs in parallel (#80, reported by @wlandau)