From 257b9cba8eccb06467547d15eac85843ea39fd83 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Mon, 5 Sep 2022 12:35:54 +0200 Subject: [PATCH] Fixing several copy issues and other stuff in doc tree --- pkg/acl/tree/change.go | 19 ++++--- pkg/acl/tree/doctree.go | 99 ++++++++++++++++++++----------------- pkg/acl/tree/treebuilder.go | 4 +- 3 files changed, 65 insertions(+), 57 deletions(-) diff --git a/pkg/acl/tree/change.go b/pkg/acl/tree/change.go index 2e1d3dce..05413f08 100644 --- a/pkg/acl/tree/change.go +++ b/pkg/acl/tree/change.go @@ -1,6 +1,7 @@ package tree import ( + "errors" "fmt" "github.com/anytypeio/go-anytype-infrastructure-experiments/pkg/acl/aclchanges/aclpb" "github.com/gogo/protobuf/proto" @@ -8,6 +9,8 @@ import ( "github.com/anytypeio/go-anytype-infrastructure-experiments/util/keys/symmetric" ) +var ErrIncorrectSignature = errors.New("change has incorrect signature") + type ChangeContent struct { ChangesData proto.Marshaler ACLData *aclpb.ACLChangeACLData @@ -21,8 +24,8 @@ type Change struct { Id string SnapshotId string IsSnapshot bool - DecryptedChange []byte // TODO: check if we need it - ParsedModel interface{} + DecryptedChange []byte // TODO: check if we need it + ParsedModel interface{} // TODO: check if we need it // iterator helpers visited bool @@ -57,8 +60,7 @@ func NewChangeFromRaw(rawChange *aclpb.RawChange) (*Change, error) { return nil, err } - ch := NewChange(rawChange.Id, unmarshalled) - ch.Sign = rawChange.Signature + ch := NewChange(rawChange.Id, unmarshalled, rawChange.Signature) return ch, nil } @@ -66,7 +68,7 @@ func NewVerifiedChangeFromRaw( rawChange *aclpb.RawChange, kch *keychain) (*Change, error) { unmarshalled := &aclpb.Change{} - err := proto.Unmarshal(rawChange.Payload, unmarshalled) + ch, err := NewChangeFromRaw(rawChange) if err != nil { return nil, err } @@ -81,13 +83,13 @@ func NewVerifiedChangeFromRaw( return nil, err } if !res { - return nil, fmt.Errorf("change has incorrect signature") + return nil, ErrIncorrectSignature } - return NewChange(rawChange.Id, unmarshalled), nil + return ch, nil } -func NewChange(id string, ch *aclpb.Change) *Change { +func NewChange(id string, ch *aclpb.Change, signature []byte) *Change { return &Change{ Next: nil, PreviousIds: ch.TreeHeadIds, @@ -95,6 +97,7 @@ func NewChange(id string, ch *aclpb.Change) *Change { Content: ch, SnapshotId: ch.SnapshotBaseId, IsSnapshot: ch.IsSnapshot, + Sign: signature, } } diff --git a/pkg/acl/tree/doctree.go b/pkg/acl/tree/doctree.go index 91bbe925..57f96e4d 100644 --- a/pkg/acl/tree/doctree.go +++ b/pkg/acl/tree/doctree.go @@ -25,8 +25,10 @@ type RWLocker interface { RUnlock() } -var ErrHasInvalidChanges = errors.New("the change is invalid") -var ErrNoCommonSnapshot = errors.New("trees doesn't have a common snapshot") +var ( + ErrHasInvalidChanges = errors.New("the change is invalid") + ErrNoCommonSnapshot = errors.New("trees doesn't have a common snapshot") +) type AddResultSummary int @@ -40,7 +42,7 @@ type AddResult struct { OldHeads []string Heads []string Added []*aclpb.RawChange - // TODO: add summary for changes + Summary AddResultSummary } @@ -64,10 +66,12 @@ type docTree struct { kch *keychain // buffers - difSnapshotBuf []*aclpb.RawChange - tmpChangesBuf []*Change - newSnapshots []*Change - notSeenIdxBuf []int + difSnapshotBuf []*aclpb.RawChange + tmpChangesBuf []*Change + newSnapshotsBuf []*Change + notSeenIdxBuf []int + + snapshotPath []string sync.RWMutex } @@ -128,7 +132,7 @@ func (d *docTree) rebuildFromStorage(aclList list.ACLList, newChanges []*Change) d.tree, err = d.treeBuilder.Build(newChanges) if err != nil { - return err + return } // during building the tree we may have marked some changes as possible roots, @@ -198,9 +202,8 @@ func (d *docTree) AddContent(ctx context.Context, aclList list.ACLList, content return nil, err } - docChange := NewChange(id, aclChange) + docChange := NewChange(id, aclChange, signature) docChange.ParsedModel = content - docChange.Sign = signature if content.IsSnapshot { // clearing tree, because we already fixed everything in the last snapshot @@ -269,10 +272,11 @@ func (d *docTree) addRawChanges(ctx context.Context, aclList list.ACLList, rawCh d.tmpChangesBuf = d.tmpChangesBuf[:0] d.notSeenIdxBuf = d.notSeenIdxBuf[:0] d.difSnapshotBuf = d.difSnapshotBuf[:0] - d.newSnapshots = d.newSnapshots[:0] + d.newSnapshotsBuf = d.newSnapshotsBuf[:0] - prevHeads := d.tree.Heads() - // TODO: if we can use new snapshot -> update tree, check if some snapshot, dfsPrev? + // this will be returned to client so we shouldn't use buffer here + prevHeadsCopy := make([]string, 0, len(d.tree.Heads())) + copy(prevHeadsCopy, d.tree.Heads()) // filtering changes, verifying and unmarshalling them for idx, ch := range rawChanges { @@ -287,7 +291,7 @@ func (d *docTree) addRawChanges(ctx context.Context, aclList list.ACLList, rawCh } if change.IsSnapshot { - d.newSnapshots = append(d.newSnapshots, change) + d.newSnapshotsBuf = append(d.newSnapshotsBuf, change) } d.tmpChangesBuf = append(d.tmpChangesBuf, change) d.notSeenIdxBuf = append(d.notSeenIdxBuf, idx) @@ -296,13 +300,19 @@ func (d *docTree) addRawChanges(ctx context.Context, aclList list.ACLList, rawCh // if no new changes, then returning if len(d.notSeenIdxBuf) == 0 { addResult = AddResult{ - OldHeads: prevHeads, - Heads: prevHeads, + OldHeads: prevHeadsCopy, + Heads: prevHeadsCopy, Summary: AddResultSummaryNothing, } return } + headsCopy := func() []string { + newHeads := make([]string, 0, len(d.tree.Heads())) + copy(newHeads, d.tree.Heads()) + return newHeads + } + // returns changes that we added to the tree getAddedChanges := func() []*aclpb.RawChange { var added []*aclpb.RawChange @@ -330,7 +340,7 @@ func (d *docTree) addRawChanges(ctx context.Context, aclList list.ACLList, rawCh if ch.SnapshotId == d.tree.RootId() { return false } - for _, sn := range d.newSnapshots { + for _, sn := range d.newSnapshotsBuf { // if change refers to newly received snapshot if ch.SnapshotId == sn.Id { return false @@ -350,8 +360,8 @@ func (d *docTree) addRawChanges(ctx context.Context, aclList list.ACLList, rawCh } addResult = AddResult{ - OldHeads: prevHeads, - Heads: d.tree.Heads(), + OldHeads: prevHeadsCopy, + Heads: headsCopy(), Added: getAddedChanges(), Summary: AddResultSummaryRebuild, } @@ -364,8 +374,8 @@ func (d *docTree) addRawChanges(ctx context.Context, aclList list.ACLList, rawCh switch mode { case Nothing: addResult = AddResult{ - OldHeads: prevHeads, - Heads: prevHeads, + OldHeads: prevHeadsCopy, + Heads: prevHeadsCopy, Summary: AddResultSummaryNothing, } return @@ -381,8 +391,8 @@ func (d *docTree) addRawChanges(ctx context.Context, aclList list.ACLList, rawCh } addResult = AddResult{ - OldHeads: prevHeads, - Heads: d.tree.Heads(), + OldHeads: prevHeadsCopy, + Heads: headsCopy(), Added: getAddedChanges(), Summary: AddResultSummaryAppend, } @@ -417,7 +427,9 @@ func (d *docTree) Close() error { } func (d *docTree) SnapshotPath() []string { - // TODO: think about caching this + if d.snapshotPathIsActual() { + return d.snapshotPath + } var path []string // TODO: think that the user may have not all of the snapshots locally @@ -430,20 +442,22 @@ func (d *docTree) SnapshotPath() []string { path = append(path, currentSnapshotId) currentSnapshotId = sn.SnapshotId } + d.snapshotPath = path + return path } func (d *docTree) ChangesAfterCommonSnapshot(theirPath []string) ([]*aclpb.RawChange, error) { var ( - isNewDocument = len(theirPath) == 0 - ourPath = d.SnapshotPath() + needFullDocument = len(theirPath) == 0 + ourPath = d.SnapshotPath() // by default returning everything we have commonSnapshot = ourPath[len(ourPath)-1] err error ) // if this is non-empty request - if !isNewDocument { + if !needFullDocument { commonSnapshot, err = commonSnapshotForTwoPaths(ourPath, theirPath) if err != nil { return nil, err @@ -457,17 +471,13 @@ func (d *docTree) ChangesAfterCommonSnapshot(theirPath []string) ([]*aclpb.RawCh Debug("getting all changes from common snapshot") if commonSnapshot == d.tree.RootId() { - return d.getChangesFromTree(isNewDocument) + return d.getChangesFromTree() } else { - return d.getChangesFromDB(commonSnapshot, isNewDocument) + return d.getChangesFromDB(commonSnapshot, needFullDocument) } } -func (d *docTree) getChangesFromTree(isNewDocument bool) (marshalledChanges []*aclpb.RawChange, err error) { - if !isNewDocument { - // ignoring root change - d.tree.Root().visited = true - } +func (d *docTree) getChangesFromTree() (rawChanges []*aclpb.RawChange, err error) { d.tree.dfsPrev(d.tree.HeadsChanges(), func(ch *Change) bool { var marshalled []byte marshalled, err = ch.Content.Marshal() @@ -479,18 +489,14 @@ func (d *docTree) getChangesFromTree(isNewDocument bool) (marshalledChanges []*a Signature: ch.Signature(), Id: ch.Id, } - marshalledChanges = append(marshalledChanges, raw) + rawChanges = append(rawChanges, raw) return true }, func(changes []*Change) {}) - if err != nil { - return nil, err - } return } -func (d *docTree) getChangesFromDB(commonSnapshot string, isNewDocument bool) (marshalledChanges []*aclpb.RawChange, err error) { - var rawChanges []*aclpb.RawChange +func (d *docTree) getChangesFromDB(commonSnapshot string, needStartSnapshot bool) (rawChanges []*aclpb.RawChange, err error) { load := func(id string) (*Change, error) { raw, err := d.treeStorage.GetRawChange(context.Background(), id) if err != nil { @@ -508,18 +514,19 @@ func (d *docTree) getChangesFromDB(commonSnapshot string, isNewDocument bool) (m _, err = d.treeBuilder.dfs(d.tree.Heads(), commonSnapshot, load) if err != nil { - return nil, err + return } - if isNewDocument { + if needStartSnapshot { // adding snapshot to raw changes _, err = load(commonSnapshot) - if err != nil { - return nil, err - } } - return rawChanges, nil + return +} + +func (d *docTree) snapshotPathIsActual() bool { + return len(d.snapshotPath) != 0 && d.snapshotPath[len(d.snapshotPath)-1] == d.tree.RootId() } func (d *docTree) DebugDump() (string, error) { diff --git a/pkg/acl/tree/treebuilder.go b/pkg/acl/tree/treebuilder.go index af229cd5..b5b2dd9c 100644 --- a/pkg/acl/tree/treebuilder.go +++ b/pkg/acl/tree/treebuilder.go @@ -6,8 +6,6 @@ import ( "fmt" "github.com/anytypeio/go-anytype-infrastructure-experiments/app/logger" "github.com/anytypeio/go-anytype-infrastructure-experiments/pkg/acl/storage" - "github.com/anytypeio/go-anytype-infrastructure-experiments/util/keys" - "github.com/anytypeio/go-anytype-infrastructure-experiments/util/keys/asymmetric/signingkey" "github.com/anytypeio/go-anytype-infrastructure-experiments/util/slice" "go.uber.org/zap" "time" @@ -133,7 +131,7 @@ func (tb *treeBuilder) loadChange(id string) (ch *Change, err error) { return nil, err } - ch, err = NewVerifiedChangeFromRaw(change, tb.identityKeys, tb.signingPubKeyDecoder) + ch, err = NewVerifiedChangeFromRaw(change, tb.kch) if err != nil { return nil, err }