diff --git a/R/rdeephaven/DESCRIPTION b/R/rdeephaven/DESCRIPTION index 63ba87a9dfc..3a8a4043b55 100644 --- 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 @@ -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 index 2297ca31d7f..2b4b3134ef0 100644 --- 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 index 2b20984d571..081c7006c2f 100644 --- 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) diff --git a/R/rdeephaven/src/client.cpp b/R/rdeephaven/src/client.cpp index ff0076234c8..46d13898900 100644 --- a/R/rdeephaven/src/client.cpp +++ b/R/rdeephaven/src/client.cpp @@ -5,9 +5,10 @@ */ #include -#include #include +#include #include +#include #include #include "deephaven/client/client.h" @@ -23,7 +24,6 @@ using deephaven::dhcore::utility::Base64Encode; // forward declaration of classes -class AggregateWrapper; class TableHandleWrapper; class ClientOptionsWrapper; class ClientWrapper; @@ -32,7 +32,6 @@ class ClientWrapper; std::vector convertRcppListToVectorOfTypeAggregate(Rcpp::List rcpp_list); std::vector convertRcppListToVectorOfTypeTableHandle(Rcpp::List rcpp_list); - // ######################### DH WRAPPERS ######################### class AggregateWrapper { @@ -116,7 +115,7 @@ AggregateWrapper* INTERNAL_agg_count(std::string columnSpec) { class TableHandleWrapper { public: TableHandleWrapper(deephaven::client::TableHandle ref_table) : - internal_tbl_hdl(std::move(ref_table)) {}; + internal_tbl_hdl(std::move(ref_table)) {}; TableHandleWrapper* Select(std::vector columnSpecs) { return new TableHandleWrapper(internal_tbl_hdl.Select(columnSpecs)); @@ -322,7 +321,7 @@ class ClientOptionsWrapper { public: ClientOptionsWrapper() : - internal_options(std::make_shared()) {} + internal_options(std::make_shared()) {} void SetDefaultAuthentication() { internal_options->SetDefaultAuthentication(); @@ -371,7 +370,24 @@ class ClientWrapper { public: ClientWrapper(std::string target, const ClientOptionsWrapper &client_options) : - internal_client(deephaven::client::Client::Connect(target, *client_options.internal_options)) {} + internal_client( + Rcpp::XPtr( + new deephaven::client::Client( + std::move( + deephaven::client::Client::Connect(target, *client_options.internal_options) + ) + ) + ) + ) {} + + // We need the ability to create a ClientWrapper from the enterprise + // client, when the underlying C++ object is already created. + ClientWrapper(SEXP sexp) : + internal_client(Rcpp::XPtr(sexp)) {} + + SEXP InternalClient() { + return internal_client; + } TableHandleWrapper* OpenTable(std::string tableName) { return new TableHandleWrapper(internal_tbl_hdl_mngr.FetchTable(tableName)); @@ -382,12 +398,17 @@ class ClientWrapper { } TableHandleWrapper* TimeTable(std::string periodISO, std::string startTimeISO) { - if((startTimeISO == "now") || (startTimeISO == "")) { + if ((startTimeISO == "now") || (startTimeISO == "")) { return new TableHandleWrapper(internal_tbl_hdl_mngr.TimeTable(periodISO)); } return new TableHandleWrapper(internal_tbl_hdl_mngr.TimeTable(periodISO, startTimeISO)); }; + + TableHandleWrapper* MakeTableHandleFromTicket(std::string ticket) { + return new TableHandleWrapper(internal_tbl_hdl_mngr.MakeTableHandleFromTicket(ticket)); + } + void RunScript(std::string code) { internal_tbl_hdl_mngr.RunScript(code); } @@ -455,15 +476,17 @@ class ClientWrapper { } void Close() { - internal_client.Close(); + internal_client->Close(); } private: - deephaven::client::Client internal_client; - const deephaven::client::TableHandleManager internal_tbl_hdl_mngr = internal_client.GetManager(); + // We let R manage the lifetime of internal_client underlying C++ object, + // according to its tracking of references. + // We hold one here, but there may be other references in the case of the enterprise client. + Rcpp::XPtr internal_client; + const deephaven::client::TableHandleManager internal_tbl_hdl_mngr = internal_client->GetManager(); }; - // ######################### RCPP GLUE ######################### using namespace Rcpp; @@ -553,14 +576,16 @@ RCPP_MODULE(DeephavenInternalModule) { class_("INTERNAL_Client") .constructor() + .constructor() + .method("internal_client", &ClientWrapper::InternalClient) .method("open_table", &ClientWrapper::OpenTable) .method("empty_table", &ClientWrapper::EmptyTable) .method("time_table", &ClientWrapper::TimeTable) .method("check_for_table", &ClientWrapper::CheckForTable) + .method("make_table_handle_from_ticket", &ClientWrapper::MakeTableHandleFromTicket) .method("run_script", &ClientWrapper::RunScript) .method("new_arrow_array_stream_ptr", &ClientWrapper::NewArrowArrayStreamPtr) .method("new_table_from_arrow_array_stream_ptr", &ClientWrapper::NewTableFromArrowArrayStreamPtr) .method("close", &ClientWrapper::Close) ; - }