Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

[#109]Apply placeholder check while binding directUrl and type #111

Closed
wants to merge 1 commit into from
Closed

[#109]Apply placeholder check while binding directUrl and type #111

wants to merge 1 commit into from

Conversation

chpengzh
Copy link
Member

@chpengzh chpengzh commented Nov 5, 2018

Motivation:

希望能提供基于spring boot environment 的测试服务直连选项.详细参考#109 (comment)

Modification:

另一部分修改在chpengzh/sofa-boot

demo参考 demo

@RestController
@PropertySource("classpath:/reference.properties")
public class DemoController {

    @SofaReference(binding = @SofaReferenceBinding(
        bindingType = "${demo.alice.scheme}",
        directUrl = "${demo.alice.url}"
    ))
    private CalculateService calculator;

    // ...
}

Result:

Fixes #109

@codecov
Copy link

codecov bot commented Nov 5, 2018

Codecov Report

Merging #111 into master will decrease coverage by 0.07%.
The diff coverage is 37.5%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #111      +/-   ##
============================================
- Coverage     54.85%   54.77%   -0.08%     
- Complexity      393      394       +1     
============================================
  Files            53       53              
  Lines          1741     1747       +6     
  Branches        310      313       +3     
============================================
+ Hits            955      957       +2     
- Misses          606      608       +2     
- Partials        180      182       +2
Impacted Files Coverage Δ Complexity Δ
...pc/boot/runtime/converter/RpcBindingConverter.java 43.69% <37.5%> (-0.29%) 24 <2> (+1)

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 b9676c3...1dd3f4f. Read the comment docs.

@JervyShi
Copy link
Member

JervyShi commented Nov 6, 2018

@chpengzh 可以添加一些测试用例在 sofa-boot-starter test 中,例如:

// 1. import DirectService using anotation

    @SofaReference(binding = @SofaReferenceBinding(
        bindingType = "${test.direct.scheme}",
        directUrl = "${test.direct.url}"
    ))
    private DirectService directService;

// 2. try to invoke sayDirect

    directService.sayDirect("hello");

// 3. confirm result is right

    assert  etc.

test.direct.scheme 配置可以放在 application.properties 中验证被替换场景。

Copy link

@leyou240 leyou240 left a comment

Choose a reason for hiding this comment

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

个人意见。其他参数也应该支持placeholder吧。另外service annotation可能也需要支持。所有的配置我们都倾向于写到配置文件中,然后在java中引用

@chpengzh
Copy link
Member Author

chpengzh commented Nov 6, 2018

征求下意见 @JervyShi, 在此PR里面顺带添加service placeholder的功能, 还是新建一个issue ticket?

@QilongZhang
Copy link
Contributor

@chpengzh @JervyShi 支持注解中的 placeholder 应该是由 SOFABoot 框架支持,在 RPC|Bolt|Dubbo Converter 做转换是不太优雅的实现。另@SofaService 和 @SofaReference 两个注解应该支持每个属性的替代符.

已经在SOFABoot 提了 PR ,可以一起 review

@QilongZhang
Copy link
Contributor

sofastack/sofa-boot#264

@QilongZhang QilongZhang closed this Nov 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants