Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/ai-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> jsonMap = (Map<String, Object>) jsonObj;

if (expected == null) {
expected = jsonToClientMap(json);
}
assertEquals(jsonObj, expected);
Map<String, Object> expectedMap = (Map<String, Object>) expected;

assertEquals(jsonMap, expectedMap);
for (Map.Entry<String, Object> 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());
}
Expand Down
102 changes: 102 additions & 0 deletions jdbc-v2/src/test/java/com/clickhouse/jdbc/ReadonlyProfileTest.java
Original file line number Diff line number Diff line change
@@ -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");
}
Comment thread
cursor[bot] marked this conversation as resolved.
}

@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());
Comment thread
chernser marked this conversation as resolved.
}
}

@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\"}");
}
}
}
}
Loading