kezhenxu94 commented on a change in pull request #8706:
URL: https://github.com/apache/skywalking/pull/8706#discussion_r829948302
##########
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java
##########
@@ -113,28 +113,39 @@ private Fields(Mappings mapping) {
* Returns ture when the input fields have already been stored in the
properties.
*/
private boolean containsAllFields(Fields fields) {
- return fields.properties.entrySet().stream()
- .allMatch(item ->
this.properties.containsKey(item.getKey()));
+ if (this.properties.size() < fields.properties.size()) {
+ return false;
+ }
+ boolean isContains = false;
+ isContains = fields.properties.entrySet().stream()
+ .allMatch(item -> {
+ Object hisItem =
this.properties.get(item.getKey());
+ if (hisItem != null) {
+ return
hisItem.toString().equals(item.getValue().toString());
+ } else {
+ return false;
+ }
+ });
+ if (!isContains) {
+ return false;
+ }
+
+ if (fields.source != null && this.source != null) {
+ isContains =
this.source.getExcludes().toString().equals(fields.source.getExcludes().toString());
Review comment:
Consider implement the `equals` method in class `Source`, instead of
comparing their `toString()` methods, which is error prone
##########
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java
##########
@@ -113,28 +113,39 @@ private Fields(Mappings mapping) {
* Returns ture when the input fields have already been stored in the
properties.
*/
private boolean containsAllFields(Fields fields) {
- return fields.properties.entrySet().stream()
- .allMatch(item ->
this.properties.containsKey(item.getKey()));
+ if (this.properties.size() < fields.properties.size()) {
+ return false;
+ }
+ boolean isContains = false;
+ isContains = fields.properties.entrySet().stream()
+ .allMatch(item -> {
+ Object hisItem =
this.properties.get(item.getKey());
+ if (hisItem != null) {
+ return
hisItem.toString().equals(item.getValue().toString());
+ } else {
+ return false;
+ }
+ });
+ if (!isContains) {
+ return false;
+ }
+
+ if (fields.source != null && this.source != null) {
+ isContains =
this.source.getExcludes().toString().equals(fields.source.getExcludes().toString());
+ } else {
+ return fields.source == null && this.source == null;
+ }
+ return isContains;
Review comment:
After you implement the `equals` method in `Source`,
```suggestion
return Objects.equals(this.source, fields.source);
```
##########
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/IndexStructures.java
##########
@@ -113,28 +113,39 @@ private Fields(Mappings mapping) {
* Returns ture when the input fields have already been stored in the
properties.
*/
private boolean containsAllFields(Fields fields) {
- return fields.properties.entrySet().stream()
- .allMatch(item ->
this.properties.containsKey(item.getKey()));
+ if (this.properties.size() < fields.properties.size()) {
+ return false;
+ }
+ boolean isContains = false;
+ isContains = fields.properties.entrySet().stream()
+ .allMatch(item -> {
+ Object hisItem =
this.properties.get(item.getKey());
+ if (hisItem != null) {
+ return
hisItem.toString().equals(item.getValue().toString());
+ } else {
+ return false;
+ }
+ });
+ if (!isContains) {
+ return false;
+ }
Review comment:
```suggestion
boolean contains =
fields.properties
.entrySet()
.stream()
.allMatch(
item ->
Objects.equals(properties.get(item.getKey()), item.getValue()));
if (!contains) {
return false;
}
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
