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 2022/05/17 12:07:39 UTC
[GitHub] [shardingsphere] cheese8 commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
cheese8 commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r874717643
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptColumn.java:
##########
@@ -64,4 +71,5 @@ public Optional<String> getAssistedQueryColumn() {
public Optional<String> getPlainColumn() {
return Strings.isNullOrEmpty(plainColumn) ? Optional.empty() : Optional.of(plainColumn);
}
+
Review Comment:
Is it a new added blank line? How about to remove it?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptColumn.java:
##########
@@ -47,6 +47,13 @@ public final class EncryptColumn {
private final String encryptorName;
+ private final Boolean queryWithCipherColumn;
+
+ public EncryptColumn(final EncryptColumnDataType logicDataType, final String cipherColumn, final EncryptColumnDataType cipherDataType, final String assistedQueryColumn,
Review Comment:
How about remove this new added constructor and adapt the original one where it is used?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-api/src/main/java/org/apache/shardingsphere/encrypt/api/config/rule/EncryptColumnRuleConfiguration.java:
##########
@@ -45,7 +45,19 @@ public final class EncryptColumnRuleConfiguration {
private final String encryptorName;
+ private final Boolean queryWithCipherColumn;
+
public EncryptColumnRuleConfiguration(final String logicColumn, final String cipherColumn, final String assistedQueryColumn, final String plainColumn, final String encryptorName) {
- this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName);
+ this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName, null);
+ }
+
+ public EncryptColumnRuleConfiguration(final String logicColumn, final String cipherColumn, final String assistedQueryColumn, final String plainColumn, final String encryptorName,
+ final Boolean queryWithCipherColumn) {
+ this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName, queryWithCipherColumn);
+ }
+
+ public EncryptColumnRuleConfiguration(final String logicColumn, final String logicDataType, final String cipherColumn, final String cipherDataType, final String assistedQueryColumn,
Review Comment:
How about remove this new added constructor and adapt the original one where it is used?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptRule.java:
##########
@@ -294,6 +294,17 @@ public boolean isQueryWithCipherColumn(final String tableName) {
return findEncryptTable(tableName).flatMap(EncryptTable::getQueryWithCipherColumn).orElse(queryWithCipherColumn);
}
+ /**
+ * Judge whether column is support QueryWithCipherColumn or not.
+ *
+ * @param logicTable logic table name
+ * @param logicColumn logic column name
+ * @return whether column is support QueryWithCipherColumn or not
+ */
+ public boolean isQueryWithCipherColumn(final String logicTable, final String logicColumn) {
Review Comment:
It looks like another `isQueryWithCipherColumn(logicTable)` exists, how about to adopt that one, and pay attention to the inheritance order on `queryWithCipherColumn` from global to table and column.
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -196,4 +206,5 @@ public Optional<Boolean> getQueryWithCipherColumn() {
public Optional<EncryptColumn> findEncryptColumn(final String logicColumn) {
return Optional.ofNullable(columns.get(logicColumn));
}
+
Review Comment:
Is it a new added blank line? How about to remove it?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -187,6 +187,16 @@ public Optional<Boolean> getQueryWithCipherColumn() {
return Optional.ofNullable(queryWithCipherColumn);
}
+ /**
+ * Get query with cipher column.
+ *
+ * @param logicColumn logic column
+ * @return query with cipher column
+ */
+ public Optional<Boolean> getQueryWithCipherColumn(final String logicColumn) {
Review Comment:
How about inherit the isQueryWithCipherColumn on table with column here?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -48,7 +48,7 @@ public EncryptTable(final EncryptTableRuleConfiguration config, final Map<String
columns.put(each.getLogicColumn(), new EncryptColumn(getEncryptColumnDataType(each.getLogicDataType(), dataTypes), each.getCipherColumn(),
getEncryptColumnDataType(each.getCipherDataType(), dataTypes), each.getAssistedQueryColumn(), getEncryptColumnDataType(each.getAssistedQueryDataType(),
dataTypes),
- each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName()));
+ each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName(), each.getQueryWithCipherColumn()));
Review Comment:
How about inherit the `isQueryWithCipherColumn` on table with column here?
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org