Skip to content

Commit

Permalink
Update async handling for Parse operations
Browse files Browse the repository at this point in the history
  • Loading branch information
DZakh committed Jul 7, 2024
1 parent 651f495 commit f7f9502
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG_NEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
- Move operations from functions to Schema methods
- Add `serializeToJsonOrThrow`
- Update operation type to be more detailed and feature it in the error message.
- S.union still doesn't support schemas with async, but treats them differently. Please don't try to use them, since the behavior is not predictable.
1 change: 0 additions & 1 deletion IDEAS.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ let trimContract: S.contract<string => string> = S.contract(s => {

- Change operation to include AsyncParse and simplify init functions (throw when asyncTransfor applied for SyncParse)
- Make S.serializeToJsonString super fast
- Make operations more treeshakable by starting passing the actual operation to the initialOperation function. Or add a condition (verify performance)
- Rename `InvalidJsonStruct` error, since after `rescript-struct`->`rescript-schema` it became misleading
- Add S.bigint

Expand Down
4 changes: 2 additions & 2 deletions packages/tests/src/core/S_Error_message_test.res
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ test("UnexpectedAsync error", t => {
operation: Parse,
path: S.Path.empty,
})->S.Error.message,
"Failed parsing at root. Reason: Encountered unexpected asynchronous transform or refine. Use S.parseAsyncWith instead of S.parseWith",
"Failed parsing at root. Reason: Encountered unexpected async transform or refine. Use ParseAsync operation instead",
(),
)
})
Expand All @@ -91,7 +91,7 @@ test("ExcessField error", t => {
operation: Parse,
path: S.Path.empty,
})->S.Error.message,
`Failed parsing at root. Reason: Encountered disallowed excess key "unknownKey" on an object. Use Deprecated to ignore a specific field, or S.Object.strip to ignore excess keys completely`,
`Failed parsing at root. Reason: Encountered disallowed excess key "unknownKey" on an object`,
(),
)
})
Expand Down
37 changes: 29 additions & 8 deletions packages/tests/src/core/S_parseAnyAsyncInStepsWith_test.res
Original file line number Diff line number Diff line change
Expand Up @@ -478,17 +478,38 @@ module Union = {
])->Promise.thenResolve(_ => ())
})

test("[Union] Fails to parse since it's not supported", t => {
test("[Union] Passes with Parse operation. Async item should fail", t => {
let schema = S.union([S.literal(2)->validAsyncRefine, S.literal(2), S.literal(3)])

t->Assert.deepEqual(2->S.parseAnyWith(schema), Ok(2), ())
})

test("[Union] Fails with Parse operation", t => {
let schema = S.union([S.literal(1), S.literal(2)->validAsyncRefine, S.literal(3)])

t->Assert.throws(
() => {
1->S.parseAnyOrRaiseWith(schema)
},
~expectations={
message: "Failed parsing at root. Reason: S.union doesn\'t support async items. Please create an issue to rescript-schema if you nead the feature",
t->U.assertErrorResult(
2->S.parseAnyWith(schema),
{
code: InvalidUnion([
U.error({
code: InvalidLiteral({expected: S.Literal.parse(1.), received: %raw("2")}),
path: S.Path.empty,
operation: Parse,
}),
U.error({
code: UnexpectedAsync,
path: S.Path.empty,
operation: Parse,
}),
U.error({
code: InvalidLiteral({expected: S.Literal.parse(3.), received: %raw("2")}),
path: S.Path.empty,
operation: Parse,
}),
]),
operation: Parse,
path: S.Path.empty,
},
(),
)
})

Expand Down
2 changes: 1 addition & 1 deletion packages/tests/src/core/S_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ test("Fails to parse strict object with exccess fields", (t) => {
},
{
name: "RescriptSchemaError",
message: `Failed parsing at root. Reason: Encountered disallowed excess key "bar" on an object. Use Deprecated to ignore a specific field, or S.Object.strip to ignore excess keys completely`,
message: `Failed parsing at root. Reason: Encountered disallowed excess key "bar" on an object`,
}
);
});
Expand Down
24 changes: 10 additions & 14 deletions src/S_Core.bs.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -224,24 +224,27 @@ function transform(b, input, operation) {
) + "})");
}

