From 88b961c11569b04af42f605c7302611d3a5d70e8 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 2 Oct 2026 15:43:06 +0200 Subject: [PATCH] Keep each model's parent out of its JSON (closes #177) Every reference from a model to the record it belongs to (a target's or entrypoint's webhook, a webhook's or API key's user, an event's webhook and entrypoint, a delivery's event and target, a delivery result's delivery) is now tagged json:"-", so a preloaded model can be marshalled without the encoder recursing between parent and child. References to child records stay. Tests marshal each of the nine references set, preloaded where it matters, and check the parent's id is absent, so deleting or restoring any one tag fails a test; preloading still fills each reference. Model: opus-5-5 --- internal/database/model_apikey.go | 5 +- internal/database/model_delivery.go | 8 +- internal/database/model_delivery_result.go | 5 +- internal/database/model_entrypoint.go | 5 +- internal/database/model_event.go | 7 +- internal/database/model_parent_test.go | 126 +++++++++++++++++++++ internal/database/model_target.go | 5 +- internal/database/model_webhook.go | 5 +- 8 files changed, 150 insertions(+), 16 deletions(-) create mode 100644 internal/database/model_parent_test.go diff --git a/internal/database/model_apikey.go b/internal/database/model_apikey.go index 5e8cee7..e39b101 100644 --- a/internal/database/model_apikey.go +++ b/internal/database/model_apikey.go @@ -15,6 +15,7 @@ type APIKey struct { Description string `json:"description"` LastUsedAt *time.Time `json:"lastUsedAt,omitempty"` - // Relations - User User `json:"user,omitzero"` + // Relations. No model marshals the record it belongs to: + // User.APIKeys leads back here, and the JSON could loop. + User User `json:"-"` } diff --git a/internal/database/model_delivery.go b/internal/database/model_delivery.go index 4424756..ffaba8a 100644 --- a/internal/database/model_delivery.go +++ b/internal/database/model_delivery.go @@ -56,8 +56,10 @@ type Delivery struct { // the index. FinishedAt *time.Time `gorm:"index:idx_deliveries_status,priority:3" json:"finishedAt,omitempty"` - // Relations - Event Event `json:"event,omitzero"` - Target Target `json:"target,omitzero"` + // Relations. No model marshals the record it belongs to: + // Event.Deliveries and Target.Deliveries lead back here, and the + // JSON could loop. + Event Event `json:"-"` + Target Target `json:"-"` DeliveryResults []DeliveryResult `json:"deliveryResults,omitempty"` } diff --git a/internal/database/model_delivery_result.go b/internal/database/model_delivery_result.go index 7bf1b88..e293320 100644 --- a/internal/database/model_delivery_result.go +++ b/internal/database/model_delivery_result.go @@ -23,6 +23,7 @@ type DeliveryResult struct { Error string `json:"error,omitempty"` Duration int64 `json:"durationMs"` // Duration in milliseconds - // Relations - Delivery Delivery `json:"delivery,omitzero"` + // Relations. No model marshals the record it belongs to: + // Delivery.DeliveryResults leads back here, and the JSON could loop. + Delivery Delivery `json:"-"` } diff --git a/internal/database/model_entrypoint.go b/internal/database/model_entrypoint.go index 3021f48..ce8e42c 100644 --- a/internal/database/model_entrypoint.go +++ b/internal/database/model_entrypoint.go @@ -15,6 +15,7 @@ type Entrypoint struct { Description string `json:"description"` Active bool `gorm:"default:true" json:"active"` - // Relations - Webhook Webhook `json:"webhook,omitzero"` + // Relations. No model marshals the record it belongs to: + // Webhook.Entrypoints leads back here, and the JSON could loop. + Webhook Webhook `json:"-"` } diff --git a/internal/database/model_event.go b/internal/database/model_event.go index eba770b..8980a53 100644 --- a/internal/database/model_event.go +++ b/internal/database/model_event.go @@ -44,8 +44,9 @@ type Event struct { // kept as the record of where the copy came from either way. ResubmittedFromID *string `gorm:"type:uuid;index" json:"resubmittedFromId,omitempty"` - // Relations - Webhook Webhook `json:"webhook,omitzero"` - Entrypoint Entrypoint `json:"entrypoint,omitzero"` + // Relations. No model marshals the record it belongs to, so + // Webhook and Entrypoint are left out of the JSON. + Webhook Webhook `json:"-"` + Entrypoint Entrypoint `json:"-"` Deliveries []Delivery `json:"deliveries,omitempty"` } diff --git a/internal/database/model_parent_test.go b/internal/database/model_parent_test.go new file mode 100644 index 0000000..2c9ee9e --- /dev/null +++ b/internal/database/model_parent_test.go @@ -0,0 +1,126 @@ +package database_test + +import ( + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/webhooker/internal/database" +) + +// TestPreloadedModelsMarshalWithoutTheirParent pins that a child's +// reference to the record it belongs to is left out of the JSON, so a +// webhook and its targets cannot marshal each other in a loop, and that +// GORM still preloads that reference, since it ignores json tags. +func TestPreloadedModelsMarshalWithoutTheirParent(t *testing.T) { + t.Parallel() + + db := startedTestDB(t) + + stored := database.Webhook{ + UserID: uuid.New().String(), + Name: testWebhookName, + Entrypoints: []database.Entrypoint{{Path: uuid.New().String()}}, + Targets: []database.Target{{ + Name: "log", + Type: database.TargetTypeLog, + }}, + } + require.NoError(t, db.Create(&stored).Error) + + entrypointID := stored.Entrypoints[0].ID + targetID := stored.Targets[0].ID + + var webhook database.Webhook + + require.NoError(t, db. + Preload("Entrypoints.Webhook"). + Preload("Targets.Webhook"). + First(&webhook, "id = ?", stored.ID).Error) + + require.Len(t, webhook.Entrypoints, 1) + require.Len(t, webhook.Targets, 1) + assert.Equal(t, stored.ID, webhook.Entrypoints[0].Webhook.ID) + assert.Equal(t, stored.ID, webhook.Targets[0].Webhook.ID) + + encoded := marshalModel(t, webhook) + + assert.Contains(t, encoded, entrypointID) + assert.Contains(t, encoded, targetID) + + // Each child holds the parent's id as its webhookId, so the parent + // is looked for by its own id field. + parentIDField := `"id":"` + stored.ID + `"` + + assert.NotContains(t, marshalModel(t, webhook.Entrypoints[0]), parentIDField) + assert.NotContains(t, marshalModel(t, webhook.Targets[0]), parentIDField) + + var target database.Target + + require.NoError(t, db. + Preload("Webhook"). + First(&target, "id = ?", targetID).Error) + + assert.Equal(t, stored.ID, target.Webhook.ID) + + encoded = marshalModel(t, target) + + assert.Contains(t, encoded, stored.ID) + assert.NotContains(t, encoded, parentIDField) +} + +// TestModelsMarshalWithoutTheirParent covers the other references to a +// parent: each model is built with its parent set, and the parent's id +// must not appear in the JSON. +func TestModelsMarshalWithoutTheirParent(t *testing.T) { + t.Parallel() + + parent := database.BaseModel{ID: uuid.New().String()} + + cases := []struct { + name string + model any + }{ + { + name: "Webhook.User", + model: database.Webhook{User: database.User{BaseModel: parent}}, + }, + { + name: "APIKey.User", + model: database.APIKey{User: database.User{BaseModel: parent}}, + }, + { + name: "Delivery.Event", + model: database.Delivery{Event: database.Event{BaseModel: parent}}, + }, + { + name: "Delivery.Target", + model: database.Delivery{Target: database.Target{BaseModel: parent}}, + }, + { + name: "DeliveryResult.Delivery", + model: database.DeliveryResult{ + Delivery: database.Delivery{BaseModel: parent}, + }, + }, + { + name: "Event.Webhook", + model: database.Event{Webhook: database.Webhook{BaseModel: parent}}, + }, + { + name: "Event.Entrypoint", + model: database.Event{ + Entrypoint: database.Entrypoint{BaseModel: parent}, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + assert.NotContains(t, marshalModel(t, tc.model), parent.ID) + }) + } +} diff --git a/internal/database/model_target.go b/internal/database/model_target.go index 9c5f95f..43b5a4f 100644 --- a/internal/database/model_target.go +++ b/internal/database/model_target.go @@ -34,7 +34,8 @@ type Target struct { MaxRetries int `json:"maxRetries,omitempty"` MaxQueueSize int `json:"maxQueueSize,omitempty"` - // Relations - Webhook Webhook `json:"webhook,omitzero"` + // Relations. No model marshals the record it belongs to: + // Webhook.Targets leads back here, and the JSON could loop. + Webhook Webhook `json:"-"` Deliveries []Delivery `json:"deliveries,omitempty"` } diff --git a/internal/database/model_webhook.go b/internal/database/model_webhook.go index eedb192..896579d 100644 --- a/internal/database/model_webhook.go +++ b/internal/database/model_webhook.go @@ -66,8 +66,9 @@ type Webhook struct { // must equal DefaultRetentionDays. RetentionDays int `gorm:"default:30" json:"retentionDays"` - // Relations - User User `json:"user,omitzero"` + // Relations. No model marshals the record it belongs to: + // User.Webhooks leads back here, and the JSON could loop. + User User `json:"-"` Entrypoints []Entrypoint `json:"entrypoints,omitempty"` Targets []Target `json:"targets,omitempty"` }