-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
bugfix:fix type casting problem when using redis as registry #2179
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2179 +/- ##
============================================
+ Coverage 53.03% 53.14% +0.1%
- Complexity 2511 2513 +2
============================================
Files 484 485 +1
Lines 15384 15360 -24
Branches 1777 1771 -6
============================================
+ Hits 8159 8163 +4
+ Misses 6437 6410 -27
+ Partials 788 787 -1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seata1.0redis自动注入失败一共有两个问题,
1:不支持密码
2:转换异常
做如下修改
@component
@ConfigurationProperties(prefix = REGISTRY_REDIS_PREFIX)
public class RegistryRedisProperties {
private String serverAddr = "localhost:6379";
private String db = "0";
private String password = "";
public String getServerAddr() {
return serverAddr;
}
public RegistryRedisProperties setServerAddr(String serverAddr) {
this.serverAddr = serverAddr;
return this;
}
public String getPassword() {
return password;
}
public RegistryRedisProperties setPassword(String password) {
this.password = password;
return this;
}
public String getDb() {
return db;
}
public RegistryRedisProperties setDb(String db) {
this.db = db;
return this;
}
}
/**
* getRedisDbFileKey()获取的是key,不是value,无法强转,导致 unable not cast
*/
int db = seataConfig.getInt(seataConfig.getConfig(getRedisDbFileKey()));
@renyuanxin111 类型已经修复,fix #2160 |
...ain/java/io/seata/spring/boot/autoconfigure/properties/registry/RegistryRedisProperties.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't actually verified this pr, but I think the problem is that we need int type but reflection returns Object. Can you add unit tests about this pr?
@slievrly I don't know how to write this unit test. I package seata-spring-boot-starter and run in my program with debug, it's ok |
seata-spring-boot-starter/src/main/resources/spring/redis_auto_injectio_test.xml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but test coverage is reduced a lot.
...ain/java/io/seata/spring/boot/autoconfigure/properties/registry/RegistryRedisProperties.java
Show resolved
Hide resolved
...rter/src/test/java/io/seata/spring/boot/autoconfigure/RedisAutoInjectionTypeConvertTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…s/2160
Ⅰ. Describe what this PR did
fix the problem of spring redis register auto injection error ,the issues address:
#2160
Ⅱ. Does this pull request fix one issue?
fixes #2160
Ⅲ. Why don't you add test cases (unit test/integration test)?
unit test
Ⅳ. Describe how to verify it
spring-redis register auto injection
Ⅴ. Special notes for reviews