Skip to content

Commit

Permalink
WIP - Allow choice in PatchSchema to make default
Browse files Browse the repository at this point in the history
  • Loading branch information
AndrewSisley committed Sep 15, 2023
1 parent 527d9dc commit efa928f
Show file tree
Hide file tree
Showing 13 changed files with 66 additions and 31 deletions.
5 changes: 4 additions & 1 deletion api/http/handlerfuncs.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,10 @@ func patchSchemaHandler(rw http.ResponseWriter, req *http.Request) {
return
}

err = db.PatchSchema(req.Context(), string(patch))
// Hardcode setDefault to true here, as that preserves the existing behaviour.
// This function will be ripped out very shortly and I don't think it is worth
// spending time/thought here. The new http api handles this correctly.
err = db.PatchSchema(req.Context(), string(patch), true)
if err != nil {
handleErr(req.Context(), rw, err, http.StatusInternalServerError)
return
Expand Down
2 changes: 1 addition & 1 deletion client/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ type Store interface {
//
// Field [FieldKind] values may be provided in either their raw integer form, or as string as per
// [FieldKindStringToEnumMapping].
PatchSchema(context.Context, string) error
PatchSchema(context.Context, string, bool) error

// SetDefaultSchemaVersion sets the default schema version to the ID provided. It will be applied to all
// collections using the schema.
Expand Down
21 changes: 11 additions & 10 deletions client/mocks/db.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions db/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,6 @@ func (db *db) updateCollection(
return nil, err
}

err = db.setDefaultSchemaVersionExplicit(ctx, txn, desc.Name, desc.Schema.SchemaID, schemaVersionID)
if err != nil {
return nil, err
}

schemaVersionHistoryKey := core.NewSchemaHistoryKey(desc.Schema.SchemaID, previousSchemaVersionID)
err = txn.Systemstore().Put(ctx, schemaVersionHistoryKey.ToDS(), []byte(schemaVersionID))
if err != nil {
Expand Down
10 changes: 9 additions & 1 deletion db/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (db *db) getCollectionDescriptions(
// The collections (including the schema version ID) will only be updated if any changes have actually
// been made, if the net result of the patch matches the current persisted description then no changes
// will be applied.
func (db *db) patchSchema(ctx context.Context, txn datastore.Txn, patchString string) error {
func (db *db) patchSchema(ctx context.Context, txn datastore.Txn, patchString string, setAsDefaultVersion bool) error {
patch, err := jsonpatch.DecodePatch([]byte(patchString))
if err != nil {
return err
Expand Down Expand Up @@ -148,6 +148,14 @@ func (db *db) patchSchema(ctx context.Context, txn datastore.Txn, patchString st
if err != nil {
return err
}

if setAsDefaultVersion {
err = db.setDefaultSchemaVersionExplicit(ctx, txn, desc.Name, desc.Schema.SchemaID, col.Schema().VersionID)
if err != nil {
return err
}
}

newDescriptions[i] = col.Description()
}

Expand Down
8 changes: 4 additions & 4 deletions db/txn_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,14 +250,14 @@ func (db *explicitTxnDB) AddSchema(ctx context.Context, schemaString string) ([]
// The collections (including the schema version ID) will only be updated if any changes have actually
// been made, if the net result of the patch matches the current persisted description then no changes
// will be applied.
func (db *implicitTxnDB) PatchSchema(ctx context.Context, patchString string) error {
func (db *implicitTxnDB) PatchSchema(ctx context.Context, patchString string, setAsDefaultVersion bool) error {
txn, err := db.NewTxn(ctx, false)
if err != nil {
return err
}
defer txn.Discard(ctx)

err = db.patchSchema(ctx, txn, patchString)
err = db.patchSchema(ctx, txn, patchString, setAsDefaultVersion)
if err != nil {
return err
}
Expand All @@ -276,8 +276,8 @@ func (db *implicitTxnDB) PatchSchema(ctx context.Context, patchString string) er
// The collections (including the schema version ID) will only be updated if any changes have actually
// been made, if the net result of the patch matches the current persisted description then no changes
// will be applied.
func (db *explicitTxnDB) PatchSchema(ctx context.Context, patchString string) error {
return db.patchSchema(ctx, db.txn, patchString)
func (db *explicitTxnDB) PatchSchema(ctx context.Context, patchString string, setAsDefaultVersion bool) error {
return db.patchSchema(ctx, db.txn, patchString, setAsDefaultVersion)
}

func (db *implicitTxnDB) SetDefaultSchemaVersion(ctx context.Context, schemaVersionID string) error {
Expand Down
14 changes: 12 additions & 2 deletions http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,20 @@ func (c *Client) AddSchema(ctx context.Context, schema string) ([]client.Collect
return cols, nil
}

func (c *Client) PatchSchema(ctx context.Context, patch string) error {
type patchSchemaBody struct {
Patch string
SetAsDefaultVersion bool
}

func (c *Client) PatchSchema(ctx context.Context, patch string, setAsDefaultVersion bool) error {
methodURL := c.http.baseURL.JoinPath("schema")

req, err := http.NewRequestWithContext(ctx, http.MethodPatch, methodURL.String(), strings.NewReader(patch))
body, err := json.Marshal(patchSchemaBody{patch, setAsDefaultVersion})
if err != nil {
return err
}

req, err := http.NewRequestWithContext(ctx, http.MethodPatch, methodURL.String(), bytes.NewBuffer(body))
if err != nil {
return err
}
Expand Down
6 changes: 4 additions & 2 deletions http/handler_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,14 @@ func (s *storeHandler) AddSchema(rw http.ResponseWriter, req *http.Request) {
func (s *storeHandler) PatchSchema(rw http.ResponseWriter, req *http.Request) {
store := req.Context().Value(storeContextKey).(client.Store)

patch, err := io.ReadAll(req.Body)
var message patchSchemaBody
err := requestJSON(req, &message)
if err != nil {
responseJSON(rw, http.StatusBadRequest, errorResponse{err})
return
}
err = store.PatchSchema(req.Context(), string(patch))

err = store.PatchSchema(req.Context(), message.Patch, message.SetAsDefaultVersion)
if err != nil {
responseJSON(rw, http.StatusBadRequest, errorResponse{err})
return
Expand Down
4 changes: 2 additions & 2 deletions http/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ func (w *Wrapper) AddSchema(ctx context.Context, schema string) ([]client.Collec
return w.client.AddSchema(ctx, schema)
}

func (w *Wrapper) PatchSchema(ctx context.Context, patch string) error {
return w.client.PatchSchema(ctx, patch)
func (w *Wrapper) PatchSchema(ctx context.Context, patch string, setAsDefaultVersion bool) error {
return w.client.PatchSchema(ctx, patch, setAsDefaultVersion)
}

func (w *Wrapper) SetDefaultSchemaVersion(ctx context.Context, schemaVersionID string) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"testing"

"github.com/lens-vm/lens/host-go/config/model"
"github.com/sourcenetwork/immutable"

"github.com/sourcenetwork/defradb/client"
testUtils "github.com/sourcenetwork/defradb/tests/integration"
Expand Down Expand Up @@ -45,6 +46,7 @@ func TestSchemaMigrationQuery_WithSetDefaultToLatest_AppliesForwardMigration(t *
{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "email", "Kind": "String"} }
]
`,
SetAsDefaultVersion: immutable.Some(false),
},
testUtils.ConfigureMigration{
LensConfig: client.LensConfig{
Expand Down Expand Up @@ -107,6 +109,7 @@ func TestSchemaMigrationQuery_WithSetDefaultToOriginal_AppliesInverseMigration(t
{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "email", "Kind": "String"} }
]
`,
SetAsDefaultVersion: immutable.Some(false),
},
testUtils.SetDefaultSchemaVersion{
SchemaVersionID: schemaVersionID2,
Expand Down Expand Up @@ -188,6 +191,7 @@ func TestSchemaMigrationQuery_WithSetDefaultToOriginalVersionThatDocWasCreatedAt
{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "email", "Kind": "String"} }
]
`,
SetAsDefaultVersion: immutable.Some(true),
},
testUtils.ConfigureMigration{
LensConfig: client.LensConfig{
Expand Down
4 changes: 4 additions & 0 deletions tests/integration/schema/with_update_set_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ package schema
import (
"testing"

"github.com/sourcenetwork/immutable"

testUtils "github.com/sourcenetwork/defradb/tests/integration"
)

Expand Down Expand Up @@ -87,6 +89,7 @@ func TestSchema_WithUpdateAndSetDefaultVersionToOriginal_NewFieldIsNotQueriable(
{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "email", "Kind": 11} }
]
`,
SetAsDefaultVersion: immutable.Some(false),
},
testUtils.SetDefaultSchemaVersion{
SchemaVersionID: "bafkreihn4qameldz3j7rfundmd4ldhxnaircuulk6h2vcwnpcgxl4oqffq",
Expand Down Expand Up @@ -123,6 +126,7 @@ func TestSchema_WithUpdateAndSetDefaultVersionToNew_AllowsQueryingOfNewField(t *
{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "email", "Kind": 11} }
]
`,
SetAsDefaultVersion: immutable.Some(false),
},
testUtils.SetDefaultSchemaVersion{
SchemaVersionID: "bafkreidejaxpsevyijnr4nah4e2l263emwhdaj57fwwv34eu5rea4ff54e",
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/test_case.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,9 @@ type SchemaPatch struct {
// If a value is not provided the patch will be applied to all nodes.
NodeID immutable.Option[int]

Patch string
ExpectedError string
Patch string
SetAsDefaultVersion immutable.Option[bool]
ExpectedError string
}

// SetDefaultSchemaVersion is an action that will set the default schema version to the
Expand Down
9 changes: 8 additions & 1 deletion tests/integration/utils2.go
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,14 @@ func patchSchema(
action SchemaPatch,
) {
for _, node := range getNodes(action.NodeID, s.nodes) {
err := node.DB.PatchSchema(s.ctx, action.Patch)
var setAsDefaultVersion bool
if action.SetAsDefaultVersion.HasValue() {
setAsDefaultVersion = action.SetAsDefaultVersion.Value()
} else {
setAsDefaultVersion = true
}

err := node.DB.PatchSchema(s.ctx, action.Patch, setAsDefaultVersion)
expectedErrorRaised := AssertError(s.t, s.testCase.Description, err, action.ExpectedError)

assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised)
Expand Down

0 comments on commit efa928f

Please sign in to comment.