Skip to content

Commit 5dce9ca

Browse files
committed
fixing database id restriction
Signed-off-by: affonsov <67347924+affonsov@users.noreply.github.com>
1 parent 5e61323 commit 5dce9ca

File tree

6 files changed

+72
-31
lines changed

6 files changed

+72
-31
lines changed

java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,15 @@ public abstract class BaseClientConfiguration {
8787

8888
/**
8989
* Index of the logical database to connect to. Must be non-negative and within the range
90-
* supported by the server. If not specified, defaults to database 0.
90+
* supported by the server configuration. If not specified, defaults to database 0.
9191
*
92-
* <p>For cluster mode, this feature requires Valkey 9.0+ with multi-database cluster support
93-
* enabled (cluster-databases configuration > 1).
92+
* <p>The valid range is determined by the server's database configuration. For standalone mode,
93+
* this is typically 0-15 by default but can be configured via the 'databases' setting. For
94+
* cluster mode, this feature requires Valkey 9.0+ with multi-database cluster support enabled
95+
* (cluster-databases configuration > 1).
96+
*
97+
* <p>Client-side validation only checks for non-negative values. Range validation is performed
98+
* server-side and will result in a connection error if the specified database ID is out of range.
9499
*/
95100
private final Integer databaseId;
96101

java/client/src/main/java/glide/managers/ConnectionManager.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,6 @@ private ConnectionRequest.Builder setupConnectionRequestBuilderBaseConfiguration
167167
throw new ConfigurationError(
168168
"databaseId must be non-negative, got: " + configuration.getDatabaseId());
169169
}
170-
if (configuration.getDatabaseId() > 15) {
171-
throw new ConfigurationError(
172-
"databaseId must be within reasonable range (0-15), got: "
173-
+ configuration.getDatabaseId());
174-
}
175170
connectionRequestBuilder.setDatabaseId(configuration.getDatabaseId());
176171
}
177172

