Jsr310Converters does not contain converters for OffsetTime and OffsetDateTime

[DeltaIII]

When using spring-data-redis and RedisSerializer.java() I noticed that a field of type OffsetTime wasn't being serialised even though it implements Serializable. The root cause is that SimpleTypeHolder assumes that any class that starts with java.time is a simple type (which in turn means CustomConversions.isSimpleType(OffsetTime.class) == true). This breaks the assumption that CustomConversions has a converter for the type as per the javadoc:

Returns whether the given type is considered to be simple. That means it's either a general simple type or we have a writing Converter registered for a particular type.

Expected behaviour:

• customConversions.isSimpleType returns false if there are no converters for OffsetDateTime or OffsetTime

Actual behaviour:

• customConversions.isSimpleType(OffsetDateTime.class) == true even if there are no converters for OffsetDateTime (and for OffsetTime

Relevant Code links:
https://github.com/spring-projects/spring-data-commons/blob/541f0ced32f3f8b2143ea5b793a489828567ff13/src/main/java/org/springframework/data/mapping/model/SimpleTypeHolder.java#L158

https://github.com/spring-projects/spring-data-commons/blob/541f0ced32f3f8b2143ea5b793a489828567ff13/src/main/java/org/springframework/data/convert/CustomConversions.java#L190

[mp911de]

I moved this ticket into Spring Data Redis as it is a module-specific concern. We have already quite a few converters for JSR-310 types, however we don't have ones for Offset[Date]Time.

One more aspect here: JdkSerializationRedisSerializer isn't actually related to CustomConversions, it would be good if you shared some code in which case you're experiencing null values so that we understand your context.

Since you're a bit familiar with our code, do you want to submit a pull request so that we can ship a fix with the next release?

[jxblum]

FTR: I was able to produce a similar problem (I suppose) by simply adding a OffsetDateTime (persistent) property to the Person class from the Spring Data Examples, Spring Data Redis Repositories example, and setting a few OffsetDateTime values for the "Starks" (e.g. "arya").

Modified Person class:

class Person {

    private OffsetDateTime lastAccessed;

    // other persistent properties

    Person lastAccessed(OffsetDateTime lastAccessed) {
        this.lastAccessed = lastAccessed;
        return this;
    }
}

Modified PersonRepositoryTest class:

class PersonRepositoryTests {

    private Person arya = new Person("arya", "stark", Gender.FEMALE).lastAccessed(OffsetDateTime.now());

    // ...
}

Then I simply ran and debugged the findBySingleProperty() test case method.

I don't necessarily agree that we need "custom" SD Converters for java.time types, particularly since java.time types are java.io.Serializable, and the Spring Data Redis JdkSerializationRedisSerializer (used by default in the RedisTemplate, which is used by the SD (Redis/KeyValue) Repository infrastructure) should be able serialize OffsetDateTime values to a byte[] for the persistent property to be mapped to Redis by the MappingRedisConverter and other supporting Repository infrastructure classes.

Of course, it would need to be given a chance to serialize and persist java.time values from persistent entities in the first place, but seemingly, the MappingRedisConverter.writeToBucket(..) expects there to be a registered, "custom" Conversion for a property of type OffsetDateTime.

The execution path for a persistent property of type OffsetDateTime on a persistent entity (such as the Person class in the examples), as witnessed from the debugger (with source references to Spring Data Redis (only), ignoring Spring Data KeyValue), follows:

0. From the SD KeyValue Repository infrastruture (on which SD Redis bulids), the SimpleKeyValueRepository.save(entity) method is called

1. Calls SD KeyValue KeyValueTemplate.insert(objectToInsert)

2. RedisKeyValueTemplate.insert(id, objectToInsert)

3. Calls (back to) SD KeyValue KeyValueTemplate.insert(id, objectToInsert)

4. Calls RedisKeyValueAdapter.put(id, item, keyspace)

5. Calls MappingRedisConverter.write(source, sink:RedisData)

6. Calls MappingRedisConverter.writeInternal(..)

7. Then fails to write the OffsetDateTime since there is no registered, "custom" Conversion in MappingRedisConverter.writeToBucket(..)

There should NOT need to be a "custom" Conversion since the OffsetDateTime, or java.time types in general, are "simple" store types that can be serialized to a byte[] (by SD Redis's JdkSerializationRedisSerializer) and stored in the Redis hash for a Person instance, AFAICT.

I have only started investigating this issue so I am still scratching the surface.

[jxblum]

On closer inspection, this may be a simple fix, by simply registering additional java.time Converters for OffsetDateTime and similar, missing java.time types (e.g. OffsetTime), as necessary in SD Redis's Jsr310Converters class (see here).

Although, since Java's OffsetDateTime (and OffsetTime) have been part of the JDK since Java 8 (for example; see @since tag), but were not added to SD Redis's Jsr310Converters class along with other java.time types on creation, I am now curious why?

I doubt they were simply "forgotten".

[mp911de]

We never provided Zoned or Offset variants of temporal types on a broader basis because in typical stores working with dates, such a field requires two components (time, offset/zone) to be properly constructed without resorting to the system timezone.

In Redis, since everything is a byte array, we weren't asked to provide converters and so they never got added in the first place.

[lduffman]

Hi, I'm facing the same problem of serialisation of an OffsetDateTime. Is there any workaround using configuration with RedisCustomConversions or RedisTemplate ? Like many i tried to implement my proper RedisTemplate with its custom serializer (ObjectMapper with JavaTimeModule from jackson-datatype-jsr310 or my custom one) but never consumed on sending.

Here an example of the configuration i've done :

Configuration:

@Slf4j
@Configuration
public class CustomRedisConfiguration {

    @Bean
    public RedisConnectionFactory redisConnectionFactory() {
        var redisURI = RedisURI.builder()
                .withDatabase(0)
                .withHost("localhost")
                .withPort(6379)
                .build();
        RedisConfiguration redisConfiguration = LettuceConnectionFactory.createRedisConfiguration(redisURI);
        return new LettuceConnectionFactory(redisConfiguration);

    }

    @Bean
    public RedisCustomConversions redisCustomConversions(OffsetDateTimeToBytesConverter offsetToBytes) {
        return new RedisCustomConversions(Collections.singletonList(offsetToBytes));
    }

    @Primary
    @Bean
    RedisTemplate<String, TestOffsetDateTime> redisTemplate(RedisConnectionFactory connectionFactory) {
        RedisTemplate<String, TestOffsetDateTime> redisTemplate = new RedisTemplate<>();
        redisTemplate.setConnectionFactory(connectionFactory);
        redisTemplate.setKeySerializer(new StringRedisSerializer());
        redisTemplate.setValueSerializer(jackson2JsonRedisSerializer());
        redisTemplate.afterPropertiesSet();

        return redisTemplate;
    }
    @Bean
    public Jackson2JsonRedisSerializer<TestOffsetDateTime> jackson2JsonRedisSerializer() {
        var serializer =  new Jackson2JsonRedisSerializer<>(TestOffsetDateTime.class);
        ObjectMapper om = new ObjectMapper();
        om.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
        om.registerModule(new JavaTimeModule());
        om.registerModule(new SimpleModule().addSerializer(OffsetDateTime.class, new OffsetDateTimeSerializer()));
        serializer.setObjectMapper(om);

        return serializer;
    }
}

Serializer:

@Component
public class OffsetDateTimeSerializer extends JsonSerializer<OffsetDateTime> {
    private static final Logger log = LoggerFactory.getLogger(OffsetDateTimeSerializer.class);

    public OffsetDateTimeSerializer() {
    }

    public void serialize(OffsetDateTime offsetDateTime, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException {
        if (offsetDateTime == null) {
            throw new IOException("OffsetDateTime argument is null.");
        } else {
            log.info("LOG> in serializer {}", offsetDateTime);
            jsonGenerator.writeString(DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(offsetDateTime));
        }
    }
}

Converter:

@Component
@WritingConverter
public class OffsetDateTimeToBytesConverter implements Converter<OffsetDateTime, byte[]> {
    @Override
    public byte[] convert(final OffsetDateTime source) {
        return source.format(DateTimeFormatter.ISO_OFFSET_DATE_TIME).getBytes();
    }
}

Object to send:

@Data
@Builder
@AllArgsConstructor
@NoArgsConstructor
public class TestOffsetDateTime {

    public String string;
    @JsonFormat(
            shape = JsonFormat.Shape.STRING,
            pattern = "yyyy-MM-dd'T'HH:mm:ss.SSSXXX"
    )
    public OffsetDateTime offsetDateTime;
    public LocalDateTime localDateTime;
    public Date date;
}

Publisher:

public RecordId test() {

        var test = TestOffsetDateTime.builder()
                .string("test")
                .localDateTime(LocalDateTime.now())
                .offsetDateTime(OffsetDateTime.now())
                .date(new Date())
                .build();

        ObjectRecord<String, TestOffsetDateTime> record = StreamRecords.newRecord()
                .ofObject(test)
                .withStreamKey("test-stream");

        RecordId r = this.redisOperations.opsForStream().add(record);
        log.info("LOG> recordId: {}", r);
        return r;
    }

[jxblum]

@mp911de - Then, I guess 1 question I have is, why are we converting ZonedDateTime to/from byte[] arrays (here).

I get that it is just a ZonedDateTime to String to byte[] conversion and back, but we could do the same for OffsetDateTime / OffsetTime values as well.

[mp911de]

Yeah, I would like to have these as well. Am 16. Aug. 2023, 19:32 +0200 schrieb John Blum ***@***.***>:

…

@mp911de - Then, I guess 1 question I have is, why are we de/serializing ZonedDateTime, here? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[jxblum]

@DeltaIII & @lduffman -

I explored more on this issue today, and while Spring Data Redis currently does not support either java.time.OffsetDateTime or java.time.OffsetTime types presently, there is a workaround.

I have demonstrated this in my redis-experiments project, within the spring-data-redis-experiments module, and specifically the RedisRepositoryWithEntityHavingOffsetDateTimePropertyIntegrationTests class.

You can see that my User application entity type (source) contains both OffsetDateTime and OffsetTime properties (here).

Additionally, I have "customized" the SD (Redis) Repository infrastructure configuration, and specifically the "RedisCustomConversions" that is registered as a bean in the Spring ApplicationContext (container) when using the SD Redis Repository infrastructure.

This allows you to add additional Converters required by your Spring (Boot) Data Redis applications.

First, I defined custom Converters for the java.time types: OffsetDateTime and OffsetTime. This is not unlike how Spring Data Redis Jsr310Converters handles conversion for many of the java.time types already (see here).

Then, I simply replace the RedisCustomConversions bean with a new instance that registers all the framework provided converters in addition to my application-specific Converters using a Spring BeanPostProcessor (see here).

Alternatively, you could simply use the ZonedDateTime type on your application entities rather that OffsetDateTime (or OffsetTime) and properties of these java.time types will be properly handled.

[jxblum]

See #2681.

[DeltaIII]

Thank you very much 😄

Any idea when this would be in a release?

[mp911de]

Check out the release calendar at https://calendar.spring.io/. As of now, the next round of releases is scheduled for tomorrow.

[lduffman]

@jxblum Thank you very much for your response and investigation. I didn't succeed to use OffsetDateTime with redis streams (no repositories in my case) but just using ZonedDateTime type will do the trick for the moment :)