-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding JMS Consumer/Publisher Client #14
base: master
Are you sure you want to change the base?
Conversation
|
||
import org.json.JSONObject; | ||
|
||
import javax.jms.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we remove the wild card imports
* Read configurations from config.xml file. | ||
*/ | ||
public class ReadConfigFile { | ||
private Properties properties = new Properties(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this final
/** | ||
* Read configurations from config.xml file. | ||
*/ | ||
public class ReadConfigFile { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better if we change the name of the class since this is more like a method name. You can try a name like ConfigFileReader so something like that
public ReadConfigFile() { | ||
|
||
// Load properties from external file if it exists | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we move this to try with resource ?
// Load default properties from JAR if not overridden externally | ||
if (properties.isEmpty()) { | ||
System.out.println("Reading internal config file"); | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try with resource
} | ||
} catch (JMSException e) { | ||
System.out.print("Error While Reading The JMS Message " + e.getMessage()); | ||
} catch (JsonProcessingException e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since both the catches has the same output and JsonProcessingException is no needed you can remove that
@Override | ||
public void onMessage(Message message) { | ||
try { | ||
Topic jmsDestination = (Topic) message.getJMSDestination(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't find any usage of this. You can remove this
System.out.println("-------- EVENT RECEIVED -------------"); | ||
System.out.println("EVENT RECEIVED :" + payloadData); | ||
String event = payloadData.get("event").asText(); | ||
if (event != null && !event.isEmpty()){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is String you can use Strings.isNullOrEmpty(event)
if (message instanceof TextMessage) { | ||
String textMessage = ((TextMessage) message).getText(); | ||
JsonNode payloadData = new ObjectMapper().readTree(textMessage).path("event").path("payloadData"); | ||
System.out.println("-------- EVENT RECEIVED -------------"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reformat the code
protected String password = "admin"; | ||
protected String topicName = "notification"; | ||
|
||
private void readProperties(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we break the rule "Single Responsibility" JMSBase is a common class and we are using to create the Publisher and Reciver. They don't need to know how the properties are extracted, you can simply move this logic to the ReadConfgFile class
Description
This pull request introduces a JMS Consumer/Publisher Client tool to address situations where JMS events might not be consumed correctly by the gateway, as indicated by the gateway listener debug logs. Even though the event publisher log adaptor is configured for the relevant stream, showing that the message was received in the event stream, it doesn't necessarily confirm the proper publication of the event to the broker through the JMSeventPublisher. This tool provides a solution by allowing manual verification of event publication and consumption, aiding in identifying any issues between the JMS publisher flow and the consumption flow.
Changes Made