Keep each model's parent out of its JSON (closes #177)

Every reference from a model to the record it belongs to is now
json:"-", so the webhook and target models (and the other parent and
child pairs in internal/database) can no longer marshal each other in
a loop. Preloading maps associations without reading json tags, so it
still fills these references. One test preloads a webhook with its
targets and entrypoints and a target with its webhook, checks the
references are filled, and checks the JSON leaves them out; another
builds each other model with its parent set and checks the parent's
id is absent from its JSON.

Model: opus-5-5

internal/database/model_parent_test.go

@@ -48,13 +48,7 @@ func TestPreloadedModelsMarshalWithoutTheirParent(t *testing.T) {
 
 	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)
+	assert.NotContains(t, encoded, `"webhook":`)
 
 	var target database.Target
 
@@ -67,7 +61,7 @@ func TestPreloadedModelsMarshalWithoutTheirParent(t *testing.T) {
 	encoded = marshalModel(t, target)
 
 	assert.Contains(t, encoded, stored.ID)
-	assert.NotContains(t, encoded, parentIDField)
+	assert.NotContains(t, encoded, `"webhook":`)
 }
 
 // TestModelsMarshalWithoutTheirParent covers the other references to a

internal/handlers/source_management.go

@@ -1911,13 +1911,9 @@ func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
 				return false, err
 			}
 
-			// Only the active column: saving the whole row would
-			// write back the name and settings read above over an
-			// edit saved since.
-			active := !tgt.Active
+			tgt.Active = !tgt.Active
 
-			return active, h.db.DB().Model(&tgt).
-				Update("active", active).Error
+			return tgt.Active, h.db.DB().Save(&tgt).Error
 		},
 		"failed to toggle target",
 		targetActivated, targetDeactivated,

internal/handlers/target_toggle_test.go

@@ -1,66 +0,0 @@
-package handlers_test
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm"
-)
-
-// TestHandleTargetToggle_DoesNotUndoAnEdit proves that a toggle which
-// loaded the target before an edit of it was saved does not write the
-// old name and settings back over the edit. The edit is submitted from
-// a callback on the toggle's own read of the target, so it is saved
-// after that read and before the toggle writes.
-func TestHandleTargetToggle_DoesNotUndoAnEdit(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh, tgt := seedHTTPTarget(t, env, "", "")
-	require.True(t, tgt.Active)
-
-	var (
-		edited   bool
-		editCode int
-	)
-
-	require.NoError(t, env.db.DB().Callback().Query().
-		After("gorm:query").
-		Register("test:edit_after_toggle_read", func(tx *gorm.DB) {
-			// The edit reads the target too; only the toggle's read,
-			// the first, submits it.
-			if tx.Statement.Table != "targets" || edited {
-				return
-			}
-
-			edited = true
-			editCode = submitTargetEdit(
-				env, wh.ID, tgt.ID,
-				editForm(editReplacedURL, "", ""),
-			).Code
-		}),
-	)
-
-	req := postRequest(
-		"/hook/"+wh.ID+"/targets/"+tgt.ID+"/toggle",
-		env.cookies,
-		map[string]string{paramSourceID: wh.ID, paramTargetID: tgt.ID},
-	)
-	w := httptest.NewRecorder()
-
-	env.handlers.HandleTargetToggle().ServeHTTP(w, req)
-
-	require.Equal(t, http.StatusSeeOther, w.Code)
-	require.Equal(t, http.StatusSeeOther, editCode)
-
-	stored := storedTarget(t, env, tgt.ID)
-	assert.False(t, stored.Active)
-	assert.Equal(t, "edited-name", stored.Name)
-	assert.Equal(t, 5, stored.MaxRetries)
-	assert.Equal(
-		t, editReplacedURL, storedHTTPConfig(t, env, tgt.ID).URL,
-	)
-}

internal/middleware/middleware_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gorilla/sessions"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/webhooker/internal/config"
@@ -77,7 +78,14 @@ func newTestSessionManager(
 		key[i] = byte(i)
 	}
 
-	store := session.NewStore(key)
+	store := sessions.NewCookieStore(key)
+	store.Options = &sessions.Options{
+		Path:     "/",
+		MaxAge:   86400 * 7,
+		HttpOnly: true,
+		Secure:   false,
+		SameSite: http.SameSiteLaxMode,
+	}
 
 	var now func() time.Time
 
@@ -923,7 +931,8 @@ func metricsAuthMiddleware(
 	}
 
 	key := make([]byte, testKeySize)
-	store := session.NewStore(key)
+	store := sessions.NewCookieStore(key)
+	store.Options = &sessions.Options{Path: "/", MaxAge: 86400}
 
 	sessManager := session.NewForTest(store, cfg, log, key, nil)
 

internal/session/export_test.go

@@ -0,0 +1,10 @@
+package session
+
+import "github.com/gorilla/sessions"
+
+// NewStore exposes the production cookie-store constructor so tests
+// exercise the store the application actually runs with, rather than a
+// lookalike assembled in the test.
+func NewStore(key []byte) *sessions.CookieStore {
+	return newStore(key)
+}

internal/session/testing.go

@@ -8,13 +8,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/config"
 )
 
-// NewStore exposes the production cookie-store constructor so tests
-// exercise the store the application actually runs with, rather than a
-// lookalike assembled in the test.
-func NewStore(key []byte) *sessions.CookieStore {
-	return newStore(key)
-}
-
 // NewForTest creates a Session with a pre-configured cookie store for use
 // in tests. This bypasses the fx lifecycle and database dependency, allowing
 // middleware and handler tests to use real session functionality. The key