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

[Dubbo-5049]support @Bean on Config class to config id as alias #5063

Merged
merged 1 commit into from
Sep 19, 2019

Conversation

sonymoon
Copy link
Contributor

@sonymoon sonymoon commented Sep 12, 2019

as #5049 mentioned, when @Service(protocol = "dubbo1") is annotated on a class, the provider starts failed, dubbo1 is the id property of ProtocolConfig.

@Bean
  public ProtocolConfig config1() {
    ProtocolConfig config = new ProtocolConfig();
    config.setId("dubbo1");
    config.setName("dubbo");
    config.setPort(20880);
    return config;
  }

The failure cause is dubbo1 is not an name or alias of ProtocolConfig bean.

When using xml way:

<dubbo:application name="demo-provider"/>

    <dubbo:registry address="zookeeper://127.0.0.1:2181"/>

    <dubbo:protocol name="dubbo" id="dubbo1"/>

    <bean id="demoService" class="org.apache.dubbo.demo.provider.DemoServiceImpl"/>

    <dubbo:service interface="org.apache.dubbo.demo.DemoService" ref="demoService" protocol="dubbo1"/>

xml config with id property assigned with dubbo1, the provider starts successfully.

I think the annotation way should work fine as well.

My suggestion approach

Use spring extension interfaces, such as:

  1. BeanDefinitionRegistry: can do the registry alias stuff
  2. BeanPostProcessor: can do 1 after config beans's initialization by implementing method postProcessAfterInitialization, which is already implemented by class DubboConfigBindingBeanPostProcessor with an empty method.

Actually, @haiyang1985 has a PR #5051 solving this issue:
But I think using getBean in postProcessBeanDefinitionRegistry method cause the initialization of ProtocolConfig's bean too early (bean registry phase in spring) as @mercyblitz mentioned

@sonymoon sonymoon changed the title [Dubbo-5049]support annotation config id property to be a alias for a… [Dubbo-5049]support @Bean on Config class to config id as alias Sep 12, 2019
@codecov-io
Copy link

codecov-io commented Sep 12, 2019

Codecov Report

Merging #5063 into master will decrease coverage by 0.69%.
The diff coverage is 80%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #5063     +/-   ##
===========================================
- Coverage     63.98%   63.29%   -0.7%     
+ Complexity      452      449      -3     
===========================================
  Files           769      790     +21     
  Lines         33206    35042   +1836     
  Branches       5237     5480    +243     
===========================================
+ Hits          21248    22180    +932     
- Misses         9533    10328    +795     
- Partials       2425     2534    +109
Impacted Files Coverage Δ Complexity Δ
...nnotation/DubboConfigBindingBeanPostProcessor.java 74.24% <80%> (+1.02%) 0 <0> (ø) ⬇️
...che/dubbo/remoting/transport/mina/MinaChannel.java 43.42% <0%> (-10.53%) 16% <0%> (-1%)
...ng/exchange/support/header/HeartbeatTimerTask.java 73.68% <0%> (-5.27%) 0% <0%> (ø)
.../apache/dubbo/qos/protocol/QosProtocolWrapper.java 65.85% <0%> (-4.88%) 0% <0%> (ø)
.../apache/dubbo/remoting/transport/AbstractPeer.java 58.69% <0%> (-4.35%) 0% <0%> (ø)
...he/dubbo/remoting/transport/netty/NettyServer.java 69.64% <0%> (-3.58%) 8% <0%> (-1%)
...e/dubbo/remoting/transport/netty/NettyChannel.java 52.27% <0%> (-3.41%) 19% <0%> (-1%)
.../rpc/protocol/dubbo/LazyConnectExchangeClient.java 56.47% <0%> (-2.36%) 0% <0%> (ø)
.../dubbo/remoting/transport/netty4/NettyChannel.java 64.77% <0%> (-2.28%) 0% <0%> (ø)
...he/dubbo/registry/multicast/MulticastRegistry.java 67.87% <0%> (-1.81%) 0% <0%> (ø)
... and 23 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 60abb6d...527f958. Read the comment docs.

@mercyblitz mercyblitz merged commit 87631aa into apache:master Sep 19, 2019
chickenlj pushed a commit that referenced this pull request Sep 19, 2019
@haiyang1985
Copy link
Member

The PR have not solved my issue. When user defined ProtocolConfig as a spring bean , service annotation cannot get the dubbo bean id.

@Service(protocol = "dubbo1")
public class DubboDemoServiceImpl implements DubboDemoService {
}
@Configuration
public class DubboConfiguration {
  @Bean
  public ProtocolConfig getConfig() {
    ProtocolConfig config = new ProtocolConfig();
    config.setId("dubbo1");
    config.setName("dubbo");
    config.setPort(20770);
    return config;
  }
}

I've tried your solution in my local, still not able to get dubbo bean id.

***************************
APPLICATION FAILED TO START
***************************

Description:

A component required a bean named 'dubbo1' that could not be found.

@sonymoon
Copy link
Contributor Author

Can you provider a GitHub address which can reproduce this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/need-triage Need maintainers to triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants