correct type checking for databricks(httpPath) (#787)

r-dbi · Apr 12, 2024 · 8c7a0e2 · 8c7a0e2
1 parent a55ef73
commit 8c7a0e2
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 8 deletions.
diff --git a/R/driver-databricks.R b/R/driver-databricks.R
@@ -65,15 +65,15 @@ setMethod("dbConnect", "DatabricksOdbcDriver",
  uid = NULL,
  pwd = NULL,
  ...) {
+ call <- caller_env()
  # For backward compatibility with RStudio connection string
- check_exclusive(httpPath, HTTPPath)
- check_string(httpPath, allow_null = TRUE)
- check_string(workspace, allow_null = TRUE)
- check_bool(useNativeQuery)
- check_string(driver, allow_null = TRUE)
- check_string(HTTPPath, allow_null = TRUE)
- check_string(uid, allow_null = TRUE)
- check_string(pwd, allow_null = TRUE)
+ http_path <- check_exclusive(httpPath, HTTPPath, .call = call)
+ check_string(get(http_path), allow_null = TRUE, arg = http_path, call = call)
+ check_string(workspace, allow_null = TRUE, call = call)
+ check_bool(useNativeQuery, call = call)
+ check_string(driver, allow_null = TRUE, call = call)
+ check_string(uid, allow_null = TRUE, call = call)
+ check_string(pwd, allow_null = TRUE, call = call)
 
  args <- databricks_args(
  httpPath = if (missing(httpPath)) HTTPPath else httpPath,

diff --git a/tests/testthat/_snaps/driver-databricks.md b/tests/testthat/_snaps/driver-databricks.md
@@ -34,3 +34,27 @@
   ! Both `uid` and `pwd` must be specified for manual authentication.
   i Or leave both unset for automated authentication.
 
+# dbConnect method errors informatively re: httpPath (#787)
+
+ Code
+  dbConnect(databricks(), httpPath = "boop", HTTPPath = "bop")
+ Condition
+  Error in `dbConnect()`:
+  ! Exactly one of `httpPath` or `HTTPPath` must be supplied.
+
+---
+
+ Code
+  dbConnect(databricks(), HTTPPath = 1L)
+ Condition
+  Error in `dbConnect()`:
+  ! `HTTPPath` must be a single string or `NULL`, not the number 1.
+
+---
+
+ Code
+  dbConnect(databricks(), httpPath = 1L)
+ Condition
+  Error in `dbConnect()`:
+  ! `httpPath` must be a single string or `NULL`, not the number 1.
+
diff --git a/tests/testthat/test-driver-databricks.R b/tests/testthat/test-driver-databricks.R
@@ -87,3 +87,25 @@ test_that("supports OAuth M2M in env var", {
  expect_equal(auth$auth_client_id, "abc")
  expect_equal(auth$auth_client_secret, "def")
 })
+
+test_that("dbConnect method handles httpPath aliases (#787)", {
+ local_mocked_bindings(
+ databricks_args = function(...) stop("made it"),
+ configure_spark = function(...) TRUE
+ )
+
+ expect_error(dbConnect(databricks(), HTTPPath = "boop"), "made it")
+ expect_error(dbConnect(databricks(), httpPath = "boop"), "made it")
+})
+
+test_that("dbConnect method errors informatively re: httpPath (#787)", {
+ local_mocked_bindings(configure_spark = function(...) TRUE)
+
+ expect_snapshot(
+ error = TRUE,
+ dbConnect(databricks(), httpPath = "boop", HTTPPath = "bop")
+ )
+
+ expect_snapshot(error = TRUE, dbConnect(databricks(), HTTPPath = 1L))
+ expect_snapshot(error = TRUE, dbConnect(databricks(), httpPath = 1L))
+})