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());
}
\$\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.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.