[BUG] Packagist webhook: support all events

Fixes #2329
2024-11-10 12:49:52 +01:00 · 2024-03-13 09:26:56 +01:00 · 2024-03-13 09:26:56 +01:00 · a47a1e0777
commit a47a1e0777
parent 226fa396df
3 changed files with 59 additions and 268 deletions
--- 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)
 }
--- 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)
+		})
+	}
 }
--- 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