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: JSON type filter #3122

Merged
merged 12 commits into from
Oct 11, 2024
3 changes: 3 additions & 0 deletions internal/connor/all.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
// matching if all of them match.
func all(condition, data any) (bool, error) {
switch t := data.(type) {
case []any:
return allSlice(condition, t)

case []string:
return allSlice(condition, t)

Expand Down
3 changes: 3 additions & 0 deletions internal/connor/any.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
// matching if any of them match.
func anyOp(condition, data any) (bool, error) {
switch t := data.(type) {
case []any:
return anySlice(condition, t)

case []string:
return anySlice(condition, t)

Expand Down
139 changes: 77 additions & 62 deletions internal/planner/mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1286,7 +1286,7 @@ func ToFilter(source request.Filter, mapping *core.DocumentMapping) *Filter {
conditions := make(map[connor.FilterKey]any, len(source.Conditions))

for sourceKey, sourceClause := range source.Conditions {
key, clause := toFilterMap(sourceKey, sourceClause, mapping)
key, clause := toFilterKeyValue(sourceKey, sourceClause, mapping)
conditions[key] = clause
}

Expand All @@ -1296,87 +1296,102 @@ func ToFilter(source request.Filter, mapping *core.DocumentMapping) *Filter {
}
}

// toFilterMap converts a consumer-defined filter key-value into a filter clause
// keyed by field index.
// toFilterKeyValue converts a consumer-defined filter key-value into a filter clause
// keyed by connor.FilterKey.
//
// Return key will either be an int (field index), or a string (operator).
func toFilterMap(
// The returned key will be one of the following:
// - Operator: if the sourceKey is one of the defined filter operators
// - PropertyIndex: if the sourceKey exists in the document mapping
// - ObjectProperty: if the sourceKey does not match one of the above
func toFilterKeyValue(
sourceKey string,
sourceClause any,
mapping *core.DocumentMapping,
) (connor.FilterKey, any) {
var returnKey connor.FilterKey
if strings.HasPrefix(sourceKey, "_") && sourceKey != request.DocIDFieldName {
key := &Operator{
returnKey = &Operator{
Operation: sourceKey,
}
// if the operator is simple (not compound) then
// it does not require further expansion
if connor.IsOpSimple(sourceKey) {
return key, sourceClause
}
switch typedClause := sourceClause.(type) {
case []any:
// If the clause is an array then we need to convert any inner maps.
returnClauses := []any{}
for _, innerSourceClause := range typedClause {
var returnClause any
switch typedInnerSourceClause := innerSourceClause.(type) {
case map[string]any:
innerMapClause := map[connor.FilterKey]any{}
for innerSourceKey, innerSourceValue := range typedInnerSourceClause {
rKey, rValue := toFilterMap(innerSourceKey, innerSourceValue, mapping)
innerMapClause[rKey] = rValue
}
returnClause = innerMapClause
default:
returnClause = innerSourceClause
}
returnClauses = append(returnClauses, returnClause)
}
return key, returnClauses
case map[string]any:
innerMapClause := map[connor.FilterKey]any{}
for innerSourceKey, innerSourceValue := range typedClause {
rKey, rValue := toFilterMap(innerSourceKey, innerSourceValue, mapping)
innerMapClause[rKey] = rValue
}
return key, innerMapClause
default:
return key, typedClause
return returnKey, sourceClause
}
} else {
} else if mapping != nil && len(mapping.IndexesByName[sourceKey]) > 0 {
// If there are multiple properties of the same name we can just take the first as
// we have no other reasonable way of identifying which property they mean if multiple
// consumer specified requestables are available. Aggregate dependencies should not
// impact this as they are added after selects.
index := mapping.FirstIndexOfName(sourceKey)
key := &PropertyIndex{
Index: index,
returnKey = &PropertyIndex{
Index: mapping.FirstIndexOfName(sourceKey),
}
switch typedClause := sourceClause.(type) {
case map[string]any:
returnClause := map[connor.FilterKey]any{}
for innerSourceKey, innerSourceValue := range typedClause {
var innerMapping *core.DocumentMapping
// innerSourceValue may refer to a child mapping or
// an inline array if we don't have a child mapping
_, ok := innerSourceValue.(map[string]any)
if ok && index < len(mapping.ChildMappings) {
// If the innerSourceValue is also a map, then we should parse the nested clause
// using the child mapping, as this key must refer to a host property in a join
// and deeper keys must refer to properties on the child items.
innerMapping = mapping.ChildMappings[index]
} else {
innerMapping = mapping
}
rKey, rValue := toFilterMap(innerSourceKey, innerSourceValue, innerMapping)
returnClause[rKey] = rValue
} else {
returnKey = &ObjectProperty{
Name: sourceKey,
}
}

switch typedClause := sourceClause.(type) {
case []any:
return returnKey, toFilterList(typedClause, mapping)

case map[string]any:
return returnKey, toFilterMap(returnKey, typedClause, mapping)

default:
return returnKey, typedClause
}
}

func toFilterMap(
sourceKey connor.FilterKey,
sourceClause map[string]any,
mapping *core.DocumentMapping,
) map[connor.FilterKey]any {
innerMapClause := make(map[connor.FilterKey]any)
for innerSourceKey, innerSourceValue := range sourceClause {
var innerMapping *core.DocumentMapping
switch t := sourceKey.(type) {
case *PropertyIndex:
_, ok := innerSourceValue.(map[string]any)
if ok && mapping != nil && t.Index < len(mapping.ChildMappings) {
// If the innerSourceValue is also a map, then we should parse the nested clause
// using the child mapping, as this key must refer to a host property in a join
// and deeper keys must refer to properties on the child items.
innerMapping = mapping.ChildMappings[t.Index]
} else {
innerMapping = mapping
}
return key, returnClause
default:
return key, sourceClause
case *ObjectProperty:
// Object properties can never refer to mapped document fields.
// Set the mapping to null for any nested filter values so
// that we don't filter any fields outside of this object.
innerMapping = nil
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 comment is very helpful - thanks Keenan

case *Operator:
innerMapping = mapping
}
rKey, rValue := toFilterKeyValue(innerSourceKey, innerSourceValue, innerMapping)
innerMapClause[rKey] = rValue
}
return innerMapClause
}

func toFilterList(sourceClause []any, mapping *core.DocumentMapping) []any {
returnClauses := make([]any, len(sourceClause))
for i, innerSourceClause := range sourceClause {
// innerSourceClause must be a map because only compound
// operators (_and, _or) can reach this function and should
// have already passed GQL type validation
typedInnerSourceClause := innerSourceClause.(map[string]any)
innerMapClause := make(map[connor.FilterKey]any)
for innerSourceKey, innerSourceValue := range typedInnerSourceClause {
rKey, rValue := toFilterKeyValue(innerSourceKey, innerSourceValue, mapping)
innerMapClause[rKey] = rValue
}
returnClauses[i] = innerMapClause
}
return returnClauses
}

func toLimit(limit immutable.Option[uint64], offset immutable.Option[uint64]) *Limit {
Expand Down
37 changes: 37 additions & 0 deletions internal/planner/mapper/targetable.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
var (
_ connor.FilterKey = (*PropertyIndex)(nil)
_ connor.FilterKey = (*Operator)(nil)
_ connor.FilterKey = (*ObjectProperty)(nil)
)

// PropertyIndex is a FilterKey that represents a property in a document.
Expand Down Expand Up @@ -71,6 +72,34 @@
return false
}

// ObjectProperty is a FilterKey that represents a property in an object.
Copy link
Contributor

Choose a reason for hiding this comment

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

todo: Similar to another comment, I think it is important to document why both this and PropertyIndex exist, as outside of the scope of this PR it will look very strange and confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

should be more clear now

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks Keenan

//
// This is used to target properties of an object when the fields
// are not explicitly mapped, such as with JSON.
type ObjectProperty struct {
// The name of the property on object.
Name string
}

func (k *ObjectProperty) GetProp(data any) any {
if data == nil {
return nil
}
object := data.(map[string]any)
return object[k.Name]
}

func (k *ObjectProperty) GetOperatorOrDefault(defaultOp string) string {
return defaultOp
}

func (k *ObjectProperty) Equal(other connor.FilterKey) bool {
if otherKey, isOk := other.(*ObjectProperty); isOk && *k == *otherKey {
return true

Check warning on line 98 in internal/planner/mapper/targetable.go

View check run for this annotation

Codecov / codecov/patch

internal/planner/mapper/targetable.go#L96-L98

Added lines #L96 - L98 were not covered by tests
}
return false

Check warning on line 100 in internal/planner/mapper/targetable.go

View check run for this annotation

Codecov / codecov/patch

internal/planner/mapper/targetable.go#L100

Added line #L100 was not covered by tests
}

// Filter represents a series of conditions that may reduce the number of
// records that a request returns.
type Filter struct {
Expand Down Expand Up @@ -144,6 +173,14 @@
default:
outmap[keyType.Operation] = v
}

case *ObjectProperty:
switch subObj := v.(type) {
case map[connor.FilterKey]any:
outmap[keyType.Name] = filterObjectToMap(mapping, subObj)
case nil:
outmap[keyType.Name] = nil

Check warning on line 182 in internal/planner/mapper/targetable.go

View check run for this annotation

Codecov / codecov/patch

internal/planner/mapper/targetable.go#L181-L182

Added lines #L181 - L182 were not covered by tests
}
}
}
return outmap
Expand Down
72 changes: 38 additions & 34 deletions internal/request/graphql/schema/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -1202,43 +1202,18 @@
}

// generate basic filter operator blocks
// @todo: Extract object field loop into its own utility func
for f, field := range obj.Fields() {
if _, ok := request.ReservedFields[f]; ok && f != request.DocIDFieldName {
_, ok := request.ReservedFields[f]
if ok && f != request.DocIDFieldName {
continue
}
// scalars (leafs)
if gql.IsLeafType(field.Type) {
var operatorName string
if list, isList := field.Type.(*gql.List); isList {
if notNull, isNotNull := list.OfType.(*gql.NonNull); isNotNull {
operatorName = "NotNull" + notNull.OfType.Name() + "ListOperatorBlock"
} else {
operatorName = list.OfType.Name() + "ListOperatorBlock"
}
} else {
operatorName = field.Type.Name() + "OperatorBlock"
}
operatorType, isFilterable := g.manager.schema.TypeMap()[operatorName]
if !isFilterable {
continue
}
fields[field.Name] = &gql.InputObjectFieldConfig{
Type: operatorType,
}
} else { // objects (relations)
fieldType := field.Type
if l, isList := field.Type.(*gql.List); isList {
// We want the FilterArg for the object, not the list of objects.
fieldType = l.OfType
}
filterType, isFilterable := g.manager.schema.TypeMap()[genTypeName(fieldType, filterInputNameSuffix)]
if !isFilterable {
filterType = &gql.InputObjectField{}
}
fields[field.Name] = &gql.InputObjectFieldConfig{
Type: filterType,
}
operatorName := genFilterOperatorName(field.Type)
filterType, isFilterable := g.manager.schema.TypeMap()[operatorName]
if !isFilterable {
continue

Check warning on line 1213 in internal/request/graphql/schema/generate.go

View check run for this annotation

Codecov / codecov/patch

internal/request/graphql/schema/generate.go#L1213

Added line #L1213 was not covered by tests
}
fields[field.Name] = &gql.InputObjectFieldConfig{
Type: filterType,
}
}

Expand Down Expand Up @@ -1408,6 +1383,35 @@
list.OfType == gql.Float
}

func genFilterOperatorName(fieldType gql.Type) string {
list, isList := fieldType.(*gql.List)
if isList {
fieldType = list.OfType
}
if !gql.IsLeafType(fieldType) {
return genTypeName(fieldType, filterInputNameSuffix)
}
notNull, isNotNull := fieldType.(*gql.NonNull)
if isNotNull {
fieldType = notNull.OfType
}
switch {
case fieldType.Name() == "JSON":
return fieldType.Name()

case isList && isNotNull:
// todo: There's a potential to have a name clash
// https://github.com/sourcenetwork/defradb/issues/3123
return "NotNull" + fieldType.Name() + "ListOperatorBlock"
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: It looks like we have a name clash here - I've created a ticket - would you mind adding a todo comment linking to the ticket, and then amend the issue-description to note taht there is a code-todo?

#3123

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.

thanks :)


case isList:
return fieldType.Name() + "ListOperatorBlock"

default:
return fieldType.Name() + "OperatorBlock"
}
}

/* Example

typeDefs := ` ... `
Expand Down
4 changes: 0 additions & 4 deletions internal/request/graphql/schema/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,13 @@ func defaultTypes(
floatOpBlock := schemaTypes.FloatOperatorBlock()
booleanOpBlock := schemaTypes.BooleanOperatorBlock()
stringOpBlock := schemaTypes.StringOperatorBlock()
jsonOpBlock := schemaTypes.JSONOperatorBlock(jsonScalarType)
blobOpBlock := schemaTypes.BlobOperatorBlock(blobScalarType)
dateTimeOpBlock := schemaTypes.DateTimeOperatorBlock()

notNullIntOpBlock := schemaTypes.NotNullIntOperatorBlock()
notNullFloatOpBlock := schemaTypes.NotNullFloatOperatorBlock()
notNullBooleanOpBlock := schemaTypes.NotNullBooleanOperatorBlock()
notNullStringOpBlock := schemaTypes.NotNullStringOperatorBlock()
notNullJSONOpBlock := schemaTypes.NotNullJSONOperatorBlock(jsonScalarType)
notNullBlobOpBlock := schemaTypes.NotNullBlobOperatorBlock(blobScalarType)

return []gql.Type{
Expand All @@ -228,7 +226,6 @@ func defaultTypes(
floatOpBlock,
booleanOpBlock,
stringOpBlock,
jsonOpBlock,
blobOpBlock,
dateTimeOpBlock,

Expand All @@ -237,7 +234,6 @@ func defaultTypes(
notNullFloatOpBlock,
notNullBooleanOpBlock,
notNullStringOpBlock,
notNullJSONOpBlock,
notNullBlobOpBlock,

// Filter scalar list blocks
Expand Down
Loading
Loading