garydgregory commented on PR #436: URL: https://github.com/apache/commons-beanutils/pull/436#issuecomment-5386183087
Hello @rootvector2 I pushed patched up version of this PR. I think this PR can be closed unless you see follow up work needed. The problematic bug I fixed is three instances of double parsing. Instead of going round and round, I applied the diff locally, made the following changes, and pushed to master. Double parsing for the `java.sql.Date` case. Validation parses with the strict formatter and valueOf parses again. Functionally correct, but wasteful. For example: ``` LocalDate.parse(value, SQL_DATE_FORMAT); return type.cast(java.sql.Date.valueOf(value)); ``` The parsed LocalDate is discarded. A more efficient form would be `Date.valueOf(LocalDate.parse(...))`. Double parsing of for the `java.sql.Time` case. Same as above. Double parsing of for the `java.sql.Timestamp` case. Same as above. I added details to Javadocs to clarify intent. I added the follow unit tests: In SqlDateConverterTest testDefaultStringToTypeConvertValidLeapYear testDefaultStringToTypeConvertInvalidNonLeapYearFeb29 testDefaultStringToTypeConvertValidBoundaryDates testDefaultStringToTypeConvertStrictValidationMessage In SqlTimeConverterTest testDefaultStringToTypeConvertValidBoundaryTimes testDefaultStringToTypeConvertInvalidTimeHourMinuteSecond testDefaultStringToTypeConvertStrictValidationMessage In SqlTimestampConverterTest testDefaultStringToTypeConvertValidTimestampWithFraction testDefaultStringToTypeConvertInvalidTimestampOutOfRangeDate testDefaultStringToTypeConvertInvalidTimestampOutOfRangeTime testDefaultStringToTypeConvertStrictValidationMessage These tests cover valid leap-year and boundary values, invalid out-of-range values for all three SQL types, timestamp fraction handling and verification that the new ConversionException messages mention strict validation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
