diff --git a/fe/fe-core/src/main/java/com/starrocks/meta/BlackListSql.java b/fe/fe-core/src/main/java/com/starrocks/meta/BlackListSql.java index d7575731e88072..7d4a3d709003fd 100644 --- a/fe/fe-core/src/main/java/com/starrocks/meta/BlackListSql.java +++ b/fe/fe-core/src/main/java/com/starrocks/meta/BlackListSql.java @@ -14,7 +14,6 @@ package com.starrocks.meta; -import java.util.Objects; import java.util.regex.Pattern; public class BlackListSql { @@ -25,27 +24,4 @@ public BlackListSql(Pattern pattern, long id) { public Pattern pattern; public long id; - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - BlackListSql that = (BlackListSql) o; - if (id != that.id) { - return false; - } - if (this.pattern == null) { - return that.pattern == null; - } - return Objects.equals(pattern.pattern(), that.pattern.pattern()); - } - - @Override - public int hashCode() { - return Objects.hash(pattern, id); - } } diff --git a/fe/fe-core/src/main/java/com/starrocks/persist/DeleteSqlBlackLists.java b/fe/fe-core/src/main/java/com/starrocks/persist/DeleteSqlBlackLists.java index d8bf0d1fbaf361..b30c4fe554ec4f 100644 --- a/fe/fe-core/src/main/java/com/starrocks/persist/DeleteSqlBlackLists.java +++ b/fe/fe-core/src/main/java/com/starrocks/persist/DeleteSqlBlackLists.java @@ -18,7 +18,6 @@ import com.starrocks.common.io.JsonWriter; import java.util.List; -import java.util.Objects; public class DeleteSqlBlackLists extends JsonWriter { public DeleteSqlBlackLists(List ids) { @@ -27,21 +26,4 @@ public DeleteSqlBlackLists(List ids) { @SerializedName("ids") public final List ids; - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - DeleteSqlBlackLists that = (DeleteSqlBlackLists) o; - return Objects.equals(ids, that.ids); - } - - @Override - public int hashCode() { - return Objects.hashCode(ids); - } } diff --git a/fe/fe-core/src/main/java/com/starrocks/persist/SqlBlackListPersistInfo.java b/fe/fe-core/src/main/java/com/starrocks/persist/SqlBlackListPersistInfo.java index 4a32fe72515746..7bb7c0b783dda7 100644 --- a/fe/fe-core/src/main/java/com/starrocks/persist/SqlBlackListPersistInfo.java +++ b/fe/fe-core/src/main/java/com/starrocks/persist/SqlBlackListPersistInfo.java @@ -17,8 +17,6 @@ import com.google.gson.annotations.SerializedName; import com.starrocks.common.io.JsonWriter; -import java.util.Objects; - public class SqlBlackListPersistInfo extends JsonWriter { public SqlBlackListPersistInfo(long id, String pattern) { this.id = id; @@ -30,21 +28,4 @@ public SqlBlackListPersistInfo(long id, String pattern) { @SerializedName("pattern") public final String pattern; - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - SqlBlackListPersistInfo that = (SqlBlackListPersistInfo) o; - return id == that.id && Objects.equals(pattern, that.pattern); - } - - @Override - public int hashCode() { - return Objects.hash(id, pattern); - } } diff --git a/fe/fe-core/src/test/java/com/starrocks/backup/RestoreJobTest.java b/fe/fe-core/src/test/java/com/starrocks/backup/RestoreJobTest.java index daf600a4ab8a0c..02302cc293378b 100644 --- a/fe/fe-core/src/test/java/com/starrocks/backup/RestoreJobTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/backup/RestoreJobTest.java @@ -557,7 +557,7 @@ public void testRunBackupListTable() { minTimes = 0; result = id.incrementAndGet(); - GlobalStateMgr.getCurrentState().getNodeMgr().getClusterInfo(); + globalStateMgr.getNodeMgr().getClusterInfo(); minTimes = 0; result = systemInfoService; } diff --git a/fe/fe-core/src/test/java/com/starrocks/server/SqlBlacklistTest.java b/fe/fe-core/src/test/java/com/starrocks/server/SqlBlacklistTest.java index 245080d589e1d3..12244274f7d3dc 100644 --- a/fe/fe-core/src/test/java/com/starrocks/server/SqlBlacklistTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/server/SqlBlacklistTest.java @@ -30,14 +30,13 @@ import com.starrocks.sql.ast.DelSqlBlackListStmt; import com.starrocks.sql.ast.ShowSqlBlackListStmt; import com.starrocks.utframe.UtFrameUtils; -import mockit.Expectations; import mockit.Mock; import mockit.MockUp; import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; -import org.junit.jupiter.api.Assertions; +import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import java.util.List; @@ -70,12 +69,8 @@ public void beforeEach() { public void testAddSQLBlacklist() throws Exception { mockupGlobalState(); - new Expectations() { - { - editLog.logAddSQLBlackList(new SqlBlackListPersistInfo(0, ".+")); - times = 1; - } - }; + ArgumentCaptor addBlacklistEditLogArgument = ArgumentCaptor + .forClass(SqlBlackListPersistInfo.class); AddSqlBlackListStmt addStatement = (AddSqlBlackListStmt) parseSql("ADD SQLBLACKLIST \".+\";"); Assert.assertEquals(addStatement.getSql(), ".+"); @@ -86,6 +81,11 @@ public void testAddSQLBlacklist() throws Exception { Assert.assertEquals(1, blackLists.size()); Assert.assertEquals(0, blackLists.get(0).id); Assert.assertEquals(".+", blackLists.get(0).pattern.pattern()); + + Mockito.verify(editLog).logAddSQLBlackList(addBlacklistEditLogArgument.capture()); + + Assert.assertEquals(0, addBlacklistEditLogArgument.getValue().id); + Assert.assertEquals(".+", addBlacklistEditLogArgument.getValue().pattern); } @Test @@ -106,23 +106,32 @@ public void testShowBlacklist() { Assert.assertFalse(resultSet.next()); } + @Test + public void testBlackListReturnsSameIdIfPatternAlreadyExists() { + mockupGlobalState(); + Pattern p = Pattern.compile("qwert"); + long id = sqlBlackList.put(p); + + Assert.assertEquals(id, sqlBlackList.put(p)); + } + @Test public void testDeleteSqlBlacklist() throws Exception { mockupGlobalState(); long id1 = sqlBlackList.put(Pattern.compile("qwert")); long id2 = sqlBlackList.put(Pattern.compile("abcde")); - new Expectations() { - { - editLog.logDeleteSQLBlackList(new DeleteSqlBlackLists(List.of(id1, id2))); - times = 1; - } - }; + ArgumentCaptor deleteBlacklistsEditLogArgument = + ArgumentCaptor.forClass(DeleteSqlBlackLists.class); StmtExecutor deleteStatementExecutor = new StmtExecutor(connectContext, new DelSqlBlackListStmt(List.of(id1, id2))); deleteStatementExecutor.execute(); Assert.assertTrue(sqlBlackList .getBlackLists().stream().noneMatch(x -> x.id == id1 || x.id != id2)); + + Mockito.verify(editLog).logDeleteSQLBlackList(deleteBlacklistsEditLogArgument.capture()); + + Assert.assertEquals(List.of(id1, id2), deleteBlacklistsEditLogArgument.getValue().ids); } @Test @@ -149,7 +158,17 @@ public void testSaveLoadBlackListImage() throws Exception { SqlBlackList recoveredBlackList = new SqlBlackList(); recoveredBlackList.load(testImage.getMetaBlockReader()); - Assertions.assertIterableEquals(originalBlacklist.getBlackLists(), recoveredBlackList.getBlackLists()); + Assert.assertEquals(originalBlacklist.getBlackLists().size(), recoveredBlackList.getBlackLists().size()); + Assert.assertEquals(originalBlacklist.getBlackLists().get(0).id, recoveredBlackList.getBlackLists().get(0).id); + Assert.assertEquals( + originalBlacklist.getBlackLists().get(0).pattern.pattern(), + recoveredBlackList.getBlackLists().get(0).pattern.pattern() + ); + Assert.assertEquals(originalBlacklist.getBlackLists().get(1).id, recoveredBlackList.getBlackLists().get(1).id); + Assert.assertEquals( + originalBlacklist.getBlackLists().get(1).pattern.pattern(), + recoveredBlackList.getBlackLists().get(1).pattern.pattern() + ); } @Test @@ -164,11 +183,12 @@ public void testSqlBlacklistJournalOperations() throws Exception { GlobalStateMgr.getCurrentState().getEditLog().logAddSQLBlackList(new SqlBlackListPersistInfo(1234, "p2")); UtFrameUtils.PseudoJournalReplayer.replayJournalToEnd(); - Assertions.assertIterableEquals( - List.of(new BlackListSql(Pattern.compile("p1"), 123L), - new BlackListSql(Pattern.compile("p2"), 1234L)), - GlobalStateMgr.getCurrentState().getSqlBlackList().getBlackLists() - ); + List resultBlackLists = GlobalStateMgr.getCurrentState().getSqlBlackList().getBlackLists(); + Assert.assertEquals(2, resultBlackLists.size()); + Assert.assertEquals(123L, resultBlackLists.get(0).id); + Assert.assertEquals("p1", resultBlackLists.get(0).pattern.pattern()); + Assert.assertEquals(1234L, resultBlackLists.get(1).id); + Assert.assertEquals("p2", resultBlackLists.get(1).pattern.pattern()); // delete blacklists @@ -176,7 +196,7 @@ public void testSqlBlacklistJournalOperations() throws Exception { UtFrameUtils.PseudoJournalReplayer.replayJournalToEnd(); Assert.assertTrue( - GlobalStateMgr.getCurrentState().getSqlBlackList().getBlackLists().stream() + sqlBlackList.getBlackLists().stream() .noneMatch(x -> x.id == 123L || x.id == 1234L) );