From 2bcbaf805d360f6d4c62d310ddfa0b4330d98639 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 16:10:00 -0400 Subject: [PATCH 01/13] PR(TEST): Document the bug --- tests/integration/acp/p2p/create_test.go | 133 +++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 tests/integration/acp/p2p/create_test.go diff --git a/tests/integration/acp/p2p/create_test.go b/tests/integration/acp/p2p/create_test.go new file mode 100644 index 0000000000..85b5c84e81 --- /dev/null +++ b/tests/integration/acp/p2p/create_test.go @@ -0,0 +1,133 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package test_acp_p2p + +import ( + "fmt" + "testing" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" + "github.com/sourcenetwork/immutable" +) + +func TestACP_P2PCreatePrivateDocumentsOnDifferentNodes_SourceHubACP(t *testing.T) { + expectedPolicyID := "fc56b7509c20ac8ce682b3b9b4fdaad868a9c70dda6ec16720298be64f16e9a4" + + test := testUtils.TestCase{ + + Description: "Test acp, p2p create private documents on different nodes, with source-hub", + + SupportedACPTypes: immutable.Some( + []testUtils.ACPType{ + testUtils.SourceHubACPType, + }, + ), + + Actions: []any{ + testUtils.RandomNetworkingConfig(), + + testUtils.RandomNetworkingConfig(), + + testUtils.AddPolicy{ + + Identity: immutable.Some(1), + + Policy: ` + name: Test Policy + + description: A Policy + + actor: + name: actor + + resources: + users: + permissions: + read: + expr: owner + reader + writer + + write: + expr: owner + writer + + nothing: + expr: dummy + + relations: + owner: + types: + - actor + + reader: + types: + - actor + + writer: + types: + - actor + + admin: + manages: + - reader + types: + - actor + + dummy: + types: + - actor + `, + + ExpectedPolicyID: expectedPolicyID, + }, + + testUtils.SchemaUpdate{ + Schema: fmt.Sprintf(` + type Users @policy( + id: "%s", + resource: "users" + ) { + name: String + age: Int + } + `, + expectedPolicyID, + ), + }, + + testUtils.CreateDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(0), + + CollectionID: 0, + + DocMap: map[string]any{ + "name": "Shahzad", + }, + }, + + testUtils.CreateDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(1), + + CollectionID: 0, + + DocMap: map[string]any{ + "name": "Shahzad Lone", + }, + + ExpectedError: "403: forbidden", // TODO: FIX THIS BUG + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} From 64b30fc392b5faf4688a3afe87cf69abba7a7d59 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 17:07:25 -0400 Subject: [PATCH 02/13] PR(TEST): Add warning & fix the getIndexes testing setup --- tests/integration/utils.go | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 85ba2f870d..10bb86c2c6 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -572,6 +572,12 @@ func getNodes(nodeID immutable.Option[int], nodes []clients.Client) []clients.Cl // // If nodeID has a value it will return collections for that node only, otherwise all collections across all // nodes will be returned. +// +// WARNING: +// The caller must not assume the returned collections are in order of the node index if the specified +// index is greater than 0. For example if requesting collections with nodeID=2 then the resulting output +// will contain only one element (at index 0) that will be the collections of the respective node, the +// caller might accidentally assume that these collections belong to node 0. func getNodeCollections(nodeID immutable.Option[int], collections [][]client.Collection) [][]client.Collection { if !nodeID.HasValue() { return collections @@ -931,11 +937,12 @@ func getIndexes( } var expectedErrorRaised bool - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, collections := range getNodeCollections(action.NodeID, s.collections) { - err := withRetry( - actionNodes, - nodeID, + + if action.NodeID.HasValue() { + nodeID := action.NodeID.Value() + collections := s.collections[nodeID] + err := withRetryOnNode( + s.nodes[nodeID], func() error { actualIndexes, err := collections[action.CollectionID].GetIndexes(s.ctx) if err != nil { @@ -950,6 +957,26 @@ func getIndexes( ) expectedErrorRaised = expectedErrorRaised || AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } else { + for nodeID, collections := range s.collections { + err := withRetryOnNode( + s.nodes[nodeID], + func() error { + actualIndexes, err := collections[action.CollectionID].GetIndexes(s.ctx) + if err != nil { + return err + } + + assertIndexesListsEqual(action.ExpectedIndexes, + actualIndexes, s.t, s.testCase.Description) + + return nil + }, + ) + expectedErrorRaised = expectedErrorRaised || + AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } + } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) From a3a89eecba856021d41fd1676ac3473677058c06 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 18:02:45 -0400 Subject: [PATCH 03/13] PR(TEST): Fix the createDoc testing setup --- tests/integration/utils.go | 59 ++++++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 10bb86c2c6..730b6ab10b 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1233,18 +1233,43 @@ func createDoc( var expectedErrorRaised bool var docIDs []client.DocID - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, collections := range getNodeCollections(action.NodeID, s.collections) { - err := withRetry( - actionNodes, - nodeID, + + if action.NodeID.HasValue() { + actionNode := s.nodes[action.NodeID.Value()] + collections := s.collections[action.NodeID.Value()] + err := withRetryOnNode( + actionNode, func() error { var err error - docIDs, err = mutation(s, action, actionNodes[nodeID], nodeID, collections[action.CollectionID]) + docIDs, err = mutation( + s, + action, + actionNode, + action.NodeID.Value(), + collections[action.CollectionID], + ) return err }, ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } else { + for nodeID, collections := range s.collections { + err := withRetryOnNode( + s.nodes[nodeID], + func() error { + var err error + docIDs, err = mutation( + s, + action, + s.nodes[nodeID], + nodeID, + collections[action.CollectionID], + ) + return err + }, + ) + expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) @@ -1734,6 +1759,28 @@ func withRetry( return nil } +// withRetryOnNode attempts to perform the given action, retrying up to a DB-defined +// maximum attempt count if a transaction conflict error is returned. +// +// If a P2P-sync commit for the given document is already in progress this +// Save call can fail as the transaction will conflict. We dont want to worry +// about this in our tests so we just retry a few times until it works (or the +// retry limit is breached - important incase this is a different error) +func withRetryOnNode( + node clients.Client, + action func() error, +) error { + for i := 0; i < node.MaxTxnRetries(); i++ { + err := action() + if errors.Is(err, datastore.ErrTxnConflict) { + time.Sleep(100 * time.Millisecond) + continue + } + return err + } + return nil +} + func getTransaction( s *state, db client.DB, From e0bb9595b831d10202f7b844fac4a6228b7747dd Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 18:20:46 -0400 Subject: [PATCH 04/13] PR(TEST): Fix the deleteDoc testing setup --- tests/integration/utils.go | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 730b6ab10b..a4c890d255 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1441,20 +1441,34 @@ func deleteDoc( docID := s.docIDs[action.CollectionID][action.DocID] var expectedErrorRaised bool - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, collections := range getNodeCollections(action.NodeID, s.collections) { + + if action.NodeID.HasValue() { + nodeID := action.NodeID.Value() + actionNode := s.nodes[nodeID] + collections := s.collections[nodeID] identity := getIdentity(s, nodeID, action.Identity) ctx := db.SetContextIdentity(s.ctx, identity) - - err := withRetry( - actionNodes, - nodeID, + err := withRetryOnNode( + actionNode, func() error { _, err := collections[action.CollectionID].Delete(ctx, docID) return err }, ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } else { + for nodeID, collections := range s.collections { + identity := getIdentity(s, nodeID, action.Identity) + ctx := db.SetContextIdentity(s.ctx, identity) + err := withRetryOnNode( + s.nodes[nodeID], + func() error { + _, err := collections[action.CollectionID].Delete(ctx, docID) + return err + }, + ) + expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) From f23842a885219009e32ad07f2b0089de616bef6c Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 18:37:48 -0400 Subject: [PATCH 05/13] PR(TEST): Fix updateDoc testing setup --- tests/integration/utils.go | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index a4c890d255..9a2dfb674d 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1499,16 +1499,42 @@ func updateDoc( } var expectedErrorRaised bool - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, collections := range getNodeCollections(action.NodeID, s.collections) { - err := withRetry( - actionNodes, - nodeID, + + if action.NodeID.HasValue() { + nodeID := action.NodeID.Value() + collections := s.collections[nodeID] + actionNode := s.nodes[nodeID] + err := withRetryOnNode( + actionNode, func() error { - return mutation(s, action, actionNodes[nodeID], nodeID, collections[action.CollectionID]) + return mutation( + s, + action, + actionNode, + nodeID, + collections[action.CollectionID], + ) }, ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } else { + for nodeID, collections := range s.collections { + actionNode := s.nodes[nodeID] + err := withRetryOnNode( + actionNode, + func() error { + return mutation( + s, + action, + actionNode, + nodeID, + collections[action.CollectionID], + ) + }, + ) + expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } + } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) From 305f333e6a217d7532669f70bede83b1d430dc96 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 19:22:09 -0400 Subject: [PATCH 06/13] PR(TEST): Fix updateDocWithFilter testing setup --- tests/integration/utils.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 9a2dfb674d..a42e6827c4 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1623,14 +1623,13 @@ func updateDocViaGQL( func updateWithFilter(s *state, action UpdateWithFilter) { var res *client.UpdateResult var expectedErrorRaised bool - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, collections := range getNodeCollections(action.NodeID, s.collections) { + if action.NodeID.HasValue() { + nodeID := action.NodeID.Value() + collections := s.collections[nodeID] identity := getIdentity(s, nodeID, action.Identity) ctx := db.SetContextIdentity(s.ctx, identity) - - err := withRetry( - actionNodes, - nodeID, + err := withRetryOnNode( + s.nodes[nodeID], func() error { var err error res, err = collections[action.CollectionID].UpdateWithFilter(ctx, action.Filter, action.Updater) @@ -1638,6 +1637,20 @@ func updateWithFilter(s *state, action UpdateWithFilter) { }, ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } else { + for nodeID, collections := range s.collections { + identity := getIdentity(s, nodeID, action.Identity) + ctx := db.SetContextIdentity(s.ctx, identity) + err := withRetryOnNode( + s.nodes[nodeID], + func() error { + var err error + res, err = collections[action.CollectionID].UpdateWithFilter(ctx, action.Filter, action.Updater) + return err + }, + ) + expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) From 27fb128ea1cf40a8e9f7e6caf45abdaf0e8d4c14 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 19:42:35 -0400 Subject: [PATCH 07/13] PR(TEST): Fix createIndex testing setup --- tests/integration/utils.go | 63 ++++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index a42e6827c4..662169169f 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1667,11 +1667,15 @@ func createIndex( ) { if action.CollectionID >= len(s.indexes) { // Expand the slice if required, so that the index can be accessed by collection index - s.indexes = append(s.indexes, - make([][][]client.IndexDescription, action.CollectionID-len(s.indexes)+1)...) + s.indexes = append( + s.indexes, + make([][][]client.IndexDescription, action.CollectionID-len(s.indexes)+1)..., + ) } - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, collections := range getNodeCollections(action.NodeID, s.collections) { + + if action.NodeID.HasValue() { + nodeID := action.NodeID.Value() + collections := s.collections[nodeID] indexDesc := client.IndexDescription{ Name: action.IndexName, } @@ -1689,23 +1693,64 @@ func createIndex( }) } } + indexDesc.Unique = action.Unique - err := withRetry( - actionNodes, - nodeID, + err := withRetryOnNode( + s.nodes[nodeID], func() error { desc, err := collections[action.CollectionID].CreateIndex(s.ctx, indexDesc) if err != nil { return err } - s.indexes[nodeID][action.CollectionID] = - append(s.indexes[nodeID][action.CollectionID], desc) + s.indexes[nodeID][action.CollectionID] = append( + s.indexes[nodeID][action.CollectionID], + desc, + ) return nil }, ) if AssertError(s.t, s.testCase.Description, err, action.ExpectedError) { return } + } else { + for nodeID, collections := range s.collections { + indexDesc := client.IndexDescription{ + Name: action.IndexName, + } + if action.FieldName != "" { + indexDesc.Fields = []client.IndexedFieldDescription{ + { + Name: action.FieldName, + }, + } + } else if len(action.Fields) > 0 { + for i := range action.Fields { + indexDesc.Fields = append(indexDesc.Fields, client.IndexedFieldDescription{ + Name: action.Fields[i].Name, + Descending: action.Fields[i].Descending, + }) + } + } + + indexDesc.Unique = action.Unique + err := withRetryOnNode( + s.nodes[nodeID], + func() error { + desc, err := collections[action.CollectionID].CreateIndex(s.ctx, indexDesc) + if err != nil { + return err + } + s.indexes[nodeID][action.CollectionID] = append( + s.indexes[nodeID][action.CollectionID], + desc, + ) + return nil + }, + ) + if AssertError(s.t, s.testCase.Description, err, action.ExpectedError) { + return + } + } } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, false) From 08a75dec0196d628f53bb3f3bda73bc5a9f68076 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 20:09:58 -0400 Subject: [PATCH 08/13] PR(TEST): Fix dropIndex testing setup --- tests/integration/utils.go | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 662169169f..fa3eef0279 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1762,21 +1762,38 @@ func dropIndex( action DropIndex, ) { var expectedErrorRaised bool - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, collections := range getNodeCollections(action.NodeID, s.collections) { + + if action.NodeID.HasValue() { + nodeID := action.NodeID.Value() + collections := s.collections[nodeID] + indexName := action.IndexName if indexName == "" { indexName = s.indexes[nodeID][action.CollectionID][action.IndexID].Name } - err := withRetry( - actionNodes, - nodeID, + err := withRetryOnNode( + s.nodes[nodeID], func() error { return collections[action.CollectionID].DropIndex(s.ctx, indexName) }, ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } else { + for nodeID, collections := range s.collections { + indexName := action.IndexName + if indexName == "" { + indexName = s.indexes[nodeID][action.CollectionID][action.IndexID].Name + } + + err := withRetryOnNode( + s.nodes[nodeID], + func() error { + return collections[action.CollectionID].DropIndex(s.ctx, indexName) + }, + ) + expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) From 404c88ee0fd0273aea2c4bd8282ade2b34a9de4a Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 20:50:32 -0400 Subject: [PATCH 09/13] PR(FIX): The test now works --- tests/integration/acp/p2p/create_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration/acp/p2p/create_test.go b/tests/integration/acp/p2p/create_test.go index 85b5c84e81..d8f37034d7 100644 --- a/tests/integration/acp/p2p/create_test.go +++ b/tests/integration/acp/p2p/create_test.go @@ -123,8 +123,6 @@ func TestACP_P2PCreatePrivateDocumentsOnDifferentNodes_SourceHubACP(t *testing.T DocMap: map[string]any{ "name": "Shahzad Lone", }, - - ExpectedError: "403: forbidden", // TODO: FIX THIS BUG }, }, } From e2694bc5ddd1fbc1a3a0c28a14d36f8650ef4893 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 21:46:45 -0400 Subject: [PATCH 10/13] PR(TEST): Fix backup import/export testing setup and mark a todo --- tests/integration/test_case.go | 6 +++-- tests/integration/utils.go | 46 ++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/tests/integration/test_case.go b/tests/integration/test_case.go index 9b0bce913b..f102294e97 100644 --- a/tests/integration/test_case.go +++ b/tests/integration/test_case.go @@ -718,7 +718,8 @@ type ClientIntrospectionRequest struct { type BackupExport struct { // NodeID may hold the ID (index) of a node to generate the backup from. // - // If a value is not provided the indexes will be retrieved from the first nodes. + // If a value is not provided the backup export will be done for all the nodes. + // todo: https://github.com/sourcenetwork/defradb/issues/3067 NodeID immutable.Option[int] // The backup configuration. @@ -738,7 +739,8 @@ type BackupExport struct { type BackupImport struct { // NodeID may hold the ID (index) of a node to generate the backup from. // - // If a value is not provided the indexes will be retrieved from the first nodes. + // If a value is not provided the backup import will be done for all the nodes. + // todo: https://github.com/sourcenetwork/defradb/issues/3067 NodeID immutable.Option[int] // The backup file path. diff --git a/tests/integration/utils.go b/tests/integration/utils.go index fa3eef0279..7ec5a7adcf 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1809,11 +1809,12 @@ func backupExport( } var expectedErrorRaised bool - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, node := range actionNodes { - err := withRetry( - actionNodes, - nodeID, + + if action.NodeID.HasValue() { + nodeID := action.NodeID.Value() + node := s.nodes[nodeID] + err := withRetryOnNode( + node, func() error { return node.BasicExport(s.ctx, &action.Config) }, ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) @@ -1821,7 +1822,20 @@ func backupExport( if !expectedErrorRaised { assertBackupContent(s.t, action.ExpectedContent, action.Config.Filepath) } + } else { + for _, node := range s.nodes { + err := withRetryOnNode( + node, + func() error { return node.BasicExport(s.ctx, &action.Config) }, + ) + expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + + if !expectedErrorRaised { + assertBackupContent(s.t, action.ExpectedContent, action.Config.Filepath) + } + } } + assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) } @@ -1839,15 +1853,27 @@ func backupImport( _ = os.WriteFile(action.Filepath, []byte(action.ImportContent), 0664) var expectedErrorRaised bool - actionNodes := getNodes(action.NodeID, s.nodes) - for nodeID, node := range actionNodes { - err := withRetry( - actionNodes, - nodeID, + + if action.NodeID.HasValue() { + nodeID := action.NodeID.Value() + node := s.nodes[nodeID] + err := withRetryOnNode( + node, func() error { return node.BasicImport(s.ctx, action.Filepath) }, ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + + } else { + for _, node := range s.nodes { + err := withRetryOnNode( + node, + func() error { return node.BasicImport(s.ctx, action.Filepath) }, + ) + expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) + } + } + assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) } From a5fd6bf058067284c6250bd57a5b0d196cc079a0 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 21:48:29 -0400 Subject: [PATCH 11/13] PR(REMOVE): Use the other retry helper instead --- tests/integration/utils.go | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 7ec5a7adcf..004588b2d0 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1877,29 +1877,6 @@ func backupImport( assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) } -// withRetry attempts to perform the given action, retrying up to a DB-defined -// maximum attempt count if a transaction conflict error is returned. -// -// If a P2P-sync commit for the given document is already in progress this -// Save call can fail as the transaction will conflict. We dont want to worry -// about this in our tests so we just retry a few times until it works (or the -// retry limit is breached - important incase this is a different error) -func withRetry( - nodes []clients.Client, - nodeID int, - action func() error, -) error { - for i := 0; i < nodes[nodeID].MaxTxnRetries(); i++ { - err := action() - if errors.Is(err, datastore.ErrTxnConflict) { - time.Sleep(100 * time.Millisecond) - continue - } - return err - } - return nil -} - // withRetryOnNode attempts to perform the given action, retrying up to a DB-defined // maximum attempt count if a transaction conflict error is returned. // From 1bd7955e932f4bef4e0a4f048f8b34f711f421c8 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 22:51:12 -0400 Subject: [PATCH 12/13] PR(TEST): Add delete and update tests for the bugs we just fixed --- tests/integration/acp/p2p/delete_test.go | 159 +++++++++++++++++++++ tests/integration/acp/p2p/update_test.go | 171 +++++++++++++++++++++++ 2 files changed, 330 insertions(+) create mode 100644 tests/integration/acp/p2p/delete_test.go create mode 100644 tests/integration/acp/p2p/update_test.go diff --git a/tests/integration/acp/p2p/delete_test.go b/tests/integration/acp/p2p/delete_test.go new file mode 100644 index 0000000000..59cae4cde9 --- /dev/null +++ b/tests/integration/acp/p2p/delete_test.go @@ -0,0 +1,159 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package test_acp_p2p + +import ( + "fmt" + "testing" + + "github.com/sourcenetwork/immutable" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +func TestACP_P2PDeletePrivateDocumentsOnDifferentNodes_SourceHubACP(t *testing.T) { + expectedPolicyID := "fc56b7509c20ac8ce682b3b9b4fdaad868a9c70dda6ec16720298be64f16e9a4" + + test := testUtils.TestCase{ + + Description: "Test acp, p2p delete private documents on different nodes, with source-hub", + + SupportedACPTypes: immutable.Some( + []testUtils.ACPType{ + testUtils.SourceHubACPType, + }, + ), + + Actions: []any{ + testUtils.RandomNetworkingConfig(), + + testUtils.RandomNetworkingConfig(), + + testUtils.AddPolicy{ + + Identity: immutable.Some(1), + + Policy: ` + name: Test Policy + + description: A Policy + + actor: + name: actor + + resources: + users: + permissions: + read: + expr: owner + reader + writer + + write: + expr: owner + writer + + nothing: + expr: dummy + + relations: + owner: + types: + - actor + + reader: + types: + - actor + + writer: + types: + - actor + + admin: + manages: + - reader + types: + - actor + + dummy: + types: + - actor + `, + + ExpectedPolicyID: expectedPolicyID, + }, + + testUtils.SchemaUpdate{ + Schema: fmt.Sprintf(` + type Users @policy( + id: "%s", + resource: "users" + ) { + name: String + age: Int + } + `, + expectedPolicyID, + ), + }, + + testUtils.ConfigureReplicator{ + SourceNodeID: 0, + TargetNodeID: 1, + }, + + testUtils.CreateDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(0), + + CollectionID: 0, + + DocMap: map[string]any{ + "name": "Shahzad", + }, + }, + + testUtils.CreateDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(1), + + CollectionID: 0, + + DocMap: map[string]any{ + "name": "Shahzad Lone", + }, + }, + + testUtils.WaitForSync{}, + + testUtils.DeleteDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(0), + + CollectionID: 0, + + DocID: 0, + }, + + testUtils.DeleteDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(1), + + CollectionID: 0, + + DocID: 1, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/acp/p2p/update_test.go b/tests/integration/acp/p2p/update_test.go new file mode 100644 index 0000000000..339babee10 --- /dev/null +++ b/tests/integration/acp/p2p/update_test.go @@ -0,0 +1,171 @@ +// Copyright 2024 Democratized Data Foundation +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package test_acp_p2p + +import ( + "fmt" + "testing" + + "github.com/sourcenetwork/immutable" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +func TestACP_P2PUpdatePrivateDocumentsOnDifferentNodes_SourceHubACP(t *testing.T) { + expectedPolicyID := "fc56b7509c20ac8ce682b3b9b4fdaad868a9c70dda6ec16720298be64f16e9a4" + + test := testUtils.TestCase{ + + Description: "Test acp, p2p update private documents on different nodes, with source-hub", + + SupportedACPTypes: immutable.Some( + []testUtils.ACPType{ + testUtils.SourceHubACPType, + }, + ), + + Actions: []any{ + testUtils.RandomNetworkingConfig(), + + testUtils.RandomNetworkingConfig(), + + testUtils.AddPolicy{ + + Identity: immutable.Some(1), + + Policy: ` + name: Test Policy + + description: A Policy + + actor: + name: actor + + resources: + users: + permissions: + read: + expr: owner + reader + writer + + write: + expr: owner + writer + + nothing: + expr: dummy + + relations: + owner: + types: + - actor + + reader: + types: + - actor + + writer: + types: + - actor + + admin: + manages: + - reader + types: + - actor + + dummy: + types: + - actor + `, + + ExpectedPolicyID: expectedPolicyID, + }, + + testUtils.SchemaUpdate{ + Schema: fmt.Sprintf(` + type Users @policy( + id: "%s", + resource: "users" + ) { + name: String + age: Int + } + `, + expectedPolicyID, + ), + }, + + testUtils.ConfigureReplicator{ + SourceNodeID: 0, + TargetNodeID: 1, + }, + + testUtils.CreateDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(0), + + CollectionID: 0, + + DocMap: map[string]any{ + "name": "Shahzad", + }, + }, + + testUtils.CreateDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(1), + + CollectionID: 0, + + DocMap: map[string]any{ + "name": "Shahzad Lone", + }, + }, + + testUtils.WaitForSync{}, + + testUtils.UpdateDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(0), + + CollectionID: 0, + + DocID: 0, + + Doc: ` + { + "name": "ShahzadLone" + } + `, + }, + + testUtils.UpdateDoc{ + Identity: immutable.Some(1), + + NodeID: immutable.Some(1), + + CollectionID: 0, + + DocID: 1, + + Doc: ` + { + "name": "ShahzadLone" + } + `, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} From 6b9be21fdddcbc94b548066bbec54e2d8c61a125 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Tue, 24 Sep 2024 22:19:07 -0400 Subject: [PATCH 13/13] PR(LINT): Make lint --- tests/integration/acp/p2p/create_test.go | 3 ++- tests/integration/utils.go | 4 ---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/integration/acp/p2p/create_test.go b/tests/integration/acp/p2p/create_test.go index d8f37034d7..8775a553d7 100644 --- a/tests/integration/acp/p2p/create_test.go +++ b/tests/integration/acp/p2p/create_test.go @@ -14,8 +14,9 @@ import ( "fmt" "testing" - testUtils "github.com/sourcenetwork/defradb/tests/integration" "github.com/sourcenetwork/immutable" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" ) func TestACP_P2PCreatePrivateDocumentsOnDifferentNodes_SourceHubACP(t *testing.T) { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 004588b2d0..e6ab296140 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -976,7 +976,6 @@ func getIndexes( expectedErrorRaised = expectedErrorRaised || AssertError(s.t, s.testCase.Description, err, action.ExpectedError) } - } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) @@ -1534,7 +1533,6 @@ func updateDoc( ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) } - } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised) @@ -1862,7 +1860,6 @@ func backupImport( func() error { return node.BasicImport(s.ctx, action.Filepath) }, ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) - } else { for _, node := range s.nodes { err := withRetryOnNode( @@ -1871,7 +1868,6 @@ func backupImport( ) expectedErrorRaised = AssertError(s.t, s.testCase.Description, err, action.ExpectedError) } - } assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised)