Allow choice in PatchSchema to make default

api/http/handlerfuncs.go

@@ -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

client/db.go

@@ -96,7 +96,8 @@ type Store interface {
 	AddSchema(context.Context, string) ([]CollectionDescription, error)
 
 	// PatchSchema takes the given JSON patch string and applies it to the set of CollectionDescriptions
-	// present in the database.
+	// present in the database. If true is provided, the new schema versions will be made default, otherwise
+	// [SetDefaultSchemaVersion] should be called to set them so.
 	//
 	// It will also update the GQL types used by the query system. It will error and not apply any of the
 	// requested, valid updates should the net result of the patch result in an invalid state.  The
@@ -109,7 +110,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.

db/collection.go

@@ -234,6 +234,7 @@ func (db *db) updateCollection(
 	existingDescriptionsByName map[string]client.CollectionDescription,
 	proposedDescriptionsByName map[string]client.CollectionDescription,
 	desc client.CollectionDescription,
+	setAsDefaultVersion bool,
 ) (client.Collection, error) {
 	hasChanged, err := db.validateUpdateCollection(ctx, txn, existingDescriptionsByName, proposedDescriptionsByName, desc)
 	if err != nil {
@@ -300,17 +301,19 @@ 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 {
 		return nil, err
 	}
 
+	if setAsDefaultVersion {
+		err = db.setDefaultSchemaVersionExplicit(ctx, txn, desc.Name, desc.Schema.SchemaID, schemaVersionID)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	return db.getCollectionByName(ctx, txn, desc.Name)
 }
 

db/schema.go

@@ -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
@@ -144,10 +144,11 @@ func (db *db) patchSchema(ctx context.Context, txn datastore.Txn, patchString st
 	}
 
 	for i, desc := range newDescriptions {
-		col, err := db.updateCollection(ctx, txn, collectionsByName, newDescriptionsByName, desc)
+		col, err := db.updateCollection(ctx, txn, collectionsByName, newDescriptionsByName, desc, setAsDefaultVersion)
 		if err != nil {
 			return err
 		}
+
 		newDescriptions[i] = col.Description()
 	}
 

db/txn_db.go

@@ -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
 	}
@@ -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 {

http/client.go

@@ -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 patchSchemaRequest 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(patchSchemaRequest{patch, setAsDefaultVersion})
+	if err != nil {
+		return err
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodPatch, methodURL.String(), bytes.NewBuffer(body))
 	if err != nil {
 		return err
 	}

http/handler_store.go

@@ -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 patchSchemaRequest
+	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

http/wrapper.go

@@ -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 {

tests/integration/schema/migrations/query/with_set_default_test.go

@@ -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"
@@ -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{
@@ -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,
@@ -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{

tests/integration/schema/updates/add/field/simple_test.go

@@ -13,6 +13,8 @@ package field
 import (
 	"testing"
 
+	"github.com/sourcenetwork/immutable"
+
 	testUtils "github.com/sourcenetwork/defradb/tests/integration"
 )
 
@@ -48,6 +50,39 @@ func TestSchemaUpdatesAddFieldSimple(t *testing.T) {
 	testUtils.ExecuteTestCase(t, test)
 }
 
+func TestSchemaUpdates_AddFieldSimpleDoNotSetDefault_Errors(t *testing.T) {
+	test := testUtils.TestCase{
+		Description: "Test schema update, add field",
+		Actions: []any{
+			testUtils.SchemaUpdate{
+				Schema: `
+					type Users {
+						name: String
+					}
+				`,
+			},
+			testUtils.SchemaPatch{
+				Patch: `
+					[
+						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "email", "Kind": 11} }
+					]
+				`,
+				SetAsDefaultVersion: immutable.Some(false),
+			},
+			testUtils.Request{
+				Request: `query {
+					Users {
+						name
+						email
+					}
+				}`,
+				ExpectedError: `Cannot query field "email" on type "Users".`,
+			},
+		},
+	}
+	testUtils.ExecuteTestCase(t, test)
+}
+
 func TestSchemaUpdatesAddFieldSimpleErrorsAddingToUnknownCollection(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "Test schema update, add to unknown collection fails",

tests/integration/schema/with_update_set_default_test.go

@@ -13,6 +13,8 @@ package schema
 import (
 	"testing"
 
+	"github.com/sourcenetwork/immutable"
+
 	testUtils "github.com/sourcenetwork/defradb/tests/integration"
 )
 
@@ -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",
@@ -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",

tests/integration/test_case.go

@@ -81,8 +81,12 @@ 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
+
+	// If SetAsDefaultVersion has a value, and that value is false then the schema version
+	// resulting from this patch will not be made default.
+	SetAsDefaultVersion immutable.Option[bool]
+	ExpectedError       string
 }
 
 // SetDefaultSchemaVersion is an action that will set the default schema version to the

tests/integration/utils2.go

@@ -1033,7 +1033,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)