Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support parsing custom REST paths from proto files and generating @Ma… #14796

Merged
merged 13 commits into from
Nov 3, 2024

Conversation

heliang666s
Copy link
Contributor

@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
@@ -59,6 +59,11 @@
<groupId>com.salesforce.servicelibs</groupId>
<artifactId>grpc-contrib</artifactId>
</dependency>
<dependency>
Copy link
Collaborator

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
Collaborator

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?

@@ -16,10 +16,14 @@
*/

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

Choose a reason for hiding this comment

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

Unnecessary indent?


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

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
Collaborator

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
Collaborator

Choose a reason for hiding this comment

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

Rename to GRequestArgumentResolver

@@ -360,6 +428,11 @@ private static class MethodContext {
public String grpcCallsMethodName;
public int methodNumber;
public String javaDoc;
public HttpMethods httpMethod;
public String path;
public String body; // requestBody
Copy link
Collaborator

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
18 checks passed
@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