Skip to content

Commit c1b72e5

Browse files
committed
PR comments resolved.
Signed-off-by: annemayor <pranne1224@gmail.com>
1 parent 5fffa27 commit c1b72e5

File tree

6 files changed

+144
-187
lines changed

6 files changed

+144
-187
lines changed

src/main/java/org/springframework/data/redis/connection/RedisVectorSetCommands.java

Lines changed: 81 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
import org.jspecify.annotations.NonNull;
1919
import org.jspecify.annotations.NullUnmarked;
2020
import org.jspecify.annotations.Nullable;
21+
import org.springframework.data.domain.Vector;
2122

22-
import java.util.HashMap;
23-
import java.util.Map;
23+
import java.util.Objects;
2424

2525
/**
2626
* Vector Set-specific commands supported by Redis.
@@ -35,53 +35,100 @@ public interface RedisVectorSetCommands {
3535
* Add a vector to a vector set using FP32 binary format.
3636
*
3737
* @param key the key
38-
* @param values the vector as FP32 binary blob
38+
* @param vector the vector as FP32 binary blob
3939
* @param element the element name
4040
* @param options the options for the command
4141
* @return true if the element was added, false if it already existed
4242
*/
43-
Boolean vAdd(byte @NonNull [] key, byte @NonNull [] values, byte @NonNull [] element, VAddOptions options);
43+
Boolean vAdd(byte @NonNull [] key, byte @NonNull [] vector, byte @NonNull [] element, @Nullable VAddOptions options);
44+
45+
/**
46+
* Add a vector to a vector set using Vector.
47+
*
48+
* @param key the key
49+
* @param vector the vector
50+
* @param element the element name
51+
* @param options the options for the command
52+
* @return true if the element was added, false if it already existed
53+
*/
54+
Boolean vAdd(byte @NonNull [] key, @NonNull Vector vector, byte @NonNull [] element, VAddOptions options);
4455

4556
/**
4657
* Add a vector to a vector set using double array.
4758
*
4859
* @param key the key
49-
* @param values the vector as double array
60+
* @param vector the vector as double array
5061
* @param element the element name
5162
* @param options the options for the command
5263
* @return true if the element was added, false if it already existed
5364
*/
54-
Boolean vAdd(byte @NonNull [] key, double @NonNull [] values, byte @NonNull [] element, VAddOptions options);
65+
default Boolean vAdd(byte @NonNull [] key, double @NonNull [] vector, byte @NonNull [] element, VAddOptions options) {
66+
return vAdd(key, Vector.unsafe(vector), element, options);
67+
}
5568

