Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for caching null values #2480

Merged
merged 6 commits into from
Sep 21, 2018
Merged

Support for caching null values #2480

merged 6 commits into from
Sep 21, 2018

Conversation

54hechuan
Copy link
Contributor

优化缓存括展,支持缓存null值

当前CacheFilter存在无法缓存null值问题

但是某些rpc接口被设计成可能返回空值(比如跟据用户名查询用户信息接口,如果查询不到,返回null)

增加一个内部类包装待缓存的数据

static class ValueWrapper {
	private final Object value;
	public ValueWrapper(Object value){
		this.value = value;
	}
	public Object get() {
		return this.value;
	}
}

Verifying this change

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a GITHUB_issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn clean install -DskipTests & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
  • If this contribution is large, please follow the Software Donation Guide.


static class ValueWrapper {

private final Object value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format your code, pls

@carryxyh carryxyh changed the title 优化缓存括展,支持缓存null值 Support for caching null values Sep 11, 2018
@carryxyh
Copy link
Member

Pls check and fix ci problem.

@diecui1202
Copy link

The unit tests failed.

Failed tests: 
  CacheFilterTest.testNull:137 expected:<null> but was:<value1>
  CacheFilterTest.testNull:137 expected:<null> but was:<value1>
  CacheFilterTest.testNull:137 expected:<null> but was:<value1>
Tests in error: 
  CacheFilterTest.testException:124 » HazelcastSerialization Failed to serialize...
  CacheFilterTest.testMethodWithArgs:110 » HazelcastSerialization Failed to seri...
  CacheFilterTest.testNonArgsMethod:97 » HazelcastSerialization Failed to serial...
  CacheFilterTest.testNull:134 » HazelcastSerialization Failed to serialize 'org...

Pls. take a look.

@diecui1202 diecui1202 added the status/waiting-for-feedback Need reporters to triage label Sep 11, 2018
@codecov-io
Copy link

Codecov Report

Merging #2480 into master will decrease coverage by 0.11%.
The diff coverage is 77.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2480      +/-   ##
============================================
- Coverage     55.09%   54.97%   -0.12%     
- Complexity     5270     5279       +9     
============================================
  Files           573      573              
  Lines         25446    25508      +62     
  Branches       4518     4527       +9     
============================================
+ Hits          14019    14023       +4     
- Misses         9335     9389      +54     
- Partials       2092     2096       +4
Impacted Files Coverage Δ Complexity Δ
...ava/org/apache/dubbo/cache/filter/CacheFilter.java 76.19% <77.77%> (-3.81%) 5 <0> (-1)
...ache/dubbo/remoting/p2p/support/AbstractGroup.java 45.45% <0%> (-11.37%) 7% <0%> (-1%)
.../dubbo/remoting/transport/netty4/NettyChannel.java 57.64% <0%> (-8.61%) 22% <0%> (-1%)
...ubbo/rpc/protocol/dubbo/ChannelWrappedInvoker.java 41.66% <0%> (-8.34%) 3% <0%> (ø)
...apache/dubbo/rpc/protocol/dubbo/FutureAdapter.java 58.06% <0%> (-6.46%) 3% <0%> (ø)
...src/main/java/org/apache/dubbo/rpc/RpcContext.java 46.62% <0%> (-5.25%) 54% <0%> (ø)
...exchange/support/header/HeaderExchangeChannel.java 79.16% <0%> (-4.35%) 35% <0%> (ø)
...ava/org/apache/dubbo/common/utils/ClassHelper.java 71.83% <0%> (-4.29%) 17% <0%> (ø)
...apache/dubbo/rpc/cluster/support/ClusterUtils.java 63.15% <0%> (-4.19%) 2% <0%> (+1%)
...ing/transport/dispatcher/ChannelEventRunnable.java 79.16% <0%> (-1.69%) 9% <0%> (ø)
... and 31 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 7a86d61...249eb02. Read the comment docs.

@diecui1202 diecui1202 merged commit e6b0bc8 into apache:master Sep 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/waiting-for-feedback Need reporters to triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants