Skip to content

Commit 47f78fd

Browse files
authored
Fixes a notes data-loss bug where purgeAllSteps(fileID) ignored its fileID (#4712)
## Summary This fixes a notes data-loss bug where purgeAllSteps(fileID) ignored its fileID argument and deleted step documents for all notes in io.cozy.notes.steps, not only the targeted note. ## Bug story We investigated a production incident where a user lost note content after a reload. - On Tuesday, the note 019d1f37-b201-7d8a-901d-39922cf7c647 accumulated a large number of unsaved note steps. - On Wednesday, Couch/stack logs showed a BulkDeleteDocs for io.cozy.notes.steps batch containing step IDs from multiple different notes. - In the stack code, purgeAllSteps(inst, fileID) was supposed to clear steps for one note after a schema update or .cozy-note overwrite/import. - But the implementation iterated the whole io.cozy.notes.steps database and deleted every step doc it found, regardless of fileID. This means that an action on note B could wipe unsaved steps for note A. The most likely production trigger is: overwrite/import of another existing .cozy-note file
2 parents 778f6e2 + b0f57ce commit 47f78fd

File tree

3 files changed

+81
-6
lines changed

3 files changed

+81
-6
lines changed

model/note/step.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package note
33
import (
44
"encoding/json"
55
"fmt"
6+
"strings"
67
"time"
78

89
"github.com/cozy/cozy-stack/model/instance"
@@ -255,11 +256,15 @@ func purgeOldSteps(inst *instance.Instance, fileID string) {
255256

256257
func purgeAllSteps(inst *instance.Instance, fileID string) {
257258
var docs []couchdb.Doc
259+
prefix := startkey(fileID)
258260
err := couchdb.ForeachDocsWithCustomPagination(inst, consts.NotesSteps, 1000, func(_ string, raw json.RawMessage) error {
259261
var doc Step
260262
if err := json.Unmarshal(raw, &doc); err != nil {
261263
return err
262264
}
265+
if !strings.HasPrefix(doc.ID(), prefix) {
266+
return nil
267+
}
263268
docs = append(docs, doc)
264269
return nil
265270
})

web/data/data_test.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@ import (
55
"fmt"
66
"strings"
77
"testing"
8+
"time"
89

910
"github.com/cozy/cozy-stack/model/instance"
1011
"github.com/cozy/cozy-stack/pkg/config/config"
1112
"github.com/cozy/cozy-stack/pkg/couchdb"
13+
"github.com/cozy/cozy-stack/pkg/couchdb/mango"
1214
"github.com/cozy/cozy-stack/tests/testutils"
1315
"github.com/stretchr/testify/assert"
1416
"github.com/stretchr/testify/require"
@@ -443,16 +445,25 @@ func TestData(t *testing.T) {
443445
_ = getDocForTest(Type, testInstance)
444446
_ = getDocForTest(Type, testInstance)
445447

446-
// Create the index
447-
e.POST("/data/"+Type+"/_index").
448+
// Create and wait for the index used by the selector.
449+
indexName := e.POST("/data/"+Type+"/_index").
448450
WithHeader("Authorization", "Bearer "+token).
449451
WithHeader("Content-Type", "application/json").
450452
WithBytes([]byte(`{ "index": { "fields": ["test"] } }`)).
451453
Expect().Status(200).
452454
JSON().Object().
453-
NotContainsKey("error")
455+
NotContainsKey("error").
456+
Value("name").String().Raw()
457+
458+
require.Eventually(t, func() bool {
459+
var results []couchdb.JSONDoc
460+
err := couchdb.FindDocs(testInstance, Type, &couchdb.FindRequest{
461+
Selector: mango.Equal("test", "value"),
462+
UseIndex: indexName,
463+
}, &results)
464+
return err == nil
465+
}, 5*time.Second, 100*time.Millisecond, "index %q should be ready within 5s", indexName)
454466

455-
// Select with the index
456467
obj := e.POST("/data/"+Type+"/_find").
457468
WithHeader("Authorization", "Bearer "+token).
458469
WithHeader("Content-Type", "application/json").
@@ -472,13 +483,25 @@ func TestData(t *testing.T) {
472483
_ = getDocForTest(Type, testInstance)
473484

474485
// Create the index
475-
e.POST("/data/"+Type+"/_index").
486+
indexName := e.POST("/data/"+Type+"/_index").
476487
WithHeader("Authorization", "Bearer "+token).
477488
WithHeader("Content-Type", "application/json").
478489
WithBytes([]byte(`{ "index": { "fields": ["test"] } }`)).
479490
Expect().Status(200).
480491
JSON().Object().
481-
NotContainsKey("error")
492+
NotContainsKey("error").
493+
Value("name").String().Raw()
494+
495+
// CouchDB builds indexes asynchronously. Wait until the index is ready
496+
// to serve queries before asserting on execution_stats.
497+
require.Eventually(t, func() bool {
498+
var results []couchdb.JSONDoc
499+
err := couchdb.FindDocs(testInstance, Type, &couchdb.FindRequest{
500+
Selector: mango.Equal("test", "value"),
501+
UseIndex: indexName,
502+
}, &results)
503+
return err == nil
504+
}, 5*time.Second, 100*time.Millisecond, "index %q should be ready within 5s", indexName)
482505

483506
// Select with the index
484507
e.POST("/data/"+Type+"/_find").

web/notes/notes_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,53 @@ func TestNotes(t *testing.T) {
500500
version = int64(obj.Path("$.data.attributes.metadata.version").Number().Raw())
501501
})
502502

503+
t.Run("PutSchema only purges steps for the updated note", func(t *testing.T) {
504+
schemaNote, err := note.Create(inst, &note.Document{
505+
Title: "Schema note",
506+
SchemaSpec: note.DefaultSchemaSpecs(),
507+
})
508+
require.NoError(t, err)
509+
510+
stepsNote, err := note.Create(inst, &note.Document{
511+
Title: "Steps note",
512+
SchemaSpec: note.DefaultSchemaSpecs(),
513+
})
514+
require.NoError(t, err)
515+
516+
stepsFile, err := inst.VFS().FileByID(stepsNote.ID())
517+
require.NoError(t, err)
518+
519+
steps := []note.Step{{
520+
"sessionID": "regression",
521+
"stepType": "replace",
522+
"from": 1,
523+
"to": 1,
524+
"slice": map[string]interface{}{
525+
"content": []interface{}{
526+
map[string]interface{}{"type": "text", "text": "A"},
527+
},
528+
},
529+
}}
530+
_, err = note.ApplySteps(inst, stepsFile, "0", steps)
531+
require.NoError(t, err)
532+
533+
existing, err := note.GetSteps(inst, stepsNote.ID(), 0)
534+
require.NoError(t, err)
535+
require.Len(t, existing, 1)
536+
537+
schemaFile, err := inst.VFS().FileByID(schemaNote.ID())
538+
require.NoError(t, err)
539+
540+
_, err = note.UpdateSchema(inst, schemaFile, note.DefaultSchemaSpecs())
541+
require.NoError(t, err)
542+
543+
time.Sleep(1 * time.Second)
544+
545+
remaining, err := note.GetSteps(inst, stepsNote.ID(), 0)
546+
require.NoError(t, err)
547+
require.Len(t, remaining, 1)
548+
})
549+
503550
t.Run("PutTelepointer", func(t *testing.T) {
504551
e := testutils.CreateTestClient(t, ts.URL)
505552

0 commit comments

Comments
 (0)