Add MQTT5 sample application with integration flows and tests - #365
Add MQTT5 sample application with integration flows and tests#365nmy6452 wants to merge 5 commits into
Conversation
Signed-off-by: Minyoung Noh <nmy6452@naver.com> spring-projects#323
cppwfs
left a comment
cppwfs
left a comment
There was a problem hiding this comment.
Thank you for creating a new PR for this sample. It is much appreciated.
Currently the test fails because you are mixing Junit4 and Junit5. In the comments below we have made recommendations on how to handle this properly.
| @@ -0,0 +1,24 @@ | |||
| <configuration> | |||
There was a problem hiding this comment.
Why are you using the logback configuration here?
There was a problem hiding this comment.
@cppwfs Is it better to delete the logback configuration?
There was a problem hiding this comment.
It can be removed unless there is a particular reason it is needed.
| * @since 5.2 | ||
| * | ||
| */ | ||
| public class BrokerRunning extends TestWatcher { |
There was a problem hiding this comment.
Please remove BrokerRunning as the method for testing the MQTT5 app. Please use the Mosquitto test container as shown here: https://github.com/spring-projects/spring-integration/blob/main/spring-integration-mqtt/src/test/java/org/springframework/integration/mqtt/MqttDslTests.java
| @Bean | ||
| public MqttConnectionOptions mqttConnectionOptions() { | ||
| MqttConnectionOptions options = new MqttConnectionOptions(); | ||
| options.setServerURIs(new String[]{ "tcp://localhost:1883" }); |
There was a problem hiding this comment.
You will need expose host and port with either ConfigurationProperties or SystemProperties instead of the hardcoded value. Especially when using test container because they use dynamic properties to establish ports.
Signed-off-by: nmy6452 <nmy6452@naver.com>
Signed-off-by: nmy6452 <nmy6452@naver.com>
Signed-off-by: nmy6452 <nmy6452@naver.com>
Signed-off-by: nmy6452 <nmy6452@naver.com>
cppwfs
left a comment
cppwfs
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
I have left a few comments. But there are a couple of things that this sample needs as well.
- Rebase to the latest main.
- The sample needs a pom.xml like the others. It can be generated by
./gradlew generatePomFileForMavenJavaPublication
|
|
||
| import org.springframework.messaging.Message; | ||
| import org.springframework.messaging.MessageHeaders; | ||
| import org.springframework.messaging.converter.AbstractMessageConverter; |
There was a problem hiding this comment.
As of 7.1 CharacterStreamReadingMessageSource has moved to org.springframework.integration.stream.inbound.CharacterStreamReadingMessageSource
| protected Object convertToInternal(Object payload, MessageHeaders headers, | ||
| Object conversionHint) { | ||
|
|
||
| return new String((byte[]) payload); |
There was a problem hiding this comment.
This conversion is incorrect, the log message shows the byte array and not the string.. Needs to be something like payload.toString().getBytes(StandardCharsets.UTF_8).
| public IntegrationFlow mqttOutFlow() { | ||
| return IntegrationFlow.from(CharacterStreamReadingMessageSource.stdin(), | ||
| e -> e.poller(Pollers.fixedDelay(1000))) | ||
| .transform(p -> p + " sent to MQTT5") |
There was a problem hiding this comment.
When running the sample after rebasing I see that this output does not show up in the result. Please investigate.
2 participants
Signed-off-by: Minyoung Noh nmy6452@naver.com
Create PR for #323 issues.
I worked on the PR base that was rejected before, please refer to it and review it. #364