Redis consumer #31
Redis consumer #31
Conversation
consumer/redis-consumer/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.testcontainers</groupId> | ||
| <artifactId>testcontainers</artifactId> | ||
| <version>1.9.1</version> |
There was a problem hiding this comment.
Nice! I wonder if Spring Boot should manage the version for us...
There was a problem hiding this comment.
Just for info: https://github.com/testcontainers/testcontainers-spring-boot.
If you are interested in we could use their Embedded Redis feature over here: https://github.com/testcontainers/testcontainers-spring-boot#embedded-redis
Probably then we won't need to worry about stopping that Redis server in the test...
| /** | ||
| * A SpEL expression to use for topic. | ||
| */ | ||
| private Expression topicExpression; |
There was a problem hiding this comment.
I think we have bumped recently an issue that Spring Boot configuration properties binding doesn't support Expression by default.
We can accept them only as strings and parse to Expression manually in the configuration class.
There was a problem hiding this comment.
stream provides conversion of String to SpEL
There was a problem hiding this comment.
Yes, but we are in the function library, we can't have a SCSt dependency over here.
We probably can extract that code into a common utility jar, but we would still need to declare a bean for that @ConfigurationPropertiesBinding...
So, probably less error-prone solution would be with a manual SpEL parsing.
| static { | ||
| GenericContainer redis = new GenericContainer("redis:3-alpine") | ||
| .withExposedPorts(6379); | ||
| redis.start(); |
There was a problem hiding this comment.
You start it here. Where do you stop then?
| @@ -49,7 +54,8 @@ public MessageHandler redisConsumerMessageHandler(RedisConnectionFactory redisCo | |||
| if (redisConsumerProperties.isKeyPresent()) { | |||
| RedisStoreWritingMessageHandler redisStoreWritingMessageHandler = new RedisStoreWritingMessageHandler( | |||
| redisConnectionFactory); | |||
| redisStoreWritingMessageHandler.setKeyExpression(redisConsumerProperties.keyExpression()); | |||
| redisStoreWritingMessageHandler.setKeyExpression( | |||
| new LiteralExpression(redisConsumerProperties.keyExpression())); | |||
There was a problem hiding this comment.
This is wrong. It has to be parsed - it is not a plain literal.
Anyway consider a convenient setKeyExpressionString() instead.
| return redisStoreWritingMessageHandler; | ||
| } | ||
| else if (redisConsumerProperties.isQueuePresent()) { | ||
| return new RedisQueueOutboundChannelAdapter(redisConsumerProperties.queueExpression(), |
There was a problem hiding this comment.
You have missed to parse an expression here.
| @SpringBootTest(properties = {"spring.redis.host=${embedded.redis.host}", | ||
| "spring.redis.port=${embedded.redis.port}", | ||
| "spring.redis.user=${embedded.redis.user}", | ||
| "spring.redis.password=${embedded.redis.password}", |
There was a problem hiding this comment.
Let's try to move all of these embedded properties into an application.properties in the test/resources!
| private RedisConnectionFactory redisConnectionFactory; | ||
|
|
||
| @Test | ||
| public void testWithKey() throws Exception{ |
There was a problem hiding this comment.
Sounds like we need more tests to cover all the possible behavior of the consumer.
If we would have them at the first place, we wouldn't talk about expressions now 😄
No description provided.