andygrunwald / go-gerrit

Go client/library for Gerrit Code Review
https://godoc.org/github.com/andygrunwald/go-gerrit
MIT License
96 stars 40 forks source link

Error in Projects.CreateProject: PUT a/projects/{name} invalid JSON in request #93

Closed wwade closed 3 years ago

wwade commented 3 years ago

Here's a sample capture:

HTTP Request:

PUT /a/projects/test/ HTTP/1.1
Host: 192.168.160.1:8080
User-Agent: Go-http-client/1.1
Content-Length: 234
Accept: application/json
Authorization: Basic [redacted]
Content-Type: application/json
Accept-Encoding: gzip

{"name":"test","permissions_only":false,"create_empty_commit":true,"branches":["main"],"use_contributor_agreements":"","use_signed_off_by":"","create_new_change_for_all_not_in_target":"","use_content_merge":"","require_change_id":""}

HTTP Response:

HTTP/1.1 400 Bad Request
Date: Thu, 09 Sep 2021 02:15:48 GMT
X-Frame-Options: DENY
Content-Disposition: attachment
X-Content-Type-Options: nosniff
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Pragma: no-cache
Expires: Mon, 01 Jan 1990 00:00:00 GMT
Content-Type: text/plain;charset=utf-8
Content-Length: 36

Invalid application/json in request

It looks like the right answer it to simply omitempty for use_contributor_agreements etc.

wwade commented 3 years ago

I have a patch in hand and will submit a PR shortly:

--- a/go-gerrit/projects.go 2021-09-08 19:04:43.850861477 -0700
+++ b/go-gerrit/projects.go 2021-09-08 19:21:12.533485144 -0700
@@ -37,7 +37,7 @@
    Owners                           []string                     `json:"owners,omitempty"`
-   UseContributorAgreements         string                       `json:"use_contributor_agreements"`
-   UseSignedOffBy                   string                       `json:"use_signed_off_by"`
-   CreateNewChangeForAllNotInTarget string                       `json:"create_new_change_for_all_not_in_target"`
-   UseContentMerge                  string                       `json:"use_content_merge"`
-   RequireChangeID                  string                       `json:"require_change_id"`
+   UseContributorAgreements         string                       `json:"use_contributor_agreements,omitempty"`
+   UseSignedOffBy                   string                       `json:"use_signed_off_by,omitempty"`
+   CreateNewChangeForAllNotInTarget string                       `json:"create_new_change_for_all_not_in_target,omitempty"`
+   UseContentMerge                  string                       `json:"use_content_merge,omitempty"`
+   RequireChangeID                  string                       `json:"require_change_id,omitempty"`
    MaxObjectSizeLimit               string                       `json:"max_object_size_limit,omitempty"`
andygrunwald commented 3 years ago

Hey @wwade, thanks for reporting the issue. I have two questions:

  1. Which Gerrit version do you use?
  2. Do you have a piece of an example code in Go to reproduce the issue?
wwade commented 3 years ago
  1. I've reproduced against the latest 3.2, 3.3, 3.4 (3.2.12, 3.3.6, and 3.4.1, respectively).
  2. Here's the snippet, nb localhost is 192.168.208.1, a local docker bridge network host.
    func initGerrit() error {
    var err error
    gerritClient, err = gerrit.NewClient(fmt.Sprintf("http://%s:8080", localhost), nil)
    if err != nil {
        return fmt.Errorf("initializing gerrit client: %w", err)
    }
    gerritClient.Authentication.SetBasicAuth("admin", "secret")
    data := &gerrit.ProjectInput{
        Name:              gerritProject,
        Branches:          []string{gerritBranch},
        CreateEmptyCommit: true,
    }
    if _, _, err := gerritClient.Projects.CreateProject(gerritProject, data); err != nil {
        return fmt.Errorf("creating test project: %w", err)
    }
    return nil
    }

It ends up with:

creating test project: API call to http://192.168.208.1:8080/a/projects/test/ failed: 400 Bad Request
andygrunwald commented 3 years ago

Great. Thanks a lot, @wwade.

I will try to find time to reproduce this as soon as I can.

wwade commented 3 years ago

Sure, thanks! FWIW, I think the PR is safe either way :)

andygrunwald commented 3 years ago

Thanks @wwade for raising the issue and following up with a fix.

Fixed in https://github.com/andygrunwald/go-gerrit/pull/94