Skip to content

Conversation

@dmccoystephenson
Copy link
Member

@dmccoystephenson dmccoystephenson commented Apr 9, 2025

This PR addresses comments on #49.

Summary of Changes

  • Added warning-level log statements for cases where elements are not found during JSON conversion
  • Set a default frameType of 'advisory' when the field is missing in the input JSON
  • Replaced System.out usage with SLF4J logging for consistency and better log management
  • Removed unused methods and imports to clean up the codebase
  • Marked applicable instance variable as final to improve immutability
  • Moved TIM test data to an external JSON file for better organization and reuse
  • Updated local deployment setup to allow configuration of ACM_LOG_LEVEL via the .env file

Testing

  • Unit tests verified to pass

Added a fallback mechanism in JsonToJavaConverter to handle cases where frameType is not present in the TravelerDataFrame during the conversion process. A warning is logged, and the frameType defaults to 'advisory' to ensure proper handling and avoid potential null values.
Removed `convertJ2735SpecialVehicleExtensionsJsonToJava` and `convertJ2735SupplementalVehicleExtensionsJsonToJava` methods, as well as their associated imports.
Marking the ObjectMapper as final ensures its immutability and prevents accidental reassignment. This change improves code stability and readability, aligning with best practices.
Moved mock OdeTravelerInformationMessage JSON content to an external file for better readability and maintainability. Updated test methods and helper functions to read the JSON data from the new file, ensuring consistency and reducing code duplication. Adjusted method signatures to handle IOException as needed.
Updated `docker-compose.yml` to use the `ACM_LOG_LEVEL` value from the `.env` file for better configurability. Added `ACM_LOG_LEVEL` to `sample.env` with a default value of `ERROR`.
@dmccoystephenson dmccoystephenson requested a review from mcook42 April 9, 2025 21:31
@dmccoystephenson dmccoystephenson changed the title Pr/addressing comments for release 2.0 Address Comments from #49: Improve Logging and JSON Handling Apr 9, 2025
@dmccoystephenson dmccoystephenson marked this pull request as ready for review April 9, 2025 21:33
Copy link
Member Author

@dmccoystephenson dmccoystephenson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review

Comment on lines -51 to -80
public J2735SpecialVehicleExtensions convertJ2735SpecialVehicleExtensionsJsonToJava(String value, int i) {

JsonNode part2Node = getPart2Node(value, i);
J2735SpecialVehicleExtensions spve = null;
try {
spve = mapper.treeToValue(part2Node, J2735SpecialVehicleExtensions.class);
} catch (JsonProcessingException e) {
e.printStackTrace();
}
return spve;
}

public J2735SupplementalVehicleExtensions convertJ2735SupplementalVehicleExtensionsJsonToJava(String value, int i) {

JsonNode part2Node = getPart2Node(value, i);
J2735SupplementalVehicleExtensions suve = null;
try {
suve = mapper.treeToValue(part2Node, J2735SupplementalVehicleExtensions.class);
} catch (JsonProcessingException e) {
e.printStackTrace();
}
return suve;
}

public JsonNode getPart2Node(String value, int i) {
JsonNode part2 = JsonUtils.getJsonNode(value, "payload").get("data").get("partII");
if (part2 != null)
return part2.get(i).get("value");
return null;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: These methods were not used

}
}
// System.out.println(metaDataNode);
log.trace("MetaDataNode: {}", metaDataNode);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This was a commented out System.out print and has been changed to a TRACE level log.

Copy link
Contributor

@payneBrandon payneBrandon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@payneBrandon payneBrandon merged commit a035a84 into release/v2.0 Apr 18, 2025
2 checks passed
@payneBrandon payneBrandon deleted the pr/addressing-comments-for-release-2.0 branch April 18, 2025 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants