Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow mutation of col sources via PatchCollection #2424

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 70 additions & 6 deletions db/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,47 @@
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
}

Check warning on line 586 in db/collection.go

View check run for this annotation

Codecov / codecov/patch

db/collection.go#L585-L586

Added lines #L585 - L586 were not covered by tests
}
}
for _, src := range existingCol.QuerySources() {
if src.Transform.HasValue() {
err = db.LensRegistry().SetMigration(ctx, existingCol.ID, model.Lens{})
if err != nil {
return err
}

Check warning on line 594 in db/collection.go

View check run for this annotation

Codecov / codecov/patch

db/collection.go#L593-L594

Added lines #L593 - L594 were not covered by tests
}
}
}

for _, src := range col.CollectionSources() {
if src.Transform.HasValue() {
err = db.LensRegistry().SetMigration(ctx, col.ID, src.Transform.Value())
if err != nil {
return err
}

Check warning on line 604 in db/collection.go

View check run for this annotation

Codecov / codecov/patch

db/collection.go#L603-L604

Added lines #L603 - L604 were not covered by tests
}
}

for _, src := range col.QuerySources() {
if src.Transform.HasValue() {
err = db.LensRegistry().SetMigration(ctx, col.ID, src.Transform.Value())
if err != nil {
return err
}

Check warning on line 613 in db/collection.go

View check run for this annotation

Codecov / codecov/patch

db/collection.go#L612-L613

Added lines #L612 - L613 were not covered by tests
}
}
}

return db.loadSchema(ctx, txn)
Expand All @@ -583,7 +624,7 @@
) error{
validateCollectionNameUnique,
validateSingleVersionActive,
validateSourcesNotModified,
validateSourcesNotRedefined,
validateIndexesNotModified,
validateFieldsNotModified,
validateIDNotZero,
Expand Down Expand Up @@ -646,7 +687,12 @@
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 {
Expand All @@ -656,10 +702,28 @@
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)
Comment on lines +705 to +726
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: This validation is no longer doing what it was doing before. This means that the name is no longer accurate. Sources can be modified. We can't add or remove them or change their ID, be they can be modified. Maybe change the name to validateSourcesModification.

Copy link
Contributor Author

@AndrewSisley AndrewSisley Mar 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot, will do.

  • Rename validation func

}
}

Expand Down
19 changes: 15 additions & 4 deletions db/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
},
},
}
Expand Down
Loading
Loading