-
Notifications
You must be signed in to change notification settings - Fork 43
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 blob scalar type #2091
Changes from all commits
cc801b5
d4ebf91
70bf6c3
92dce71
d03da82
212dfec
da25d3c
cf090ae
9cb51fc
d67c1be
a540518
95c453d
389ffef
1aa1a26
8089b49
871990c
438894b
a46fad6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
// Copyright 2023 Democratized Data Foundation | ||
// | ||
// Use of this software is governed by the Business Source License | ||
// included in the file licenses/BSL.txt. | ||
// | ||
// As of the Change Date specified in that file, in accordance with | ||
// the Business Source License, use of this software will be governed | ||
// by the Apache License, Version 2.0, included in the file | ||
// licenses/APL.txt. | ||
|
||
package types | ||
|
||
import ( | ||
"encoding/hex" | ||
"regexp" | ||
|
||
"github.com/sourcenetwork/graphql-go" | ||
"github.com/sourcenetwork/graphql-go/language/ast" | ||
) | ||
|
||
// BlobPattern is a regex for validating blob hex strings | ||
var BlobPattern = regexp.MustCompile("^[0-9a-fA-F]+$") | ||
|
||
// coerceBlob converts the given value into a valid hex string. | ||
// If the value cannot be converted nil is returned. | ||
func coerceBlob(value any) any { | ||
switch value := value.(type) { | ||
case []byte: | ||
return hex.EncodeToString(value) | ||
|
||
case *[]byte: | ||
return coerceBlob(*value) | ||
|
||
case string: | ||
if !BlobPattern.MatchString(value) { | ||
return nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Why does this return nil instead of EDIT:
todo: If so, please document There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just following how the other types work in the |
||
} | ||
return value | ||
|
||
case *string: | ||
return coerceBlob(*value) | ||
|
||
default: | ||
return nil | ||
} | ||
} | ||
|
||
var BlobScalarType = graphql.NewScalar(graphql.ScalarConfig{ | ||
Name: "Blob", | ||
Description: "The `Blob` scalar type represents a binary large object.", | ||
// Serialize converts the value to a hex string | ||
Serialize: coerceBlob, | ||
// ParseValue converts the value to a hex string | ||
ParseValue: coerceBlob, | ||
// ParseLiteral converts the ast value to a hex string | ||
ParseLiteral: func(valueAST ast.Value) any { | ||
switch valueAST := valueAST.(type) { | ||
case *ast.StringValue: | ||
return coerceBlob(valueAST.Value) | ||
default: | ||
// return nil if the value cannot be parsed | ||
return nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. todo: Similar to another comment, please document this - it looks like a silent error, but I only know that by guessing given the:
stuff in other code blocks |
||
} | ||
}, | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
// Copyright 2023 Democratized Data Foundation | ||
// | ||
// Use of this software is governed by the Business Source License | ||
// included in the file licenses/BSL.txt. | ||
// | ||
// As of the Change Date specified in that file, in accordance with | ||
// the Business Source License, use of this software will be governed | ||
// by the Apache License, Version 2.0, included in the file | ||
// licenses/APL.txt. | ||
|
||
package types | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/sourcenetwork/graphql-go/language/ast" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestBlobScalarTypeSerialize(t *testing.T) { | ||
stringInput := "00ff" | ||
bytesInput := []byte{0, 255} | ||
|
||
cases := []struct { | ||
input any | ||
expect any | ||
}{ | ||
{stringInput, "00ff"}, | ||
{&stringInput, "00ff"}, | ||
{bytesInput, "00ff"}, | ||
{&bytesInput, "00ff"}, | ||
{nil, nil}, | ||
{0, nil}, | ||
{false, nil}, | ||
} | ||
for _, c := range cases { | ||
result := BlobScalarType.Serialize(c.input) | ||
assert.Equal(t, c.expect, result) | ||
} | ||
} | ||
|
||
func TestBlobScalarTypeParseValue(t *testing.T) { | ||
stringInput := "00ff" | ||
bytesInput := []byte{0, 255} | ||
// invalid string containing non-hex characters | ||
invalidHexString := "!@#$%^&*" | ||
|
||
cases := []struct { | ||
input any | ||
expect any | ||
}{ | ||
{stringInput, "00ff"}, | ||
{&stringInput, "00ff"}, | ||
{bytesInput, "00ff"}, | ||
{&bytesInput, "00ff"}, | ||
{invalidHexString, nil}, | ||
{&invalidHexString, nil}, | ||
{nil, nil}, | ||
{0, nil}, | ||
{false, nil}, | ||
} | ||
for _, c := range cases { | ||
result := BlobScalarType.ParseValue(c.input) | ||
assert.Equal(t, c.expect, result) | ||
} | ||
} | ||
|
||
func TestBlobScalarTypeParseLiteral(t *testing.T) { | ||
cases := []struct { | ||
input ast.Value | ||
expect any | ||
}{ | ||
{&ast.StringValue{Value: "00ff"}, "00ff"}, | ||
{&ast.StringValue{Value: "00!@#$%^&*"}, nil}, | ||
{&ast.StringValue{Value: "!@#$%^&*00"}, nil}, | ||
{&ast.IntValue{}, nil}, | ||
{&ast.BooleanValue{}, nil}, | ||
{&ast.NullValue{}, nil}, | ||
{&ast.EnumValue{}, nil}, | ||
{&ast.FloatValue{}, nil}, | ||
{&ast.ListValue{}, nil}, | ||
{&ast.ObjectValue{}, nil}, | ||
} | ||
for _, c := range cases { | ||
result := BlobScalarType.ParseLiteral(c.input) | ||
assert.Equal(t, c.expect, result) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
// Copyright 2023 Democratized Data Foundation | ||
// | ||
// Use of this software is governed by the Business Source License | ||
// included in the file licenses/BSL.txt. | ||
// | ||
// As of the Change Date specified in that file, in accordance with | ||
// the Business Source License, use of this software will be governed | ||
// by the Apache License, Version 2.0, included in the file | ||
// licenses/APL.txt. | ||
|
||
package field_kinds | ||
|
||
import ( | ||
"testing" | ||
|
||
testUtils "github.com/sourcenetwork/defradb/tests/integration" | ||
) | ||
|
||
func TestMutationUpdate_WithBlobField(t *testing.T) { | ||
test := testUtils.TestCase{ | ||
Description: "Simple update of blob field", | ||
Actions: []any{ | ||
testUtils.SchemaUpdate{ | ||
Schema: ` | ||
type Users { | ||
name: String | ||
data: Blob | ||
} | ||
`, | ||
}, | ||
testUtils.CreateDoc{ | ||
Doc: `{ | ||
"name": "John", | ||
"data": "00FE" | ||
}`, | ||
}, | ||
testUtils.UpdateDoc{ | ||
Doc: `{ | ||
"data": "00FF" | ||
}`, | ||
}, | ||
testUtils.Request{ | ||
Request: ` | ||
query { | ||
Users { | ||
data | ||
} | ||
} | ||
`, | ||
Results: []map[string]any{ | ||
{ | ||
"data": "00FF", | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
|
||
testUtils.ExecuteTestCase(t, test) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: Seeing this make me wonder if the type should be added to the
graphql-go
package.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to that. Should we move the other types there as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me nervous, I think we added DateTime to that package, but
graphql-go
is supposed to be a general purpose gql package, not a defra package. And we may/do want to get rid of it/replace it at somepointThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, DateTime was already defined as a scalar in the
graphql-go
package, but there were some problems with it as it relates to Defra integration, there was PR that got merged on thegraphql-go
package related to the DateTime stuff, but it wasn't the implementation/definition of the DateTime scalar. (reference: #931 and sourcenetwork/graphql-go#8)I do agree though that we shouldn't have to define the scalar in the
graphql-go
package, it exposes the necessary types/functions for us to define our own scalars outside that package, exactly as they're being used here. Moreover, the fact that we have our own defined custom scalars implies that w.e package we may replace thegraphql-go
package with, should implement the "base" scalars as defined in the GQL spec, and all we have to worry about is the specific "custom" scalars that are neatly organized in a single place.