Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ Suggests:
rsconnect,
spelling,
testthat,
webshot2
webshot2,
withr
VignetteBuilder:
knitr
RdMacros:
Expand Down
5 changes: 4 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
# connectapi 0.2.0.9000
# Unreleased

- Fixed a bug where timestamps from Connect not in UTC were parsed as `NA` (#290)
- Fixed a bug where timestamps sent to Connect may have added the difference between the local time zone and UTC (#291)

# connectapi 0.2.0

Expand Down
56 changes: 53 additions & 3 deletions R/parse.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ make_timestamp <- function(input) {
# TODO: make sure this is the right timestamp format
return(input)
}
safe_format(input, "%Y-%m-%dT%H:%M:%SZ")

# In the call to `safe_format`:
# - The format specifier adds a literal "Z" to the end of the timestamp, which
# tells Connect "This is UTC".
# - The `tz` argument tells R to produce times in the UTC time zone.
# - The `usetz` argument says "Don't concatenate ' UTC' to the end of the string".
safe_format(input, "%Y-%m-%dT%H:%M:%SZ", tz = "UTC", usetz = FALSE)
}

ensure_columns <- function(.data, ptype) {
Expand Down Expand Up @@ -107,8 +113,7 @@ coerce_datetime <- function(x, to, ...) {
} else if (is.numeric(x)) {
vctrs::new_datetime(as.double(x), tzone = tzone(to))
} else if (is.character(x)) {
# Parse as ISO8601
as.POSIXct(strptime(x, format = "%Y-%m-%dT%H:%M:%SZ"), tz = tzone(to))
parse_connect_rfc3339(x)
} else if (inherits(x, "POSIXct")) {
x
} else if (all(is.logical(x) & is.na(x)) && length(is.logical(x) & is.na(x)) > 0) {
Expand All @@ -118,6 +123,51 @@ coerce_datetime <- function(x, to, ...) {
}
}

# Parses a character vector of dates received from Connect, using use RFC 3339,
# returning a vector of POSIXct datetimes.
#
# R parses character timestamps as ISO 8601. When specifying %z, it expects time
# zones to be specified as `-1400` to `+1400`.
#
# Connect's API sends times in a specific RFC 3339 format: indicating time zone
# offsets with `-14:00` to `+14:00`, and zero offset with `Z`.
# https://github.com/golang/go/blob/54fe0fd43fcf8609666c16ae6d15ed92873b1564/src/time/format.go#L86
# For example:
# - "2023-08-22T14:13:14Z"
# - "2023-08-22T15:13:14+01:00"
# - "2020-01-01T00:02:03-01:00"
parse_connect_rfc3339 <- function(x) {
# Convert any timestamps with offsets to a format recognized by `strptime`.
x <- gsub("([+-]\\d\\d):(\\d\\d)$", "\\1\\2", x)

# `purrr::map2_vec()` converts to POSIXct automatically, but we need
# `as.POSIXct()` in there to account vectors of length 1, which it seems are
# not converted.
#
# Parse with an inner call to `strptime()`; convert the resulting `POSIXlt`
# object to `POSIXct`.
#
# We must specify `tz` in the inner call to correctly compute date math.
# Specifying `tz` when parsing just changes the time zone without doing any
# date math!
#
# > xlt
# [1] "2024-08-29 16:36:33 EDT"
# > tzone(xlt)
# [1] "America/New_York"
# > as.POSIXct(xlt, tz = "UTC")
# [1] "2024-08-29 16:36:33 UTC"
purrr::map_vec(x, function(.x) {
# Times with and without offsets require different formats.
format_string = ifelse(
grepl("Z$", .x),
"%Y-%m-%dT%H:%M:%SZ",
"%Y-%m-%dT%H:%M:%S%z"
)
as.POSIXct(strptime(.x, format = format_string, tz = "UTC"))
})
}

vec_cast.POSIXct.double <- function(x, to, ...) {
warn_experimental("vec_cast.POSIXct.double")
vctrs::new_datetime(x, tzone = tzone(to))
Expand Down
2 changes: 1 addition & 1 deletion man/ContentTask.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/EnvironmentR6.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/Vanity.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/VariantR6.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/VariantSchedule.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/VariantTask.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion man/connectapi-package.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions man/content_render.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions man/content_restart.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

127 changes: 119 additions & 8 deletions tests/testthat/test-parse.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,125 @@ test_that("coerce_datetime fills the void", {
expect_error(coerce_datetime(NA_complex_, NA_datetime_, name = "complexity"), class = "vctrs_error_incompatible_type")
})

test_that("make_timestamp works with POSIXct", {
outcome <- "2020-01-01T01:02:03Z"
ts <- coerce_datetime(outcome, NA_datetime_)
expect_equal(make_timestamp(ts), outcome)
expect_equal(make_timestamp(rep(ts, 10)), rep(outcome, 10))

# idempotent
expect_equal(make_timestamp(make_timestamp(ts)), outcome)
test_that("parse_connect_rfc3339 parses timestamps we expect from Connect", {
withr::defer(Sys.setenv(TZ = Sys.getenv("TZ")))

x_mixed <- c(
"2023-08-22T14:13:14Z",
"2020-01-01T01:02:03Z",
"2023-08-22T15:13:14+01:00",
"2020-01-01T00:02:03-01:00"
)

x_zero_offset <- c(
"2023-08-22T14:13:14Z",
"2020-01-01T01:02:03Z"
)

x_plus_one <- c(
"2023-08-22T15:13:14+01:00",
"2020-01-01T02:02:03+01:00"
)

x_minus_one <- c(
"2023-08-22T13:13:14-01:00",
"2020-01-01T00:02:03-01:00"
)

single_zero_offset <- "2023-08-22T14:13:14Z"

single_offset <- "2023-08-22T15:13:14+01:00"

expected <- as.POSIXct(strptime(c(
"2023-08-22T14:13:14+0000",
"2020-01-01T01:02:03+0000"
), format = "%Y-%m-%dT%H:%M:%S%z", tz = "UTC"))

Sys.setenv(TZ = "America/New_York")
expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2))
expect_identical(parse_connect_rfc3339(x_zero_offset), expected)
expect_identical(parse_connect_rfc3339(x_plus_one), expected)
expect_identical(parse_connect_rfc3339(x_minus_one), expected)
expect_identical(parse_connect_rfc3339(single_zero_offset), expected[1])
expect_identical(parse_connect_rfc3339(single_offset), expected[1])

Sys.setenv(TZ = "UTC")
expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2))
expect_identical(parse_connect_rfc3339(x_zero_offset), expected)
expect_identical(parse_connect_rfc3339(x_plus_one), expected)
expect_identical(parse_connect_rfc3339(x_minus_one), expected)
expect_identical(parse_connect_rfc3339(single_zero_offset), expected[1])
expect_identical(parse_connect_rfc3339(single_offset), expected[1])

Sys.setenv(TZ = "Asia/Tokyo")
expect_identical(parse_connect_rfc3339(x_mixed), rep(expected, 2))
expect_identical(parse_connect_rfc3339(x_zero_offset), expected)
expect_identical(parse_connect_rfc3339(x_plus_one), expected)
expect_identical(parse_connect_rfc3339(x_minus_one), expected)
expect_identical(parse_connect_rfc3339(single_zero_offset), expected[1])
expect_identical(parse_connect_rfc3339(single_offset), expected[1])
})

test_that("make_timestamp produces expected output", {
withr::defer(Sys.setenv(TZ = Sys.getenv("TZ")))

x_mixed <- c(
"2023-08-22T14:13:14Z",
"2020-01-01T01:02:03Z",
"2023-08-22T15:13:14+01:00",
"2020-01-01T00:02:03-01:00"
)

x_zero_offset <- c(
"2023-08-22T14:13:14Z",
"2020-01-01T01:02:03Z"
)

x_plus_one <- c(
"2023-08-22T15:13:14+01:00",
"2020-01-01T02:02:03+01:00"
)

x_minus_one <- c(
"2023-08-22T13:13:14-01:00",
"2020-01-01T00:02:03-01:00"
)

single_zero_offset <- "2023-08-22T14:13:14Z"

single_offset <- "2023-08-22T15:13:14+01:00"

outcome <- c(
"2023-08-22T14:13:14Z",
"2020-01-01T01:02:03Z"
)

Sys.setenv(TZ = "America/New_York")
expect_equal(make_timestamp(coerce_datetime(x_mixed, NA_datetime_)), rep(outcome, 2))
expect_equal(make_timestamp(coerce_datetime(x_zero_offset, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(x_plus_one, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(x_minus_one, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(single_zero_offset, NA_datetime_)), outcome[1])
expect_equal(make_timestamp(coerce_datetime(single_offset, NA_datetime_)), outcome[1])
expect_equal(make_timestamp(outcome), outcome)

Sys.setenv(TZ = "UTC")
expect_equal(make_timestamp(coerce_datetime(x_mixed, NA_datetime_)), rep(outcome, 2))
expect_equal(make_timestamp(coerce_datetime(x_zero_offset, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(x_plus_one, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(x_minus_one, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(single_zero_offset, NA_datetime_)), outcome[1])
expect_equal(make_timestamp(coerce_datetime(single_offset, NA_datetime_)), outcome[1])
expect_equal(make_timestamp(outcome), outcome)

Sys.setenv(TZ = "Asia/Tokyo")
expect_equal(make_timestamp(coerce_datetime(x_mixed, NA_datetime_)), rep(outcome, 2))
expect_equal(make_timestamp(coerce_datetime(x_zero_offset, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(x_plus_one, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(x_minus_one, NA_datetime_)), outcome)
expect_equal(make_timestamp(coerce_datetime(single_zero_offset, NA_datetime_)), outcome[1])
expect_equal(make_timestamp(coerce_datetime(single_offset, NA_datetime_)), outcome[1])
expect_equal(make_timestamp(outcome), outcome)
})

test_that("make_timestamp is safe for strings", {
Expand Down
Loading