You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ch...@apache.org on 2014/04/23 18:02:52 UTC

svn commit: r1589446 - in /commons/proper/lang/trunk/src: main/java/org/apache/commons/lang3/time/FastDateParser.java test/java/org/apache/commons/lang3/time/FastDateParserTest.java

Author: chas
Date: Wed Apr 23 16:02:52 2014
New Revision: 1589446

URL: http://svn.apache.org/r1589446
Log:
LANG-966 - FastDateParser should be case insensitive

Modified:
    commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java
    commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java

Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java?rev=1589446&r1=1589445&r2=1589446&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java (original)
+++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java Wed Apr 23 16:02:52 2014
@@ -25,6 +25,7 @@ import java.text.ParsePosition;
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -586,6 +587,7 @@ public class FastDateParser implements D
      private static class TextStrategy extends Strategy {
         private final int field;
         private final Map<String, Integer> keyValues;
+        private final Map<String, Integer> lKeyValues;
 
         /**
          * Construct a Strategy that parses a Text field
@@ -596,6 +598,11 @@ public class FastDateParser implements D
         TextStrategy(final int field, final Calendar definingCalendar, final Locale locale) {
             this.field= field;
             this.keyValues= getDisplayNames(field, definingCalendar, locale);
+            this.lKeyValues= new HashMap<String,Integer>();
+
+            for(Map.Entry<String, Integer> entry : keyValues.entrySet()) {
+            	lKeyValues.put(entry.getKey().toLowerCase(), entry.getValue());
+            }
         }
 
         /**
@@ -603,7 +610,7 @@ public class FastDateParser implements D
          */
         @Override
         boolean addRegex(final FastDateParser parser, final StringBuilder regex) {
-            regex.append('(');
+            regex.append("((?i)(?u)");
             for(final String textKeyValue : keyValues.keySet()) {
                 escapeRegex(regex, textKeyValue, false).append('|');
             }
@@ -616,7 +623,7 @@ public class FastDateParser implements D
          */
         @Override
         void setCalendar(final FastDateParser parser, final Calendar cal, final String value) {
-            final Integer iVal = keyValues.get(value);
+            final Integer iVal = lKeyValues.get(value.toLowerCase());
             if(iVal == null) {
                 final StringBuilder sb= new StringBuilder(value);
                 sb.append(" not in (");

Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java?rev=1589446&r1=1589445&r2=1589446&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java (original)
+++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java Wed Apr 23 16:02:52 2014
@@ -540,4 +540,16 @@ public class FastDateParserTest {
         final DateParser parser= getInstance(yMdHmsSZ, REYKJAVIK);
         assertEquals(REYKJAVIK, parser.getTimeZone());
     }
+    
+    @Test
+    public void testLang996() throws ParseException {
+        Calendar expected = Calendar.getInstance(NEW_YORK, Locale.US);
+        expected.clear();
+        expected.set(2014, 4, 14);
+
+        final DateParser fdp = getInstance("ddMMMyyyy", NEW_YORK, Locale.US);        
+        assertEquals(expected.getTime(), fdp.parse("14may2014"));
+        assertEquals(expected.getTime(), fdp.parse("14MAY2014"));
+        assertEquals(expected.getTime(), fdp.parse("14May2014"));
+    }
 }



Re: svn commit: r1589446 - in /commons/proper/lang/trunk/src: main/java/org/apache/commons/lang3/time/FastDateParser.java test/java/org/apache/commons/lang3/time/FastDateParserTest.java

Posted by sebb <se...@gmail.com>.
On 24 April 2014 16:58, Honton, Charles <Ch...@intuit.com> wrote:
> TextStrategy is only for parsing finite set of string choices. Literal
> text is handled by CopyQuotedStrategy.
>

I see.

In which case I suggest renaming it to reflect its new case-blind behaviour.
Also remove keyValues - it wastes space.

>
> On 4/23/14, 6:14 PM, "sebb" <se...@gmail.com> wrote:
>
>>On 23 April 2014 21:19, Honton, Charles <Ch...@intuit.com> wrote:
>>> TextStrategy is used for:
>>> E - DAY_OF_WEEK
>>> G - ERA
>>> M - MONTH
>>> a - AM_PM
>>
>>Is that the only possible use of TextStrategy?
>>What about literal text?
>>
>>> SimpleDateFormat uses case-insensitive parsing for each of these fields.
>>> I will add tests for each of those fields in multiple Locales.
>>
>>Thanks.
>>
>>> The (?u)(?i) modifier is active just for the duration of the group.
>>
>>I did not know that.
>>Eventually found it documented but hidden away in the section on
>>differences from Perl.
>>
>>Note: could be written as (?iu)
>>
>>> Consider the following unit test:
>>>
>>> @Test
>>>     public void testCaseSensitiveModifier() throws Exception {
>>>    Pattern aabb = Pattern.compile("((?u)(?i)AA)BB");
>>>    assertTrue(aabb.matcher("aaBB").matches());
>>>    assertTrue(aabb.matcher("AABB").matches());
>>>    assertFalse(aabb.matcher("aabb").matches());
>>>    assertFalse(aabb.matcher("AAbb").matches());
>>> }
>>>
>>> Regards,
>>> chas
> . . .
>>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1589446 - in /commons/proper/lang/trunk/src: main/java/org/apache/commons/lang3/time/FastDateParser.java test/java/org/apache/commons/lang3/time/FastDateParserTest.java

Posted by Benedikt Ritter <br...@apache.org>.
In any case, please remember to add your changes to changes xml.


2014-04-24 17:58 GMT+02:00 Honton, Charles <Ch...@intuit.com>:

> TextStrategy is only for parsing finite set of string choices. Literal
> text is handled by CopyQuotedStrategy.
>
>
> On 4/23/14, 6:14 PM, "sebb" <se...@gmail.com> wrote:
>
> >On 23 April 2014 21:19, Honton, Charles <Ch...@intuit.com>
> wrote:
> >> TextStrategy is used for:
> >> E - DAY_OF_WEEK
> >> G - ERA
> >> M - MONTH
> >> a - AM_PM
> >
> >Is that the only possible use of TextStrategy?
> >What about literal text?
> >
> >> SimpleDateFormat uses case-insensitive parsing for each of these fields.
> >> I will add tests for each of those fields in multiple Locales.
> >
> >Thanks.
> >
> >> The (?u)(?i) modifier is active just for the duration of the group.
> >
> >I did not know that.
> >Eventually found it documented but hidden away in the section on
> >differences from Perl.
> >
> >Note: could be written as (?iu)
> >
> >> Consider the following unit test:
> >>
> >> @Test
> >>     public void testCaseSensitiveModifier() throws Exception {
> >>    Pattern aabb = Pattern.compile("((?u)(?i)AA)BB");
> >>    assertTrue(aabb.matcher("aaBB").matches());
> >>    assertTrue(aabb.matcher("AABB").matches());
> >>    assertFalse(aabb.matcher("aabb").matches());
> >>    assertFalse(aabb.matcher("AAbb").matches());
> >> }
> >>
> >> Regards,
> >> chas
> . . .
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter

Re: svn commit: r1589446 - in /commons/proper/lang/trunk/src: main/java/org/apache/commons/lang3/time/FastDateParser.java test/java/org/apache/commons/lang3/time/FastDateParserTest.java

Posted by "Honton, Charles" <Ch...@intuit.com>.
TextStrategy is only for parsing finite set of string choices. Literal
text is handled by CopyQuotedStrategy.


On 4/23/14, 6:14 PM, "sebb" <se...@gmail.com> wrote:

>On 23 April 2014 21:19, Honton, Charles <Ch...@intuit.com> wrote:
>> TextStrategy is used for:
>> E - DAY_OF_WEEK
>> G - ERA
>> M - MONTH
>> a - AM_PM
>
>Is that the only possible use of TextStrategy?
>What about literal text?
>
>> SimpleDateFormat uses case-insensitive parsing for each of these fields.
>> I will add tests for each of those fields in multiple Locales.
>
>Thanks.
>
>> The (?u)(?i) modifier is active just for the duration of the group.
>
>I did not know that.
>Eventually found it documented but hidden away in the section on
>differences from Perl.
>
>Note: could be written as (?iu)
>
>> Consider the following unit test:
>>
>> @Test
>>     public void testCaseSensitiveModifier() throws Exception {
>>    Pattern aabb = Pattern.compile("((?u)(?i)AA)BB");
>>    assertTrue(aabb.matcher("aaBB").matches());
>>    assertTrue(aabb.matcher("AABB").matches());
>>    assertFalse(aabb.matcher("aabb").matches());
>>    assertFalse(aabb.matcher("AAbb").matches());
>> }
>>
>> Regards,
>> chas
. . .
>>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1589446 - in /commons/proper/lang/trunk/src: main/java/org/apache/commons/lang3/time/FastDateParser.java test/java/org/apache/commons/lang3/time/FastDateParserTest.java

Posted by sebb <se...@gmail.com>.
On 23 April 2014 21:19, Honton, Charles <Ch...@intuit.com> wrote:
> TextStrategy is used for:
> E - DAY_OF_WEEK
> G - ERA
> M - MONTH
> a - AM_PM

Is that the only possible use of TextStrategy?
What about literal text?

> SimpleDateFormat uses case-insensitive parsing for each of these fields.
> I will add tests for each of those fields in multiple Locales.

Thanks.

> The (?u)(?i) modifier is active just for the duration of the group.

I did not know that.
Eventually found it documented but hidden away in the section on
differences from Perl.

Note: could be written as (?iu)

> Consider the following unit test:
>
> @Test
>     public void testCaseSensitiveModifier() throws Exception {
>    Pattern aabb = Pattern.compile("((?u)(?i)AA)BB");
>    assertTrue(aabb.matcher("aaBB").matches());
>    assertTrue(aabb.matcher("AABB").matches());
>    assertFalse(aabb.matcher("aabb").matches());
>    assertFalse(aabb.matcher("AAbb").matches());
> }
>
> Regards,
> chas
>
>
>
> On 4/23/14, 12:04 PM, "sebb" <se...@gmail.com> wrote:
>
>>On 23 April 2014 17:02,  <ch...@apache.org> wrote:
>>> Author: chas
>>> Date: Wed Apr 23 16:02:52 2014
>>> New Revision: 1589446
>>>
>>> URL: http://svn.apache.org/r1589446
>>> Log:
>>> LANG-966 - FastDateParser should be case insensitive
>>>
>>> Modified:
>>>
>>>commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/Fas
>>>tDateParser.java
>>>
>>>commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/Fas
>>>tDateParserTest.java
>>>
>>> Modified:
>>>commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/Fas
>>>tDateParser.java
>>> URL:
>>>http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/
>>>apache/commons/lang3/time/FastDateParser.java?rev=1589446&r1=1589445&r2=1
>>>589446&view=diff
>>>
>>>=========================================================================
>>>=====
>>> ---
>>>commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/Fas
>>>tDateParser.java (original)
>>> +++
>>>commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/Fas
>>>tDateParser.java Wed Apr 23 16:02:52 2014
>>> @@ -25,6 +25,7 @@ import java.text.ParsePosition;
>>>  import java.util.ArrayList;
>>>  import java.util.Calendar;
>>>  import java.util.Date;
>>> +import java.util.HashMap;
>>>  import java.util.List;
>>>  import java.util.Locale;
>>>  import java.util.Map;
>>> @@ -586,6 +587,7 @@ public class FastDateParser implements D
>>>       private static class TextStrategy extends Strategy {
>>>          private final int field;
>>>          private final Map<String, Integer> keyValues;
>>> +        private final Map<String, Integer> lKeyValues;
>>
>>This is an extra data area for ALL Text fields, but is only needed for
>>Month parsing.
>>
>>I think a separate class is needed.
>>
>>>          /**
>>>           * Construct a Strategy that parses a Text field
>>> @@ -596,6 +598,11 @@ public class FastDateParser implements D
>>>          TextStrategy(final int field, final Calendar definingCalendar,
>>>final Locale locale) {
>>>              this.field= field;
>>>              this.keyValues= getDisplayNames(field, definingCalendar,
>>>locale);
>>> +            this.lKeyValues= new HashMap<String,Integer>();
>>> +
>>> +            for(Map.Entry<String, Integer> entry :
>>>keyValues.entrySet()) {
>>> +               lKeyValues.put(entry.getKey().toLowerCase(),
>>>entry.getValue());
>>> +            }
>>>          }
>>>
>>>          /**
>>> @@ -603,7 +610,7 @@ public class FastDateParser implements D
>>>           */
>>>          @Override
>>>          boolean addRegex(final FastDateParser parser, final
>>>StringBuilder regex) {
>>> -            regex.append('(');
>>> +            regex.append("((?i)(?u)");
>>>              for(final String textKeyValue : keyValues.keySet()) {
>>>                  escapeRegex(regex, textKeyValue, false).append('|');
>>
>>-1
>>
>>AFAICT, this will enable incorrect matching of text fields that are
>>not supposed to be case-blind.
>>Also, the case-blind matching is not disabled at the end of the
>>section, so can affect later matches.
>>
>>Using a separate class for case-blind matching will fix the first
>>issue, and will avoid creating duplicate data unnecessarily.
>>
>>>              }
>>> @@ -616,7 +623,7 @@ public class FastDateParser implements D
>>>           */
>>>          @Override
>>>          void setCalendar(final FastDateParser parser, final Calendar
>>>cal, final String value) {
>>> -            final Integer iVal = keyValues.get(value);
>>> +            final Integer iVal = lKeyValues.get(value.toLowerCase());
>>>              if(iVal == null) {
>>>                  final StringBuilder sb= new StringBuilder(value);
>>>                  sb.append(" not in (");
>>>
>>> Modified:
>>>commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/Fas
>>>tDateParserTest.java
>>> URL:
>>>http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/
>>>apache/commons/lang3/time/FastDateParserTest.java?rev=1589446&r1=1589445&
>>>r2=1589446&view=diff
>>>
>>>=========================================================================
>>>=====
>>> ---
>>>commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/Fas
>>>tDateParserTest.java (original)
>>> +++
>>>commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/Fas
>>>tDateParserTest.java Wed Apr 23 16:02:52 2014
>>> @@ -540,4 +540,16 @@ public class FastDateParserTest {
>>>          final DateParser parser= getInstance(yMdHmsSZ, REYKJAVIK);
>>>          assertEquals(REYKJAVIK, parser.getTimeZone());
>>>      }
>>> +
>>> +    @Test
>>> +    public void testLang996() throws ParseException {
>>> +        Calendar expected = Calendar.getInstance(NEW_YORK, Locale.US);
>>> +        expected.clear();
>>> +        expected.set(2014, 4, 14);
>>> +
>>> +        final DateParser fdp = getInstance("ddMMMyyyy", NEW_YORK,
>>>Locale.US);
>>> +        assertEquals(expected.getTime(), fdp.parse("14may2014"));
>>> +        assertEquals(expected.getTime(), fdp.parse("14MAY2014"));
>>> +        assertEquals(expected.getTime(), fdp.parse("14May2014"));
>>> +    }
>>
>>Need more tests to show that the parsing of other case-significant
>>dates is not affected.
>>
>>>  }
>>>
>>>
>>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>For additional commands, e-mail: dev-help@commons.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1589446 - in /commons/proper/lang/trunk/src: main/java/org/apache/commons/lang3/time/FastDateParser.java test/java/org/apache/commons/lang3/time/FastDateParserTest.java

Posted by "Honton, Charles" <Ch...@intuit.com>.
TextStrategy is used for:
E - DAY_OF_WEEK
G - ERA
M - MONTH
a - AM_PM 

SimpleDateFormat uses case-insensitive parsing for each of these fields.
I will add tests for each of those fields in multiple Locales.

The (?u)(?i) modifier is active just for the duration of the group.
Consider the following unit test:

@Test
    public void testCaseSensitiveModifier() throws Exception {
   Pattern aabb = Pattern.compile("((?u)(?i)AA)BB");
   assertTrue(aabb.matcher("aaBB").matches());
   assertTrue(aabb.matcher("AABB").matches());
   assertFalse(aabb.matcher("aabb").matches());
   assertFalse(aabb.matcher("AAbb").matches());
}

Regards,
chas



On 4/23/14, 12:04 PM, "sebb" <se...@gmail.com> wrote:

>On 23 April 2014 17:02,  <ch...@apache.org> wrote:
>> Author: chas
>> Date: Wed Apr 23 16:02:52 2014
>> New Revision: 1589446
>>
>> URL: http://svn.apache.org/r1589446
>> Log:
>> LANG-966 - FastDateParser should be case insensitive
>>
>> Modified:
>>     
>>commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/Fas
>>tDateParser.java
>>     
>>commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/Fas
>>tDateParserTest.java
>>
>> Modified: 
>>commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/Fas
>>tDateParser.java
>> URL: 
>>http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/
>>apache/commons/lang3/time/FastDateParser.java?rev=1589446&r1=1589445&r2=1
>>589446&view=diff
>> 
>>=========================================================================
>>=====
>> --- 
>>commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/Fas
>>tDateParser.java (original)
>> +++ 
>>commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/Fas
>>tDateParser.java Wed Apr 23 16:02:52 2014
>> @@ -25,6 +25,7 @@ import java.text.ParsePosition;
>>  import java.util.ArrayList;
>>  import java.util.Calendar;
>>  import java.util.Date;
>> +import java.util.HashMap;
>>  import java.util.List;
>>  import java.util.Locale;
>>  import java.util.Map;
>> @@ -586,6 +587,7 @@ public class FastDateParser implements D
>>       private static class TextStrategy extends Strategy {
>>          private final int field;
>>          private final Map<String, Integer> keyValues;
>> +        private final Map<String, Integer> lKeyValues;
>
>This is an extra data area for ALL Text fields, but is only needed for
>Month parsing.
>
>I think a separate class is needed.
>
>>          /**
>>           * Construct a Strategy that parses a Text field
>> @@ -596,6 +598,11 @@ public class FastDateParser implements D
>>          TextStrategy(final int field, final Calendar definingCalendar,
>>final Locale locale) {
>>              this.field= field;
>>              this.keyValues= getDisplayNames(field, definingCalendar,
>>locale);
>> +            this.lKeyValues= new HashMap<String,Integer>();
>> +
>> +            for(Map.Entry<String, Integer> entry :
>>keyValues.entrySet()) {
>> +               lKeyValues.put(entry.getKey().toLowerCase(),
>>entry.getValue());
>> +            }
>>          }
>>
>>          /**
>> @@ -603,7 +610,7 @@ public class FastDateParser implements D
>>           */
>>          @Override
>>          boolean addRegex(final FastDateParser parser, final
>>StringBuilder regex) {
>> -            regex.append('(');
>> +            regex.append("((?i)(?u)");
>>              for(final String textKeyValue : keyValues.keySet()) {
>>                  escapeRegex(regex, textKeyValue, false).append('|');
>
>-1
>
>AFAICT, this will enable incorrect matching of text fields that are
>not supposed to be case-blind.
>Also, the case-blind matching is not disabled at the end of the
>section, so can affect later matches.
>
>Using a separate class for case-blind matching will fix the first
>issue, and will avoid creating duplicate data unnecessarily.
>
>>              }
>> @@ -616,7 +623,7 @@ public class FastDateParser implements D
>>           */
>>          @Override
>>          void setCalendar(final FastDateParser parser, final Calendar
>>cal, final String value) {
>> -            final Integer iVal = keyValues.get(value);
>> +            final Integer iVal = lKeyValues.get(value.toLowerCase());
>>              if(iVal == null) {
>>                  final StringBuilder sb= new StringBuilder(value);
>>                  sb.append(" not in (");
>>
>> Modified: 
>>commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/Fas
>>tDateParserTest.java
>> URL: 
>>http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/
>>apache/commons/lang3/time/FastDateParserTest.java?rev=1589446&r1=1589445&
>>r2=1589446&view=diff
>> 
>>=========================================================================
>>=====
>> --- 
>>commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/Fas
>>tDateParserTest.java (original)
>> +++ 
>>commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/Fas
>>tDateParserTest.java Wed Apr 23 16:02:52 2014
>> @@ -540,4 +540,16 @@ public class FastDateParserTest {
>>          final DateParser parser= getInstance(yMdHmsSZ, REYKJAVIK);
>>          assertEquals(REYKJAVIK, parser.getTimeZone());
>>      }
>> +
>> +    @Test
>> +    public void testLang996() throws ParseException {
>> +        Calendar expected = Calendar.getInstance(NEW_YORK, Locale.US);
>> +        expected.clear();
>> +        expected.set(2014, 4, 14);
>> +
>> +        final DateParser fdp = getInstance("ddMMMyyyy", NEW_YORK,
>>Locale.US);
>> +        assertEquals(expected.getTime(), fdp.parse("14may2014"));
>> +        assertEquals(expected.getTime(), fdp.parse("14MAY2014"));
>> +        assertEquals(expected.getTime(), fdp.parse("14May2014"));
>> +    }
>
>Need more tests to show that the parsing of other case-significant
>dates is not affected.
>
>>  }
>>
>>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>For additional commands, e-mail: dev-help@commons.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1589446 - in /commons/proper/lang/trunk/src: main/java/org/apache/commons/lang3/time/FastDateParser.java test/java/org/apache/commons/lang3/time/FastDateParserTest.java

Posted by sebb <se...@gmail.com>.
On 23 April 2014 17:02,  <ch...@apache.org> wrote:
> Author: chas
> Date: Wed Apr 23 16:02:52 2014
> New Revision: 1589446
>
> URL: http://svn.apache.org/r1589446
> Log:
> LANG-966 - FastDateParser should be case insensitive
>
> Modified:
>     commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java
>     commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java
>
> Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java
> URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java?rev=1589446&r1=1589445&r2=1589446&view=diff
> ==============================================================================
> --- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java (original)
> +++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/time/FastDateParser.java Wed Apr 23 16:02:52 2014
> @@ -25,6 +25,7 @@ import java.text.ParsePosition;
>  import java.util.ArrayList;
>  import java.util.Calendar;
>  import java.util.Date;
> +import java.util.HashMap;
>  import java.util.List;
>  import java.util.Locale;
>  import java.util.Map;
> @@ -586,6 +587,7 @@ public class FastDateParser implements D
>       private static class TextStrategy extends Strategy {
>          private final int field;
>          private final Map<String, Integer> keyValues;
> +        private final Map<String, Integer> lKeyValues;

This is an extra data area for ALL Text fields, but is only needed for
Month parsing.

I think a separate class is needed.

>          /**
>           * Construct a Strategy that parses a Text field
> @@ -596,6 +598,11 @@ public class FastDateParser implements D
>          TextStrategy(final int field, final Calendar definingCalendar, final Locale locale) {
>              this.field= field;
>              this.keyValues= getDisplayNames(field, definingCalendar, locale);
> +            this.lKeyValues= new HashMap<String,Integer>();
> +
> +            for(Map.Entry<String, Integer> entry : keyValues.entrySet()) {
> +               lKeyValues.put(entry.getKey().toLowerCase(), entry.getValue());
> +            }
>          }
>
>          /**
> @@ -603,7 +610,7 @@ public class FastDateParser implements D
>           */
>          @Override
>          boolean addRegex(final FastDateParser parser, final StringBuilder regex) {
> -            regex.append('(');
> +            regex.append("((?i)(?u)");
>              for(final String textKeyValue : keyValues.keySet()) {
>                  escapeRegex(regex, textKeyValue, false).append('|');

-1

AFAICT, this will enable incorrect matching of text fields that are
not supposed to be case-blind.
Also, the case-blind matching is not disabled at the end of the
section, so can affect later matches.

Using a separate class for case-blind matching will fix the first
issue, and will avoid creating duplicate data unnecessarily.

>              }
> @@ -616,7 +623,7 @@ public class FastDateParser implements D
>           */
>          @Override
>          void setCalendar(final FastDateParser parser, final Calendar cal, final String value) {
> -            final Integer iVal = keyValues.get(value);
> +            final Integer iVal = lKeyValues.get(value.toLowerCase());
>              if(iVal == null) {
>                  final StringBuilder sb= new StringBuilder(value);
>                  sb.append(" not in (");
>
> Modified: commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java
> URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java?rev=1589446&r1=1589445&r2=1589446&view=diff
> ==============================================================================
> --- commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java (original)
> +++ commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/time/FastDateParserTest.java Wed Apr 23 16:02:52 2014
> @@ -540,4 +540,16 @@ public class FastDateParserTest {
>          final DateParser parser= getInstance(yMdHmsSZ, REYKJAVIK);
>          assertEquals(REYKJAVIK, parser.getTimeZone());
>      }
> +
> +    @Test
> +    public void testLang996() throws ParseException {
> +        Calendar expected = Calendar.getInstance(NEW_YORK, Locale.US);
> +        expected.clear();
> +        expected.set(2014, 4, 14);
> +
> +        final DateParser fdp = getInstance("ddMMMyyyy", NEW_YORK, Locale.US);
> +        assertEquals(expected.getTime(), fdp.parse("14may2014"));
> +        assertEquals(expected.getTime(), fdp.parse("14MAY2014"));
> +        assertEquals(expected.getTime(), fdp.parse("14May2014"));
> +    }

Need more tests to show that the parsing of other case-significant
dates is not affected.

>  }
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org