From 8c7a0e2d2d1236bd4172dfbc3081b129737e89b4 Mon Sep 17 00:00:00 2001
From: "Simon P. Couch" <simonpatrickcouch@gmail.com>
Date: Fri, 12 Apr 2024 14:59:12 -0500
Subject: [PATCH] correct type checking for `databricks(httpPath)` (#787)

---
 R/driver-databricks.R                      | 16 +++++++--------
 tests/testthat/_snaps/driver-databricks.md | 24 ++++++++++++++++++++++
 tests/testthat/test-driver-databricks.R    | 22 ++++++++++++++++++++
 3 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/R/driver-databricks.R b/R/driver-databricks.R
index a933b2a0..7b946057 100644
--- 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
index 3b4ec15b..3883abd8 100644
--- 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
index d3711c8e..8e35735f 100644
--- 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))
+})