pagination: consumer scan params cannot use non-string types

[davepacheco]

If consumers try to define a ScanParams type for use with PaginationParams, and their type uses a non-String like a boolean or u64, this will go badly at runtime. In particular, they'll never successfully deserialize that parameter. Take this example in test_pagination.rs, which has a doit: bool querystring parameter. If you remove the workaround described below and run the test, it fails like this:

$ cargo test --test=test_pagination test_paginate_with_required_params -- --nocapture
   Compiling dropshot v0.3.0 (/Users/dap/oxide/dropshot/dropshot)
    Finished test [unoptimized + debuginfo] target(s) in 13.49s
     Running target/debug/deps/test_pagination-ed16123546a18dbb

running 1 test
log file: "/var/folders/x3/6tbrhnss6zz677z4zjsyvkqc0000gn/T/test_pagination-ed16123546a18dbb-required_params.92084.0.log"
note: configured to log to "/var/folders/x3/6tbrhnss6zz677z4zjsyvkqc0000gn/T/test_pagination-ed16123546a18dbb-required_params.92084.0.log"
thread 'test_paginate_with_required_params' panicked at 'assertion failed: `(left == right)`
  left: `"unable to parse query string: data did not match any variant of untagged enum RawWhichPage"`,
 right: `"you did not say to do it"`', dropshot/tests/test_pagination.rs:655:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test test_paginate_with_required_params ... FAILED

failures:

failures:
    test_paginate_with_required_params

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 8 filtered out

error: test failed, to rerun pass '-p dropshot --test test_pagination'

The operative part is the error unable to parse query string: data did not match any variant of untagged enum RawWhichPage. This is wrong -- the querystring did match the variant (and there's no querystring you can use that will work).

The root cause here is serde-rs/serde#1183, via nox/serde_urlencoded#66 (and, under an earlier alternative design, nox/serde_urlencoded#33). We use an untagged enum in RawWhichPage in order to distinguish between the WhichPage::First case and the WhichPage::Next case.

Fixing this

The problem we have is that we want the querystring parameters to match what's idiomatic for REST APIs -- i.e., presence of "page_token" means WhichPage::Next, while anything else means WhichPage::First. An untagged enum implements exactly this behavior but has the above problem. The challenge is that whatever form I can think of that we can parse the querystring to that allows us to see whether "page_token" is present, we can't then try to deserialize the result again as the user's struct.

I looked into using serde_value to do this, because that's an intermediate form that you can serialize to or deserialize from and it has a Map(BTreeMap<Value, Value>) variant. But I believe when coming from serde_urlencoded, the value in the map is always a String and it won't try to convert it to a bool or u64 or whatever when you deserialize it.

@ahl and I talked about implementing Deserializer (not Deserialize) for BTreeMap<String, String> that does try to parse a string value to some other type when that other type is requested.

Workaround

There's a workaround for this from @dtolnay's comment on serde-rs/serde#1183. This works, but you also need to apply a schemars attribute because schemars only supports serde(with) attributes whose values are actual types, not modules. The workaround looks like this:

    /* Work around serde-rs/serde#1183 */
    #[schemars(with = "bool")]
    #[serde(with = "serde_with::rust::display_fromstr")]
    doit: bool,

[davepacheco]

I think the fix for this would also address an analogous limitation for path parameters. See #42/#43.