Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AerospikeCache requires ID property on entity even though key has already been provided #702

Open
dhamilton opened this issue Feb 16, 2024 · 1 comment

Comments

@dhamilton
Copy link

dhamilton commented Feb 16, 2024

When upgrading to spring-data-aerospike 4.6.0 from version 3.4.1, we started encountering the following errors when trying to use @Cacheable with entities that did not have an @Id property present.

com.aerospike.client.AerospikeException: Error 26: Id has not been provided
	at org.springframework.data.aerospike.convert.MappingAerospikeWriteConverter.getNewKey(MappingAerospikeWriteConverter.java:144)
	at org.springframework.data.aerospike.convert.MappingAerospikeWriteConverter.write(MappingAerospikeWriteConverter.java:93)
	at org.springframework.data.aerospike.convert.MappingAerospikeConverter.write(MappingAerospikeConverter.java:83)
	at com.wayfair.waycomm.garage.config.helpers.AerospikeConfigurationHelper$2.write(AerospikeConfigurationHelper.java:67)
	at com.wayfair.waycomm.garage.config.helpers.AerospikeConfigurationHelper$2.write(AerospikeConfigurationHelper.java:52)
	at org.springframework.data.aerospike.cache.AerospikeCache.serializeAndPut(AerospikeCache.java:204)

This is a regression from v3. Looking at MappingAerospikeWriteConverter.java we see that the requirement for an @Id property is not enforced if the AerospikeWriteData object has already been populated with a key (https://github.com/aerospike/spring-data-aerospike/blob/main/src/main/java/org/springframework/data/aerospike/convert/MappingAerospikeWriteConverter.java#L119). Furthermore, the requirement serves no purpose since AerospikeCache.java uses the key provided to the cache API when writing the object (https://github.com/aerospike/spring-data-aerospike/blob/main/src/main/java/org/springframework/data/aerospike/cache/AerospikeCache.java#L205).

We have verified that this issue can be addressed by forking the code and updating AerospikeCache.serializeAndPut() to set the key on the AerospikeWriteData before calling the converter as shown below:

    private void serializeAndPut(WritePolicy writePolicy, Object key, Object value) {
        Key aerospikeKey = getKey(key);
        AerospikeWriteData data = AerospikeWriteData.forWrite(aerospikeKey.namespace);
        data.setKey(aerospikeKey);  // Set the key on the data object
        aerospikeConverter.write(value, data);
        client.put(writePolicy, aerospikeKey, data.getBinsAsArray());
    }

Can a change like this be made to the project? If you want me to open a PR I'd be happy to do so if provided with guidance. Thanks!

@agrgr
Copy link

agrgr commented Apr 24, 2024

Hello @dhamilton!
The code you have provided looks legitimate, it would be great if you open a PR with the change and also with a relevant test. To do that you can fork the Spring Data Aerospike repository, make the changes and open a pull request which we will then review.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants