Skip to content

Commit c07309c

Browse files
Address PR feedback
1 parent 3300bc3 commit c07309c

File tree

3 files changed

+32
-73
lines changed

3 files changed

+32
-73
lines changed

src/main/java/redis/clients/jedis/ClientSetInfoConfig.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
* <p>
88
* This class supports two modes of operation:
99
* <ul>
10-
* <li>Legacy mode: Using {@link #withLibNameSuffix(String)} for simple suffix customization</li>
11-
* <li>Driver info mode: Using {@link #ClientSetInfoConfig(DriverInfo)} for structured driver
12-
* information with upstream drivers</li>
10+
* <li>Advanced mode: Using {@link #withLibNameSuffix(String)} for advanced suffix customization,
11+
* where the provided string is already preformatted according to the rules of the `CLIENT SETINFO`
12+
* command</li>
13+
* <li>Simple mode: Using {@link #ClientSetInfoConfig(DriverInfo)} used when the command parameter
14+
* will be built by the driver based on the {@link DriverInfo} provided</li>
1315
* </ul>
1416
* <p>
1517
* For backward compatibility, {@link #getUpstreamDrivers()} returns the upstream drivers string
@@ -76,15 +78,13 @@ public ClientSetInfoConfig(DriverInfo driverInfo) {
7678
}
7779

7880
/**
79-
* Returns whether CLIENT SETINFO is disabled.
8081
* @return {@code true} if CLIENT SETINFO is disabled, {@code false} otherwise
8182
*/
8283
public boolean isDisabled() {
8384
return disabled;
8485
}
8586

8687
/**
87-
* Returns the driver information.
8888
* @return the driver information
8989
*/
9090
public DriverInfo getDriverInfo() {

src/main/java/redis/clients/jedis/DriverInfo.java

Lines changed: 20 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
/*
2+
* Copyright 2025-Present, Redis Ltd. and Contributors All rights reserved. Licensed under the MIT
3+
* License.
4+
*/
15
package redis.clients.jedis;
26

3-
import java.io.Serializable;
47
import java.util.ArrayList;
58
import java.util.Arrays;
69
import java.util.Collections;
@@ -20,19 +23,7 @@
2023
* @see ClientSetInfoConfig
2124
* @see <a href="https://redis.io/docs/latest/commands/client-setinfo/">CLIENT SETINFO</a>
2225
*/
23-
public final class DriverInfo implements Serializable {
24-
25-
private static final long serialVersionUID = 1L;
26-
27-
/**
28-
* Regex pattern for driver name validation. The name must start with a lowercase letter and
29-
* contain only lowercase letters, digits, hyphens, and underscores. Dots are only allowed after
30-
* digits (for Scala cross-version naming like akka-redis_2.13). Follows Maven artifactId naming
31-
* conventions but also allows underscores and version-specific dots.
32-
* @see <a href="https://maven.apache.org/guides/mini/guide-naming-conventions.html">Maven Naming
33-
* Conventions</a>
34-
*/
35-
private static final String DRIVER_NAME_PATTERN = "^[a-z][a-z0-9_-]*(?:[0-9]\\.[0-9]+)?$";
26+
public final class DriverInfo {
3627

3728
/**
3829
* Set of brace characters that are not allowed in driver names or versions. These characters are
@@ -120,7 +111,7 @@ public String getName() {
120111
*/
121112
public String getUpstreamDrivers() {
122113
if (upstreamDrivers.isEmpty()) {
123-
return null;
114+
return "";
124115
}
125116
return String.join(";", upstreamDrivers);
126117
}
@@ -196,8 +187,8 @@ public Builder addUpstreamDriver(String driverName, String driverVersion) {
196187
if (driverVersion == null) {
197188
throw new JedisValidationException("Driver version must not be null");
198189
}
199-
validateDriverName(driverName);
200-
validateDriverVersion(driverVersion);
190+
validateDriverField(driverName, "Driver name");
191+
validateDriverField(driverVersion, "Driver version");
201192
String formattedDriverInfo = formatDriverInfo(driverName, driverVersion);
202193
this.upstreamDrivers.add(0, formattedDriverInfo);
203194
return this;
@@ -207,7 +198,7 @@ public Builder addUpstreamDriver(String driverName) {
207198
if (driverName == null) {
208199
throw new JedisValidationException("Driver name must not be null");
209200
}
210-
validateDriverName(driverName);
201+
validateDriverField(driverName, "Driver name");
211202
this.upstreamDrivers.add(0, driverName);
212203
return this;
213204
}
@@ -222,48 +213,22 @@ public DriverInfo build() {
222213
}
223214

224215
/**
225-
* Validates that the driver name follows Maven artifactId naming conventions: lowercase letters,
226-
* digits, hyphens, and underscores only, starting with a lowercase letter. Dots are only allowed
227-
* after digits (for Scala cross-version naming like akka-redis_2.13).
216+
* Validates that the value does not contain characters that would violate the format of the Redis
217+
* CLIENT LIST reply.
228218
* <p>
229-
* Additionally validates Redis CLIENT LIST constraints: no spaces, newlines, non-printable
230-
* characters, or braces.
231-
* @param driverName the driver name to validate
232-
* @throws JedisValidationException if the driver name does not follow the expected naming
233-
* conventions
234-
* @see <a href="https://maven.apache.org/guides/mini/guide-naming-conventions.html">Maven Naming
235-
* Conventions</a>
236-
* @see <a href="https://redis.io/docs/latest/commands/client-setinfo/">CLIENT SETINFO</a>
237-
*/
238-
private static void validateDriverName(String driverName) {
239-
if (driverName.trim().isEmpty()) {
240-
throw new JedisValidationException("Driver name must not be empty");
241-
}
242-
243-
validateNoInvalidCharacters(driverName, "Driver name");
244-
245-
if (!driverName.matches(DRIVER_NAME_PATTERN)) {
246-
throw new JedisValidationException(
247-
"Upstream driver name must follow Maven artifactId naming conventions: "
248-
+ "lowercase letters, digits, hyphens, and underscores only, starting with a lowercase letter "
249-
+ "(e.g., 'spring-data-redis', 'lettuce-core', 'akka-redis_2.13')");
250-
}
251-
}
252-
253-
/**
254-
* Validates that the driver version does not contain characters that would violate the format of
255-
* the Redis CLIENT LIST reply: no spaces, newlines, non-printable characters, or brace
256-
* characters.
257-
* @param driverVersion the driver version to validate
258-
* @throws JedisValidationException if the driver version contains invalid characters
219+
* Only printable ASCII characters (0x21-0x7E, i.e., '!' to '~') are allowed, excluding braces.
220+
* @param value the value to validate
221+
* @param fieldName the name of the field for error messages (e.g., "Driver name", "Driver
222+
* version")
223+
* @throws JedisValidationException if the value is empty or contains invalid characters
259224
* @see <a href="https://redis.io/docs/latest/commands/client-setinfo/">CLIENT SETINFO</a>
260225
*/
261-
private static void validateDriverVersion(String driverVersion) {
262-
if (driverVersion.trim().isEmpty()) {
263-
throw new JedisValidationException("Driver version must not be empty");
226+
private static void validateDriverField(String value, String fieldName) {
227+
if (value.trim().isEmpty()) {
228+
throw new JedisValidationException(fieldName + " must not be empty");
264229
}
265230

266-
validateNoInvalidCharacters(driverVersion, "Driver version");
231+
validateNoInvalidCharacters(value, fieldName);
267232
}
268233

269234
/**

src/test/java/redis/clients/jedis/misc/ClientSetInfoConfigTest.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import static org.junit.jupiter.api.Assertions.assertEquals;
1111
import static org.junit.jupiter.api.Assertions.assertFalse;
1212
import static org.junit.jupiter.api.Assertions.assertNotNull;
13-
import static org.junit.jupiter.api.Assertions.assertNull;
1413
import static org.junit.jupiter.api.Assertions.assertThrows;
1514
import static org.junit.jupiter.api.Assertions.assertTrue;
1615

@@ -20,7 +19,7 @@ public class ClientSetInfoConfigTest {
2019
public void defaultConfig() {
2120
ClientSetInfoConfig config = ClientSetInfoConfig.DEFAULT;
2221
assertFalse(config.isDisabled());
23-
assertNull(config.getUpstreamDrivers());
22+
assertEquals("", config.getUpstreamDrivers());
2423
assertNotNull(config.getDriverInfo());
2524
assertEquals("jedis", config.getDriverInfo().getName());
2625
assertEquals("jedis", config.getDriverInfo().getFormattedName());
@@ -30,7 +29,7 @@ public void defaultConfig() {
3029
public void disabledConfig() {
3130
ClientSetInfoConfig config = ClientSetInfoConfig.DISABLED;
3231
assertTrue(config.isDisabled());
33-
assertNull(config.getUpstreamDrivers());
32+
assertEquals("", config.getUpstreamDrivers());
3433
}
3534

3635
@Test
@@ -109,7 +108,7 @@ public void builderCustomName() {
109108
DriverInfo driverInfo = DriverInfo.builder().name("my-custom-client").build();
110109
assertEquals("my-custom-client", driverInfo.getName());
111110
assertEquals("my-custom-client", driverInfo.getFormattedName());
112-
assertNull(driverInfo.getUpstreamDrivers());
111+
assertEquals("", driverInfo.getUpstreamDrivers());
113112
}
114113

115114
@Test
@@ -202,16 +201,11 @@ public void driverNameValidation() {
202201
DriverInfo.builder().addUpstreamDriver("redis-client", "1.0.0");
203202
DriverInfo.builder().addUpstreamDriver("my_driver", "1.0.0");
204203
DriverInfo.builder().addUpstreamDriver("driver123", "1.0.0");
204+
DriverInfo.builder().addUpstreamDriver("Spring-Data", "1.0.0");
205+
DriverInfo.builder().addUpstreamDriver("123driver", "1.0.0");
206+
DriverInfo.builder().addUpstreamDriver("driver@name", "1.0.0");
207+
DriverInfo.builder().addUpstreamDriver("driver.name", "1.0.0");
205208

206-
// Invalid names
207-
assertThrows(JedisValidationException.class,
208-
() -> DriverInfo.builder().addUpstreamDriver("Spring-Data", "1.0.0")); // uppercase
209-
assertThrows(JedisValidationException.class,
210-
() -> DriverInfo.builder().addUpstreamDriver("123driver", "1.0.0")); // starts with digit
211-
assertThrows(JedisValidationException.class,
212-
() -> DriverInfo.builder().addUpstreamDriver("driver@name", "1.0.0")); // special char
213-
assertThrows(JedisValidationException.class,
214-
() -> DriverInfo.builder().addUpstreamDriver("driver.name", "1.0.0")); // dot
215209
assertThrows(JedisValidationException.class,
216210
() -> DriverInfo.builder().addUpstreamDriver("driver name", "1.0.0")); // space
217211
}

0 commit comments

Comments
 (0)