Enable jsonschema support in debug page for annotated too #5253

anuraaga · 2023-10-20T08:59:15Z

Motivation:

Annotated services can define JSON bodies which benefit from autocomplete.

Modifications:

Query for JSON schema for annotated services too when there is a body parameter by setting it as the "root parameter", have docs client always render with autocomplete when there is a root parameter regardless of service type.

Result:

Annotated services with a request body can take advantage of autocomplete

anuraaga

I'm not super familiar with annotated service but tried making this PR, as in my go implementation I'm also trying to add support for REST, and want it to also have autocomplete :)

https://github.com/curioswitch/go-docs-handler/pull/2/files

I tried running AnnotatedDocServiceTest and could get completion in the JSON method, so AFAICT this should be pretty nice for annotated users if the change makes sense.

anuraaga · 2023-10-20T09:00:01Z

core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java

+    /**
+     * Creates a new instance.
+     */
+    public MethodInfo(String serviceName, String name,


I have added yet more craziness to this file. Hopefully not for this PR, but probably need to get a builder in

anuraaga · 2023-10-20T09:00:28Z

core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java

@@ -130,7 +152,6 @@ public MethodInfo(String serviceName, String name,

        this.returnTypeSignature = requireNonNull(returnTypeSignature, "returnTypeSignature");
        this.parameters = ImmutableList.copyOf(requireNonNull(parameters, "parameters"));
-        assert !useParameterAsRoot || this.parameters.size() == 1;


I could tweak the assertion to look for a single root parameter but it seemed like overkill

github-actions · 2023-10-20T10:10:21Z

🔍 Build Scan® (commit: `b41f945`)

Job name	Status	Build Scan®
build-windows-latest-jdk-19	✅	https://ge.armeria.dev/s/3mqciidk57tyo
build-self-hosted-unsafe-jdk-8	✅	https://ge.armeria.dev/s/iwl44wxa3bl66
build-self-hosted-unsafe-jdk-19-snapshot-blockhound	✅	https://ge.armeria.dev/s/jswg455kbo4qk
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/zghoqmixftl6m
build-self-hosted-unsafe-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/tkpkdgnnd6rva
build-self-hosted-unsafe-jdk-17-leak	✅	https://ge.armeria.dev/s/ahrgqib6ktc62
build-self-hosted-unsafe-jdk-11	✅	https://ge.armeria.dev/s/24phdss6ga7oq
build-macos-12-jdk-19	✅	https://ge.armeria.dev/s/zx4h3hndafkii

codecov · 2023-10-23T01:31:45Z

Codecov Report

Attention: Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.11%. Comparing base (b8eb810) to head (d8af561).
Report is 272 commits behind head on main.

❗ Current head d8af561 differs from pull request most recent head b41f945

Please upload reports for the commit b41f945 to get more accurate results.

Files	Patch %	Lines
...l/server/annotation/AnnotatedDocServicePlugin.java	96.15%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5253      +/-   ##
============================================
+ Coverage     73.95%   74.11%   +0.15%     
- Complexity    20115    20982     +867     
============================================
  Files          1730     1819      +89     
  Lines         74161    77324    +3163     
  Branches       9465     9879     +414     
============================================
+ Hits          54847    57308    +2461     
- Misses        14837    15365     +528     
- Partials       4477     4651     +174

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

anuraaga · 2023-11-06T10:44:44Z

Hello - I think things were hectic with release when sending this but is it possible to get a review now? Thanks

jrhee17

Although I do think the current change will cover most cases, let me think a little more of a way to filter out false-positive cases

jrhee17 · 2023-11-07T05:39:27Z

core/src/main/java/com/linecorp/armeria/server/docs/JsonSchemaGenerator.java

@@ -130,6 +130,7 @@ private ObjectNode generate(MethodInfo methodInfo) {
                methodFields = ImmutableList.of();
            } else {
                methodFields = structInfo.fields();
+                root.put("useParamaterAsRoot", true);


Suggested change

root.put("useParamaterAsRoot", true);

root.put("useParameterAsRoot", true);

jrhee17 · 2023-11-07T05:52:30Z

...src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedDocServicePlugin.java

-                    methodInfos.computeIfAbsent(serviceClass, unused -> new HashSet<>()).add(methodInfo);
-                });
+        List<FieldInfo> fieldInfos = fieldInfos(service.annotatedValueResolvers());
+        boolean useParamaterAsRoot = false;


Suggested change

boolean useParamaterAsRoot = false;

boolean useParameterAsRoot = false;

jrhee17 · 2023-11-07T06:37:00Z

docs-client/src/containers/MethodPage/RequestBody.tsx

-
+  const hasJsonSchema = useMemo(() => {
+    const schema = jsonSchemas.find((s: any) => s.$id === method.id);
+    if (schema && (schema.useParamaterAsRoot || forceJsonSchema)) {


Suggested change

if (schema && (schema.useParamaterAsRoot || forceJsonSchema)) {

if (schema && (schema.useParameterAsRoot || forceJsonSchema)) {

jrhee17 · 2023-11-07T07:27:30Z

core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java

@@ -130,7 +152,6 @@ public MethodInfo(String serviceName, String name,

        this.returnTypeSignature = requireNonNull(returnTypeSignature, "returnTypeSignature");
        this.parameters = ImmutableList.copyOf(requireNonNull(parameters, "parameters"));
-        assert !useParameterAsRoot || this.parameters.size() == 1;


Note: the following is also a valid method that has multiple resolvers, but uses headers/query params instead of the body to fill the bean.

Since the last non-param/query will be used, in the worst case it is possible that wrong autocomplete/recommendations are done.

@Post("/bean2") public HttpResponse bean2(@RequestObject JsonRequest jsonRequest, CompositeBean compositeBean1) { return HttpResponse.of("world"); }

Having said this, I'm not sure if there is a non-trivial way to intelligently find the parameter that should be used as the body.
Let me think about this a little more..

jrhee17 · 2023-11-14T09:24:19Z

Hey, sorry about the late update. So talked with the team and a couple of ideas:

I think one of the concerns was autocompletion for the wrong schema which may confuse users.

Introduce a new @JsonSchema annotation which accepts a class (or json schema url). Only annotated service methods annotated with this annotation will be available for autocompletion.
e.g.

@JsonSchema(class = JsonRequest.class)
public void hello(JsonRequest jsonRequest, Bean bean) {
}

The downsides of this idea was that

Users have to manually add the above annotation to each method to enable autocomplete
We still can't know perfectly how the user intends to deserialize the request. For instance, the user may add a custom RequestConverterFunction which may differ from the JsonRequest schema.

Just add autocompletion for all possible parameters.

We accept that we can't know for sure how exactly the user would like to deserialize the request. For this reason, we disable the monaco validate option for annotated services, and just offer autocompletion for all parameters that seem like a request body as a best-effort.

Also pass RequestConverterFunction information as part of the doc service schema.

And the user can choose which RequestConverterFunction they would like to use.
Each RequestConverterFunction can define how they would like to define the json schema, and this information can be sent down to the doc service.

This approach probaby requires a lot of changes.

cc. @line/dx let me know if I missed/misunderstood any ideas.

Alternatively, if all of these approaches sounds like too much, I think it's also reasonable to add client-side changes only for this iteration. (since I heard that you are working on a separate server implementation which also tries to use the doc service)

jrhee17 · 2024-04-04T14:43:04Z

Haven't forgotten about this issue, but there's a lot going on at the moment 😅 Let me try to brainstorm a little more before the next release

…to jsonschema-annotated

anuraaga · 2024-05-07T06:02:45Z

@jrhee17 Sorry for letting this PR be stale, haven't been heads-down in Java enough to give a deep thought on it and it hasn't been on my critical path yet. Happy to make some changes though if we decide on an approach to take.

Enable jsonschema support in debug page for annotated too

10d5da5

anuraaga requested review from ikhoon, jrhee17, minwoox and trustin as code owners October 20, 2023 08:59

anuraaga commented Oct 20, 2023

View reviewed changes

Style

e6df943

ikhoon added the new feature label Oct 23, 2023

ikhoon added this to the 1.27.0 milestone Oct 23, 2023

Merge branch 'main' into jsonschema-annotated

5d0d9d5

jrhee17 reviewed Nov 7, 2023

View reviewed changes

minwoox modified the milestones: 1.27.0, 1.28.0 Jan 24, 2024

Merge branch 'main' into jsonschema-annotated

d8af561

jrhee17 modified the milestones: 1.28.0, 1.29.0 Apr 4, 2024

github-actions bot added the Stale label May 5, 2024

anuraaga added 4 commits May 7, 2024 11:02

Merge branch 'main' of github.com:line/armeria into jsonschema-annotated

d871ee8

Merge branch 'jsonschema-annotated' of github.com:anuraaga/armeria in…

65d1278

…to jsonschema-annotated

Import order

8c7b3bd

Import order

b41f945

github-actions bot removed the Stale label May 8, 2024

minwoox modified the milestones: 1.29.0, 1.30.0 May 21, 2024

github-actions bot added the Stale label Jun 21, 2024

ikhoon modified the milestones: 1.30.0, 1.31.0 Aug 1, 2024

github-actions bot removed the Stale label Aug 2, 2024

github-actions bot added the Stale label Sep 9, 2024

jrhee17 modified the milestones: 1.31.0, 1.32.0 Nov 5, 2024

github-actions bot removed the Stale label Nov 10, 2024

github-actions bot added the Stale label Dec 16, 2024

minwoox modified the milestones: 1.32.0, 1.33.0 Feb 11, 2025

github-actions bot removed the Stale label Feb 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable jsonschema support in debug page for annotated too #5253

Enable jsonschema support in debug page for annotated too #5253

anuraaga commented Oct 20, 2023

anuraaga left a comment

anuraaga Oct 20, 2023

anuraaga Oct 20, 2023

github-actions bot commented Oct 20, 2023 •

edited

Loading

codecov bot commented Oct 23, 2023 •

edited

Loading

anuraaga commented Nov 6, 2023

jrhee17 left a comment •

edited

Loading

jrhee17 Nov 7, 2023

jrhee17 Nov 7, 2023

jrhee17 Nov 7, 2023

jrhee17 Nov 7, 2023

jrhee17 commented Nov 14, 2023 •

edited

Loading

jrhee17 commented Apr 4, 2024

anuraaga commented May 7, 2024

	root.put("useParamaterAsRoot", true);
	root.put("useParameterAsRoot", true);

	boolean useParamaterAsRoot = false;
	boolean useParameterAsRoot = false;

	if (schema && (schema.useParamaterAsRoot \|\| forceJsonSchema)) {
	if (schema && (schema.useParameterAsRoot \|\| forceJsonSchema)) {

Enable jsonschema support in debug page for annotated too #5253

Are you sure you want to change the base?

Enable jsonschema support in debug page for annotated too #5253

Conversation

anuraaga commented Oct 20, 2023

anuraaga left a comment

Choose a reason for hiding this comment

anuraaga Oct 20, 2023

Choose a reason for hiding this comment

anuraaga Oct 20, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 20, 2023 • edited Loading

🔍 Build Scan® (commit: b41f945)

codecov bot commented Oct 23, 2023 • edited Loading

Codecov Report

anuraaga commented Nov 6, 2023

jrhee17 left a comment • edited Loading

Choose a reason for hiding this comment

jrhee17 Nov 7, 2023

Choose a reason for hiding this comment

jrhee17 Nov 7, 2023

Choose a reason for hiding this comment

jrhee17 Nov 7, 2023

Choose a reason for hiding this comment

jrhee17 Nov 7, 2023

Choose a reason for hiding this comment

jrhee17 commented Nov 14, 2023 • edited Loading

jrhee17 commented Apr 4, 2024

anuraaga commented May 7, 2024

github-actions bot commented Oct 20, 2023 •

edited

Loading

🔍 Build Scan® (commit: `b41f945`)

codecov bot commented Oct 23, 2023 •

edited

Loading

jrhee17 left a comment •

edited

Loading

jrhee17 commented Nov 14, 2023 •

edited

Loading