Skip to content

Commit

Permalink
Code review fixups
Browse files Browse the repository at this point in the history
  • Loading branch information
islamaliev committed Jul 3, 2024
1 parent 9a99d3f commit 8bc56c7
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 59 deletions.
20 changes: 8 additions & 12 deletions datastore/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,30 +34,26 @@ type Rootstore interface {
type MultiStore interface {
Rootstore() DSReaderWriter

// Datastore is a wrapped root DSReaderWriter
// under the /data namespace
// Datastore is a wrapped root DSReaderWriter under the /data namespace
Datastore() DSReaderWriter

// Encstore is a wrapped root DSReaderWriter
// under the /enc namespace
// Encstore is a wrapped root DSReaderWriter under the /enc namespace
// This store is used for storing symmetric encryption keys for doc encryption.
// The store keys are comprised of docID + field name.
Encstore() DSReaderWriter

// Headstore is a wrapped root DSReaderWriter
// under the /head namespace
// Headstore is a wrapped root DSReaderWriter under the /head namespace
Headstore() DSReaderWriter

// Peerstore is a wrapped root DSReaderWriter
// as a ds.Batching, embedded into a DSBatching
// Peerstore is a wrapped root DSReaderWriter as a ds.Batching, embedded into a DSBatching
// under the /peers namespace
Peerstore() DSBatching

// Blockstore is a wrapped root DSReaderWriter
// as a Blockstore, embedded into a Blockstore
// Blockstore is a wrapped root DSReaderWriter as a Blockstore, embedded into a Blockstore
// under the /blocks namespace
Blockstore() Blockstore

// Headstore is a wrapped root DSReaderWriter
// under the /system namespace
// Headstore is a wrapped root DSReaderWriter under the /system namespace
Systemstore() DSReaderWriter
}

Expand Down
1 change: 1 addition & 0 deletions internal/core/block/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ type Block struct {
// Links are the links to other blocks in the DAG.
Links []DAGLink
// IsEncrypted is a flag that indicates if the block's delta is encrypted.
// It needs to be a pointer so that it can be translated from and to `optional Bool` in the IPLD schema.
IsEncrypted *bool
}

Expand Down
44 changes: 21 additions & 23 deletions internal/planner/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type createNode struct {
input []map[string]any
docs []*client.Document

err error
didCreate bool

results planNode

Expand Down Expand Up @@ -73,9 +73,8 @@ func documentsToDocIDs(docs []*client.Document) []string {
return docIDs
}

func (n *createNode) Start() (err error) {
func (n *createNode) Start() error {
n.docs = make([]*client.Document, len(n.input))
defer func() { n.err = err }()

for i, input := range n.input {
doc, err := client.NewDocFromMap(input, n.collection.Definition())
Expand All @@ -85,31 +84,30 @@ func (n *createNode) Start() (err error) {
n.docs[i] = doc
}

if len(n.docs) == 1 {
err = n.collection.Create(n.p.ctx, n.docs[0])
} else {
err = n.collection.CreateMany(n.p.ctx, n.docs)
}

if err != nil {
return err
}

n.results.Spans(docIDsToSpans(documentsToDocIDs(n.docs), n.collection.Description()))

err = n.results.Init()
if err != nil {
return err
}

return n.results.Start()
return nil
}

func (n *createNode) Next() (bool, error) {
n.execInfo.iterations++

if n.err != nil {
return false, n.err
if !n.didCreate {
err := n.collection.CreateMany(n.p.ctx, n.docs)
if err != nil {
return false, err
}

n.results.Spans(docIDsToSpans(documentsToDocIDs(n.docs), n.collection.Description()))

err = n.results.Init()
if err != nil {
return false, err

Check warning on line 103 in internal/planner/create.go

View check run for this annotation

Codecov / codecov/patch

internal/planner/create.go#L103

Added line #L103 was not covered by tests
}

err = n.results.Start()
if err != nil {
return false, err

Check warning on line 108 in internal/planner/create.go

View check run for this annotation

Codecov / codecov/patch

internal/planner/create.go#L108

Added line #L108 was not covered by tests
}
n.didCreate = true
}

next, err := n.results.Next()
Expand Down
15 changes: 6 additions & 9 deletions tests/integration/encryption/commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,6 @@ func TestDocEncryption_UponUpdateOnCounterCRDT_ShouldEncryptedCommitDelta(t *tes
}

func TestDocEncryption_UponEncryptionSeveralDocs_ShouldStoreAllCommitsDeltaEncrypted(t *testing.T) {
const johnDocID = "bae-c9fb0fa4-1195-589c-aa54-e68333fb90b3"
const islamDocID = "bae-d55bd956-1cc4-5d26-aa71-b98807ad49d6"

test := testUtils.TestCase{
Actions: []any{
updateUserCollectionSchema(),
Expand Down Expand Up @@ -323,32 +320,32 @@ func TestDocEncryption_UponEncryptionSeveralDocs_ShouldStoreAllCommitsDeltaEncry
{
"cid": "bafyreih7ry7ef26xn3lm2rhxusf2rbgyvl535tltrt6ehpwtvdnhlmptiu",
"delta": encrypt(testUtils.CBORValue(21)),
"docID": johnDocID,
"docID": testUtils.NewDocIndex(0, 0),
},
{
"cid": "bafyreifusejlwidaqswasct37eorazlfix6vyyn5af42pmjvktilzj5cty",
"delta": encrypt(testUtils.CBORValue("John")),
"docID": johnDocID,
"docID": testUtils.NewDocIndex(0, 0),
},
{
"cid": "bafyreicvxlfxeqghmc3gy56rp5rzfejnbng4nu77x5e3wjinfydl6wvycq",
"delta": nil,
"docID": johnDocID,
"docID": testUtils.NewDocIndex(0, 0),
},
{
"cid": "bafyreibe24bo67owxewoso3ekinera2bhusguij5qy2ahgyufaq3fbvaxa",
"delta": encrypt(testUtils.CBORValue(33)),
"docID": islamDocID,
"docID": testUtils.NewDocIndex(0, 1),
},
{
"cid": "bafyreie2fddpidgc62fhd2fjrsucq3spgh2mgvto2xwolcdmdhb5pdeok4",
"delta": encrypt(testUtils.CBORValue("Islam")),
"docID": islamDocID,
"docID": testUtils.NewDocIndex(0, 1),
},
{
"cid": "bafyreifulxdkf4m3wmmdxjg43l4mw7uuxl5il27eabklc22nptilrh64sa",
"delta": nil,
"docID": islamDocID,
"docID": testUtils.NewDocIndex(0, 1),
},
},
},
Expand Down
7 changes: 1 addition & 6 deletions tests/integration/encryption/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestDocEncryption_WithEncryption_ShouldFetchDecrypted(t *testing.T) {
}`,
Results: []map[string]any{
{
"_docID": "bae-0b2f15e5-bfe7-5cb7-8045-471318d7dbc3",
"_docID": testUtils.NewDocIndex(0, 0),
"name": "John",
"age": int64(21),
},
Expand All @@ -57,12 +57,9 @@ func TestDocEncryption_WithEncryption_ShouldFetchDecrypted(t *testing.T) {
}

func TestDocEncryption_WithEncryptionOnCounterCRDT_ShouldFetchDecrypted(t *testing.T) {
const docID = "bae-ab8ae7d9-6473-5101-ba02-66b217948d7a"

const query = `
query {
Users {
_docID
name
points
}
Expand All @@ -88,7 +85,6 @@ func TestDocEncryption_WithEncryptionOnCounterCRDT_ShouldFetchDecrypted(t *testi
Request: query,
Results: []map[string]any{
{
"_docID": docID,
"name": "John",
"points": 5,
},
Expand All @@ -102,7 +98,6 @@ func TestDocEncryption_WithEncryptionOnCounterCRDT_ShouldFetchDecrypted(t *testi
Request: query,
Results: []map[string]any{
{
"_docID": docID,
"name": "John",
"points": 8,
},
Expand Down
2 changes: 2 additions & 0 deletions tests/integration/encryption/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
testUtils "github.com/sourcenetwork/defradb/tests/integration"
)

// we explicitly set LWW CRDT type because we want to test encryption with this specific CRDT type
// and we don't wat to rely on the default behavior
const userCollectionGQLSchema = (`
type Users {
name: String
Expand Down
17 changes: 8 additions & 9 deletions tests/integration/utils2.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/sourcenetwork/defradb/client"
"github.com/sourcenetwork/defradb/client/request"
"github.com/sourcenetwork/defradb/crypto"
"github.com/sourcenetwork/defradb/datastore"
badgerds "github.com/sourcenetwork/defradb/datastore/badger/v4"
Expand Down Expand Up @@ -107,6 +108,7 @@ func init() {
if value, ok := os.LookupEnv(skipNetworkTestsEnvName); ok {
skipNetworkTests, _ = strconv.ParseBool(value)
}
mutationType = GQLRequestMutationType
}

// AssertPanic asserts that the code inside the specified PanicTestFunc panics.
Expand Down Expand Up @@ -1302,7 +1304,7 @@ func createDocViaGQL(
collection := collections[action.CollectionID]
var input string

paramName := "input"
paramName := request.Input

var err error
if action.DocMap != nil {
Expand All @@ -1311,7 +1313,7 @@ func createDocViaGQL(
var docMaps []map[string]any
err = json.Unmarshal([]byte(action.Doc), &docMaps)
require.NoError(s.t, err)
paramName = "inputs"
paramName = request.Inputs
input, err = arrayToGQL(docMaps)
} else {
input, err = jsonToGQL(action.Doc)
Expand All @@ -1321,10 +1323,10 @@ func createDocViaGQL(
params := paramName + ": " + input

if action.IsEncrypted {
params = params + ", encrypt: true"
params = params + ", " + request.EncryptArgName + ": true"
}

request := fmt.Sprintf(
req := fmt.Sprintf(
`mutation {
create_%s(%s) {
_docID
Expand All @@ -1338,10 +1340,7 @@ func createDocViaGQL(

ctx := makeContextForDocCreate(db.SetContextTxn(s.ctx, txn), &action)

result := node.ExecRequest(
ctx,
request,
)
result := node.ExecRequest(ctx, req)
if len(result.GQL.Errors) > 0 {
return nil, result.GQL.Errors[0]
}
Expand All @@ -1354,7 +1353,7 @@ func createDocViaGQL(
docs := make([]*client.Document, len(resultantDocs))

for i, docMap := range resultantDocs {
docIDString := docMap["_docID"].(string)
docIDString := docMap[request.DocIDFieldName].(string)
docID, err := client.NewDocIDFromString(docIDString)
require.NoError(s.t, err)

Expand Down

0 comments on commit 8bc56c7

Please sign in to comment.