diff --git a/docs/ai-review.md b/docs/ai-review.md index 82b5350ce..717bddc10 100644 --- a/docs/ai-review.md +++ b/docs/ai-review.md @@ -137,6 +137,11 @@ Prefer the smallest safe optimization that preserves behavior. ## Tests and docs +**Note**: carefully evaluate tests that involve creating database, users and deal with `GRANT` operations. If user is created within test +it must have secure random password. This password should never be logged or accessible externally. User should be deleted in the `finally` block. +If `GRANT` is executed then it must be against test database only. Question if `GRANT ALL` should be used. Test can be run against cloud instance, +and it is crucial to review what is created in database. Flag any unusual things in queries. + Call out missing focused tests when a change affects: - public API behavior diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java index e2dd6a025..ed0b00cb9 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java @@ -2863,12 +2863,23 @@ public void testJSONRead(String json, Object expected) throws Exception { assertTrue(rs.next()); Object jsonObj = rs.getObject(1); + assertTrue(jsonObj instanceof Map, "JSON should be read as a Map"); + Map jsonMap = (Map) jsonObj; + if (expected == null) { expected = jsonToClientMap(json); } - assertEquals(jsonObj, expected); + Map expectedMap = (Map) expected; + + assertEquals(jsonMap, expectedMap); + for (Map.Entry entry : expectedMap.entrySet()) { + assertTrue(jsonMap.containsKey(entry.getKey()), "Map should contain key: " + entry.getKey()); + assertEquals(jsonMap.get(entry.getKey()), entry.getValue(), "Values should match for key: " + entry.getKey()); + } + assertTrue(rs.next()); Object emptyJsonObj = rs.getObject(1); + assertTrue(emptyJsonObj instanceof Map, "Empty JSON should be returned as Map"); assertEquals(emptyJsonObj, EMPTY_JSON); assertFalse(rs.next()); } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java new file mode 100644 index 000000000..2c34ed447 --- /dev/null +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java @@ -0,0 +1,102 @@ +package com.clickhouse.jdbc; + +import com.clickhouse.client.api.ClientConfigProperties; +import com.clickhouse.client.api.internal.ServerSettings; +import org.testng.Assert; +import org.testng.SkipException; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Properties; +import java.util.UUID; + +public class ReadonlyProfileTest extends JdbcIntegrationTest { + + private String password; + + @BeforeClass(groups = { "integration" }) + public void setup() throws SQLException { + if (isCloud()) { + return; + } + // Append fixed character classes so the random password satisfies server + // password-complexity policies (uppercase, lowercase, digit, special). + password = UUID.randomUUID().toString() + "Aa1!"; + com.clickhouse.client.ClickHouseServerForTest.beforeSuite(); + try (Connection conn = getJdbcConnection(); + Statement stmt = conn.createStatement()) { + stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS jdbc_test_profile_readonly_1 SETTINGS readonly=1"); + stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS jdbc_test_profile_readonly_2 SETTINGS readonly=2"); + stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_1"); + stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_2"); + stmt.execute("CREATE USER jdbc_test_user_readonly_1 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE jdbc_test_profile_readonly_1"); + stmt.execute("CREATE USER jdbc_test_user_readonly_2 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE jdbc_test_profile_readonly_2"); + String db = getDatabase(); + stmt.execute("GRANT SELECT ON " + db + ".* TO jdbc_test_user_readonly_1"); + stmt.execute("GRANT SELECT ON " + db + ".* TO jdbc_test_user_readonly_2"); + } + } + + @AfterClass(groups = { "integration" }) + public void teardown() throws SQLException { + if (isCloud()) { + return; + } + try (Connection conn = getJdbcConnection(); + Statement stmt = conn.createStatement()) { + stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_1"); + stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_2"); + stmt.execute("DROP SETTINGS PROFILE IF EXISTS jdbc_test_profile_readonly_1"); + stmt.execute("DROP SETTINGS PROFILE IF EXISTS jdbc_test_profile_readonly_2"); + } + } + + @Test(groups = { "integration" }) + public void testReadonly1CannotChangeSettings() throws Exception { + if (isCloud()) { + throw new SkipException("Should not be tested in cloud because creates users and profiles"); + } + Properties properties = new Properties(); + properties.setProperty("user", "jdbc_test_user_readonly_1"); + properties.setProperty("password", password); + properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1"); + + try (Connection conn = getJdbcConnection(properties)) { + try (Statement stmt = conn.createStatement(); + ResultSet rs = stmt.executeQuery("SELECT 1")) { + Assert.fail("Should have thrown an exception because readonly=1 prevents changing settings"); + } + } catch (SQLException e) { + Assert.assertTrue(e.getMessage().contains("Cannot modify"), "Exception message should indicate setting cannot be modified: " + e.getMessage()); + } + } + + @Test(groups = { "integration" }) + public void testReadonly2CanChangeSettings() throws Exception { + if (isCloud()) { + throw new SkipException("Should not be tested in cloud because creates users and profiles"); + } + if (isVersionMatch("(,24.8]")) { + throw new SkipException("Old versions do not support JSON"); + } + + Properties properties = new Properties(); + properties.setProperty("user", "jdbc_test_user_readonly_2"); + properties.setProperty("password", password); + properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1"); + properties.put(ClientConfigProperties.serverSetting("allow_experimental_json_type"), "1"); + + try (Connection conn = getJdbcConnection(properties)) { + try (Statement stmt = conn.createStatement(); + ResultSet rs = stmt.executeQuery("SELECT '{\"key\":\"value\"}'::JSON")) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getString(1), "{\"key\":\"value\"}"); + } + } + } +}