This is an automated email from the ASF dual-hosted git repository.
sunnianjun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new b8ecca09365 Refactor MySQLErrPacket (#28131)
b8ecca09365 is described below
commit b8ecca093659664cc40f97ac3af8fd8ac235c49a
Author: Liang Zhang <[email protected]>
AuthorDate: Thu Aug 17 06:33:21 2023 +0800
Refactor MySQLErrPacket (#28131)
---
.../mysql/codec/MySQLPacketCodecEngine.java | 4 +--
.../mysql/packet/generic/MySQLErrPacket.java | 20 ++++++++---
.../mysql/packet/generic/MySQLErrPacketTest.java | 24 +++++++++----
.../client/netty/MySQLNegotiateHandlerTest.java | 7 ++--
.../mysql/err/MySQLErrorPacketFactory.java | 17 ++--------
.../mysql/err/MySQLErrorPacketFactoryTest.java | 39 +---------------------
6 files changed, 42 insertions(+), 69 deletions(-)
diff --git
a/db-protocol/mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/codec/MySQLPacketCodecEngine.java
b/db-protocol/mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/codec/MySQLPacketCodecEngine.java
index 57d54ae8269..9adb3d73dff 100644
---
a/db-protocol/mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/codec/MySQLPacketCodecEngine.java
+++
b/db-protocol/mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/codec/MySQLPacketCodecEngine.java
@@ -29,7 +29,6 @@ import
org.apache.shardingsphere.db.protocol.packet.DatabasePacket;
import
org.apache.shardingsphere.infra.exception.core.external.sql.type.generic.UnknownSQLException;
import java.nio.charset.Charset;
-import java.sql.SQLException;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
@@ -95,8 +94,7 @@ public final class MySQLPacketCodecEngine implements
DatabasePacketCodecEngine {
} catch (final RuntimeException ex) {
// CHECKSTYLE:ON
out.resetWriterIndex();
- SQLException unknownSQLException = new
UnknownSQLException(ex).toSQLException();
- new MySQLErrPacket(unknownSQLException.getErrorCode(),
unknownSQLException.getSQLState(),
unknownSQLException.getMessage()).write(payload);
+ new MySQLErrPacket(new
UnknownSQLException(ex).toSQLException()).write(payload);
} finally {
if (out.readableBytes() - PAYLOAD_LENGTH - SEQUENCE_LENGTH <
MAX_PACKET_LENGTH) {
updateMessageHeader(out,
context.channel().attr(MySQLConstants.MYSQL_SEQUENCE_ID).get().getAndIncrement());
diff --git
a/db-protocol/mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/packet/generic/MySQLErrPacket.java
b/db-protocol/mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/packet/generic/MySQLErrPacket.java
index 3ae0749bf4b..842041f4471 100644
---
a/db-protocol/mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/packet/generic/MySQLErrPacket.java
+++
b/db-protocol/mysql/src/main/java/org/apache/shardingsphere/db/protocol/mysql/packet/generic/MySQLErrPacket.java
@@ -18,18 +18,19 @@
package org.apache.shardingsphere.db.protocol.mysql.packet.generic;
import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
import lombok.Getter;
-import lombok.RequiredArgsConstructor;
import org.apache.shardingsphere.db.protocol.mysql.packet.MySQLPacket;
import org.apache.shardingsphere.db.protocol.mysql.payload.MySQLPacketPayload;
-import
org.apache.shardingsphere.infra.exception.core.external.sql.vendor.VendorError;
+import org.apache.shardingsphere.infra.exception.mysql.vendor.MySQLVendorError;
+
+import java.sql.SQLException;
/**
* ERR packet protocol for MySQL.
*
* @see <a
href="https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_err_packet.html";>ERR
Packet</a>
*/
-@RequiredArgsConstructor
@Getter
public final class MySQLErrPacket extends MySQLPacket {
@@ -46,8 +47,17 @@ public final class MySQLErrPacket extends MySQLPacket {
private final String errorMessage;
- public MySQLErrPacket(final VendorError vendorError, final Object...
errorMessageArgs) {
- this(vendorError.getVendorCode(),
vendorError.getSqlState().getValue(), String.format(vendorError.getReason(),
errorMessageArgs));
+ public MySQLErrPacket(final SQLException exception) {
+ if (null == exception.getSQLState()) {
+ errorCode = MySQLVendorError.ER_INTERNAL_ERROR.getVendorCode();
+ sqlState =
MySQLVendorError.ER_INTERNAL_ERROR.getSqlState().getValue();
+ errorMessage =
String.format(MySQLVendorError.ER_INTERNAL_ERROR.getReason(),
+ null == exception.getNextException() ||
!Strings.isNullOrEmpty(exception.getMessage()) ? exception.getMessage() :
exception.getNextException().getMessage());
+ } else {
+ errorCode = exception.getErrorCode();
+ sqlState = exception.getSQLState();
+ errorMessage = exception.getMessage();
+ }
}
public MySQLErrPacket(final MySQLPacketPayload payload) {
diff --git
a/db-protocol/mysql/src/test/java/org/apache/shardingsphere/db/protocol/mysql/packet/generic/MySQLErrPacketTest.java
b/db-protocol/mysql/src/test/java/org/apache/shardingsphere/db/protocol/mysql/packet/generic/MySQLErrPacketTest.java
index 83b17d126de..ddf6a46a742 100644
---
a/db-protocol/mysql/src/test/java/org/apache/shardingsphere/db/protocol/mysql/packet/generic/MySQLErrPacketTest.java
+++
b/db-protocol/mysql/src/test/java/org/apache/shardingsphere/db/protocol/mysql/packet/generic/MySQLErrPacketTest.java
@@ -17,13 +17,16 @@
package org.apache.shardingsphere.db.protocol.mysql.packet.generic;
-import org.apache.shardingsphere.infra.exception.mysql.vendor.MySQLVendorError;
import org.apache.shardingsphere.db.protocol.mysql.payload.MySQLPacketPayload;
+import
org.apache.shardingsphere.infra.exception.core.external.sql.sqlstate.XOpenSQLState;
+import org.apache.shardingsphere.infra.exception.mysql.vendor.MySQLVendorError;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
+import java.sql.SQLException;
+
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.mockito.Mockito.verify;
@@ -37,10 +40,18 @@ class MySQLErrPacketTest {
@Test
void assertNewErrPacketWithServerErrorCode() {
- MySQLErrPacket actual = new
MySQLErrPacket(MySQLVendorError.ER_ACCESS_DENIED_ERROR, "root", "localhost",
"root");
- assertThat(actual.getErrorCode(),
is(MySQLVendorError.ER_ACCESS_DENIED_ERROR.getVendorCode()));
- assertThat(actual.getSqlState(),
is(MySQLVendorError.ER_ACCESS_DENIED_ERROR.getSqlState().getValue()));
- assertThat(actual.getErrorMessage(),
is(String.format(MySQLVendorError.ER_ACCESS_DENIED_ERROR.getReason(), "root",
"localhost", "root")));
+ MySQLErrPacket actual = new MySQLErrPacket(new SQLException("test",
"FOO_STATE", 1));
+ assertThat(actual.getErrorCode(), is(1));
+ assertThat(actual.getSqlState(), is("FOO_STATE"));
+ assertThat(actual.getErrorMessage(), is("test"));
+ }
+
+ @Test
+ void assertNewErrPacketWithInternalServerError() {
+ MySQLErrPacket actual = new MySQLErrPacket(new SQLException("No
reason"));
+ assertThat(actual.getErrorCode(), is(1815));
+ assertThat(actual.getSqlState(),
is(XOpenSQLState.GENERAL_ERROR.getValue()));
+ assertThat(actual.getErrorMessage(), is("Internal error: No reason"));
}
@Test
@@ -58,7 +69,8 @@ class MySQLErrPacketTest {
@Test
void assertWrite() {
- new MySQLErrPacket(MySQLVendorError.ER_NO_DB_ERROR).write(payload);
+ new MySQLErrPacket(new
SQLException(MySQLVendorError.ER_NO_DB_ERROR.getReason(),
+ MySQLVendorError.ER_NO_DB_ERROR.getSqlState().getValue(),
MySQLVendorError.ER_NO_DB_ERROR.getVendorCode())).write(payload);
verify(payload).writeInt1(MySQLErrPacket.HEADER);
verify(payload).writeInt2(1046);
verify(payload).writeStringFix("#");
diff --git
a/kernel/data-pipeline/dialect/mysql/src/test/java/org/apache/shardingsphere/data/pipeline/mysql/ingest/client/netty/MySQLNegotiateHandlerTest.java
b/kernel/data-pipeline/dialect/mysql/src/test/java/org/apache/shardingsphere/data/pipeline/mysql/ingest/client/netty/MySQLNegotiateHandlerTest.java
index 87a71a1d687..08da85668ea 100644
---
a/kernel/data-pipeline/dialect/mysql/src/test/java/org/apache/shardingsphere/data/pipeline/mysql/ingest/client/netty/MySQLNegotiateHandlerTest.java
+++
b/kernel/data-pipeline/dialect/mysql/src/test/java/org/apache/shardingsphere/data/pipeline/mysql/ingest/client/netty/MySQLNegotiateHandlerTest.java
@@ -40,10 +40,12 @@ import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
+import java.sql.SQLException;
+
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -101,7 +103,8 @@ class MySQLNegotiateHandlerTest {
@Test
void assertChannelReadErrorPacket() {
- MySQLErrPacket errorPacket = new
MySQLErrPacket(MySQLVendorError.ER_NO_DB_ERROR);
+ MySQLErrPacket errorPacket = new MySQLErrPacket(
+ new SQLException(MySQLVendorError.ER_NO_DB_ERROR.getReason(),
MySQLVendorError.ER_NO_DB_ERROR.getSqlState().getValue(),
MySQLVendorError.ER_NO_DB_ERROR.getVendorCode()));
assertThrows(RuntimeException.class, () ->
mysqlNegotiateHandler.channelRead(channelHandlerContext, errorPacket));
}
}
diff --git
a/proxy/frontend/type/mysql/src/main/java/org/apache/shardingsphere/proxy/frontend/mysql/err/MySQLErrorPacketFactory.java
b/proxy/frontend/type/mysql/src/main/java/org/apache/shardingsphere/proxy/frontend/mysql/err/MySQLErrorPacketFactory.java
index e6c0b1d2c21..d75fef84eeb 100644
---
a/proxy/frontend/type/mysql/src/main/java/org/apache/shardingsphere/proxy/frontend/mysql/err/MySQLErrorPacketFactory.java
+++
b/proxy/frontend/type/mysql/src/main/java/org/apache/shardingsphere/proxy/frontend/mysql/err/MySQLErrorPacketFactory.java
@@ -17,17 +17,13 @@
package org.apache.shardingsphere.proxy.frontend.mysql.err;
-import com.google.common.base.Strings;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
import
org.apache.shardingsphere.db.protocol.mysql.packet.generic.MySQLErrPacket;
-import
org.apache.shardingsphere.infra.exception.dialect.SQLExceptionTransformEngine;
-import org.apache.shardingsphere.infra.exception.mysql.vendor.MySQLVendorError;
import org.apache.shardingsphere.infra.database.core.type.DatabaseType;
+import
org.apache.shardingsphere.infra.exception.dialect.SQLExceptionTransformEngine;
import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
-import java.sql.SQLException;
-
/**
* Error packet factory for MySQL.
*/
@@ -41,15 +37,6 @@ public final class MySQLErrorPacketFactory {
* @return created instance
*/
public static MySQLErrPacket newInstance(final Exception cause) {
- SQLException sqlException =
SQLExceptionTransformEngine.toSQLException(cause,
TypedSPILoader.getService(DatabaseType.class, "MySQL"));
- return null == sqlException.getSQLState() ? new
MySQLErrPacket(MySQLVendorError.ER_INTERNAL_ERROR,
getErrorMessage(sqlException)) : createErrPacket(sqlException);
- }
-
- private static String getErrorMessage(final SQLException cause) {
- return null == cause.getNextException() ||
!Strings.isNullOrEmpty(cause.getMessage()) ? cause.getMessage() :
cause.getNextException().getMessage();
- }
-
- private static MySQLErrPacket createErrPacket(final SQLException cause) {
- return new MySQLErrPacket(cause.getErrorCode(), cause.getSQLState(),
cause.getMessage());
+ return new
MySQLErrPacket(SQLExceptionTransformEngine.toSQLException(cause,
TypedSPILoader.getService(DatabaseType.class, "MySQL")));
}
}
diff --git
a/proxy/frontend/type/mysql/src/test/java/org/apache/shardingsphere/proxy/frontend/mysql/err/MySQLErrorPacketFactoryTest.java
b/proxy/frontend/type/mysql/src/test/java/org/apache/shardingsphere/proxy/frontend/mysql/err/MySQLErrorPacketFactoryTest.java
index 90980da18b1..a47366380c7 100644
---
a/proxy/frontend/type/mysql/src/test/java/org/apache/shardingsphere/proxy/frontend/mysql/err/MySQLErrorPacketFactoryTest.java
+++
b/proxy/frontend/type/mysql/src/test/java/org/apache/shardingsphere/proxy/frontend/mysql/err/MySQLErrorPacketFactoryTest.java
@@ -18,53 +18,16 @@
package org.apache.shardingsphere.proxy.frontend.mysql.err;
import
org.apache.shardingsphere.db.protocol.mysql.packet.generic.MySQLErrPacket;
-import
org.apache.shardingsphere.infra.exception.dialect.exception.syntax.database.UnknownDatabaseException;
import
org.apache.shardingsphere.infra.exception.core.external.sql.sqlstate.XOpenSQLState;
-import
org.apache.shardingsphere.proxy.frontend.exception.CircuitBreakException;
import org.junit.jupiter.api.Test;
-import java.sql.SQLException;
-
import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
class MySQLErrorPacketFactoryTest {
@Test
- void assertNewInstanceWithSQLExceptionForNullSQLState() {
- MySQLErrPacket actual = MySQLErrorPacketFactory.newInstance(new
SQLException(""));
- assertThat(actual.getErrorCode(), is(1815));
- assertThat(actual.getSqlState(),
is(XOpenSQLState.GENERAL_ERROR.getValue()));
- assertThat(actual.getErrorMessage(), startsWith("Internal error"));
- }
-
- @Test
- void assertNewInstanceWithSQLException() {
- MySQLErrPacket actual = MySQLErrorPacketFactory.newInstance(new
SQLException("No reason", "XXX", 30000, new RuntimeException("")));
- assertThat(actual.getErrorCode(), is(30000));
- assertThat(actual.getSqlState(), is("XXX"));
- assertThat(actual.getErrorMessage(), is("No reason"));
- }
-
- @Test
- void assertNewInstanceWithShardingSphereSQLException() {
- MySQLErrPacket actual = MySQLErrorPacketFactory.newInstance(new
CircuitBreakException());
- assertThat(actual.getErrorCode(), is(13010));
- assertThat(actual.getSqlState(),
is(XOpenSQLState.GENERAL_WARNING.getValue()));
- assertThat(actual.getErrorMessage(), is("Circuit break open, the
request has been ignored."));
- }
-
- @Test
- void assertNewInstanceWithSQLDialectException() {
- MySQLErrPacket actual = MySQLErrorPacketFactory.newInstance(new
UnknownDatabaseException("foo_db"));
- assertThat(actual.getErrorCode(), is(1049));
- assertThat(actual.getSqlState(),
is(XOpenSQLState.SYNTAX_ERROR.getValue()));
- assertThat(actual.getErrorMessage(), is("Unknown database 'foo_db'"));
- }
-
- @Test
- void assertNewInstanceWithUnknownException() {
+ void assertNewInstance() {
MySQLErrPacket actual = MySQLErrorPacketFactory.newInstance(new
RuntimeException("No reason"));
assertThat(actual.getErrorCode(), is(30000));
assertThat(actual.getSqlState(),
is(XOpenSQLState.GENERAL_ERROR.getValue()));
