Skip to content

Commit

Permalink
Ensure proper deletion of documents in MemDB (#920)
Browse files Browse the repository at this point in the history
In Dashboard, there was an issue where re-creating and deleting a
previously deleted document did not remove the document as expected.
This issue occurred because the already deleted document was not
properly filtered out and returned again upon re-deletion.

This commit fixes this issue by adding a filter to prevent previously
deleted documents from being returned during document retrieval for
deletion.
  • Loading branch information
hackerwins authored Jul 5, 2024
1 parent 1d83899 commit 28ef053
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 46 deletions.
84 changes: 38 additions & 46 deletions server/backend/database/memory/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,48 +683,17 @@ func (d *DB) FindDocInfoByKeyAndOwner(
txn := d.db.Txn(true)
defer txn.Abort()

// TODO(hackerwins): Removed documents should be filtered out by the query, but
// somehow it does not work. This is a workaround.
// val, err := txn.First(tblDocuments, "project_id_key_removed_at", projectID.String(), key.String(), gotime.Time{})
iter, err := txn.Get(
tblDocuments,
"project_id_key_removed_at",
clientRefKey.ProjectID.String(),
key.String(),
gotime.Time{},
)
if err != nil {
return nil, fmt.Errorf("find document by key: %w", err)
}
var raw interface{}
for val := iter.Next(); val != nil; val = iter.Next() {
if val != nil && val.(*database.DocInfo).RemovedAt.IsZero() {
raw = val
}
}

info, err := d.findDocInfoByKey(txn, clientRefKey.ProjectID, key)
if err != nil {
return nil, fmt.Errorf("find document by key: %w", err)
return info, err
}
if !createDocIfNotExist && raw == nil {
raw, err = txn.First(
tblDocuments,
"project_id_key",
clientRefKey.ProjectID.String(),
key.String(),
)
if err != nil {
return nil, fmt.Errorf("find document by key: %w", err)
}
if raw == nil {
return nil, fmt.Errorf("%s: %w", key, database.ErrDocumentNotFound)
}
if !createDocIfNotExist && info == nil {
return nil, fmt.Errorf("%s: %w", key, database.ErrDocumentNotFound)
}

now := gotime.Now()
var docInfo *database.DocInfo
if raw == nil {
docInfo = &database.DocInfo{
if info == nil {
now := gotime.Now()
info = &database.DocInfo{
ID: newID(),
ProjectID: clientRefKey.ProjectID,
Key: key,
Expand All @@ -733,15 +702,38 @@ func (d *DB) FindDocInfoByKeyAndOwner(
CreatedAt: now,
AccessedAt: now,
}
if err := txn.Insert(tblDocuments, docInfo); err != nil {
if err := txn.Insert(tblDocuments, info); err != nil {
return nil, fmt.Errorf("create document: %w", err)
}
txn.Commit()
} else {
docInfo = raw.(*database.DocInfo)
}

return docInfo.DeepCopy(), nil
return info.DeepCopy(), nil
}

// findDocInfoByKey finds the document of the given key.
func (d *DB) findDocInfoByKey(txn *memdb.Txn, projectID types.ID, key key.Key) (*database.DocInfo, error) {
// TODO(hackerwins): Removed documents should be filtered out by the query, but
// somehow it does not work. This is a workaround.
// val, err := txn.First(tblDocuments, "project_id_key_removed_at", projectID.String(), key.String(), gotime.Time{})
iter, err := txn.Get(
tblDocuments,
"project_id_key_removed_at",
projectID.String(),
key.String(),
gotime.Time{},
)
if err != nil {
return nil, fmt.Errorf("find doc info by key: %w", err)
}
var docInfo *database.DocInfo
for val := iter.Next(); val != nil; val = iter.Next() {
if info := val.(*database.DocInfo); info.RemovedAt.IsZero() {
docInfo = info
}
}

return docInfo, nil
}

// FindDocInfoByKey finds the document of the given key.
Expand All @@ -753,15 +745,15 @@ func (d *DB) FindDocInfoByKey(
txn := d.db.Txn(false)
defer txn.Abort()

raw, err := txn.First(tblDocuments, "project_id_key_removed_at", projectID.String(), key.String(), gotime.Time{})
info, err := d.findDocInfoByKey(txn, projectID, key)
if err != nil {
return nil, fmt.Errorf("find document by key: %w", err)
return nil, fmt.Errorf("find doc info by key: %w", err)
}
if raw == nil {
if info == nil {
return nil, fmt.Errorf("%s: %w", key, database.ErrDocumentNotFound)
}

return raw.(*database.DocInfo).DeepCopy(), nil
return info.DeepCopy(), nil
}

// FindDocInfoByRefKey finds a docInfo of the given refKey.
Expand Down
20 changes: 20 additions & 0 deletions server/backend/database/testcases/testcases.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,26 @@ func RunFindDocInfoTest(
assert.NoError(t, err)
assert.Equal(t, docKey, docInfo.Key)
})

t.Run("find docInfo by key test", func(t *testing.T) {
ctx := context.Background()
clientInfo, err := db.ActivateClient(ctx, projectID, t.Name())
assert.NoError(t, err)

// 01. Create a document
docKey := helper.TestDocKey(t)
info, err := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), docKey, true)
assert.NoError(t, err)
assert.Equal(t, docKey, info.Key)

// 02. Remove the document
err = db.CreateChangeInfos(ctx, projectID, info, 0, []*change.Change{}, true)
assert.NoError(t, err)

// 03. Find the document
info, err = db.FindDocInfoByKey(ctx, projectID, docKey)
assert.ErrorIs(t, err, database.ErrDocumentNotFound)
})
}

// RunFindProjectInfoBySecretKeyTest runs the FindProjectInfoBySecretKey test for the given db.
Expand Down

0 comments on commit 28ef053

Please sign in to comment.