Add OkHttp integration

[zhaoyuguang]

resolve #1435

[sczyh30]

How about OkHttpResourceExtractor or other better?

[zhaoyuguang]

是的，class name和method名称修改了

[sczyh30]

How about carrying HTTP method prefix in the resource name? And it could be better if users could add customized prefix here (e.g. okhttp:xxx).

[zhaoyuguang]

恩 默认的DefaultOkHttpResourceExtractor 增加了HTTP METHOD+":"的记录，且增加了 config里面的prefix属性 默认""

[sczyh30]

Remove the redundant blank line?

[zhaoyuguang]

fixed

[cdfive]

#Line38 check not null for prefix not extractor.
#Line47 OkHttpResourceExtractor, extractor and cleaner seems not very uniform, but it's OK : )
@see SentinelOkHttpConfigTest#testConfigSetPrefix.

[zhaoyuguang]

code & testcase 修复了， 写正规以免有歧义

[cdfive]

Spring is used for only unit test, here whether it can be removed?

[zhaoyuguang]

copy 别的 readme 忘记删了

[cdfive]

It's better to indent here, make the format good looking.

[zhaoyuguang]

fixed

[cdfive]

1999-2020 is better, since time goes by.

[zhaoyuguang]

恩 暂且吧本提交涉及的改了

[cdfive]

This can be omitted in demo, since sentinel-okhttp-adapter has the dependency. Adding it is all right, but new user will feel uncertain if this is is must when he use this adapter.

[zhaoyuguang]

恩 修复了 之前还奇怪 idea为啥报警告呢

稍后 我在其他pr 把其他demo 类似也一起fix了

[cdfive]

How about also adding demo for post method?

When I add a post controller for test:

@PostMapping("/okhttp/post")
public String post(HttpServletRequest request) throws Exception {
    byte[] body = StreamUtils.copyToByteArray(request.getInputStream());
    System.out.println(new String(body));
    return "post";
}

@RequestMapping("/okhttp/testcase/post")
public String testcasePost() throws Exception {
    MediaType mediaType = MediaType.parse("application/json; charset=utf-8");
    String requestBody = "{\"name\":\"cdfive\"}";
    OkHttpClient client = new OkHttpClient.Builder()
            .addInterceptor(new SentinelOkHttpInterceptor())
            .build();
    Request request = new Request.Builder()
            .url("http://localhost:" + port + "/okhttp/post")
            .post(RequestBody.create(mediaType, requestBody))
            .build();
    Response response = client.newCall(request).execute();
    return response.body().string();
}

I find the resource name in dashboard are:
http://localhost:8085/okhttp/back
http://localhost:8085/okhttp/post
They have no GET or POST, and also no okhttp: prefix

This is due to the custom extrator in OkHttpDemoApplication class.
My point is whether the interface OkHttpResourceExtractor#extract(Request request, Connection connection)
granularity is too big? When user use their custom extrator, they can't be benefit from or reuse the logic of DefaultOkHttpResourceExtractor, which provide common prefix and method logic.

The UrlCleaner interface in sentinel-web-servlet module can be referred.
UrlCleaner: String clean(String originUrl);
and the CommonFilter append the http method prefix outside the clean method.

Now, the advantage of DefaultOkHttpResourceExtractor is that it allows users to define resources completely when use their custom extrator.
It may need take info consideration compromise in design and decide which is better.
How about your option? @sczyh30 @zhaoyuguang

[zhaoyuguang]

恩 prefix默认是空的，demo中 自定义提取器写的粗暴一点呢

        SentinelOkHttpConfig.setExtractor(new OkHttpResourceExtractor() {
            @Override
            public String extract(Request request, Connection connection) {
                String url = request.url().toString();
                String regex = "/okhttp/back/";
                if (url.contains(regex)) {
                    url = url.substring(0, url.indexOf(regex) + regex.length()) + "{id}";
                }
                return url;
            }
        });

