feat: revision-check material writes
- add jz_bm_materials.revision (default 1), read-only for GORM
- bump it only on content changes (models.ReviseMaterial)
- PUT /materials/{id} locks the row and answers 409 to a stale revision
- management answers carry revision and updated_at, the student detail
carries revision
This commit is contained in:
@@ -241,11 +241,13 @@ func newMaterialListJSON(m models.Material, files int64) materialListJSON {
|
||||
|
||||
// materialDetailJSON is a published material the caller may see, with its
|
||||
// body and files. CanManage is true for the caller who may change its access.
|
||||
// Revision is the content revision an edit sends back.
|
||||
type materialDetailJSON struct {
|
||||
materialListJSON
|
||||
Body *string `json:"body"`
|
||||
Files []fileJSON `json:"files"`
|
||||
CanManage bool `json:"can_manage"`
|
||||
Revision uint `json:"revision"`
|
||||
}
|
||||
|
||||
// fileJSON is one attached file.
|
||||
|
||||
@@ -22,6 +22,7 @@ import (
|
||||
"git.golem15.com/jakub/sm-bm-plugin/classes"
|
||||
"git.golem15.com/jakub/sm-bm-plugin/models"
|
||||
"gorm.io/gorm"
|
||||
"gorm.io/gorm/clause"
|
||||
)
|
||||
|
||||
// The management API (/api/v1/bm) is what the console commands do, over
|
||||
@@ -155,6 +156,11 @@ type managedMaterialJSON struct {
|
||||
AuthorUserID *uint `json:"author_user_id"`
|
||||
Author *userRefJSON `json:"author"`
|
||||
Shares []userRefJSON `json:"shares"`
|
||||
// Revision is the material's content revision, which a write may send
|
||||
// back to be refused when the material changed since; UpdatedAt is
|
||||
// when the row was last written.
|
||||
Revision uint `json:"revision"`
|
||||
UpdatedAt string `json:"updated_at"`
|
||||
// Snippet is the matched passage of a search result, as on the
|
||||
// student list; nil, and the key absent, without q.
|
||||
Snippet any `json:"snippet,omitempty"`
|
||||
@@ -193,6 +199,7 @@ func newManagedMaterialJSON(m models.Material, people materialPeople) managedMat
|
||||
ID: m.ID, CourseID: m.CourseID, Course: newCourseRef(m.Course), MeetingID: m.MeetingID, Title: m.Title, Type: m.Type,
|
||||
Body: m.Body, Source: m.Source, URL: m.URL, Tags: newTagsJSON(m.Tags),
|
||||
IsPrivate: m.IsPrivate, AuthorUserID: m.AuthorUserID, Shares: people.shares[m.ID],
|
||||
Revision: m.Revision, UpdatedAt: models.FormatDateTime(m.UpdatedAt),
|
||||
}
|
||||
if m.AuthorUserID != nil {
|
||||
if a, ok := people.authors[*m.AuthorUserID]; ok {
|
||||
@@ -274,6 +281,10 @@ func (h *Handlers) ManagedMaterials(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
// materialInput is the body of POST /materials and PUT /materials/{id}.
|
||||
// Revision is the revision the writer last read: when sent and not the
|
||||
// stored one the write is a 409 and nothing is written; absent means
|
||||
// unchecked, so the write overwrites as callers that never send it expect
|
||||
// (D-04).
|
||||
type materialInput struct {
|
||||
CourseID *uint `json:"course_id"`
|
||||
MeetingID *uint `json:"meeting_id"`
|
||||
@@ -288,6 +299,8 @@ type materialInput struct {
|
||||
IsPrivate *bool `json:"is_private"`
|
||||
AuthorUserID *uint `json:"author_user_id"`
|
||||
ShareUserIDs *[]uint `json:"share_user_ids"`
|
||||
|
||||
Revision *uint `json:"revision"`
|
||||
}
|
||||
|
||||
// materialDraft is a decoded and validated material body, ready to apply.
|
||||
@@ -313,26 +326,50 @@ type materialDraft struct {
|
||||
// PUT /materials/{id}. It answers 400, 413, 422 or 500 itself and returns
|
||||
// false then.
|
||||
func (h *Handlers) readMaterial(w http.ResponseWriter, r *http.Request, db *gorm.DB) (materialDraft, bool) {
|
||||
var d materialDraft
|
||||
raw, ok := body(w, r)
|
||||
if !ok {
|
||||
return materialDraft{}, false
|
||||
}
|
||||
d, err := decodeMaterial(raw)
|
||||
if err != nil {
|
||||
writeInvalid(w, "invalid JSON: "+err.Error())
|
||||
return d, false
|
||||
}
|
||||
if !h.checkMaterial(w, db, &d, nil) {
|
||||
return d, false
|
||||
}
|
||||
return d, true
|
||||
}
|
||||
|
||||
// decodeMaterial decodes a material body, refusing unknown keys, and notes
|
||||
// which of published_at, meeting_id and author_user_id were sent. It does
|
||||
// not validate the values; checkMaterial does.
|
||||
func decodeMaterial(raw []byte) (materialDraft, error) {
|
||||
var d materialDraft
|
||||
dec := json.NewDecoder(bytes.NewReader(raw))
|
||||
dec.DisallowUnknownFields()
|
||||
if err := dec.Decode(&d.in); err != nil {
|
||||
writeInvalid(w, "invalid JSON: "+err.Error())
|
||||
return d, false
|
||||
return d, err
|
||||
}
|
||||
var keys map[string]json.RawMessage
|
||||
_ = json.Unmarshal(raw, &keys)
|
||||
_, d.hasPublishedAt = keys["published_at"]
|
||||
_, d.hasMeetingID = keys["meeting_id"]
|
||||
_, d.hasAuthor = keys["author_user_id"]
|
||||
return d, nil
|
||||
}
|
||||
|
||||
// checkMaterial validates a decoded material body against db and fills in
|
||||
// its published_at, tags and shares. Its messages join errs, which a caller
|
||||
// may pre-fill with its own rules (nil means none); when there are any it
|
||||
// answers one 422 with all of them, and on a database error a 500, and
|
||||
// returns false then.
|
||||
func (h *Handlers) checkMaterial(w http.ResponseWriter, db *gorm.DB, d *materialDraft, errs map[string][]string) bool {
|
||||
if errs == nil {
|
||||
errs = map[string][]string{}
|
||||
}
|
||||
in := &d.in
|
||||
in.Title = strings.TrimSpace(in.Title)
|
||||
errs := map[string][]string{}
|
||||
if in.Title == "" || len(in.Title) > 255 {
|
||||
errs["title"] = []string{"The title is required and may not be longer than 255 characters."}
|
||||
}
|
||||
@@ -369,7 +406,7 @@ func (h *Handlers) readMaterial(w http.ResponseWriter, r *http.Request, db *gorm
|
||||
var n int64
|
||||
if err := db.Model(&models.Course{}).Where("id = ?", *in.CourseID).Count(&n).Error; err != nil {
|
||||
writeOpaque500(w)
|
||||
return d, false
|
||||
return false
|
||||
}
|
||||
if n == 0 {
|
||||
errs["course_id"] = []string{"The selected course_id is invalid."}
|
||||
@@ -379,7 +416,7 @@ func (h *Handlers) readMaterial(w http.ResponseWriter, r *http.Request, db *gorm
|
||||
var n int64
|
||||
if err := db.Model(&models.Meeting{}).Where("id = ?", *in.MeetingID).Count(&n).Error; err != nil {
|
||||
writeOpaque500(w)
|
||||
return d, false
|
||||
return false
|
||||
}
|
||||
if n == 0 {
|
||||
errs["meeting_id"] = []string{"The selected meeting_id is invalid."}
|
||||
@@ -389,7 +426,7 @@ func (h *Handlers) readMaterial(w http.ResponseWriter, r *http.Request, db *gorm
|
||||
found, err := userRefsByID(db, []uint{*in.AuthorUserID})
|
||||
if err != nil {
|
||||
writeOpaque500(w)
|
||||
return d, false
|
||||
return false
|
||||
}
|
||||
if _, ok := found[*in.AuthorUserID]; !ok {
|
||||
errs["author_user_id"] = []string{"The selected author_user_id is invalid."}
|
||||
@@ -399,7 +436,7 @@ func (h *Handlers) readMaterial(w http.ResponseWriter, r *http.Request, db *gorm
|
||||
clean, msg, err := cleanUserIDs(db, *in.ShareUserIDs)
|
||||
if err != nil {
|
||||
writeOpaque500(w)
|
||||
return d, false
|
||||
return false
|
||||
}
|
||||
if msg != "" {
|
||||
errs["share_user_ids"] = []string{msg}
|
||||
@@ -408,9 +445,9 @@ func (h *Handlers) readMaterial(w http.ResponseWriter, r *http.Request, db *gorm
|
||||
}
|
||||
if len(errs) > 0 {
|
||||
writeValidation(w, errs)
|
||||
return d, false
|
||||
return false
|
||||
}
|
||||
return d, true
|
||||
return true
|
||||
}
|
||||
|
||||
// identity scopes a query to the materials with the draft's title and
|
||||
@@ -551,7 +588,11 @@ func (h *Handlers) UpsertMaterial(w http.ResponseWriter, r *http.Request) {
|
||||
// UpdateMaterial answers PUT /materials/{id}: the same body as POST, applied
|
||||
// to that material, so a title or course can change. A title and course
|
||||
// another material already has is a 422, because POST would then be
|
||||
// ambiguous. An unknown id is a 404.
|
||||
// ambiguous. An unknown id is a 404. The row is locked for the write: a sent
|
||||
// revision that is not the stored one is a 409 with the current version
|
||||
// (writeConflict) and nothing is written; without revision the write
|
||||
// overwrites. The revision goes up only when the content changed
|
||||
// (models.ReviseMaterial).
|
||||
func (h *Handlers) UpdateMaterial(w http.ResponseWriter, r *http.Request) {
|
||||
callerID, ok := h.caller(w, r)
|
||||
if !ok {
|
||||
@@ -571,9 +612,9 @@ func (h *Handlers) UpdateMaterial(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
var m models.Material
|
||||
found, taken := false, false
|
||||
err := lagoon.Transaction(r.Context(), db, func(_ context.Context, tx *gorm.DB) error {
|
||||
res := tx.Where("id = ?", id).Limit(1).Find(&m)
|
||||
found, taken, conflict := false, false, false
|
||||
err := lagoon.Transaction(r.Context(), db, func(ctx context.Context, tx *gorm.DB) error {
|
||||
res := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Where("id = ?", id).Limit(1).Find(&m)
|
||||
if res.Error != nil {
|
||||
return res.Error
|
||||
}
|
||||
@@ -581,6 +622,14 @@ func (h *Handlers) UpdateMaterial(w http.ResponseWriter, r *http.Request) {
|
||||
return nil
|
||||
}
|
||||
found = true
|
||||
if staleRevision(d.in.Revision, m.Revision) {
|
||||
conflict = true
|
||||
return nil
|
||||
}
|
||||
before, err := models.MaterialContent(tx, m.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
var n int64
|
||||
if err := d.identity(tx.Model(&models.Material{})).Where("id <> ?", m.ID).Count(&n).Error; err != nil {
|
||||
return err
|
||||
@@ -589,13 +638,18 @@ func (h *Handlers) UpdateMaterial(w http.ResponseWriter, r *http.Request) {
|
||||
taken = true
|
||||
return nil
|
||||
}
|
||||
return d.apply(tx, &m, false, h.now(), callerID)
|
||||
if err := d.apply(tx, &m, false, h.now(), callerID); err != nil {
|
||||
return err
|
||||
}
|
||||
return h.revise(ctx, tx, m.ID, before)
|
||||
})
|
||||
switch {
|
||||
case err != nil:
|
||||
writeOpaque500(w)
|
||||
case !found:
|
||||
writeNotFound(w)
|
||||
case conflict:
|
||||
writeConflict(w, r, db, m.ID)
|
||||
case taken:
|
||||
writeValidation(w, map[string][]string{"title": {"Another material already has this title and course."}})
|
||||
default:
|
||||
|
||||
58
controllers/api/material_revision.go
Normal file
58
controllers/api/material_revision.go
Normal file
@@ -0,0 +1,58 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/wire"
|
||||
"git.golem15.com/jakub/sm-bm-plugin/models"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
// A write of a material's content may carry the revision the writer last
|
||||
// read (D-01, D-04). When it does and the stored revision differs, the write
|
||||
// is refused with 409 and nothing is written; when it does not, the write
|
||||
// overwrites as before, so callers that never send a revision keep working.
|
||||
// The compare and the write run on the row locked FOR UPDATE, so two writers
|
||||
// holding the same revision cannot both succeed.
|
||||
|
||||
// conflictMessage is the message of a 409 answer to a stale revision.
|
||||
const conflictMessage = "The material was changed since the revision you sent."
|
||||
|
||||
// staleRevision reports whether a write that sent revision sent must be
|
||||
// refused: true when it sent one and it is not the stored one.
|
||||
func staleRevision(sent *uint, stored uint) bool {
|
||||
return sent != nil && *sent != stored
|
||||
}
|
||||
|
||||
// revise bumps the revision of material id inside tx when its content
|
||||
// differs from before (models.ReviseMaterial). Every content writer of the
|
||||
// API calls it after its write.
|
||||
func (h *Handlers) revise(ctx context.Context, tx *gorm.DB, id uint, before string) error {
|
||||
_, _, err := models.ReviseMaterial(tx.WithContext(ctx), id, before)
|
||||
return err
|
||||
}
|
||||
|
||||
// writeConflict answers 409 to a write that sent a stale revision, with the
|
||||
// current revision, its updated_at and the material as the management API
|
||||
// shows it, so the writer can diff or pull. Under MCP the body is left out,
|
||||
// as in every other MCP answer.
|
||||
func writeConflict(w http.ResponseWriter, r *http.Request, db *gorm.DB, id uint) {
|
||||
var cur models.Material
|
||||
if err := preloadMaterial(db).First(&cur, id).Error; err != nil {
|
||||
writeOpaque500(w)
|
||||
return
|
||||
}
|
||||
people, err := loadMaterialPeople(db, cur)
|
||||
if err != nil {
|
||||
writeOpaque500(w)
|
||||
return
|
||||
}
|
||||
wire.WriteJSON(w, http.StatusConflict, map[string]any{
|
||||
"error": true,
|
||||
"message": conflictMessage,
|
||||
"revision": cur.Revision,
|
||||
"updated_at": models.FormatDateTime(cur.UpdatedAt),
|
||||
"data": managedMaterialView(newManagedMaterialJSON(cur, people), viaMCP(r), false),
|
||||
})
|
||||
}
|
||||
92
controllers/api/material_revision_test.go
Normal file
92
controllers/api/material_revision_test.go
Normal file
@@ -0,0 +1,92 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.golem15.com/jakub/sm-bm-plugin/models"
|
||||
)
|
||||
|
||||
// TestDecodeMaterialRevision checks that revision is an optional key of the
|
||||
// material body: sent, it decodes to a pointer; absent, to nil; and unknown
|
||||
// keys are still refused.
|
||||
func TestDecodeMaterialRevision(t *testing.T) {
|
||||
d, err := decodeMaterial([]byte(`{"title":"Notatka","type":"notatka","revision":3}`))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if d.in.Revision == nil || *d.in.Revision != 3 {
|
||||
t.Errorf("revision = %v, want a pointer to 3", d.in.Revision)
|
||||
}
|
||||
d, err = decodeMaterial([]byte(`{"title":"Notatka","type":"notatka","meeting_id":null}`))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if d.in.Revision != nil {
|
||||
t.Errorf("absent revision = %v, want nil", *d.in.Revision)
|
||||
}
|
||||
if !d.hasMeetingID || d.hasPublishedAt || d.hasAuthor {
|
||||
t.Errorf("key flags = meeting %v published %v author %v, want only meeting", d.hasMeetingID, d.hasPublishedAt, d.hasAuthor)
|
||||
}
|
||||
if _, err := decodeMaterial([]byte(`{"title":"Notatka","type":"notatka","rev":3}`)); err == nil {
|
||||
t.Error("an unknown key was accepted")
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleRevision checks that only a sent revision different from the
|
||||
// stored one is stale.
|
||||
func TestStaleRevision(t *testing.T) {
|
||||
one, two := uint(1), uint(2)
|
||||
for _, tc := range []struct {
|
||||
sent *uint
|
||||
stored uint
|
||||
want bool
|
||||
}{
|
||||
{nil, 1, false},
|
||||
{nil, 7, false},
|
||||
{&one, 1, false},
|
||||
{&two, 1, true},
|
||||
{&one, 2, true},
|
||||
} {
|
||||
if got := staleRevision(tc.sent, tc.stored); got != tc.want {
|
||||
t.Errorf("staleRevision(%v, %d) = %v, want %v", tc.sent, tc.stored, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestManagedMaterialJSONCarriesRevision checks that a managed material
|
||||
// carries revision and updated_at.
|
||||
func TestManagedMaterialJSONCarriesRevision(t *testing.T) {
|
||||
m := models.Material{ID: 3, Title: "Notatka", Type: "notatka", Revision: 4, UpdatedAt: time.Date(2026, 11, 9, 12, 0, 0, 0, time.UTC)}
|
||||
b, err := json.Marshal(newManagedMaterialJSON(m, materialPeople{}))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var keys map[string]any
|
||||
if err := json.Unmarshal(b, &keys); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if keys["revision"] != float64(4) {
|
||||
t.Errorf("revision = %v in %s, want 4", keys["revision"], b)
|
||||
}
|
||||
if keys["updated_at"] != models.FormatDateTime(m.UpdatedAt) {
|
||||
t.Errorf("updated_at = %v in %s, want %s", keys["updated_at"], b, models.FormatDateTime(m.UpdatedAt))
|
||||
}
|
||||
}
|
||||
|
||||
// TestMaterialJSONCarriesRevision checks that the student material detail
|
||||
// carries revision.
|
||||
func TestMaterialJSONCarriesRevision(t *testing.T) {
|
||||
b, err := json.Marshal(materialDetailJSON{materialListJSON: materialListJSON{ID: 1, Title: "Notatka"}, Revision: 5})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var keys map[string]any
|
||||
if err := json.Unmarshal(b, &keys); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if keys["revision"] != float64(5) {
|
||||
t.Errorf("revision = %v in %s, want 5", keys["revision"], b)
|
||||
}
|
||||
}
|
||||
@@ -234,6 +234,7 @@ func (h *Handlers) Material(w http.ResponseWriter, r *http.Request) {
|
||||
materialListJSON: newMaterialListJSON(m, int64(len(files))),
|
||||
Body: m.Body,
|
||||
Files: make([]fileJSON, 0, len(files)),
|
||||
Revision: m.Revision,
|
||||
}
|
||||
for i := range files {
|
||||
f := &files[i]
|
||||
|
||||
Reference in New Issue
Block a user