Class with @RedisHash and byte[] property is turned into Map<String, Byte>

[bergerst]

I discovered that if you define a class with @RedisHash and a byte[] property, one key-value pair is stored for each byte.

Here's some example code:

@RedisHash("myclass")
@lombok.Data
public class MyClass {
  @Id
  private String id;
  private byte[] property;

  public MyClass(String id) {
    this.id = id;
    property = new byte[4000];
    Random rd = new Random();
    rd.nextBytes(property);
  }
}

When an object of this class is stored in Redis using a CrudRepository<MyClass, String>, the Redis content will look like this:

property.[3018] = 50
property.[298] = 156
property.[1892] = 2
...

When you then use findById on the repository, Bucket.extract() will be called by MappingRedisConverter.readCollectionOrArray() for each key, leading to about 16 million String.startsWith() calls.

In my opinion, the byte[] should be stored as a String value, not as a Map<String, Byte>.

Replacing the byte[] with a String reduced the run time of my webservice operation from 1200 ms to 45 ms.

[bergerst]

Generally speaking, the performance of MappingRedisConverter.readCollectionOrArray() can be improved in all cases when large collections or arrays are stored.

Do you want me to create a PR for that part? It may not be the optimal solution, but it does the job much better.

A second change will be needed in order to read byte[] properties at once. How it's done right now:

1. Read byte as byte[]
2. Convert byte[] to Byte
3. Add Byte to Collection<Byte>
4. Convert Collection<Byte> to byte[]
5. Set property with byte[]

[christophstrobl]

We'd be happy to review a PR.
Please follow the contribution guidelines and make sure all tests are passing. The above change seems break some in MappingRedisConverterUnitTests.

[bergerst]

You're right, I didn't understand the full logic of the existing implementation.

The unit tests pass for my submitted PR.

[mp911de]

It would make sense to consider byte[] as a native simple type as the underlying data structure for Strings, numbers, etc. is a byte[]. Following the path opens a discussion about backward compatibility. Our code should be able to read byte arrays in the current format and directly from the hash value.

I'd be generally fine with tweaking the representation as of the next feature release. Any objections @christophstrobl?

[christophstrobl]

fine with me.