commercetools / rmf-codegen

Provides RAML code generators based on RMF.
Apache License 2.0
13 stars 7 forks source link

fix: switched from values.Set to values.Add #310

Closed demeyerthom closed 9 months ago

demeyerthom commented 9 months ago

We currently overwrite values being set in GET queries. This changes the generator to use Add instead, which will append values

jenschude commented 9 months ago

I would suggest you to provide both options. In the Java SDK there are with-Methods and add-Methods for query parameters. This allows to prepare requests and either set or add the parameters. Also possible to "reset" a requests query parameter

demeyerthom commented 9 months ago

@jenschude I will take a look if I can implement a smarter way of interacting with it

demeyerthom commented 9 months ago

@jenschude I did some quick research, and I think in this case the above fix achieves what we want to do:

It will generate the following bit of code:

for k, v := range input.PredicateVar {
        for _, x := range v {
            values.Add(k, x) //Instead of .Set
        }
    }

The input here is

type ByProjectKeyApiClientsRequestMethodGetInput struct {
    Expand       []string
    Sort         []string
    Limit        *int
    Offset       *int
    WithTotal    *bool
    Where        []string
    PredicateVar map[string][]string
}

So users can fill or clear PredicateVar however they want. For the generated code it is only necessary to add all the inputs provided to the query params, so i don't think we need any additional (re)setters here.

Do you agree?

jenschude commented 9 months ago

If you think it works in Go I'm fine with it. 👍

Also didn't saw initially that it's about the pattern parameters.