-
Notifications
You must be signed in to change notification settings - Fork 1
Adjust TIM Pathing To Use The ArcGIS API #27
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
base: dev
Are you sure you want to change the base?
Conversation
Move ODE dependencies and add jpo-asn-pojos dependency
…rInputData directly
…mcook42/feat/asn1-pojos-cv-data-service
…dependency injection
…n DataControllerConfigProperties
…d in MilepostDbImplTest
| @@ -0,0 +1,23 @@ | |||
| package com.trihydro.cvdatacontroller.model.Measure; | |||
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.
chore(blocking): package names should be lowercased. Measure -> measure (https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html)
note: this applies elsewhere, too. I can see at least Route needs to be updated, too, but I'd look for other places in this PR where this naming convention needs to be followed
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.
ah, those were combined into a GisResponse package. I went ahead and updated it so that the package is lowercase.
cv-data-controller/src/main/java/com/trihydro/cvdatacontroller/services/GISMilepostImpl.java
Outdated
Show resolved
Hide resolved
cv-data-controller/src/main/java/com/trihydro/cvdatacontroller/services/MilepostDbService.java
Outdated
Show resolved
Hide resolved
cv-data-controller/src/main/java/com/trihydro/cvdatacontroller/services/MilepostDbService.java
Show resolved
Hide resolved
|
|
||
| private void setupWydotTim() { | ||
| wydotTim = new WydotTim(); | ||
| private WydotTim setupWydotTim() { |
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.
praise: great choice to add a return value to this method so wydotTim is changed from a global variable to a local variable in the tests
…es, and logging to align with standard conventions
| ResponseEntity<String> startMeasureDetailsJson = gisConnector.getMeasureAtPoint(startLong, startLat); | ||
| GisResponse startGisResponseDetails = | ||
| objectMapper.readValue(startMeasureDetailsJson.getBody(), GisResponse.class); | ||
| String startRoute = startGisResponseDetails.getFeatures().get(0).getAttributes().getRoute(); |
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.
suggestion: this is a pretty deep chain with geting features, then the one, then attributes, then route. we'll want to add some null checks in here to prevent errors
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.
i added a check for those null values and added a log statement to prevent those exceptions
| var attributes = gisResponseDetails.getFeatures().get(0).getAttributes(); | ||
| double measure = attributes.getMeasure(); | ||
| double bufferMeasure; | ||
| if (route.toLowerCase().endsWith("_dec")) { |
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.
question: does this handle edge cases? ie when we're at measure 0 and on a _deccase, what happens? Likewise if we run into the end of a roadway, what happens?
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.
yes it does. If it runs into the end of a roadway, it stops at the end of the road way. And likewise for the 0 measure. That's where the logic below comes into play where we buffer it and then check if the buffered measure is less then or greater then the MMIN or MMAX and if so, set it to the MMIN or MMAX. I didn't have a unit test addressing it, so I added a unit test to test buffering beyond those values.
| import com.trihydro.library.model.WydotTim; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.beans.factory.annotation.Autowired; |
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.
suggestion: unused import
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.
fixed
dmccoystephenson
left a comment
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.
This looks good to me! The unit tests pass and the apps start up successfully after copying the local-deployment sample.env. I was able to verify that the two REST bodies provided in the PR description resulted in successful submission to the ODE and that TIM JSON showed up in the topic.OdeTimJson topic.
Note: The logger-kafka-consumer module fails to process the TIM, but this problem was not introduced in this PR and resolving it may be outside of the scope:
logger-kafka-consumer-1 | 2026-01-08 17:46:31 [main] [,] ERROR JsonToJavaConverter - A NullPointerException occurred while converting TMC TIM JSON to Java: Cannot invoke "com.fasterxml.jackson.databind.JsonNode.asInt()" because the return value of "com.fasterxml.jackson.databind.JsonNode.get(String)" is null
logger-kafka-consumer-1 | 2026-01-08 17:46:31 [main] [,] ERROR LoggerKafkaConsumer - Failed to parse topic.OdeTimJson message, database insertion skipped
…sponse` model for route handling
…onse` model and improve readability
…n values and checks in the GIS Connector
PR Details
Description
The milepost db was previously removed since it contained no Colorado mileposts, but it was used in the creation of a TIM path. Since removal, inserted TIMs have been unable to generate a path and could not be inserted into the database. This PR updates the TIM pathing logic to use CDOTs ArcGIS endpoints to generate a path for the TIMs in Colorado.
A configuration has been added to easily switch between the WyDOTs Milepost db and CDOTs ArcGIS services via a new milepostProvider environment variable.
How Has This Been Tested?
All unit tests were verified to pass. Integration testing was preformed to ensure a local deployment is easily spined up. Vsl, rw, rc, cc, parking, and incident TIMs were inserted into the db in order to verify a route was created. A single point incident was inserted to verify functionality of single point buffering.
Example TIMs sent through:
{ "timParkingList": [ { "direction": "I", "startPoint": { "latitude": 39.613221750299076, "longitude": -104.89570514902218, "valid": true }, "endPoint": { "latitude": 40.73654117648727, "longitude": -104.99349024587299, "valid": true }, "route": "025A_DEC", "roadCode": "1", "advisory": [ "3" ], "segment": "2", "clientId": "testid", "itisCodes": [ "12559" ], "mileMarker": 1.0, "availability": 1, "exit": "12559" } ] }{ "timIncidentList": [ { "direction": "I", "startPoint": { "latitude": 39.613221750299076, "longitude": -104.89570514902218, "valid": true }, "route": "025A_DEC", "roadCode": "1", "advisory": [ "3" ], "segment": "2", "clientId": "testid", "itisCodes": [ "12559" ], "problem": "mudslide", "effect": "test", "action": "stop", "incidentId": "test", "highway": "025A_DEC", "pk": 1 } ] }It was also tested for switching the provider to the db and verified it attempted to use the milepost graph db. Note: no WyDOT milepost integration testing was performed beyond those in the unit tests.
Types of changes
Checklist: