feat: support for default values in `InputFieldDefinition` #2117

shashitnak · 2024-06-05T11:13:24Z

Summary:
Briefly describe the changes made in this PR.

Issue Reference(s):
Fixes #... (Replace "..." with the issue number)

Build & Testing:

I ran cargo test successfully.
I have run ./lint.sh --mode=fix to fix all linting issues raised by ./lint.sh --mode=check.

Checklist:

I have added relevant unit & integration tests.
I have updated the documentation accordingly.
I have performed a self-review of my code.
PR follows the naming convention of <type>(<optional scope>): <title>

codecov · 2024-06-05T11:16:35Z

Codecov Report

Attention: Patch coverage is 95.71429% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.18%. Comparing base (2234777) to head (b268722).

Files	Patch %	Lines
src/core/blueprint/into_schema.rs	85.71%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2117      +/-   ##
==========================================
+ Coverage   84.12%   84.18%   +0.05%     
==========================================
  Files         212      212              
  Lines       20054    20108      +54     
==========================================
+ Hits        16870    16927      +57     
+ Misses       3184     3181       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

meskill · 2024-06-05T13:40:43Z

src/core/blueprint/blueprint.rs

@@ -262,3 +263,73 @@ impl Blueprint {
        schema.finish().unwrap()
    }
 }
+
+#[cfg(test)]
+mod tests {


I think proper .md test will be more descriptive and valuable here since with unit test we limited only to config/blueprint definitions

do you mean, I should take snapshots of the blueprint that is generated from config defined in .md files?

I tried adding blueprint as snapshot but I think since HashMap is being used in the type, the order keeps changing and new snapshot gets created everytime

no, no, my suggestion is to not focus on blueprint structure at all and focus on the fact that default values should work properly when they are defined. I.e. in test when making query without providing value verify that request hits the mock for default value, and if we do provide some value in query than another mock should be hit

Apparently, the same issue that was encountered in #1939 for default value in field arguments, seems to also be affecting default values in input types. It will not be possible to test this change by running a query until that issue with async-graphql is fixed

ah, ok. Let's maybe wait for resolution of this

Delete these tests perhaps. I think @meskill Is correct we should implement them as integration tests. We can mark them as skipped so that with our new GraphQL engine we should be able to execute it.

@tusharmath @meskill added a test and marked it as skipped. The added test will pass once the support for default value is added for input types

tests/core/snapshots/default-value-input-type.md_0.snap

tusharmath · 2024-06-07T05:36:12Z

src/core/blueprint/into_schema.rs

@@ -123,6 +123,10 @@ fn to_type(def: &Definition) -> dynamic::Type {
                if let Some(description) = &field.description {
                    input_field = input_field.description(description);
                }
+                if let Some(default_value) = field.default_value.clone() {
+                    input_field =
+                        input_field.default_value(ConstValue::from_json(default_value).unwrap())


Drop unwrap.

use trace::warning to show that transformation failed over here.

tusharmath · 2024-06-07T05:40:00Z

tests/execution/default-value-input-type.md

+}
+
+type Query {
+  abc(input: Input!): Int @http(path: "/foo/{{.args.input.id}}")


Suggested change

abc(input: Input!): Int @http(path: "/foo/{{.args.input.id}}")

abc(input: Input!): Int @http(path: "/foo/{{.args.input.id}}")

pqr(input: Input! = {id: 2}): Int @http(path: "/foo/{{.args.input.id}}")

Add this test, and mark it as not-passing and point it to the PR which fixes this issues in Async GraphQL.

added two separate tests, one for default value in argument and another for default value in input type

tusharmath · 2024-06-07T05:40:45Z

tests/core/snapshots/default-value-input-type.md_merged.snap

+}
+
+input Input {
+  id: Int


default value is missing in merged.

tusharmath · 2024-06-07T05:43:53Z

tests/execution/default-value-input-type.md

+      query {
+        abc(input: {id:2})
+      }
+```


Split this into two-tests.

Where you don't make any requests. Mark this test as skipped.

Where we only test for merged and client schemas.

github-actions · 2024-06-09T13:02:52Z

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

github-actions · 2024-06-09T14:04:31Z

Bencher

Report	Fri, June 14, 2024 at 06:01:55 UTC
Project	tailcall
Branch	2117/merge
Testbed	benchmarking-runner

Click to view all benchmark results

Benchmark	Latency	Latency Results nanoseconds (ns) \| (Δ%)	Latency Upper Boundary nanoseconds (ns) \| (%)
from_json_bench	✅ (view plot)	7,154,600.00 (+0.45%)	7,258,313.20 (98.57%)
group_by	✅ (view plot)	551.78 (-4.31%)	646.08 (85.40%)
input/args.missing	✅ (view plot)	23.08 (-7.90%)	56.33 (40.98%)
input/args.nested.existing	✅ (view plot)	42.71 (-13.12%)	84.64 (50.46%)
input/args.nested.missing	✅ (view plot)	38.13 (-2.42%)	69.35 (54.98%)
input/args.root	✅ (view plot)	38.33 (-15.71%)	81.46 (47.05%)
input/headers.existing	✅ (view plot)	33.31 (+5.25%)	33.70 (98.86%)
input/headers.missing	✅ (view plot)	31.55 (+2.68%)	33.54 (94.09%)
input/value.missing	✅ (view plot)	23.59 (+0.53%)	25.08 (94.07%)
input/value.nested.existing	✅ (view plot)	41.75 (+0.42%)	44.87 (93.06%)
input/value.nested.missing	✅ (view plot)	37.94 (+3.68%)	38.59 (98.31%)
input/value.root	✅ (view plot)	38.53 (+0.94%)	41.49 (92.85%)
input/vars.existing	✅ (view plot)	6.93 (-12.22%)	8.93 (77.62%)
input/vars.missing	✅ (view plot)	10.54 (+11.08%)	12.78 (82.45%)
test_batched_body	✅ (view plot)	2,774.30 (-99.47%)	2,154,829.08 (0.13%)
test_batched_body #2	✅ (view plot)	1,662,200.00 (-2.68%)	1,836,610.92 (90.50%)
test_data_loader	✅ (view plot)	452,290.00 (-3.64%)	486,828.83 (92.91%)
test_handle_request	✅ (view plot)	151,070.00 (-3.94%)	174,590.03 (86.53%)
test_http_execute_method	✅ (view plot)	18,139.00 (-0.98%)	19,426.09 (93.37%)
with_mustache_expressions	✅ (view plot)	1,126.10 (-3.49%)	1,230.20 (91.54%)
with_mustache_literal	✅ (view plot)	671.20 (-7.02%)	775.86 (86.51%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

tusharmath · 2024-06-11T12:11:00Z

src/core/config/from_document.rs

@@ -449,6 +458,7 @@ trait Fieldlike {
    fn type_of(&self) -> &Type;
    fn description(&self) -> &Option<Positioned<String>>;
    fn directives(&self) -> &[Positioned<ConstDirective>];
+    fn default_value(&self) -> Option<ConstValue>;


We don't need this to be implemented via FieldLike

ssddOnTop · 2024-06-12T08:07:29Z

src/core/blueprint/into_schema.rs

+                        dynamic::InputValue::new(arg.name.clone(), to_type_ref(&arg.of_type));
+                    let input_value =
+                        insert_serde_value_to_input_value(input_value, arg.default_value.clone());
+                    dyn_schema_field = dyn_schema_field.argument(input_value);


for arg in field.args.iter() { let iv = dynamic::InputValue::new( arg.name.clone(), to_type_ref(&arg.of_type), ); let iv = iv.default_value(arg.default_value.as_ref().map(|v | ConstValue::from_json(v.clone()).ok()).flatten().unwrap()); // TODO: handle unwrap dyn_schema_field = dyn_schema_field.argument(iv); }

I think with v7.0.6 we can just pass the default value and everything else should work as is

for arg in field.args.iter() { let iv = dynamic::InputValue::new( arg.name.clone(), to_type_ref(&arg.of_type), ); let iv = iv.default_value(arg.default_value.as_ref().map(|v | ConstValue::from_json(v.clone()).ok()).flatten().unwrap()); // TODO: handle unwrap dyn_schema_field = dyn_schema_field.argument(iv); }

I think with v7.0.6 we can just pass the default value and everything else should work as is

I am doing this already inside the insert_serde_value_to_input_value function but its not working for default value in input types

…efault-value

shashitnak added 2 commits June 5, 2024 15:23

adding default_value to InputObject

bcbec86

Merge branch 'main' into default-value

1ab8a1a

github-actions bot added the type: feature Brand new functionality, features, pages, workflows, endpoints, etc. label Jun 5, 2024

adding test

7586308

meskill reviewed Jun 5, 2024

View reviewed changes

shashitnak added 8 commits June 6, 2024 15:23

Merge branch 'main' into default-value

3f5c56d

passing default value to async-graphql

10890a5

adding test for default value

1fa7111

Merge branch 'main' into default-value

26f5e5d

adding mock test

e753f5a

lint fix

8fbaf18

fixing wrong test

719fa8a

lint fix

061a578

meskill approved these changes Jun 6, 2024

View reviewed changes

meskill reviewed Jun 6, 2024

View reviewed changes

tests/core/snapshots/default-value-input-type.md_0.snap Outdated Show resolved Hide resolved

tusharmath reviewed Jun 7, 2024

View reviewed changes

shashitnak added 2 commits June 7, 2024 14:09

addressing PR comments

a5021bd

Merge branch 'main' into default-value

5c7c967

tusharmath changed the title ~~feat: storing default_value in InputFieldDefinition~~ fix: default_value in InputFieldDefinition Jun 7, 2024

fixing build

19bbc26

github-actions bot added the type: fix Iterations on existing features or infrastructure. label Jun 7, 2024

github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Jun 9, 2024

shashitnak added 2 commits June 9, 2024 19:07

Merge branch 'main' into default-value

6b5ad5b

fixing default value tests

1234605

github-actions bot added the ci: benchmark Runs benchmarks label Jun 9, 2024

Merge branch 'main' into default-value

ffcf662

github-actions bot removed the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Jun 9, 2024

shashitnak and others added 2 commits June 10, 2024 16:03

Merge branch 'main' into default-value

d274d76

Merge branch 'main' into default-value

7b73408

tusharmath reviewed Jun 11, 2024

View reviewed changes

tusharmath marked this pull request as draft June 11, 2024 12:17

update trait name

b9b8ea3

ssddOnTop reviewed Jun 12, 2024

View reviewed changes

shashitnak added 4 commits June 12, 2024 16:09

Merge branch 'main' into default-value

e326763

removing default_value method from FieldLike trait

4418d9f

cleaning up

c5c58bf

Merge branch 'default-value' of github.com:tailcallhq/tailcall into d…

0747184

…efault-value

shashitnak marked this pull request as ready for review June 12, 2024 11:05

tusharmath added 2 commits June 12, 2024 18:10

add a doc based marker

9666e63

add a doc for the helper function

b268722

tusharmath enabled auto-merge (squash) June 12, 2024 12:54

tusharmath approved these changes Jun 12, 2024

View reviewed changes

tusharmath disabled auto-merge June 14, 2024 05:46

tusharmath changed the title ~~fix: default_value in InputFieldDefinition~~ feat: support for default values in InputFieldDefinition Jun 14, 2024

tusharmath removed the type: fix Iterations on existing features or infrastructure. label Jun 14, 2024

tusharmath merged commit c4dfd8b into main Jun 14, 2024
35 checks passed

tusharmath deleted the default-value branch June 14, 2024 05:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support for default values in `InputFieldDefinition` #2117

feat: support for default values in `InputFieldDefinition` #2117

shashitnak commented Jun 5, 2024 •

edited

Loading

codecov bot commented Jun 5, 2024 •

edited

Loading

meskill Jun 5, 2024

shashitnak Jun 5, 2024

shashitnak Jun 5, 2024 •

edited

Loading

meskill Jun 6, 2024

shashitnak Jun 6, 2024 •

edited

Loading

meskill Jun 6, 2024

tusharmath Jun 6, 2024

shashitnak Jun 6, 2024

tusharmath Jun 7, 2024

tusharmath Jun 7, 2024

shashitnak Jun 7, 2024

tusharmath Jun 7, 2024

tusharmath Jun 7, 2024

shashitnak Jun 7, 2024

tusharmath Jun 7, 2024

shashitnak Jun 7, 2024

tusharmath Jun 7, 2024

shashitnak Jun 7, 2024

github-actions bot commented Jun 9, 2024

github-actions bot commented Jun 9, 2024 •

edited

Loading

tusharmath Jun 11, 2024

shashitnak Jun 12, 2024

ssddOnTop Jun 12, 2024

shashitnak Jun 12, 2024

	abc(input: Input!): Int @http(path: "/foo/{{.args.input.id}}")
	abc(input: Input!): Int @http(path: "/foo/{{.args.input.id}}")
	pqr(input: Input! = {id: 2}): Int @http(path: "/foo/{{.args.input.id}}")

feat: support for default values in InputFieldDefinition #2117

feat: support for default values in InputFieldDefinition #2117

Conversation

shashitnak commented Jun 5, 2024 • edited Loading

codecov bot commented Jun 5, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashitnak Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashitnak Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 9, 2024

github-actions bot commented Jun 9, 2024 • edited Loading

Bencher

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: support for default values in `InputFieldDefinition` #2117

feat: support for default values in `InputFieldDefinition` #2117

shashitnak commented Jun 5, 2024 •

edited

Loading

codecov bot commented Jun 5, 2024 •

edited

Loading

shashitnak Jun 5, 2024 •

edited

Loading

shashitnak Jun 6, 2024 •

edited

Loading

github-actions bot commented Jun 9, 2024 •

edited

Loading