From 54e55028c124983c3997c2419a006dfef427c824 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 3 Jul 2021 09:18:48 +0000 Subject: [PATCH 01/15] Add support for listing and getting repository webhook deliveries Resolves #1933 --- github/github-accessors.go | 136 +++++++++++++++++ github/github-accessors_test.go | 161 ++++++++++++++++++++ github/github-stringify_test.go | 21 +++ github/github.go | 44 ++++-- github/repos_hooks_deliveries.go | 205 ++++++++++++++++++++++++++ github/repos_hooks_deliveries_test.go | 106 +++++++++++++ 6 files changed, 659 insertions(+), 14 deletions(-) create mode 100644 github/repos_hooks_deliveries.go create mode 100644 github/repos_hooks_deliveries_test.go diff --git a/github/github-accessors.go b/github/github-accessors.go index 1c17384eac0..d34ae1dc8c5 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -4692,6 +4692,142 @@ func (h *HookConfig) GetURL() string { return *h.URL } +// GetAction returns the Action field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetAction() string { + if h == nil || h.Action == nil { + return "" + } + return *h.Action +} + +// GetDeliveredAt returns the DeliveredAt field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetDeliveredAt() time.Time { + if h == nil || h.DeliveredAt == nil { + return time.Time{} + } + return *h.DeliveredAt +} + +// GetDuration returns the Duration field. +func (h *HookDelivery) GetDuration() *float64 { + if h == nil { + return nil + } + return h.Duration +} + +// GetEvent returns the Event field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetEvent() string { + if h == nil || h.Event == nil { + return "" + } + return *h.Event +} + +// GetGUID returns the GUID field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetGUID() string { + if h == nil || h.GUID == nil { + return "" + } + return *h.GUID +} + +// GetID returns the ID field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetID() int64 { + if h == nil || h.ID == nil { + return 0 + } + return *h.ID +} + +// GetInstallationID returns the InstallationID field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetInstallationID() string { + if h == nil || h.InstallationID == nil { + return "" + } + return *h.InstallationID +} + +// GetRedelivery returns the Redelivery field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetRedelivery() bool { + if h == nil || h.Redelivery == nil { + return false + } + return *h.Redelivery +} + +// GetRepositoryID returns the RepositoryID field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetRepositoryID() int64 { + if h == nil || h.RepositoryID == nil { + return 0 + } + return *h.RepositoryID +} + +// GetRequest returns the Request field. +func (h *HookDelivery) GetRequest() *HookRequest { + if h == nil { + return nil + } + return h.Request +} + +// GetResponse returns the Response field. +func (h *HookDelivery) GetResponse() *HookResponse { + if h == nil { + return nil + } + return h.Response +} + +// GetStatus returns the Status field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetStatus() string { + if h == nil || h.Status == nil { + return "" + } + return *h.Status +} + +// GetStatusCode returns the StatusCode field if it's non-nil, zero value otherwise. +func (h *HookDelivery) GetStatusCode() int { + if h == nil || h.StatusCode == nil { + return 0 + } + return *h.StatusCode +} + +// GetHeader returns the Header map if it's non-nil, an empty map otherwise. +func (h *HookRequest) GetHeader() map[string]string { + if h == nil || h.Header == nil { + return map[string]string{} + } + return h.Header +} + +// GetRawPayload returns the RawPayload field if it's non-nil, zero value otherwise. +func (h *HookRequest) GetRawPayload() json.RawMessage { + if h == nil || h.RawPayload == nil { + return json.RawMessage{} + } + return *h.RawPayload +} + +// GetHeader returns the Header map if it's non-nil, an empty map otherwise. +func (h *HookResponse) GetHeader() map[string]string { + if h == nil || h.Header == nil { + return map[string]string{} + } + return h.Header +} + +// GetRawPayload returns the RawPayload field if it's non-nil, zero value otherwise. +func (h *HookResponse) GetRawPayload() json.RawMessage { + if h == nil || h.RawPayload == nil { + return json.RawMessage{} + } + return *h.RawPayload +} + // GetActiveHooks returns the ActiveHooks field if it's non-nil, zero value otherwise. func (h *HookStats) GetActiveHooks() int { if h == nil || h.ActiveHooks == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 750c6a40f8f..280285d0bd9 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -5524,6 +5524,167 @@ func TestHookConfig_GetURL(tt *testing.T) { h.GetURL() } +func TestHookDelivery_GetAction(tt *testing.T) { + var zeroValue string + h := &HookDelivery{Action: &zeroValue} + h.GetAction() + h = &HookDelivery{} + h.GetAction() + h = nil + h.GetAction() +} + +func TestHookDelivery_GetDeliveredAt(tt *testing.T) { + var zeroValue time.Time + h := &HookDelivery{DeliveredAt: &zeroValue} + h.GetDeliveredAt() + h = &HookDelivery{} + h.GetDeliveredAt() + h = nil + h.GetDeliveredAt() +} + +func TestHookDelivery_GetDuration(tt *testing.T) { + h := &HookDelivery{} + h.GetDuration() + h = nil + h.GetDuration() +} + +func TestHookDelivery_GetEvent(tt *testing.T) { + var zeroValue string + h := &HookDelivery{Event: &zeroValue} + h.GetEvent() + h = &HookDelivery{} + h.GetEvent() + h = nil + h.GetEvent() +} + +func TestHookDelivery_GetGUID(tt *testing.T) { + var zeroValue string + h := &HookDelivery{GUID: &zeroValue} + h.GetGUID() + h = &HookDelivery{} + h.GetGUID() + h = nil + h.GetGUID() +} + +func TestHookDelivery_GetID(tt *testing.T) { + var zeroValue int64 + h := &HookDelivery{ID: &zeroValue} + h.GetID() + h = &HookDelivery{} + h.GetID() + h = nil + h.GetID() +} + +func TestHookDelivery_GetInstallationID(tt *testing.T) { + var zeroValue string + h := &HookDelivery{InstallationID: &zeroValue} + h.GetInstallationID() + h = &HookDelivery{} + h.GetInstallationID() + h = nil + h.GetInstallationID() +} + +func TestHookDelivery_GetRedelivery(tt *testing.T) { + var zeroValue bool + h := &HookDelivery{Redelivery: &zeroValue} + h.GetRedelivery() + h = &HookDelivery{} + h.GetRedelivery() + h = nil + h.GetRedelivery() +} + +func TestHookDelivery_GetRepositoryID(tt *testing.T) { + var zeroValue int64 + h := &HookDelivery{RepositoryID: &zeroValue} + h.GetRepositoryID() + h = &HookDelivery{} + h.GetRepositoryID() + h = nil + h.GetRepositoryID() +} + +func TestHookDelivery_GetRequest(tt *testing.T) { + h := &HookDelivery{} + h.GetRequest() + h = nil + h.GetRequest() +} + +func TestHookDelivery_GetResponse(tt *testing.T) { + h := &HookDelivery{} + h.GetResponse() + h = nil + h.GetResponse() +} + +func TestHookDelivery_GetStatus(tt *testing.T) { + var zeroValue string + h := &HookDelivery{Status: &zeroValue} + h.GetStatus() + h = &HookDelivery{} + h.GetStatus() + h = nil + h.GetStatus() +} + +func TestHookDelivery_GetStatusCode(tt *testing.T) { + var zeroValue int + h := &HookDelivery{StatusCode: &zeroValue} + h.GetStatusCode() + h = &HookDelivery{} + h.GetStatusCode() + h = nil + h.GetStatusCode() +} + +func TestHookRequest_GetHeader(tt *testing.T) { + zeroValue := map[string]string{} + h := &HookRequest{Header: zeroValue} + h.GetHeader() + h = &HookRequest{} + h.GetHeader() + h = nil + h.GetHeader() +} + +func TestHookRequest_GetRawPayload(tt *testing.T) { + var zeroValue json.RawMessage + h := &HookRequest{RawPayload: &zeroValue} + h.GetRawPayload() + h = &HookRequest{} + h.GetRawPayload() + h = nil + h.GetRawPayload() +} + +func TestHookResponse_GetHeader(tt *testing.T) { + zeroValue := map[string]string{} + h := &HookResponse{Header: zeroValue} + h.GetHeader() + h = &HookResponse{} + h.GetHeader() + h = nil + h.GetHeader() +} + +func TestHookResponse_GetRawPayload(tt *testing.T) { + var zeroValue json.RawMessage + h := &HookResponse{RawPayload: &zeroValue} + h.GetRawPayload() + h = &HookResponse{} + h.GetRawPayload() + h = nil + h.GetRawPayload() +} + func TestHookStats_GetActiveHooks(tt *testing.T) { var zeroValue int h := &HookStats{ActiveHooks: &zeroValue} diff --git a/github/github-stringify_test.go b/github/github-stringify_test.go index cf0c7ebe817..7be98bd0c46 100644 --- a/github/github-stringify_test.go +++ b/github/github-stringify_test.go @@ -544,6 +544,27 @@ func TestHook_String(t *testing.T) { } } +func TestHookDelivery_String(t *testing.T) { + v := HookDelivery{ + ID: Int64(0), + GUID: String(""), + Redelivery: Bool(false), + Duration: Float64(0.0), + Status: String(""), + StatusCode: Int(0), + Event: String(""), + Action: String(""), + InstallationID: String(""), + RepositoryID: Int64(0), + Request: &HookRequest{}, + Response: &HookResponse{}, + } + want := `github.HookDelivery{ID:0, GUID:"", Redelivery:false, Duration:0, Status:"", StatusCode:0, Event:"", Action:"", InstallationID:"", RepositoryID:0, Request:github.HookRequest{}, Response:github.HookResponse{}}` + if got := v.String(); got != want { + t.Errorf("HookDelivery.String = %v, want %v", got, want) + } +} + func TestHookStats_String(t *testing.T) { v := HookStats{ TotalHooks: Int(0), diff --git a/github/github.go b/github/github.go index efde8f8dbab..32a3352314c 100644 --- a/github/github.go +++ b/github/github.go @@ -212,6 +212,9 @@ type ListCursorOptions struct { // A cursor, as given in the Link header. If specified, the query only searches for events before this cursor. Before string `url:"before,omitempty"` + + // A cursor, as given in the Link header. If specified, the query continues the search using this cursor. + Cursor string `url:"cursor,omitempty"` } // UploadOptions specifies the parameters to methods that support uploads. @@ -445,6 +448,11 @@ type Response struct { // calling the endpoint again. NextPageToken string + // For APIs that support cursor pagination, such as RepositoryService.ListRepositoryHookDeliveries, + // the following field will be populated to point to the next page. + // Set ListCursorOptions.Cursor to this value when calling the endpoint again. + Cursor string + // Explicitly specify the Rate type so Rate's String() receiver doesn't // propagate to Response. Rate Rate @@ -481,25 +489,33 @@ func (r *Response) populatePageValues() { if err != nil { continue } - page := url.Query().Get("page") - if page == "" { + + if page := url.Query().Get("page"); page != "" { + for _, segment := range segments[1:] { + switch strings.TrimSpace(segment) { + case `rel="next"`: + if r.NextPage, err = strconv.Atoi(page); err != nil { + r.NextPageToken = page + } + case `rel="prev"`: + r.PrevPage, _ = strconv.Atoi(page) + case `rel="first"`: + r.FirstPage, _ = strconv.Atoi(page) + case `rel="last"`: + r.LastPage, _ = strconv.Atoi(page) + } + } + continue } - for _, segment := range segments[1:] { - switch strings.TrimSpace(segment) { - case `rel="next"`: - if r.NextPage, err = strconv.Atoi(page); err != nil { - r.NextPageToken = page + if cursor := url.Query().Get("cursor"); cursor != "" { + for _, segment := range segments[1:] { + switch strings.TrimSpace(segment) { + case `rel="next"`: + r.Cursor = cursor } - case `rel="prev"`: - r.PrevPage, _ = strconv.Atoi(page) - case `rel="first"`: - r.FirstPage, _ = strconv.Atoi(page) - case `rel="last"`: - r.LastPage, _ = strconv.Atoi(page) } - } } } diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go new file mode 100644 index 00000000000..aeb4bdfcaff --- /dev/null +++ b/github/repos_hooks_deliveries.go @@ -0,0 +1,205 @@ +// Copyright 2021 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package github + +import ( + "context" + "encoding/json" + "fmt" + "time" +) + +// HookDelivery represents the data that is received from GitHub's Webhook Delivery API +// +// GitHub API docs: +// - https://docs.github.com/en/rest/reference/repos#list-deliveries-for-a-repository-webhook +// - https://docs.github.com/en/rest/reference/repos#get-a-delivery-for-a-repository-webhook +type HookDelivery struct { + ID *int64 `json:"id"` + GUID *string `json:"guid"` + DeliveredAt *time.Time `json:"delivered_at"` + Redelivery *bool `json:"redelivery"` + Duration *float64 `json:"duration"` + Status *string `json:"status"` + StatusCode *int `json:"status_code"` + Event *string `json:"event"` + Action *string `json:"action"` + InstallationID *string `json:"installation_id"` + RepositoryID *int64 `json:"repository_id"` + + // Request is populated by GetHookDelivery + Request *HookRequest `json:"request,omitempty"` + // Response is populated by GetHookDelivery + Response *HookResponse `json:"response,omitempty"` +} + +func (d HookDelivery) String() string { + return Stringify(d) +} + +type HookRequest struct { + Header map[string]string `json:"headers,omitempty"` + RawPayload *json.RawMessage `json:"payload,omitempty"` +} + +func (r HookRequest) String() string { + return Stringify(r) +} + +type HookResponse struct { + Header map[string]string `json:"headers,omitempty"` + RawPayload *json.RawMessage `json:"payload,omitempty"` +} + +func (r HookResponse) String() string { + return Stringify(r) +} + +// ListHookDeliveries lists webhook deliveries for a webhook configured in a repository. +// +// GitHub API docs: https://docs.github.com/en/rest/reference/repos#list-deliveries-for-a-repository-webhook +func (s *RepositoriesService) ListHookDeliveries(ctx context.Context, owner, repo string, id int64, opts *ListCursorOptions) ([]*HookDelivery, *Response, error) { + u := fmt.Sprintf("repos/%v/%v/hooks/%d/deliveries", owner, repo, id) + u, err := addOptions(u, opts) + if err != nil { + return nil, nil, err + } + + req, err := s.client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + + deliveries := []*HookDelivery{} + resp, err := s.client.Do(ctx, req, &deliveries) + if err != nil { + return nil, resp, err + } + + return deliveries, resp, nil +} + +// GetHookDelivery returns a delivery for a webhook configured in a repository. +// +// GitHub API docs: https://docs.github.com/en/rest/reference/repos#get-a-delivery-for-a-repository-webhook +func (s *RepositoriesService) GetHookDelivery(ctx context.Context, owner, repo string, hookID, deliveryID int64) (*HookDelivery, *Response, error) { + u := fmt.Sprintf("repos/%v/%v/hooks/%d/deliveries/%d", owner, repo, hookID, deliveryID) + req, err := s.client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + h := new(HookDelivery) + resp, err := s.client.Do(ctx, req, h) + if err != nil { + return nil, resp, err + } + + return h, resp, nil +} + +func (d *HookDelivery) ParseRequestPayload() (interface{}, error) { + var payload interface{} + switch *d.Event { + case "check_run": + payload = &CheckRunEvent{} + case "check_suite": + payload = &CheckSuiteEvent{} + case "commit_comment": + payload = &CommitCommentEvent{} + case "content_reference": + payload = &ContentReferenceEvent{} + case "create": + payload = &CreateEvent{} + case "delete": + payload = &DeleteEvent{} + case "deploy_ket": + payload = &DeployKeyEvent{} + case "deployment": + payload = &DeploymentEvent{} + case "deployment_status": + payload = &DeploymentStatusEvent{} + case "fork": + payload = &ForkEvent{} + case "github_app_authorization": + payload = &GitHubAppAuthorizationEvent{} + case "gollum": + payload = &GollumEvent{} + case "installation": + payload = &InstallationEvent{} + case "installation_repositories": + payload = &InstallationRepositoriesEvent{} + case "issue_comment": + payload = &IssueCommentEvent{} + case "issues": + payload = &IssuesEvent{} + case "label": + payload = &LabelEvent{} + case "marketplace_purchase": + payload = &MarketplacePurchaseEvent{} + case "member_event": + payload = &MemberEvent{} + case "membership_event": + payload = &MembershipEvent{} + case "meta": + payload = &MetaEvent{} + case "milestone": + payload = &MilestoneEvent{} + case "organization": + payload = &OrganizationEvent{} + case "org_block": + payload = &OrgBlockEvent{} + case "package": + payload = &PackageEvent{} + case "page_build": + payload = &PageBuildEvent{} + case "ping": + payload = &PingEvent{} + case "project": + payload = &ProjectEvent{} + case "project_card": + payload = &ProjectCardEvent{} + case "project_column": + payload = &ProjectColumnEvent{} + case "public": + payload = &PublicEvent{} + case "pull_request": + payload = &PullRequestEvent{} + case "pull_request_review": + payload = &PullRequestReviewEvent{} + case "pull_request_review_comment": + payload = &PullRequestReviewCommentEvent{} + case "pull_request_target": + payload = &PullRequestTargetEvent{} + case "push": + payload = &PushEvent{} + case "release": + payload = &ReleaseEvent{} + case "repository": + payload = &RepositoryEvent{} + case "repository_dispatch": + payload = &RepositoryDispatchEvent{} + case "repository_vulnerability_alert": + payload = &RepositoryVulnerabilityAlertEvent{} + case "star": + payload = &StarEvent{} + case "status": + payload = &StatusEvent{} + case "team": + payload = &TeamEvent{} + case "team_add": + payload = &TeamAddEvent{} + case "user": + payload = &UserEvent{} + case "watch": + payload = &WatchEvent{} + case "workflow_dispatch": + payload = &WorkflowDispatchEvent{} + case "workflow_run": + payload = &WorkflowRunEvent{} + } + err := json.Unmarshal(*d.Request.RawPayload, &payload) + return payload, err +} diff --git a/github/repos_hooks_deliveries_test.go b/github/repos_hooks_deliveries_test.go new file mode 100644 index 00000000000..116763cd4b0 --- /dev/null +++ b/github/repos_hooks_deliveries_test.go @@ -0,0 +1,106 @@ +// Copyright 2021 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package github + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestRepositoriesService_ListHookDeliveries(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/hooks/1/deliveries", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testFormValues(t, r, values{"cursor": "v1_12077215967"}) + fmt.Fprint(w, `[{"id":1}, {"id":2}]`) + }) + + opt := &ListCursorOptions{Cursor: "v1_12077215967"} + + ctx := context.Background() + hooks, _, err := client.Repositories.ListHookDeliveries(ctx, "o", "r", 1, opt) + if err != nil { + t.Errorf("Repositories.ListHookDeliveries returned error: %v", err) + } + + want := []*HookDelivery{{ID: Int64(1)}, {ID: Int64(2)}} + if d := cmp.Diff(hooks, want); d != "" { + t.Errorf("Repositories.ListHooks want (-), got (+):\n%s", d) + } + + const methodName = "ListHookDeliveries" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.ListHookDeliveries(ctx, "\n", "\n", -1, opt) + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.ListHookDeliveries(ctx, "o", "r", 1, opt) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestRepositoriesService_ListHookDeliveries_invalidOwner(t *testing.T) { + client, _, _, teardown := setup() + defer teardown() + + ctx := context.Background() + _, _, err := client.Repositories.ListHookDeliveries(ctx, "%", "%", 1, nil) + testURLParseError(t, err) +} + +func TestRepositoriesService_GetHookDelivery(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/hooks/1/deliveries/1", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprint(w, `{"id":1}`) + }) + + ctx := context.Background() + hook, _, err := client.Repositories.GetHookDelivery(ctx, "o", "r", 1, 1) + if err != nil { + t.Errorf("Repositories.GetHookDelivery returned error: %v", err) + } + + want := &HookDelivery{ID: Int64(1)} + if !cmp.Equal(hook, want) { + t.Errorf("Repositories.GetHookDelivery returned %+v, want %+v", hook, want) + } + + const methodName = "GetHookDelivery" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.GetHookDelivery(ctx, "\n", "\n", -1, -1) + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.GetHookDelivery(ctx, "o", "r", 1, 1) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestRepositoriesService_GetHookDelivery_invalidOwner(t *testing.T) { + client, _, _, teardown := setup() + defer teardown() + + ctx := context.Background() + _, _, err := client.Repositories.GetHookDelivery(ctx, "%", "%", 1, 1) + testURLParseError(t, err) +} From f5889f8200461de646aa58f20f5faac0d46db176 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 6 Jul 2021 03:45:56 +0000 Subject: [PATCH 02/15] Add test for HookDelivery.ParseRequestPayload --- github/repos_hooks_deliveries_test.go | 82 +++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/github/repos_hooks_deliveries_test.go b/github/repos_hooks_deliveries_test.go index 116763cd4b0..5a50c17974d 100644 --- a/github/repos_hooks_deliveries_test.go +++ b/github/repos_hooks_deliveries_test.go @@ -7,8 +7,10 @@ package github import ( "context" + "encoding/json" "fmt" "net/http" + "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -104,3 +106,83 @@ func TestRepositoriesService_GetHookDelivery_invalidOwner(t *testing.T) { _, _, err := client.Repositories.GetHookDelivery(ctx, "%", "%", 1, 1) testURLParseError(t, err) } + +var hookDeliveryPayloadTypeToStruct = map[string]interface{}{ + "check_run": &CheckRunEvent{}, + "check_suite": &CheckSuiteEvent{}, + "commit_comment": &CommitCommentEvent{}, + "content_reference": &ContentReferenceEvent{}, + "create": &CreateEvent{}, + "delete": &DeleteEvent{}, + "deploy_ket": &DeployKeyEvent{}, + "deployment": &DeploymentEvent{}, + "deployment_status": &DeploymentStatusEvent{}, + "fork": &ForkEvent{}, + "github_app_authorization": &GitHubAppAuthorizationEvent{}, + "gollum": &GollumEvent{}, + "installation": &InstallationEvent{}, + "installation_repositories": &InstallationRepositoriesEvent{}, + "issue_comment": &IssueCommentEvent{}, + "issues": &IssuesEvent{}, + "label": &LabelEvent{}, + "marketplace_purchase": &MarketplacePurchaseEvent{}, + "member_event": &MemberEvent{}, + "membership_event": &MembershipEvent{}, + "meta": &MetaEvent{}, + "milestone": &MilestoneEvent{}, + "organization": &OrganizationEvent{}, + "org_block": &OrgBlockEvent{}, + "package": &PackageEvent{}, + "page_build": &PageBuildEvent{}, + "ping": &PingEvent{}, + "project": &ProjectEvent{}, + "project_card": &ProjectCardEvent{}, + "project_column": &ProjectColumnEvent{}, + "public": &PublicEvent{}, + "pull_request": &PullRequestEvent{}, + "pull_request_review": &PullRequestReviewEvent{}, + "pull_request_review_comment": &PullRequestReviewCommentEvent{}, + "pull_request_target": &PullRequestTargetEvent{}, + "push": &PushEvent{}, + "release": &ReleaseEvent{}, + "repository": &RepositoryEvent{}, + "repository_dispatch": &RepositoryDispatchEvent{}, + "repository_vulnerability_alert": &RepositoryVulnerabilityAlertEvent{}, + "star": &StarEvent{}, + "status": &StatusEvent{}, + "team": &TeamEvent{}, + "team_add": &TeamAddEvent{}, + "user": &UserEvent{}, + "watch": &WatchEvent{}, + "workflow_dispatch": &WorkflowDispatchEvent{}, + "workflow_run": &WorkflowRunEvent{}, +} + +func TestHookDelivery_ParsePayload(t *testing.T) { + for evt, obj := range hookDeliveryPayloadTypeToStruct { + t.Run(evt, func(t *testing.T) { + bs, err := json.Marshal(obj) + if err != nil { + t.Fatal(err) + } + + p := json.RawMessage(bs) + + d := &HookDelivery{ + Event: String(evt), + Request: &HookRequest{ + RawPayload: &p, + }, + } + + got, err := d.ParseRequestPayload() + if err != nil { + t.Error(err) + } + + if !reflect.DeepEqual(obj, got) { + t.Errorf("want %T %v, got %T %v", obj, obj, got, got) + } + }) + } +} From f6d688a03ab69a2114f4d29f41d790dc171fe464 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 6 Jul 2021 21:46:07 +0000 Subject: [PATCH 03/15] Restructure populatePageValues to reduce nesting --- github/github.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/github/github.go b/github/github.go index 32a3352314c..09d4ddc4c1e 100644 --- a/github/github.go +++ b/github/github.go @@ -490,31 +490,34 @@ func (r *Response) populatePageValues() { continue } - if page := url.Query().Get("page"); page != "" { + q := url.Query() + + if cursor := q.Get("cursor"); cursor != "" { for _, segment := range segments[1:] { switch strings.TrimSpace(segment) { case `rel="next"`: - if r.NextPage, err = strconv.Atoi(page); err != nil { - r.NextPageToken = page - } - case `rel="prev"`: - r.PrevPage, _ = strconv.Atoi(page) - case `rel="first"`: - r.FirstPage, _ = strconv.Atoi(page) - case `rel="last"`: - r.LastPage, _ = strconv.Atoi(page) + r.Cursor = cursor } } + } + page := q.Get("page") + if page == "" { continue } - if cursor := url.Query().Get("cursor"); cursor != "" { - for _, segment := range segments[1:] { - switch strings.TrimSpace(segment) { - case `rel="next"`: - r.Cursor = cursor + for _, segment := range segments[1:] { + switch strings.TrimSpace(segment) { + case `rel="next"`: + if r.NextPage, err = strconv.Atoi(page); err != nil { + r.NextPageToken = page } + case `rel="prev"`: + r.PrevPage, _ = strconv.Atoi(page) + case `rel="first"`: + r.FirstPage, _ = strconv.Atoi(page) + case `rel="last"`: + r.LastPage, _ = strconv.Atoi(page) } } } From ff5a47a9e3d968697afa6cf425a5220f774f9c9c Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 6 Jul 2021 21:48:57 +0000 Subject: [PATCH 04/15] Use Timestamp instead of time.Time for delivered_at --- github/github-accessors.go | 4 ++-- github/github-accessors_test.go | 2 +- github/github-stringify_test.go | 3 ++- github/repos_hooks_deliveries.go | 3 +-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index d34ae1dc8c5..dd025826b0e 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -4701,9 +4701,9 @@ func (h *HookDelivery) GetAction() string { } // GetDeliveredAt returns the DeliveredAt field if it's non-nil, zero value otherwise. -func (h *HookDelivery) GetDeliveredAt() time.Time { +func (h *HookDelivery) GetDeliveredAt() Timestamp { if h == nil || h.DeliveredAt == nil { - return time.Time{} + return Timestamp{} } return *h.DeliveredAt } diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 280285d0bd9..2c764076661 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -5535,7 +5535,7 @@ func TestHookDelivery_GetAction(tt *testing.T) { } func TestHookDelivery_GetDeliveredAt(tt *testing.T) { - var zeroValue time.Time + var zeroValue Timestamp h := &HookDelivery{DeliveredAt: &zeroValue} h.GetDeliveredAt() h = &HookDelivery{} diff --git a/github/github-stringify_test.go b/github/github-stringify_test.go index 7be98bd0c46..589febf9f56 100644 --- a/github/github-stringify_test.go +++ b/github/github-stringify_test.go @@ -548,6 +548,7 @@ func TestHookDelivery_String(t *testing.T) { v := HookDelivery{ ID: Int64(0), GUID: String(""), + DeliveredAt: &Timestamp{}, Redelivery: Bool(false), Duration: Float64(0.0), Status: String(""), @@ -559,7 +560,7 @@ func TestHookDelivery_String(t *testing.T) { Request: &HookRequest{}, Response: &HookResponse{}, } - want := `github.HookDelivery{ID:0, GUID:"", Redelivery:false, Duration:0, Status:"", StatusCode:0, Event:"", Action:"", InstallationID:"", RepositoryID:0, Request:github.HookRequest{}, Response:github.HookResponse{}}` + want := `github.HookDelivery{ID:0, GUID:"", DeliveredAt:github.Timestamp{0001-01-01 00:00:00 +0000 UTC}, Redelivery:false, Duration:0, Status:"", StatusCode:0, Event:"", Action:"", InstallationID:"", RepositoryID:0, Request:github.HookRequest{}, Response:github.HookResponse{}}` if got := v.String(); got != want { t.Errorf("HookDelivery.String = %v, want %v", got, want) } diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go index aeb4bdfcaff..2e2bc750f23 100644 --- a/github/repos_hooks_deliveries.go +++ b/github/repos_hooks_deliveries.go @@ -9,7 +9,6 @@ import ( "context" "encoding/json" "fmt" - "time" ) // HookDelivery represents the data that is received from GitHub's Webhook Delivery API @@ -20,7 +19,7 @@ import ( type HookDelivery struct { ID *int64 `json:"id"` GUID *string `json:"guid"` - DeliveredAt *time.Time `json:"delivered_at"` + DeliveredAt *Timestamp `json:"delivered_at"` Redelivery *bool `json:"redelivery"` Duration *float64 `json:"duration"` Status *string `json:"status"` From dffbb1cd1b1975ba93f77fb2d7331dffae56d198 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 6 Jul 2021 21:50:58 +0000 Subject: [PATCH 05/15] Fix HookResponse.Header to Headers for consistency with the underlying json field --- github/github-accessors.go | 16 ++++++++-------- github/github-accessors_test.go | 20 ++++++++++---------- github/repos_hooks_deliveries.go | 4 ++-- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index dd025826b0e..61eb48070ca 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -4796,12 +4796,12 @@ func (h *HookDelivery) GetStatusCode() int { return *h.StatusCode } -// GetHeader returns the Header map if it's non-nil, an empty map otherwise. -func (h *HookRequest) GetHeader() map[string]string { - if h == nil || h.Header == nil { +// GetHeaders returns the Headers map if it's non-nil, an empty map otherwise. +func (h *HookRequest) GetHeaders() map[string]string { + if h == nil || h.Headers == nil { return map[string]string{} } - return h.Header + return h.Headers } // GetRawPayload returns the RawPayload field if it's non-nil, zero value otherwise. @@ -4812,12 +4812,12 @@ func (h *HookRequest) GetRawPayload() json.RawMessage { return *h.RawPayload } -// GetHeader returns the Header map if it's non-nil, an empty map otherwise. -func (h *HookResponse) GetHeader() map[string]string { - if h == nil || h.Header == nil { +// GetHeaders returns the Headers map if it's non-nil, an empty map otherwise. +func (h *HookResponse) GetHeaders() map[string]string { + if h == nil || h.Headers == nil { return map[string]string{} } - return h.Header + return h.Headers } // GetRawPayload returns the RawPayload field if it's non-nil, zero value otherwise. diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 2c764076661..7d70caf1b43 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -5645,14 +5645,14 @@ func TestHookDelivery_GetStatusCode(tt *testing.T) { h.GetStatusCode() } -func TestHookRequest_GetHeader(tt *testing.T) { +func TestHookRequest_GetHeaders(tt *testing.T) { zeroValue := map[string]string{} - h := &HookRequest{Header: zeroValue} - h.GetHeader() + h := &HookRequest{Headers: zeroValue} + h.GetHeaders() h = &HookRequest{} - h.GetHeader() + h.GetHeaders() h = nil - h.GetHeader() + h.GetHeaders() } func TestHookRequest_GetRawPayload(tt *testing.T) { @@ -5665,14 +5665,14 @@ func TestHookRequest_GetRawPayload(tt *testing.T) { h.GetRawPayload() } -func TestHookResponse_GetHeader(tt *testing.T) { +func TestHookResponse_GetHeaders(tt *testing.T) { zeroValue := map[string]string{} - h := &HookResponse{Header: zeroValue} - h.GetHeader() + h := &HookResponse{Headers: zeroValue} + h.GetHeaders() h = &HookResponse{} - h.GetHeader() + h.GetHeaders() h = nil - h.GetHeader() + h.GetHeaders() } func TestHookResponse_GetRawPayload(tt *testing.T) { diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go index 2e2bc750f23..4dcf7de97c6 100644 --- a/github/repos_hooks_deliveries.go +++ b/github/repos_hooks_deliveries.go @@ -40,7 +40,7 @@ func (d HookDelivery) String() string { } type HookRequest struct { - Header map[string]string `json:"headers,omitempty"` + Headers map[string]string `json:"headers,omitempty"` RawPayload *json.RawMessage `json:"payload,omitempty"` } @@ -49,7 +49,7 @@ func (r HookRequest) String() string { } type HookResponse struct { - Header map[string]string `json:"headers,omitempty"` + Headers map[string]string `json:"headers,omitempty"` RawPayload *json.RawMessage `json:"payload,omitempty"` } From 5f0e1e4684945ac2272bac0eccf18fb15b0920c6 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 6 Jul 2021 21:54:02 +0000 Subject: [PATCH 06/15] Use %v instead of %d for formatting int64 --- github/repos_hooks_deliveries.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go index 4dcf7de97c6..6444d348e8a 100644 --- a/github/repos_hooks_deliveries.go +++ b/github/repos_hooks_deliveries.go @@ -61,7 +61,7 @@ func (r HookResponse) String() string { // // GitHub API docs: https://docs.github.com/en/rest/reference/repos#list-deliveries-for-a-repository-webhook func (s *RepositoriesService) ListHookDeliveries(ctx context.Context, owner, repo string, id int64, opts *ListCursorOptions) ([]*HookDelivery, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/hooks/%d/deliveries", owner, repo, id) + u := fmt.Sprintf("repos/%v/%v/hooks/%v/deliveries", owner, repo, id) u, err := addOptions(u, opts) if err != nil { return nil, nil, err @@ -85,7 +85,7 @@ func (s *RepositoriesService) ListHookDeliveries(ctx context.Context, owner, rep // // GitHub API docs: https://docs.github.com/en/rest/reference/repos#get-a-delivery-for-a-repository-webhook func (s *RepositoriesService) GetHookDelivery(ctx context.Context, owner, repo string, hookID, deliveryID int64) (*HookDelivery, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/hooks/%d/deliveries/%d", owner, repo, hookID, deliveryID) + u := fmt.Sprintf("repos/%v/%v/hooks/%v/deliveries/%v", owner, repo, hookID, deliveryID) req, err := s.client.NewRequest("GET", u, nil) if err != nil { return nil, nil, err From 431c77ca7945b5503f3e15a74249fa2b4352bbdd Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 6 Jul 2021 22:04:48 +0000 Subject: [PATCH 07/15] Add more tests for ParseRequestPayload --- github/repos_hooks_deliveries_test.go | 32 +++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/github/repos_hooks_deliveries_test.go b/github/repos_hooks_deliveries_test.go index 5a50c17974d..9ba72e3897e 100644 --- a/github/repos_hooks_deliveries_test.go +++ b/github/repos_hooks_deliveries_test.go @@ -186,3 +186,35 @@ func TestHookDelivery_ParsePayload(t *testing.T) { }) } } + +func TestHookDelivery_ParsePayload_invalidEvent(t *testing.T) { + p := json.RawMessage(nil) + + d := &HookDelivery{ + Event: String("some_invalid_event"), + Request: &HookRequest{ + RawPayload: &p, + }, + } + + _, err := d.ParseRequestPayload() + if err == nil || err.Error() != "unexpected end of JSON input" { + t.Errorf("unexpected error: %v", err) + } +} + +func TestHookDelivery_ParsePayload_invalidPayload(t *testing.T) { + p := json.RawMessage([]byte(`{"check_run":{"id":"invalid"}}`)) + + d := &HookDelivery{ + Event: String("check_run"), + Request: &HookRequest{ + RawPayload: &p, + }, + } + + _, err := d.ParseRequestPayload() + if err == nil || err.Error() != "json: cannot unmarshal string into Go struct field CheckRun.check_run.id of type int64" { + t.Errorf("unexpected error: %v", err) + } +} From 23922a838e3b17c930068442afd23185e878216c Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Tue, 6 Jul 2021 22:55:39 +0000 Subject: [PATCH 08/15] Enhance cursor pagination test for more coverage --- github/github_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/github/github_test.go b/github/github_test.go index 828898fd0e9..6b4aacbf62e 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -635,6 +635,20 @@ func TestResponse_cursorPagination(t *testing.T) { if got, want := response.NextPageToken, "url-encoded-next-page-token"; want != got { t.Errorf("response.NextPageToken: %v, want %v", got, want) } + + // cursor-based pagination with "cursor" param + r = http.Response{ + Header: http.Header{ + "Link": { + `; rel="next"`, + }, + }, + } + + response = newResponse(&r) + if got, want := response.Cursor, "v1_12345678"; got != want { + t.Errorf("response.Cursor: %v, want %v", got, want) + } } func TestResponse_populatePageValues_invalid(t *testing.T) { From 72c33805ecfe59246eb941b213eeedfd479208a8 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 8 Jul 2021 11:09:15 +0000 Subject: [PATCH 09/15] Make it clear that cursor and page are mutually exclusive --- github/github.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/github/github.go b/github/github.go index 09d4ddc4c1e..1748c6372c8 100644 --- a/github/github.go +++ b/github/github.go @@ -499,6 +499,8 @@ func (r *Response) populatePageValues() { r.Cursor = cursor } } + + continue } page := q.Get("page") From 5b86affe3f577b63a173907b19da3db4e0ac6d6d Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 8 Jul 2021 11:10:57 +0000 Subject: [PATCH 10/15] Add periods to field comments --- github/repos_hooks_deliveries.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go index 6444d348e8a..8b0dd32381a 100644 --- a/github/repos_hooks_deliveries.go +++ b/github/repos_hooks_deliveries.go @@ -29,9 +29,9 @@ type HookDelivery struct { InstallationID *string `json:"installation_id"` RepositoryID *int64 `json:"repository_id"` - // Request is populated by GetHookDelivery + // Request is populated by GetHookDelivery. Request *HookRequest `json:"request,omitempty"` - // Response is populated by GetHookDelivery + // Response is populated by GetHookDelivery. Response *HookResponse `json:"response,omitempty"` } From 194eff6f09c3f902219b927eb58c77c48abe5fe0 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 8 Jul 2021 11:14:27 +0000 Subject: [PATCH 11/15] Add comments on HookRequest and HookResponse --- github/repos_hooks_deliveries.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go index 8b0dd32381a..ab7ba52e0d1 100644 --- a/github/repos_hooks_deliveries.go +++ b/github/repos_hooks_deliveries.go @@ -39,6 +39,8 @@ func (d HookDelivery) String() string { return Stringify(d) } +// HookRequest is a part of HookDelivery that contains +// the HTTP headers and the JSON payload of the webhook request. type HookRequest struct { Headers map[string]string `json:"headers,omitempty"` RawPayload *json.RawMessage `json:"payload,omitempty"` @@ -48,6 +50,8 @@ func (r HookRequest) String() string { return Stringify(r) } +// HookResponse is a part of HookDelivery that contains +// the HTTP headers and the response body served by the webhook endpoint. type HookResponse struct { Headers map[string]string `json:"headers,omitempty"` RawPayload *json.RawMessage `json:"payload,omitempty"` From 1381c0f3a94a92fb26d5cc36e0c5b818245a2739 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Jul 2021 00:01:43 +0000 Subject: [PATCH 12/15] Accept suggestions from gmlewis with thanks :) https://github.com/google/go-github/pull/1934#pullrequestreview-702120314 --- github/repos_hooks_deliveries.go | 108 ++------------------------ github/repos_hooks_deliveries_test.go | 8 +- 2 files changed, 12 insertions(+), 104 deletions(-) diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go index ab7ba52e0d1..f16e917359c 100644 --- a/github/repos_hooks_deliveries.go +++ b/github/repos_hooks_deliveries.go @@ -103,106 +103,14 @@ func (s *RepositoriesService) GetHookDelivery(ctx context.Context, owner, repo s return h, resp, nil } +// ParseRequestPayload parses the request payload. For recognized event types, +// a value of the corresponding struct type will be returned. func (d *HookDelivery) ParseRequestPayload() (interface{}, error) { - var payload interface{} - switch *d.Event { - case "check_run": - payload = &CheckRunEvent{} - case "check_suite": - payload = &CheckSuiteEvent{} - case "commit_comment": - payload = &CommitCommentEvent{} - case "content_reference": - payload = &ContentReferenceEvent{} - case "create": - payload = &CreateEvent{} - case "delete": - payload = &DeleteEvent{} - case "deploy_ket": - payload = &DeployKeyEvent{} - case "deployment": - payload = &DeploymentEvent{} - case "deployment_status": - payload = &DeploymentStatusEvent{} - case "fork": - payload = &ForkEvent{} - case "github_app_authorization": - payload = &GitHubAppAuthorizationEvent{} - case "gollum": - payload = &GollumEvent{} - case "installation": - payload = &InstallationEvent{} - case "installation_repositories": - payload = &InstallationRepositoriesEvent{} - case "issue_comment": - payload = &IssueCommentEvent{} - case "issues": - payload = &IssuesEvent{} - case "label": - payload = &LabelEvent{} - case "marketplace_purchase": - payload = &MarketplacePurchaseEvent{} - case "member_event": - payload = &MemberEvent{} - case "membership_event": - payload = &MembershipEvent{} - case "meta": - payload = &MetaEvent{} - case "milestone": - payload = &MilestoneEvent{} - case "organization": - payload = &OrganizationEvent{} - case "org_block": - payload = &OrgBlockEvent{} - case "package": - payload = &PackageEvent{} - case "page_build": - payload = &PageBuildEvent{} - case "ping": - payload = &PingEvent{} - case "project": - payload = &ProjectEvent{} - case "project_card": - payload = &ProjectCardEvent{} - case "project_column": - payload = &ProjectColumnEvent{} - case "public": - payload = &PublicEvent{} - case "pull_request": - payload = &PullRequestEvent{} - case "pull_request_review": - payload = &PullRequestReviewEvent{} - case "pull_request_review_comment": - payload = &PullRequestReviewCommentEvent{} - case "pull_request_target": - payload = &PullRequestTargetEvent{} - case "push": - payload = &PushEvent{} - case "release": - payload = &ReleaseEvent{} - case "repository": - payload = &RepositoryEvent{} - case "repository_dispatch": - payload = &RepositoryDispatchEvent{} - case "repository_vulnerability_alert": - payload = &RepositoryVulnerabilityAlertEvent{} - case "star": - payload = &StarEvent{} - case "status": - payload = &StatusEvent{} - case "team": - payload = &TeamEvent{} - case "team_add": - payload = &TeamAddEvent{} - case "user": - payload = &UserEvent{} - case "watch": - payload = &WatchEvent{} - case "workflow_dispatch": - payload = &WorkflowDispatchEvent{} - case "workflow_run": - payload = &WorkflowRunEvent{} + eType, ok := eventTypeMapping[*d.Event] + if !ok { + return nil, fmt.Errorf("unsupported event type %q", *d.Event) } - err := json.Unmarshal(*d.Request.RawPayload, &payload) - return payload, err + + e := &Event{Type: &eType, RawPayload: d.Request.RawPayload} + return e.ParsePayload() } diff --git a/github/repos_hooks_deliveries_test.go b/github/repos_hooks_deliveries_test.go index 9ba72e3897e..5c6fb86d7eb 100644 --- a/github/repos_hooks_deliveries_test.go +++ b/github/repos_hooks_deliveries_test.go @@ -114,7 +114,7 @@ var hookDeliveryPayloadTypeToStruct = map[string]interface{}{ "content_reference": &ContentReferenceEvent{}, "create": &CreateEvent{}, "delete": &DeleteEvent{}, - "deploy_ket": &DeployKeyEvent{}, + "deploy_key": &DeployKeyEvent{}, "deployment": &DeploymentEvent{}, "deployment_status": &DeploymentStatusEvent{}, "fork": &ForkEvent{}, @@ -126,8 +126,8 @@ var hookDeliveryPayloadTypeToStruct = map[string]interface{}{ "issues": &IssuesEvent{}, "label": &LabelEvent{}, "marketplace_purchase": &MarketplacePurchaseEvent{}, - "member_event": &MemberEvent{}, - "membership_event": &MembershipEvent{}, + "member": &MemberEvent{}, + "membership": &MembershipEvent{}, "meta": &MetaEvent{}, "milestone": &MilestoneEvent{}, "organization": &OrganizationEvent{}, @@ -198,7 +198,7 @@ func TestHookDelivery_ParsePayload_invalidEvent(t *testing.T) { } _, err := d.ParseRequestPayload() - if err == nil || err.Error() != "unexpected end of JSON input" { + if err == nil || err.Error() != `unsupported event type "some_invalid_event"` { t.Errorf("unexpected error: %v", err) } } From 0a3739c94ba1eb5fe9f040f465b1e5387eb802e1 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sun, 11 Jul 2021 07:58:16 +0000 Subject: [PATCH 13/15] Add support for listing and getting organization webhook deliveries Ref #1933 --- github/orgs_hooks_deliveries.go | 48 ++++++++++++ github/orgs_hooks_deliveries_test.go | 106 +++++++++++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 github/orgs_hooks_deliveries.go create mode 100644 github/orgs_hooks_deliveries_test.go diff --git a/github/orgs_hooks_deliveries.go b/github/orgs_hooks_deliveries.go new file mode 100644 index 00000000000..6703912af4d --- /dev/null +++ b/github/orgs_hooks_deliveries.go @@ -0,0 +1,48 @@ +package github + +import ( + "context" + "fmt" +) + +// ListHookDeliveries lists webhook deliveries for a webhook configured in an organization. +// +// GitHub API docs: https://docs.github.com/en/rest/reference/orgs#list-deliveries-for-an-organization-webhook +func (s *OrganizationsService) ListHookDeliveries(ctx context.Context, org string, id int64, opts *ListCursorOptions) ([]*HookDelivery, *Response, error) { + u := fmt.Sprintf("orgs/%v/hooks/%v/deliveries", org, id) + u, err := addOptions(u, opts) + if err != nil { + return nil, nil, err + } + + req, err := s.client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + + deliveries := []*HookDelivery{} + resp, err := s.client.Do(ctx, req, &deliveries) + if err != nil { + return nil, resp, err + } + + return deliveries, resp, nil +} + +// GetHookDelivery returns a delivery for a webhook configured in an organization. +// +// GitHub API docs: https://docs.github.com/en/rest/reference/orgs#get-a-webhook-delivery-for-an-organization-webhook +func (s *OrganizationsService) GetHookDelivery(ctx context.Context, owner string, hookID, deliveryID int64) (*HookDelivery, *Response, error) { + u := fmt.Sprintf("orgs/%v/hooks/%v/deliveries/%v", owner, hookID, deliveryID) + req, err := s.client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + h := new(HookDelivery) + resp, err := s.client.Do(ctx, req, h) + if err != nil { + return nil, resp, err + } + + return h, resp, nil +} diff --git a/github/orgs_hooks_deliveries_test.go b/github/orgs_hooks_deliveries_test.go new file mode 100644 index 00000000000..c1efc527aa8 --- /dev/null +++ b/github/orgs_hooks_deliveries_test.go @@ -0,0 +1,106 @@ +// Copyright 2021 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package github + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestOrganizationsService_ListHookDeliveries(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/orgs/o/hooks/1/deliveries", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testFormValues(t, r, values{"cursor": "v1_12077215967"}) + fmt.Fprint(w, `[{"id":1}, {"id":2}]`) + }) + + opt := &ListCursorOptions{Cursor: "v1_12077215967"} + + ctx := context.Background() + hooks, _, err := client.Organizations.ListHookDeliveries(ctx, "o", 1, opt) + if err != nil { + t.Errorf("Organizations.ListHookDeliveries returned error: %v", err) + } + + want := []*HookDelivery{{ID: Int64(1)}, {ID: Int64(2)}} + if d := cmp.Diff(hooks, want); d != "" { + t.Errorf("Organizations.ListHooks want (-), got (+):\n%s", d) + } + + const methodName = "ListHookDeliveries" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Organizations.ListHookDeliveries(ctx, "\n", -1, opt) + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.ListHookDeliveries(ctx, "o", 1, opt) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestOrganizationsService_ListHookDeliveries_invalidOwner(t *testing.T) { + client, _, _, teardown := setup() + defer teardown() + + ctx := context.Background() + _, _, err := client.Organizations.ListHookDeliveries(ctx, "%", 1, nil) + testURLParseError(t, err) +} + +func TestOrganizationsService_GetHookDelivery(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/orgs/o/hooks/1/deliveries/1", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + fmt.Fprint(w, `{"id":1}`) + }) + + ctx := context.Background() + hook, _, err := client.Organizations.GetHookDelivery(ctx, "o", 1, 1) + if err != nil { + t.Errorf("Organizations.GetHookDelivery returned error: %v", err) + } + + want := &HookDelivery{ID: Int64(1)} + if !cmp.Equal(hook, want) { + t.Errorf("Organizations.GetHookDelivery returned %+v, want %+v", hook, want) + } + + const methodName = "GetHookDelivery" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Organizations.GetHookDelivery(ctx, "\n", -1, -1) + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.GetHookDelivery(ctx, "o", 1, 1) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestOrganizationsService_GetHookDelivery_invalidOwner(t *testing.T) { + client, _, _, teardown := setup() + defer teardown() + + ctx := context.Background() + _, _, err := client.Organizations.GetHookDelivery(ctx, "%", 1, 1) + testURLParseError(t, err) +} From fc8d20ffc9b9e8e47e5e77df694331ea9ecefbb1 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Mon, 12 Jul 2021 23:40:41 +0000 Subject: [PATCH 14/15] Add copyright notice to orgs_hooks_deliveries.go --- github/orgs_hooks_deliveries.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/github/orgs_hooks_deliveries.go b/github/orgs_hooks_deliveries.go index 6703912af4d..3c952f4fc29 100644 --- a/github/orgs_hooks_deliveries.go +++ b/github/orgs_hooks_deliveries.go @@ -1,3 +1,8 @@ +// Copyright 2021 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + package github import ( From 123bafa36d5a6d2c5ee4a3a57644653ad6ec9560 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Mon, 12 Jul 2021 23:41:53 +0000 Subject: [PATCH 15/15] Add blank after if err block for consistency --- github/orgs_hooks_deliveries.go | 1 + github/repos_hooks_deliveries.go | 1 + 2 files changed, 2 insertions(+) diff --git a/github/orgs_hooks_deliveries.go b/github/orgs_hooks_deliveries.go index 3c952f4fc29..6ab2d7aa244 100644 --- a/github/orgs_hooks_deliveries.go +++ b/github/orgs_hooks_deliveries.go @@ -43,6 +43,7 @@ func (s *OrganizationsService) GetHookDelivery(ctx context.Context, owner string if err != nil { return nil, nil, err } + h := new(HookDelivery) resp, err := s.client.Do(ctx, req, h) if err != nil { diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go index f16e917359c..122674463ae 100644 --- a/github/repos_hooks_deliveries.go +++ b/github/repos_hooks_deliveries.go @@ -94,6 +94,7 @@ func (s *RepositoriesService) GetHookDelivery(ctx context.Context, owner, repo s if err != nil { return nil, nil, err } + h := new(HookDelivery) resp, err := s.client.Do(ctx, req, h) if err != nil {