From 29a48cf24ff23a045f3dca66a7fb8f1505cd8848 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Mon, 18 Mar 2024 14:15:12 -0400 Subject: [PATCH 1/3] Allow mutation of collection sources via PatchCollection --- db/collection.go | 67 ++++++++- db/errors.go | 19 ++- .../updates/add/sources_test.go | 2 +- .../remove/col_source_transform_test.go | 80 ++++++++++ .../replace/col_source_source_id_test.go | 53 +++++++ .../replace/col_source_transform_test.go | 88 +++++++++++ .../updates/replace/id_test.go | 2 +- .../replace/query_source_query_test.go | 141 ++++++++++++++++++ .../replace/query_source_transform_test.go | 113 ++++++++++++++ .../updates/replace/sources_test.go | 24 ++- 10 files changed, 578 insertions(+), 11 deletions(-) create mode 100644 tests/integration/collection_description/updates/remove/col_source_transform_test.go create mode 100644 tests/integration/collection_description/updates/replace/col_source_source_id_test.go create mode 100644 tests/integration/collection_description/updates/replace/col_source_transform_test.go create mode 100644 tests/integration/collection_description/updates/replace/query_source_query_test.go create mode 100644 tests/integration/collection_description/updates/replace/query_source_transform_test.go diff --git a/db/collection.go b/db/collection.go index b1fc102943..9055adf0ae 100644 --- a/db/collection.go +++ b/db/collection.go @@ -572,6 +572,47 @@ func (db *db) patchCollection( if err != nil { return err } + + existingCol, ok := existingColsByID[col.ID] + if ok { + // Clear any existing migrations in the registry, using this semi-hacky way + // to avoid adding more functions to a public interface that we wish to remove. + + for _, src := range existingCol.CollectionSources() { + if src.Transform.HasValue() { + err = db.LensRegistry().SetMigration(ctx, existingCol.ID, model.Lens{}) + if err != nil { + return err + } + } + } + for _, src := range existingCol.QuerySources() { + if src.Transform.HasValue() { + err = db.LensRegistry().SetMigration(ctx, existingCol.ID, model.Lens{}) + if err != nil { + return err + } + } + } + } + + for _, src := range col.CollectionSources() { + if src.Transform.HasValue() { + err = db.LensRegistry().SetMigration(ctx, col.ID, src.Transform.Value()) + if err != nil { + return err + } + } + } + + for _, src := range col.QuerySources() { + if src.Transform.HasValue() { + err = db.LensRegistry().SetMigration(ctx, col.ID, src.Transform.Value()) + if err != nil { + return err + } + } + } } return db.loadSchema(ctx, txn) @@ -656,10 +697,28 @@ func validateSourcesNotModified( continue } - // DeepEqual is temporary, as this validation is temporary, for example soon - // users will be able to be able to change the migration - if !reflect.DeepEqual(oldCol.Sources, newCol.Sources) { - return NewErrCollectionSourcesCannotBeMutated(newCol.ID) + newColSources := newCol.CollectionSources() + oldColSources := oldCol.CollectionSources() + + if len(newColSources) != len(oldColSources) { + return NewErrCollectionSourcesCannotBeAddedRemoved(newCol.ID) + } + + for i := range newColSources { + if newColSources[i].SourceCollectionID != oldColSources[i].SourceCollectionID { + return NewErrCollectionSourceIDMutated( + newCol.ID, + newColSources[i].SourceCollectionID, + oldColSources[i].SourceCollectionID, + ) + } + } + + newQuerySources := newCol.QuerySources() + oldQuerySources := oldCol.QuerySources() + + if len(newQuerySources) != len(oldQuerySources) { + return NewErrCollectionSourcesCannotBeAddedRemoved(newCol.ID) } } diff --git a/db/errors.go b/db/errors.go index bda5154f79..c32da44671 100644 --- a/db/errors.go +++ b/db/errors.go @@ -85,7 +85,8 @@ const ( errInvalidViewQuery string = "the query provided is not valid as a View" errCollectionAlreadyExists string = "collection already exists" errMultipleActiveCollectionVersions string = "multiple versions of same collection cannot be active" - errCollectionSourcesCannotBeMutated string = "collection sources cannot be mutated" + errCollectionSourcesCannotBeAddedRemoved string = "collection sources cannot be added or removed" + errCollectionSourceIDMutated string = "collection source ID cannot be mutated" errCollectionIndexesCannotBeMutated string = "collection indexes cannot be mutated" errCollectionFieldsCannotBeMutated string = "collection fields cannot be mutated" errCollectionRootIDCannotBeMutated string = "collection root ID cannot be mutated" @@ -113,7 +114,8 @@ var ( ErrInvalidViewQuery = errors.New(errInvalidViewQuery) ErrCanNotIndexNonUniqueFields = errors.New(errCanNotIndexNonUniqueFields) ErrMultipleActiveCollectionVersions = errors.New(errMultipleActiveCollectionVersions) - ErrCollectionSourcesCannotBeMutated = errors.New(errCollectionSourcesCannotBeMutated) + ErrCollectionSourcesCannotBeAddedRemoved = errors.New(errCollectionSourcesCannotBeAddedRemoved) + ErrCollectionSourceIDMutated = errors.New(errCollectionSourceIDMutated) ErrCollectionIndexesCannotBeMutated = errors.New(errCollectionIndexesCannotBeMutated) ErrCollectionFieldsCannotBeMutated = errors.New(errCollectionFieldsCannotBeMutated) ErrCollectionRootIDCannotBeMutated = errors.New(errCollectionRootIDCannotBeMutated) @@ -573,13 +575,22 @@ func NewErrMultipleActiveCollectionVersions(name string, root uint32) error { ) } -func NewErrCollectionSourcesCannotBeMutated(colID uint32) error { +func NewErrCollectionSourcesCannotBeAddedRemoved(colID uint32) error { return errors.New( - errCollectionSourcesCannotBeMutated, + errCollectionSourcesCannotBeAddedRemoved, errors.NewKV("CollectionID", colID), ) } +func NewErrCollectionSourceIDMutated(colID uint32, newSrcID uint32, oldSrcID uint32) error { + return errors.New( + errCollectionSourceIDMutated, + errors.NewKV("CollectionID", colID), + errors.NewKV("NewCollectionSourceID", newSrcID), + errors.NewKV("OldCollectionSourceID", oldSrcID), + ) +} + func NewErrCollectionIndexesCannotBeMutated(colID uint32) error { return errors.New( errCollectionIndexesCannotBeMutated, diff --git a/tests/integration/collection_description/updates/add/sources_test.go b/tests/integration/collection_description/updates/add/sources_test.go index 37010aa15c..c58ff4a660 100644 --- a/tests/integration/collection_description/updates/add/sources_test.go +++ b/tests/integration/collection_description/updates/add/sources_test.go @@ -30,7 +30,7 @@ func TestColDescrUpdateAddSources_Errors(t *testing.T) { { "op": "add", "path": "/1/Sources/-", "value": {"SourceCollectionID": 1} } ] `, - ExpectedError: "collection sources cannot be mutated. CollectionID: 1", + ExpectedError: "collection sources cannot be added or removed. CollectionID: 1", }, }, } diff --git a/tests/integration/collection_description/updates/remove/col_source_transform_test.go b/tests/integration/collection_description/updates/remove/col_source_transform_test.go new file mode 100644 index 0000000000..73179c16b0 --- /dev/null +++ b/tests/integration/collection_description/updates/remove/col_source_transform_test.go @@ -0,0 +1,80 @@ +// 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 remove + +import ( + "testing" + + "github.com/lens-vm/lens/host-go/config/model" + "github.com/sourcenetwork/immutable" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" + "github.com/sourcenetwork/defradb/tests/lenses" +) + +func TestColDescrUpdateRemoveCollectionSourceTransform(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Users { + name: String + } + `, + }, + testUtils.CreateDoc{ + Doc: `{ + "name": "Shahzad" + }`, + }, + testUtils.SchemaPatch{ + Patch: ` + [ + { "op": "add", "path": "/Users/Fields/-", "value": {"Name": "email", "Kind": 11} } + ] + `, + Lens: immutable.Some(model.Lens{ + Lenses: []model.LensModule{ + { + Path: lenses.SetDefaultModulePath, + Arguments: map[string]any{ + "dst": "name", + "value": "Fred", + }, + }, + }, + }), + }, + testUtils.PatchCollection{ + Patch: ` + [ + { "op": "remove", "path": "/2/Sources/0/Transform" } + ] + `, + }, + testUtils.Request{ + Request: `query { + Users { + name + } + }`, + // If the transform was not removed, `"Fred"` would have been returned + Results: []map[string]any{ + { + "name": "Shahzad", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/collection_description/updates/replace/col_source_source_id_test.go b/tests/integration/collection_description/updates/replace/col_source_source_id_test.go new file mode 100644 index 0000000000..3ca1a749f6 --- /dev/null +++ b/tests/integration/collection_description/updates/replace/col_source_source_id_test.go @@ -0,0 +1,53 @@ +// 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 replace + +import ( + "testing" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +func TestColDescrUpdateReplaceCollectionSourceSourceCollectionID_Errors(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Users { + name: String + } + `, + }, + testUtils.CreateDoc{ + Doc: `{ + "name": "Shahzad" + }`, + }, + testUtils.SchemaPatch{ + Patch: ` + [ + { "op": "add", "path": "/Users/Fields/-", "value": {"Name": "email", "Kind": 11} } + ] + `, + }, + testUtils.PatchCollection{ + Patch: ` + [ + { "op": "replace", "path": "/2/Sources/0/SourceCollectionID", "value": 3 } + ] + `, + ExpectedError: "collection source ID cannot be mutated. CollectionID: 2, NewCollectionSourceID: 3, OldCollectionSourceID: 1", + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/collection_description/updates/replace/col_source_transform_test.go b/tests/integration/collection_description/updates/replace/col_source_transform_test.go new file mode 100644 index 0000000000..b933dcd2ed --- /dev/null +++ b/tests/integration/collection_description/updates/replace/col_source_transform_test.go @@ -0,0 +1,88 @@ +// 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 replace + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/lens-vm/lens/host-go/config/model" + "github.com/stretchr/testify/require" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" + "github.com/sourcenetwork/defradb/tests/lenses" +) + +func TestColDescrUpdateReplaceCollectionSourceTransform(t *testing.T) { + transformCfgJson, err := json.Marshal( + model.Lens{ + Lenses: []model.LensModule{ + { + Path: lenses.SetDefaultModulePath, + Arguments: map[string]any{ + "dst": "name", + "value": "Fred", + }, + }, + }, + }, + ) + require.NoError(t, err) + + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Users { + name: String + } + `, + }, + testUtils.CreateDoc{ + Doc: `{ + "name": "Shahzad" + }`, + }, + testUtils.SchemaPatch{ + Patch: ` + [ + { "op": "add", "path": "/Users/Fields/-", "value": {"Name": "email", "Kind": 11} } + ] + `, + }, + testUtils.PatchCollection{ + Patch: fmt.Sprintf(` + [ + { "op": "replace", "path": "/2/Sources/0/Transform", "value": %s } + ] + `, + transformCfgJson, + ), + }, + testUtils.Request{ + Request: `query { + Users { + name + } + }`, + // Without the new transform, `"Shahzad"` would have been returned + Results: []map[string]any{ + { + "name": "Fred", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/collection_description/updates/replace/id_test.go b/tests/integration/collection_description/updates/replace/id_test.go index b83c634385..a89dad193b 100644 --- a/tests/integration/collection_description/updates/replace/id_test.go +++ b/tests/integration/collection_description/updates/replace/id_test.go @@ -87,7 +87,7 @@ func TestColDescrUpdateReplaceID_WithExistingSameRoot_Errors(t *testing.T) { { "op": "replace", "path": "/2/ID", "value": 1 } ] `, - ExpectedError: "collection sources cannot be mutated.", + ExpectedError: "collection sources cannot be added or removed.", }, }, } diff --git a/tests/integration/collection_description/updates/replace/query_source_query_test.go b/tests/integration/collection_description/updates/replace/query_source_query_test.go new file mode 100644 index 0000000000..789f4b2d7b --- /dev/null +++ b/tests/integration/collection_description/updates/replace/query_source_query_test.go @@ -0,0 +1,141 @@ +// 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 replace + +import ( + "testing" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +func TestColDescrUpdateReplaceQuerySourceQuery(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Users { + name: String + } + `, + }, + testUtils.SchemaUpdate{ + Schema: ` + type Books { + name: String + } + `, + }, + testUtils.CreateView{ + // Create the view on the `Books` collection + Query: ` + Books { + name + } + `, + SDL: ` + type View { + name: String + } + `, + }, + testUtils.CreateDoc{ + Doc: `{ + "name": "Shahzad" + }`, + }, + testUtils.PatchCollection{ + // Patch the view query definition so that it now queries the `Users` collection + Patch: ` + [ + { "op": "replace", "path": "/3/Sources/0/Query", "value": {"Name": "Users", "Fields":[{"Name":"name"}]} } + ] + `, + }, + testUtils.Request{ + Request: `query { + View { + name + } + }`, + // If the view was still querying `Books` there would be no results + Results: []map[string]any{ + { + "name": "Shahzad", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestColDescrUpdateReplaceQuerySourceQueryName(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Users { + name: String + } + `, + }, + testUtils.SchemaUpdate{ + Schema: ` + type Books { + name: String + } + `, + }, + testUtils.CreateView{ + // Create the view on the `Books` collection + Query: ` + Books { + name + } + `, + SDL: ` + type View { + name: String + } + `, + }, + testUtils.CreateDoc{ + Doc: `{ + "name": "Shahzad" + }`, + }, + testUtils.PatchCollection{ + // Patch the view query definition so that it now queries the `Users` collection + Patch: ` + [ + { "op": "replace", "path": "/3/Sources/0/Query/Name", "value": "Users" } + ] + `, + }, + testUtils.Request{ + Request: `query { + View { + name + } + }`, + // If the view was still querying `Books` there would be no results + Results: []map[string]any{ + { + "name": "Shahzad", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/collection_description/updates/replace/query_source_transform_test.go b/tests/integration/collection_description/updates/replace/query_source_transform_test.go new file mode 100644 index 0000000000..7c32caa86d --- /dev/null +++ b/tests/integration/collection_description/updates/replace/query_source_transform_test.go @@ -0,0 +1,113 @@ +// 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 replace + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/lens-vm/lens/host-go/config/model" + "github.com/sourcenetwork/immutable" + "github.com/stretchr/testify/require" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" + "github.com/sourcenetwork/defradb/tests/lenses" +) + +func TestColDescrUpdateReplaceQuerySourceTransform(t *testing.T) { + newTransformCfgJson, err := json.Marshal( + model.Lens{ + Lenses: []model.LensModule{ + { + Path: lenses.CopyModulePath, + Arguments: map[string]any{ + "src": "lastName", + "dst": "fullName", + }, + }, + }, + }, + ) + require.NoError(t, err) + + test := testUtils.TestCase{ + Description: "Simple view with transform", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type User { + firstName: String + lastName: String + } + `, + }, + testUtils.CreateView{ + Query: ` + User { + firstName + lastName + } + `, + SDL: ` + type UserView { + fullName: String + } + `, + Transform: immutable.Some(model.Lens{ + // This transform will copy the value from `firstName` into the `fullName` field, + // like an overly-complicated alias + Lenses: []model.LensModule{ + { + Path: lenses.CopyModulePath, + Arguments: map[string]any{ + "src": "firstName", + "dst": "fullName", + }, + }, + }, + }), + }, + testUtils.PatchCollection{ + Patch: fmt.Sprintf(` + [ + { "op": "replace", "path": "/2/Sources/0/Transform", "value": %s } + ] + `, + newTransformCfgJson, + ), + }, + testUtils.CreateDoc{ + // Set the `name` field only + Doc: `{ + "firstName": "John", + "lastName": "S" + }`, + }, + testUtils.Request{ + Request: ` + query { + UserView { + fullName + } + } + `, + Results: []map[string]any{ + { + "fullName": "S", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/collection_description/updates/replace/sources_test.go b/tests/integration/collection_description/updates/replace/sources_test.go index 2d06e01d4a..2f6bf7ca69 100644 --- a/tests/integration/collection_description/updates/replace/sources_test.go +++ b/tests/integration/collection_description/updates/replace/sources_test.go @@ -30,7 +30,29 @@ func TestColDescrUpdateReplaceSources_Errors(t *testing.T) { { "op": "replace", "path": "/1/Sources", "value": [{"SourceCollectionID": 1}] } ] `, - ExpectedError: "collection sources cannot be mutated. CollectionID: 1", + ExpectedError: "collection sources cannot be added or removed. CollectionID: 1", + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestColDescrUpdateReplaceSourcesWithQuerySource_Errors(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Users {} + `, + }, + testUtils.PatchCollection{ + Patch: ` + [ + { "op": "replace", "path": "/1/Sources", "value": [{"Query": {"Name": "Users"}}] } + ] + `, + ExpectedError: "collection sources cannot be added or removed. CollectionID: 1", }, }, } From ca123287c519cd72d06140652279abd70473f902 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 20 Mar 2024 17:06:43 -0400 Subject: [PATCH 2/3] PR FIXUP - Rename validation rule --- db/collection.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/db/collection.go b/db/collection.go index 9055adf0ae..49bdf01e71 100644 --- a/db/collection.go +++ b/db/collection.go @@ -624,7 +624,7 @@ var patchCollectionValidators = []func( ) error{ validateCollectionNameUnique, validateSingleVersionActive, - validateSourcesNotModified, + validateSourcesNotRedefined, validateIndexesNotModified, validateFieldsNotModified, validateIDNotZero, @@ -687,7 +687,12 @@ func validateSingleVersionActive( return nil } -func validateSourcesNotModified( +// validateSourcesNotRedefined specifies the limitations on how the collection sources +// can be mutated. +// +// Currently new sources cannot be added, existing cannot be removed, and CollectionSources +// cannot be redirected to other collections. +func validateSourcesNotRedefined( oldColsByID map[uint32]client.CollectionDescription, newColsByID map[uint32]client.CollectionDescription, ) error { From 2fae5b5d6358bc3dd91e9821976a2fba117b000c Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 20 Mar 2024 17:08:39 -0400 Subject: [PATCH 3/3] PR FIXUP - Tweak json spacing --- .../updates/replace/query_source_transform_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/collection_description/updates/replace/query_source_transform_test.go b/tests/integration/collection_description/updates/replace/query_source_transform_test.go index 7c32caa86d..89a2598010 100644 --- a/tests/integration/collection_description/updates/replace/query_source_transform_test.go +++ b/tests/integration/collection_description/updates/replace/query_source_transform_test.go @@ -88,8 +88,8 @@ func TestColDescrUpdateReplaceQuerySourceTransform(t *testing.T) { testUtils.CreateDoc{ // Set the `name` field only Doc: `{ - "firstName": "John", - "lastName": "S" + "firstName": "John", + "lastName": "S" }`, }, testUtils.Request{