2.2.5.RELEASE version https://github.com/spring-cloud/spring-cloud-commons/blob/dc3a4674c44011e7b421b8ab25b04c2a22eae19d/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/NamedContextFactory.java#L182-L188
2.2.6.RELEASE version https://github.com/spring-cloud/spring-cloud-commons/blob/82fb22649b17cad6b3b996800908154787d9080a/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/NamedContextFactory.java#L225-L229
Could someone help me understand why null is removed as a return value for the above method as part of the https://github.com/spring-cloud/spring-cloud-commons/pull/826/files ?
I encounter issue due to this change because another package is expecting null instead of empty map. Not sure if there are other places but I found it here https://github.com/spring-cloud/spring-cloud-openfeign/blob/d30540d1445afd7acfdf9f30bd8cb3ae654c9fed/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java#L206-L212
Comment From: ryanjbaxter
Looks like the change was made due to this bug in Spring Cloud Gateway https://github.com/spring-cloud/spring-cloud-gateway/issues/1941
I think all we need to do here is check for the empty Map instead of null
CC: @spencergibb @OlgaMaciaszek what do you think?
Comment From: jBesic
Map<String, T> res = BeanFactoryUtils.beansOfTypeIncludingAncestors(context, type);
return res.length > 0 ? res : null;
@ryanjbaxter Is there any reason why the change can't be like above (store in local variable, check length and return the result from variable) ?
Comment From: ryanjbaxter
@spencergibb would be able to say for sure. To me that seems like it would still result in the same problem.
Comment From: jBesic
@spencergibb @ryanjbaxter - anything on this, when can we expect fix ?
Comment From: spencergibb
I'm still unsure what problem this is causing. Like @ryanjbaxter said, a simple check for an empty map where you pointed to is an optimization, but it isn't broken.
Comment From: jBesic
@spencergibb - from what I see getInheritedAwareInstances method is using beansOfTypeIncludingAncestors which returns either the [1] Map of matching bean instances, or [2] an empty Map if none. It will never return null. That means the code in if block will always run and it will cause unexpected results.
That's why I would say that this is a bug, flow is being changed accidentally.
Comment From: jBesic
@spencergibb - If the Feign.bulder is configured with interceptors before the configureUsingConfiguration call is made, due to requestInterceptors != null condition, all interceptors are removed/overriden with empty List.
I'm not that familiar with spring concepts so it's very hard for me to explain it in better words, sorry for that.
Comment From: OlgaMaciaszek
@jBesic What you've described is happening, but it does not seem to be a bug - it's just how the method was implemented. If it's causing actual errors or issues, please provide a minimal, complete, verifiable example that reproduces the issue.
Comment From: spring-cloud-issues
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.
Comment From: spring-cloud-issues
Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.