Added assertions on input types

DESCRIPTION

@@ -22,6 +22,7 @@ Description:
 Depends:
     R (>= 3.3.0)
 Imports:
+    assertthat (>= 0.2.0),
     data.table,
     futile.logger,
     httr,

NAMESPACE

@@ -7,6 +7,12 @@ export(get_counts)
 export(get_fields)
 export(parse_date_time)
 export(unpack_nested_data)
+importFrom(assertthat,is.count)
+importFrom(assertthat,is.flag)
+importFrom(assertthat,is.number)
+importFrom(assertthat,is.string)
+importFrom(assertthat,is.writeable)
+importFrom(assertthat,see_if)
 importFrom(data.table,":=")
 importFrom(data.table,as.data.table)
 importFrom(data.table,copy)

R/elasticsearch_eda_funs.R

@@ -2,6 +2,7 @@
 #' @title Examine the distribution of distinct values for a field in Elasticsearch
 #' @name get_counts
 #' @description For a given field, return a data.table with its unique values in a time range.
+#' @importFrom assertthat is.count is.number is.string
 #' @importFrom data.table := data.table setnames setkeyv
 #' @importFrom httr content RETRY
 #' @importFrom purrr transpose
@@ -44,6 +45,24 @@ get_counts <- function(field
     # Input checking
     es_host <- .ValidateAndFormatHost(es_host)
 
+    # Other input checks we don't have explicit error messages for
+    .assert(
+        assertthat::is.string(field)
+        , field != ""
+        , assertthat::is.string(es_index)
+        , es_index != ""
+        , assertthat::is.string(start_date)
+        , start_date != ""
+        , assertthat::is.string(end_date)
+        , end_date != ""
+        , assertthat::is.string(time_field)
+        , time_field != ""
+        , assertthat::is.string(use_na)
+        , use_na != ""
+        , assertthat::is.count(max_terms)
+        , max_terms > 0
+    )
+
     #===== Format and execute query =====#
 
     # Support un-dated queries
@@ -137,6 +156,11 @@ get_fields <- function(es_host
     # Input checking
     url <- .ValidateAndFormatHost(es_host)
 
+    .assert(
+        is.character("es_indices")
+        , length(es_indices) > 0
+    )
+
     # collapse character vectors into comma separated strings. If any arguments
     # are NULL, create an empty string
     indices <- paste(es_indices, collapse = ',')

R/elasticsearch_parsers.R

@@ -8,6 +8,7 @@
 #'              
 #'              This is a side-effect-free function: it returns a new data.table and the
 #'              input data.table is unmodified.
+#' @importFrom assertthat is.string
 #' @importFrom data.table copy
 #' @importFrom purrr map2 simplify
 #' @importFrom stringr str_extract
@@ -65,6 +66,12 @@ parse_date_time <- function(input_df
         log_fatal(msg)
     }
 
+    # Other input checks we don't have explicit error messages for
+    .assert(
+        assertthat::is.string(assume_tz)
+        , assume_tz != ""
+    )
+
     # Work on a copy of the DT to avoid side effects
     outDT <- data.table::copy(input_df)
 
@@ -576,6 +583,7 @@ chomp_hits <- function(hits_json = NULL, keep_nested_data_cols = TRUE) {
 #'        want to change this behavior, provide a path here. `es_search` will create 
 #'        and write to a temporary directory under whatever path you provide.
 #' @inheritParams doc_shared
+#' @importFrom assertthat is.count is.flag is.number is.string is.writeable
 #' @importFrom parallel detectCores
 #' @export
 #' @examples
@@ -628,6 +636,28 @@ es_search <- function(es_host
         log_fatal(msg)
     }
 
+    # Other input checks we don't have explicit error messages for
+    .assert(
+        assertthat::is.string(es_host)
+        , es_host != ""
+        , assertthat::is.string(es_index)
+        , es_index != ""
+        , assertthat::is.string(query_body)
+        , query_body != ""
+        , assertthat::is.string(scroll)
+        , scroll != ""
+        , assertthat::is.count(max_hits)
+        , max_hits >= 0
+        , assertthat::is.count(n_cores)
+        , n_cores >= 1
+        , assertthat::is.flag(break_on_duplicates)
+        , !is.na(break_on_duplicates)
+        , assertthat::is.flag(ignore_scroll_restriction)
+        , !is.na(ignore_scroll_restriction)
+        , assertthat::is.string(intermediates_dir)
+        , assertthat::is.writeable(intermediates_dir)
+    )
+
     # Aggregation Request
     if (grepl('aggs', query_body)){
 

R/utils.R

@@ -0,0 +1,21 @@
+
+# [title] assert_that wrapper
+# [name] assert
+# [description] When making an assertion you might call:
+#   
+#   \code{assertthat::assert_that(assertthat::is.date(x))}
+#   
+#   or something like that. This is an alias to \code{\link[assertthat]{assert_that}} to be used
+#   for two benefits: \enumerate{
+#     \item{This uses \code{\link{log_fatal}} instead of \code{\link{stop}} on failure}
+#     \item{Much less clutter in the source code}
+#   }
+#' @importFrom assertthat see_if
+.assert <- function(..., msg = NULL) {
+    res <- assertthat::see_if(..., env = parent.frame(), msg = msg)
+    if (res) {
+        return(invisible(TRUE))
+    } else {
+        log_fatal(attr(res, "msg"))
+    }
+}

tests/testthat/test-elasticsearch_eda_funs.R

@@ -20,7 +20,7 @@ futile.logger::flog.threshold(0)
               {
                   expect_error(get_fields(es_host = "http://es.custdb.mycompany.com:9200"
                                           , es_indices = NULL),
-                               regexp = "get_fields must be passed a valid es_indices")
+                               regexp = "not greater than 0")
                   expect_error(get_fields(es_host = "http://es.custdb.mycompany.com:9200"
                                           , es_indices = ''),
                                regexp = "get_fields must be passed a valid es_indices")