Feature Request: Allow non-string atomic types in `wi_th` query params

Session Info

# starting a new R session
> sessionInfo()
#> R version 4.0.2 (2020-06-22)
#> Platform: x86_64-conda_cos6-linux-gnu (64-bit)
#> Running under: Ubuntu 20.04.1 LTS
#> 
#> Matrix products: default
#> BLAS/LAPACK: /home/name/.miniconda3/envs/R/lib/libopenblasp-r0.3.7.so
#> 
#> locale:
#>  [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8     LC_MONETARY=en_US.UTF-8   
#>  [6] LC_MESSAGES=en_US.UTF-8    LC_PAPER=en_US.UTF-8       LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C            
#> [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] httr_1.4.2     webmockr_0.6.3.92
#> 
#> loaded via a namespace (and not attached):
#> [1] compiler_4.0.2 magrittr_1.5   R6_2.4.1       whisker_0.4    httpcode_0.3.0 curl_4.3       fauxpas_0.5.0  crul_1.0.0

While debugging #106 I realized that one of the issues I was encountering was due to passing a query parameter as a numeric.

library(webmockr)
library(httr)
httr_mock()

webmockr::stub_registry_clear()

stub_request("get", "https://api.github.com/repos/ropensci/webmockr/issues") %>%
  wi_th(query = list(state = "all", per_page = 30)) %>%  # integer per_page param
  to_return(body = "test body")

GET("https://api.github.com/repos/ropensci/webmockr/issues?state=all&per_page=30")
#> Error: Real HTTP connections are disabled.
#> Unregistered request:
#>   GET:  https://api.github.com/repos/ropensci/webmockr/issues?state=all&per_page=30   with headers {Accept: application/json, text/xml, application/xml, */*}

Digging into why this was failing, I found that <RequestPattern>$uri_pattern$query_params_matches() will use the raw values provided to wi_th and compare those against the extracted query params from the request uri. The values extracted from the uri will always be character, and will fail to match against the query params provided as non-character types.

webmockr/R/RequestPattern.R

Lines 552 to 558 in 0d0a7b4

    
           query_params_matches = function(uri) { 
        
             if (!self$regex) { # not regex 
        
               return(identical(self$query_params, self$extract_query(uri))) 
        
             } 
        
             ## There is no regex for query params separately; so, just compare to uri 
        
             grepl(self$pattern, uri) # regex 
        
           },

This isn't strictly a bug, but the character restriction isn't described in the wi_th documentation and there aren't any warnings or errors thrown when passing non-character types even though they will always fail to match. It would be nice to either add some type checking or cast all query values as.character before registering the stub.

As one follow-up note, I don't believe this behavior in the comment in query_params_matches

webmockr/R/RequestPattern.R

Line 556 in 0d0a7b4

## There is no regex for query params separately; so, just compare to uri

is described in the documentation. At least, it's not documented in some of the places where it's probably most relevant to an end user such as stub_request or wi_th. I encountered this while trying to mix a uri_regex for the uri host and path with specific query params, but only found out when rooting through the code that wi_th(query = ...) isn't compatible with a uri_regex request stub.

library(webmockr)

# example stub
stub <- stub_request("get", uri_regex = "https://api.github.com/repos/.+/.+/issues") %>%
  wi_th(query = list(state = "all", per_page = "30")) %>% 
  to_return(body = "test body")

# build a request pattern object from stub
req_pat <- RequestPattern$new(
  method = stub$method,
  uri = stub$uri,
  uri_regex = stub$uri_regex,
  query = stub$query,
  body = stub$body,
  headers = stub$request_headers)

# inspect composite uri + query params
req_pat$uri_pattern$pattern
#> [1] "https?://api.github.com/repos/.*/.*/issues?state=all&per_page=30"
#                             treated as regex ~~~^

When a wi_ith(query = ...) is provided with a uri_regex, the <RequestPattern>$uri_pattern$pattern is a concatenation of the regex and query parameters (https?://api.github.com/repos/.*/.*/issues?state=all&per_page=30) which will always fail because the ? separator before the query parameter is treated as regex syntax.

It seems like quite the rabbit hole to go down to allow all these different regex scenarios, but it might be a good stop-gap solution to throw an error when wi_th(query = ...) is used with a regex stub.

I think it makes most sense to coerce all stub request elements to character before registering the stub.

it might be a good stop-gap solution to throw an error when ...

good idea. though, i'd like to address this fully sooner rather than later. That is, probably use the url minus any query params for the regex comparison (if the user gives uri_regex), and compare the query separately (if given)

I guess a user may give query params in the url given to uri_regex - in which case there will likely be a non-escaped ? which will break things - solution here I guess is just to document it

@dgkf reinstall and let me know what you think

Looks good, thanks for making the fix! If I encounter any other unexpected behaviors I'll kick off a new issue.

	query_params_matches = function(uri) {
	if (!self$regex) { # not regex
	return(identical(self$query_params, self$extract_query(uri)))
	}
	## There is no regex for query params separately; so, just compare to uri
	grepl(self$pattern, uri) # regex
	},