You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/11/25 08:38:30 UTC
[GitHub] [shardingsphere-elasticjob] TeslaCN opened a new pull request #1734: Add null check in SnapshotService to avoid NPE
TeslaCN opened a new pull request #1734:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1734
Fixes #1732
Changes proposed in this pull request:
- Add null check in SnapshotService to avoid NPE.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] codecov-io edited a comment on pull request #1734: Add null check in SnapshotService to avoid NPE
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1734:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1734#issuecomment-733558715
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=h1) Report
> Merging [#1734](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=desc) (3b2a2ed) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/1df7beaac29d4907a176710750f7c3dd56c116b1?el=desc) (1df7bea) will **increase** coverage by `0.03%`.
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1734 +/- ##
============================================
+ Coverage 86.18% 86.22% +0.03%
Complexity 105 105
============================================
Files 252 252
Lines 5682 5683 +1
Branches 878 878
============================================
+ Hits 4897 4900 +3
+ Misses 471 470 -1
+ Partials 314 313 -1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ticjob/lite/internal/snapshot/SnapshotService.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc25hcHNob3QvU25hcHNob3RTZXJ2aWNlLmphdmE=) | `84.37% <85.71%> (+3.42%)` | `1.00 <0.00> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=footer). Last update [1df7bea...3b2a2ed](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] codecov-io commented on pull request #1734: Add null check in SnapshotService to avoid NPE
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1734:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1734#issuecomment-733558715
# [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=h1) Report
> Merging [#1734](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=desc) (3b2a2ed) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/1df7beaac29d4907a176710750f7c3dd56c116b1?el=desc) (1df7bea) will **decrease** coverage by `0.06%`.
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1734 +/- ##
============================================
- Coverage 86.18% 86.11% -0.07%
Complexity 105 105
============================================
Files 252 252
Lines 5682 5683 +1
Branches 878 878
============================================
- Hits 4897 4894 -3
- Misses 471 475 +4
Partials 314 314
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ticjob/lite/internal/snapshot/SnapshotService.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1saXRlL2VsYXN0aWNqb2ItbGl0ZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9lbGFzdGljam9iL2xpdGUvaW50ZXJuYWwvc25hcHNob3QvU25hcHNob3RTZXJ2aWNlLmphdmE=) | `79.68% <85.71%> (-1.27%)` | `1.00 <0.00> (ø)` | |
| [...e/shardingsphere/elasticjob/infra/env/IpUtils.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734/diff?src=pr&el=tree#diff-ZWxhc3RpY2pvYi1pbmZyYS9lbGFzdGljam9iLWluZnJhLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZWxhc3RpY2pvYi9pbmZyYS9lbnYvSXBVdGlscy5qYXZh) | `60.00% <0.00%> (-4.62%)` | `0.00% <0.00%> (ø%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=footer). Last update [1df7bea...3b2a2ed](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/1734?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1734: Add null check in SnapshotService to avoid NPE
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1734:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1734#discussion_r530406723
##########
File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/snapshot/SnapshotService.java
##########
@@ -109,14 +109,15 @@ private void process(final Socket socket) throws IOException {
private void dumpDirectly(final String path, final String jobName, final List<String> result) {
for (String each : regCenter.getChildrenKeys(path)) {
String zkPath = path + "/" + each;
- String zkValue = regCenter.get(zkPath);
- if (null == zkValue) {
- zkValue = "";
- }
+ String zkValue = Optional.ofNullable(regCenter.get(zkPath)).orElse("");
+ String cachePath = zkPath;
+ String cacheValue = zkValue;
CuratorCache cache = (CuratorCache) regCenter.getRawCache("/" + jobName);
- Optional<ChildData> cacheData = cache.get(zkPath);
- String cachePath = cacheData.map(ChildData::getPath).orElse("");
- String cacheValue = cacheData.map(childData -> new String(childData.getData())).orElse("");
+ if (null != cache) {
Review comment:
If cache == null, that means cachePath equals zkPath. and will let the logic at line 121 always be true. it's not the same logic as before.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a change in pull request #1734: Add null check in SnapshotService to avoid NPE
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1734:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1734#discussion_r530411602
##########
File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/snapshot/SnapshotService.java
##########
@@ -109,14 +109,15 @@ private void process(final Socket socket) throws IOException {
private void dumpDirectly(final String path, final String jobName, final List<String> result) {
for (String each : regCenter.getChildrenKeys(path)) {
String zkPath = path + "/" + each;
- String zkValue = regCenter.get(zkPath);
- if (null == zkValue) {
- zkValue = "";
- }
+ String zkValue = Optional.ofNullable(regCenter.get(zkPath)).orElse("");
+ String cachePath = zkPath;
+ String cacheValue = zkValue;
CuratorCache cache = (CuratorCache) regCenter.getRawCache("/" + jobName);
- Optional<ChildData> cacheData = cache.get(zkPath);
- String cachePath = cacheData.map(ChildData::getPath).orElse("");
- String cacheValue = cacheData.map(childData -> new String(childData.getData())).orElse("");
+ if (null != cache) {
Review comment:
`cache == null` means there is no cache, so there is no need to dump both zk data and cache data.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a change in pull request #1734: Add null check in SnapshotService to avoid NPE
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1734:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1734#discussion_r530411602
##########
File path: elasticjob-lite/elasticjob-lite-core/src/main/java/org/apache/shardingsphere/elasticjob/lite/internal/snapshot/SnapshotService.java
##########
@@ -109,14 +109,15 @@ private void process(final Socket socket) throws IOException {
private void dumpDirectly(final String path, final String jobName, final List<String> result) {
for (String each : regCenter.getChildrenKeys(path)) {
String zkPath = path + "/" + each;
- String zkValue = regCenter.get(zkPath);
- if (null == zkValue) {
- zkValue = "";
- }
+ String zkValue = Optional.ofNullable(regCenter.get(zkPath)).orElse("");
+ String cachePath = zkPath;
+ String cacheValue = zkValue;
CuratorCache cache = (CuratorCache) regCenter.getRawCache("/" + jobName);
- Optional<ChildData> cacheData = cache.get(zkPath);
- String cachePath = cacheData.map(ChildData::getPath).orElse("");
- String cacheValue = cacheData.map(childData -> new String(childData.getData())).orElse("");
+ if (null != cache) {
Review comment:
If `cache == null` means there is no cache, so there is no need to dump both zk data and cache data.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] Technoboy- merged pull request #1734: Add null check in SnapshotService to avoid NPE
Posted by GitBox <gi...@apache.org>.
Technoboy- merged pull request #1734:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1734
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org