5669
/**
5770
* Options for the VADD command.
5871
*
5972
* Note on attributes:
60-
* - Attributes are serialized to JSON and must be JavaScript/JSON compatible types
61-
* - Supported types: String, Number (Integer, Long, Double, Float), Boolean, null
62-
* - Collections (List, Map) are supported for nested structures
63-
* - Custom objects require proper JSON serialization support
64-
* - Date/Time objects should be converted to String or timestamp before use
73+
* - Attributes should be provided as a JSON string
74+
* - The caller is responsible for JSON serialization
6575
*/
6676
class VAddOptions {
77+
78+
private static final VAddOptions DEFAULT = new VAddOptions(null, false, QuantizationType.Q8, null, null, null);
79+
6780
private final @Nullable Integer reduceDim;
6881
private final boolean cas;
6982
private final QuantizationType quantization;
7083
private final @Nullable Integer efBuildFactor;
71-
private final @Nullable Map<String, Object> attributes;
84+
private final @Nullable String attributes;
7285
private final @Nullable Integer maxConnections;
7386

74-
private VAddOptions(Builder builder) {
75-
this.reduceDim = builder.reduceDim;
76-
this.cas = builder.cas;
77-
this.quantization = builder.quantization;
78-
this.efBuildFactor = builder.efBuildFactor;
79-
this.attributes = builder.attributes;
80-
this.maxConnections = builder.maxConnections;
87+
public VAddOptions(@Nullable Integer reduceDim, boolean cas, QuantizationType quantization,
88+
@Nullable Integer efBuildFactor, @Nullable String attributes,
89+
@Nullable Integer maxConnections) {
90+
this.reduceDim = reduceDim;
91+
this.cas = cas;
92+
this.quantization = quantization;
93+
this.efBuildFactor = efBuildFactor;
94+
this.attributes = attributes;
95+
this.maxConnections = maxConnections;
96+
}
97+
98+
public static VAddOptions defaults() {
99+
return DEFAULT;
100+
}
101+
102+
public static VAddOptions reduceDim(@Nullable Integer reduceDim) {
103+
return new VAddOptions(reduceDim, false, QuantizationType.Q8, null, null, null);
81104
}
82105

83-
public static Builder builder() {
84-
return new Builder();
106+
public static VAddOptions cas(boolean cas) {
107+
return new VAddOptions(null, cas, QuantizationType.Q8, null, null, null);
108+
}
109+
110+
public static VAddOptions quantization(QuantizationType quantization) {
111+
return new VAddOptions(null, false, quantization, null, null, null);
112+
}
113+
114+
public static VAddOptions efBuildFactor(@Nullable Integer efBuildFactor) {
115+
return new VAddOptions(null, false, QuantizationType.Q8, efBuildFactor, null, null);
116+
}
117+
118+
public static VAddOptions attributes(@Nullable String attributes) {
119+
return new VAddOptions(null, false, QuantizationType.Q8, null, attributes, null);
120+
}
121+
122+
public static VAddOptions maxConnections(@Nullable Integer maxConnections) {
123+
return new VAddOptions(null, false, QuantizationType.Q8, null, null, maxConnections);
124+
}
125+
126+
public static VAddOptions casWithQuantization(boolean cas, QuantizationType quantization) {
127+
return new VAddOptions(null, cas, quantization, null, null, null);
128+
}
129+
130+
public static VAddOptions reduceDimWithQuantization(Integer reduceDim, QuantizationType quantization) {
131+
return new VAddOptions(reduceDim, false, quantization, null, null, null);
85132
}
86133

87134
public @Nullable Integer getReduceDim() {
@@ -100,65 +147,26 @@ public QuantizationType getQuantization() {
100147
return efBuildFactor;
101148
}
102149

103-
public @Nullable Map<String, Object> getAttributes() {
150+
public @Nullable String getAttributes() {
104151
return attributes;
105152
}
106153

107154
public @Nullable Integer getMaxConnections() {
108155
return maxConnections;
109156
}
110157

111-
public static class Builder {
112-
private @Nullable Integer reduceDim;
113-
private boolean cas = false;
114-
private QuantizationType quantization = QuantizationType.Q8;
115-
private @Nullable Integer efBuildFactor;
116-
private @Nullable Map<String, Object> attributes;
117-
private @Nullable Integer maxConnections;
118-
119-
private Builder() {}
120-
121-
public Builder reduceDim(@Nullable Integer reduceDim) {
122-
this.reduceDim = reduceDim;
123-
return this;
124-
}
125-
126-
public Builder cas(boolean cas) {
127-
this.cas = cas;
128-
return this;
129-
}
130-
131-
public Builder quantization(QuantizationType quantization) {
132-
this.quantization = quantization;
133-
return this;
134-
}
135-
136-
public Builder efBuildFactor(@Nullable Integer efBuildFactor) {
137-
this.efBuildFactor = efBuildFactor;
138-
return this;
139-
}
140-
141-
public Builder attributes(@Nullable Map<String, Object> attributes) {
142-
this.attributes = attributes;
143-
return this;
144-
}
145-
146-
public Builder attribute(String key, Object value) {
147-
if (this.attributes == null) {
148-
this.attributes = new HashMap<>();
149-
}
150-
this.attributes.put(key, value);
151-
return this;
152-
}
153-
154-
public Builder maxConnections(@Nullable Integer maxConnections) {
155-
this.maxConnections = maxConnections;
156-
return this;
157-
}
158-
159-
public VAddOptions build() {
160-
return new VAddOptions(this);
161-
}
158+
@Override
159+
public boolean equals(Object o) {
160+
if (!(o instanceof VAddOptions that)) {return false;}
161+
return cas == that.cas && Objects.equals(reduceDim, that.reduceDim)
162+
&& quantization == that.quantization && Objects.equals(efBuildFactor, that.efBuildFactor)
163+
&& Objects.equals(attributes, that.attributes) && Objects.equals(maxConnections,
164+
that.maxConnections);
165+
}
166+
167+
@Override
168+
public int hashCode() {
169+
return Objects.hash(reduceDim, cas, quantization, efBuildFactor, attributes, maxConnections);
162170
}
163171

164172
public enum QuantizationType {

src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterVSetCommands.java

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.jspecify.annotations.NonNull;
1919
import org.jspecify.annotations.NullUnmarked;
2020
import org.springframework.dao.DataAccessException;
21+
import org.springframework.data.domain.Vector;
2122
import org.springframework.data.redis.connection.RedisVectorSetCommands;
2223
import org.springframework.util.Assert;
2324

@@ -40,60 +41,55 @@ class JedisClusterVSetCommands implements RedisVectorSetCommands {
4041
}
4142

4243
@Override
43-
public Boolean vAdd(byte @NonNull [] key, byte @NonNull [] values, byte @NonNull [] element,
44+
public Boolean vAdd(byte @NonNull [] key, byte @NonNull [] vector, byte @NonNull [] element,
4445
VAddOptions options) {
4546
Assert.notNull(key, "Key must not be null");
46-
Assert.notNull(values, "Values must not be null");
47+
Assert.notNull(vector, "Vector must not be null");
4748
Assert.notNull(element, "Element must not be null");
4849

4950
try {
5051
if (options == null) {
51-
return connection.getCluster().vaddFP32(key, values, element);
52+
return connection.getCluster().vaddFP32(key, vector, element);
5253
}
5354

5455
VAddParams params = JedisConverters.toVAddParams(options);
5556

5657
if (options.getReduceDim() != null) {
5758
// With REDUCE dimension
58-
return connection.getCluster().vaddFP32(key, values, element, options.getReduceDim(), params);
59+
return connection.getCluster().vaddFP32(key, vector, element, options.getReduceDim(), params);
5960
}
6061

61-
return connection.getCluster().vaddFP32(key, values, element, params);
62+
return connection.getCluster().vaddFP32(key, vector, element, params);
6263
} catch (Exception ex) {
6364
throw convertJedisAccessException(ex);
6465
}
6566
}
6667

67-
@Override
68-
public Boolean vAdd(byte @NonNull [] key, double @NonNull [] values, byte @NonNull [] element,
69-
VAddOptions options) {
70-
Assert.notNull(key, "Key must not be null");
71-
Assert.notNull(values, "Values must not be null");
72-
Assert.notNull(element, "Element must not be null");
68+
@Override
69+
public Boolean vAdd(byte @NonNull [] key, @NonNull Vector vector, byte @NonNull [] element,
70+
VAddOptions options) {
7371

74-
// Convert double[] to float[] since Jedis uses float[]
75-
float[] floatValues = new float[values.length];
76-
for (int i = 0; i < values.length; i++) {
77-
floatValues[i] = (float) values[i];
78-
}
72+
Assert.notNull(key, "Key must not be null");
73+
Assert.notNull(vector, "Vector must not be null");
74+
Assert.notNull(element, "Element must not be null");
7975

80-
try {
81-
if (options == null) {
82-
return connection.getCluster().vadd(key, floatValues, element);
83-
}
76+
try {
77+
if (options == null) {
78+
return connection.getCluster().vadd(key, vector.toFloatArray(), element);
79+
}
8480

85-
redis.clients.jedis.params.VAddParams params = JedisConverters.toVAddParams(options);
81+
VAddParams params = JedisConverters.toVAddParams(options);
8682

87-
if (options.getReduceDim() != null) {
88-
// With REDUCE dimension
89-
return connection.getCluster().vadd(key, floatValues, element, options.getReduceDim(), params);
90-
}
83+
if (options.getReduceDim() != null) {
84+
// With REDUCE dimension
85+
return connection.getCluster().vadd(key, vector.toFloatArray(), element, options.getReduceDim(), params);
86+
}
9187

92-
return connection.getCluster().vadd(key, floatValues, element, params);
93-
} catch (Exception ex) {
94-
throw convertJedisAccessException(ex);
95-
}
96-
}
88+
return connection.getCluster().vadd(key, vector.toFloatArray(), element, params);
89+
} catch (Exception ex) {
90+
throw convertJedisAccessException(ex);
91+
}
92+
}
9793

9894
private DataAccessException convertJedisAccessException(Exception ex) {
9995
return connection.convertJedisAccessException(ex);

src/main/java/org/springframework/data/redis/connection/jedis/JedisConverters.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@
3535
import redis.clients.jedis.resps.GeoRadiusResponse;
3636
import redis.clients.jedis.util.SafeEncoder;
3737

38-
import com.fasterxml.jackson.core.JsonProcessingException;
39-
import com.fasterxml.jackson.databind.ObjectMapper;
4038

4139
import java.nio.ByteBuffer;
4240
import java.util.ArrayList;
@@ -54,7 +52,6 @@
5452
import org.jspecify.annotations.Nullable;
5553

5654
import org.springframework.core.convert.converter.Converter;
57-
import org.springframework.dao.InvalidDataAccessApiUsageException;
5855
import org.springframework.data.domain.Sort;
5956
import org.springframework.data.geo.Distance;
6057
import org.springframework.data.geo.GeoResult;
@@ -689,8 +686,6 @@ static ZAddParams toZAddParams(ZAddArgs source) {
689686
return target;
690687
}
691688

692-
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
693-
694689
/**
695690
* Convert {@link VAddOptions} into {@link VAddParams}.
696691
*
@@ -734,12 +729,7 @@ static VAddParams toVAddParams(@Nullable VAddOptions source) {
734729

735730
// Attributes as JSON
736731
if (source.getAttributes() != null) {
737-
try {
738-
String jsonAttributes = OBJECT_MAPPER.writeValueAsString(source.getAttributes());
739-
params.setAttr(jsonAttributes);
740-
} catch (JsonProcessingException e) {
741-
throw new InvalidDataAccessApiUsageException("Failed to serialize attributes to JSON", e);
742-
}
732+
params.setAttr(source.getAttributes());
743733
}
744734

745735
// M numlinks

0 commit comments

Comments
 (0)