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: Default scalar field values #2997

Merged
merged 10 commits into from
Sep 16, 2024
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
7 changes: 7 additions & 0 deletions client/collection_field_description.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ type CollectionFieldDescription struct {
//
// Otherwise will be [None].
RelationName immutable.Option[string]

// DefaultValue contains the default value for this field.
//
// This value has no effect on views.
DefaultValue any
Copy link
Contributor

Choose a reason for hiding this comment

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

todo: Please test the PatchCollection behaviour for this new property

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

todo: Please test and document what happens when this is set on a View - and consider adding a validation rule to prevent that (as it makes no sense to me) (and test that, including on Patch).

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a test to show the current behavior. Since it has no effect on the results of the view, I think allowing it is better than having the schema definition and view definition languages diverge and have slightly different rules.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay cheers, I'm happy with that :) - can you please document that the property is ignored for views on the directive, and on this CollectionFieldDescription prop so that users know?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

}

func (f FieldID) String() string {
Expand All @@ -50,6 +55,7 @@ type collectionFieldDescription struct {
Name string
ID FieldID
RelationName immutable.Option[string]
DefaultValue any

// Properties below this line are unmarshalled using custom logic in [UnmarshalJSON]
Kind json.RawMessage
Expand All @@ -64,6 +70,7 @@ func (f *CollectionFieldDescription) UnmarshalJSON(bytes []byte) error {

f.Name = descMap.Name
f.ID = descMap.ID
f.DefaultValue = descMap.DefaultValue
f.RelationName = descMap.RelationName
kind, err := parseFieldKind(descMap.Kind)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions client/definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ type FieldDefinition struct {

// If true, this is the primary half of a relation, otherwise is false.
IsPrimaryRelation bool

// DefaultValue contains the default value for this field.
DefaultValue any
}

// NewFieldDefinition returns a new [FieldDefinition], combining the given local and global elements
Expand All @@ -164,6 +167,7 @@ func NewFieldDefinition(local CollectionFieldDescription, global SchemaFieldDesc
RelationName: local.RelationName.Value(),
Typ: global.Typ,
IsPrimaryRelation: kind.IsObject() && !kind.IsArray(),
DefaultValue: local.DefaultValue,
}
}

Expand All @@ -174,6 +178,7 @@ func NewLocalFieldDefinition(local CollectionFieldDescription) FieldDefinition {
ID: local.ID,
Kind: local.Kind.Value(),
RelationName: local.RelationName.Value(),
DefaultValue: local.DefaultValue,
}
}

Expand Down
48 changes: 38 additions & 10 deletions client/document.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,25 +84,34 @@
collectionDefinition CollectionDefinition
}

