trjhtr

I have couple of MessageProcessors. One of the Processor will log the payload. Other Processor will save into an embedded database.

Here is the code I have written so far:

I am using annotation-driven configuration so I have omitted other classes which are not used in this use case.

@Component
public class ProcessorConfiguration 
 @Bean
 @Qualifier("textMp")
 public MessageProcessor textMessageProcessor() 
 return new TextMessageProcessor();
 

 @Bean
 @Qualifier("employMp")
 public MessageProcessor employMessageProcessor() 
 return new EmployMessageProcessor();
 

 @Bean
 private List<MessageProcessor> messageProcessorList(@Qualifier("textMp") MessageProcessor textMessageProcessor,
 @Qualifier("employMp") MessageProcessor employMessageProcessor) 
 List<MessageProcessor> list = new ArrayList<>();
 list.add(textMessageProcessor);
 list.add(employMessageProcessor);
 return list;
 

This class is responsible for all the JMS messages the application receives.

public class MessageHandler 
 @Autowired
 private List<MessageProcessor> messageProcessors;

 public void handleMessage(Notification notification) 
 messageProcessors.forEach(processor -> processor.doProcess(notification));
 


public interface MessageProcessor 
 void doProcess(Notification notification);


public class TextMessageProcessor implements MessageProcessor 
 private static final Logger logger = Logger.getLogger(TextMessageProcessor.class);
 @Override
 public void doProcess(Notification notification) 
 logger.info("The payload is " + notification.getText());
 

I have created a builder which takes String as input and returns an Employ object.

@Service
public class EmployMessageProcessor implements MessageProcessor 
 @Autowired
 private EmployDao dao;

 @Override
 public void doProcess(Notification notification) 
 Employ employ = EmployBuilder.buildEmploy(notification.getText());
 dao.save(employ);
 



public interface Notification 
 String getText();

I think, the way I am injecting the processors can be improved. Please review my code and provide your valuable feedback.

This is a learning exercise for me as well so I hope I can be helpful with my answer.

• I think you can loose the Configuration class entirely. Add
  @Component/@Service annotation to your TextMessageProcessor. Spring should be
  capable of figuring out MessageProcessor List wiring on its own. Have you already tried that?

If you are worried about the order of the beans inside MessageProcessor List then you can use @Order annotation on your Service and/or Component.

@Component
@Order(value=1)
public class TextMessageProcessor implements MessageProcessor 

• Your Configuration class in its current form is conflicting with Open Closed principle. In other words, whenever you add a new MessageProcessor, your configuration will grow. Imagine that configuration after you have 20 processors.



• 
  

  Spring creates qualifier names automatically as your bean names.
  From Spring documentation:

  
  
  

  
  

  For a fallback match, the bean name is considered as a default qualifier value.

  
  

  
  



• I recommend doing your wiring through constructors instead of fields (I am especially eyeballing that DAO of yours). That way you preserve an ability to test your code later on. You will need to inject objects through constructors in your tests.




• You could move Logging to MessageProcessor if it was an abstract class. Make a field or a method for Logging which returns the logging instance for current class. (Stackoverflow link)