Skip to content

Fix NPE in RouterChain.java#1049

Merged
EvenLjj merged 3 commits intosofastack:masterfrom
containerAnalyzer:master
Oct 20, 2021
Merged

Fix NPE in RouterChain.java#1049
EvenLjj merged 3 commits intosofastack:masterfrom
containerAnalyzer:master

Conversation

@containerAnalyzer
Copy link
Copy Markdown
Contributor

Here is the PR for the issue 1048

@sofastack-bot
Copy link
Copy Markdown

sofastack-bot bot commented Jun 15, 2021

Hi @containerAnalyzer, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

@sofastack-bot sofastack-bot bot added bug Something isn't working cla:no Need sign CLA First-time contributor First-time contributor size/XS labels Jun 15, 2021
@OrezzerO
Copy link
Copy Markdown
Contributor

Bug fix needs a test case to indicate the bug is fixed. @containerAnalyzer

@containerAnalyzer
Copy link
Copy Markdown
Contributor Author

Here is the test case

@Test
public void buildConsumerChain() {
    ConsumerConfig consumerConfig = new ConsumerConfig<>();
    consumerConfig.setBootstrap("test");
    ArrayList<String> router = new ArrayList<>();
    router.add("notExist");
    consumerConfig.setRouter(router);
    ConsumerBootstrap consumerBootstrap = Bootstraps.from(consumerConfig);
    RouterChain routerChain = RouterChain.buildConsumerChain(consumerBootstrap);
}

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 5, 2021

Codecov Report

Merging #1049 (4179a2d) into master (03abf33) will increase coverage by 0.21%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1049      +/-   ##
============================================
+ Coverage     68.84%   69.05%   +0.21%     
- Complexity      820      831      +11     
============================================
  Files           407      409       +2     
  Lines         17619    17723     +104     
  Branches       2728     2750      +22     
============================================
+ Hits          12130    12239     +109     
+ Misses         4085     4068      -17     
- Partials       1404     1416      +12     
Impacted Files Coverage Δ
...n/java/com/alipay/sofa/rpc/client/RouterChain.java 90.12% <100.00%> (+0.25%) ⬆️
...ipay/sofa/rpc/server/bolt/BoltServerProcessor.java 63.23% <0.00%> (-5.15%) ⬇️
.../com/alipay/sofa/rpc/common/utils/StringUtils.java 75.00% <0.00%> (-3.79%) ⬇️
...n/java/com/alipay/sofa/rpc/common/SofaConfigs.java 84.90% <0.00%> (-1.89%) ⬇️
...pay/sofa/rpc/transport/ClientTransportFactory.java 76.92% <0.00%> (-1.54%) ⬇️
...lipay/sofa/rpc/message/AbstractResponseFuture.java 56.14% <0.00%> (-0.88%) ⬇️
...ipay/sofa/rpc/codec/bolt/SofaRpcSerialization.java 72.95% <0.00%> (-0.82%) ⬇️
...ipay/sofa/rpc/tracer/sofatracer/RpcSofaTracer.java 88.42% <0.00%> (-0.60%) ⬇️
...va/com/alipay/sofa/rpc/client/AbstractCluster.java 68.57% <0.00%> (-0.28%) ⬇️
.../java/com/alipay/sofa/rpc/client/ProviderInfo.java 74.07% <0.00%> (ø)
... and 13 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 03abf33...4179a2d. Read the comment docs.

@EvenLjj EvenLjj linked an issue Jul 23, 2021 that may be closed by this pull request
@leizhiyuan
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor

@leizhiyuan leizhiyuan left a comment

Choose a reason for hiding this comment

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

LGTM

@JervyShi
Copy link
Copy Markdown
Member

JervyShi commented Sep 10, 2021

Here is the test case

@Test
public void buildConsumerChain() {
    ConsumerConfig consumerConfig = new ConsumerConfig<>();
    consumerConfig.setBootstrap("test");
    ArrayList<String> router = new ArrayList<>();
    router.add("notExist");
    consumerConfig.setRouter(router);
    ConsumerBootstrap consumerBootstrap = Bootstraps.from(consumerConfig);
    RouterChain routerChain = RouterChain.buildConsumerChain(consumerBootstrap);
}

Could you add test cases to codebase?

@sofastack-bot sofastack-bot bot added cla:yes CLA is ok and removed cla:no Need sign CLA labels Oct 20, 2021
Copy link
Copy Markdown
Collaborator

@EvenLjj EvenLjj left a comment

Choose a reason for hiding this comment

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

LGTM

@EvenLjj EvenLjj merged commit bfa2118 into sofastack:master Oct 20, 2021
@EvenLjj EvenLjj added this to the 5.8.0 milestone Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cla:yes CLA is ok First-time contributor First-time contributor size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

One NPE in RouterChain.java

5 participants