Skip to content

Commit

Permalink
fix(i): Add self reference import handling (#1701)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #1697 

## Description

This PR add the capability to handle self referencing docs on import.

Co-authored-by: Shahzad Lone <[email protected]>
  • Loading branch information
fredcarle and shahzadlone authored Jul 25, 2023
1 parent 3061bf0 commit cb94add
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 7 deletions.
25 changes: 25 additions & 0 deletions db/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,19 @@ func (db *db) basicImport(ctx context.Context, txn datastore.Txn, filepath strin
return NewErrJSONDecode(err)
}

// check if self referencing and remove from docMap for key creation
resetMap := map[string]any{}
for _, field := range col.Schema().Fields {
if field.Kind == client.FieldKind_FOREIGN_OBJECT {
if val, ok := docMap[field.Name+request.RelatedObjectID]; ok {
if docMap["_newKey"] == val {
resetMap[field.Name+request.RelatedObjectID] = val
delete(docMap, field.Name+request.RelatedObjectID)
}
}
}
}

delete(docMap, "_key")
delete(docMap, "_newKey")

Expand All @@ -81,6 +94,18 @@ func (db *db) basicImport(ctx context.Context, txn datastore.Txn, filepath strin
if err != nil {
return NewErrDocCreate(err)
}

// add back the self referencing fields and update doc.
for k, v := range resetMap {
err := doc.Set(k, v)
if err != nil {
return NewErrDocUpdate(err)
}
err = col.WithTxn(txn).Update(ctx, doc)
if err != nil {
return NewErrDocUpdate(err)
}
}
}
_, err = d.Token()
if err != nil {
Expand Down
12 changes: 10 additions & 2 deletions db/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const (
errJSONDecode string = "failed to decode JSON"
errDocFromMap string = "failed to create a new doc from map"
errDocCreate string = "failed to save a new doc to collection"
errDocUpdate string = "failed to update doc to collection"
errExpectedJSONObject string = "expected JSON object"
errExpectedJSONArray string = "expected JSON array"
)
Expand Down Expand Up @@ -133,6 +134,7 @@ var (
ErrJSONDecode = errors.New(errJSONDecode)
ErrDocFromMap = errors.New(errDocFromMap)
ErrDocCreate = errors.New(errDocCreate)
ErrDocUpdate = errors.New(errDocUpdate)
ErrExpectedJSONObject = errors.New(errExpectedJSONObject)
ErrExpectedJSONArray = errors.New(errExpectedJSONArray)
)
Expand Down Expand Up @@ -467,14 +469,20 @@ func NewErrJSONDecode(inner error) error {
return errors.Wrap(errJSONDecode, inner)
}

// NewErrDocFromMap returns a new error indicating there was a failure in create
// NewErrDocFromMap returns a new error indicating there was a failure to create
// a new doc from a map
func NewErrDocFromMap(inner error) error {
return errors.Wrap(errDocFromMap, inner)
}

// NewErrDocCreate returns a new error indicating there was a failure in save
// NewErrDocCreate returns a new error indicating there was a failure to save
// a new doc to a collection
func NewErrDocCreate(inner error) error {
return errors.Wrap(errDocCreate, inner)
}

// NewErrDocUpdate returns a new error indicating there was a failure to update
// a doc to a collection
func NewErrDocUpdate(inner error) error {
return errors.Wrap(errDocUpdate, inner)
}
2 changes: 1 addition & 1 deletion request/graphql/schema/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ func finalizeRelations(relationManager *RelationManager, descriptions []client.C
}

// if not finalized then we are missing one side of the relationship
if !rel.finalized && field.Schema != description.Name {
if !rel.finalized {
return NewErrRelationOneSided(field.Schema)
}

Expand Down
55 changes: 55 additions & 0 deletions tests/integration/backup/one_to_one/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,58 @@ func TestBackupImport_WithMultipleNoKeyAndMultipleCollectionsAndMultipleUpdatedD

executeTestCase(t, test)
}

func TestBackupImport_DoubleRelationshipWithUpdate_NoError(t *testing.T) {
test := testUtils.TestCase{
Actions: []any{
testUtils.SchemaUpdate{
Schema: `
type User {
name: String
age: Int
book: Book @relation(name: "written_books")
favouriteBook: Book @relation(name: "favourite_books")
}
type Book {
name: String
author: User @relation(name: "written_books")
favourite: User @relation(name: "favourite_books")
}
`,
},
testUtils.BackupImport{
ImportContent: `{"Book":[{"_key":"bae-236c14bd-4621-5d43-bc03-4442f3b8719e","_newKey":"bae-6dbb3738-d3db-5121-acee-6fbdd97ff7a8","author_id":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","favourite_id":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","name":"John and the sourcerers' stone"},{"_key":"bae-da7f2d88-05c4-528a-846a-0d18ab26603b","_newKey":"bae-da7f2d88-05c4-528a-846a-0d18ab26603b","name":"Game of chains"}],"User":[{"_key":"bae-0648f44e-74e8-593b-a662-3310ec278927","_newKey":"bae-0648f44e-74e8-593b-a662-3310ec278927","age":31,"name":"Bob"},{"_key":"bae-e933420a-988a-56f8-8952-6c245aebd519","_newKey":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","age":31,"name":"John"}]}`,
},
testUtils.Request{
Request: `
query {
Book {
name
author {
name
favouriteBook {
name
}
}
}
}`,
Results: []map[string]any{
{
"name": "John and the sourcerers' stone",
"author": map[string]any{
"name": "John",
"favouriteBook": map[string]any{
"name": "John and the sourcerers' stone",
},
},
},
{
"name": "Game of chains",
},
},
},
},
}

testUtils.ExecuteTestCase(t, []string{"User", "Book"}, test)
}
7 changes: 3 additions & 4 deletions tests/integration/backup/self_reference/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ func TestBackupSelfRefImport_Simple_NoError(t *testing.T) {
executeTestCase(t, test)
}

// This test documents undesirable behaviour, as the document is not linked to itself.
// https://github.com/sourcenetwork/defradb/issues/1697
func TestBackupSelfRefImport_SelfRef_NoError(t *testing.T) {
expectedExportData := `{` +
`"User":[` +
Expand Down Expand Up @@ -125,7 +123,9 @@ func TestBackupSelfRefImport_SelfRef_NoError(t *testing.T) {
Results: []map[string]any{
{
"name": "Bob",
"boss": nil,
"boss": map[string]any{
"name": "Bob",
},
},
},
},
Expand Down Expand Up @@ -264,7 +264,6 @@ func TestBackupSelfRefImport_PrimaryRelationWithSecondCollectionWrongOrder_NoErr
}

// This test documents undesirable behaviour, as the documents are not linked.
// https://github.com/sourcenetwork/defradb/issues/1697
// https://github.com/sourcenetwork/defradb/issues/1704
func TestBackupSelfRefImport_SplitPrimaryRelationWithSecondCollection_NoError(t *testing.T) {
expectedExportData := `{` +
Expand Down
18 changes: 18 additions & 0 deletions tests/integration/schema/relations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,21 @@ func TestSchemaRelationErrorsGivenOneSidedRelationField(t *testing.T) {

testUtils.ExecuteTestCase(t, []string{"Dog", "User"}, test)
}

func TestSchemaRelation_GivenSelfReferemceRelationField_ReturnError(t *testing.T) {
test := testUtils.TestCase{
Actions: []any{
testUtils.SchemaUpdate{
Schema: `
type Dog {
name: String
bestMate: Dog
}
`,
ExpectedError: "relation must be defined on both schemas. Type: Dog",
},
},
}

testUtils.ExecuteTestCase(t, []string{"Dog", "User"}, test)
}

0 comments on commit cb94add

Please sign in to comment.