fix!: Limit built-in scalars to the ones defined by the spec #158
Conversation
stuebingerb
force-pushed
the
fix/fix-sdl-for-scalars
branch
2 times, most recently
from
349f10f to
2331c76
Compare
| @@ -140,6 +141,12 @@ class SchemaBuilder internal constructor() { | |||
| booleanScalar(T::class, block) | |||
| } | |||
|
|
|||
| fun extendedScalars() { | |||
There was a problem hiding this comment.
Are there tests specifying that with not invoking this, the additional scalars are not present?
There was a problem hiding this comment.
I've seen all tests fail that I adjusted but it would make sense to have it validated explicitly. Will update but I have found some strange behavior with the implemented coercions that I want to analyze first.
There was a problem hiding this comment.
This looks more complicated: #160
But I have at least added some validation
Partially resolves #83 by splitting up the current `BuiltInScalars` into two: - The actual built-in scalars as defined by the spec - Some extended scalars supported by KGraphQL BREAKING CHANGE: Those extended scalars are no longer automatically added to a schema but must be manually included using `extendedScalars()` if needed. BREAKING CHANGE: Shorts are returned as numeric value, no longer as string.
stuebingerb
force-pushed
the
fix/fix-sdl-for-scalars
branch
from
2331c76 to
5bcffd4
Compare
|
|
||
| val response = deserialize(schema.executeBlocking("{ int float double string boolean }")) | ||
| assertThat(response.extract<Int>("data/int"), equalTo(1)) | ||
| assertThat(response.extract<Float>("data/float"), equalTo(2.0)) |
There was a problem hiding this comment.
IMHO this should be equalTo(2.0f) but haven't found out why that doesn't work.
|
|
||
| val response = deserialize(schema.executeBlocking("{ long short }")) | ||
| assertThat(response.extract<Long>("data/long"), equalTo(9223372036854775807L)) | ||
| assertThat(response.extract<Int>("data/short"), equalTo(2)) |
There was a problem hiding this comment.
IMHO this should be extract<Short> and equalTo(2.toShort()) but haven't found out why that doesn't work. There is also a cast in TestUtils.extract that apparently successfully casts an Int to a Short, which shouldn't be possible. Hopefully that will all become clearer during work on #160.
Partially resolves #83 by splitting up the current
BuiltInScalarsinto two:BREAKING CHANGE: Those extended scalars are no longer automatically added to a schema but must be manually included using
extendedScalars()if needed.BREAKING CHANGE: Shorts are returned as numeric value, no longer as string.