Skip to content

Commit

Permalink
Fix deletes
Browse files Browse the repository at this point in the history
  • Loading branch information
burdiyan committed Nov 11, 2024
1 parent 2e0ed4b commit fa7b3d9
Show file tree
Hide file tree
Showing 23 changed files with 389 additions and 255 deletions.
2 changes: 1 addition & 1 deletion .github/actions/ci-setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ runs:
- name: "Setup Go"
uses: actions/setup-go@v5
with:
go-version: "1.23"
go-version: "1.23.3"

- name: "Install native packages"
if: inputs.matrix-os == 'ubuntu-20.04'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/generate-docker-images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: "1.23"
go-version: "1.23.3"
- run: go test --count 1 ./backend/...
# Run tests again with the race-detector.
# Using the same job to reuse the build cache.
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.23"
go-version: "1.23.3"
- uses: actions/checkout@v4
- uses: golangci/golangci-lint-action@v6
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: "1.23"
go-version: "1.23.3"
- run: go test --count 1 ./backend/...
# Run tests again with the race-detector.
# Using the same job to reuse the build cache.
Expand Down
10 changes: 5 additions & 5 deletions backend/api/documents/v3alpha/dochistory.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ func (srv *Server) ListDocumentChanges(ctx context.Context, in *documents.ListDo
foundCursor = true
}
var nextCursor string
for _, change := range changes {
cc := change.CID.String()
for c, change := range changes {
cc := c.String()
if !foundCursor {
if cc == cursor.StartFrom {
foundCursor = true
Expand All @@ -98,9 +98,9 @@ func (srv *Server) ListDocumentChanges(ctx context.Context, in *documents.ListDo
}
out.Changes = append(out.Changes, &documents.DocumentChangeInfo{
Id: cc,
Author: change.Data.Signer.String(),
Deps: colx.SliceMap(change.Data.Deps, cid.Cid.String),
CreateTime: timestamppb.New(change.Data.Ts),
Author: change.Signer.String(),
Deps: colx.SliceMap(change.Deps, cid.Cid.String),
CreateTime: timestamppb.New(change.Ts),
})
}

Expand Down
9 changes: 3 additions & 6 deletions backend/api/documents/v3alpha/docmodel/crdt.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (e *docCRDT) Version() Version {
}

// BFTDeps returns a single-use iterator that does breadth-first traversal of the Change DAG deps.
func (e *docCRDT) BFTDeps(start []cid.Cid) (iter.Seq2[int, blob.ChangeRecord], error) {
func (e *docCRDT) BFTDeps(start []cid.Cid) (iter.Seq2[cid.Cid, *blob.Change], error) {
visited := make(map[int]struct{}, len(e.cids))
queue := make([]int, 0, len(e.cids))
var scratch []int
Expand Down Expand Up @@ -254,7 +254,7 @@ func (e *docCRDT) BFTDeps(start []cid.Cid) (iter.Seq2[int, blob.ChangeRecord], e
}
enqueueNodes(scratch)

return func(yield func(int, blob.ChangeRecord) bool) {
return func(yield func(cid.Cid, *blob.Change) bool) {
var i int
for len(queue) > 0 {
c := queue[0]
Expand All @@ -265,10 +265,7 @@ func (e *docCRDT) BFTDeps(start []cid.Cid) (iter.Seq2[int, blob.ChangeRecord], e
visited[c] = struct{}{}

enqueueNodes(e.deps[c])
if !yield(i, blob.ChangeRecord{
CID: e.cids[c],
Data: e.changes[c],
}) {
if !yield(e.cids[c], e.changes[c]) {
break
}

Expand Down
17 changes: 15 additions & 2 deletions backend/api/documents/v3alpha/docmodel/docmodel.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"seed/backend/core"
documents "seed/backend/genproto/documents/v3alpha"
"seed/backend/util/cclock"
"seed/backend/util/maybe"
"slices"
"sort"
"time"
Expand Down Expand Up @@ -45,6 +46,8 @@ type Document struct {

dirtyBlocks map[string]mvRegValue[blob.Block]
dirtyMetadata map[string]mvRegValue[any]

Generation maybe.Value[int64]
}

// originFromCID creates a CRDT origin from the last 8 chars of the hash.
Expand Down Expand Up @@ -155,6 +158,11 @@ func (dm *Document) Checkout(heads []cid.Cid) (*Document, error) {
return doc, nil
}

// Genesis returns the CID of the genesis blob.
func (dm *Document) Genesis() cid.Cid {
return dm.crdt.cids[0]
}

// ApplyChange to the state. Can only do that before any mutations were made.
func (dm *Document) ApplyChange(c cid.Cid, ch *blob.Change) error {
if dm.dirty {
Expand Down Expand Up @@ -340,7 +348,12 @@ func (dm *Document) Ref(kp core.KeyPair, cap cid.Cid) (ref blob.Encoded[*blob.Re
return ref, err
}

return blob.NewRef(kp, genesis, space, path, []cid.Cid{headCID}, cap, head.Ts)
// If we haven't set a generation yet, we use the timestamp of the new change as the generation.
if !dm.Generation.IsSet() {
dm.Generation = maybe.New(head.Ts.UnixMilli())
}

return blob.NewRef(kp, dm.Generation.Value(), genesis, space, path, []cid.Cid{headCID}, cap, head.Ts)
}

func (dm *Document) cleanupPatch() (out blob.ChangeBody) {
Expand Down Expand Up @@ -451,7 +464,7 @@ func (dm *Document) NumChanges() int {
}

// BFTDeps returns a breadth-first traversal iterator for the document change DAG.
func (dm *Document) BFTDeps(start []cid.Cid) (iter.Seq2[int, blob.ChangeRecord], error) {
func (dm *Document) BFTDeps(start []cid.Cid) (iter.Seq2[cid.Cid, *blob.Change], error) {
return dm.crdt.BFTDeps(start)
}

Expand Down
37 changes: 22 additions & 15 deletions backend/api/documents/v3alpha/documents.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"seed/backend/util/cclock"
"seed/backend/util/dqb"
"seed/backend/util/errutil"
"seed/backend/util/maybe"
"seed/backend/util/sqlite"
"seed/backend/util/sqlite/sqlitex"
"time"
Expand Down Expand Up @@ -443,13 +444,22 @@ func (srv *Server) CreateRef(ctx context.Context, in *documents.CreateRefRequest
ts = cclock.New().MustNow()
}

doc, err := srv.loadDocument(ctx, ns, in.Path, nil, false)
if err != nil {
return nil, err
}

if !doc.Generation.IsSet() {
return nil, fmt.Errorf("BUG: generation is not set on a loaded document")
}

var refBlob blob.Encoded[*blob.Ref]

switch in.Target.Target.(type) {
case *documents.RefTarget_Version_:
return nil, status.Errorf(codes.Unimplemented, "version Ref target is not implemented yet")
case *documents.RefTarget_Tombstone_:
refBlob, err = blob.NewRefTombstone(kp, ns, in.Path, ts)
refBlob, err = blob.NewRef(kp, doc.Generation.Value(), doc.Genesis(), ns, in.Path, nil, capc, ts)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -504,7 +514,7 @@ func refToProto(c cid.Cid, ref *blob.Ref) (*documents.Ref, error) {
},
},
}
case !ref.GenesisBlob.Defined() && len(ref.Heads) == 0:
case ref.GenesisBlob.Defined() && len(ref.Heads) == 0:
pb.Target = &documents.RefTarget{
Target: &documents.RefTarget_Tombstone_{
Tombstone: &documents.RefTarget_Tombstone{},
Expand Down Expand Up @@ -550,7 +560,7 @@ func (srv *Server) ensureProfileGenesis(ctx context.Context, kp core.KeyPair) er
return err
}

ebr, err := blob.NewRef(kp, ebc.CID, space, path, []cid.Cid{ebc.CID}, cid.Undef, blob.ZeroUnixTime())
ebr, err := blob.NewRef(kp, 0, ebc.CID, space, path, []cid.Cid{ebc.CID}, cid.Undef, blob.ZeroUnixTime())
if err != nil {
return err
}
Expand Down Expand Up @@ -578,20 +588,17 @@ func (srv *Server) loadDocument(ctx context.Context, account core.Principal, pat
return nil, err
}

// We only check if it's deleted if we are asked about the latest version.
if len(heads) == 0 {
isDeleted, err := srv.idx.IsDeleted(ctx, iri)
if err != nil {
return nil, err
}

if isDeleted {
return nil, status.Errorf(codes.FailedPrecondition, "document is marked as deleted")
changes, check := srv.idx.IterChanges(ctx, iri, heads)
for ch := range changes {
if doc.Generation.IsSet() {
if doc.Generation.Value() != ch.Generation {
err = fmt.Errorf("BUG: IterChanges returned changes with different generations")
break
}
} else {
doc.Generation = maybe.New(ch.Generation)
}
}

changes, check := srv.idx.IterChangesFromHeads(ctx, iri, heads)
for _, ch := range changes {
if aerr := doc.ApplyChange(ch.CID, ch.Data); aerr != nil {
err = errors.Join(err, aerr)
break
Expand Down
93 changes: 92 additions & 1 deletion backend/api/documents/v3alpha/documents_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package documents

import (
"cmp"
"context"
"seed/backend/blob"
"seed/backend/core"
Expand All @@ -10,6 +11,7 @@ import (
"seed/backend/storage"
"seed/backend/testutil"
"seed/backend/util/must"
"slices"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -623,8 +625,97 @@ func TestTombstoneRef(t *testing.T) {
require.NoError(t, err)
testutil.StructsEqual(republished, got).Compare(t, "getting latest must return the second generation")
}

// Check new generation works with latest.
{
got, err := alice.GetDocument(ctx, &documents.GetDocumentRequest{
Account: republished.Account,
Path: republished.Path,
Version: republished.Version,
})
require.NoError(t, err)
testutil.StructsEqual(republished, got).Compare(t, "getting republished with version must work")
}

// Check get with version works for previous generation.
// Check list responses.
{
got, err := alice.GetDocument(ctx, &documents.GetDocumentRequest{
Account: doc.Account,
Path: doc.Path,
Version: doc.Version,
})
require.NoError(t, err)
testutil.StructsEqual(doc, got).Compare(t, "getting republished with version must work")
}

// Check list contains latest.
{
list, err := alice.ListDocuments(ctx, &documents.ListDocumentsRequest{
Account: alice.me.Account.Principal().String(),
PageSize: 100,
})
require.NoError(t, err)
require.Len(t, list.Documents, 2, "list must contain the home doc and the second generation of the other doc")

want := &documents.ListDocumentsResponse{
Documents: []*documents.DocumentListItem{
DocumentToListItem(home),
DocumentToListItem(republished),
},
}

slices.SortFunc(want.Documents, func(a, b *documents.DocumentListItem) int { return cmp.Compare(a.Version, b.Version) })
slices.SortFunc(list.Documents, func(a, b *documents.DocumentListItem) int { return cmp.Compare(a.Version, b.Version) })

testutil.StructsEqual(want, list).Compare(t, "listing must contain home doc and republished doc")
}

// Changes with no base version must fail when there's a live document.
{
_, err := alice.CreateDocumentChange(ctx, &documents.CreateDocumentChangeRequest{
SigningKeyName: "main",
Account: alice.me.Account.Principal().String(),
Path: "/hello",
Changes: []*documents.DocumentChange{
{Op: &documents.DocumentChange_SetMetadata_{
SetMetadata: &documents.DocumentChange_SetMetadata{Key: "title", Value: "Hello World Republished Must Fail"},
}},
},
})
require.Error(t, err, "changes with no base version must fail")
}

// Changes with base version of the old generation must not overwrite the newer generation
{
got, err := alice.CreateDocumentChange(ctx, &documents.CreateDocumentChangeRequest{
SigningKeyName: "main",
Account: doc.Account,
Path: doc.Path,
Changes: []*documents.DocumentChange{
{Op: &documents.DocumentChange_SetMetadata_{
SetMetadata: &documents.DocumentChange_SetMetadata{Key: "title", Value: "Hello World Revived Old Generation"},
}},
},
BaseVersion: doc.Version,
})
require.NoError(t, err)

gotv, err := alice.GetDocument(ctx, &documents.GetDocumentRequest{
Account: got.Account,
Path: doc.Path,
Version: got.Version,
})
require.NoError(t, err)
testutil.StructsEqual(got, gotv).Compare(t, "getting version of the old generation must take into account new changes")

gotLatest, err := alice.GetDocument(ctx, &documents.GetDocumentRequest{
Account: doc.Account,
Path: doc.Path,
})
require.NoError(t, err)

testutil.StructsEqual(republished, gotLatest).Compare(t, "changes with base version of the old generation must not overwrite the newer generation")
}
}

type testServer struct {
Expand Down
Loading

0 comments on commit fa7b3d9

Please sign in to comment.