function raise(b, code, path) {
throw new RescriptSchemaError(code, b.g.o, path);
}

function embedSyncOperation(b, input, fn) {
return transform(b, input, (function (b, input) {
return map(b, "e[" + (b.g.e.push(fn) - 1) + "]", input);
}));
}

function embedAsyncOperation(b, input, fn) {
if (b.g.o !== "ParseAsync") {
raise(b, "UnexpectedAsync", "");
}
return transform(b, input, (function (b, input) {
var val = map(b, "e[" + (b.g.e.push(fn) - 1) + "]", input);
val.a = true;
return val;
}));
}

function raise(b, code, path) {
throw new RescriptSchemaError(code, b.g.o, path);
}

function failWithArg(b, path, fn, arg) {
return "e[" + (b.g.e.push(function (arg) {
return raise(b, fn(arg), path);
Expand Down Expand Up @@ -376,10 +379,6 @@ function noopOperation(i) {
return i;
}

function unexpectedAsyncOperation(param) {
throw new RescriptSchemaError("UnexpectedAsync", "Parse", "");
}

function build(builder, schema, operation) {
var b = {
c: "",
Expand Down Expand Up @@ -407,9 +406,6 @@ function build(builder, schema, operation) {
}
schema.i = output.a;
}
if (operation === "Parse" && output.a) {
return unexpectedAsyncOperation;
}
if (b.c === "" && output === input) {
return noopOperation;
}
Expand Down Expand Up @@ -624,7 +620,7 @@ function isAsyncParse(schema) {
return v;
}
try {
build(schema.p, schema, "Parse");
build(schema.p, schema, "ParseAsync");
return schema.i;
}
catch (raw_exn){
Expand Down Expand Up @@ -2171,7 +2167,7 @@ function reason(error, nestedLevelOpt) {
var nestedLevel = nestedLevelOpt !== undefined ? nestedLevelOpt : 0;
var reason$1 = error.code;
if (typeof reason$1 !== "object") {
return "Encountered unexpected asynchronous transform or refine. Use S.parseAsyncWith instead of S.parseWith";
return "Encountered unexpected async transform or refine. Use ParseAsync operation instead";
}
switch (reason$1.TAG) {
case "OperationFailed" :
Expand All @@ -2183,7 +2179,7 @@ function reason(error, nestedLevelOpt) {
case "InvalidLiteral" :
return "Expected " + reason$1.expected.s + ", received " + parseInternal(reason$1.received).s;
case "ExcessField" :
return "Encountered disallowed excess key " + JSON.stringify(reason$1._0) + " on an object. Use Deprecated to ignore a specific field, or S.Object.strip to ignore excess keys completely";
return "Encountered disallowed excess key " + JSON.stringify(reason$1._0) + " on an object";
case "InvalidUnion" :
var lineBreak = "\n" + " ".repeat((nestedLevel << 1));
var reasonsDict = {};
Expand Down
28 changes: 11 additions & 17 deletions src/S_Core.res
Original file line number Diff line number Diff line change
Expand Up @@ -547,24 +547,27 @@ module Builder = {
}
}

let raise = (b: b, ~code, ~path) => {
Stdlib.Exn.raiseAny(InternalError.make(~code, ~operation=b.global.operation, ~path))
}

let embedSyncOperation = (b: b, ~input, ~fn: 'input => 'output) => {
b->transform(~input, (b, ~input) => {
b->Val.map(b->embed(fn), input)
})
}

let embedAsyncOperation = (b: b, ~input, ~fn: 'input => unit => promise<'output>) => {
if b.global.operation !== ParseAsync {
b->raise(~code=UnexpectedAsync, ~path=Path.empty)
}
b->transform(~input, (b, ~input) => {
let val = b->Val.map(b->embed(fn), input)
val.isAsync = true
val
})
}

let raise = (b: b, ~code, ~path) => {
Stdlib.Exn.raiseAny(InternalError.make(~code, ~operation=b.global.operation, ~path))
}

let failWithArg = (b: b, ~path, fn: 'arg => errorCode, arg) => {
`${b->embed(arg => {
b->raise(~path, ~code=fn(arg))
Expand Down Expand Up @@ -721,11 +724,6 @@ module Builder = {

let noopOperation = i => i->Obj.magic

let unexpectedAsyncOperation = _ =>
Stdlib.Exn.raiseAny(
InternalError.make(~path=Path.empty, ~code=UnexpectedAsync, ~operation=Parse),
)

@inline
let intitialInputVar = "i"

Expand Down Expand Up @@ -757,11 +755,7 @@ module Builder = {
schema.isAsyncParse = Value(output.isAsync)
}

// FIXME:

if operation === Parse && output.isAsync {
unexpectedAsyncOperation
} else if b.code === "" && output === input {
if b.code === "" && output === input {
noopOperation
} else {
let inlinedFunction = `${intitialInputVar}=>{${b.code}return ${b->B.Val.inline(output)}}`
Expand Down Expand Up @@ -1059,7 +1053,7 @@ let isAsyncParse = schema => {
switch schema.isAsyncParse {
| Unknown =>
try {
let _ = schema.parseOperationBuilder->Builder.build(~schema, ~operation=Parse)
let _ = schema.parseOperationBuilder->Builder.build(~schema, ~operation=ParseAsync)
schema.isAsyncParse->(Obj.magic: isAsyncParse => bool)
} catch {
| exn => {
Expand Down Expand Up @@ -3138,9 +3132,9 @@ module Error = {
switch error.code {
| OperationFailed(reason) => reason
| InvalidOperation({description}) => description
| UnexpectedAsync => "Encountered unexpected asynchronous transform or refine. Use S.parseAsyncWith instead of S.parseWith"
| UnexpectedAsync => "Encountered unexpected async transform or refine. Use ParseAsync operation instead"
| ExcessField(fieldName) =>
`Encountered disallowed excess key ${fieldName->Stdlib.Inlined.Value.fromString} on an object. Use Deprecated to ignore a specific field, or S.Object.strip to ignore excess keys completely`
`Encountered disallowed excess key ${fieldName->Stdlib.Inlined.Value.fromString} on an object`
| InvalidType({expected, received}) =>
`Expected ${expected.name()}, received ${received->Literal.parse->Literal.toString}`
| InvalidLiteral({expected, received}) =>
Expand Down

2 comments on commit f7f9502

@github-actions
Copy link

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: f7f9502 Previous: 06df40b Ratio
Parse string 816102503 ops/sec (±0.10%) 818372259 ops/sec (±0.13%) 1.00
Serialize string 816705609 ops/sec (±0.07%) 819110535 ops/sec (±0.05%) 1.00
Advanced object schema factory 463381 ops/sec (±0.68%) 449156 ops/sec (±0.56%) 0.97
Parse advanced object 51291363 ops/sec (±0.41%) 45100200 ops/sec (±0.16%) 0.88
Create and parse advanced object 90485 ops/sec (±0.75%) 35041 ops/sec (±1.18%) 0.39
Parse advanced strict object 24233651 ops/sec (±0.30%) 22748126 ops/sec (±0.24%) 0.94
Serialize advanced object 75366017 ops/sec (±0.14%) 806595235 ops/sec (±0.11%) 10.70

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: f7f9502 Previous: 06df40b Ratio
Serialize advanced object 75366017 ops/sec (±0.14%) 806595235 ops/sec (±0.11%) 10.70

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.