Skip to content

Conversation

@heliang666s
Copy link
Member

@heliang666s heliang666s commented Oct 19, 2024

What is the purpose of the change?

Support parsing custom REST paths from proto files and generating @Mapping annotations. #13369

Checklist

  • Make sure there is a GitHub_issue field for the change.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Make sure gitHub actions can pass. Why the workflow is failing and how to fix it?

@oxsean oxsean self-requested a review October 21, 2024 03:01
<groupId>com.salesforce.servicelibs</groupId>
<artifactId>grpc-contrib</artifactId>
</dependency>
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this module just to generate code?

}

if (!httpRule.getGet().isEmpty()) {
methodContext.path = httpRule.getGet();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can it be mege with line 199?


{{#packageName}}
package {{packageName}};
package {{packageName}};
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary indent?

CompletableFuture<{{outputType}}> {{methodName}}Async({{inputType}} request);

{{#hasMappings}}
@Mapping(method = HttpMethods.{{httpMethod}}, path = "{{path}}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to make sure that the indents of the generated code is correct.

@Target(ElementType.PARAMETER)
@Retention(RetentionPolicy.RUNTIME)
@Documented
public @interface Grequest {
Copy link
Contributor

Choose a reason for hiding this comment

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

GRequest

import java.util.Map;

@Activate
public class GrpcBodyArgumentResolver implements AnnotationBaseArgumentResolver<Grequest> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to GRequestArgumentResolver

public String javaDoc;
public HttpMethods httpMethod;
public String path;
public String body; // requestBody
Copy link
Contributor

Choose a reason for hiding this comment

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

Make the comments readable
e.g.: Specify the message field that the HTTP request body mapping to

@oxsean oxsean merged commit 9741de3 into apache:3.3 Nov 3, 2024
@heliang666s heliang666s deleted the custom-uri branch November 4, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants