MobilityData / gtfs-realtime-validator

Java-based tool that validates General Transit Feed Specification (GTFS)-realtime feeds
Other
38 stars 9 forks source link

fix: Bump Hibernate to 5.6.7.Final, use only named parameters in queries #140

Closed barbeau closed 2 years ago

barbeau commented 2 years ago

Summary:

This PR bumps the Hibernate version to 5.6.7.Final. However, errors are encountered:

javax.servlet.ServletException: java.lang.IllegalArgumentException: Could not locate ordinal parameter [0], expecting one of [1, 2, 3]
...

Full stack trace is detailed in https://github.com/MobilityData/gtfs-realtime-validator/issues/139.

EDIT - Problems seem to be solved by using named parameters only in queries - see https://github.com/MobilityData/gtfs-realtime-validator/pull/140#issuecomment-1076840004.

Closes #139.

Expected behavior:

Everything should work as it previously did on the main branch - website should show validation results. However, the website currently appears blank.

Please make sure these boxes are checked before submitting your pull request - thanks!

barbeau commented 2 years ago

There were two issues, apparently starting with Hibernate 5.4:

  1. Ordinal positional parameters (i.e., with numbers) must start at 1. Ours started at 0, and also just had a ? instead of a number too like ?1 in the query itself. See https://stackoverflow.com/questions/68619839/why-does-could-not-locate-ordinal-parameter-0-expecting-one-of-happen.
  2. If we update the ordinal parameters to start at 1 and the queries to add numbers after ?, we get another error "org.hibernate.engine.query.ParameterRecognitionException: Mixed parameter strategies - use just one of named, positional or JPA-ordinal strategy". See https://docs.jboss.org/hibernate/orm/5.4/userguide/html_single/Hibernate_User_Guide.html#jpql-api.

So, in this patch we update everything to use named parameters instead of a mix of ordinal and named parameters, which appears to solve both issues.

I tested on Windows 10 with JDK 17.0.2 and the webapp seems to work ok now, and the test all pass.