Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QueryChanges converting query parameters into HTML encoding #18

Closed
perolausson opened this issue Oct 3, 2016 · 2 comments
Closed

QueryChanges converting query parameters into HTML encoding #18

perolausson opened this issue Oct 3, 2016 · 2 comments
Assignees
Labels
Milestone

Comments

@perolausson
Copy link
Contributor

I think there is a defect in QueryChanges or in the Gerrit REST API.

Consider a query that is constructed with multiple filters such as the one documented in the REST API:

https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes

In an example there under Request we find:

GET /changes/?q=is:open**+**owner:self&q=is:open+reviewer:self+-owner:self&q=is:closed+owner:self+limit:5&o=LABELS HTTP/1.0

Notice that the + is a literal + ascii.

However, when performing a query using the go-gerrit API, the + is translated into %2B

However, the Gerrit API does not recognise this and returns an empty result set.

Note that the query parameters sent in, are unrelated independent queries. They are not added together. I suspect this causes further confusion - potentially that is a separate bug, since I assumed something different and got some Json un-marshalling bugs ...

I think this needs a cleanup. Personally I would prefer a simpler interface and focus on just one query, but of course the question is how to model the query language. Is each query an AND or an OR etc.

Hope this makes sense to you.

@perolausson
Copy link
Contributor Author

perolausson commented Oct 3, 2016

Some code to explain the problem better:

func main() {
    instance := "https://gerrit-review.googlesource.com"
    client, err := gerrit.NewClient(instance, nil)
    if err != nil {
        panic(err)
    }

    opt  := &gerrit.QueryChangeOptions{}
    opt2 := &gerrit.QueryChangeOptions{}
    opt.Query = []string{
        "status:open", "topic:checkstyle-cleanup",
    }
    opt2.Query = []string{
        "status:open+topic:checkstyle-cleanup",
    }
    opt.Limit = 2
    opt.AdditionalFields = []string{"LABELS"}
    opt2.Limit = 2
    opt2.AdditionalFields = []string{"LABELS"}
    _, _, err = client.Changes.QueryChanges(opt)
    fmt.Println(err)
    //if err != nil {
    //  panic(err)
    //}
    _, _, err = client.Changes.QueryChanges(opt2)
    fmt.Println(err)

}

Result:

json: cannot unmarshal array into Go value of type gerrit.ChangeInfo
API call to https://gerrit-review.googlesource.com/changes/?n=2&o=LABELS&q=status%3Aopen%2Btopic%3Acheckstyle-cleanup failed: 400 Bad Request

The right answer would be: https://gerrit-review.googlesource.com/#/q/status:open+topic:checkstyle-cleanup

@opalmer
Copy link
Contributor

opalmer commented Oct 12, 2016

So I've done some research on this, looks like this might be an issue with google/go-querystring/query which is doing the url encoding for us. Either that or we're using it wrong. Still looking into exactly how to fix it without breaking stuff.

For my own notes, this works perfectly fine so it's definitely an issue with the encoding:

> curl 'https://gerrit-review.googlesource.com/changes/?n=2&o=LABELS&q=status:open+topic:checkstyle-cleanup'
)]}'
[]

This also works so multiple query parameters are not the issue either:

curl 'https://gerrit-review.googlesource.com/changes/?n=2&o=LABELS&q=status:open+topic:checkstyle-cleanup&q=status:abandoned

This however fails like go-gerrit does which seems to validate the initial problem:

> curl 'https://gerrit-review.googlesource.com/changes/?n=2&o=LABELS&q=status:open%2Btopic:checkstyle-cleanup' --fail
curl: (22) The requested URL returned error: 400 Bad Request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants