-
Notifications
You must be signed in to change notification settings - Fork 29
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
Byte array converter + converters cleanup #388
Conversation
roimenashe
commented
May 29, 2022
- Byte arrays should not be converted or waste time on unnecessary convert collection flow (read flow only, write flow treat Byte arrays as simple fields).
- Aerospike converters cleanup, remove unnecessary/unused converters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may need to add the same check to the MappingAerospikeWriteConverter
.
@reugn Also test |
@@ -132,6 +132,10 @@ private <T> T readValue(Object source, TypeInformation<?> propertyType) { | |||
if (conversions.hasCustomReadTarget(source.getClass(), targetClass)) { | |||
return (T) conversionService.convert(source, targetClass); | |||
} else if (propertyType.isCollectionLike()) { | |||
/* Byte arrays should not be converted or waste time on unnecessary convert collection flow */ | |||
if (source instanceof byte[]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please prepare a test where:
- you write data using document:
class Doc {
byte[] array;
}
- you read data using document:
class Doc {
List<Byte> array;
}
Existing implementation should not fail, but introduced change will fail with exception. This is because you need to check not only for source, but also for target type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for performance improvement you need to have the same logic for arrays as with collections:
https://github.com/aerospike-community/spring-data-aerospike/blob/f12909f642ac9f2f7e7bb9eebbb2aded8897d6f7/src/main/java/org/springframework/data/aerospike/convert/MappingAerospikeReadConverter.java#L191
Add unit tests (Anastasiia).
@Aloren |