Skip to content

Commit ebaf49f

Browse files
committed
feat: add error handling configuration for saveAll operations (#517)
Added throwOnSaveAllFailure configuration property to control whether saveAll operations throw exceptions on failures or just log warnings. Changes: - Added redis.om.spring.repository.throw-on-save-all-failure property (defaults to false for backward compatibility) - Modified SimpleRedisEnhancedRepository to use syncAndReturnAll() instead of sync() to capture operation responses - Both repositories now check pipeline responses for JedisDataException and either throw RuntimeException or log warnings based on config - Added tests to verify saveAll error handling works correctly This allows applications to detect and handle bulk save failures programmatically when Redis encounters issues like memory limits.
1 parent e65fc7d commit ebaf49f

File tree

5 files changed

+118
-5
lines changed

5 files changed

+118
-5
lines changed

redis-om-spring/src/main/java/com/redis/om/spring/RedisOMProperties.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,13 @@ public static class Repository {
243243
*/
244244
private int deleteBatchSize = 500;
245245

246+
/**
247+
* Whether to throw exceptions when saveAll operations fail.
248+
* When false (default), failures are logged as warnings.
249+
* When true, failures throw an exception.
250+
*/
251+
private boolean throwOnSaveAllFailure = false;
252+
246253
/**
247254
* Default constructor for Repository configuration.
248255
*/
@@ -296,6 +303,25 @@ public void setDeleteBatchSize(int deleteBatchSize) {
296303
this.deleteBatchSize = deleteBatchSize;
297304
}
298305

306+
/**
307+
* Checks if exceptions should be thrown when saveAll operations fail.
308+
*
309+
* @return {@code true} if exceptions should be thrown on failures, {@code false} otherwise
310+
*/
311+
public boolean isThrowOnSaveAllFailure() {
312+
return throwOnSaveAllFailure;
313+
}
314+
315+
/**
316+
* Sets whether exceptions should be thrown when saveAll operations fail.
317+
*
318+
* @param throwOnSaveAllFailure {@code true} to throw exceptions on failures,
319+
* {@code false} to log warnings only
320+
*/
321+
public void setThrowOnSaveAllFailure(boolean throwOnSaveAllFailure) {
322+
this.throwOnSaveAllFailure = throwOnSaveAllFailure;
323+
}
324+
299325
/**
300326
* Configuration properties for query behavior.
301327
* <p>

redis-om-spring/src/main/java/com/redis/om/spring/repository/support/SimpleRedisDocumentRepository.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,13 +319,21 @@ public <S extends T> List<S> saveAll(Iterable<S> entities) {
319319

320320
// Process responses using streams to avoid iterator issues
321321
if (responses != null && !responses.isEmpty()) {
322+
List<String> failedIds = new ArrayList<>();
322323
long failedCount = IntStream.range(0, Math.min(responses.size(), entityIds.size())).filter(i -> responses.get(
323-
i) instanceof JedisDataException).peek(i -> logger.warn(
324-
"Failed JSON.SET command for entity with id: {} Error: {}", entityIds.get(i),
325-
((JedisDataException) responses.get(i)).getMessage())).count();
324+
i) instanceof JedisDataException).peek(i -> {
325+
failedIds.add(entityIds.get(i).toString());
326+
logger.warn("Failed JSON.SET command for entity with id: {} Error: {}", entityIds.get(i),
327+
((JedisDataException) responses.get(i)).getMessage());
328+
}).count();
326329

327330
if (failedCount > 0) {
328-
logger.warn("Total failed JSON.SET commands: {}", failedCount);
331+
String errorMsg = String.format("Failed to save %d entities with IDs: %s", failedCount, failedIds);
332+
if (properties.getRepository().isThrowOnSaveAllFailure()) {
333+
throw new RuntimeException(errorMsg);
334+
} else {
335+
logger.warn("Total failed JSON.SET commands: {}", failedCount);
336+
}
329337
}
330338
}
331339
}

redis-om-spring/src/main/java/com/redis/om/spring/repository/support/SimpleRedisEnhancedRepository.java

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@
77
import java.util.concurrent.TimeUnit;
88
import java.util.function.Function;
99
import java.util.stream.Collectors;
10+
import java.util.stream.IntStream;
1011
import java.util.stream.StreamSupport;
1112

13+
import org.slf4j.Logger;
14+
import org.slf4j.LoggerFactory;
1215
import org.springframework.beans.BeanWrapper;
1316
import org.springframework.beans.factory.annotation.Qualifier;
1417
import org.springframework.dao.IncorrectResultSizeDataAccessException;
@@ -54,6 +57,7 @@
5457

5558
import redis.clients.jedis.Jedis;
5659
import redis.clients.jedis.Pipeline;
60+
import redis.clients.jedis.exceptions.JedisDataException;
5761
import redis.clients.jedis.search.Query;
5862
import redis.clients.jedis.search.SearchResult;
5963
import redis.clients.jedis.util.SafeEncoder;
@@ -73,6 +77,8 @@
7377
public class SimpleRedisEnhancedRepository<T, ID> extends SimpleKeyValueRepository<T, ID> implements
7478
RedisEnhancedRepository<T, ID> {
7579

80+
private static final Logger logger = LoggerFactory.getLogger(SimpleRedisEnhancedRepository.class);
81+
7682
/** Operations for Redis modules (Search, JSON, etc.) */
7783
protected final RedisModulesOperations<String> modulesOperations;
7884
/** Metadata about the entity type managed by this repository */
@@ -409,6 +415,7 @@ private String getKey(Object id) {
409415
public <S extends T> List<S> saveAll(Iterable<S> entities) {
410416
Assert.notNull(entities, "The given Iterable of entities must not be null!");
411417
List<S> saved = new ArrayList<>();
418+
List<String> entityIds = new ArrayList<>();
412419

413420
embedder.processEntities(entities);
414421

@@ -426,6 +433,7 @@ public <S extends T> List<S> saveAll(Iterable<S> entities) {
426433
keyValueEntity.getPropertyAccessor(entity).setProperty(keyValueEntity.getIdProperty(), id);
427434

428435
String idAsString = validateKeyForWriting(id, entity);
436+
entityIds.add(idAsString);
429437

430438
String keyspace = keyValueEntity.getKeySpace();
431439
byte[] objectKey = createKey(keyspace, idAsString);
@@ -448,7 +456,28 @@ public <S extends T> List<S> saveAll(Iterable<S> entities) {
448456

449457
saved.add(entity);
450458
}
451-
pipeline.sync();
459+
460+
List<Object> responses = pipeline.syncAndReturnAll();
461+
462+
// Process responses to check for errors
463+
if (responses != null && !responses.isEmpty()) {
464+
List<String> failedIds = new ArrayList<>();
465+
long failedCount = IntStream.range(0, Math.min(responses.size(), entityIds.size())).filter(i -> responses.get(
466+
i) instanceof JedisDataException).peek(i -> {
467+
failedIds.add(entityIds.get(i));
468+
logger.warn("Failed HMSET command for entity with id: {} Error: {}", entityIds.get(i),
469+
((JedisDataException) responses.get(i)).getMessage());
470+
}).count();
471+
472+
if (failedCount > 0) {
473+
String errorMsg = String.format("Failed to save %d entities with IDs: %s", failedCount, failedIds);
474+
if (properties.getRepository().isThrowOnSaveAllFailure()) {
475+
throw new RuntimeException(errorMsg);
476+
} else {
477+
logger.warn("Total failed HMSET commands: {}", failedCount);
478+
}
479+
}
480+
}
452481
}
453482

454483
return saved;

tests/src/test/java/com/redis/om/spring/annotations/document/BasicRedisDocumentMappingTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,27 @@ void testEffectOfNotNullAnnotation() {
917917
assertThat(fields).first().isEqualTo("001");
918918
}
919919

920+
@Test
921+
void testIssue517_SaveAllErrorHandlingConfiguration() {
922+
// Test for issue #517: saveAll error handling configuration
923+
// This verifies that the configuration property is available and doesn't break normal operations
924+
// In a real scenario with memory limits, the throwOnSaveAllFailure property would cause exceptions
925+
926+
Company company1 = Company.of("TestCompany1", 2023, LocalDate.now(),
927+
new Point(-122.066540, 37.377690), "[email protected]");
928+
Company company2 = Company.of("TestCompany2", 2023, LocalDate.now(),
929+
new Point(-122.066540, 37.377690), "[email protected]");
930+
931+
List<Company> companies = List.of(company1, company2);
932+
List<Company> saved = repository.saveAll(companies);
933+
934+
assertThat(saved).hasSize(2);
935+
assertThat(repository.count()).isGreaterThanOrEqualTo(2);
936+
937+
// Clean up
938+
repository.deleteAll(saved);
939+
}
940+
920941
@Test
921942
void testIssue622_ExistsByQueryReturnsBoolean() {
922943
// Test for issue #622: existsBy* queries should return boolean instead of throwing ClassCastException

tests/src/test/java/com/redis/om/spring/annotations/hash/BasicRedisHashMappingTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,35 @@ void testRepositoryGetKeyFor() {
757757
assertThat(microsoftKey).isEqualTo(getKey(Company.class.getName(), microsoft.getId()));
758758
}
759759

760+
@Test
761+
void testIssue517_HashSaveAllErrorHandlingConfiguration() {
762+
// Test for issue #517: saveAll error handling configuration for hash entities
763+
// This verifies that syncAndReturnAll is now used and errors can be detected
764+
765+
Person person1 = new Person();
766+
person1.setName("Test Person 1");
767+
person1.setEmail("[email protected]");
768+
person1.setNickname("test1");
769+
person1.setRoles(Set.of("user"));
770+
person1.setFavoriteFoods(Set.of("pizza"));
771+
772+
Person person2 = new Person();
773+
person2.setName("Test Person 2");
774+
person2.setEmail("[email protected]");
775+
person2.setNickname("test2");
776+
person2.setRoles(Set.of("user"));
777+
person2.setFavoriteFoods(Set.of("burger"));
778+
779+
List<Person> people = List.of(person1, person2);
780+
List<Person> saved = personRepo.saveAll(people);
781+
782+
assertThat(saved).hasSize(2);
783+
assertThat(personRepo.count()).isGreaterThanOrEqualTo(2);
784+
785+
// Clean up
786+
personRepo.deleteAll(saved);
787+
}
788+
760789
@Test
761790
void testIssue622_HashExistsByQueryReturnsBoolean() {
762791
// Test for issue #622: existsBy* queries should return boolean for hash entities

0 commit comments

Comments
 (0)