我还是建议给用户开放 竟可能多的参数，issue #1287 反应的就像更高阶的定义资源

[cdfive]

开放更多的参数没问题，prefix:method:url，只是prefix:method看要不要固定，
比如：
String extract(String target, Request request, Connection connection)
或者 String extract(String url, Request request, Connection connection)
让整个target或者url用户进一步自定义。

个人觉得现在这样也不错，灵活性最大。只是正好想到这个参考下哈~

[zhaoyuguang]

target 和 url 是哪里来的呢？是okhttp-adapter默认提取出来的 然后提供给用户么？

[cdfive]

@zhaoyuguang 大概想到这2个思路：
1.先用prefix:method:url处理好，假设处理后值为target，
然后通过接口String extract(String target, Request request, Connection connection)让用户进一步自定义处理，默认返回target。

2.先得到url，
然后通过接口String extract(String url, Request request, Connection connection)让用户自定义处理，默认返回url，
最后在前面加上prefix:method:。

第1种是整体用前缀处理好，由用户进一步处理；
第2种是用户处理url后，再加前缀；
值得注意的是：两种里面的prefix如果不需要或者要自定义都可以通过(SentinelOkHttpConfig#setPrefix)设置；但method目前暂时不行。
web-servlet模块的CommonFilter就是第2种方式。

上面2种就是有一定的规则约定，在此基础上自定义；
目前的方式是最灵活，如果用户自定义，则整个资源名都自定义。
@sczyh30 宿何空了也看看呢~

[sczyh30]

这里叫 ResourceExtractor 的原因是不希望给用户“只能将 URL 作为资源名”的印象。建议 prefix 可以放到 extractor 外边，然后 method:URL 这种 就作为默认实现好了，用户有需要 就自定义。

[zhaoyuguang]

使用了 第二种 ResourceExtractor 方法是
String extract(String url, Request request, Connection connection);
url默认是method:URL

[sczyh30]

传入的 URL 里面就不要带 ${method}: 了，参数就按照 URL 来传入，只不过 default extractor 里面的实现带上这个 method 前缀；另外总的那个 prefix 带到 interceptor 里面来作为总体控制的，如果有配就 append 上来。

[zhaoyuguang]

Fixed 抽空 review下

[codecov-io]

Codecov Report

Merging #1456 into master will increase coverage by 0.12%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##             master    #1456      +/-   ##
============================================
+ Coverage     43.80%   43.92%   +0.12%     
- Complexity     1703     1717      +14     
============================================
  Files           372      376       +4     
  Lines         10605    10661      +56     
  Branches       1414     1418       +4     
============================================
+ Hits           4645     4683      +38     
- Misses         5393     5411      +18     
  Partials        567      567              

Impacted Files	Coverage Δ	Complexity Δ
...inel/adapter/okhttp/SentinelOkHttpInterceptor.java	50.00% <50.00%> (ø)	2.00 <2.00> (?)
...el/adapter/okhttp/config/SentinelOkHttpConfig.java	68.75% <68.75%> (ø)	5.00 <5.00> (?)
...okhttp/cleaner/DefaultOkHttpResourceExtractor.java	83.33% <83.33%> (ø)	2.00 <2.00> (?)
...adapter/okhttp/fallback/DefaultOkHttpFallback.java	100.00% <100.00%> (ø)	2.00 <2.00> (?)
...dapter/gateway/common/rule/GatewayRuleManager.java	64.88% <0.00%> (-4.47%)	13.00% <0.00%> (ø%)
...inel/slots/block/flow/param/ParamFlowRuleUtil.java	62.63% <0.00%> (+1.09%)	31.00% <0.00%> (+1.00%)
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba39109...1e3b346. Read the comment docs.

[sczyh30]

LGTM

[sczyh30]

Nice work. Thanks for contributing and look forward more!

[zhaoyuguang]

Thanks for @cdfive @sczyh30 patient guidance. too