reta commented on code in PR #3373:
URL: https://github.com/apache/cxf/pull/3373#discussion_r3839081856
##########
rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingInInterceptor.java:
##########
@@ -71,6 +73,9 @@ public LoggingInInterceptor(PrintWriter writer) {
public LoggingInInterceptor(LogEventSender sender) {
super(Phase.PRE_INVOKE, sender);
+
+ //Make sure that the LIVE_LOGGING_PROP won't be propagated into the
ResponseContext from IN Messages
+ addResponseContextExcludedInProperty(LIVE_LOGGING_PROP);
Review Comment:
> For me your solution in the logging module (change LIVE_LOGGING_PROP
adding true/false based on client/server) works fine and giving the results of
the test solve the ghost RESP_OUT.
> If I were U, I would only consider to add the string "client/server"
instead of "true/false" in order to be more understandable for the posterity (
but is up to U ... U are the pro one :) ).
Thank you @vp340 , yes, I think it is good idea to make the fix more
understandable
> In the core module...the other solution that I thought right now (to be
taken with a grain of salt) is to change approach in the ClientImpl ... and add
a sort of white-list of the properties that needs to be propagated in the
ResponseContext. But this will change completely the actual policy from... let
pass all and remove one ....to ... let pass only the needed.
The issue to be fair has nothing to do with the CXF but the way Camel does
pass the context from in- message to out- message (see please [1]) inside its
CXF wrappers, so I think the fix within CXF is not even needed (but we could
probably try the one we already have to help). So in my opinion, going with the
simple solution on CXF side is more than enough, we could not (and should not)
introduce the complexity here, thank you.
[1]
https://github.com/apache/camel/blob/main/components/camel-cxf/camel-cxf-soap/src/main/java/org/apache/camel/component/cxf/jaxws/DefaultCxfBinding.java#L530
##########
rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingInInterceptor.java:
##########
@@ -71,6 +73,9 @@ public LoggingInInterceptor(PrintWriter writer) {
public LoggingInInterceptor(LogEventSender sender) {
super(Phase.PRE_INVOKE, sender);
+
+ //Make sure that the LIVE_LOGGING_PROP won't be propagated into the
ResponseContext from IN Messages
+ addResponseContextExcludedInProperty(LIVE_LOGGING_PROP);
Review Comment:
> For me your solution in the logging module (change LIVE_LOGGING_PROP
adding true/false based on client/server) works fine and giving the results of
the test solve the ghost RESP_OUT.
> If I were U, I would only consider to add the string "client/server"
instead of "true/false" in order to be more understandable for the posterity (
but is up to U ... U are the pro one :) ).
Thank you @vp340 , yes, I think it is good idea to make the fix more
understandable, I will work on it
> In the core module...the other solution that I thought right now (to be
taken with a grain of salt) is to change approach in the ClientImpl ... and add
a sort of white-list of the properties that needs to be propagated in the
ResponseContext. But this will change completely the actual policy from... let
pass all and remove one ....to ... let pass only the needed.
The issue to be fair has nothing to do with the CXF but the way Camel does
pass the context from in- message to out- message (see please [1]) inside its
CXF wrappers, so I think the fix within CXF is not even needed (but we could
probably try the one we already have to help). So in my opinion, going with the
simple solution on CXF side is more than enough, we could not (and should not)
introduce the complexity here, thank you.
[1]
https://github.com/apache/camel/blob/main/components/camel-cxf/camel-cxf-soap/src/main/java/org/apache/camel/component/cxf/jaxws/DefaultCxfBinding.java#L530
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
