1
\$\begingroup\$

I would greatly appreciate your feedback and review of our Spring Boot Azure Service Bus listener implementation. Your insights and suggestions will help us ensure its efficiency and reliability. Please take a moment to test it and provide your valuable feedback. Your input is invaluable in making our service even better.

I have chosen the JMS approach to monitor the topic for incoming messages from my producer class. However, I want to ensure that I've considered all possible scenarios, such as system downtime or handling failed messages. Your additional insights and review would be highly valuable in optimizing and enhancing this implementation. Your input is greatly appreciated.

Below are my implementation steps

Application YAML

spring
 jms:
 servicebus:
 connection-string: ${AZURE_SERVICE_BUS_ENDPOINT:#{null}}
 pricing-tier: standard
 topic-client-id: ${TOPIC_CLIENT_ID:#{null}}
 listener:
 default-retry:
 enabled: true
 max-attempts: 3
app:
 sfm:
 base-url: "https://sso-int.vit-uk-life-dto.azure.net"
azure:
 service-bus:
 topic: app-int-messages
 subscription: app-sub-messages

JmsConfig configuration class for the listener

@Component
@EnableJms
public class JmsConfig {
@Bean
@ConditionalOnMissingBean(name = "topicJmsListenerContainerFactory")
public DefaultJmsListenerContainerFactory jmsListenerContainerFactory(DefaultJmsListenerContainerFactoryConfigurer Configure,
 ConnectionFactory connectionFactory) {
 DefaultJmsListenerContainerFactory factory = new DefaultJmsListenerContainerFactory();
 SingleConnectionFactory singleConnectionFactory = new SingleConnectionFactory(connectionFactory);
 factory.setConnectionFactory(singleConnectionFactory);
 factory.setSessionAcknowledgeMode(Session.CLIENT_ACKNOWLEDGE);
 Configure.configure(factory, connectionFactory);
 return factory;
}

MessageConsumerService

@Autowired
private SfmService sfmService;
private final ModelMapper modelMapper = new ModelMapper();
@JmsListener(destination = "${app.azure.service-bus.topic}", subscription = "${app.azure.service-bus.subscription}", containerFactory = "topicJmsListenerContainerFactory")
public void onMessage(@Header(JmsHeaders.MESSAGE_ID) String messageId, String message){
 ObjectMapper objectMapper = new ObjectMapper();
 try {
 SendMeasureRequestPayload measureRequestPayload = objectMapper.readValue(message, SendMeasureRequestPayload.class);
 log.info(" consumed :" + measureRequestPayload.getMeasureDate() +" at "+ new Date() + " with message id: " + messageId);
 AccessTokenDTO oidcAccessToken = sfmService.getOIDCAccessToken();
 List<MeasureValues> values = measureValues(measureRequestPayload);
 SendMeasureResponse sendMeasureResponse = Objects.nonNull(oidcAccessToken) ? sfmService.sendMeasures(measureRequestPayload.getMeasureDate(), measureRequestPayload.getMeasureType(), oidcAccessToken, values) : null;
 if(Objects.nonNull(sendMeasureResponse)){
 log.info("Measure response for "+ measureRequestPayload.getMeasureDate() + ":" + sendMeasureResponse.toString());
 }
 }catch (RuntimeException | JsonProcessingException e){
 log.info("Exception occurred in message: "+e.getMessage());
 }
}
private List<MeasureValues> measureValues(SendMeasureRequestPayload payload){
 return payload.getValues()
 .stream()
 .map(sendMeasureValues -> modelMapper.map(sendMeasureValues, MeasureValues.class))
 .collect(Collectors.toList());
}
pacmaninbw
26.2k13 gold badges47 silver badges113 bronze badges
asked Sep 12, 2023 at 9:16
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

At least don't create an ObjectMapper every time in onMessage(), as it's thread-safe and you don't need more instances unless the configuration differs. You can autowire the default objectmapper to the class directly.

Objects.nonNull() is primarily useful for method references, e.g. stream.filter(Objects::nonNull).collect(Collectors.toList()); I would avoid using it manually as a replacement for foo != null, and would concentrate on getting rid of nulls altogether.

You're also doing bad logging, by concatenating the values in your log string. I suggest switching to the professional way, i.e. log.info("Foo was {}, bar was {}", foo, bar); (unless you're using some non-standard logging library).

I would clean out the Objects.nonNull() calls and consider returning Optional<>s in strategic places to avoid all that playing around with nulls your code is doing.

answered Sep 15, 2023 at 10:36
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.