From 237069c827ab6640771c4c582ed343c3964fd96f Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Sun, 8 Nov 2026 23:33:34 +0100 Subject: [PATCH] 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 --- controllers/api/json.go | 2 + controllers/api/manage.go | 84 ++++++-- controllers/api/material_revision.go | 58 ++++++ controllers/api/material_revision_test.go | 92 +++++++++ controllers/api/materials.go | 1 + material_revision_postgres_test.go | 181 ++++++++++++++++++ models/material.go | 5 + models/material_revision.go | 114 +++++++++++ models/material_revision_test.go | 63 ++++++ plugin_test.go | 4 +- registry.gen.go | 1 + ...9120000_add_revision_to_jz_bm_materials.go | 23 +++ 12 files changed, 611 insertions(+), 17 deletions(-) create mode 100644 controllers/api/material_revision.go create mode 100644 controllers/api/material_revision_test.go create mode 100644 material_revision_postgres_test.go create mode 100644 models/material_revision.go create mode 100644 models/material_revision_test.go create mode 100644 updates/20261109120000_add_revision_to_jz_bm_materials.go diff --git a/controllers/api/json.go b/controllers/api/json.go index 84e41d4..5dd4414 100644 --- a/controllers/api/json.go +++ b/controllers/api/json.go @@ -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. diff --git a/controllers/api/manage.go b/controllers/api/manage.go index cae8f05..36d5928 100644 --- a/controllers/api/manage.go +++ b/controllers/api/manage.go @@ -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: diff --git a/controllers/api/material_revision.go b/controllers/api/material_revision.go new file mode 100644 index 0000000..cbfe00c --- /dev/null +++ b/controllers/api/material_revision.go @@ -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), + }) +} diff --git a/controllers/api/material_revision_test.go b/controllers/api/material_revision_test.go new file mode 100644 index 0000000..fa60438 --- /dev/null +++ b/controllers/api/material_revision_test.go @@ -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) + } +} diff --git a/controllers/api/materials.go b/controllers/api/materials.go index bd0a6b0..41a98e3 100644 --- a/controllers/api/materials.go +++ b/controllers/api/materials.go @@ -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] diff --git a/material_revision_postgres_test.go b/material_revision_postgres_test.go new file mode 100644 index 0000000..a4a5b92 --- /dev/null +++ b/material_revision_postgres_test.go @@ -0,0 +1,181 @@ +package bm_test + +import ( + "bytes" + "encoding/json" + "io" + "maps" + "net/http" + "net/http/httptest" + "strconv" + "testing" + + userclasses "git.golem15.com/golem15/sm-user-plugin/classes" + "git.golem15.com/golem15/summercms/modules/surf" + "git.golem15.com/jakub/sm-bm-plugin/models" +) + +// TestMaterialRevisionAgainstDatabase checks the revision of materials +// through the management API (D-01..D-05): a new material is at revision 1; +// a write with a stale revision is a 409 with the current version and +// changes nothing; a write without revision overwrites; only content +// changes bump; and the student detail shows the same revision. +func TestMaterialRevisionAgainstDatabase(t *testing.T) { + app, plugins, db := migrated(t) + seedFixture(t, plugins, "testdata/seed.example.json") + + var courses []models.Course + if err := db.Order("id").Limit(2).Find(&courses).Error; err != nil { + t.Fatal(err) + } + if len(courses) < 2 { + t.Fatalf("the fixture has %d courses, want 2", len(courses)) + } + userID := createUser(t, db, "bm-revision@example.test") + var groupID uint + if err := db.Raw(` +INSERT INTO user_groups (name, code, permissions, created_at, updated_at) +VALUES ('BM materials', 'bm-materials', '{"jz.bm.access_materials":1}', NOW(), NOW()) +RETURNING id`).Scan(&groupID).Error; err != nil { + t.Fatal(err) + } + if err := db.Exec(`INSERT INTO users_groups (user_id, user_group_id) VALUES (?, ?)`, userID, groupID).Error; err != nil { + t.Fatal(err) + } + secret, token, err := userclasses.MintAPIToken("usr_", userID, "BM notes", []string{"read", "write"}, nil, nil) + if err != nil { + t.Fatal(err) + } + if err := db.Create(token).Error; err != nil { + t.Fatal(err) + } + handler, err := surf.Assemble(app, plugins) + if err != nil { + t.Fatal(err) + } + student := apiClient{t: t, h: handler, token: tokenFor(t, userID)} + + send := func(method, path string, payload any) (int, map[string]any) { + t.Helper() + var rd io.Reader + if payload != nil { + raw, err := json.Marshal(payload) + if err != nil { + t.Fatal(err) + } + rd = bytes.NewReader(raw) + } + req := httptest.NewRequest(method, path, rd) + req.Header.Set("Authorization", "Bearer "+secret) + if payload != nil { + req.Header.Set("Content-Type", "application/json") + } + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + var out map[string]any + if rec.Body.Len() > 0 { + _ = json.Unmarshal(rec.Body.Bytes(), &out) + } + return rec.Code, out + } + data := func(out map[string]any) map[string]any { + t.Helper() + d, _ := out["data"].(map[string]any) + if d == nil { + t.Fatalf("no data object in %v", out) + } + return d + } + + note := map[string]any{"title": "Rewizje", "type": "notatka", "course_id": courses[0].ID, "body": "pierwsza\n", "tags": []string{"Globalizacja"}} + code, out := send(http.MethodPost, "/api/v1/bm/materials", note) + if code != http.StatusCreated { + t.Fatalf("create = %d %v", code, out) + } + created := data(out) + if created["revision"] != float64(1) || created["updated_at"] == nil { + t.Fatalf("created = %v, want revision 1 and updated_at", created) + } + path := "/api/v1/bm/materials/" + strconv.Itoa(int(created["id"].(float64))) + studentPath := "/_bm/api/v1/materials/" + strconv.Itoa(int(created["id"].(float64))) + + revisionOf := func() float64 { + t.Helper() + code, out := send(http.MethodGet, path, nil) + if code != http.StatusOK { + t.Fatalf("show = %d %v", code, out) + } + r, _ := data(out)["revision"].(float64) + return r + } + if r := revisionOf(); r != 1 { + t.Fatalf("new material revision = %v, want 1", r) + } + with := func(changes map[string]any) map[string]any { + out := maps.Clone(note) + maps.Copy(out, changes) + return out + } + + code, out = send(http.MethodPut, path, with(map[string]any{"body": "druga\n", "revision": 1})) + if code != http.StatusOK || data(out)["revision"] != float64(2) { + t.Fatalf("PUT with the current revision = %d %v, want 200 and revision 2", code, out) + } + note["body"] = "druga\n" + + code, out = send(http.MethodPut, path, with(map[string]any{"body": "nieaktualna\n", "revision": 1})) + if code != http.StatusConflict { + t.Fatalf("PUT with a stale revision = %d %v, want 409", code, out) + } + if out["error"] != true || out["revision"] != float64(2) || out["updated_at"] == nil || out["message"] == nil { + t.Errorf("409 body = %v, want error, message, revision 2 and updated_at", out) + } + if data(out)["body"] != "druga\n" { + t.Errorf("409 data.body = %v, want the stored body", data(out)["body"]) + } + var stored models.Material + if err := db.First(&stored, created["id"]).Error; err != nil { + t.Fatal(err) + } + if stored.Body == nil || *stored.Body != "druga\n" || stored.Revision != 2 { + t.Errorf("after the 409 the row is body %v revision %d, want unchanged", stored.Body, stored.Revision) + } + + code, out = send(http.MethodPut, path, with(map[string]any{"body": "trzecia\n"})) + if code != http.StatusOK || data(out)["revision"] != float64(3) { + t.Fatalf("PUT without revision = %d %v, want 200 and revision 3", code, out) + } + note["body"] = "trzecia\n" + + for name, changes := range map[string]map[string]any{ + "published_at only": {"published_at": "2026-10-01T10:00:00+02:00"}, + "is_private only": {"is_private": true}, + "identical content": {}, + "identical, current": {"revision": 3}, + } { + code, out = send(http.MethodPut, path, with(changes)) + if code != http.StatusOK || data(out)["revision"] != float64(3) { + t.Errorf("PUT with %s = %d %v, want 200 and revision 3", name, code, out) + } + } + + code, out = send(http.MethodPut, path, with(map[string]any{"tags": []string{"Globalizacja", "Teoria"}})) + if code != http.StatusOK || data(out)["revision"] != float64(4) { + t.Fatalf("PUT changing the tags = %d %v, want revision 4", code, out) + } + note["tags"] = []string{"Teoria", "Globalizacja"} + code, out = send(http.MethodPut, path, note) + if code != http.StatusOK || data(out)["revision"] != float64(4) { + t.Errorf("PUT with the same tags reordered = %d %v, want revision 4", code, out) + } + code, out = send(http.MethodPut, path, with(map[string]any{"course_id": courses[1].ID})) + if code != http.StatusOK || data(out)["revision"] != float64(5) { + t.Fatalf("PUT changing the course = %d %v, want revision 5", code, out) + } + note["course_id"] = courses[1].ID + + detail := obj(student.ok(http.MethodGet, studentPath)["data"]) + if detail["revision"] != float64(5) { + t.Errorf("student detail revision = %v, want 5", detail["revision"]) + } +} diff --git a/models/material.go b/models/material.go index 2683c65..8f3f877 100644 --- a/models/material.go +++ b/models/material.go @@ -36,6 +36,11 @@ type Material struct { AuthorUserID *uint `gorm:"column:author_user_id"` CreatedAt time.Time `gorm:"column:created_at"` UpdatedAt time.Time `gorm:"column:updated_at"` + // Revision goes up by one on every content change (ReviseMaterial). + // It is read-only for GORM, so a full-row Save of a stale in-memory + // model never writes an old revision back; only ReviseMaterial changes + // it. + Revision uint `gorm:"column:revision;->"` Tags []Tag `gorm:"many2many:jz_bm_material_tag;joinForeignKey:material_id;joinReferences:tag_id"` } diff --git a/models/material_revision.go b/models/material_revision.go new file mode 100644 index 0000000..c89c57f --- /dev/null +++ b/models/material_revision.go @@ -0,0 +1,114 @@ +package models + +import ( + "encoding/json" + "slices" + "strings" + + "gorm.io/gorm" +) + +// A material's revision counts changes to its content only (D-03): the +// title, type, body, source, url, course, zjazd (meeting) and tag set. +// Visibility, shares, publication, the author and attached files never bump +// it, so a writer holding an older revision is refused only when the note +// itself changed since. + +// materialFingerprint is the content of a material that its revision +// counts, in a fixed key order. +type materialFingerprint struct { + CourseID *uint `json:"course_id"` + MeetingID *uint `json:"meeting_id"` + Title string `json:"title"` + Type string `json:"type"` + Body *string `json:"body"` + Source *string `json:"source"` + URL *string `json:"url"` + Tags []string `json:"tags"` +} + +// MaterialFingerprint is the comparable form of m's content with the given +// tag slugs: two materials with the same fingerprint have the same content +// under D-03. Blank optional text counts as null, as BeforeSave stores it, +// and the tag order does not matter. published_at, is_private, the author +// and the shares are not inputs. +func MaterialFingerprint(m Material, tagSlugs []string) string { + body, source, url := m.Body, m.Source, m.URL + blank(&body, &source, &url) + tags := slices.Clone(tagSlugs) + if tags == nil { + tags = []string{} + } + slices.Sort(tags) + raw, _ := json.Marshal(materialFingerprint{ + CourseID: m.CourseID, + MeetingID: m.MeetingID, + Title: m.Title, + Type: m.Type, + Body: body, + Source: source, + URL: url, + Tags: tags, + }) + return string(raw) +} + +// materialState reads the content fingerprint and the revision of material +// id through tx; a missing row is gorm.ErrRecordNotFound. +func materialState(tx *gorm.DB, id uint) (string, uint, error) { + db := tx.Session(&gorm.Session{NewDB: true}) + var m Material + res := db.Select("id", "course_id", "meeting_id", "title", "type", "body", "source", "url", "revision"). + Where("id = ?", id).Limit(1).Find(&m) + if res.Error != nil { + return "", 0, res.Error + } + if res.RowsAffected == 0 { + return "", 0, gorm.ErrRecordNotFound + } + var slugs []string + err := db.Table(Tag{}.TableName()). + Joins("JOIN "+MaterialTag{}.TableName()+" ON "+MaterialTag{}.TableName()+".tag_id = "+Tag{}.TableName()+".id"). + Where(MaterialTag{}.TableName()+".material_id = ?", id). + Pluck(Tag{}.TableName()+".slug", &slugs).Error + if err != nil { + return "", 0, err + } + for i := range slugs { + slugs[i] = strings.TrimSpace(slugs[i]) + } + return MaterialFingerprint(m, slugs), m.Revision, nil +} + +// MaterialContent reads the content fingerprint (MaterialFingerprint) of +// material id and its tags through tx. A writer takes it before a write and +// hands it to ReviseMaterial after; a missing row is gorm.ErrRecordNotFound. +func MaterialContent(tx *gorm.DB, id uint) (string, error) { + fp, _, err := materialState(tx, id) + return fp, err +} + +// ReviseMaterial bumps the revision of material id by one when its content +// read through tx differs from before (a MaterialContent taken before the +// write), and answers the revision the row has then and whether it went +// up. Only content counts (D-03): a write that changed only visibility, +// shares, publication or files, or that sent identical content, keeps the +// revision. It is the only writer of the revision column, which GORM treats +// as read-only. +func ReviseMaterial(tx *gorm.DB, id uint, before string) (uint, bool, error) { + after, current, err := materialState(tx, id) + if err != nil { + return 0, false, err + } + if after == before { + return current, false, nil + } + var revision uint + err = tx.Session(&gorm.Session{NewDB: true}). + Raw("UPDATE jz_bm_materials SET revision = revision + 1 WHERE id = ? RETURNING revision", id). + Scan(&revision).Error + if err != nil { + return 0, false, err + } + return revision, true, nil +} diff --git a/models/material_revision_test.go b/models/material_revision_test.go new file mode 100644 index 0000000..19a59f4 --- /dev/null +++ b/models/material_revision_test.go @@ -0,0 +1,63 @@ +package models_test + +import ( + "testing" + "time" + + "git.golem15.com/jakub/sm-bm-plugin/models" +) + +func TestMaterialFingerprint(t *testing.T) { + str := func(s string) *string { return &s } + id := func(n uint) *uint { return &n } + base := func() models.Material { + return models.Material{ + ID: 7, CourseID: id(1), MeetingID: id(2), Title: "Definicje", Type: "notatka", + Body: str("# Robertson\n"), Source: str("wykład"), URL: str("https://example.test/a"), + } + } + tags := []string{"globalizacja", "teoria-2"} + want := models.MaterialFingerprint(base(), tags) + + if got := models.MaterialFingerprint(base(), []string{"teoria-2", "globalizacja"}); got != want { + t.Errorf("tag order changed the fingerprint: %s != %s", got, want) + } + + changes := map[string]func(m *models.Material, tags *[]string){ + "title": func(m *models.Material, _ *[]string) { m.Title = "Inne" }, + "type": func(m *models.Material, _ *[]string) { m.Type = "link" }, + "body": func(m *models.Material, _ *[]string) { m.Body = str("# Robertson\n\nwięcej\n") }, + "source": func(m *models.Material, _ *[]string) { m.Source = str("seminarium") }, + "url": func(m *models.Material, _ *[]string) { m.URL = str("https://example.test/b") }, + "course": func(m *models.Material, _ *[]string) { m.CourseID = id(3) }, + "no course": func(m *models.Material, _ *[]string) { m.CourseID = nil }, + "meeting": func(m *models.Material, _ *[]string) { m.MeetingID = id(4) }, + "tag": func(_ *models.Material, tags *[]string) { *tags = append(*tags, "nowy") }, + "untag": func(_ *models.Material, tags *[]string) { *tags = (*tags)[:1] }, + } + for name, change := range changes { + m, ts := base(), append([]string(nil), tags...) + change(&m, &ts) + if got := models.MaterialFingerprint(m, ts); got == want { + t.Errorf("a %s change kept the fingerprint", name) + } + } + + empty, null := base(), base() + empty.Body, empty.Source, empty.URL = str(""), str(" "), str("") + null.Body, null.Source, null.URL = nil, nil, nil + if models.MaterialFingerprint(empty, tags) != models.MaterialFingerprint(null, tags) { + t.Error("blank and nil optional text give different fingerprints") + } + if models.MaterialFingerprint(base(), nil) != models.MaterialFingerprint(base(), []string{}) { + t.Error("nil and empty tag lists give different fingerprints") + } + + now := time.Now() + other := base() + other.ID, other.PublishedAt, other.IsPrivate, other.AuthorUserID = 8, &now, true, id(9) + other.UpdatedAt, other.Revision = now, 5 + if got := models.MaterialFingerprint(other, tags); got != want { + t.Errorf("published_at, is_private, author, id or revision changed the fingerprint: %s != %s", got, want) + } +} diff --git a/plugin_test.go b/plugin_test.go index d78bd63..6b45b7b 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -79,8 +79,8 @@ func TestPluginActivates(t *testing.T) { if n := len(p.Models()); n != 14 { t.Errorf("Models = %d, want 14", n) } - if n := len(p.Migrations()); n != 21 { - t.Errorf("Migrations = %d, want 21", n) + if n := len(p.Migrations()); n != 22 { + t.Errorf("Migrations = %d, want 22", n) } if !slices.ContainsFunc(p.Commands(), func(c bonfire.Command) bool { return c.Name == "bm:seed" }) { t.Error("bm:seed is not registered") diff --git a/registry.gen.go b/registry.gen.go index 55d9021..e0595f7 100644 --- a/registry.gen.go +++ b/registry.gen.go @@ -53,6 +53,7 @@ func generatedMigrations() []*gormigrate.Migration { updates.AddPrivacyToMaterials(), updates.CreateMaterialFileTexts(), updates.MoveZjazdTagsToMeetings(), + updates.AddRevisionToMaterials(), } } diff --git a/updates/20261109120000_add_revision_to_jz_bm_materials.go b/updates/20261109120000_add_revision_to_jz_bm_materials.go new file mode 100644 index 0000000..b523ca1 --- /dev/null +++ b/updates/20261109120000_add_revision_to_jz_bm_materials.go @@ -0,0 +1,23 @@ +package updates + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// AddRevisionToMaterials returns the +// 20261109120000_add_revision_to_jz_bm_materials gormigrate entry: a material +// carries a revision, a version token that goes up by one on every change to +// its content, so a writer holding an older revision can be refused instead +// of overwriting a newer note. Existing materials start at revision 1. +func AddRevisionToMaterials() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "20261109120000_add_revision_to_jz_bm_materials", + Migrate: func(tx *gorm.DB) error { + return execAll(tx, `ALTER TABLE jz_bm_materials ADD COLUMN revision INTEGER NOT NULL DEFAULT 1`) + }, + Rollback: func(tx *gorm.DB) error { + return execAll(tx, `ALTER TABLE jz_bm_materials DROP COLUMN IF EXISTS revision`) + }, + } +}