From 575929f01ae3cd35e026885f08a46f36bdba6c1b Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Thu, 10 Aug 2023 06:58:58 -0700 Subject: [PATCH 1/6] Add WebhookTypes and EventForType methods --- github/event.go | 131 ++--------------------------------- github/messages.go | 149 ++++++++++++++++++++++++++++++++++++++++ github/messages_test.go | 8 +++ 3 files changed, 162 insertions(+), 126 deletions(-) diff --git a/github/event.go b/github/event.go index 2349cb77600..9921f2b2016 100644 --- a/github/event.go +++ b/github/event.go @@ -27,132 +27,11 @@ func (e Event) String() string { // ParsePayload parses the event payload. For recognized event types, // a value of the corresponding struct type will be returned. -func (e *Event) ParsePayload() (payload interface{}, err error) { - switch *e.Type { - case "BranchProtectionRuleEvent": - payload = &BranchProtectionRuleEvent{} - case "CheckRunEvent": - payload = &CheckRunEvent{} - case "CheckSuiteEvent": - payload = &CheckSuiteEvent{} - case "CodeScanningAlertEvent": - payload = &CodeScanningAlertEvent{} - case "CommitCommentEvent": - payload = &CommitCommentEvent{} - case "ContentReferenceEvent": - payload = &ContentReferenceEvent{} - case "CreateEvent": - payload = &CreateEvent{} - case "DeleteEvent": - payload = &DeleteEvent{} - case "DeployKeyEvent": - payload = &DeployKeyEvent{} - case "DeploymentEvent": - payload = &DeploymentEvent{} - case "DeploymentProtectionRuleEvent": - payload = &DeploymentProtectionRuleEvent{} - case "DeploymentStatusEvent": - payload = &DeploymentStatusEvent{} - case "DiscussionEvent": - payload = &DiscussionEvent{} - case "DiscussionCommentEvent": - payload = &DiscussionCommentEvent{} - case "ForkEvent": - payload = &ForkEvent{} - case "GitHubAppAuthorizationEvent": - payload = &GitHubAppAuthorizationEvent{} - case "GollumEvent": - payload = &GollumEvent{} - case "InstallationEvent": - payload = &InstallationEvent{} - case "InstallationRepositoriesEvent": - payload = &InstallationRepositoriesEvent{} - case "InstallationTargetEvent": - payload = &InstallationTargetEvent{} - case "IssueCommentEvent": - payload = &IssueCommentEvent{} - case "IssuesEvent": - payload = &IssuesEvent{} - case "LabelEvent": - payload = &LabelEvent{} - case "MarketplacePurchaseEvent": - payload = &MarketplacePurchaseEvent{} - case "MemberEvent": - payload = &MemberEvent{} - case "MembershipEvent": - payload = &MembershipEvent{} - case "MergeGroupEvent": - payload = &MergeGroupEvent{} - case "MetaEvent": - payload = &MetaEvent{} - case "MilestoneEvent": - payload = &MilestoneEvent{} - case "OrganizationEvent": - payload = &OrganizationEvent{} - case "OrgBlockEvent": - payload = &OrgBlockEvent{} - case "PackageEvent": - payload = &PackageEvent{} - case "PageBuildEvent": - payload = &PageBuildEvent{} - case "PersonalAccessTokenRequestEvent": - payload = &PersonalAccessTokenRequestEvent{} - case "PingEvent": - payload = &PingEvent{} - case "ProjectEvent": - payload = &ProjectEvent{} - case "ProjectCardEvent": - payload = &ProjectCardEvent{} - case "ProjectColumnEvent": - payload = &ProjectColumnEvent{} - case "PublicEvent": - payload = &PublicEvent{} - case "PullRequestEvent": - payload = &PullRequestEvent{} - case "PullRequestReviewEvent": - payload = &PullRequestReviewEvent{} - case "PullRequestReviewCommentEvent": - payload = &PullRequestReviewCommentEvent{} - case "PullRequestReviewThreadEvent": - payload = &PullRequestReviewThreadEvent{} - case "PullRequestTargetEvent": - payload = &PullRequestTargetEvent{} - case "PushEvent": - payload = &PushEvent{} - case "ReleaseEvent": - payload = &ReleaseEvent{} - case "RepositoryEvent": - payload = &RepositoryEvent{} - case "RepositoryDispatchEvent": - payload = &RepositoryDispatchEvent{} - case "RepositoryImportEvent": - payload = &RepositoryImportEvent{} - case "RepositoryVulnerabilityAlertEvent": - payload = &RepositoryVulnerabilityAlertEvent{} - case "SecretScanningAlertEvent": - payload = &SecretScanningAlertEvent{} - case "SecurityAdvisoryEvent": - payload = &SecurityAdvisoryEvent{} - case "StarEvent": - payload = &StarEvent{} - case "StatusEvent": - payload = &StatusEvent{} - case "TeamEvent": - payload = &TeamEvent{} - case "TeamAddEvent": - payload = &TeamAddEvent{} - case "UserEvent": - payload = &UserEvent{} - case "WatchEvent": - payload = &WatchEvent{} - case "WorkflowDispatchEvent": - payload = &WorkflowDispatchEvent{} - case "WorkflowJobEvent": - payload = &WorkflowJobEvent{} - case "WorkflowRunEvent": - payload = &WorkflowRunEvent{} - } - err = json.Unmarshal(*e.RawPayload, &payload) +func (e *Event) ParsePayload() (interface{}, error) { + // It would be nice if e.Type were the snake_case name of the event, + // but the existing interface uses the struct name instead. + payload := EventForType(typeToMessageMapping[*e.Type]) + err := json.Unmarshal(*e.RawPayload, &payload) return payload, err } diff --git a/github/messages.go b/github/messages.go index 929587d337f..903247eb04f 100644 --- a/github/messages.go +++ b/github/messages.go @@ -106,8 +106,16 @@ var ( "workflow_job": "WorkflowJobEvent", "workflow_run": "WorkflowRunEvent", } + // Inverse map of the above + typeToMessageMapping = make(map[string]string, len(eventTypeMapping)) ) +func init() { + for k, v := range eventTypeMapping { + typeToMessageMapping[v] = k + } +} + // genMAC generates the HMAC signature for a message provided the secret key // and hashFunc. func genMAC(message, key []byte, hashFunc func() hash.Hash) []byte { @@ -307,3 +315,144 @@ func ParseWebHook(messageType string, payload []byte) (interface{}, error) { } return event.ParsePayload() } + +// WebhookTypes returns a list of all the known GitHub event type strings +// supported by go-github. +func MessageTypes() []string { + types := make([]string, 0, len(eventTypeMapping)) + for t := range eventTypeMapping { + types = append(types, t) + } + return types +} + +// EventForType returns an empty struct matching the specified GitHub event type. +// If messageType does not match any known event types, it returns nil. +func EventForType(messageType string) interface{} { + switch messageType { + case "branch_protection_rule": + return &BranchProtectionRuleEvent{} + case "check_run": + return &CheckRunEvent{} + case "check_suite": + return &CheckSuiteEvent{} + case "code_scanning_alert": + return &CodeScanningAlertEvent{} + case "commit_comment": + return &CommitCommentEvent{} + case "content_reference": + return &ContentReferenceEvent{} + case "create": + return &CreateEvent{} + case "delete": + return &DeleteEvent{} + case "deploy_key": + return &DeployKeyEvent{} + case "deployment": + return &DeploymentEvent{} + case "deployment_protection_rule": + return &DeploymentProtectionRuleEvent{} + case "deployment_status": + return &DeploymentStatusEvent{} + case "discussion": + return &DiscussionEvent{} + case "discussion_comment": + return &DiscussionCommentEvent{} + case "fork": + return &ForkEvent{} + case "github_app_authorization": + return &GitHubAppAuthorizationEvent{} + case "gollum": + return &GollumEvent{} + case "installation": + return &InstallationEvent{} + case "installation_repositories": + return &InstallationRepositoriesEvent{} + case "installation_target": + return &InstallationTargetEvent{} + case "issue_comment": + return &IssueCommentEvent{} + case "issues": + return &IssuesEvent{} + case "label": + return &LabelEvent{} + case "marketplace_purchase": + return &MarketplacePurchaseEvent{} + case "member": + return &MemberEvent{} + case "membership": + return &MembershipEvent{} + case "merge_group": + return &MergeGroupEvent{} + case "meta": + return &MetaEvent{} + case "milestone": + return &MilestoneEvent{} + case "organization": + return &OrganizationEvent{} + case "org_block": + return &OrgBlockEvent{} + case "package": + return &PackageEvent{} + case "page_build": + return &PageBuildEvent{} + case "personal_access_token_request": + return &PersonalAccessTokenRequestEvent{} + case "ping": + return &PingEvent{} + case "project": + return &ProjectEvent{} + case "project_card": + return &ProjectCardEvent{} + case "project_column": + return &ProjectColumnEvent{} + case "public": + return &PublicEvent{} + case "pull_request": + return &PullRequestEvent{} + case "pull_request_review": + return &PullRequestReviewEvent{} + case "pull_request_review_comment": + return &PullRequestReviewCommentEvent{} + case "pull_request_review_thread": + return &PullRequestReviewThreadEvent{} + case "pull_request_target": + return &PullRequestTargetEvent{} + case "push": + return &PushEvent{} + case "release": + return &ReleaseEvent{} + case "repository": + return &RepositoryEvent{} + case "repository_dispatch": + return &RepositoryDispatchEvent{} + case "repository_import": + return &RepositoryImportEvent{} + case "repository_vulnerability_alert": + return &RepositoryVulnerabilityAlertEvent{} + case "secret_scanning_alert": + return &SecretScanningAlertEvent{} + case "security_advisory": + return &SecurityAdvisoryEvent{} + case "star": + return &StarEvent{} + case "status": + return &StatusEvent{} + case "team": + return &TeamEvent{} + case "team_add": + return &TeamAddEvent{} + case "user": + return &UserEvent{} + case "watch": + return &WatchEvent{} + case "workflow_dispatch": + return &WorkflowDispatchEvent{} + case "workflow_job": + return &WorkflowJobEvent{} + case "workflow_run": + return &WorkflowRunEvent{} + default: + return nil + } +} diff --git a/github/messages_test.go b/github/messages_test.go index 8ee38eb346b..628754cbfdb 100644 --- a/github/messages_test.go +++ b/github/messages_test.go @@ -513,6 +513,14 @@ func TestParseWebHook(t *testing.T) { } } +func TestAllMessageTypesMapped(t *testing.T) { + for _, mt := range MessageTypes() { + if obj := EventForType(mt); obj == nil { + t.Errorf("messageMap missing message type %q", mt) + } + } +} + func TestParseWebHook_BadMessageType(t *testing.T) { if _, err := ParseWebHook("bogus message type", []byte("{}")); err == nil { t.Fatal("ParseWebHook returned nil; wanted error") From 98b730e4b67dfeee66170dddf7c996304ff317da Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Thu, 10 Aug 2023 08:36:56 -0700 Subject: [PATCH 2/6] Switch to a map of types and reflection for names, reduce future code changes needed --- github/event.go | 2 +- github/messages.go | 264 +++++++++---------------------- github/repos_hooks_deliveries.go | 4 +- 3 files changed, 78 insertions(+), 192 deletions(-) diff --git a/github/event.go b/github/event.go index 9921f2b2016..947ff71ecc7 100644 --- a/github/event.go +++ b/github/event.go @@ -30,7 +30,7 @@ func (e Event) String() string { func (e *Event) ParsePayload() (interface{}, error) { // It would be nice if e.Type were the snake_case name of the event, // but the existing interface uses the struct name instead. - payload := EventForType(typeToMessageMapping[*e.Type]) + payload := EventForType(typeToMessageMapping[e.GetType()]) err := json.Unmarshal(*e.RawPayload, &payload) return payload, err } diff --git a/github/messages.go b/github/messages.go index 903247eb04f..dd3719681f5 100644 --- a/github/messages.go +++ b/github/messages.go @@ -22,6 +22,7 @@ import ( "mime" "net/http" "net/url" + "reflect" "strings" ) @@ -43,76 +44,80 @@ const ( var ( // eventTypeMapping maps webhooks types to their corresponding go-github struct types. - eventTypeMapping = map[string]string{ - "branch_protection_rule": "BranchProtectionRuleEvent", - "check_run": "CheckRunEvent", - "check_suite": "CheckSuiteEvent", - "code_scanning_alert": "CodeScanningAlertEvent", - "commit_comment": "CommitCommentEvent", - "content_reference": "ContentReferenceEvent", - "create": "CreateEvent", - "delete": "DeleteEvent", - "deploy_key": "DeployKeyEvent", - "deployment": "DeploymentEvent", - "deployment_status": "DeploymentStatusEvent", - "deployment_protection_rule": "DeploymentProtectionRuleEvent", - "discussion": "DiscussionEvent", - "discussion_comment": "DiscussionCommentEvent", - "fork": "ForkEvent", - "github_app_authorization": "GitHubAppAuthorizationEvent", - "gollum": "GollumEvent", - "installation": "InstallationEvent", - "installation_repositories": "InstallationRepositoriesEvent", - "installation_target": "InstallationTargetEvent", - "issue_comment": "IssueCommentEvent", - "issues": "IssuesEvent", - "label": "LabelEvent", - "marketplace_purchase": "MarketplacePurchaseEvent", - "member": "MemberEvent", - "membership": "MembershipEvent", - "merge_group": "MergeGroupEvent", - "meta": "MetaEvent", - "milestone": "MilestoneEvent", - "organization": "OrganizationEvent", - "org_block": "OrgBlockEvent", - "package": "PackageEvent", - "page_build": "PageBuildEvent", - "personal_access_token_request": "PersonalAccessTokenRequestEvent", - "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_review_thread": "PullRequestReviewThreadEvent", - "pull_request_target": "PullRequestTargetEvent", - "push": "PushEvent", - "repository": "RepositoryEvent", - "repository_dispatch": "RepositoryDispatchEvent", - "repository_import": "RepositoryImportEvent", - "repository_vulnerability_alert": "RepositoryVulnerabilityAlertEvent", - "release": "ReleaseEvent", - "secret_scanning_alert": "SecretScanningAlertEvent", - "security_advisory": "SecurityAdvisoryEvent", - "star": "StarEvent", - "status": "StatusEvent", - "team": "TeamEvent", - "team_add": "TeamAddEvent", - "user": "UserEvent", - "watch": "WatchEvent", - "workflow_dispatch": "WorkflowDispatchEvent", - "workflow_job": "WorkflowJobEvent", - "workflow_run": "WorkflowRunEvent", + eventTypeMapping = map[string]interface{}{ + "branch_protection_rule": &BranchProtectionRuleEvent{}, + "check_run": &CheckRunEvent{}, + "check_suite": &CheckSuiteEvent{}, + "code_scanning_alert": &CodeScanningAlertEvent{}, + "commit_comment": &CommitCommentEvent{}, + "content_reference": &ContentReferenceEvent{}, + "create": &CreateEvent{}, + "delete": &DeleteEvent{}, + "deploy_key": &DeployKeyEvent{}, + "deployment": &DeploymentEvent{}, + "deployment_status": &DeploymentStatusEvent{}, + "deployment_protection_rule": &DeploymentProtectionRuleEvent{}, + "discussion": &DiscussionEvent{}, + "discussion_comment": &DiscussionCommentEvent{}, + "fork": &ForkEvent{}, + "github_app_authorization": &GitHubAppAuthorizationEvent{}, + "gollum": &GollumEvent{}, + "installation": &InstallationEvent{}, + "installation_repositories": &InstallationRepositoriesEvent{}, + "installation_target": &InstallationTargetEvent{}, + "issue_comment": &IssueCommentEvent{}, + "issues": &IssuesEvent{}, + "label": &LabelEvent{}, + "marketplace_purchase": &MarketplacePurchaseEvent{}, + "member": &MemberEvent{}, + "membership": &MembershipEvent{}, + "merge_group": &MergeGroupEvent{}, + "meta": &MetaEvent{}, + "milestone": &MilestoneEvent{}, + "organization": &OrganizationEvent{}, + "org_block": &OrgBlockEvent{}, + "package": &PackageEvent{}, + "page_build": &PageBuildEvent{}, + "personal_access_token_request": &PersonalAccessTokenRequestEvent{}, + "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_review_thread": &PullRequestReviewThreadEvent{}, + "pull_request_target": &PullRequestTargetEvent{}, + "push": &PushEvent{}, + "repository": &RepositoryEvent{}, + "repository_dispatch": &RepositoryDispatchEvent{}, + "repository_import": &RepositoryImportEvent{}, + "repository_vulnerability_alert": &RepositoryVulnerabilityAlertEvent{}, + "release": &ReleaseEvent{}, + "secret_scanning_alert": &SecretScanningAlertEvent{}, + "security_advisory": &SecurityAdvisoryEvent{}, + "star": &StarEvent{}, + "status": &StatusEvent{}, + "team": &TeamEvent{}, + "team_add": &TeamAddEvent{}, + "user": &UserEvent{}, + "watch": &WatchEvent{}, + "workflow_dispatch": &WorkflowDispatchEvent{}, + "workflow_job": &WorkflowJobEvent{}, + "workflow_run": &WorkflowRunEvent{}, } + // forward mapping of event types to the sting names of the structs + messageToTypeName = make(map[string]string, len(eventTypeMapping)) // Inverse map of the above typeToMessageMapping = make(map[string]string, len(eventTypeMapping)) ) func init() { for k, v := range eventTypeMapping { - typeToMessageMapping[v] = k + typename := reflect.TypeOf(v).Elem().Name() + messageToTypeName[k] = typename + typeToMessageMapping[typename] = k } } @@ -304,7 +309,7 @@ func DeliveryID(r *http.Request) string { // } // } func ParseWebHook(messageType string, payload []byte) (interface{}, error) { - eventType, ok := eventTypeMapping[messageType] + eventType, ok := messageToTypeName[messageType] if !ok { return nil, fmt.Errorf("unknown X-Github-Event in message: %v", messageType) } @@ -329,130 +334,11 @@ func MessageTypes() []string { // EventForType returns an empty struct matching the specified GitHub event type. // If messageType does not match any known event types, it returns nil. func EventForType(messageType string) interface{} { - switch messageType { - case "branch_protection_rule": - return &BranchProtectionRuleEvent{} - case "check_run": - return &CheckRunEvent{} - case "check_suite": - return &CheckSuiteEvent{} - case "code_scanning_alert": - return &CodeScanningAlertEvent{} - case "commit_comment": - return &CommitCommentEvent{} - case "content_reference": - return &ContentReferenceEvent{} - case "create": - return &CreateEvent{} - case "delete": - return &DeleteEvent{} - case "deploy_key": - return &DeployKeyEvent{} - case "deployment": - return &DeploymentEvent{} - case "deployment_protection_rule": - return &DeploymentProtectionRuleEvent{} - case "deployment_status": - return &DeploymentStatusEvent{} - case "discussion": - return &DiscussionEvent{} - case "discussion_comment": - return &DiscussionCommentEvent{} - case "fork": - return &ForkEvent{} - case "github_app_authorization": - return &GitHubAppAuthorizationEvent{} - case "gollum": - return &GollumEvent{} - case "installation": - return &InstallationEvent{} - case "installation_repositories": - return &InstallationRepositoriesEvent{} - case "installation_target": - return &InstallationTargetEvent{} - case "issue_comment": - return &IssueCommentEvent{} - case "issues": - return &IssuesEvent{} - case "label": - return &LabelEvent{} - case "marketplace_purchase": - return &MarketplacePurchaseEvent{} - case "member": - return &MemberEvent{} - case "membership": - return &MembershipEvent{} - case "merge_group": - return &MergeGroupEvent{} - case "meta": - return &MetaEvent{} - case "milestone": - return &MilestoneEvent{} - case "organization": - return &OrganizationEvent{} - case "org_block": - return &OrgBlockEvent{} - case "package": - return &PackageEvent{} - case "page_build": - return &PageBuildEvent{} - case "personal_access_token_request": - return &PersonalAccessTokenRequestEvent{} - case "ping": - return &PingEvent{} - case "project": - return &ProjectEvent{} - case "project_card": - return &ProjectCardEvent{} - case "project_column": - return &ProjectColumnEvent{} - case "public": - return &PublicEvent{} - case "pull_request": - return &PullRequestEvent{} - case "pull_request_review": - return &PullRequestReviewEvent{} - case "pull_request_review_comment": - return &PullRequestReviewCommentEvent{} - case "pull_request_review_thread": - return &PullRequestReviewThreadEvent{} - case "pull_request_target": - return &PullRequestTargetEvent{} - case "push": - return &PushEvent{} - case "release": - return &ReleaseEvent{} - case "repository": - return &RepositoryEvent{} - case "repository_dispatch": - return &RepositoryDispatchEvent{} - case "repository_import": - return &RepositoryImportEvent{} - case "repository_vulnerability_alert": - return &RepositoryVulnerabilityAlertEvent{} - case "secret_scanning_alert": - return &SecretScanningAlertEvent{} - case "security_advisory": - return &SecurityAdvisoryEvent{} - case "star": - return &StarEvent{} - case "status": - return &StatusEvent{} - case "team": - return &TeamEvent{} - case "team_add": - return &TeamAddEvent{} - case "user": - return &UserEvent{} - case "watch": - return &WatchEvent{} - case "workflow_dispatch": - return &WorkflowDispatchEvent{} - case "workflow_job": - return &WorkflowJobEvent{} - case "workflow_run": - return &WorkflowRunEvent{} - default: - return nil + prototype := eventTypeMapping[messageType] + if prototype != nil { + // return a _copy_ of the pointed-to-object. Unfortunately, for this + // we need to use reflection. + return reflect.New(reflect.TypeOf(prototype).Elem()).Interface() } + return prototype } diff --git a/github/repos_hooks_deliveries.go b/github/repos_hooks_deliveries.go index cbd2d3819a1..5f2f8807e4b 100644 --- a/github/repos_hooks_deliveries.go +++ b/github/repos_hooks_deliveries.go @@ -126,9 +126,9 @@ func (s *RepositoriesService) RedeliverHookDelivery(ctx context.Context, owner, // 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) { - eType, ok := eventTypeMapping[*d.Event] + eType, ok := messageToTypeName[d.GetEvent()] if !ok { - return nil, fmt.Errorf("unsupported event type %q", *d.Event) + return nil, fmt.Errorf("unsupported event type %q", d.GetEvent()) } e := &Event{Type: &eType, RawPayload: d.Request.RawPayload} From 2b545ee043cfceef526df2be5ad97add46c2778e Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Thu, 10 Aug 2023 10:01:35 -0700 Subject: [PATCH 3/6] Fix Unmarshal(nil) error and add tests for ParsePayload on a zero-value Event{} --- github/event.go | 12 ++++++++---- github/event_test.go | 5 +++++ github/messages_test.go | 9 +++++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/github/event.go b/github/event.go index 947ff71ecc7..8088bf9cac7 100644 --- a/github/event.go +++ b/github/event.go @@ -27,12 +27,16 @@ func (e Event) String() string { // ParsePayload parses the event payload. For recognized event types, // a value of the corresponding struct type will be returned. -func (e *Event) ParsePayload() (interface{}, error) { +func (e *Event) ParsePayload() (payload interface{}, err error) { // It would be nice if e.Type were the snake_case name of the event, // but the existing interface uses the struct name instead. - payload := EventForType(typeToMessageMapping[e.GetType()]) - err := json.Unmarshal(*e.RawPayload, &payload) - return payload, err + typed := EventForType(typeToMessageMapping[e.GetType()]) + if typed != nil { + payload = typed + } + err = json.Unmarshal(e.GetRawPayload(), &payload) + + return } // Payload returns the parsed event payload. For recognized event types, diff --git a/github/event_test.go b/github/event_test.go index 6d20582c2f7..a62f2bfb944 100644 --- a/github/event_test.go +++ b/github/event_test.go @@ -30,6 +30,11 @@ func TestPayload_NoPanic(t *testing.T) { e.Payload() } +func TestEmptyEvent_NoPanic(t *testing.T) { + e := &Event{} + e.ParsePayload() +} + func TestEvent_Marshal(t *testing.T) { testJSONMarshal(t, &Event{}, "{}") diff --git a/github/messages_test.go b/github/messages_test.go index 628754cbfdb..f0024e56502 100644 --- a/github/messages_test.go +++ b/github/messages_test.go @@ -521,6 +521,15 @@ func TestAllMessageTypesMapped(t *testing.T) { } } +func TestUnknownMessageType(t *testing.T) { + if obj := EventForType("unknown"); obj != nil { + t.Errorf("EventForType(unknown) = %#v, want nil", obj) + } + if obj := EventForType(""); obj != nil { + t.Errorf(`EventForType("") = %#v, want nil`, obj) + } +} + func TestParseWebHook_BadMessageType(t *testing.T) { if _, err := ParseWebHook("bogus message type", []byte("{}")); err == nil { t.Fatal("ParseWebHook returned nil; wanted error") From 759f9bedfd992295cbeed8dffaa61644e1dc0384 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 14 Aug 2023 13:23:16 -0700 Subject: [PATCH 4/6] Address gmlewis feedback --- github/event_test.go | 9 ++++++++- github/messages.go | 12 +++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/github/event_test.go b/github/event_test.go index a62f2bfb944..66a6bcd7e45 100644 --- a/github/event_test.go +++ b/github/event_test.go @@ -32,7 +32,14 @@ func TestPayload_NoPanic(t *testing.T) { func TestEmptyEvent_NoPanic(t *testing.T) { e := &Event{} - e.ParsePayload() + if _, err := e.ParsePayload(); err == nil { + t.Error("ParsePayload unexpectedly succeeded on empty event") + } + + e = nil + if _, err := e.ParsePayload(); err == nil { + t.Error("ParsePayload unexpectedly succeeded on nil event") + } } func TestEvent_Marshal(t *testing.T) { diff --git a/github/messages.go b/github/messages.go index 22a22b10f98..d3a1730d84a 100644 --- a/github/messages.go +++ b/github/messages.go @@ -336,10 +336,12 @@ func MessageTypes() []string { // If messageType does not match any known event types, it returns nil. func EventForType(messageType string) interface{} { prototype := eventTypeMapping[messageType] - if prototype != nil { - // return a _copy_ of the pointed-to-object. Unfortunately, for this - // we need to use reflection. - return reflect.New(reflect.TypeOf(prototype).Elem()).Interface() + if prototype == nil { + return nil } - return prototype + // return a _copy_ of the pointed-to-object. Unfortunately, for this we + // need to use reflection. If we store the actual objects in the map, + // we still need to use reflection to convert from `any` to the actual + // type, so this was deemed the lesser of two weevils. (#2865) + return reflect.New(reflect.TypeOf(prototype).Elem()).Interface() } From ed582399cf2c87594998f3d716cb3564ddf5d2ee Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 15 Aug 2023 11:52:31 -0700 Subject: [PATCH 5/6] Use :=, since go figures out the type correctly --- github/event.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/event.go b/github/event.go index c0520b80256..e98606bce5d 100644 --- a/github/event.go +++ b/github/event.go @@ -30,7 +30,7 @@ func (e Event) String() string { func (e *Event) ParsePayload() (interface{}, error) { // It would be nice if e.Type were the snake_case name of the event, // but the existing interface uses the struct name instead. - var payload interface{} = EventForType(typeToMessageMapping[e.GetType()]) + payload := EventForType(typeToMessageMapping[e.GetType()]) if err := json.Unmarshal(e.GetRawPayload(), &payload); err != nil { return nil, err From 848850aceb108d988216ca118036318b726c23ed Mon Sep 17 00:00:00 2001 From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> Date: Wed, 16 Aug 2023 12:58:26 -0400 Subject: [PATCH 6/6] Update github/messages.go Co-authored-by: Parker77 <20973702+Parker77@users.noreply.github.com> --- github/messages.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/messages.go b/github/messages.go index a6836c83b95..e4fa6f6aa01 100644 --- a/github/messages.go +++ b/github/messages.go @@ -111,7 +111,7 @@ var ( "workflow_job": &WorkflowJobEvent{}, "workflow_run": &WorkflowRunEvent{}, } - // forward mapping of event types to the sting names of the structs + // forward mapping of event types to the string names of the structs messageToTypeName = make(map[string]string, len(eventTypeMapping)) // Inverse map of the above typeToMessageMapping = make(map[string]string, len(eventTypeMapping))