OpenFeign/feign

Feign.Builder have a requestInterceptor bug

hdmvp opened this issue · 3 comments

hdmvp commented

for example:
public static T getHttpApi(Class api, String url) {
ApplicationContext applicationContext = SpringContext.getApplicationContext();
return applicationContext.getBean(Feign.Builder.class).requestInterceptor(new CustomFeignInterceptor()).target(api, url);
}
add the requestInterceptor at here cause :

java.util.ConcurrentModificationException

at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1631)....

suggest:

 /**
     * Adds a single request interceptor to the builder.
     */
    public Builder requestInterceptor(RequestInterceptor requestInterceptor) {
      this.requestInterceptors.add(requestInterceptor);
      return this;
    }
 private final List<RequestInterceptor> requestInterceptors =
        new ArrayList<RequestInterceptor>();

the soureCode requestInterceptors should be a CopyOnWriteArrayList

requestInterceptor只需要add一次就可以吧? 你这里貌似是并发添加了,可以控制一下调用的地方,只add一次

hdmvp commented

requestInterceptor只需要add一次就可以吧? 你这里貌似是并发添加了,可以控制一下调用的地方,只add一次

这里每次的调用确实会存在并发添加的问题,但是基于暴露的方法来看,如果不点进源码去看,根本就不会发现这个问题。所以基于一个良好的体验来讲,我觉得这个requestInterceptor方法的内部集合实现有改进的空间,换成 CopyOnWriteArraySet会更友好一点