Sorry, I'm sure this is getting annoying by now, but  It looks like the
default and the override are handled differently.  The current code
compiles the default case insensitive, but not the passed in pattern.
Should that be consistent?

private Pattern acceptedPattern =
Pattern.compile(ACCEPTED_PATTERN,Pattern.CASE_INSENSITIVE);

public void setAcceptCookieNames (String pattern) {
  acceptedPattern = Pattern.compile(pattern);
}


On Tue Dec 16 2014 at 1:38:45 PM Chris Pratt <thechrispr...@gmail.com>
wrote:

> Oh, on another note, it would be ever so slightly more efficient to invert
> the tests.  You are always testing both matches && LOG.isTraceEnabled() in
> the current implementation, but there's no need to check matches if trace
> isn't enabled.  So, maybe something like:
>
> /**
>  * Checks if name of Cookie match {@link #acceptedPattern}
>  *
>  * @param name of Cookie
>  * @return true|false
>  */
> protected boolean isAccepted(String name){
>   boolean matches = acceptedPattern.matcher(name).matches();
>
>   if(LOG.isTraceEnabled()) {
>     if(matches) {
>
>
>       LOG.trace("Cookie [#0] matches acceptedPattern 
> [#1]",name,acceptedPattern.pattern());
>     } else {
>
>       LOG.trace("Cookie [#0] doesn't match acceptedPattern 
> [#1]",name,acceptedPattern.pattern());
>     }
>   }
>   return matches;
> }
>
>
> On Tue Dec 16 2014 at 1:28:38 PM Chris Pratt <thechrispr...@gmail.com>
> wrote:
>
>> Sorry, I don't have an environment set up to create a patch, but I found
>> an error in the isAccepted() method.  It currently looks like:
>>
>> /**
>>  * Checks if name of Cookie match {@link #acceptedPattern}
>>  *
>>  * @param name of Cookie
>>  * @return true|false
>>  */
>> protected boolean isAccepted(String name) {
>>     boolean matches = acceptedPattern.matcher(name).matches();
>>     if (matches) {
>>         if (LOG.isTraceEnabled()) {
>>             LOG.trace("Cookie [#0] matches acceptedPattern [#1]", name, 
>> ACCEPTED_PATTERN);
>>         }
>>     } else {
>>         if (LOG.isTraceEnabled()) {
>>             LOG.trace("Cookie [#0] doesn't match acceptedPattern [#1]", 
>> name, ACCEPTED_PATTERN);
>>         }
>>     }
>>     return matches;
>> }
>>
>>
>> But it would be more useful if it actually reported the RegEx being used
>> instead of the default.  So something more like:
>>
>> /**
>>  * Checks if name of Cookie match {@link #acceptedPattern}
>>  *
>>  * @param name of Cookie
>>  * @return true|false
>>  */
>> protected boolean isAccepted (String name) {
>>   boolean matches = acceptedPattern.matcher(name).matches();
>>   if(matches) {
>>     if(LOG.isTraceEnabled()) {
>>       LOG.trace("Cookie [#0] matches acceptedPattern 
>> [#1]",name,acceptedPattern.pattern());
>>     }
>>   } else {
>>     if(LOG.isTraceEnabled()) {
>>       LOG.trace("Cookie [#0] doesn't match acceptedPattern 
>> [#1]",name,acceptedPattern.pattern());
>>     }
>>   }
>>   return matches;
>> }
>>
>>   (*Chris*)
>>
>

Reply via email to