diff --git a/acltree/acltree.go b/acltree/acltree.go index 2a671d53..1e0e3e00 100644 --- a/acltree/acltree.go +++ b/acltree/acltree.go @@ -84,38 +84,54 @@ func BuildACLTree( if err != nil { return nil, err } + aclTree.removeOrphans() return aclTree, nil } // TODO: this is not used for now, in future we should think about not making full tree rebuild -func (a *aclTree) rebuildFromTree(validateSnapshot bool) (err error) { - if validateSnapshot { - err = a.snapshotValidator.Init(a.aclTreeFromStart) - if err != nil { - return err - } +//func (a *aclTree) rebuildFromTree(validateSnapshot bool) (err error) { +// if validateSnapshot { +// err = a.snapshotValidator.Init(a.aclTreeFromStart) +// if err != nil { +// return err +// } +// +// valid, err := a.snapshotValidator.ValidateSnapshot(a.fullTree.root) +// if err != nil { +// return err +// } +// if !valid { +// return a.rebuildFromThread(true) +// } +// } +// +// err = a.aclStateBuilder.Init(a.fullTree) +// if err != nil { +// return err +// } +// +// a.aclState, err = a.aclStateBuilder.Build() +// if err != nil { +// return err +// } +// +// return nil +//} - valid, err := a.snapshotValidator.ValidateSnapshot(a.fullTree.root) - if err != nil { - return err +func (a *aclTree) removeOrphans() { + // removing attached or invalid orphans + var toRemove []string + + for _, orphan := range a.thread.Orphans() { + if _, exists := a.fullTree.attached[orphan]; exists { + toRemove = append(toRemove, orphan) } - if !valid { - return a.rebuildFromThread(true) + if _, exists := a.fullTree.invalidChanges[orphan]; exists { + toRemove = append(toRemove, orphan) } } - - err = a.aclStateBuilder.Init(a.fullTree) - if err != nil { - return err - } - - a.aclState, err = a.aclStateBuilder.Build() - if err != nil { - return err - } - - return nil + a.thread.RemoveOrphans(toRemove...) } func (a *aclTree) rebuildFromThread(fromStart bool) error { @@ -213,18 +229,8 @@ func (a *aclTree) AddChanges(changes ...*Change) (AddResult, error) { if err != nil { return } - // removing attached or invalid orphans - var toRemove []string - - for _, orphan := range a.thread.Orphans() { - if _, exists := a.fullTree.attached[orphan]; exists { - toRemove = append(toRemove, orphan) - } - if _, exists := a.fullTree.invalidChanges[orphan]; exists { - toRemove = append(toRemove, orphan) - } - } - a.thread.RemoveOrphans(toRemove...) + a.removeOrphans() + a.thread.SetHeads(a.fullTree.Heads()) switch mode { case Append: a.updateListener.Update(a) @@ -265,6 +271,8 @@ func (a *aclTree) AddChanges(changes ...*Change) (AddResult, error) { Summary: AddResultSummaryRebuild, }, nil default: + // just rebuilding the state from start without reloading everything from thread + // as an optimization we could've started from current heads, but I didn't implement that a.aclState, err = a.aclStateBuilder.Build() if err != nil { return AddResult{}, err diff --git a/acltree/acltree_test.go b/acltree/acltree_test.go index 7ca67c5c..163c47e2 100644 --- a/acltree/acltree_test.go +++ b/acltree/acltree_test.go @@ -33,7 +33,6 @@ func TestACLTree_UserJoinBuild(t *testing.T) { t.Fatalf("should Build acl ACLState without err: %v", err) } aclState := tree.ACLState() - //fmt.Println(ctx.Tree.Graph()) aId := keychain.GeneratedIdentities["A"] bId := keychain.GeneratedIdentities["B"] cId := keychain.GeneratedIdentities["C"] @@ -51,6 +50,55 @@ func TestACLTree_UserJoinBuild(t *testing.T) { assert.Equal(t, changeIds, []string{"A.1.1", "A.1.2", "B.1.1", "B.1.2"}) } +func TestACLTree_UserJoinUpdate_Append(t *testing.T) { + thr, err := threadbuilder.NewThreadBuilderWithTestName("userjoinexampleupdate.yml") + if err != nil { + t.Fatal(err) + } + keychain := thr.GetKeychain() + accountData := &account.AccountData{ + Identity: keychain.GetIdentity("A"), + SignKey: keychain.SigningKeys["A"], + EncKey: keychain.EncryptionKeys["A"], + } + listener := &mockListener{} + tree, err := BuildACLTree(thr, accountData, listener) + if err != nil { + t.Fatalf("should Build acl ACLState without err: %v", err) + } + rawChanges := thr.GetUpdates("append") + var changes []*Change + for _, ch := range rawChanges { + newCh, err := NewFromRawChange(ch) + if err != nil { + t.Fatalf("should be able to create change from raw: %v", err) + } + changes = append(changes, newCh) + } + + res, err := tree.AddChanges(changes...) + assert.Equal(t, res.Summary, AddResultSummaryAppend) + + aclState := tree.ACLState() + aId := keychain.GeneratedIdentities["A"] + bId := keychain.GeneratedIdentities["B"] + cId := keychain.GeneratedIdentities["C"] + dId := keychain.GeneratedIdentities["D"] + + assert.Equal(t, aclState.identity, aId) + assert.Equal(t, aclState.userStates[aId].Permissions, pb.ACLChange_Admin) + assert.Equal(t, aclState.userStates[bId].Permissions, pb.ACLChange_Writer) + assert.Equal(t, aclState.userStates[cId].Permissions, pb.ACLChange_Reader) + assert.Equal(t, aclState.userStates[dId].Permissions, pb.ACLChange_Writer) + + var changeIds []string + tree.Iterate(func(c *Change) (isContinue bool) { + changeIds = append(changeIds, c.Id) + return true + }) + assert.Equal(t, changeIds, []string{"A.1.1", "A.1.2", "B.1.1", "B.1.2", "B.1.3", "A.1.4"}) +} + func TestACLTree_UserRemoveBuild(t *testing.T) { thr, err := threadbuilder.NewThreadBuilderWithTestName("userremoveexample.yml") if err != nil { @@ -68,7 +116,6 @@ func TestACLTree_UserRemoveBuild(t *testing.T) { t.Fatalf("should Build acl ACLState without err: %v", err) } aclState := tree.ACLState() - //fmt.Println(ctx.Tree.Graph()) aId := keychain.GeneratedIdentities["A"] assert.Equal(t, aclState.identity, aId) @@ -99,7 +146,6 @@ func TestACLTree_UserRemoveBeforeBuild(t *testing.T) { t.Fatalf("should Build acl ACLState without err: %v", err) } aclState := tree.ACLState() - //fmt.Println(ctx.Tree.Graph()) for _, s := range []string{"A", "C", "E"} { assert.Equal(t, aclState.userStates[keychain.GetIdentity(s)].Permissions, pb.ACLChange_Admin) } @@ -131,7 +177,6 @@ func TestACLTree_InvalidSnapshotBuild(t *testing.T) { t.Fatalf("should Build acl ACLState without err: %v", err) } aclState := tree.ACLState() - //fmt.Println(ctx.Tree.Graph()) for _, s := range []string{"A", "B", "C", "D", "E", "F"} { assert.Equal(t, aclState.userStates[keychain.GetIdentity(s)].Permissions, pb.ACLChange_Admin) } @@ -162,7 +207,6 @@ func TestACLTree_ValidSnapshotBuild(t *testing.T) { t.Fatalf("should Build acl ACLState without err: %v", err) } aclState := tree.ACLState() - //fmt.Println(ctx.Tree.Graph()) for _, s := range []string{"A", "B", "C", "D", "E", "F"} { assert.Equal(t, aclState.userStates[keychain.GetIdentity(s)].Permissions, pb.ACLChange_Admin) } diff --git a/testutils/yamltests/userjoinexampleupdate.yml b/testutils/yamltests/userjoinexampleupdate.yml index cebfaf4a..0dca174a 100644 --- a/testutils/yamltests/userjoinexampleupdate.yml +++ b/testutils/yamltests/userjoinexampleupdate.yml @@ -123,9 +123,6 @@ updates: permission: writer encryptionKey: key.Enc.D encryptedReadKeys: [ key.Read.1 ] - changes: - - textAppend: - text: "second" readKey: key.Read.1 graph: - id: B.1.3