e52b1ecd2c
A retry after a failed attachment or a failed CompleteJob created another task for the same report. The task id is now stored as soon as the task is created, a retry with a stored id only re-attempts the attachment, a sent submission with a task id is skipped, and a CompleteJob failure is logged instead of returned.
406 lines
16 KiB
Go
406 lines
16 KiB
Go
package classes_test
|
||
|
||
import (
|
||
"bytes"
|
||
"context"
|
||
"encoding/hex"
|
||
"encoding/json"
|
||
"errors"
|
||
"image"
|
||
"image/color"
|
||
"image/gif"
|
||
"image/jpeg"
|
||
"image/png"
|
||
"io"
|
||
"net/http"
|
||
"os"
|
||
"path/filepath"
|
||
"strings"
|
||
"testing"
|
||
|
||
// The plugin registers itself; pgtest activates it.
|
||
_ "git.golem15.com/golem15/sm-feedback-plugin"
|
||
"git.golem15.com/golem15/sm-feedback-plugin/classes"
|
||
"git.golem15.com/golem15/sm-feedback-plugin/internal/pgtest"
|
||
"git.golem15.com/golem15/sm-feedback-plugin/models"
|
||
"git.golem15.com/golem15/summercms/modules/backpack"
|
||
"git.golem15.com/golem15/summercms/modules/compass"
|
||
"git.golem15.com/golem15/summercms/modules/fetchguard"
|
||
"git.golem15.com/golem15/summercms/modules/lagoon"
|
||
"git.golem15.com/golem15/summercms/modules/lagoon/attach"
|
||
"git.golem15.com/golem15/summercms/modules/tide"
|
||
"gocloud.dev/blob"
|
||
_ "gocloud.dev/blob/memblob"
|
||
"gorm.io/gorm"
|
||
)
|
||
|
||
func mustHex(t *testing.T, s string) []byte {
|
||
t.Helper()
|
||
b, err := hex.DecodeString(s)
|
||
if err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
return b
|
||
}
|
||
|
||
func encoded(t *testing.T, enc func(*bytes.Buffer, image.Image) error) []byte {
|
||
t.Helper()
|
||
img := image.NewRGBA(image.Rect(0, 0, 3, 2))
|
||
img.Set(1, 1, color.RGBA{R: 200, A: 255})
|
||
var buf bytes.Buffer
|
||
if err := enc(&buf, img); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
return buf.Bytes()
|
||
}
|
||
|
||
// TestImageGuardCopy pins the feedback copy of ImageContentGuard: the
|
||
// shared vectors (the PHP-recorded screenshot fixtures among them) get the
|
||
// verdicts PHP gave and the framework's own guard (attach.IsAllowedImage)
|
||
// gives, and the sniffed type of each accepted image is one of the four.
|
||
func TestImageGuardCopy(t *testing.T) {
|
||
cases := []struct {
|
||
name string
|
||
data []byte
|
||
want bool
|
||
}{
|
||
{"png", encoded(t, func(b *bytes.Buffer, i image.Image) error { return png.Encode(b, i) }), true},
|
||
{"jpeg", encoded(t, func(b *bytes.Buffer, i image.Image) error { return jpeg.Encode(b, i, nil) }), true},
|
||
{"gif", encoded(t, func(b *bytes.Buffer, i image.Image) error { return gif.Encode(b, i, nil) }), true},
|
||
// The parity fixtures: a PNG and a WebP PHP accepted, and a JPEG
|
||
// header without an image PHP refused ("The file is not a valid
|
||
// image.").
|
||
{"fixture-png", mustHex(t, "89504e470d0a1a0a0000000d49484452000000080000000808020000004b6d29dc0000001949444154789c626160a8906360c0442c20021b189c128001005509023d9970f43c0000000049454e44ae426082"), true},
|
||
{"fixture-webp", mustHex(t, "524946464800000057454250565038203c0000005003009d012a180010003e6d2c9245a422a19804004006c4a009d3284600fc000108a000feec35853fff497f283d1d7ff3f3396879687ee0b7790000"), true},
|
||
{"fixture-spoofed-jpeg", mustHex(t, "ffd8ffe000104a46494600010100000100010000706f6c79676c6f743a2061204a5045472068656164657220776974686f757420616e7920696d616765ffd9"), false},
|
||
{"text", []byte("plain text, not a picture"), false},
|
||
{"empty", nil, false},
|
||
{"svg", []byte(`<svg xmlns="http://www.w3.org/2000/svg" width="1" height="1"></svg>`), false},
|
||
{"bmp", append([]byte("BM"), make([]byte, 60)...), false},
|
||
{"png-header-only", mustHex(t, "89504e470d0a1a0a"), false},
|
||
}
|
||
for _, c := range cases {
|
||
got := classes.IsAllowedImage(c.data)
|
||
if got != c.want {
|
||
t.Errorf("%s: IsAllowedImage = %v, want %v", c.name, got, c.want)
|
||
}
|
||
if fw := attach.IsAllowedImage(c.data); fw != got {
|
||
t.Errorf("%s: the framework guard says %v, the copy %v", c.name, fw, got)
|
||
}
|
||
if got {
|
||
switch classes.SniffImageMIME(c.data) {
|
||
case "image/png", "image/jpeg", "image/gif", "image/webp":
|
||
default:
|
||
t.Errorf("%s: sniffed %q", c.name, classes.SniffImageMIME(c.data))
|
||
}
|
||
}
|
||
}
|
||
}
|
||
|
||
// roundTrip adapts a func to http.RoundTripper.
|
||
type roundTrip func(*http.Request) (*http.Response, error)
|
||
|
||
func (f roundTrip) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
|
||
|
||
func jsonAnswer(status int, body string) roundTrip {
|
||
return func(r *http.Request) (*http.Response, error) {
|
||
if r.Body != nil {
|
||
_, _ = io.Copy(io.Discard, r.Body)
|
||
}
|
||
return &http.Response{
|
||
StatusCode: status,
|
||
Header: http.Header{"Content-Type": {"application/json"}},
|
||
Body: io.NopCloser(strings.NewReader(body)),
|
||
Request: r,
|
||
}, nil
|
||
}
|
||
}
|
||
|
||
// syncEnv is an app over the shared test database with a memory uploads
|
||
// bucket, for StoreSubmission and SyncG15Office.
|
||
func syncEnv(t *testing.T) (*backpack.App, *gorm.DB, *blob.Bucket) {
|
||
t.Helper()
|
||
gdb := pgtest.DB(t)
|
||
dir := t.TempDir()
|
||
if err := os.WriteFile(filepath.Join(dir, "app.yaml"), []byte("name: feedback-classes\n"), 0o644); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
cfg, err := compass.Open(compass.Options{Dir: dir, Environ: []string{"SUMMER_ENV=development"}})
|
||
if err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
app := backpack.New(cfg)
|
||
if err := lagoon.Publish(app, pgtest.SQL(t), gdb); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
bucket, err := blob.OpenBucket(t.Context(), "mem://")
|
||
if err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
t.Cleanup(func() { _ = bucket.Close() })
|
||
return app, gdb, bucket
|
||
}
|
||
|
||
// recordedSubmission stores the submission PHP filed when the sidecar was
|
||
// recorded (the same fields and screenshot) and returns it.
|
||
func recordedSubmission(t *testing.T, app *backpack.App, gdb *gorm.DB, bucket *blob.Bucket) *models.Submission {
|
||
t.Helper()
|
||
str := func(s string) *string { return &s }
|
||
sub := &models.Submission{
|
||
Message: "Okładka albumu\tjest zła.\n\nDruga linia z dłuższym opisem problemu, który na pewno przekracza osiemdziesiąt znaków tytułu zadania.",
|
||
Type: "feature",
|
||
Email: str("reporter@parity.test"),
|
||
PageURL: "https://app.parity.test/albums/7?tab=photos",
|
||
UserAgent: str("Mozilla/5.0 (X11; Linux x86_64) Parity/1.0"),
|
||
ConsoleLog: str("[log] " + strings.Repeat("żółć ąę ", 520)),
|
||
}
|
||
shot := mustHex(t, "89504e470d0a1a0a0000000d49484452000000080000000808020000004b6d29dc0000001949444154789c626160a8906360c0442c20021b189c128001005509023d9970f43c0000000049454e44ae426082")
|
||
if err := classes.StoreSubmission(t.Context(), app, gdb, bucket, sub, &classes.Screenshot{Filename: "screenshot.png", Data: shot}); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
return sub
|
||
}
|
||
|
||
func reload(t *testing.T, gdb *gorm.DB, id uint) models.Submission {
|
||
t.Helper()
|
||
var s models.Submission
|
||
if err := gdb.Take(&s, id).Error; err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
return s
|
||
}
|
||
|
||
func defaultSettings(t *testing.T, gdb *gorm.DB) {
|
||
t.Helper()
|
||
if err := gdb.Exec(`DELETE FROM golem15_feedback_settings`).Error; err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
// Background: Cleanup runs it after the test's context is cancelled.
|
||
if _, err := models.Instance(context.Background(), gdb); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
}
|
||
|
||
const parityToken = "parityG15OfficeTokenNotReal01"
|
||
|
||
func parityClient() *classes.G15OfficeClient {
|
||
return classes.NewG15OfficeClientWith("https://office.parity.test/", parityToken, "parity-project", nil)
|
||
}
|
||
|
||
// TestSyncG15Office replays the G15Office exchange PHP's
|
||
// SyncFeedbackToG15Office made (testdata/g15office-task.upstream.yaml,
|
||
// recorded through the upstream proxy): the task JSON, the Bearer token,
|
||
// Accept: application/json, no User-Agent, the screenshot as the multipart
|
||
// part "file" with its name, type and sha256. The submission ends as PHP's
|
||
// did (sent, task tskParity01). The failure paths store status failed and
|
||
// PHP's message and return the error so the job is retried; an unconfigured
|
||
// client sends nothing.
|
||
func TestSyncG15Office(t *testing.T) {
|
||
app, gdb, bucket := syncEnv(t)
|
||
defaultSettings(t, gdb)
|
||
|
||
t.Run("php-recorded", func(t *testing.T) {
|
||
sub := recordedSubmission(t, app, gdb, bucket)
|
||
sidecar, err := tide.LoadUpstream(filepath.Join("testdata", "g15office-task.upstream.yaml"))
|
||
if err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
store, err := tide.OpenStore("")
|
||
if err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
store.Set("secret:g15office-token", parityToken)
|
||
fake := tide.NewUpstreamFake(sidecar, store)
|
||
ctx := fetchguard.WithTransport(t.Context(), fake)
|
||
if err := classes.SyncG15Office(ctx, classes.SyncDeps{DB: gdb, Bucket: bucket, Client: parityClient()}, classes.SyncG15OfficeArgs{SubmissionID: sub.ID}); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
if err := fake.Verify(); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
got := reload(t, gdb, sub.ID)
|
||
if got.Status != models.StatusSent || got.G15OfficeTaskID == nil || *got.G15OfficeTaskID != "tskParity01" || got.G15OfficeError != nil {
|
||
t.Fatalf("submission %+v", got)
|
||
}
|
||
})
|
||
|
||
failing := func(t *testing.T, client *classes.G15OfficeClient, rt http.RoundTripper, wantErr string) {
|
||
t.Helper()
|
||
sub := recordedSubmission(t, app, gdb, bucket)
|
||
ctx := t.Context()
|
||
if rt != nil {
|
||
ctx = fetchguard.WithTransport(ctx, rt)
|
||
}
|
||
err := classes.SyncG15Office(ctx, classes.SyncDeps{DB: gdb, Bucket: bucket, Client: client}, classes.SyncG15OfficeArgs{SubmissionID: sub.ID})
|
||
if err == nil || err.Error() != wantErr {
|
||
t.Fatalf("error %v, want %q", err, wantErr)
|
||
}
|
||
got := reload(t, gdb, sub.ID)
|
||
if got.Status != models.StatusFailed || got.G15OfficeError == nil || *got.G15OfficeError != wantErr || got.G15OfficeTaskID != nil {
|
||
t.Fatalf("submission %+v", got)
|
||
}
|
||
}
|
||
never := roundTrip(func(r *http.Request) (*http.Response, error) {
|
||
t.Errorf("unexpected request to %s", r.URL)
|
||
return nil, errors.New("no request expected")
|
||
})
|
||
|
||
t.Run("unconfigured-sends-nothing", func(t *testing.T) {
|
||
for _, c := range []*classes.G15OfficeClient{
|
||
classes.NewG15OfficeClient(nil),
|
||
classes.NewG15OfficeClientWith("https://office.parity.test", "", "parity-project", nil),
|
||
classes.NewG15OfficeClientWith("https://office.parity.test", parityToken, "", nil),
|
||
classes.NewG15OfficeClientWith("", parityToken, "parity-project", nil),
|
||
} {
|
||
if c.IsConfigured() {
|
||
t.Fatal("a partial configuration counts as configured")
|
||
}
|
||
failing(t, c, never, "g15office is not configured (G15_OFFICE_BASE_URL / G15_OFFICE_TOKEN / G15_OFFICE_PROJECT).")
|
||
}
|
||
})
|
||
t.Run("error-envelope", func(t *testing.T) {
|
||
failing(t, parityClient(), jsonAnswer(404, `{"error":true,"message":"Project not found"}`), "g15office create task failed: Project not found")
|
||
failing(t, parityClient(), jsonAnswer(500, `{"error":1}`), "g15office create task failed: unknown error")
|
||
})
|
||
t.Run("missing-task-id", func(t *testing.T) {
|
||
failing(t, parityClient(), jsonAnswer(201, `{"success":true,"data":{"number":7}}`), "g15office did not return a task id.")
|
||
failing(t, parityClient(), jsonAnswer(201, `{"data":{"id":""}}`), "g15office did not return a task id.")
|
||
})
|
||
t.Run("invalid-body", func(t *testing.T) {
|
||
failing(t, parityClient(), jsonAnswer(502, `<html>Bad gateway</html>`), "g15office create task failed: invalid response body.")
|
||
failing(t, parityClient(), jsonAnswer(200, `"ok"`), "g15office create task failed: invalid response body.")
|
||
})
|
||
t.Run("connection-error", func(t *testing.T) {
|
||
failing(t, parityClient(), roundTrip(func(*http.Request) (*http.Response, error) { return nil, errors.New("dial failed") }), "g15office create task failed: connection error.")
|
||
})
|
||
t.Run("attach-failure", func(t *testing.T) {
|
||
// The task id is kept from the first attempt, so the retry only
|
||
// re-attempts the attachment and files no second task (WR-06).
|
||
tasks, attaches := 0, 0
|
||
attachStatus := 413
|
||
rt := roundTrip(func(r *http.Request) (*http.Response, error) {
|
||
if strings.HasSuffix(r.URL.Path, "/tasks") {
|
||
tasks++
|
||
return jsonAnswer(201, `{"data":{"id":"tskX"}}`)(r)
|
||
}
|
||
attaches++
|
||
if !strings.Contains(r.URL.Path, "/tasks/tskX/") {
|
||
t.Errorf("attachment to %s", r.URL.Path)
|
||
}
|
||
if attachStatus != 201 {
|
||
return jsonAnswer(attachStatus, `{"error":true,"message":"File too large"}`)(r)
|
||
}
|
||
return jsonAnswer(201, `{"data":{"id":"att"}}`)(r)
|
||
})
|
||
sub := recordedSubmission(t, app, gdb, bucket)
|
||
deps := classes.SyncDeps{DB: gdb, Bucket: bucket, Client: parityClient()}
|
||
ctx := fetchguard.WithTransport(t.Context(), rt)
|
||
const wantErr = "g15office attach file failed: File too large"
|
||
if err := classes.SyncG15Office(ctx, deps, classes.SyncG15OfficeArgs{SubmissionID: sub.ID}); err == nil || err.Error() != wantErr {
|
||
t.Fatalf("error %v, want %q", err, wantErr)
|
||
}
|
||
got := reload(t, gdb, sub.ID)
|
||
if got.Status != models.StatusFailed || got.G15OfficeError == nil || *got.G15OfficeError != wantErr || got.G15OfficeTaskID == nil || *got.G15OfficeTaskID != "tskX" {
|
||
t.Fatalf("submission %+v", got)
|
||
}
|
||
attachStatus = 201
|
||
if err := classes.SyncG15Office(ctx, deps, classes.SyncG15OfficeArgs{SubmissionID: sub.ID}); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
got = reload(t, gdb, sub.ID)
|
||
if got.Status != models.StatusSent || got.G15OfficeError != nil || got.G15OfficeTaskID == nil || *got.G15OfficeTaskID != "tskX" {
|
||
t.Fatalf("retried submission %+v", got)
|
||
}
|
||
// A sent submission with its task is never filed again.
|
||
if err := classes.SyncG15Office(ctx, deps, classes.SyncG15OfficeArgs{SubmissionID: sub.ID}); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
if tasks != 1 || attaches != 2 {
|
||
t.Fatalf("%d task and %d attachment requests", tasks, attaches)
|
||
}
|
||
})
|
||
t.Run("http-base-url-is-refused", func(t *testing.T) {
|
||
failing(t, classes.NewG15OfficeClientWith("http://office.parity.test", parityToken, "parity-project", nil), never, "g15office create task failed: connection error.")
|
||
})
|
||
t.Run("missing-submission", func(t *testing.T) {
|
||
if err := classes.SyncG15Office(fetchguard.WithTransport(t.Context(), never), classes.SyncDeps{DB: gdb, Bucket: bucket, Client: parityClient()}, classes.SyncG15OfficeArgs{SubmissionID: 999999999}); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
})
|
||
t.Run("settings-status-and-priority", func(t *testing.T) {
|
||
status, priority := "Nowe", "urgent"
|
||
if err := gdb.Model(&models.Settings{}).Where("id = ?", models.SettingsID).Updates(map[string]any{
|
||
"g15office_task_status": status, "g15office_task_priority": priority,
|
||
}).Error; err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
t.Cleanup(func() { defaultSettings(t, gdb) })
|
||
var body map[string]any
|
||
rt := roundTrip(func(r *http.Request) (*http.Response, error) {
|
||
if strings.HasSuffix(r.URL.Path, "/tasks") {
|
||
raw, _ := io.ReadAll(r.Body)
|
||
if err := json.Unmarshal(raw, &body); err != nil {
|
||
t.Errorf("task body %s", raw)
|
||
}
|
||
return jsonAnswer(201, `{"data":{"id":"tskY"}}`)(r)
|
||
}
|
||
return jsonAnswer(201, `{"data":{"id":"att"}}`)(r)
|
||
})
|
||
sub := recordedSubmission(t, app, gdb, bucket)
|
||
if err := classes.SyncG15Office(fetchguard.WithTransport(t.Context(), rt), classes.SyncDeps{DB: gdb, Bucket: bucket, Client: parityClient()}, classes.SyncG15OfficeArgs{SubmissionID: sub.ID}); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
if body["status"] != status || body["priority"] != priority || body["type"] != "Feature" {
|
||
t.Fatalf("task body %v", body)
|
||
}
|
||
})
|
||
}
|
||
|
||
// TestTaskText pins buildTitle and buildDescription: whitespace collapsed
|
||
// as PCRE's \s (vertical tab included, no-break space not), Str::limit by
|
||
// display width (wide characters count two columns), and the optional
|
||
// metadata lines skipped for "" and "0" as PHP's truthiness does.
|
||
func TestTaskText(t *testing.T) {
|
||
limits := []struct{ in, want string }{
|
||
{"abc", "abc"},
|
||
{strings.Repeat("a", 80), strings.Repeat("a", 80)},
|
||
{strings.Repeat("a", 81), strings.Repeat("a", 80) + "..."},
|
||
{strings.Repeat("中", 41), strings.Repeat("中", 40) + "..."},
|
||
{"a" + strings.Repeat("中", 40), "a" + strings.Repeat("中", 39) + "..."},
|
||
{strings.Repeat("a", 79) + " b", strings.Repeat("a", 79) + "..."},
|
||
{strings.Repeat("ą", 81), strings.Repeat("ą", 80) + "..."},
|
||
}
|
||
for _, c := range limits {
|
||
if got := classes.StrLimit(c.in, 80, "..."); got != c.want {
|
||
t.Errorf("StrLimit(%q) = %q", c.in, got)
|
||
}
|
||
}
|
||
if got := classes.TaskTitle(" a\x0b\x0c b c \n"); got != "Feedback: a b c" {
|
||
t.Errorf("title %q", got)
|
||
}
|
||
if w := classes.StrWidth("ą中😀ｱＡ"); w != 1+2+2+1+2 {
|
||
t.Errorf("width %d", w)
|
||
}
|
||
zero, empty := "0", ""
|
||
sub := &models.Submission{Message: "Hi", PageURL: "https://example.test/", Email: &zero, UserAgent: &empty, ConsoleLog: &zero}
|
||
if got := classes.TaskDescription(sub); got != "Hi\n\n---\n\n- **Page:** https://example.test/" {
|
||
t.Errorf("description %q", got)
|
||
}
|
||
email, ua, log := "a@example.test", "UA", "line1\nline2"
|
||
sub = &models.Submission{Message: "Hi", PageURL: "/p", Email: &email, UserAgent: &ua, ConsoleLog: &log}
|
||
want := "Hi\n\n---\n\n- **Page:** /p\n- **Reporter email:** a@example.test\n- **User agent:** UA\n\n**Console log:**\n```\nline1\nline2\n```"
|
||
if got := classes.TaskDescription(sub); got != want {
|
||
t.Errorf("description %q", got)
|
||
}
|
||
long := strings.Repeat("x", 4001)
|
||
sub.ConsoleLog = &long
|
||
if got := classes.TaskDescription(sub); !strings.HasSuffix(got, strings.Repeat("x", 4000)+"… (truncated)\n```") {
|
||
t.Errorf("console log not cut at 4000 columns")
|
||
}
|
||
for typ, want := range map[string]string{"bug": "Bug", "feature": "Feature", "other": "Task", "x": "Task"} {
|
||
if got := (models.Submission{Type: typ}).G15OfficeTaskType(); got != want {
|
||
t.Errorf("type %s -> %s", typ, got)
|
||
}
|
||
}
|
||
}
|