Skip to content

Commit 51c7955

Browse files
authored
Merge pull request #2883 from ClickHouse/06/17/26/read_only_profile_tests
[JDBC-v2] Readonly Profile tests
2 parents b444455 + c170d3d commit 51c7955

3 files changed

Lines changed: 119 additions & 1 deletion

File tree

docs/ai-review.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ Prefer the smallest safe optimization that preserves behavior.
137137

138138
## Tests and docs
139139

140+
**Note**: carefully evaluate tests that involve creating database, users and deal with `GRANT` operations. If user is created within test
141+
it must have secure random password. This password should never be logged or accessible externally. User should be deleted in the `finally` block.
142+
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,
143+
and it is crucial to review what is created in database. Flag any unusual things in queries.
144+
140145
Call out missing focused tests when a change affects:
141146

142147
- public API behavior

jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2863,12 +2863,23 @@ public void testJSONRead(String json, Object expected) throws Exception {
28632863

28642864
assertTrue(rs.next());
28652865
Object jsonObj = rs.getObject(1);
2866+
assertTrue(jsonObj instanceof Map, "JSON should be read as a Map");
2867+
Map<String, Object> jsonMap = (Map<String, Object>) jsonObj;
2868+
28662869
if (expected == null) {
28672870
expected = jsonToClientMap(json);
28682871
}
2869-
assertEquals(jsonObj, expected);
2872+
Map<String, Object> expectedMap = (Map<String, Object>) expected;
2873+
2874+
assertEquals(jsonMap, expectedMap);
2875+
for (Map.Entry<String, Object> entry : expectedMap.entrySet()) {
2876+
assertTrue(jsonMap.containsKey(entry.getKey()), "Map should contain key: " + entry.getKey());
2877+
assertEquals(jsonMap.get(entry.getKey()), entry.getValue(), "Values should match for key: " + entry.getKey());
2878+
}
2879+
28702880
assertTrue(rs.next());
28712881
Object emptyJsonObj = rs.getObject(1);
2882+
assertTrue(emptyJsonObj instanceof Map, "Empty JSON should be returned as Map");
28722883
assertEquals(emptyJsonObj, EMPTY_JSON);
28732884
assertFalse(rs.next());
28742885
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package com.clickhouse.jdbc;
2+
3+
import com.clickhouse.client.api.ClientConfigProperties;
4+
import com.clickhouse.client.api.internal.ServerSettings;
5+
import org.testng.Assert;
6+
import org.testng.SkipException;
7+
import org.testng.annotations.AfterClass;
8+
import org.testng.annotations.BeforeClass;
9+
import org.testng.annotations.Test;
10+
11+
import java.sql.Connection;
12+
import java.sql.ResultSet;
13+
import java.sql.SQLException;
14+
import java.sql.Statement;
15+
import java.util.Properties;
16+
import java.util.UUID;
17+
18+
public class ReadonlyProfileTest extends JdbcIntegrationTest {
19+
20+
private String password;
21+
22+
@BeforeClass(groups = { "integration" })
23+
public void setup() throws SQLException {
24+
if (isCloud()) {
25+
return;
26+
}
27+
// Append fixed character classes so the random password satisfies server
28+
// password-complexity policies (uppercase, lowercase, digit, special).
29+
password = UUID.randomUUID().toString() + "Aa1!";
30+
com.clickhouse.client.ClickHouseServerForTest.beforeSuite();
31+
try (Connection conn = getJdbcConnection();
32+
Statement stmt = conn.createStatement()) {
33+
stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS jdbc_test_profile_readonly_1 SETTINGS readonly=1");
34+
stmt.execute("CREATE SETTINGS PROFILE IF NOT EXISTS jdbc_test_profile_readonly_2 SETTINGS readonly=2");
35+
stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_1");
36+
stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_2");
37+
stmt.execute("CREATE USER jdbc_test_user_readonly_1 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE jdbc_test_profile_readonly_1");
38+
stmt.execute("CREATE USER jdbc_test_user_readonly_2 IDENTIFIED WITH plaintext_password BY '" + password + "' SETTINGS PROFILE jdbc_test_profile_readonly_2");
39+
String db = getDatabase();
40+
stmt.execute("GRANT SELECT ON " + db + ".* TO jdbc_test_user_readonly_1");
41+
stmt.execute("GRANT SELECT ON " + db + ".* TO jdbc_test_user_readonly_2");
42+
}
43+
}
44+
45+
@AfterClass(groups = { "integration" })
46+
public void teardown() throws SQLException {
47+
if (isCloud()) {
48+
return;
49+
}
50+
try (Connection conn = getJdbcConnection();
51+
Statement stmt = conn.createStatement()) {
52+
stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_1");
53+
stmt.execute("DROP USER IF EXISTS jdbc_test_user_readonly_2");
54+
stmt.execute("DROP SETTINGS PROFILE IF EXISTS jdbc_test_profile_readonly_1");
55+
stmt.execute("DROP SETTINGS PROFILE IF EXISTS jdbc_test_profile_readonly_2");
56+
}
57+
}
58+
59+
@Test(groups = { "integration" })
60+
public void testReadonly1CannotChangeSettings() throws Exception {
61+
if (isCloud()) {
62+
throw new SkipException("Should not be tested in cloud because creates users and profiles");
63+
}
64+
Properties properties = new Properties();
65+
properties.setProperty("user", "jdbc_test_user_readonly_1");
66+
properties.setProperty("password", password);
67+
properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1");
68+
69+
try (Connection conn = getJdbcConnection(properties)) {
70+
try (Statement stmt = conn.createStatement();
71+
ResultSet rs = stmt.executeQuery("SELECT 1")) {
72+
Assert.fail("Should have thrown an exception because readonly=1 prevents changing settings");
73+
}
74+
} catch (SQLException e) {
75+
Assert.assertTrue(e.getMessage().contains("Cannot modify"), "Exception message should indicate setting cannot be modified: " + e.getMessage());
76+
}
77+
}
78+
79+
@Test(groups = { "integration" })
80+
public void testReadonly2CanChangeSettings() throws Exception {
81+
if (isCloud()) {
82+
throw new SkipException("Should not be tested in cloud because creates users and profiles");
83+
}
84+
if (isVersionMatch("(,24.8]")) {
85+
throw new SkipException("Old versions do not support JSON");
86+
}
87+
88+
Properties properties = new Properties();
89+
properties.setProperty("user", "jdbc_test_user_readonly_2");
90+
properties.setProperty("password", password);
91+
properties.put(ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), "1");
92+
properties.put(ClientConfigProperties.serverSetting("allow_experimental_json_type"), "1");
93+
94+
try (Connection conn = getJdbcConnection(properties)) {
95+
try (Statement stmt = conn.createStatement();
96+
ResultSet rs = stmt.executeQuery("SELECT '{\"key\":\"value\"}'::JSON")) {
97+
Assert.assertTrue(rs.next());
98+
Assert.assertEquals(rs.getString(1), "{\"key\":\"value\"}");
99+
}
100+
}
101+
}
102+
}

0 commit comments

Comments
 (0)