Corrections based on PR comments. Returned to use list, cleaned up check_labels and corrected tests

Author: JulianUmbhau

R/bq-parse.R

@@ -1,6 +1,6 @@
 bq_parse_single <- function(value, type, ...) {
   field <- bq_field("", type, ...)
-  field_j <- jsonlite::toJSON(as_json(field), auto_unbox = TRUE)
+  field_j <- jsonlite::toJSON(as_json(field))
   value_j <- jsonlite::toJSON(value, auto_unbox = TRUE)
 
   bq_field_init(field_j, value_j)

R/bq-perform.R

@@ -168,7 +168,7 @@ bq_perform_upload <- function(
   metadata <- bq_body(metadata, ...)
   metadata <- list(
     "type" = "application/json; charset=UTF-8",
-    "content" = jsonlite::toJSON(metadata, auto_unbox = TRUE, pretty = TRUE)
+    "content" = jsonlite::toJSON(metadata, auto_unbox = TRUE, pretty = TRUE, digits = json_digits)
   )
 
   if (source_format == "NEWLINE_DELIMITED_JSON") {

R/utils.R

@@ -87,27 +87,13 @@ has_bigrquerystorage <- function() {
 }
 
 check_labels <- function(labels) {
-  if (is.null(labels) || length(labels) == 0 || (length(labels) == 1 && is.na(labels))) {
+  if (is.null(labels) || length(labels) == 0) {
     return(NULL)
   }
 
-  if (!is.character(labels) || is.null(names(labels)) || anyNA(names(labels)) || any(names(labels) == "")) {
-    warning("Labels must be a named character vector; dropping labels", immediate. = TRUE, call. = FALSE)
-    return(NULL)
-  }
-
-  nms <- names(labels)
-  bad_keys <- nms[nms != tolower(nms)]
-  if (length(bad_keys) > 0) {
-    warning(sprintf("Label key '%s' must match ^[a-z0-9_-]{0,62}$; dropping labels", bad_keys[[1]]), immediate. = TRUE, call. = FALSE)
-    return(NULL)
-  }
-
-  bad_vals <- labels[labels != tolower(labels)]
-  if (length(bad_vals) > 0) {
-    warning(sprintf("Label value '%s' must be empty or match ^[a-z0-9_-]{0,62}$; dropping labels", bad_vals[[1]]), immediate. = TRUE, call. = FALSE)
-    return(NULL)
+  if (!is.list(labels) || any(names2(labels) == "")) {
+    cli::cli_abort("Labels must be a named list.")
   }
 
-  return(labels)
+  labels
 }

tests/testthat/test-utils.R

@@ -8,16 +8,14 @@ test_that("bq_check_namespace() works", {
 
 test_that("check_labels() accepts valid labels and NULL-like inputs", {
   expect_null(check_labels(NULL))
-  expect_null(check_labels(NA))
-  expect_null(check_labels(character()))
+  expect_null(check_labels(list()))
 
-  expect_equal(check_labels(c(env = "prod")), c(env = "prod"))
-  expect_equal(check_labels(c(env = "prod", team = "data")), c(env = "prod", team = "data"))
+  expect_equal(check_labels(list(env = "prod")), list(env = "prod"))
+  expect_equal(check_labels(list(env = "prod", team = "data")), list(env = "prod", team = "data"))
+  expect_equal(check_labels(list(env = "")), list(env = ""))
 })
 
-test_that("check_labels() warns and returns NULL for invalid inputs", {
-  expect_warning(check_labels(list(env = "prod")), "named character vector")
-  expect_warning(check_labels(c("no-name")), "named character vector")
-  expect_warning(check_labels(c(ENV = "prod")), "must match")
-  expect_warning(check_labels(c(env = "Prod")), "must be empty or match")
+test_that("check_labels() errors on invalid inputs", {
+  expect_error(check_labels(c(env = "prod")), "named list")
+  expect_error(check_labels(list("no-name")), "named list")
 })