feat: support repeated query params and simplified "csv" cases

[zaquestion]

This allows for leveraging repeated fields in the query via foo=bar&foo=bar2, this also simplifies the "csv" case for repeated strings. Before this using repeated string foo in the query would require quoted values foo="bar","bar2", which is a little weird as it departs from how we normally pass values in query params, so as special case for strings (and a more efficient one), foo=bar,bar2 is supported

Example generation using the protos from the 2-repeated integration test String (note: zero json marshaling required for these)

    if MGetRepeatedStrArr, ok := queryParams["M"]; ok {
        MGetRepeatedStr := MGetRepeatedStrArr[0]

        var MGetRepeated []string
        if len(MGetRepeatedStrArr) > 1 {
            MGetRepeated = MGetRepeatedStrArr
        } else {
            MGetRepeated = strings.Split(MGetRepeatedStr, ",")
        }
        req.M = MGetRepeated
    }

Non string

    if LGetRepeatedStrArr, ok := queryParams["L"]; ok {
        LGetRepeatedStr := LGetRepeatedStrArr[0]

        var LGetRepeated []bool
        if len(LGetRepeatedStrArr) > 1 {
            LGetRepeated = make([]bool, len(LGetRepeatedStrArr))
            for i, v := range LGetRepeatedStrArr {
                converted, err := strconv.ParseBool(v)
                if err != nil {
                    return nil, errors.Wrapf(err, "couldn't decode LGetRepeated from %v", LGetRepeatedStr)
                }
                LGetRepeated[i] = converted
            }
        } else {
            err = json.Unmarshal([]byte(LGetRepeatedStr), &LGetRepeated)
            if err != nil {
                LGetRepeatedStr = "[" + LGetRepeatedStr + "]"
            }
            err = json.Unmarshal([]byte(LGetRepeatedStr), &LGetRepeated)
        }
        if err != nil {
            return nil, errors.Wrapf(err, "couldn't decode LGetRepeated from %v", LGetRepeatedStr)
        }
        req.L = LGetRepeated
    }

Note: the else case above represents the existing code gen, while the else case for string had been optimized and removes the need for ", technically an in compatibility

[lelandbatey]

This MR is a welcome change. I've never been super happy with the way we deal with encoding more complex structs into query parameters, so in general I very much welcome any improvement to the query handling.

For this MR in particular, my request is that we have a new test added showing support for this different behavior.

[zwiedmann-isp]

This change will address https://github.com/metaverse/truss/issues/238 as well.

[adamryman]

Could you uncomment this line in the TestGetWithRepeatedQueryRequest integration test and see if it passes?

https://github.com/metaverse/truss/blob/db19ea775bc1e5adbf9b68f880017a2c220076d6/cmd/_integration-tests/transport/http_test.go#L112

I believe it should test the feature you are adding. Though I'm not 100% sure.

It it does not test your feature, could you drop a test in the linked file?

[adamryman]

Excited to see how everything looks once this is rebased on master.

[zaquestion]

@adamryman PTAL, I've brought the latest changes with #309 and uncommented the tests. You'll notice that I removed a few cases as these changes do cause a few incompatibilities. Namely quoted array and quoted csv style no longer work for repeated strings. IMO having to do this before was so broken I'm content to call it a bug fix. If not, we can move these changes onto a v1 branch and I can go from there.

[zaquestion]

Oh and good call about the clients, tests immediately caught the break there, I've updated the clients to use the new strategies when possible and both causes are already covered in the tests.

[adamryman]

I know this is already reviewed, though great job supporting the backwards compatibility!