fix(17-06): singleton route retries a lost first-or-create race
Two first requests to GET .../singleton both missed the record; the loser's insert hit the unique index and the route answered 500. A unique violation from SingletonRecord now reruns the transaction once, which finds the winner's record. TestSingleton fires six concurrent first requests and requires 200 with the same id from each.
This commit is contained in:
@@ -448,7 +448,7 @@ Bulk actions have their own namespace next to the toolbar and widget actions: a
|
||||
|
||||
## Singleton controllers
|
||||
|
||||
A controller that edits exactly one record, such as a site-wide definition, implements `pact.AdminSingleton`. Its `SingletonRecord(ctx, db)` returns that record and creates it first when it does not exist (first or create), with `db` the request's transaction. Make the creation safe against two first requests at once, for example with a unique key column and GORM's `FirstOrCreate`.
|
||||
A controller that edits exactly one record, such as a site-wide definition, implements `pact.AdminSingleton`. Its `SingletonRecord(ctx, db)` returns that record and creates it first when it does not exist (first or create), with `db` the request's transaction. Make the creation safe against two first requests at once, for example with a unique key column and GORM's `FirstOrCreate`: when the losing request's insert hits that unique index (a Postgres unique violation), the singleton route runs its transaction once more, and the second `SingletonRecord` call finds the winner's record.
|
||||
|
||||
- `GET <prefix>/api/v1/{vendor}/{plugin}/{controller}/singleton` answers the record's id (`cabana.AdminSingletonResult`), after checking that the form scope (`pact.FormExtendQuery`) can load it. The admin then opens the record's form.
|
||||
- The form schema carries `singleton: true`. Show and update work as on any controller.
|
||||
|
||||
@@ -41,7 +41,7 @@ Use cases:
|
||||
- Rich text: `type: richeditor` edits HTML in a host text column (the admin SPA shows a WYSIWYG editor). It takes the common field keys plus `size`; any other key fails boot, and settings and relation forms refuse the type (supported in controller forms only). On create and update a submitted value must be a string or null (anything else is a 422 on the field) and is sanitized through [seaglass](../seaglass/README.md) before it is filled: HTML the policy keeps whole is stored byte for byte, otherwise the clean HTML is stored and what was removed is reported in `meta.notices` under the field (`backend::lang.richeditor.removed_elements`, `removed_attributes`, `removed_comments` with the count and names, or `markup_changed`).
|
||||
- Repeater: `type: repeater` edits a JSON list in one column (`lagoon.Jsonable`) with a nested `form.fields` of `text`, `textarea`, `number`, `checkbox`, `switch`, `dropdown` (static, method or `dependsOn` a field of the same row) and `richeditor` fields, plus `prompt`, `minItems`, `maxItems` and `titleFrom` (`cabana.RepeaterForm` in the schema; the span is always full). Other nested types, a nested repeater, nested `tab`, `context` or `preset`, and WinterCMS's `groups` and `style` fail boot; settings and relation forms refuse the type. A save decodes the rows before the projection: null is an empty list, anything but a list of objects is a 422 on the field, each row keeps only the nested names with scalars coerced per type, a required nested value missing in a row is a 422 under `field.index.nested`, the row bounds are 422s on the field (`repeater.max_items`, `repeater.min_items`), nested richeditor values are sanitized with notices under the row path, and nested dependent dropdowns are rechecked. Record responses carry the list of rows.
|
||||
- Tree reorder (D-09): a `config_reorder.yaml` (`title`, `nameFrom`, `modelClass`, `parentColumn` default `parent_id`, `sortColumn` default `sort_order`, `maxDepth`, and an optional `partition` with `column`, `relation`, `nameFrom` and `label`) gives the controller a reorder page and the built-in `reorder` toolbar button; unknown keys, columns that are not record columns, a partition relation that is not a belongsTo relation field over the column, and a `reorder` button without the file fail boot. The list schema carries `reorder` (`cabana.ListReorder`). GET `.../reorder` answers the scoped tree (`cabana.AdminReorderTree`); POST `.../reorder` locks the scoped set (ListExtendQuery plus the partition) in one transaction, answers 409 `reorder_stale` when the posted ids differ from it in any way, 422 for a duplicate, a foreign parent, a cycle or a depth above `maxDepth`, and otherwise writes both tree columns without model hooks.
|
||||
- Singleton controllers (D-11): a controller implementing `pact.AdminSingleton` needs only `config_form.yaml`; GET `.../singleton` answers the id of the record `SingletonRecord` returns (created when absent) after checking the form scope, the form schema carries `singleton`, and the list, create, bulk and delete routes answer 404. A settings link entry may open it.
|
||||
- Singleton controllers (D-11): a controller implementing `pact.AdminSingleton` needs only `config_form.yaml`; GET `.../singleton` answers the id of the record `SingletonRecord` returns (created when absent; a first request that loses the creation race on a unique index runs once more and finds the record) after checking the form scope, the form schema carries `singleton`, and the list, create, bulk and delete routes answer 404. A settings link entry may open it.
|
||||
- Dependent dropdowns: `dependsOn: <field>` on a `type: dropdown` field names a sibling with a scalar value (the same form, or the same repeater row); the dropdown takes no `options` and its controller implements `pact.DependentOptionsProvider`, else boot fails. GET `.../fields/{field}/dependent-options?values[<sibling>]=<value>` answers `cabana.AdminDependentOptionsResult` (`applies` and the localized `options` with their `depth`); `{field}` is `repeater.field` for a dropdown in a repeater row; the form schema carries `dependsOn`. Every save rechecks the value with the submitted sibling value (per row in a repeater): a value that does not apply is stored null, one outside the options is a 422 `form.dependent_invalid` on the field. A menu or sitemap controller returns `signpost.ReferenceOptions` here, the one admin binding of the [signpost](../signpost/README.md) menu item type registry. Settings and relation forms refuse `dependsOn` (supported in controller forms only).
|
||||
- Preset fields: `preset` on a `type: text` or `type: mltext` field (a source field name, or a mapping with `field` and `type`, `slug` or `exact`) makes the field follow another text or mltext field of the same form on the create screen until the administrator edits it, per locale for an mltext pair. The schema reports it as `preset` (`cabana.FieldPreset`); the server does not fill the field.
|
||||
- Server-rendered partials: `headerPartial: <name>` in `config_list.yaml` (a strip above the list) and `type: partial` with `path: <name>` in `fields.yaml` render the template `{ConfigDir}/_<name>.htm` with `html/template` against a view model from the controller's `pact.AdminPartialData`. The result reaches the SPA as an allowlisted node tree, never as an HTML string. A missing or unparsable template, a free-form path or a controller without `pact.AdminPartialData` fails boot.
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"net/http"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/pact"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
@@ -25,11 +26,26 @@ func singletonRefuses(w http.ResponseWriter, r *http.Request, cc *CompiledContro
|
||||
return true
|
||||
}
|
||||
|
||||
// singletonRace is a SingletonRecord that lost a first-or-create race: a
|
||||
// concurrent first request created the record between its read and its
|
||||
// insert, so the insert hit a unique index.
|
||||
type singletonRace struct{}
|
||||
|
||||
func (singletonRace) Error() string { return "cabana: singleton record created concurrently" }
|
||||
|
||||
// uniqueViolation reports a Postgres unique violation (SQLSTATE 23505).
|
||||
func uniqueViolation(err error) bool {
|
||||
var pgErr *pgconn.PgError
|
||||
return errors.As(err, &pgErr) && pgErr.Code == "23505"
|
||||
}
|
||||
|
||||
// singletonRoute serves GET {vendor}/{plugin}/{controller}/singleton: in one
|
||||
// transaction it asks the controller's SingletonRecord for the record
|
||||
// (created when absent) and answers its id, after checking that the form
|
||||
// scope (FormExtendQuery) can load it. It is guarded like the form schema; a
|
||||
// controller that is not a singleton answers 404.
|
||||
// scope (FormExtendQuery) can load it. When two first requests race and the
|
||||
// loser's insert hits the record's unique index, the transaction runs once
|
||||
// more and finds the record the winner created. It is guarded like the form
|
||||
// schema; a controller that is not a singleton answers 404.
|
||||
func (s *service) singletonRoute(w http.ResponseWriter, r *http.Request) {
|
||||
s.protect(w, r, func(cc *CompiledController) {
|
||||
provider, ok := cc.Controller.(pact.AdminSingleton)
|
||||
@@ -44,10 +60,13 @@ func (s *service) singletonRoute(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
ctx := r.Context()
|
||||
var result AdminSingletonResult
|
||||
err = db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
|
||||
attempt := func(tx *gorm.DB) error {
|
||||
txCtx := withTx(ctx, tx)
|
||||
record, err := provider.SingletonRecord(txCtx, tx)
|
||||
if err != nil {
|
||||
if uniqueViolation(err) {
|
||||
return singletonRace{}
|
||||
}
|
||||
return lifecycleFailure(cc, err)
|
||||
}
|
||||
var id uint
|
||||
@@ -70,7 +89,15 @@ func (s *service) singletonRoute(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
result.ID = id
|
||||
return nil
|
||||
})
|
||||
}
|
||||
err = db.WithContext(ctx).Transaction(attempt)
|
||||
var race singletonRace
|
||||
if errors.As(err, &race) {
|
||||
err = db.WithContext(ctx).Transaction(attempt)
|
||||
if errors.As(err, &race) {
|
||||
err = lifecycleFailure(cc, err)
|
||||
}
|
||||
}
|
||||
if err != nil {
|
||||
var missing recordNotFound
|
||||
if !errors.As(err, &missing) {
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/pact"
|
||||
@@ -47,11 +48,36 @@ func (plainProfiles) RequiredPermissions() []string { return []string{"acme.comp
|
||||
func (plainProfiles) NewRecord() any { return &companionsProfile{} }
|
||||
|
||||
// TestSingleton pins the D-11 routes on Postgres: the singleton route
|
||||
// creates the record once and answers its id; the record shows and updates; list, create, delete, bulk delete
|
||||
// creates the record once, also when first requests race (each answers 200
|
||||
// with the same id), and answers its id; the record shows and updates; list, create, delete, bulk delete
|
||||
// and reorder answer 404; the form schema says singleton; a controller that
|
||||
// is not a singleton answers 404 on the route; anonymous requests 401.
|
||||
func TestSingleton(t *testing.T) {
|
||||
e := newCompanionsEnv(t)
|
||||
// Concurrent first requests: the losers of the first-or-create race
|
||||
// retry and find the winner's record.
|
||||
var wg sync.WaitGroup
|
||||
answers := make([]*struct {
|
||||
code int
|
||||
body string
|
||||
}, 6)
|
||||
for i := range answers {
|
||||
wg.Add(1)
|
||||
go func(i int) {
|
||||
defer wg.Done()
|
||||
rec := e.send(t, http.MethodGet, "/acme/companions/profiles/singleton", nil, true)
|
||||
answers[i] = &struct {
|
||||
code int
|
||||
body string
|
||||
}{rec.Code, rec.Body.String()}
|
||||
}(i)
|
||||
}
|
||||
wg.Wait()
|
||||
for i, answer := range answers {
|
||||
if answer.code != http.StatusOK || answer.body != answers[0].body {
|
||||
t.Fatalf("concurrent first request %d: status=%d body=%s (first: %s)", i, answer.code, answer.body, answers[0].body)
|
||||
}
|
||||
}
|
||||
first := e.send(t, http.MethodGet, "/acme/companions/profiles/singleton", nil, true)
|
||||
id := dataID(t, first.Body.Bytes())
|
||||
for range 2 {
|
||||
|
||||
Reference in New Issue
Block a user