func newEmptyDoc(collectionDefinition CollectionDefinition) *Document {
return &Document{
func newEmptyDoc(collectionDefinition CollectionDefinition) (*Document, error) {
doc := &Document{
fields: make(map[string]Field),
values: make(map[Field]*FieldValue),
collectionDefinition: collectionDefinition,
}
if err := doc.setDefaultValues(); err != nil {
return nil, err

Check warning on line 94 in client/document.go

View check run for this annotation

Codecov / codecov/patch

client/document.go#L94

Added line #L94 was not covered by tests
}
return doc, nil
}

// NewDocWithID creates a new Document with a specified key.
func NewDocWithID(docID DocID, collectionDefinition CollectionDefinition) *Document {
doc := newEmptyDoc(collectionDefinition)
func NewDocWithID(docID DocID, collectionDefinition CollectionDefinition) (*Document, error) {
doc, err := newEmptyDoc(collectionDefinition)
if err != nil {
return nil, err

Check warning on line 103 in client/document.go

View check run for this annotation

Codecov / codecov/patch

client/document.go#L103

Added line #L103 was not covered by tests
}
doc.id = docID
return doc
return doc, nil
}

// NewDocFromMap creates a new Document from a data map.
func NewDocFromMap(data map[string]any, collectionDefinition CollectionDefinition) (*Document, error) {
var err error
doc := newEmptyDoc(collectionDefinition)
doc, err := newEmptyDoc(collectionDefinition)
if err != nil {
return nil, err

Check warning on line 113 in client/document.go

View check run for this annotation

Codecov / codecov/patch

client/document.go#L113

Added line #L113 was not covered by tests
}

// check if document contains special _docID field
k, hasDocID := data[request.DocIDFieldName]
Expand Down Expand Up @@ -142,8 +151,11 @@

// NewFromJSON creates a new instance of a Document from a raw JSON object byte array.
func NewDocFromJSON(obj []byte, collectionDefinition CollectionDefinition) (*Document, error) {
doc := newEmptyDoc(collectionDefinition)
err := doc.SetWithJSON(obj)
doc, err := newEmptyDoc(collectionDefinition)
if err != nil {
return nil, err

Check warning on line 156 in client/document.go

View check run for this annotation

Codecov / codecov/patch

client/document.go#L156

Added line #L156 was not covered by tests
}
err = doc.SetWithJSON(obj)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -172,7 +184,10 @@
if err != nil {
return nil, err
}
doc := newEmptyDoc(collectionDefinition)
doc, err := newEmptyDoc(collectionDefinition)
if err != nil {
return nil, err

Check warning on line 189 in client/document.go

View check run for this annotation

Codecov / codecov/patch

client/document.go#L189

Added line #L189 was not covered by tests
}
err = doc.setWithFastJSONObject(o)
if err != nil {
return nil, err
Expand Down Expand Up @@ -653,6 +668,19 @@
return nil
}

func (doc *Document) setDefaultValues() error {
for _, field := range doc.collectionDefinition.GetFields() {
if field.DefaultValue == nil {
continue // no default value to set
}
err := doc.Set(field.Name, field.DefaultValue)
if err != nil {
return err

Check warning on line 678 in client/document.go

View check run for this annotation

Codecov / codecov/patch

client/document.go#L678

Added line #L678 was not covered by tests
}
}
return nil
}

// Fields gets the document fields as a map.
func (doc *Document) Fields() map[string]Field {
doc.mu.RLock()
Expand Down
2 changes: 2 additions & 0 deletions docs/website/references/http/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
"Fields": {
"items": {
"properties": {
"DefaultValue": {},
"ID": {
"maximum": 4294967295,
"minimum": 0,
Expand Down Expand Up @@ -160,6 +161,7 @@
"Fields": {
"items": {
"properties": {
"DefaultValue": {},
"ID": {
"maximum": 4294967295,
"minimum": 0,
Expand Down
5 changes: 4 additions & 1 deletion http/client_collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,10 @@
if err != nil {
return nil, err
}
doc := client.NewDocWithID(docID, c.def)
doc, err := client.NewDocWithID(docID, c.def)
if err != nil {
return nil, err

Check warning on line 312 in http/client_collection.go

View check run for this annotation

Codecov / codecov/patch

http/client_collection.go#L312

Added line #L312 was not covered by tests
}
err = doc.SetWithJSON(data)
if err != nil {
return nil, err
Expand Down
5 changes: 4 additions & 1 deletion internal/db/fetcher/encoded_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@
return nil, err
}

doc := client.NewDocWithID(docID, collectionDefinition)
doc, err := client.NewDocWithID(docID, collectionDefinition)
if err != nil {
return nil, err

Check warning on line 117 in internal/db/fetcher/encoded_doc.go

View check run for this annotation

Codecov / codecov/patch

internal/db/fetcher/encoded_doc.go#L117

Added line #L117 was not covered by tests
}
properties, err := encdoc.Properties(false)
if err != nil {
return nil, err
Expand Down
81 changes: 69 additions & 12 deletions internal/request/graphql/schema/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"sort"
"strings"

gql "github.com/sourcenetwork/graphql-go"
"github.com/sourcenetwork/graphql-go/language/ast"
gqlp "github.com/sourcenetwork/graphql-go/language/parser"
"github.com/sourcenetwork/graphql-go/language/source"
Expand All @@ -26,6 +27,29 @@
"github.com/sourcenetwork/defradb/internal/request/graphql/schema/types"
)

const (
typeID string = "ID"
typeBoolean string = "Boolean"
typeInt string = "Int"
typeFloat string = "Float"
typeDateTime string = "DateTime"
typeString string = "String"
typeBlob string = "Blob"
typeJSON string = "JSON"
)

// this mapping is used to check that the default prop value
// matches the field type
var TypeToDefaultPropName = map[string]string{
typeString: types.DefaultDirectivePropString,
typeBoolean: types.DefaultDirectivePropBool,
typeInt: types.DefaultDirectivePropInt,
typeFloat: types.DefaultDirectivePropFloat,
typeDateTime: types.DefaultDirectivePropDateTime,
typeJSON: types.DefaultDirectivePropJSON,
typeBlob: types.DefaultDirectivePropBlob,
}

// FromString parses a GQL SDL string into a set of collection descriptions.
func FromString(ctx context.Context, schemaString string) (
[]client.CollectionDefinition,
Expand Down Expand Up @@ -369,6 +393,39 @@
}, nil
}

func defaultFromAST(
field *ast.FieldDefinition,
directive *ast.Directive,
) (any, error) {
astNamed, ok := field.Type.(*ast.Named)
if !ok {
return nil, NewErrDefaultValueNotAllowed(field.Name.Value, field.Type.String())
Copy link
Contributor

Choose a reason for hiding this comment

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

todo: Please add tests for this error condition

Copy link
Member Author

Choose a reason for hiding this comment

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

done

}
propName, ok := TypeToDefaultPropName[astNamed.Name.Value]
if !ok {
return nil, NewErrDefaultValueNotAllowed(field.Name.Value, astNamed.Name.Value)
}
var value any
for _, arg := range directive.Arguments {
if propName != arg.Name.Value {
return nil, NewErrDefaultValueInvalid(field.Name.Value, propName, arg.Name.Value)
}
switch t := arg.Value.(type) {
case *ast.IntValue:
value = gql.Int.ParseLiteral(arg.Value)
case *ast.FloatValue:
value = gql.Float.ParseLiteral(arg.Value)

Check warning on line 417 in internal/request/graphql/schema/collection.go

View check run for this annotation

Codecov / codecov/patch

internal/request/graphql/schema/collection.go#L416-L417

Added lines #L416 - L417 were not covered by tests
case *ast.BooleanValue:
value = t.Value
case *ast.StringValue:
value = t.Value
default:
value = arg.Value.GetValue()

Check warning on line 423 in internal/request/graphql/schema/collection.go

View check run for this annotation

Codecov / codecov/patch

internal/request/graphql/schema/collection.go#L422-L423

Added lines #L422 - L423 were not covered by tests
}
}
return value, nil
}

func fieldsFromAST(
field *ast.FieldDefinition,
hostObjectName string,
Expand All @@ -392,6 +449,16 @@
}
hostMap[field.Name.Value] = cType

var defaultValue any
for _, directive := range field.Directives {
if directive.Name.Value == types.DefaultDirectiveLabel {
defaultValue, err = defaultFromAST(field, directive)
if err != nil {
return nil, nil, err
}
}
}

schemaFieldDescriptions := []client.SchemaFieldDescription{}
collectionFieldDescriptions := []client.CollectionFieldDescription{}

Expand Down Expand Up @@ -467,7 +534,8 @@
collectionFieldDescriptions = append(
collectionFieldDescriptions,
client.CollectionFieldDescription{
Name: field.Name.Value,
Name: field.Name.Value,
DefaultValue: defaultValue,
},
)
}
Expand Down Expand Up @@ -529,17 +597,6 @@
}

func astTypeToKind(t ast.Type) (client.FieldKind, error) {
const (
typeID string = "ID"
typeBoolean string = "Boolean"
typeInt string = "Int"
typeFloat string = "Float"
typeDateTime string = "DateTime"
typeString string = "String"
typeBlob string = "Blob"
typeJSON string = "JSON"
)

Comment on lines -532 to -542
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 moving

switch astTypeVal := t.(type) {
case *ast.List:
switch innerAstTypeVal := astTypeVal.Type.(type) {
Expand Down
19 changes: 19 additions & 0 deletions internal/request/graphql/schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ const (
errPolicyUnknownArgument string = "policy with unknown argument"
errPolicyInvalidIDProp string = "policy directive with invalid id property"
errPolicyInvalidResourceProp string = "policy directive with invalid resource property"
errDefaultValueInvalid string = "default value type must match field type"
errDefaultValueNotAllowed string = "default value is not allowed for this field type"
)

var (
Expand Down Expand Up @@ -136,3 +138,20 @@ func NewErrRelationNotFound(relationName string) error {
errors.NewKV("RelationName", relationName),
)
}

func NewErrDefaultValueInvalid(name string, expected string, actual string) error {
return errors.New(
errDefaultValueInvalid,
errors.NewKV("Name", name),
errors.NewKV("Expected", expected),
errors.NewKV("Actual", actual),
)
}

func NewErrDefaultValueNotAllowed(fieldName, fieldType string) error {
return errors.New(
errDefaultValueNotAllowed,
errors.NewKV("Name", fieldName),
errors.NewKV("Type", fieldType),
)
}
3 changes: 2 additions & 1 deletion internal/request/graphql/schema/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,8 @@ func (g *Generator) buildMutationInputTypes(collections []client.CollectionDefin
}

fields[field.Name] = &gql.InputObjectFieldConfig{
Type: ttype,
Type: ttype,
DefaultValue: field.DefaultValue,
}
}

Expand Down
1 change: 1 addition & 0 deletions internal/request/graphql/schema/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func defaultDirectivesType(
) []*gql.Directive {
return []*gql.Directive{
schemaTypes.CRDTFieldDirective(crdtEnum),
schemaTypes.DefaultDirective(),
schemaTypes.ExplainDirective(explainEnum),
schemaTypes.PolicyDirective(),
schemaTypes.IndexDirective(orderEnum, indexFieldInput),
Expand Down
Loading
Loading