From a47a1e0777dea46af766f54b643384ee25e1ebcc Mon Sep 17 00:00:00 2001 From: oliverpool Date: Wed, 13 Mar 2024 09:26:56 +0100 Subject: [PATCH] [BUG] Packagist webhook: support all events Fixes #2329 --- services/webhook/packagist.go | 89 ++--------- services/webhook/packagist_test.go | 235 ++++++----------------------- services/webhook/payloader.go | 3 + 3 files changed, 59 insertions(+), 268 deletions(-) diff --git a/services/webhook/packagist.go b/services/webhook/packagist.go index 7880d8b606..0a2b62c89a 100644 --- a/services/webhook/packagist.go +++ b/services/webhook/packagist.go @@ -11,12 +11,11 @@ import ( webhook_model "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" - api "code.gitea.io/gitea/modules/structs" - webhook_module "code.gitea.io/gitea/modules/webhook" ) type ( - // PackagistPayload represents + // PackagistPayload represents a packagist payload + // as expected by https://packagist.org/about PackagistPayload struct { PackagistRepository struct { URL string `json:"url"` @@ -40,85 +39,19 @@ func GetPackagistHook(w *webhook_model.Webhook) *PackagistMeta { return s } -// Create implements PayloadConvertor Create method -func (pc packagistConvertor) Create(_ *api.CreatePayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// Delete implements PayloadConvertor Delete method -func (pc packagistConvertor) Delete(_ *api.DeletePayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// Fork implements PayloadConvertor Fork method -func (pc packagistConvertor) Fork(_ *api.ForkPayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// Push implements PayloadConvertor Push method -// https://packagist.org/about -func (pc packagistConvertor) Push(_ *api.PushPayload) (PackagistPayload, error) { - return PackagistPayload{ - PackagistRepository: struct { - URL string `json:"url"` - }{ - URL: pc.PackageURL, - }, - }, nil -} - -// Issue implements PayloadConvertor Issue method -func (pc packagistConvertor) Issue(_ *api.IssuePayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// IssueComment implements PayloadConvertor IssueComment method -func (pc packagistConvertor) IssueComment(_ *api.IssueCommentPayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// PullRequest implements PayloadConvertor PullRequest method -func (pc packagistConvertor) PullRequest(_ *api.PullRequestPayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// Review implements PayloadConvertor Review method -func (pc packagistConvertor) Review(_ *api.PullRequestPayload, _ webhook_module.HookEventType) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// Repository implements PayloadConvertor Repository method -func (pc packagistConvertor) Repository(_ *api.RepositoryPayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// Wiki implements PayloadConvertor Wiki method -func (pc packagistConvertor) Wiki(_ *api.WikiPayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -// Release implements PayloadConvertor Release method -func (pc packagistConvertor) Release(_ *api.ReleasePayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -func (pc packagistConvertor) Package(_ *api.PackagePayload) (PackagistPayload, error) { - return PackagistPayload{}, nil -} - -type packagistConvertor struct { - PackageURL string -} - -var _ payloadConvertor[PackagistPayload] = packagistConvertor{} - +// newPackagistRequest creates a request with the [PackagistPayload] for packagist (same payload for all events). func newPackagistRequest(ctx context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { meta := &PackagistMeta{} if err := json.Unmarshal([]byte(w.Meta), meta); err != nil { return nil, nil, fmt.Errorf("newpackagistRequest meta json: %w", err) } - pc := packagistConvertor{ - PackageURL: meta.PackageURL, + + payload := PackagistPayload{ + PackagistRepository: struct { + URL string `json:"url"` + }{ + URL: meta.PackageURL, + }, } - return newJSONRequest(pc, w, t, true) + return newJSONRequestWithPayload(payload, w, t, false) } diff --git a/services/webhook/packagist_test.go b/services/webhook/packagist_test.go index e9b0695baa..e6a963a9dd 100644 --- a/services/webhook/packagist_test.go +++ b/services/webhook/packagist_test.go @@ -5,6 +5,7 @@ package webhook import ( "context" + "fmt" "testing" webhook_model "code.gitea.io/gitea/models/webhook" @@ -17,199 +18,53 @@ import ( ) func TestPackagistPayload(t *testing.T) { - pc := packagistConvertor{ - PackageURL: "https://packagist.org/packages/example", + payloads := []api.Payloader{ + createTestPayload(), + deleteTestPayload(), + forkTestPayload(), + pushTestPayload(), + issueTestPayload(), + issueCommentTestPayload(), + pullRequestCommentTestPayload(), + pullRequestTestPayload(), + repositoryTestPayload(), + packageTestPayload(), + wikiTestPayload(), + pullReleaseTestPayload(), } - t.Run("Create", func(t *testing.T) { - p := createTestPayload() - pl, err := pc.Create(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) + for _, payloader := range payloads { + t.Run(fmt.Sprintf("%T", payloader), func(t *testing.T) { + data, err := payloader.JSONPayload() + require.NoError(t, err) - t.Run("Delete", func(t *testing.T) { - p := deleteTestPayload() + hook := &webhook_model.Webhook{ + RepoID: 3, + IsActive: true, + Type: webhook_module.PACKAGIST, + URL: "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", + Meta: `{"package_url":"https://packagist.org/packages/example"}`, + HTTPMethod: "POST", + } + task := &webhook_model.HookTask{ + HookID: hook.ID, + EventType: webhook_module.HookEventPush, + PayloadContent: string(data), + PayloadVersion: 2, + } - pl, err := pc.Delete(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) + req, reqBody, err := newPackagistRequest(context.Background(), hook, task) + require.NotNil(t, req) + require.NotNil(t, reqBody) + require.NoError(t, err) - t.Run("Fork", func(t *testing.T) { - p := forkTestPayload() - - pl, err := pc.Fork(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("Push", func(t *testing.T) { - p := pushTestPayload() - - pl, err := pc.Push(p) - require.NoError(t, err) - - assert.Equal(t, "https://packagist.org/packages/example", pl.PackagistRepository.URL) - }) - - t.Run("Issue", func(t *testing.T) { - p := issueTestPayload() - - p.Action = api.HookIssueOpened - pl, err := pc.Issue(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - - p.Action = api.HookIssueClosed - pl, err = pc.Issue(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("IssueComment", func(t *testing.T) { - p := issueCommentTestPayload() - - pl, err := pc.IssueComment(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("PullRequest", func(t *testing.T) { - p := pullRequestTestPayload() - - pl, err := pc.PullRequest(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("PullRequestComment", func(t *testing.T) { - p := pullRequestCommentTestPayload() - - pl, err := pc.IssueComment(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("Review", func(t *testing.T) { - p := pullRequestTestPayload() - p.Action = api.HookIssueReviewed - - pl, err := pc.Review(p, webhook_module.HookEventPullRequestReviewApproved) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("Repository", func(t *testing.T) { - p := repositoryTestPayload() - - pl, err := pc.Repository(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("Package", func(t *testing.T) { - p := packageTestPayload() - - pl, err := pc.Package(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("Wiki", func(t *testing.T) { - p := wikiTestPayload() - - p.Action = api.HookWikiCreated - pl, err := pc.Wiki(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - - p.Action = api.HookWikiEdited - pl, err = pc.Wiki(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - - p.Action = api.HookWikiDeleted - pl, err = pc.Wiki(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) - - t.Run("Release", func(t *testing.T) { - p := pullReleaseTestPayload() - - pl, err := pc.Release(p) - require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) - }) -} - -func TestPackagistJSONPayload(t *testing.T) { - p := pushTestPayload() - data, err := p.JSONPayload() - require.NoError(t, err) - - hook := &webhook_model.Webhook{ - RepoID: 3, - IsActive: true, - Type: webhook_module.PACKAGIST, - URL: "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", - Meta: `{"package_url":"https://packagist.org/packages/example"}`, - HTTPMethod: "POST", - } - task := &webhook_model.HookTask{ - HookID: hook.ID, - EventType: webhook_module.HookEventPush, - PayloadContent: string(data), - PayloadVersion: 2, - } - - req, reqBody, err := newPackagistRequest(context.Background(), hook, task) - require.NotNil(t, req) - require.NotNil(t, reqBody) - require.NoError(t, err) - - assert.Equal(t, "POST", req.Method) - assert.Equal(t, "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", req.URL.String()) - assert.Equal(t, "sha256=", req.Header.Get("X-Hub-Signature-256")) - assert.Equal(t, "application/json", req.Header.Get("Content-Type")) - var body PackagistPayload - err = json.NewDecoder(req.Body).Decode(&body) - assert.NoError(t, err) - assert.Equal(t, "https://packagist.org/packages/example", body.PackagistRepository.URL) -} - -func TestPackagistEmptyPayload(t *testing.T) { - p := createTestPayload() - data, err := p.JSONPayload() - require.NoError(t, err) - - hook := &webhook_model.Webhook{ - RepoID: 3, - IsActive: true, - Type: webhook_module.PACKAGIST, - URL: "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", - Meta: `{"package_url":"https://packagist.org/packages/example"}`, - HTTPMethod: "POST", - } - task := &webhook_model.HookTask{ - HookID: hook.ID, - EventType: webhook_module.HookEventCreate, - PayloadContent: string(data), - PayloadVersion: 2, - } - - req, reqBody, err := newPackagistRequest(context.Background(), hook, task) - require.NotNil(t, req) - require.NotNil(t, reqBody) - require.NoError(t, err) - - assert.Equal(t, "POST", req.Method) - assert.Equal(t, "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", req.URL.String()) - assert.Equal(t, "sha256=", req.Header.Get("X-Hub-Signature-256")) - assert.Equal(t, "application/json", req.Header.Get("Content-Type")) - var body PackagistPayload - err = json.NewDecoder(req.Body).Decode(&body) - assert.NoError(t, err) - assert.Equal(t, "", body.PackagistRepository.URL) + assert.Equal(t, "POST", req.Method) + assert.Equal(t, "https://packagist.org/api/update-package?username=THEUSERNAME&apiToken=TOPSECRETAPITOKEN", req.URL.String()) + assert.Equal(t, "application/json", req.Header.Get("Content-Type")) + var body PackagistPayload + err = json.NewDecoder(req.Body).Decode(&body) + assert.NoError(t, err) + assert.Equal(t, "https://packagist.org/packages/example", body.PackagistRepository.URL) + }) + } } diff --git a/services/webhook/payloader.go b/services/webhook/payloader.go index 54a11a5868..f87e6e4eec 100644 --- a/services/webhook/payloader.go +++ b/services/webhook/payloader.go @@ -88,7 +88,10 @@ func newJSONRequest[T any](pc payloadConvertor[T], w *webhook_model.Webhook, t * if err != nil { return nil, nil, err } + return newJSONRequestWithPayload(payload, w, t, withDefaultHeaders) +} +func newJSONRequestWithPayload(payload any, w *webhook_model.Webhook, t *webhook_model.HookTask, withDefaultHeaders bool) (*http.Request, []byte, error) { body, err := json.MarshalIndent(payload, "", " ") if err != nil { return nil, nil, err