[Dubbo-#1362] cache provider always lru cache#1396
[Dubbo-#1362] cache provider always lru cache#1396beiwei30 merged 7 commits intoapache:masterfrom maxiaoguang64:master
Conversation
|
Please synchronize the origin/master branch to resolve the problem of CI check failure |
从dubbo更新代码
Codecov Report
@@ Coverage Diff @@
## master #1396 +/- ##
==========================================
- Coverage 33.04% 32.94% -0.11%
==========================================
Files 691 691
Lines 34625 34625
Branches 6851 6850 -1
==========================================
- Hits 11443 11408 -35
- Misses 21238 21269 +31
- Partials 1944 1948 +4
Continue to review full report at Codecov.
|
|
@lovepoem All checks have passed, please review it |
|
|
||
| public Cache getCache(URL url) { | ||
| public Cache getCache(URL url, Invocation invocation) { | ||
| url = url.addParameter(Constants.METHOD_KEY, invocation.getMethodName()); |
There was a problem hiding this comment.
can you attach a testcase for these modified codes, thx.
There was a problem hiding this comment.
ok, i need some help because i don't know how to test.
l debug in idea run ok.
There was a problem hiding this comment.
you can mock an invocation and cacheFactory,
then assert the result hitting your cache.
There was a problem hiding this comment.
how to confirm use annother cache not lru
There was a problem hiding this comment.
The change looks good, except for this:
url = url.addParameter(Constants.METHOD_KEY, invocation.getMethodName());I don't think we need addParameter for method name in getCache() method
添加cache的测试用例
|
@kimmking l add some test for cache, please review it thanks |
beiwei30
left a comment
There was a problem hiding this comment.
I think I got the point, good job.
|
|
||
| public Cache getCache(URL url) { | ||
| public Cache getCache(URL url, Invocation invocation) { | ||
| url = url.addParameter(Constants.METHOD_KEY, invocation.getMethodName()); |
There was a problem hiding this comment.
The change looks good, except for this:
url = url.addParameter(Constants.METHOD_KEY, invocation.getMethodName());I don't think we need addParameter for method name in getCache() method
* remotes/upstream/master: (226 commits) clean up imports for CacheTest [Dubbo-apache#1362] cache provider always lru cache (apache#1396) Remove author info and add apache license Fix "promoteTransitiveDependencies=false" of maven-shade-plugin apache#1411: Locale deserialize 'zh-hant_CN' update README format update readme to add some details (apache#1403) fix number type is lost in yaml config file (apache#1401) fix hessian lite test case fail bug (apache#1394) Merge pull request apache#1391, fix typo of method name in qos module. Fix time unit problem related with FutureAdapter in UT Fix time unit problem related with FutureAdapter in UT Fix time unit problem in UT Fixed apache#1398, revert bugs introduced from apache#1375 Change Mailing list address Merge pull request apache#1242, remove redundant null check. Merge pull request apache#1040, refactor: replace some deprecated methods related with jedis. Merge pull request apache#1384, fix build string bug. Merge pull request apache#1376, do not instantiate load balance if there is no invokers Merge pull request apache#1331, add optional parameter to support hessian protocol method overload and request protocol version. ...
* #1362修复 #1362修复 * #1362修复 #1362修复 * #1362修复 #1362修复 * 修改bug 修改bug * 修改bug 修改bug * 添加cache的测试用例 添加cache的测试用例
fix #1362