poc: java sdk support multiple ip#37
Conversation
|
自测结果: |
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| <version>3.0.0-M5</version> | ||
| <version>3.5.5</version> |
There was a problem hiding this comment.
testng 版本和对应版本的 surefire 不兼容,以前都是 ide 插件跑的,这次用 mvn 跑发现了问题,就升级了一下,另外匹配升级了 mvn 的最小版本,这些都是开发时的依赖,不影响构建产物
fanyang89
left a comment
There was a problem hiding this comment.
这版先别合,重复造轮子太多,而且不是“多写点代码”这么简单,是把 OkHttp/JDK 已经保证好的语义自己重写了一遍,然后还写坏了。
主要问题:
-
src/main/java/com/smartx/tower/ActivePassiveApiClient.java:664开始手写okhttp3.Call,这就是在复刻 OkHttp 的RealCall。SDK 不应该自己实现一套execute/enqueue/cancel/timeout/clone。这个需求更适合放到 OkHttpInterceptor或一个很薄的 endpoint resolver 里,最终仍然让 OkHttp 创建真实Call。 -
src/main/java/com/smartx/tower/ActivePassiveApiClient.java:704的enqueue()直接拿dispatcher().executorService().execute(...)跑任务,绕开了 OkHttpDispatcher.enqueue/finished的请求计数、maxRequests、maxRequestsPerHost、idle callback 等机制。这不是优化,这是把 OkHttp 的调度器拆了重新糊一个残版。 -
src/main/java/com/smartx/tower/ActivePassiveApiClient.java:731的cancel()只改了一个AtomicBoolean,已经发出去的 probe/request 根本取消不了。真实 OkHttpCall.cancel()会取消 socket/stream,这里用户以为取消了,实际请求还在跑。 -
src/main/java/com/smartx/tower/ActivePassiveApiClient.java:747的timeout()返回的是一个新建Timeout,没有绑定到任何真实 call,也不会约束正在执行的请求。OkHttp 已经有 call timeout,这里返回一个摆设会误导调用方。 -
src/main/java/com/smartx/tower/ActivePassiveApiClient.java:114到219这一大坨 setter 只是super.xxx(...); return this;,纯样板代码。为了链式调用窄化返回类型复制 20 多个方法不值,保留真正有行为差异的 override 就行。 -
src/main/java/com/smartx/tower/ApiClient.java:8到10明确写着 OpenAPI Generator 生成、不要手改。这个 PR 直接往ApiClient里塞 active/passive probe,下次重新生成 SDK 就没了。active/passive 逻辑应该放到非生成类里,或者改 generator/template,而不是手改生成产物。 -
src/main/java/com/smartx/tower/ApiClient.java:1134和src/main/java/com/smartx/tower/ActivePassiveApiClient.java:524各写了一份apiException();closeQuietly()也重复写了两份。已有handleResponse()/Response.close()可以复用的话就复用,真需要 helper 也抽一处,别复制粘贴。 -
src/main/java/com/smartx/tower/ActivePassiveApiClient.java:550手写join()没必要,JDK 早就有String.join(delimiter, values)。src/main/java/com/smartx/tower/ActivePassiveApiClient.java:542的trimTrailingSlash()还带"http://x"这种魔法字符串,URL 规范化交给HttpUrl/URI,别自己抠字符串。 -
src/main/java/com/smartx/tower/ActivePassiveApiClient.java:384到390在 synchronized 里做网络探测。一个慢 endpoint 就能把所有并发请求堵在锁后面,最坏是 endpoint 数量 * probe timeout。锁只应该保护状态读写,不应该包住网络 I/O。 -
src/test/java/com/smartx/tower/ActivePassiveApiClientTest.java:217还在用字符串字面量"UTF-8",JDK 有StandardCharsets.UTF_8。小问题,但和上面的问题是一类:标准库有的东西别手搓。
建议重做:不要加一个 761 行的代理 client 去复刻 OkHttp。把“发现 active endpoint / 收到 307 后清缓存并重试一次”做成小的 resolver/interceptor,尽量让原来的 ApiClient.buildRequest() 和 OkHttp 的真实 Call 继续工作。这样代码量会少很多,也不会破坏取消、超时、异步调度这些基础语义。
|
修改了请求的发送模式,并通过 interceptor 来进行切主选主 override 很多直接调用 super 的代码,主要是为了保证 builder 模式可以正确返回指定的 classname |
- introduce active passivce client - fix imcompatibile surefire plugin for testng 7.4.0
测试用代码: