From 436e1fd9b52e2dba98a16bc8b07aa4a5bcbf0148 Mon Sep 17 00:00:00 2001 From: Sven Palberg Date: Sun, 11 Aug 2024 03:37:23 +0200 Subject: [PATCH] feat!: Widen CustomProperties type to map[string]interface{} to align with GitHub API (#3230) Fixes: #3229. BREAKING CHANGE: PushEventRepository.CustomProperties is changed from map[string]string to map[string]interface{}. --- AUTHORS | 1 + github/event_types.go | 76 +++++++++++------------ github/github-accessors.go | 16 ----- github/github-accessors_test.go | 20 ------ github/repos.go | 106 ++++++++++++++++---------------- github/repos_test.go | 47 ++++++++++++++ 6 files changed, 139 insertions(+), 127 deletions(-) diff --git a/AUTHORS b/AUTHORS index 0197b94a23a..075ba8dfa3d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -431,6 +431,7 @@ Steve Teuber Stian Eikeland Suhaib Mujahid sushmita wable +Sven Palberg Szymon Kodrebski Søren Hansen T.J. Corrigan diff --git a/github/event_types.go b/github/event_types.go index e5ae33a5fa0..df8d9e033e2 100644 --- a/github/event_types.go +++ b/github/event_types.go @@ -1348,44 +1348,44 @@ func (h HeadCommit) String() string { // PushEventRepository represents the repo object in a PushEvent payload. type PushEventRepository struct { - ID *int64 `json:"id,omitempty"` - NodeID *string `json:"node_id,omitempty"` - Name *string `json:"name,omitempty"` - FullName *string `json:"full_name,omitempty"` - Owner *User `json:"owner,omitempty"` - Private *bool `json:"private,omitempty"` - Description *string `json:"description,omitempty"` - Fork *bool `json:"fork,omitempty"` - CreatedAt *Timestamp `json:"created_at,omitempty"` - PushedAt *Timestamp `json:"pushed_at,omitempty"` - UpdatedAt *Timestamp `json:"updated_at,omitempty"` - Homepage *string `json:"homepage,omitempty"` - PullsURL *string `json:"pulls_url,omitempty"` - Size *int `json:"size,omitempty"` - StargazersCount *int `json:"stargazers_count,omitempty"` - WatchersCount *int `json:"watchers_count,omitempty"` - Language *string `json:"language,omitempty"` - HasIssues *bool `json:"has_issues,omitempty"` - HasDownloads *bool `json:"has_downloads,omitempty"` - HasWiki *bool `json:"has_wiki,omitempty"` - HasPages *bool `json:"has_pages,omitempty"` - ForksCount *int `json:"forks_count,omitempty"` - Archived *bool `json:"archived,omitempty"` - Disabled *bool `json:"disabled,omitempty"` - OpenIssuesCount *int `json:"open_issues_count,omitempty"` - DefaultBranch *string `json:"default_branch,omitempty"` - MasterBranch *string `json:"master_branch,omitempty"` - Organization *string `json:"organization,omitempty"` - URL *string `json:"url,omitempty"` - ArchiveURL *string `json:"archive_url,omitempty"` - HTMLURL *string `json:"html_url,omitempty"` - StatusesURL *string `json:"statuses_url,omitempty"` - GitURL *string `json:"git_url,omitempty"` - SSHURL *string `json:"ssh_url,omitempty"` - CloneURL *string `json:"clone_url,omitempty"` - SVNURL *string `json:"svn_url,omitempty"` - Topics []string `json:"topics,omitempty"` - CustomProperties map[string]string `json:"custom_properties,omitempty"` + ID *int64 `json:"id,omitempty"` + NodeID *string `json:"node_id,omitempty"` + Name *string `json:"name,omitempty"` + FullName *string `json:"full_name,omitempty"` + Owner *User `json:"owner,omitempty"` + Private *bool `json:"private,omitempty"` + Description *string `json:"description,omitempty"` + Fork *bool `json:"fork,omitempty"` + CreatedAt *Timestamp `json:"created_at,omitempty"` + PushedAt *Timestamp `json:"pushed_at,omitempty"` + UpdatedAt *Timestamp `json:"updated_at,omitempty"` + Homepage *string `json:"homepage,omitempty"` + PullsURL *string `json:"pulls_url,omitempty"` + Size *int `json:"size,omitempty"` + StargazersCount *int `json:"stargazers_count,omitempty"` + WatchersCount *int `json:"watchers_count,omitempty"` + Language *string `json:"language,omitempty"` + HasIssues *bool `json:"has_issues,omitempty"` + HasDownloads *bool `json:"has_downloads,omitempty"` + HasWiki *bool `json:"has_wiki,omitempty"` + HasPages *bool `json:"has_pages,omitempty"` + ForksCount *int `json:"forks_count,omitempty"` + Archived *bool `json:"archived,omitempty"` + Disabled *bool `json:"disabled,omitempty"` + OpenIssuesCount *int `json:"open_issues_count,omitempty"` + DefaultBranch *string `json:"default_branch,omitempty"` + MasterBranch *string `json:"master_branch,omitempty"` + Organization *string `json:"organization,omitempty"` + URL *string `json:"url,omitempty"` + ArchiveURL *string `json:"archive_url,omitempty"` + HTMLURL *string `json:"html_url,omitempty"` + StatusesURL *string `json:"statuses_url,omitempty"` + GitURL *string `json:"git_url,omitempty"` + SSHURL *string `json:"ssh_url,omitempty"` + CloneURL *string `json:"clone_url,omitempty"` + SVNURL *string `json:"svn_url,omitempty"` + Topics []string `json:"topics,omitempty"` + CustomProperties map[string]interface{} `json:"custom_properties,omitempty"` } // PushEventRepoOwner is a basic representation of user/org in a PushEvent payload. diff --git a/github/github-accessors.go b/github/github-accessors.go index 0b121aac7cb..7d4a0782869 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -17726,14 +17726,6 @@ func (p *PushEventRepository) GetCreatedAt() Timestamp { return *p.CreatedAt } -// GetCustomProperties returns the CustomProperties map if it's non-nil, an empty map otherwise. -func (p *PushEventRepository) GetCustomProperties() map[string]string { - if p == nil || p.CustomProperties == nil { - return map[string]string{} - } - return p.CustomProperties -} - // GetDefaultBranch returns the DefaultBranch field if it's non-nil, zero value otherwise. func (p *PushEventRepository) GetDefaultBranch() string { if p == nil || p.DefaultBranch == nil { @@ -18878,14 +18870,6 @@ func (r *Repository) GetCreatedAt() Timestamp { return *r.CreatedAt } -// GetCustomProperties returns the CustomProperties map if it's non-nil, an empty map otherwise. -func (r *Repository) GetCustomProperties() map[string]string { - if r == nil || r.CustomProperties == nil { - return map[string]string{} - } - return r.CustomProperties -} - // GetDefaultBranch returns the DefaultBranch field if it's non-nil, zero value otherwise. func (r *Repository) GetDefaultBranch() string { if r == nil || r.DefaultBranch == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index f41615c9e8e..a401ecc1693 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -20520,16 +20520,6 @@ func TestPushEventRepository_GetCreatedAt(tt *testing.T) { p.GetCreatedAt() } -func TestPushEventRepository_GetCustomProperties(tt *testing.T) { - zeroValue := map[string]string{} - p := &PushEventRepository{CustomProperties: zeroValue} - p.GetCustomProperties() - p = &PushEventRepository{} - p.GetCustomProperties() - p = nil - p.GetCustomProperties() -} - func TestPushEventRepository_GetDefaultBranch(tt *testing.T) { var zeroValue string p := &PushEventRepository{DefaultBranch: &zeroValue} @@ -21891,16 +21881,6 @@ func TestRepository_GetCreatedAt(tt *testing.T) { r.GetCreatedAt() } -func TestRepository_GetCustomProperties(tt *testing.T) { - zeroValue := map[string]string{} - r := &Repository{CustomProperties: zeroValue} - r.GetCustomProperties() - r = &Repository{} - r.GetCustomProperties() - r = nil - r.GetCustomProperties() -} - func TestRepository_GetDefaultBranch(tt *testing.T) { var zeroValue string r := &Repository{DefaultBranch: &zeroValue} diff --git a/github/repos.go b/github/repos.go index 630ce748c94..59ef2329f07 100644 --- a/github/repos.go +++ b/github/repos.go @@ -27,59 +27,59 @@ type RepositoriesService service // Repository represents a GitHub repository. type Repository struct { - ID *int64 `json:"id,omitempty"` - NodeID *string `json:"node_id,omitempty"` - Owner *User `json:"owner,omitempty"` - Name *string `json:"name,omitempty"` - FullName *string `json:"full_name,omitempty"` - Description *string `json:"description,omitempty"` - Homepage *string `json:"homepage,omitempty"` - CodeOfConduct *CodeOfConduct `json:"code_of_conduct,omitempty"` - DefaultBranch *string `json:"default_branch,omitempty"` - MasterBranch *string `json:"master_branch,omitempty"` - CreatedAt *Timestamp `json:"created_at,omitempty"` - PushedAt *Timestamp `json:"pushed_at,omitempty"` - UpdatedAt *Timestamp `json:"updated_at,omitempty"` - HTMLURL *string `json:"html_url,omitempty"` - CloneURL *string `json:"clone_url,omitempty"` - GitURL *string `json:"git_url,omitempty"` - MirrorURL *string `json:"mirror_url,omitempty"` - SSHURL *string `json:"ssh_url,omitempty"` - SVNURL *string `json:"svn_url,omitempty"` - Language *string `json:"language,omitempty"` - Fork *bool `json:"fork,omitempty"` - ForksCount *int `json:"forks_count,omitempty"` - NetworkCount *int `json:"network_count,omitempty"` - OpenIssuesCount *int `json:"open_issues_count,omitempty"` - OpenIssues *int `json:"open_issues,omitempty"` // Deprecated: Replaced by OpenIssuesCount. For backward compatibility OpenIssues is still populated. - StargazersCount *int `json:"stargazers_count,omitempty"` - SubscribersCount *int `json:"subscribers_count,omitempty"` - WatchersCount *int `json:"watchers_count,omitempty"` // Deprecated: Replaced by StargazersCount. For backward compatibility WatchersCount is still populated. - Watchers *int `json:"watchers,omitempty"` // Deprecated: Replaced by StargazersCount. For backward compatibility Watchers is still populated. - Size *int `json:"size,omitempty"` - AutoInit *bool `json:"auto_init,omitempty"` - Parent *Repository `json:"parent,omitempty"` - Source *Repository `json:"source,omitempty"` - TemplateRepository *Repository `json:"template_repository,omitempty"` - Organization *Organization `json:"organization,omitempty"` - Permissions map[string]bool `json:"permissions,omitempty"` - AllowRebaseMerge *bool `json:"allow_rebase_merge,omitempty"` - AllowUpdateBranch *bool `json:"allow_update_branch,omitempty"` - AllowSquashMerge *bool `json:"allow_squash_merge,omitempty"` - AllowMergeCommit *bool `json:"allow_merge_commit,omitempty"` - AllowAutoMerge *bool `json:"allow_auto_merge,omitempty"` - AllowForking *bool `json:"allow_forking,omitempty"` - WebCommitSignoffRequired *bool `json:"web_commit_signoff_required,omitempty"` - DeleteBranchOnMerge *bool `json:"delete_branch_on_merge,omitempty"` - UseSquashPRTitleAsDefault *bool `json:"use_squash_pr_title_as_default,omitempty"` - SquashMergeCommitTitle *string `json:"squash_merge_commit_title,omitempty"` // Can be one of: "PR_TITLE", "COMMIT_OR_PR_TITLE" - SquashMergeCommitMessage *string `json:"squash_merge_commit_message,omitempty"` // Can be one of: "PR_BODY", "COMMIT_MESSAGES", "BLANK" - MergeCommitTitle *string `json:"merge_commit_title,omitempty"` // Can be one of: "PR_TITLE", "MERGE_MESSAGE" - MergeCommitMessage *string `json:"merge_commit_message,omitempty"` // Can be one of: "PR_BODY", "PR_TITLE", "BLANK" - Topics []string `json:"topics,omitempty"` - CustomProperties map[string]string `json:"custom_properties,omitempty"` - Archived *bool `json:"archived,omitempty"` - Disabled *bool `json:"disabled,omitempty"` + ID *int64 `json:"id,omitempty"` + NodeID *string `json:"node_id,omitempty"` + Owner *User `json:"owner,omitempty"` + Name *string `json:"name,omitempty"` + FullName *string `json:"full_name,omitempty"` + Description *string `json:"description,omitempty"` + Homepage *string `json:"homepage,omitempty"` + CodeOfConduct *CodeOfConduct `json:"code_of_conduct,omitempty"` + DefaultBranch *string `json:"default_branch,omitempty"` + MasterBranch *string `json:"master_branch,omitempty"` + CreatedAt *Timestamp `json:"created_at,omitempty"` + PushedAt *Timestamp `json:"pushed_at,omitempty"` + UpdatedAt *Timestamp `json:"updated_at,omitempty"` + HTMLURL *string `json:"html_url,omitempty"` + CloneURL *string `json:"clone_url,omitempty"` + GitURL *string `json:"git_url,omitempty"` + MirrorURL *string `json:"mirror_url,omitempty"` + SSHURL *string `json:"ssh_url,omitempty"` + SVNURL *string `json:"svn_url,omitempty"` + Language *string `json:"language,omitempty"` + Fork *bool `json:"fork,omitempty"` + ForksCount *int `json:"forks_count,omitempty"` + NetworkCount *int `json:"network_count,omitempty"` + OpenIssuesCount *int `json:"open_issues_count,omitempty"` + OpenIssues *int `json:"open_issues,omitempty"` // Deprecated: Replaced by OpenIssuesCount. For backward compatibility OpenIssues is still populated. + StargazersCount *int `json:"stargazers_count,omitempty"` + SubscribersCount *int `json:"subscribers_count,omitempty"` + WatchersCount *int `json:"watchers_count,omitempty"` // Deprecated: Replaced by StargazersCount. For backward compatibility WatchersCount is still populated. + Watchers *int `json:"watchers,omitempty"` // Deprecated: Replaced by StargazersCount. For backward compatibility Watchers is still populated. + Size *int `json:"size,omitempty"` + AutoInit *bool `json:"auto_init,omitempty"` + Parent *Repository `json:"parent,omitempty"` + Source *Repository `json:"source,omitempty"` + TemplateRepository *Repository `json:"template_repository,omitempty"` + Organization *Organization `json:"organization,omitempty"` + Permissions map[string]bool `json:"permissions,omitempty"` + AllowRebaseMerge *bool `json:"allow_rebase_merge,omitempty"` + AllowUpdateBranch *bool `json:"allow_update_branch,omitempty"` + AllowSquashMerge *bool `json:"allow_squash_merge,omitempty"` + AllowMergeCommit *bool `json:"allow_merge_commit,omitempty"` + AllowAutoMerge *bool `json:"allow_auto_merge,omitempty"` + AllowForking *bool `json:"allow_forking,omitempty"` + WebCommitSignoffRequired *bool `json:"web_commit_signoff_required,omitempty"` + DeleteBranchOnMerge *bool `json:"delete_branch_on_merge,omitempty"` + UseSquashPRTitleAsDefault *bool `json:"use_squash_pr_title_as_default,omitempty"` + SquashMergeCommitTitle *string `json:"squash_merge_commit_title,omitempty"` // Can be one of: "PR_TITLE", "COMMIT_OR_PR_TITLE" + SquashMergeCommitMessage *string `json:"squash_merge_commit_message,omitempty"` // Can be one of: "PR_BODY", "COMMIT_MESSAGES", "BLANK" + MergeCommitTitle *string `json:"merge_commit_title,omitempty"` // Can be one of: "PR_TITLE", "MERGE_MESSAGE" + MergeCommitMessage *string `json:"merge_commit_message,omitempty"` // Can be one of: "PR_BODY", "PR_TITLE", "BLANK" + Topics []string `json:"topics,omitempty"` + CustomProperties map[string]interface{} `json:"custom_properties,omitempty"` + Archived *bool `json:"archived,omitempty"` + Disabled *bool `json:"disabled,omitempty"` // Only provided when using RepositoriesService.Get while in preview License *License `json:"license,omitempty"` diff --git a/github/repos_test.go b/github/repos_test.go index e07b1b367d5..f18828067d1 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -14,6 +14,7 @@ import ( "net/url" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" ) @@ -4463,3 +4464,49 @@ func TestRepositoriesService_IsPrivateReportingEnabled(t *testing.T) { return resp, err }) } + +func TestRepository_UnmarshalJSON(t *testing.T) { + var testCases = map[string]struct { + data []byte + wantRepository Repository + wantErr bool + }{ + "Empty": { + data: []byte("{}"), + wantRepository: Repository{}, + wantErr: false, + }, + "Invalid JSON": { + data: []byte("{"), + wantRepository: Repository{}, + wantErr: true, + }, + "Partial project": { + data: []byte(`{"id":10270722,"name":"go-github","private":false,"owner":{"login":"google"},"created_at":"2013-05-24T16:42:58Z","license":{},"topics":["github"],"permissions":{"pull":true},"custom_properties":{},"organization":{"login":"google"}}`), + wantRepository: Repository{ID: Int64(10270722), Name: String("go-github"), Private: Bool(false), Owner: &User{Login: String("google")}, CreatedAt: &Timestamp{time.Date(2013, 5, 24, 16, 42, 58, 0, time.UTC)}, License: &License{}, Topics: []string{"github"}, Permissions: map[string]bool{"pull": true}, CustomProperties: map[string]interface{}{}, Organization: &Organization{Login: String("google")}}, + wantErr: false, + }, + "With custom properties": { + data: []byte(`{"custom_properties":{"boolean":"false","text":"a","single-select":"a","multi-select":["a","b","c"]}}`), + wantRepository: Repository{CustomProperties: map[string]interface{}{"boolean": "false", "text": "a", "single-select": "a", "multi-select": []interface{}{"a", "b", "c"}}}, + wantErr: false, + }, + } + + for name, tt := range testCases { + tt := tt + t.Run(name, func(t *testing.T) { + pk := Repository{} + err := json.Unmarshal(tt.data, &pk) + if err == nil && tt.wantErr { + t.Errorf("Repository.UnmarshalJSON returned nil instead of an error") + } + if err != nil && !tt.wantErr { + t.Errorf("Repository.UnmarshalJSON returned an unexpected error: %+v", err) + } + if !cmp.Equal(tt.wantRepository, pk) { + t.Errorf("Repository.UnmarshalJSON expected repository %+v, got %+v", tt.wantRepository, pk) + } + }) + } +}