Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-3391

Investigate and clean up Rice's date validation/parsing mechanisms.

    Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0.1.1
    • Component/s: Quality Assurance
    • Labels:
      None
    • Similar issues:
      KULRICE-4192Clean up references to "kew.standalone.server" and other out-of-date Rice configuration parameters
      KULRICE-400rice_template clean up work - add new test harness in, ensure that web content is up to date
      KULRICE-228Clean up lib directory
      KULRICE-4066Clean up configuration system
      KULRICE-12814Clean up datasource configuration
      KULRICE-2579Clean up xml bootstrap files
      KULRICE-6047Implement feedback on cleaning up install documentation
      KULRICE-3021Clean up the StandardGenericXMLSearchableAttribute and related code
      KULRICE-4881Clean up utility classes
      KULRICE-2593Review various Spring override points in the codebase and clean them up
    • Rice Team:
      QA

      Description

      Rice's current mechanisms for validating/parsing dates are in need of modification. For instance, as mentioned in the resolution of KULRICE-3227, the doc search for Rice 1.0 currently runs into NullPointerException problems if the entered date values contain an unusual number of digits in various places, or if extra "garbage" characters are appended to the end of the date. Part of the problem is because DateTimeServiceImpl currently relies on Java's SimpleDateFormat class, and according to Java's documentation of this class, SimpleDateFormat may ignore extra text occuring after the date value when parsing (see http://java.sun.com/j2se/1.5.0/docs/api/java/text/SimpleDateFormat.html for more info). Among other things, the DateTimeServiceImpl needs to be changed so that it uses something other than SimpleDateFormat for parsing or validating dates.

      Additional analysis is also needed to identify what other areas of date validation/parsing need cleaning up (and to possibly apply something similar to the processing mechanisms of other non-date values as well).

        Issue Links

          Activity

          Hide
          Eric Westfall added a comment -

          Deferring to 1.0.2

          Show
          Eric Westfall added a comment - Deferring to 1.0.2
          Hide
          Garey Taylor added a comment -

          Dealing with some of the validation work mentioned in this issue.

          Show
          Garey Taylor added a comment - Dealing with some of the validation work mentioned in this issue.
          Hide
          Eric Westfall added a comment -

          Garey said he would have chad go through this and verify whether or not we can resolve this issue.

          Show
          Eric Westfall added a comment - Garey said he would have chad go through this and verify whether or not we can resolve this issue.
          Hide
          Chad Hagstrom added a comment -

          It looks like Garey fixed about all of the date validation problems (including the one related to "garbage" characters after the date string) by having the DateTimeServiceImpl ensure that the parsed date equals the original date string when it is reformatted back into a string. However, with this new validation step in place, the DateTimeServiceImpl cannot parse dates with single-digit days/months that omit the optional zeros, since I believe that the date-reformatting process adds those optional zero digit(s) back into the date, regardless of whether or not the original string included them. For example, "12/04/2009" would pass validation, but "12/4/2009" would not. Is it absolutely necessary for Rice to support such date formats? If not, then I believe this issue is ready to be resolved.

          Show
          Chad Hagstrom added a comment - It looks like Garey fixed about all of the date validation problems (including the one related to "garbage" characters after the date string) by having the DateTimeServiceImpl ensure that the parsed date equals the original date string when it is reformatted back into a string. However, with this new validation step in place, the DateTimeServiceImpl cannot parse dates with single-digit days/months that omit the optional zeros, since I believe that the date-reformatting process adds those optional zero digit(s) back into the date, regardless of whether or not the original string included them. For example, "12/04/2009" would pass validation, but "12/4/2009" would not. Is it absolutely necessary for Rice to support such date formats? If not, then I believe this issue is ready to be resolved.
          Hide
          Chad Hagstrom added a comment -

          After further discussion, it was decided to investigate some alternative date validation means in order to avoid the reformatting comparison problems mentioned earlier. A couple alternatives are already available via the Apache Commons JAR(s) used by Rice, including the DateValidator and DateUtils classes; however, I believe these approaches rely on an underlying SimpleDateFormat instance anyway, which could result in many of the same problems as before. Another possibility is to use the Joda Time API, which is independent of SimpleDateFormat and similar time-related Java SE classes, and which seems to be validating Rice's supported date formats properly (even with single-digit months/days with the optional zeros omitted). On the other hand, I can achieve similar (though not exact) results as those from the Joda API (at least in terms of parsing) if I create a non-lenient SimpleDateFormat, parse a given date string using the "parse" method that also takes a ParsePosition as an argument, and then verify that the ParsePosition's current index equals the length of the original date string (meaning that the entire string was used when parsing the date). And aside from possible differences in two-digit year parsing, the major parsing differences I have noticed between the Joda Time and SimpleDateFormat-with-ParsePosition approaches is that the former allows negative year values to be parsed without errors, while the latter allows for days/months/hours/minutes/seconds of 3 or more digits in length to be parsed without errors as long as those extra digits are zero digits. For example, "10/12/-3" would succeed with Joda Time, and "12/012/2015" would succeed with the SimpleDateFormat-plus-ParsePosition approach. Thus, I do not believe that another API like the Joda API is needed for our date parsing, unless it is absolutely necessary to restrict such non-year numeric date data to two digits or less without the additional aid of regular expressions (even when the third digit or beyond is a zero digit).

          (Also, note that in order for the Joda Time or SimpleDateFormat-with-ParsePosition solutions to work as expected, I believe the KNS parameters containing the valid date/timestamp parsing formats need to have the "MM/dd/yy" and "MM/dd/yy HH:mm:ss" patterns come before the "MM/dd/yyyy" and "MM/dd/yyyy HH:mm:ss" patterns, respectively; otherwise, two-digit years will get interpreted literally instead of being translated into the appropriate century and millenium).

          However, I have been running into at least one other date-related problem as well. For instance, specifying a year of 10000 or higher will not cause validation or SQL construction to fail, but it will cause the SQL execution to fail, thus resulting in a user-unfriendly error message being displayed. Do we need to explicitly enforce a specific date range as being the "allowed" range in our validation code (as opposed to letting the SQL execution throw the error)? And if so, should this range just be the intersections of the allowed Oracle and MySQL date ranges, or do we need a more restrictive range?

          Show
          Chad Hagstrom added a comment - After further discussion, it was decided to investigate some alternative date validation means in order to avoid the reformatting comparison problems mentioned earlier. A couple alternatives are already available via the Apache Commons JAR(s) used by Rice, including the DateValidator and DateUtils classes; however, I believe these approaches rely on an underlying SimpleDateFormat instance anyway, which could result in many of the same problems as before. Another possibility is to use the Joda Time API, which is independent of SimpleDateFormat and similar time-related Java SE classes, and which seems to be validating Rice's supported date formats properly (even with single-digit months/days with the optional zeros omitted). On the other hand, I can achieve similar (though not exact) results as those from the Joda API (at least in terms of parsing) if I create a non-lenient SimpleDateFormat, parse a given date string using the "parse" method that also takes a ParsePosition as an argument, and then verify that the ParsePosition's current index equals the length of the original date string (meaning that the entire string was used when parsing the date). And aside from possible differences in two-digit year parsing, the major parsing differences I have noticed between the Joda Time and SimpleDateFormat-with-ParsePosition approaches is that the former allows negative year values to be parsed without errors, while the latter allows for days/months/hours/minutes/seconds of 3 or more digits in length to be parsed without errors as long as those extra digits are zero digits. For example, "10/12/-3" would succeed with Joda Time, and "12/012/2015" would succeed with the SimpleDateFormat-plus-ParsePosition approach. Thus, I do not believe that another API like the Joda API is needed for our date parsing, unless it is absolutely necessary to restrict such non-year numeric date data to two digits or less without the additional aid of regular expressions (even when the third digit or beyond is a zero digit). (Also, note that in order for the Joda Time or SimpleDateFormat-with-ParsePosition solutions to work as expected, I believe the KNS parameters containing the valid date/timestamp parsing formats need to have the "MM/dd/yy" and "MM/dd/yy HH:mm:ss" patterns come before the "MM/dd/yyyy" and "MM/dd/yyyy HH:mm:ss" patterns, respectively; otherwise, two-digit years will get interpreted literally instead of being translated into the appropriate century and millenium). However, I have been running into at least one other date-related problem as well. For instance, specifying a year of 10000 or higher will not cause validation or SQL construction to fail, but it will cause the SQL execution to fail, thus resulting in a user-unfriendly error message being displayed. Do we need to explicitly enforce a specific date range as being the "allowed" range in our validation code (as opposed to letting the SQL execution throw the error)? And if so, should this range just be the intersections of the allowed Oracle and MySQL date ranges, or do we need a more restrictive range?
          Hide
          Chad Hagstrom added a comment -

          I've found another problem as well. For the regular workflow doc search date fields (like "Date Created From"), it is possible to have dates containing string-specific wildcards (such as "12/11*/2009") and yet still have them pass validation, resulting in a doc search SQL execution failure and a user-unfriendly error message. I believe the problem is that I had configured those regular workflow date fields so that they would be validated by the DictionaryValidationService as part of the work for KULRICE-3227, but now that the DictionaryValidationServiceImpl has been recently modified to only clean the wildcards specific to a validated field's data type, the string-specific wildcards are getting cleaned off of those problematic dates because DocSearchCriteriaDTO currently stores them as strings. (Theoretically, this problem could also affect DD searchable attributes that are validated as dates/numbers but are stored as strings in the business object or document.) Would it be best to just have those regular workflow dates be validated by something other than DictionaryValidationService (like SearchableAttributeDateTimeValue or SqlBuilder or DateTimeService)? Or should DocSearchCriteriaDTO store those date values as actual Date objects instead? Or does the DictionaryValidationService need to change the way that it determines field data types, at least for the purposes of validation?

          Show
          Chad Hagstrom added a comment - I've found another problem as well. For the regular workflow doc search date fields (like "Date Created From"), it is possible to have dates containing string-specific wildcards (such as "12/11*/2009") and yet still have them pass validation, resulting in a doc search SQL execution failure and a user-unfriendly error message. I believe the problem is that I had configured those regular workflow date fields so that they would be validated by the DictionaryValidationService as part of the work for KULRICE-3227 , but now that the DictionaryValidationServiceImpl has been recently modified to only clean the wildcards specific to a validated field's data type, the string-specific wildcards are getting cleaned off of those problematic dates because DocSearchCriteriaDTO currently stores them as strings. (Theoretically, this problem could also affect DD searchable attributes that are validated as dates/numbers but are stored as strings in the business object or document.) Would it be best to just have those regular workflow dates be validated by something other than DictionaryValidationService (like SearchableAttributeDateTimeValue or SqlBuilder or DateTimeService)? Or should DocSearchCriteriaDTO store those date values as actual Date objects instead? Or does the DictionaryValidationService need to change the way that it determines field data types, at least for the purposes of validation?
          Hide
          Chad Hagstrom added a comment -

          Discussed this with Garey. We decided on having the date-validation code enforce a specific year range on dates (1000 to 9999 to be exact), which I now have working locally. We also agreed on attempting to add another DictionaryValidationService method that would allow users to specify the attribute's data type explicitly (for validation purposes, that is), in an effort to correct the problem from my previous post. Such a solution does fix that string-specific wildcard problem, but I have discovered some other problems as well. For instance, adding range characters in odd places in the regular workflow "From" dates (like "12/>=12/09") will result in those unusual range characters being cleaned out during validation and SQL creation, although the corresponding "To" dates do catch these formatting problems at the SQL generation level (see StandardDocumentSearchGenerator.establishDateString). Furthermore, "From" date strings that start with "&&" or "|" will cause exceptions when executing the SQL, and "From" and "To" date strings that start with ".." can cause null pointer exceptions during validation. I will look into these problems (and their solutions) tomorrow.

          Show
          Chad Hagstrom added a comment - Discussed this with Garey. We decided on having the date-validation code enforce a specific year range on dates (1000 to 9999 to be exact), which I now have working locally. We also agreed on attempting to add another DictionaryValidationService method that would allow users to specify the attribute's data type explicitly (for validation purposes, that is), in an effort to correct the problem from my previous post. Such a solution does fix that string-specific wildcard problem, but I have discovered some other problems as well. For instance, adding range characters in odd places in the regular workflow "From" dates (like "12/>=12/09") will result in those unusual range characters being cleaned out during validation and SQL creation, although the corresponding "To" dates do catch these formatting problems at the SQL generation level (see StandardDocumentSearchGenerator.establishDateString). Furthermore, "From" date strings that start with "&&" or "|" will cause exceptions when executing the SQL, and "From" and "To" date strings that start with ".." can cause null pointer exceptions during validation. I will look into these problems (and their solutions) tomorrow.
          Hide
          Garey Taylor added a comment -

          Hi Guys,

          We're planning on adding an extra method to the DictionaryValidationService (described in previous comment), but wanted to check and see if this would cause you any issues. We didn't see any issues on the KFS side, but we are not sure about KC.

          We will hold off the change until tomorrow and I will add a quick agenda item to the KTI so we can chat about this tomorrow if you see any issues.

          Thanks,

          Garey

          Show
          Garey Taylor added a comment - Hi Guys, We're planning on adding an extra method to the DictionaryValidationService (described in previous comment), but wanted to check and see if this would cause you any issues. We didn't see any issues on the KFS side, but we are not sure about KC. We will hold off the change until tomorrow and I will add a quick agenda item to the KTI so we can chat about this tomorrow if you see any issues. Thanks, Garey
          Hide
          Chad Hagstrom added a comment -

          It was decided that the proposed modification to the DictionaryValidationService would be acceptable, so I have committed all the relevant changes for this issue. I will be creating a database issue shortly for making a minor change to the master DBs that is needed for dates containing two-digit years to be processed properly.

          Show
          Chad Hagstrom added a comment - It was decided that the proposed modification to the DictionaryValidationService would be acceptable, so I have committed all the relevant changes for this issue. I will be creating a database issue shortly for making a minor change to the master DBs that is needed for dates containing two-digit years to be processed properly.

            People

            • Assignee:
              Chad Hagstrom
              Reporter:
              Chad Hagstrom
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 3 days
                3d
                Remaining:
                Remaining Estimate - 3 days
                3d
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Structure Helper Panel