ropensci / ruODK

ruODK: An R Client for the ODK Central API
https://docs.ropensci.org/ruODK/
GNU General Public License v3.0
41 stars 12 forks source link

odata_submission_get() times out due to null skip and top parameters #126

Closed yanokwa closed 3 years ago

yanokwa commented 3 years ago

Problem

odata_submission_get() times out while collecting to Central v1.2.

Reproducible example

> library(ruODK)
> httr::set_config(httr::verbose())
> ru_setup(
+   url = "https://dev.central.getodk.org",
+   un = "yanokwa@getodk.org",
+   pw = "turtles",
+   tz = "America/Los_Angeles",
+   pid = "1",
+   fid = "field_mapper",
+   verbose = TRUE
+ )
<ruODK settings>
  Default ODK Central Project ID: 1
  Default ODK Central Form ID: field_mapper
  Default ODK Central URL: https://dev.central.getodk.org
  Default ODK Central Username: yanokwa@getodk.org
  Default ODK Central Password: run ruODK::get_default_pw() to show
  Default ODK Central Passphrase: run ruODK::get_default_pp() to show
  Default Time Zone: America/Los_Angeles
  Default ODK Central Version: 1.1
  Default HTTP GET retries: 3
  Verbose messages: TRUE
  Test ODK Central Project ID:
  Test ODK Central Form ID:
  Test ODK Central Form ID (ZIP tests):
  Test ODK Central Form ID (Attachment tests):
  Test ODK Central Form ID (Parsing tests):
  Test ODK Central Form ID (WKT tests):
  Test ODK Central URL:
  Test ODK Central Username:
  Test ODK Central Password: run ruODK::get_test_pw() to show
  Test ODK Central Passphrase: run ruODK::get_test_pp() to show
  Test ODK Central Version: 1.1
> odata_submission_get()
ℹ Downloading submissions...
-> GET /v1/projects/1/forms/field_mapper.svc/Submissions?%24skip=&%24top=&%24count=false&%24wkt=false HTTP/1.1
-> Host: dev.central.getodk.org
-> Authorization: Basic turtlesturtlesturtlesturtlesturtlesturtlesturtlesturtlesturtles==
-> User-Agent: libcurl/7.68.0 r-curl/4.3.1 httr/1.4.2
-> Accept-Encoding: deflate, gzip, br
-> Accept: application/json
->
<- HTTP/1.1 502 Bad Gateway
<- Server: nginx
<- Date: Thu, 03 Jun 2021 21:02:54 GMT
<- Content-Type: text/html
<- Content-Length: 166
<- Connection: keep-alive
<-

Likely solution

In my testing with a simple Python script, I noticed that dropping the $skip and $top parameters would result in a 200, but adding them as ruODK does would result in a 502.

The OData documentation at https://www.odata.org/documentation/odata-version-2-0/uri-conventions says that for $skip and $top, "N is an integer greater than or equal to zero specified by this query option. If a value less than zero is specified, the URI should be considered malformed."

I don't know if this is new behavior in Central, but regardless, I think ruODK should do what the OData spec says.

https://github.com/ropensci/ruODK/blob/main/R/odata_submission_get.R#L184 is the relevant line of code for this issue and a code search shows it's the only place that has $skip and $top.

mtyszler commented 3 years ago

Tks @yanokwa

@florianm

Solution is simple, I believe, just add:

qry<-qry[qry!=""]

at https://github.com/ropensci/ruODK/blob/0f9acf8a55952f6d7b22d46e39a2484b2c07ec84/R/odata_submission_get.R#L191

florianm commented 3 years ago

@mtyszler thanks for the idea - definitely a good catch-all. In my moderately sleep-deprived state I went and dropped skip and top explicitly. Unit tests are passing against my quick test project on the ODK Central Sandbox (v1.2) and my own test Central (v1.1). I'm pushing ruODK version 0.10.2 with the patch now and see how GitHub Actions feels about them.