Allow the creation of R Clients from other libraries (useful for DnD …

…and in general) (#4441) * Update R client to some macro renaming in C++ client. * Modifications to allow rdnd to create rdeephaven Client. * Use SEXP. * Add comments. * Followup to review comments. * Followup to review comments. * Add test for make_table_handle_from_ticket.
deephaven · Sep 6, 2023 · fa4fee9 · fa4fee9
1 parent 46b96f6
commit fa4fee9
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 50 deletions.
diff --git a/R/rdeephaven/DESCRIPTION b/R/rdeephaven/DESCRIPTION
@@ -1,7 +1,7 @@
 Package: rdeephaven
 Type: Package
 Title: R Client for Deephaven Core
-Version: 0.27.1
+Version: 0.28
 Date: 2023-05-12
 Author: Deephaven Data Labs
 Maintainer: Alex Peters <alexpeters@deephaven.io>
@@ -13,7 +13,7 @@ Description: The `rdeephaven` package provides an R API for communicating with t
              and bind it to a server-side variable so you can access it from any Deephaven client. Finally, you can run Python or Groovy
              scripts on the Deephaven server, so long as your server is equipped with that capability.
 License: Apache License (== 2.0)
-Depends: R (> 4.1.2), Rcpp (>= 1.0.10), arrow (>= 12.0.0), R6 (>= 2.5.0), dplyr (>= 1.1.0)
+Depends: R (>= 4.1.2), Rcpp (>= 1.0.10), arrow (>= 12.0.0), R6 (>= 2.5.0), dplyr (>= 1.1.0)
 Imports: Rcpp (>= 1.0.10), arrow (>= 12.0.0), R6 (>= 2.5.0), dplyr (>= 1.1.0)
 LinkingTo: Rcpp
 Suggests: testthat (>= 3.0.0)

diff --git a/R/rdeephaven/R/client_wrapper.R b/R/rdeephaven/R/client_wrapper.R
@@ -3,7 +3,28 @@ Client <- R6Class("Client",
   cloneable = FALSE,
   public = list(
     .internal_rcpp_object = NULL,
-    initialize = function(target,
+    initialize = function(...) {
+      args <- list(...)
+      if (length(args) == 1) {
+        first_arg <- args[[1]]
+        first_arg_class = first_class(first_arg)
+        if (first_arg_class != "character" && first_arg_class != "list") {
+          if (first_arg_class != "externalptr") {
+            stop(paste0(
+              "Client initialize first argument must be ",
+              "either a string or an Rcpp::XPtr object."))
+          }
+          return(self$initialize_for_xptr(first_arg))
+        }
+      }
+      return(do.call(self$initialize_for_target, args))
+    },
+    initialize_for_xptr = function(xptr) {
+      verify_type("xptr", xptr, "externalptr", "XPtr", TRUE)
+      self$.internal_rcpp_object = new(INTERNAL_Client, xptr)
+    },
+    initialize_for_target = function(
+                          target,
                           auth_type = "anonymous",
                           username = "",
                           password = "",
@@ -149,6 +170,10 @@ Client <- R6Class("Client",
         stop(paste0("'table_object' must be a single data frame, tibble, arrow table, or record batch reader. Got an object of class ", table_object_class[[1]], "."))
       }
     },
+    make_table_handle_from_ticket = function(ticket) {
+      verify_string("ticket", ticket, TRUE)
+      return(TableHandle$new(self$.internal_rcpp_object$make_table_handle_from_ticket(ticket)))
+    },
     run_script = function(script) {
       verify_string("script", script, TRUE)
       self$.internal_rcpp_object$run_script(script)

diff --git a/R/rdeephaven/inst/tests/testthat/test_client_wrapper.R b/R/rdeephaven/inst/tests/testthat/test_client_wrapper.R
@@ -1,6 +1,8 @@
 library(testthat)
 library(rdeephaven)
 
+target <- "localhost:10000"
+
 setup <- function() {
   df1 <- data.frame(
     string_col = c("I", "am", "a", "string", "column"),
@@ -30,14 +32,14 @@ setup <- function() {
 test_that("client dhConnection works in the simple case of anonymous authentication", {
 
   # TODO: assumes server is actually running on localhost:10000, this is probably bad for CI
-  expect_no_error(client <- Client$new(target = "localhost:10000"))
+  expect_no_error(client <- Client$new(target = target))
 
 })
 
 test_that("import_table does not fail with data frame inputs of simple column types", {
   data <- setup()
 
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_no_error(client$import_table(data$df1))
   expect_no_error(client$import_table(data$df2))
@@ -50,7 +52,7 @@ test_that("import_table does not fail with data frame inputs of simple column ty
 test_that("import_table does not fail with tibble inputs of simple column types", {
   data <- setup()
 
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_no_error(client$import_table(as_tibble(data$df1)))
   expect_no_error(client$import_table(as_tibble(data$df2)))
@@ -63,7 +65,7 @@ test_that("import_table does not fail with tibble inputs of simple column types"
 test_that("import_table does not fail with arrow table inputs of simple column types", {
   data <- setup()
 
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_no_error(client$import_table(as_arrow_table(data$df1)))
   expect_no_error(client$import_table(as_arrow_table(data$df2)))
@@ -76,7 +78,7 @@ test_that("import_table does not fail with arrow table inputs of simple column t
 test_that("import_table does not fail with record batch reader inputs of simple column types", {
   data <- setup()
 
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_no_error(client$import_table(as_record_batch_reader(data$df1)))
   expect_no_error(client$import_table(as_record_batch_reader(data$df2)))
@@ -93,7 +95,7 @@ test_that("import_table does not fail with record batch reader inputs of simple
 test_that("open_table opens the correct table from the server", {
   data <- setup()
 
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   th1 <- client$import_table(data$df1)
   th1$bind_to_variable("table1")
@@ -115,7 +117,7 @@ test_that("open_table opens the correct table from the server", {
 })
 
 test_that("empty_table correctly creates tables on the server", {
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   th1 <- client$empty_table(10)$update("X = i")
   df1 <- data.frame(X = c(0, 1, 2, 3, 4, 5, 6, 7, 8, 9))
@@ -127,7 +129,7 @@ test_that("empty_table correctly creates tables on the server", {
 # TODO: Test time_table good inputs
 
 test_that("run_script correctly runs a python script", {
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_no_error(client$run_script(
     '
@@ -151,91 +153,91 @@ int_col("Name_Int_Col", [44, 55, 66])
 test_that("client constructor fails nicely with bad inputs", {
 
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "basic"),
+    Client$new(target = target, auth_type = "basic"),
     "Basic authentication was requested, but 'auth_token' was not provided, and at most one of 'username' or 'password' was provided. Please provide either 'username' and 'password', or 'auth_token'."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "basic", username = "user"),
+    Client$new(target = target, auth_type = "basic", username = "user"),
     "Basic authentication was requested, but 'auth_token' was not provided, and at most one of 'username' or 'password' was provided. Please provide either 'username' and 'password', or 'auth_token'."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "basic", password = "pass"),
+    Client$new(target = target, auth_type = "basic", password = "pass"),
     "Basic authentication was requested, but 'auth_token' was not provided, and at most one of 'username' or 'password' was provided. Please provide either 'username' and 'password', or 'auth_token'."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "basic", username = "user", auth_token = "token"),
+    Client$new(target = target, auth_type = "basic", username = "user", auth_token = "token"),
     "Basic authentication was requested, but 'auth_token' was provided, as well as least one of 'username' and 'password'. Please provide either 'username' and 'password', or 'auth_token'."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "basic", password = "pass", auth_token = "token"),
+    Client$new(target = target, auth_type = "basic", password = "pass", auth_token = "token"),
     "Basic authentication was requested, but 'auth_token' was provided, as well as least one of 'username' and 'password'. Please provide either 'username' and 'password', or 'auth_token'."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "basic", username = "user", password = "pass", auth_token = "token"),
+    Client$new(target = target, auth_type = "basic", username = "user", password = "pass", auth_token = "token"),
     "Basic authentication was requested, but 'auth_token' was provided, as well as least one of 'username' and 'password'. Please provide either 'username' and 'password', or 'auth_token'."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "psk"),
+    Client$new(target = target, auth_type = "psk"),
     "Pre-shared key authentication was requested, but no 'auth_token' was provided."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "custom"),
+    Client$new(target = target, auth_type = "custom"),
     "Custom authentication was requested, but no 'auth_token' was provided."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = ""),
+    Client$new(target = target, auth_type = ""),
     "'auth_type' should be a non-empty string."
   )
   expect_error(
-    Client$new(target = "localhost:10000", auth_type = "basic", auth_token = 1234),
+    Client$new(target = target, auth_type = "basic", auth_token = 1234),
     "'auth_token' must be a single string. Got an object of class numeric."
   )
   expect_error(
-    Client$new(target = "localhost:10000", session_type = "blahblah"),
+    Client$new(target = target, session_type = "blahblah"),
     "'session_type' must be 'python' or 'groovy', but got blahblah."
   )
   expect_error(
-    Client$new(target = "localhost:10000", session_type = 1234),
+    Client$new(target = target, session_type = 1234),
     "'session_type' must be 'python' or 'groovy', but got 1234."
   )
   expect_error(
-    Client$new(target = "localhost:10000", use_tls = "banana"),
+    Client$new(target = target, use_tls = "banana"),
     "'use_tls' must be a single boolean. Got an object of class character."
   )
   expect_error(
-    Client$new(target = "localhost:10000", int_options = 1234),
+    Client$new(target = target, int_options = 1234),
     "'int_options' must be a named list. Got an object of class numeric."
   )
   expect_error(
-    Client$new(target = "localhost:10000", int_options = list(1, 2, 3)),
+    Client$new(target = target, int_options = list(1, 2, 3)),
     "'int_options' must be a named list. Got a list with 3 elements and 0 names."
   )
   expect_error(
-    Client$new(target = "localhost:10000", int_options = list(a = 12.34)),
+    Client$new(target = target, int_options = list(a = 12.34)),
     "'value' must be an integer. Got 'value' = 12.34."
   )
   expect_error(
-    Client$new(target = "localhost:10000", string_options = 1234),
+    Client$new(target = target, string_options = 1234),
     "'string_options' must be a named list. Got an object of class numeric."
   )
   expect_error(
-    Client$new(target = "localhost:10000", string_options = list("a", "b", "c")),
+    Client$new(target = target, string_options = list("a", "b", "c")),
     "'string_options' must be a named list. Got a list with 3 elements and 0 names."
   )
   expect_error(
-    Client$new(target = "localhost:10000", string_options = list(a = 123)),
+    Client$new(target = target, string_options = list(a = 123)),
     "'value' must be a single string. Got an object of class numeric."
   )
   expect_error(
-    Client$new(target = "localhost:10000", extra_headers = 1234),
+    Client$new(target = target, extra_headers = 1234),
     "'extra_headers' must be a named list. Got an object of class numeric."
   )
   expect_error(
-    Client$new(target = "localhost:10000", extra_headers = list("a", "b", "c")),
+    Client$new(target = target, extra_headers = list("a", "b", "c")),
     "'extra_headers' must be a named list. Got a list with 3 elements and 0 names."
   )
   expect_error(
-    Client$new(target = "localhost:10000", extra_headers = list(a = 123)),
+    Client$new(target = target, extra_headers = list(a = 123)),
     "'value' must be a single string. Got an object of class numeric."
   )
 
@@ -244,7 +246,7 @@ test_that("client constructor fails nicely with bad inputs", {
 test_that("import_table fails nicely with bad inputs", {
   library(datasets)
 
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_error(client$import_table(12345), "'table_object' must be a single data frame, tibble, arrow table, or record batch reader. Got an object of class numeric.")
   expect_error(client$import_table(c("I", "am", "a", "string")), "'table_object' must be a single data frame, tibble, arrow table, or record batch reader. Got an object of class character.")
@@ -256,7 +258,7 @@ test_that("import_table fails nicely with bad inputs", {
 })
 
 test_that("open_table fails nicely with bad inputs", {
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_error(client$open_table(""), "The table '' does not exist on the server.")
   expect_error(client$open_table(12345), "'name' must be a single string. Got an object of class numeric.")
@@ -266,7 +268,7 @@ test_that("open_table fails nicely with bad inputs", {
 })
 
 test_that("empty_table fails nicely with bad inputs", {
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_error(client$empty_table(-3), "'size' must be a nonnegative integer. Got 'size' = -3.")
   expect_error(client$empty_table(1.2345), "'size' must be an integer. Got 'size' = 1.2345.")
@@ -277,7 +279,7 @@ test_that("empty_table fails nicely with bad inputs", {
 })
 
 test_that("time_table fails nicely with bad inputs", {
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_error(client$time_table(1000), "'period' must be a single string. Got an object of class numeric.")
   expect_error(client$time_table("PT1s", 10000), "'start_time' must be a single string. Got an object of class numeric.")
@@ -288,10 +290,37 @@ test_that("time_table fails nicely with bad inputs", {
 })
 
 test_that("run_script fails nicely with bad input types", {
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = target)
 
   expect_error(client$run_script(12345), "'script' must be a single string. Got an object of class numeric.")
   expect_error(client$run_script(c("I", "am", "a", "string")), "'script' must be a single string. Got a vector of length 4.")
 
   client$close()
 })
+
+test_that("Running Client$new with wrong argument types gives good errors", {
+  expect_error(Client$new(12345),
+    "Client initialize first argument must be either a string or an Rcpp::XPtr object.")
+})
+
+test_that("A Client created from an Rcpp::XPtr is functional.", {
+  client <- Client$new(target = target)
+  client$empty_table(1)$update("A = 42")$bind_to_variable("t")
+  client_xptr <- client$.internal_rcpp_object$internal_client()
+  client2 <- Client$new(client_xptr)
+  t <- client2$open_table("t")
+  df <- t$as_data_frame()
+  expect_true(df[1,1] == 42)
+  client$close()
+})
+
+test_that("make_table_handle_from_ticket works.", {
+  client <- Client$new(target = target)
+  client$empty_table(1)$update("A = 43")$bind_to_variable("t")
+  t <- client$make_table_handle_from_ticket("s/t")
+  df <- t$as_data_frame()
+  expect_true(df[1,1] == 43)
+  client$close()
+})
+
+rm(target)