reta commented on PR #1033: URL: https://github.com/apache/cxf/pull/1033#issuecomment-1325805132
> Hi @reta, thank you too for reproducing this issue and looking at code changes. The case when the last argument is omitted is not covered. The "argument type mismatch while invoking" error is blocking our migration from Axis to CXF. I put my effort mostly to understand if proposed change is isolated, safe and will not have negative impact on existing codebase. Thanks @aziubin , the `MessageContentsList` is very widely used with CXF and may be outside (since this is a public class), so none of the "isolated, safe and will not have negative impact on existing codebase" could be asserted for sure. Also, the change would introduce a very tricky class of errors, where `REMOVED_MARKER` will still appear in `toArray(T[])`, `subList(...)`, `sort(...)` ... unless we override everything (probably not a good idea). > I did the following: verified that `REMOVED_MARKER` is not used in CXF code or out there; reviewed the code where `MessageContentsList.toArray` is used (that way I discovered an issue with bean validation, which is covered by this fix); reviewed the history of `MessageContentsList.java` file (that way I found CXF 2.0.3 SVN commit 587171, which introduced this issue - see [[CXF-1129] Fix issues with out of band headers causing marshalling issues Re-add asm jars to distribution](http://svn.apache.org/viewvc/cxf/trunk/api/src/main/java/org/apache/cxf/message/MessageContentsList.java?r1=587170&r2=587171&pathrev=1434564&;)). As a result, overriding `toArray` in my opinion looks like a missing change, which should have been done as part of commit 587171. > @dkulp I think you where working on that at the time, do you have opinion on the matter? May be we could keep the `null` placeholder but mirror the `REMOVED_MARKER` through complementary array? > Wider fix, which will also cover the case when the last argument is omitted, would need research and additional development and I am opened for it. Potentially, this is not going to be isolated and safe change, so I need to hear your opinion first before starting. > I am planning to look for the clarification in spec regarding the omitted operation input, will get back with my findings. > The case with primitive type is different. In opposite to the issue, which I am trying to fix, this case can be work around with a method wrapper, which has compatible wrapper class in the place of primitive type, for example `void allocateWrapper(Integer i) { if (i != null) allocate(i); } void allocate(int i) { }`. What do you think? We can look at improving error message for this case if necessary. If we have to cover the omitted operation input, we should cover such cases as well, we should not be asking users for workarounds. -- 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]
