Skip to content

Conversation

@dmccoystephenson
Copy link
Member

@dmccoystephenson dmccoystephenson commented Apr 8, 2025

Changes

The following changes affect a subset of files and were not sweeping changes.

Code Quality & Style

  • Reformatted code for consistency
  • Removed unused imports and code
  • Reduced code duplication
  • Simplified logic and improved clarity
  • Added TODOs for future improvements
  • Refactored constructors to use this for clarity
  • Made instance variables private and final where applicable
  • Removed unnecessary instance variables

Logging Enhancements

  • Transitioned from utility.logWithDate and System.out to SLF4J
  • Added @Slf4j annotation
  • Replaced direct stack trace printing with SLF4J logging
  • Added more logging for better observability
  • Updated logging levels for consistency
  • Introduced new environment variable: LOGGING_LEVEL_COM_TRIHYDRO
  • Made ACM_LOG_LEVEL more easily configurable in local deployment

Testing Improvements

  • Added unit tests
  • Introduced Mockito annotations for test support

Dependency & Deployment Updates

  • Pulled in changes from Azure mirror
  • Removed milepost-graph-db directory and related service from local-deployment compose file
  • Updated ODE Docker tags in local compose file to 2025-q1
  • Made updates to reflect renamed fields and structure changes in J2735 2024
  • Updated version to 2.0.0
  • Updated test data to match new topic.OdeTimJson format

Code Behavior & Resource Management

  • Improved frame type resolution in ActiveTimController: now defaults to "advisory" when original value is unusable
  • Refactored DbInteractions.validateDbConfig to reduce nesting by returning early if dataSource is not null
  • Updated resource handling to use try-with-resources for better safety and readability
  • Enhanced incident condition logic to support GVW restriction reporting through the existing "other" problem type:
    • When "other" is selected, the system now checks the problemOtherText field for GVW-related keywords
    • If a GVW restriction is identified, the specified weight is extracted and mapped to the appropriate ITIS code using the current translation mechanism

Documentation

  • Updated README to use placeholder versions for JARs
  • Added note about GMT time usage in sample.env

Testing

  • Unit tests verified to pass
  • Local deployment testing
    • Verified successful startup
    • Verified TIM creation, resubmission & deletion

Other Info

Purpose

This PR proposes merging release/v2.0 into the main branch. The release/v2.0 branch includes the latest updates from both main and dev, which contain additional changes beyond the ODE upgrade.

Version Tag

Once merged, a new version tag will be created to reflect the update.

Major Version Bump

The major version bump is due to the integration of the 2025 Q1 ODE release, which introduces breaking changes, along with other updates from main and dev.

…iolation-tim-rsu-service' into feature/support-gvw-restriction-through-incident-endpoint
…oblemGVW_ShouldReturnGVWItisCodes` unit test
… set instance variables to final and added null check
Sync dev with latest changes from main
Converted the inline comment for `performTaskUsingCron` into a detailed JavaDoc. This enhances clarity by explaining the method's purpose and behavior directly in the documentation.
Adjusted the cron expression to trigger at 1:00 AM daily across multiple files for consistency. Replaced hardcoded value with environment variable reference in local-deployment files.
dmccoystephenson and others added 8 commits April 8, 2025 16:49
…ka-consumer-log-volume

# Conflicts:
#	logger-kafka-consumer/src/main/java/com/trihydro/loggerkafkaconsumer/app/services/DataFrameService.java
…r-log-volume

Structured Logging Improvements in Logger Kafka Consumer
# Conflicts:
#	cv-data-controller/src/main/java/com/trihydro/cvdatacontroller/controller/ActiveTimController.java
#	docker-compose.yml
# Conflicts:
#	docker-compose.yml
Sync dev with latest changes from main (4/8/2025)
…ent-endpoint

# Conflicts:
#	ode-wrapper/src/main/java/com/trihydro/odewrapper/controller/WydotTimIncidentController.java
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

generatedKeys.close();
} catch (Exception e) {
e.printStackTrace();
log.trace("------ Generated {} {} --------------", type, id);
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 log statement was flooding the console. Changing the log level to TRACE ensures it won't appear in the logs under normal circumstances.

Comment on lines -62 to -65
dataFrame.setNotUsed((short) 0); // as of J2735 2020 this should be set to 0 and is ignored
dataFrame.setNotUsed1((short) 0); // as of J2735 2020 this should be set to 0 and is ignored
dataFrame.setNotUsed2((short) 0); // as of J2735 2020 this should be set to 0 and is ignored
dataFrame.setNotUsed3((short) 0); // as of J2735 2020 this should be set to 0 and is ignored
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: Testing has shown that when these values are not provided, the ODE applies default values. Since they are not used, we do not need to set them in the TIM Manager.

Comment on lines +92 to +94
log.trace("Before processing JSON: {}", tdw.getData());
odeData = timDataConverter.processTimJson(tdw.getData());
log.trace("After processing JSON: {}", gson.toJson(odeData));
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 log statements were sometimes overwhelming the console. Looking at the JSON can be useful for debugging though, so maybe these should be DEBUG level instead of TRACE level.

}

@Test
public void processTimJson_unsigned() throws IOException {
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 test case was removed in the CDOT fork and a relevant note can be found at CDOT-CV#7 (comment)

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 file was used in a removed test case that used to be in TimDataConverterTest

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`.
…se-2.0

Address Comments from #49: Improve Logging and JSON Handling
@dmccoystephenson dmccoystephenson marked this pull request as ready for review April 18, 2025 19:40
@dmccoystephenson dmccoystephenson merged commit 23f789f into main Apr 22, 2025
2 checks passed
@dmccoystephenson dmccoystephenson deleted the release/v2.0 branch April 22, 2025 14:34
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.

5 participants