feat: revision-check the upsert, MCP and admin writes
- POST /materials locks the matched row and answers 409 to a stale revision when it updates; a create ignores revision - bm_manage_save_material and bm_manage_update_material take revision - /bmadmin saves bump the revision on content changes (not checked) - document revisions and the 409 in the README
This commit is contained in:
@@ -549,7 +549,11 @@ func writeManaged(w http.ResponseWriter, r *http.Request, db *gorm.DB, id uint,
|
||||
// updates the one with the same title and course, so sending a document
|
||||
// twice changes nothing. body, source and url are replaced as sent; see
|
||||
// materialDraft.apply for published_at and tags. A renamed material is a
|
||||
// new one here: PUT /materials/{id} renames.
|
||||
// new one here: PUT /materials/{id} renames. revision is checked only when
|
||||
// the upsert updates an existing material, on its row locked for the write:
|
||||
// a stale one is a 409 with the current version (writeConflict) and nothing
|
||||
// is written. A create ignores it and starts at revision 1. The revision
|
||||
// goes up only when the content changed (models.ReviseMaterial).
|
||||
func (h *Handlers) UpsertMaterial(w http.ResponseWriter, r *http.Request) {
|
||||
callerID, ok := h.caller(w, r)
|
||||
if !ok {
|
||||
@@ -564,19 +568,37 @@ func (h *Handlers) UpsertMaterial(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
var m models.Material
|
||||
created := false
|
||||
err := lagoon.Transaction(r.Context(), db, func(_ context.Context, tx *gorm.DB) error {
|
||||
res := d.identity(tx).Order("id").Limit(1).Find(&m)
|
||||
created, conflict := false, false
|
||||
err := lagoon.Transaction(r.Context(), db, func(ctx context.Context, tx *gorm.DB) error {
|
||||
res := d.identity(tx.Clauses(clause.Locking{Strength: "UPDATE"})).Order("id").Limit(1).Find(&m)
|
||||
if res.Error != nil {
|
||||
return res.Error
|
||||
}
|
||||
created = res.RowsAffected == 0
|
||||
return d.apply(tx, &m, created, h.now(), callerID)
|
||||
if created {
|
||||
return d.apply(tx, &m, true, h.now(), callerID)
|
||||
}
|
||||
if staleRevision(d.in.Revision, m.Revision) {
|
||||
conflict = true
|
||||
return nil
|
||||
}
|
||||
before, err := models.MaterialContent(tx, m.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := d.apply(tx, &m, false, h.now(), callerID); err != nil {
|
||||
return err
|
||||
}
|
||||
return h.revise(ctx, tx, m.ID, before)
|
||||
})
|
||||
if err != nil {
|
||||
writeOpaque500(w)
|
||||
return
|
||||
}
|
||||
if conflict {
|
||||
writeConflict(w, r, db, m.ID)
|
||||
return
|
||||
}
|
||||
h.MaterialIndex.Sync(r.Context(), db, m.ID)
|
||||
status := http.StatusOK
|
||||
if created {
|
||||
|
||||
@@ -3,6 +3,7 @@ package controllers
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/cabana"
|
||||
@@ -12,11 +13,18 @@ import (
|
||||
)
|
||||
|
||||
var (
|
||||
_ pact.FormAfterCreate = materialsAdmin{}
|
||||
_ pact.FormAfterUpdate = materialsAdmin{}
|
||||
_ pact.FormAfterDelete = materialsAdmin{}
|
||||
_ pact.FormBeforeUpdate = materialsAdmin{}
|
||||
_ pact.FormAfterCreate = materialsAdmin{}
|
||||
_ pact.FormAfterUpdate = materialsAdmin{}
|
||||
_ pact.FormAfterDelete = materialsAdmin{}
|
||||
)
|
||||
|
||||
// pendingRevisions holds, per material id, the content fingerprint
|
||||
// (models.MaterialContent) FormBeforeUpdate took before an admin save, for
|
||||
// FormAfterUpdate to compare. The row lock FormBeforeUpdate takes keeps one
|
||||
// pending save per id at a time.
|
||||
var pendingRevisions sync.Map
|
||||
|
||||
// MaterialIndex keeps the materials search index current once a write
|
||||
// commits (classes.MaterialIndexer). QueueExtract enqueues the file text job
|
||||
// inside tx. Its calls never fail the write.
|
||||
@@ -49,11 +57,56 @@ func (materialsAdmin) FormAfterCreate(ctx context.Context, model any) error {
|
||||
return materialAfterCommit(ctx, model, false)
|
||||
}
|
||||
|
||||
// FormAfterUpdate queues the file text job in the save transaction and
|
||||
// syncs the material once it commits. A save that added no file queues a
|
||||
// job that finds no new PDF and only resyncs; a file removed in the form
|
||||
// leaves the document through the cascade and this sync.
|
||||
// FormBeforeUpdate runs the embedded date check, then, inside the save
|
||||
// transaction, locks the material's row and snapshots its content for
|
||||
// FormAfterUpdate. An admin save is not revision-checked (the form sends no
|
||||
// revision); it only bumps the revision when the content changed, so a later
|
||||
// push holding an older revision is refused instead of overwriting the
|
||||
// admin edit. Outside a write (no transaction) it does nothing more.
|
||||
func (c materialsAdmin) FormBeforeUpdate(ctx context.Context, model any) error {
|
||||
if err := c.temporalHooks.FormBeforeUpdate(ctx, model); err != nil {
|
||||
return err
|
||||
}
|
||||
m, ok := model.(*models.Material)
|
||||
if !ok || m == nil {
|
||||
return errors.New("materials admin: form model is not a material")
|
||||
}
|
||||
tx, ok := cabana.TxFromContext(ctx)
|
||||
if !ok || m.ID == 0 {
|
||||
return nil
|
||||
}
|
||||
var locked int
|
||||
if err := tx.Session(&gorm.Session{NewDB: true}).Raw("SELECT 1 FROM jz_bm_materials WHERE id = ? FOR UPDATE", m.ID).Scan(&locked).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
before, err := models.MaterialContent(tx, m.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
pendingRevisions.Store(m.ID, before)
|
||||
return nil
|
||||
}
|
||||
|
||||
// FormAfterUpdate bumps the revision when the save changed the material's
|
||||
// content, then queues the file text job in the save transaction and syncs
|
||||
// the material once it commits. The compare runs here, not in
|
||||
// FormBeforeUpdate, because the course, the zjazd and the tags are final
|
||||
// only after the row write and the pivot sync. A revise error fails the save
|
||||
// like any database error. A save that added no file queues a job that finds
|
||||
// no new PDF and only resyncs; a file removed in the form leaves the
|
||||
// document through the cascade and this sync.
|
||||
func (materialsAdmin) FormAfterUpdate(ctx context.Context, model any) error {
|
||||
m, ok := model.(*models.Material)
|
||||
if !ok || m == nil {
|
||||
return errors.New("materials admin: form model is not a material")
|
||||
}
|
||||
if pending, found := pendingRevisions.LoadAndDelete(m.ID); found {
|
||||
if tx, ok := cabana.TxFromContext(ctx); ok {
|
||||
if _, _, err := models.ReviseMaterial(tx, m.ID, pending.(string)); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
}
|
||||
return materialAfterCommit(ctx, model, false)
|
||||
}
|
||||
|
||||
|
||||
@@ -2,10 +2,13 @@ package controllers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"slices"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/cabana"
|
||||
"git.golem15.com/jakub/sm-bm-plugin/models"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
@@ -86,3 +89,42 @@ func TestMaterialsAdminHooks(t *testing.T) {
|
||||
t.Errorf("calls for a wrong model = %v", rec.calls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestMaterialsAdminRevision checks the revision hooks of the materials
|
||||
// admin outside a database write: without a transaction they do nothing and
|
||||
// store no snapshot, a model that is not a material is an error, and the
|
||||
// embedded date check still runs first.
|
||||
func TestMaterialsAdminRevision(t *testing.T) {
|
||||
t.Cleanup(func() { UseMaterialIndex(nil) })
|
||||
UseMaterialIndex(nil)
|
||||
var c materialsAdmin
|
||||
ctx := context.Background()
|
||||
m := &models.Material{ID: 41, Title: "Notatka", Type: "notatka"}
|
||||
|
||||
if err := c.FormBeforeUpdate(ctx, m); err != nil {
|
||||
t.Errorf("before update without a transaction: %v", err)
|
||||
}
|
||||
if _, found := pendingRevisions.Load(m.ID); found {
|
||||
t.Error("before update without a transaction stored a snapshot")
|
||||
}
|
||||
if err := c.FormAfterUpdate(ctx, m); err != nil {
|
||||
t.Errorf("after update without a transaction: %v", err)
|
||||
}
|
||||
|
||||
if err := c.FormBeforeUpdate(ctx, &models.Course{ID: 41}); err == nil {
|
||||
t.Error("before update with a course: no error")
|
||||
}
|
||||
var nilMaterial *models.Material
|
||||
if err := c.FormBeforeUpdate(ctx, nilMaterial); err == nil {
|
||||
t.Error("before update with a nil material: no error")
|
||||
}
|
||||
|
||||
at := func(h, m int) time.Time { return time.Date(2026, 10, 3, h, m, 0, 0, models.Location) }
|
||||
bad := &models.ClassSession{StartsAt: at(8, 0), EndsAt: at(7, 0)}
|
||||
var ve *cabana.ValidationError
|
||||
if err := c.FormBeforeUpdate(ctx, bad); !errors.As(err, &ve) {
|
||||
t.Errorf("before update with bad dates: err = %v, want a cabana.ValidationError", err)
|
||||
} else if _, ok := ve.Details["ends_at"]; !ok {
|
||||
t.Errorf("before update with bad dates: details %v name no ends_at", ve.Details)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user