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: Multiple schema file support in schema add #3352

Merged
merged 12 commits into from
Jan 17, 2025
14 changes: 14 additions & 0 deletions cli/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
errRequiredFlag string = "the required flag [--%s|-%s] is %s"
errInvalidAscensionOrder string = "invalid order: expected ASC or DESC"
errInvalidInxedFieldDescription string = "invalid or malformed field description"
errEmptySchemaString string = "schema cannot be empty"
)

var (
Expand All @@ -36,6 +37,7 @@
ErrPolicyFileArgCanNotBeEmpty = errors.New("policy file argument can not be empty")
ErrPurgeForceFlagRequired = errors.New("run this command again with --force if you really want to purge all data")
ErrMissingKeyringSecret = errors.New("missing keyring secret")
ErrEmptySchemaString = errors.New(errEmptySchemaString)
)

func NewErrRequiredFlagEmpty(longName string, shortName string) error {
Expand Down Expand Up @@ -65,3 +67,15 @@
func NewErrInvalidInxedFieldDescription(fieldName string) error {
return errors.New(errInvalidInxedFieldDescription, errors.NewKV("Field", fieldName))
}

func NewErrFailedToReadSchemaFile(schemaFile string, inner error) error {
return errors.Wrap(fmt.Sprintf("failed to read file %s", schemaFile), inner)

Check warning on line 72 in cli/errors.go

View check run for this annotation

Codecov / codecov/patch

cli/errors.go#L71-L72

Added lines #L71 - L72 were not covered by tests
}

func NewErrFailedToReadSchemaFromStdin(inner error) error {
return errors.Wrap("failed to read schema from stdin", inner)

Check warning on line 76 in cli/errors.go

View check run for this annotation

Codecov / codecov/patch

cli/errors.go#L75-L76

Added lines #L75 - L76 were not covered by tests
}

func NewErrFailedToAddSchema(inner error) error {
return errors.Wrap("failed to add schema", inner)
}
Comment on lines +71 to +81
Copy link
Member

Choose a reason for hiding this comment

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

praise: Thanks for doing this suggestion :)

48 changes: 33 additions & 15 deletions cli/schema_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@
package cli

import (
"fmt"
"io"
"os"

"github.com/spf13/cobra"
)

func MakeSchemaAddCommand() *cobra.Command {
var schemaFile string
var schemaFiles []string
var cmd = &cobra.Command{
Use: "add [schema]",
Short: "Add new schema",
Expand All @@ -36,40 +35,59 @@
Example: add from file:
defradb client schema add -f schema.graphql

Example: add from multiple files:
defradb client schema add -f schema1.graphql -f schema2.graphql
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: IMO it would be nice to standardise this, so that all file-based inputs can be expected to work the same way.


Example: add from multiple files:
defradb client schema add -f schema1.graphql,schema2.graphql

Example: add from stdin:
cat schema.graphql | defradb client schema add -

Learn more about the DefraDB GraphQL Schema Language on https://docs.source.network.`,
RunE: func(cmd *cobra.Command, args []string) error {
store := mustGetContextStore(cmd)

var schema string
var combinedSchema string
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: This is a nice and simple way of ensure the whole operation takes place in a single txn.

thought: This does introduce a difference between the 3 clients - it might be nicer for users if instead we changed the other 2 to accept multiple strings. You would then be able to test this using integration tests instead of CLI specific stuff.

I think there is also an argument to be had that without changing the other 2 clients, the CLI should not have this feature, and instead users can just use cat/type in their terminal.

switch {
case schemaFile != "":
data, err := os.ReadFile(schemaFile)
if err != nil {
return err
case len(schemaFiles) > 0:
// Read schemas from files and concatenate them
for _, schemaFile := range schemaFiles {
data, err := os.ReadFile(schemaFile)
if err != nil {
return NewErrFailedToReadSchemaFile(schemaFile, err)
}
combinedSchema += string(data) + "\n"

Check warning on line 60 in cli/schema_add.go

View check run for this annotation

Codecov / codecov/patch

cli/schema_add.go#L53-L60

Added lines #L53 - L60 were not covered by tests
}
schema = string(data)

case len(args) > 0 && args[0] == "-":
// Read schema from stdin

Check warning on line 64 in cli/schema_add.go

View check run for this annotation

Codecov / codecov/patch

cli/schema_add.go#L64

Added line #L64 was not covered by tests
data, err := io.ReadAll(cmd.InOrStdin())
if err != nil {
return err
return NewErrFailedToReadSchemaFromStdin(err)

Check warning on line 67 in cli/schema_add.go

View check run for this annotation

Codecov / codecov/patch

cli/schema_add.go#L67

Added line #L67 was not covered by tests
}
schema = string(data)
combinedSchema += string(data) + "\n"

Check warning on line 69 in cli/schema_add.go

View check run for this annotation

Codecov / codecov/patch

cli/schema_add.go#L69

Added line #L69 was not covered by tests

case len(args) > 0:
schema = args[0]
// Read schema from argument string
combinedSchema += args[0] + "\n"

default:
return fmt.Errorf("schema cannot be empty")
return ErrEmptySchemaString

Check warning on line 76 in cli/schema_add.go

View check run for this annotation

Codecov / codecov/patch

cli/schema_add.go#L76

Added line #L76 was not covered by tests
}

cols, err := store.AddSchema(cmd.Context(), schema)
// Process the combined schema
cols, err := store.AddSchema(cmd.Context(), combinedSchema)
if err != nil {
return NewErrFailedToAddSchema(err)
}
if err := writeJSON(cmd, cols); err != nil {
return err
}
return writeJSON(cmd, cols)

return nil
},
}
cmd.Flags().StringVarP(&schemaFile, "file", "f", "", "File to load a schema from")
cmd.Flags().StringSliceVarP(&schemaFiles, "file", "f", []string{}, "File to load schema from")
return cmd
}
10 changes: 8 additions & 2 deletions docs/website/references/cli/defradb_client_schema_add.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ Example: add from an argument string:
Example: add from file:
defradb client schema add -f schema.graphql

Example: add from multiple files:
defradb client schema add -f schema1.graphql -f schema2.graphql

Example: add from multiple files:
defradb client schema add -f schema1.graphql,schema2.graphql

Example: add from stdin:
cat schema.graphql | defradb client schema add -

Expand All @@ -29,8 +35,8 @@ defradb client schema add [schema] [flags]
### Options

```
-f, --file string File to load a schema from
-h, --help help for add
-f, --file strings File to load schema from
-h, --help help for add
```

### Options inherited from parent commands
Expand Down
Loading