Cxf 9233 fix response context

[vp340]

Pull request with a new possible solution to Jira Cxf-9233

[reta]

@vp340 thanks for another alternative, the problem I see with this one is two fold: LoggingXxxInteceptor has to be aware about the ClientImpl specifics/internals but this is generic feature that works for client (we have many) or/and server (same, many).

[vp340]

Hi @reta , I understand your point.
All module must be discern (I thought wrongly that the logging module could be aware of the core one ... having his dependency).
Thanks a lot for all the lessons about design pattern in such big project like cxf :) and for guiding me through all these solutions .

I'm glad at least we have discover the full picture about the problem!
So the only solutions remains acting on the single modules.

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 :) ).

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.
I don't have the knowledge to know what are the properties needed and I don't even know if this is a suitable idea...
If U find it ok and want to try to implement it let me know ...I could prepare at least the skeleton where to add the white-list properties if U want.
((In the last message where U said the you have many clients I wondered if this 'problem' of propagating all props into ResponseContext is common for other Client-s other than the ClientImpl... if so this last idea is less appetizing and would need to change all ...or find some common point to put all the props if they are the same)).

Let me know what U think.
If the last idea isn't suitable in my opinion we can merge your #3372 .

Have a great job!

Valentino Porta

[reta]

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

[vp340]

Hi @reta ,
Yeah the camel CXF wrapper does :
// make sure the "requestor role" property does not get propagated as we do switch role
responseContext.remove(Message.REQUESTOR_ROLE);
outMessage.putAll(responseContext);
// Do we still need to put the response context back like this
outMessage.put(CxfConstants.RESPONSE_CONTEXT, responseContext);
...
but as we said for the cxf core module, being the LIVE_LOGGING_PROP protected the camel module is not aware of it so it can't remove it like it does with Message.REQUESTOR_ROLE .
The only way would be adding a white-list there, but as U said (and I fully agree) for this side log feature your simple fix is more than enough as long as we see the RESP_OUT log. :)
I'll leave it up to U whether it's worth pursuing a Camel-side fix as well, but as long as it doesn't emerge a more tricky problem with the propagation of the response context ...I don't find it really necessary and could be also risky.

Thanks for all the time U dedicated to this problem.
See U in the next one (hopefully not ;) ... )

Keep up the great work!

Valentino Porta