Skip to content

Commit

Permalink
Remove AuditSQLException (#30777)
Browse files Browse the repository at this point in the history
* Refactor CDCChannelInboundHandler

* Rename DMLWithoutShardingKeyException

* Remove AuditSQLException

* Remove AuditSQLException
  • Loading branch information
terrymanu authored Apr 4, 2024
1 parent 14e5643 commit 78b89b6
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ SQL 错误码以标准的 SQL State,Vendor Code 和详细错误信息提供,
| 12100 | 42000 | You have an error in your SQL syntax: %s |
| 12101 | 42000 | Can not accept SQL type '%s'. |
| 12200 | 42000 | Hint data source '%s' does not exist. |
| 12201 | 42000 | SQL audit failed, error message: %s. |

### 连接

Expand Down Expand Up @@ -187,6 +186,7 @@ SQL 错误码以标准的 SQL State,Vendor Code 和详细错误信息提供,
| 20086 | 44000 | Some routed data sources do not belong to configured data sources. routed data sources: \`%s\`, configured data sources: \`%s\`. |
| 20087 | 44000 | Please check your sharding conditions \`%s\` to avoid same record in table \`%s\` routing to multiple data nodes. |
| 20088 | 44000 | Cannot found routing table factor, data source: %s, actual table: %s. |
| 20090 | 42000 | Not allow DML operation without sharding conditions. |

### 读写分离

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ SQL error codes provide by standard `SQL State`, `Vendor Code` and `Reason`, whi
| 20086 | 44000 | Some routed data sources do not belong to configured data sources. routed data sources: \`%s\`, configured data sources: \`%s\`. |
| 20087 | 44000 | Please check your sharding conditions \`%s\` to avoid same record in table \`%s\` routing to multiple data nodes. |
| 20088 | 44000 | Cannot found routing table factor, data source: %s, actual table: %s. |
| 20090 | 42000 | Not allow DML operation without sharding conditions. |

### Readwrite-splitting

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
package org.apache.shardingsphere.sharding.algorithm.audit;

import org.apache.shardingsphere.infra.binder.context.statement.SQLStatementContext;
import org.apache.shardingsphere.infra.exception.syntax.audit.SQLAuditException;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.database.rule.RuleMetaData;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.infra.exception.core.ShardingSpherePreconditions;
import org.apache.shardingsphere.sharding.exception.audit.DMLWithoutShardingKeyException;
import org.apache.shardingsphere.sharding.route.engine.condition.engine.ShardingConditionEngine;
import org.apache.shardingsphere.sharding.rule.ShardingRule;
import org.apache.shardingsphere.sharding.spi.ShardingAuditAlgorithm;
Expand All @@ -42,7 +42,7 @@ public void check(final SQLStatementContext sqlStatementContext, final List<Obje
ShardingRule rule = database.getRuleMetaData().getSingleRule(ShardingRule.class);
if (sqlStatementContext.getTablesContext().getTableNames().stream().anyMatch(rule::isShardingTable)) {
ShardingSpherePreconditions.checkState(!new ShardingConditionEngine(globalRuleMetaData, database, rule).createShardingConditions(sqlStatementContext, params).isEmpty(),
() -> new SQLAuditException("Not allow DML operation without sharding conditions"));
DMLWithoutShardingKeyException::new);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,19 @@
* limitations under the License.
*/

package org.apache.shardingsphere.infra.exception.syntax.audit;
package org.apache.shardingsphere.sharding.exception.audit;

import org.apache.shardingsphere.infra.exception.core.external.sql.sqlstate.XOpenSQLState;
import org.apache.shardingsphere.sharding.exception.ShardingSQLException;

/**
* SQL audit exception.
* DML without sharding key exception.
*/
public final class SQLAuditException extends AuditSQLException {
public final class DMLWithoutShardingKeyException extends ShardingSQLException {

private static final long serialVersionUID = 4183020614721058122L;
private static final long serialVersionUID = -2856743119521264343L;

public SQLAuditException(final String errorMessage) {
super(XOpenSQLState.SYNTAX_ERROR, 1, "SQL audit failed, error message: %s.", errorMessage);
public DMLWithoutShardingKeyException() {
super(XOpenSQLState.SYNTAX_ERROR, 90, "Not allow DML operation without sharding conditions.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
package org.apache.shardingsphere.sharding.algorithm.audit;

import org.apache.shardingsphere.infra.binder.context.statement.SQLStatementContext;
import org.apache.shardingsphere.infra.exception.syntax.audit.SQLAuditException;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.database.rule.RuleMetaData;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
import org.apache.shardingsphere.sharding.exception.audit.DMLWithoutShardingKeyException;
import org.apache.shardingsphere.sharding.rule.ShardingRule;
import org.apache.shardingsphere.sharding.spi.ShardingAuditAlgorithm;
import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DDLStatement;
Expand Down Expand Up @@ -70,6 +70,6 @@ void assertEmptyShardingConditionsCheck() {
when(sqlStatementContext.getSqlStatement()).thenReturn(mock(DMLStatement.class));
when(database.getRuleMetaData()).thenReturn(new RuleMetaData(Collections.singletonList(rule)));
when(rule.isShardingTable("t_order")).thenReturn(true);
assertThrows(SQLAuditException.class, () -> shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), mock(RuleMetaData.class), database));
assertThrows(DMLWithoutShardingKeyException.class, () -> shardingAuditAlgorithm.check(sqlStatementContext, Collections.emptyList(), mock(Grantee.class), mock(RuleMetaData.class), database));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package org.apache.shardingsphere.sharding.auditor;

import org.apache.shardingsphere.infra.binder.context.statement.CommonSQLStatementContext;
import org.apache.shardingsphere.infra.exception.syntax.audit.SQLAuditException;
import org.apache.shardingsphere.sharding.exception.audit.DMLWithoutShardingKeyException;
import org.apache.shardingsphere.infra.hint.HintValueContext;
import org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import org.apache.shardingsphere.infra.metadata.database.rule.RuleMetaData;
Expand Down Expand Up @@ -97,11 +97,11 @@ void assertCheckSuccessByDisableAuditNames() {
void assertCheckFailed() {
ShardingAuditAlgorithm auditAlgorithm = rule.getAuditors().get("auditor_1");
RuleMetaData globalRuleMetaData = mock(RuleMetaData.class);
doThrow(new SQLAuditException("Not allow DML operation without sharding conditions"))
doThrow(new DMLWithoutShardingKeyException())
.when(auditAlgorithm).check(sqlStatementContext, Collections.emptyList(), grantee, globalRuleMetaData, databases.get("foo_db"));
SQLAuditException ex = assertThrows(SQLAuditException.class,
DMLWithoutShardingKeyException ex = assertThrows(DMLWithoutShardingKeyException.class,
() -> new ShardingSQLAuditor().audit(sqlStatementContext, Collections.emptyList(), grantee, globalRuleMetaData, databases.get("foo_db"), rule, hintValueContext));
assertThat(ex.getMessage(), is("SQL audit failed, error message: Not allow DML operation without sharding conditions."));
assertThat(ex.getMessage(), is("Not allow DML operation without sharding conditions."));
verify(rule.getAuditors().get("auditor_1")).check(sqlStatementContext, Collections.emptyList(), grantee, globalRuleMetaData, databases.get("foo_db"));
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package org.apache.shardingsphere.data.pipeline.cdc.exception;

import lombok.Getter;
import org.apache.shardingsphere.infra.exception.core.external.sql.ShardingSphereSQLException;

/**
* CDC exception wrapper.
Expand All @@ -30,11 +29,11 @@ public final class CDCExceptionWrapper extends RuntimeException {

private final String requestId;

private final ShardingSphereSQLException exception;
private final Exception cause;

public CDCExceptionWrapper(final String requestId, final ShardingSphereSQLException exception) {
super(exception.getMessage());
public CDCExceptionWrapper(final String requestId, final Exception cause) {
super(cause.getMessage());
this.requestId = requestId;
this.exception = exception;
this.cause = cause;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,22 @@
import org.apache.shardingsphere.data.pipeline.cdc.protocol.response.CDCResponse.Status;
import org.apache.shardingsphere.data.pipeline.cdc.protocol.response.ServerGreetingResult;
import org.apache.shardingsphere.data.pipeline.core.exception.param.PipelineInvalidParameterException;
import org.apache.shardingsphere.infra.exception.metadata.rule.MissingRequiredRuleException;
import org.apache.shardingsphere.infra.autogen.version.ShardingSphereVersion;
import org.apache.shardingsphere.infra.exception.core.ShardingSpherePreconditions;
import org.apache.shardingsphere.infra.exception.core.external.sql.ShardingSphereSQLException;
import org.apache.shardingsphere.infra.exception.core.external.sql.sqlstate.XOpenSQLState;
import org.apache.shardingsphere.infra.exception.core.external.sql.type.kernel.category.PipelineSQLException;
import org.apache.shardingsphere.infra.exception.syntax.audit.SQLAuditException;
import org.apache.shardingsphere.infra.exception.dialect.SQLExceptionTransformEngine;
import org.apache.shardingsphere.infra.exception.dialect.exception.syntax.database.UnknownDatabaseException;
import org.apache.shardingsphere.infra.exception.metadata.rule.MissingRequiredRuleException;
import org.apache.shardingsphere.infra.exception.mysql.exception.AccessDeniedException;
import org.apache.shardingsphere.infra.metadata.user.Grantee;
import org.apache.shardingsphere.infra.metadata.user.ShardingSphereUser;
import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
import org.apache.shardingsphere.proxy.frontend.protocol.FrontDatabaseProtocolTypeFactory;

import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.sql.SQLException;
import java.util.Objects;
import java.util.Optional;

Expand Down Expand Up @@ -91,8 +94,8 @@ public void exceptionCaught(final ChannelHandlerContext ctx, final Throwable cau
ChannelFuture channelFuture;
if (cause instanceof CDCExceptionWrapper) {
CDCExceptionWrapper wrapper = (CDCExceptionWrapper) cause;
ShardingSphereSQLException exception = wrapper.getException();
channelFuture = ctx.writeAndFlush(CDCResponseUtils.failed(wrapper.getRequestId(), exception.toSQLException().getSQLState(), exception.getMessage()));
SQLException sqlException = SQLExceptionTransformEngine.toSQLException(wrapper.getCause(), FrontDatabaseProtocolTypeFactory.getDatabaseType());
channelFuture = ctx.writeAndFlush(CDCResponseUtils.failed(wrapper.getRequestId(), sqlException.getSQLState(), sqlException.getMessage()));
} else {
channelFuture = ctx.writeAndFlush(CDCResponseUtils.failed("", XOpenSQLState.GENERAL_ERROR.getValue(), String.valueOf(cause.getMessage())));
}
Expand Down Expand Up @@ -151,9 +154,9 @@ private void checkPrivileges(final String requestId, final Grantee grantee, fina
AuthorityRule authorityRule = ProxyContext.getInstance().getContextManager().getMetaDataContexts().getMetaData().getGlobalRuleMetaData().findSingleRule(AuthorityRule.class)
.orElseThrow(() -> new CDCExceptionWrapper(requestId, new MissingRequiredRuleException("authority")));
ShardingSpherePrivileges privileges = authorityRule.findPrivileges(grantee)
.orElseThrow(() -> new CDCExceptionWrapper(requestId, new SQLAuditException(String.format("Access denied for user '%s'", grantee))));
.orElseThrow(() -> new CDCExceptionWrapper(requestId, new AccessDeniedException(grantee.getUsername(), grantee.getHostname(), false)));
ShardingSpherePreconditions.checkState(privileges.hasPrivileges(currentDatabase),
() -> new CDCExceptionWrapper(requestId, new SQLAuditException(String.format("Unknown database '%s'", currentDatabase))));
() -> new CDCExceptionWrapper(requestId, new UnknownDatabaseException(currentDatabase)));
}

private String getHostAddress(final ChannelHandlerContext context) {
Expand Down

0 comments on commit 78b89b6

Please sign in to comment.