Stripes

Custom DateTypeConverter classes mysteriously fail

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: Release 1.4.3
  • Fix Version/s: Release 1.5
  • Component/s: Validation
  • Labels:
    None
  • Environment:
    Solaris 11

Description

I was trying to get Stripes to accept dd/MM/YYYY format dates, so I extended DateTypeConverter as follows:

protected String[] getFormatStrings() { String[] o = super.getFormatStrings(); String[] n = new String[o.length + 1]; n[0] = "dd/MM/yyyy"; System.arraycopy(o, 0, n, 1, o.length); return n; }

And in my Action class I had:

@Validate(converter=my.project.MyDateTypeConverter.class)
private Date startDate;

And it didn't work - it rejected perfectly valid dates such as "31/12/2007". The reason is that DateFormat replaces characters such as "/" in the input string with spaces, but it doesn't do the same to the format strings, so it ends up trying to match "31 12 2007" against "dd/MM/yyyy", which fails.

I'm not sure if this is a documentation bug (i.e. the docs should point out that custom format strings shouldn't include non-space characters), or a code bug (i.e. format strings should be subject to the same replacements as the input string), but it surely is a bug.

Activity

Hide
Frederic Daoud added a comment - 11/Dec/07 7:22 AM

Please refer to <a href="http://stripes.mc4j.org/jira/browse/STS-445">STS-445</a> which hopefully gives an easy way to resolve this issue.

Show
Frederic Daoud added a comment - 11/Dec/07 7:22 AM Please refer to <a href="http://stripes.mc4j.org/jira/browse/STS-445">STS-445</a> which hopefully gives an easy way to resolve this issue.
Hide
Tim Fennell added a comment - 12/Dec/07 4:24 AM

I'm not actually sure what to say to this. From the current javadoc at http://stripes.sourceforge.net/docs/1.4.3/javadoc/ :

protected String[] getFormatString()

Returns an array of format strings that will be used, in order, to try and parse the date.
This method can be overridden to make DateTypeConverter use a different set of format Strings. Two caveats apply:

Given that pre-processing converts most common separator characters into spaces, patterns
should be expressed with spaces as separators, not slashes, hyphens etc.
In all cases getDateFormats() will return formats based on this list preceeded by the standard
SHORT, MEDIUM and LONG formats for the input locale.

I understand that you may disagree with what's being done, but the documentation of the exact method you are overriding is quite specific on this point. We are making some changes for 1.5 and I'll ensure that there's also class level javadoc describing this.

Show
Tim Fennell added a comment - 12/Dec/07 4:24 AM I'm not actually sure what to say to this. From the current javadoc at http://stripes.sourceforge.net/docs/1.4.3/javadoc/ : protected String[] getFormatString() Returns an array of format strings that will be used, in order, to try and parse the date. This method can be overridden to make DateTypeConverter use a different set of format Strings. Two caveats apply: Given that pre-processing converts most common separator characters into spaces, patterns should be expressed with spaces as separators, not slashes, hyphens etc. In all cases getDateFormats() will return formats based on this list preceeded by the standard SHORT, MEDIUM and LONG formats for the input locale. I understand that you may disagree with what's being done, but the documentation of the exact method you are overriding is quite specific on this point. We are making some changes for 1.5 and I'll ensure that there's also class level javadoc describing this.
Hide
Tim Fennell added a comment - 12/Dec/07 4:41 AM

Marking as resolved. See STS-445 for the full description of what's changed. I believe this is now better documented - if you think it's the wrong thing to be doing period, then let's have that discussion

Show
Tim Fennell added a comment - 12/Dec/07 4:41 AM Marking as resolved. See STS-445 for the full description of what's changed. I believe this is now better documented - if you think it's the wrong thing to be doing period, then let's have that discussion
Hide
Alan Burlison added a comment - 12/Dec/07 9:24 AM

I've read the details of the fixes in the latest patch for STS-445, and I think it addresses the issues I was concerned about.

Thanks a bunch!

Show
Alan Burlison added a comment - 12/Dec/07 9:24 AM I've read the details of the fixes in the latest patch for STS-445, and I think it addresses the issues I was concerned about. Thanks a bunch!

People

Vote (0)
Watch (1)

Dates

  • Created:
    22/Nov/07 3:56 PM
    Updated:
    04/Jan/11 2:59 PM
    Resolved:
    12/Dec/07 4:41 AM