You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/08/11 01:59:53 UTC
[GitHub] [skywalking] whfjam opened a new issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
whfjam opened a new issue #5291:
URL: https://github.com/apache/skywalking/issues/5291
Please answer these questions before submitting your issue.
- Why do you submit this issue?
- [*] Question or discussion
- [ ] Bug
- [ ] Requirement
- [ ] Feature or performance improvement
___
### Question
Since the issue #4907 I think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes” which returns “” by now
___
### Bug
- Which version of SkyWalking, OS and JRE?
- Which company or project?
- What happened?
If possible, provide a way to reproduce the error. e.g. demo application, component version.
___
### Requirement or improvement
- Please describe your requirements or improvement suggestions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] whfjam commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
whfjam commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672014698
>
>
> Ping @BFergerson for a closer look.
>
> @whfjam If you have known what we're missing, could you send a pull request directly?
yes, i'd like to
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] BFergerson commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
BFergerson commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672021692
https://github.com/apache/skywalking/pull/5085 is my attempt at fixing this. The only thing not working is the e2e tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672627894
@whfjam You could review that PR directly, comment on the specific line(s). And the codes should be well formated by using markdown(GitHub supports this).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] whfjam commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
whfjam commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672054191
>
>
> #5085 is my attempt at fixing this. The only thing not working is the e2e tests.
here is my codes if any helps to you
@Override
public String getAcceptedMethodTypes(Method method) {
return ParsePathUtil.recursiveParseMethodAnnotaion(method, m -> {
if (AnnotationUtils.getAnnotation(m, GetMapping.class) != null) {
return RequestMethod.GET.toString();
}
if (AnnotationUtils.getAnnotation(m, PostMapping.class) != null) {
return RequestMethod.POST.toString();
}
if (AnnotationUtils.getAnnotation(m, PutMapping.class) != null) {
return RequestMethod.PUT.toString();
}
if (AnnotationUtils.getAnnotation(m, DeleteMapping.class) != null) {
return RequestMethod.DELETE.toString();
}
if (AnnotationUtils.getAnnotation(m, PatchMapping.class) != null) {
return RequestMethod.PATCH.toString();
}
return null;
});
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] whfjam commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
whfjam commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672047230
>
>
> #5085 is my attempt at fixing this. The only thing not working is the e2e tests.
i have saw your code， i noticed that in the last else situation u used the <return ""> i think this code
will cause the recursive to be ended because in the method ParsePathUtil.recursiveParseMethodAnnotaion it judges the result by
Objects.isNull(result) , i doubt if you have notice that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng closed issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
wu-sheng closed issue #5291:
URL: https://github.com/apache/skywalking/issues/5291
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-671950860
I think #5084 has fixed this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-687625347
#5085 merged, if you still have some doubts, submit your proposal change through a pull request.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] whfjam commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
whfjam commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672051413
>
>
> #5085 is my attempt at fixing this. The only thing not working is the e2e tests.
here is my code if any helps to you
@Override
public String getAcceptedMethodTypes(Method method) {
return ParsePathUtil.recursiveParseMethodAnnotaion(method, m -> {
GetMapping getMapping = AnnotationUtils.getAnnotation(m, GetMapping.class);
if (getMapping != null) {
return RequestMethod.GET.toString();
}
PostMapping postMapping = AnnotationUtils.getAnnotation(m, PostMapping.class);
if (postMapping != null) {
return RequestMethod.POST.toString();
}
PutMapping putMapping = AnnotationUtils.getAnnotation(m, PutMapping.class);
if (putMapping != null) {
return RequestMethod.PUT.toString();
}
DeleteMapping deleteMapping = AnnotationUtils.getAnnotation(m, DeleteMapping.class);
if (deleteMapping != null) {
return RequestMethod.DELETE.toString();
}
PatchMapping patchMapping = AnnotationUtils.getAnnotation(m, PatchMapping.class);
if (patchMapping != null) {
return RequestMethod.PATCH.toString();
}
return null;
});
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] whfjam commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
whfjam commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672001820
>
>
> I think #5084 has fixed this.
in #5084 it changed the code in the class AbstractMethodInterceptor use “getAcceptedMethodTypes(method)” but the method “getAcceptedMethodTypes” is a abstract one ，the implements are RequestMappingMethodInterceptor.getAcceptedMethodTypes and RestMappingMethodInterceptor.getAcceptedMethodTypes，then the RequestMappingMethodInterceptor.getAcceptedMethodTypes has handle the case but the RestMappingMethodInterceptor.getAcceptedMethodTypes just return a empty string, did i miss something..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng closed issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
wu-sheng closed issue #5291:
URL: https://github.com/apache/skywalking/issues/5291
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] whfjam removed a comment on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
whfjam removed a comment on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672051413
>
>
> #5085 is my attempt at fixing this. The only thing not working is the e2e tests.
here is my code if any helps to you
@Override
public String getAcceptedMethodTypes(Method method) {
return ParsePathUtil.recursiveParseMethodAnnotaion(method, m -> {
GetMapping getMapping = AnnotationUtils.getAnnotation(m, GetMapping.class);
if (getMapping != null) {
return RequestMethod.GET.toString();
}
PostMapping postMapping = AnnotationUtils.getAnnotation(m, PostMapping.class);
if (postMapping != null) {
return RequestMethod.POST.toString();
}
PutMapping putMapping = AnnotationUtils.getAnnotation(m, PutMapping.class);
if (putMapping != null) {
return RequestMethod.PUT.toString();
}
DeleteMapping deleteMapping = AnnotationUtils.getAnnotation(m, DeleteMapping.class);
if (deleteMapping != null) {
return RequestMethod.DELETE.toString();
}
PatchMapping patchMapping = AnnotationUtils.getAnnotation(m, PatchMapping.class);
if (patchMapping != null) {
return RequestMethod.PATCH.toString();
}
return null;
});
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #5291: since the issue #4907 i think the class RestMappingMethodInterceptor should implements the method “getAcceptedMethodTypes”
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #5291:
URL: https://github.com/apache/skywalking/issues/5291#issuecomment-672004793
Ping @BFergerson for a closer look.
@whfjam If you have known what we're missing, could you send a pull request directly?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org