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: Add collection response information on creation #1499

Merged
merged 5 commits into from
May 16, 2023
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
22 changes: 20 additions & 2 deletions api/http/handlerfuncs.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ func execGQLHandler(rw http.ResponseWriter, req *http.Request) {
sendJSON(req.Context(), rw, newGQLResult(result.GQL), http.StatusOK)
}

type collectionResponse struct {
Name string `json:"name"`
ID string `json:"id"`
}

func loadSchemaHandler(rw http.ResponseWriter, req *http.Request) {
sdl, err := readWithLimit(req.Body, rw)
if err != nil {
Expand All @@ -168,16 +173,29 @@ func loadSchemaHandler(rw http.ResponseWriter, req *http.Request) {
return
}

err = db.AddSchema(req.Context(), string(sdl))
colDescs, err := db.AddSchema(req.Context(), string(sdl))
if err != nil {
handleErr(req.Context(), rw, err, http.StatusInternalServerError)
return
}

colResp := make([]collectionResponse, len(colDescs))
for i, desc := range colDescs {
col, err := db.GetCollectionByName(req.Context(), desc.Name)
if err != nil {
handleErr(req.Context(), rw, err, http.StatusInternalServerError)
return
}
colResp[i] = collectionResponse{
Name: col.Name(),
ID: col.SchemaID(),
}
}

sendJSON(
req.Context(),
rw,
simpleDataResponse("result", "success"),
simpleDataResponse("result", "success", "collections", colResp),
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Do we still want to return "result", "success"? Do we do this for all successful calls across the http api?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I had a similar thought. Its a somewhat meaningless item to include, but I have no strong preference either way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It wasn't meaningless when there was nothing else we were returning but now I think we can safely remove it.

http.StatusOK,
)
}
Expand Down
12 changes: 10 additions & 2 deletions api/http/handlerfuncs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,15 @@ type user {

switch v := resp.Data.(type) {
case map[string]any:
assert.Equal(t, "success", v["result"])
assert.Equal(t, map[string]any{
"result": "success",
"collections": []any{
map[string]any{
"name": "user",
Copy link
Member

Choose a reason for hiding this comment

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

lol: 🤣 we still have a lowercase GQL type.

Copy link
Contributor

Choose a reason for hiding this comment

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

lol - we have a lot, I think the entire query/simple directory, and the P2P tests are largely still uppercase.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah not sure why those didn't get picked up. The api folder make sense, def spaced that. But the query/simple types are perplexing xD

Copy link
Contributor

Choose a reason for hiding this comment

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

we have +1000 tests atm, would have been amazing if you/Orpheus managed to get all of them lol

Copy link
Contributor

Choose a reason for hiding this comment

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

I think all the cid related ones are done, others are easy to change as/when spotted

"id": "bafkreigrucdl7x3lsa4xwgz2bn7lbqmiwkifnspgx7hlkpaal3o55325bq",
},
},
}, v)

default:
t.Fatalf("data should be of type map[string]any but got %T\n%v", resp.Data, v)
Expand Down Expand Up @@ -1024,7 +1032,7 @@ type user {
verified: Boolean
points: Float
}`
err := db.AddSchema(ctx, stmt)
_, err := db.AddSchema(ctx, stmt)
if err != nil {
t.Fatal(err)
}
Expand Down
10 changes: 9 additions & 1 deletion cli/schema_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
httpapi "github.com/sourcenetwork/defradb/api/http"
"github.com/sourcenetwork/defradb/config"
"github.com/sourcenetwork/defradb/errors"
"github.com/sourcenetwork/defradb/logging"
)

func MakeSchemaAddCommand(cfg *config.Config) *cobra.Command {
Expand Down Expand Up @@ -134,14 +135,21 @@ Learn more about the DefraDB GraphQL Schema Language on https://docs.source.netw
} else {
type schemaResponse struct {
Data struct {
Result string `json:"result"`
Result string `json:"result"`
Collections []struct {
Name string `json:"name"`
ID string `json:"id"`
} `json:"collections"`
} `json:"data"`
}
r := schemaResponse{}
err = json.Unmarshal(response, &r)
if err != nil {
return errors.Wrap("failed to unmarshal response", err)
}
if r.Data.Result == "success" {
log.FeedbackInfo(cmd.Context(), "Successfully added schema.", logging.NewKV("Collections", r.Data.Collections))
}
log.FeedbackInfo(cmd.Context(), r.Data.Result)
}
}
Expand Down
2 changes: 1 addition & 1 deletion client/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ type Store interface {
//
// All schema types provided must not exist prior to calling this, and they may not reference existing
// types previously defined.
AddSchema(context.Context, string) error
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.
Expand Down
23 changes: 15 additions & 8 deletions db/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,36 @@ import (

// addSchema takes the provided schema in SDL format, and applies it to the database,
// and creates the necessary collections, request types, etc.
func (db *db) addSchema(ctx context.Context, txn datastore.Txn, schemaString string) error {
func (db *db) addSchema(
ctx context.Context,
txn datastore.Txn,
schemaString string,
) ([]client.CollectionDescription, error) {
existingDescriptions, err := db.getCollectionDescriptions(ctx, txn)
if err != nil {
return err
return nil, err
}

newDescriptions, err := db.parser.ParseSDL(ctx, schemaString)
if err != nil {
return err
return nil, err
}

err = db.parser.SetSchema(ctx, txn, append(existingDescriptions, newDescriptions...))
if err != nil {
return err
return nil, err
}

for _, desc := range newDescriptions {
if _, err := db.createCollection(ctx, txn, desc); err != nil {
return err
returnDescriptions := make([]client.CollectionDescription, len(newDescriptions))
for i, desc := range newDescriptions {
col, err := db.createCollection(ctx, txn, desc)
if err != nil {
return nil, err
}
returnDescriptions[i] = col.Description()
}

return nil
return returnDescriptions, nil
}

func (db *db) loadSchema(ctx context.Context, txn datastore.Txn) error {
Expand Down
15 changes: 9 additions & 6 deletions db/txn_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,27 +191,30 @@ func (db *explicitTxnDB) GetAllCollections(ctx context.Context) ([]client.Collec
//
// All schema types provided must not exist prior to calling this, and they may not reference existing
// types previously defined.
func (db *implicitTxnDB) AddSchema(ctx context.Context, schemaString string) error {
func (db *implicitTxnDB) AddSchema(ctx context.Context, schemaString string) ([]client.CollectionDescription, error) {
txn, err := db.NewTxn(ctx, false)
if err != nil {
return err
return nil, err
}
defer txn.Discard(ctx)

err = db.addSchema(ctx, txn, schemaString)
cols, err := db.addSchema(ctx, txn, schemaString)
if err != nil {
return err
return nil, err
}

return txn.Commit(ctx)
if err := txn.Commit(ctx); err != nil {
return nil, err
}
return cols, nil
}

// AddSchema takes the provided GQL schema in SDL format, and applies it to the database,
// creating the necessary collections, request types, etc.
//
// All schema types provided must not exist prior to calling this, and they may not reference existing
// types previously defined.
func (db *explicitTxnDB) AddSchema(ctx context.Context, schemaString string) error {
func (db *explicitTxnDB) AddSchema(ctx context.Context, schemaString string) ([]client.CollectionDescription, error) {
return db.addSchema(ctx, db.txn, schemaString)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/bench/bench_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func SetupCollections(

// b.Logf("Loading schema: \n%s", schema)

if err := db.AddSchema(ctx, schema); err != nil {
if _, err := db.AddSchema(ctx, schema); err != nil {
return nil, errors.Wrap("couldn't load schema", err)
}

Expand Down
6 changes: 4 additions & 2 deletions tests/integration/cli/client_schema_add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func TestAddSchemaFromFile(t *testing.T) {

nodeLog := stopDefra()

assert.Contains(t, stdout, `{"data":{"result":"success"}}`)
jsonReponse := `{"data":{"collections":[{"name":"User","id":"bafkreib5hb7mr7ecbdufd7mvv6va6mpxukjai7hpnqkhxonnw7lzwfqlja"}],"result":"success"}}`
assert.Contains(t, stdout, jsonReponse)
assertNotContainsSubstring(t, nodeLog, "ERROR")
}

Expand All @@ -46,6 +47,7 @@ func TestAddSchemaWithDuplicateType(t *testing.T) {

_ = stopDefra()

assertContainsSubstring(t, stdout1, `{"data":{"result":"success"}}`)
jsonReponse := `{"data":{"collections":[{"name":"Post","id":"bafkreicgpbla5wlogpinnm32arcqzptusdc5tzdznipqrf6nkroav6b25a"}],"result":"success"}}`
assertContainsSubstring(t, stdout1, jsonReponse)
assertContainsSubstring(t, stdout2, `schema type already exists. Name: Post`)
}
2 changes: 1 addition & 1 deletion tests/integration/collection/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func ExecuteRequestTestCase(
t.Fatal(err)
}

err = db.AddSchema(ctx, schema)
_, err = db.AddSchema(ctx, schema)
if assertError(t, testCase.Description, err, testCase.ExpectedError) {
return
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/events/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func ExecuteRequestTestCase(
db, err := testUtils.NewBadgerMemoryDB(ctx, db.WithUpdateEvents())
require.NoError(t, err)

err = db.AddSchema(ctx, schema)
_, err = db.AddSchema(ctx, schema)
require.NoError(t, err)

setupDatabase(ctx, t, db, testCase)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/explain/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ func setupDatabase(
updates immutable.Option[map[int]map[int][]string],
) {
db := dbi.db
err := db.AddSchema(ctx, schema)
_, err := db.AddSchema(ctx, schema)
if testUtils.AssertError(t, description, err, expectedError) {
return
}
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/net/order/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ func setupDefraNode(t *testing.T, cfg *config.Config, seeds []string) (*node.Nod
}

func seedSchema(ctx context.Context, db client.DB) error {
return db.AddSchema(ctx, userCollectionGQLSchema)
_, err := db.AddSchema(ctx, userCollectionGQLSchema)
return err
}

func seedDocument(ctx context.Context, db client.DB, document string) (client.DocKey, error) {
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/utils2.go
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ func updateSchema(
action SchemaUpdate,
) {
for _, node := range getNodes(action.NodeID, nodes) {
err := node.DB.AddSchema(ctx, action.Schema)
_, err := node.DB.AddSchema(ctx, action.Schema)
expectedErrorRaised := AssertError(t, testCase.Description, err, action.ExpectedError)

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