java/client/src/main/java/redis/clients/jedis/ClusterConfigurationMapper.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,6 @@ public static GlideClusterClientConfiguration createDefaultClusterConfig(
102102
*/
103103
public static void validateClusterConfiguration(JedisClientConfig jedisConfig) {
104104
// Check for unsupported cluster features
105-
if (jedisConfig.getDatabase() != 0) {
106-
logger.warning("Database selection is not supported in cluster mode");
107-
}
108-
109105
if (jedisConfig.getBlockingSocketTimeoutMillis() > 0) {
110106
logger.warning(
111107
"Blocking socket timeout configuration may not be fully supported in cluster mode");

java/client/src/test/java/glide/api/models/configuration/BaseClientConfigurationTest.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,10 @@ public void testDatabaseIdValidValues() {
6161
}
6262

6363
@ParameterizedTest
64-
@ValueSource(ints = {0, 1, 5, 10, 15})
64+
@ValueSource(ints = {0, 1, 5, 10, 15, 50, 100, 1000})
6565
public void testDatabaseIdValidRange(int databaseId) {
66-
// Test that valid database IDs are accepted
66+
// Test that non-negative database IDs are accepted (server-side validation will handle range
67+
// checks)
6768
TestClientConfiguration config =
6869
TestClientConfiguration.builder().databaseId(databaseId).build();
6970
assertEquals(databaseId, config.getDatabaseId());
@@ -78,11 +79,20 @@ public void testDatabaseIdNegativeValue() {
7879

7980
@Test
8081
public void testDatabaseIdLargeValue() {
81-
// Test that large database IDs are handled (validation should be done at connection time)
82+
// Test that large database IDs are handled (server-side validation will handle range checks)
8283
TestClientConfiguration config = TestClientConfiguration.builder().databaseId(100).build();
8384
assertEquals(100, config.getDatabaseId());
8485
}
8586

87+
@ParameterizedTest
88+
@ValueSource(ints = {999, 10000, 50000})
89+
public void testDatabaseIdVeryLargeValues(int databaseId) {
90+
// Test that very large database IDs are accepted by client (server will validate range)
91+
TestClientConfiguration config =
92+
TestClientConfiguration.builder().databaseId(databaseId).build();
93+
assertEquals(databaseId, config.getDatabaseId());
94+
}
95+
8696
@Test
8797
public void testDatabaseIdWithOtherConfiguration() {
8898
// Test that databaseId works with other configuration options

java/integTest/src/test/java/glide/MultiDatabaseTests.java

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ private static Stream<Arguments> getDatabaseIds() {
4444
Arguments.of(0), Arguments.of(1), Arguments.of(2), Arguments.of(5), Arguments.of(10));
4545
}
4646

47+
/** Helper method to provide larger database IDs for server validation testing */
48+
private static Stream<Arguments> getLargeDatabaseIds() {
49+
return Stream.of(Arguments.of(50), Arguments.of(100), Arguments.of(999));
50+
}
51+
4752
@ParameterizedTest
4853
@MethodSource("getClients")
4954
@SneakyThrows
@@ -216,21 +221,26 @@ public void testInvalidNegativeDatabaseId(int invalidDatabaseId) {
216221
}
217222

218223
@ParameterizedTest
219-
@ValueSource(ints = {16, 20, 100})
224+
@ValueSource(ints = {50, 100, 999})
220225
@SneakyThrows
221-
public void testInvalidLargeDatabaseId(int invalidDatabaseId) {
222-
// Test that database IDs beyond reasonable range result in configuration errors
223-
ConfigurationError exception =
224-
assertThrows(
225-
ConfigurationError.class,
226-
() -> {
227-
GlideClient.createClient(commonClientConfig().databaseId(invalidDatabaseId).build())
228-
.get();
229-
});
230-
231-
// Should contain information about the reasonable range
232-
assertTrue(exception.getMessage().toLowerCase().contains("reasonable range"));
233-
assertTrue(exception.getMessage().contains(String.valueOf(invalidDatabaseId)));
226+
public void testLargeDatabaseIdServerValidation(int databaseId) {
227+
// Test that large database IDs are handled by server-side validation
228+
// The client should accept these values, but the server may reject them based on its
229+
// configuration
230+
try {
231+
GlideClient client =
232+
GlideClient.createClient(commonClientConfig().databaseId(databaseId).build()).get();
233+
// If we reach here, the server supports this database ID
234+
client.close();
235+
} catch (ExecutionException e) {
236+
// Server-side validation should provide appropriate error messages
237+
// This is expected behavior for out-of-range database IDs
238+
assertTrue(
239+
e.getMessage().toLowerCase().contains("database")
240+
|| e.getMessage().toLowerCase().contains("db")
241+
|| e.getMessage().toLowerCase().contains("select")
242+
|| e.getMessage().toLowerCase().contains("invalid"));
243+
}
234244
}
235245

236246
@Test
@@ -256,4 +266,30 @@ public void testClusterClientWithNonZeroDatabaseOnOlderServer() {
256266
|| exception.getMessage().toLowerCase().contains("cluster")
257267
|| exception.getMessage().toLowerCase().contains("database"));
258268
}
269+
270+
@ParameterizedTest
271+
@MethodSource("getLargeDatabaseIds")
272+
@EnabledIf("isServerVersionAtLeast9_0")
273+
@SneakyThrows
274+
public void testClusterClientLargeDatabaseIdServerValidation(int databaseId) {
275+
// Test that large database IDs in cluster mode are handled by server-side validation
276+
assumeTrue(isServerVersionAtLeast9_0(), "Multi-DB cluster mode requires Valkey 9.0+");
277+
278+
try {
279+
GlideClusterClient client =
280+
GlideClusterClient.createClient(
281+
commonClusterClientConfig().databaseId(databaseId).build())
282+
.get();
283+
// If we reach here, the server supports this database ID
284+
client.close();
285+
} catch (ExecutionException e) {
286+
// Server-side validation should provide appropriate error messages
287+
// This is expected behavior for out-of-range database IDs
288+
assertTrue(
289+
e.getMessage().toLowerCase().contains("database")
290+
|| e.getMessage().toLowerCase().contains("db")
291+
|| e.getMessage().toLowerCase().contains("select")
292+
|| e.getMessage().toLowerCase().contains("invalid"));
293+
}
294+
}
259295
}

java/integTest/src/test/java/glide/SharedCommandTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import static glide.TestUtilities.commonClusterClientConfig;
88
import static glide.api.BaseClient.OK;
99
import static glide.api.models.GlideString.gs;
10-
import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_NODES;
1110
import static glide.api.models.commands.LInsertOptions.InsertPosition.AFTER;
1211
import static glide.api.models.commands.LInsertOptions.InsertPosition.BEFORE;
1312
import static glide.api.models.commands.RangeOptions.InfScoreBound.NEGATIVE_INFINITY;
@@ -19,6 +18,7 @@
1918
import static glide.api.models.commands.SetOptions.Expiry.Milliseconds;
2019
import static glide.api.models.commands.SortBaseOptions.OrderBy.ASC;
2120
import static glide.api.models.commands.SortBaseOptions.OrderBy.DESC;
21+
import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_NODES;
2222
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
2323
import static org.junit.jupiter.api.Assertions.assertEquals;
2424
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -110,7 +110,6 @@
110110
import glide.api.models.commands.stream.StreamReadOptions;
111111
import glide.api.models.commands.stream.StreamTrimOptions.MaxLen;
112112
import glide.api.models.commands.stream.StreamTrimOptions.MinId;
113-
import glide.api.models.configuration.GlideClientConfiguration;
114113
import glide.api.models.configuration.ProtocolVersion;
115114
import glide.api.models.exceptions.RequestException;
116115
import glide.cluster.ValkeyCluster;

0 commit comments

Comments
 (0)