You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by gg...@apache.org on 2015/06/27 09:40:08 UTC

logging-log4j2 git commit: Make sure the value associated with the key is returned.

Repository: logging-log4j2
Updated Branches:
  refs/heads/master a7e04f804 -> 70b72ad6f


Make sure the value associated with the key is returned.

Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/70b72ad6
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/70b72ad6
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/70b72ad6

Branch: refs/heads/master
Commit: 70b72ad6fabf976feb386715fa27d1b6061ca704
Parents: a7e04f8
Author: ggregory <gg...@apache.org>
Authored: Sat Jun 27 00:40:06 2015 -0700
Committer: ggregory <gg...@apache.org>
Committed: Sat Jun 27 00:40:06 2015 -0700

----------------------------------------------------------------------
 .../log4j/core/config/plugins/convert/TypeConverterRegistry.java  | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/70b72ad6/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
index d6e0670..12d364f 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
@@ -96,8 +96,7 @@ public class TypeConverterRegistry {
             if (TypeUtil.isAssignable(type, key)) {
                 LOGGER.debug("Found compatible TypeConverter<{}> for type [{}].", key, type);
                 final TypeConverter<?> value = entry.getValue();
-                registry.putIfAbsent(type, value);
-                return value;
+                return registry.putIfAbsent(type, value);
             }
         }
         throw new UnknownFormatConversionException(type.toString());


Re: logging-log4j2 git commit: Make sure the value associated with the key is returned.

Posted by Gary Gregory <ga...@gmail.com>.
Yep, I did not think that one through. Please accept my apologies.

Gary

On Sat, Jun 27, 2015 at 2:36 PM, Ralph Goers <ra...@dslextreme.com>
wrote:

> The current code has a valid converter and tries to add it to the
> registry. Even if it isn’t added it is still used since it if valid.  After
> the change the value from putIfAbsent was used, which will be null if there
> wasn’t a matching key in the map. To literally be correct the code would
> have to save the return value and return it if it is not null or the value
> being stored if it is.  I’m not sure it is worth the trouble.
>
> Ralph
>
> On Jun 27, 2015, at 11:14 AM, Gary Gregory <ga...@gmail.com> wrote:
>
> It must have and it's my fault. I changed this based on a FindBugs report.
>
> Gary
>
> On Sat, Jun 27, 2015 at 9:05 AM, Ralph Goers <ra...@dslextreme.com>
> wrote:
>
>> Did this change break the build?  Jenkins is showing test failures in
>> TypeConverterRegistryTest.
>>
>> Ralph
>>
>> Begin forwarded message:
>>
>> *From: *ggregory@apache.org
>> *Subject: **logging-log4j2 git commit: Make sure the value associated
>> with the key is returned.*
>> *Date: *June 27, 2015 at 12:40:08 AM MST
>> *To: *commits@logging.apache.org
>> *Reply-To: *dev@logging.apache.org
>>
>> Repository: logging-log4j2
>> Updated Branches:
>>  refs/heads/master a7e04f804 -> 70b72ad6f
>>
>>
>> Make sure the value associated with the key is returned.
>>
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> Commit:
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/70b72ad6
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/70b72ad6
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/70b72ad6
>>
>> Branch: refs/heads/master
>> Commit: 70b72ad6fabf976feb386715fa27d1b6061ca704
>> Parents: a7e04f8
>> Author: ggregory <gg...@apache.org>
>> Authored: Sat Jun 27 00:40:06 2015 -0700
>> Committer: ggregory <gg...@apache.org>
>> Committed: Sat Jun 27 00:40:06 2015 -0700
>>
>> ----------------------------------------------------------------------
>> .../log4j/core/config/plugins/convert/TypeConverterRegistry.java  | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/70b72ad6/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> index d6e0670..12d364f 100644
>> ---
>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> +++
>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> @@ -96,8 +96,7 @@ public class TypeConverterRegistry {
>>             if (TypeUtil.isAssignable(type, key)) {
>>                 LOGGER.debug("Found compatible TypeConverter<{}> for type
>> [{}].", key, type);
>>                 final TypeConverter<?> value = entry.getValue();
>> -                registry.putIfAbsent(type, value);
>> -                return value;
>> +                return registry.putIfAbsent(type, value);
>>             }
>>         }
>>         throw new UnknownFormatConversionException(type.toString());
>>
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: logging-log4j2 git commit: Make sure the value associated with the key is returned.

Posted by Ralph Goers <ra...@dslextreme.com>.
The current code has a valid converter and tries to add it to the registry. Even if it isn’t added it is still used since it if valid.  After the change the value from putIfAbsent was used, which will be null if there wasn’t a matching key in the map. To literally be correct the code would have to save the return value and return it if it is not null or the value being stored if it is.  I’m not sure it is worth the trouble.

Ralph

> On Jun 27, 2015, at 11:14 AM, Gary Gregory <ga...@gmail.com> wrote:
> 
> It must have and it's my fault. I changed this based on a FindBugs report.
> 
> Gary
> 
> On Sat, Jun 27, 2015 at 9:05 AM, Ralph Goers <ralph.goers@dslextreme.com <ma...@dslextreme.com>> wrote:
> Did this change break the build?  Jenkins is showing test failures in TypeConverterRegistryTest.
> 
> Ralph
> 
>> Begin forwarded message:
>> 
>> From: ggregory@apache.org <ma...@apache.org>
>> Subject: logging-log4j2 git commit: Make sure the value associated with the key is returned.
>> Date: June 27, 2015 at 12:40:08 AM MST
>> To: commits@logging.apache.org <ma...@logging.apache.org>
>> Reply-To: dev@logging.apache.org <ma...@logging.apache.org>
>> 
>> Repository: logging-log4j2
>> Updated Branches:
>>  refs/heads/master a7e04f804 -> 70b72ad6f
>> 
>> 
>> Make sure the value associated with the key is returned.
>> 
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo <http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo>
>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/70b72ad6 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/70b72ad6>
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/70b72ad6 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/70b72ad6>
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/70b72ad6 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/70b72ad6>
>> 
>> Branch: refs/heads/master
>> Commit: 70b72ad6fabf976feb386715fa27d1b6061ca704
>> Parents: a7e04f8
>> Author: ggregory <ggregory@apache.org <ma...@apache.org>>
>> Authored: Sat Jun 27 00:40:06 2015 -0700
>> Committer: ggregory <ggregory@apache.org <ma...@apache.org>>
>> Committed: Sat Jun 27 00:40:06 2015 -0700
>> 
>> ----------------------------------------------------------------------
>> .../log4j/core/config/plugins/convert/TypeConverterRegistry.java  | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> ----------------------------------------------------------------------
>> 
>> 
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/70b72ad6/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java <http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/70b72ad6/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java>
>> ----------------------------------------------------------------------
>> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> index d6e0670..12d364f 100644
>> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> @@ -96,8 +96,7 @@ public class TypeConverterRegistry {
>>             if (TypeUtil.isAssignable(type, key)) {
>>                 LOGGER.debug("Found compatible TypeConverter<{}> for type [{}].", key, type);
>>                 final TypeConverter<?> value = entry.getValue();
>> -                registry.putIfAbsent(type, value);
>> -                return value;
>> +                return registry.putIfAbsent(type, value);
>>             }
>>         }
>>         throw new UnknownFormatConversionException(type.toString());
>> 
> 
> 
> 
> 
> -- 
> E-Mail: garydgregory@gmail.com <ma...@gmail.com> | ggregory@apache.org  <ma...@apache.org>
> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> 
> Home: http://garygregory.com/ <http://garygregory.com/>
> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>

Re: logging-log4j2 git commit: Make sure the value associated with the key is returned.

Posted by Gary Gregory <ga...@gmail.com>.
It must have and it's my fault. I changed this based on a FindBugs report.

Gary

On Sat, Jun 27, 2015 at 9:05 AM, Ralph Goers <ra...@dslextreme.com>
wrote:

> Did this change break the build?  Jenkins is showing test failures in
> TypeConverterRegistryTest.
>
> Ralph
>
> Begin forwarded message:
>
> *From: *ggregory@apache.org
> *Subject: **logging-log4j2 git commit: Make sure the value associated
> with the key is returned.*
> *Date: *June 27, 2015 at 12:40:08 AM MST
> *To: *commits@logging.apache.org
> *Reply-To: *dev@logging.apache.org
>
> Repository: logging-log4j2
> Updated Branches:
>  refs/heads/master a7e04f804 -> 70b72ad6f
>
>
> Make sure the value associated with the key is returned.
>
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/70b72ad6
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/70b72ad6
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/70b72ad6
>
> Branch: refs/heads/master
> Commit: 70b72ad6fabf976feb386715fa27d1b6061ca704
> Parents: a7e04f8
> Author: ggregory <gg...@apache.org>
> Authored: Sat Jun 27 00:40:06 2015 -0700
> Committer: ggregory <gg...@apache.org>
> Committed: Sat Jun 27 00:40:06 2015 -0700
>
> ----------------------------------------------------------------------
> .../log4j/core/config/plugins/convert/TypeConverterRegistry.java  | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/70b72ad6/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> ----------------------------------------------------------------------
> diff --git
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> index d6e0670..12d364f 100644
> ---
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> +++
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> @@ -96,8 +96,7 @@ public class TypeConverterRegistry {
>             if (TypeUtil.isAssignable(type, key)) {
>                 LOGGER.debug("Found compatible TypeConverter<{}> for type
> [{}].", key, type);
>                 final TypeConverter<?> value = entry.getValue();
> -                registry.putIfAbsent(type, value);
> -                return value;
> +                return registry.putIfAbsent(type, value);
>             }
>         }
>         throw new UnknownFormatConversionException(type.toString());
>
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: logging-log4j2 git commit: Make sure the value associated with the key is returned.

Posted by Ralph Goers <ra...@dslextreme.com>.
I verified that this change broke the build so I reverted it. It isn’t clear from the commit message why this change is needed.

Ralph

> On Jun 27, 2015, at 9:05 AM, Ralph Goers <ra...@dslextreme.com> wrote:
> 
> Did this change break the build?  Jenkins is showing test failures in TypeConverterRegistryTest.
> 
> Ralph
> 
>> Begin forwarded message:
>> 
>> From: ggregory@apache.org <ma...@apache.org>
>> Subject: logging-log4j2 git commit: Make sure the value associated with the key is returned.
>> Date: June 27, 2015 at 12:40:08 AM MST
>> To: commits@logging.apache.org <ma...@logging.apache.org>
>> Reply-To: dev@logging.apache.org <ma...@logging.apache.org>
>> 
>> Repository: logging-log4j2
>> Updated Branches:
>>  refs/heads/master a7e04f804 -> 70b72ad6f
>> 
>> 
>> Make sure the value associated with the key is returned.
>> 
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo <http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo>
>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/70b72ad6 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/70b72ad6>
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/70b72ad6 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/70b72ad6>
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/70b72ad6 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/70b72ad6>
>> 
>> Branch: refs/heads/master
>> Commit: 70b72ad6fabf976feb386715fa27d1b6061ca704
>> Parents: a7e04f8
>> Author: ggregory <ggregory@apache.org <ma...@apache.org>>
>> Authored: Sat Jun 27 00:40:06 2015 -0700
>> Committer: ggregory <ggregory@apache.org <ma...@apache.org>>
>> Committed: Sat Jun 27 00:40:06 2015 -0700
>> 
>> ----------------------------------------------------------------------
>> .../log4j/core/config/plugins/convert/TypeConverterRegistry.java  | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> ----------------------------------------------------------------------
>> 
>> 
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/70b72ad6/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java <http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/70b72ad6/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java>
>> ----------------------------------------------------------------------
>> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> index d6e0670..12d364f 100644
>> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
>> @@ -96,8 +96,7 @@ public class TypeConverterRegistry {
>>             if (TypeUtil.isAssignable(type, key)) {
>>                 LOGGER.debug("Found compatible TypeConverter<{}> for type [{}].", key, type);
>>                 final TypeConverter<?> value = entry.getValue();
>> -                registry.putIfAbsent(type, value);
>> -                return value;
>> +                return registry.putIfAbsent(type, value);
>>             }
>>         }
>>         throw new UnknownFormatConversionException(type.toString());
>> 
> 


Fwd: logging-log4j2 git commit: Make sure the value associated with the key is returned.

Posted by Ralph Goers <ra...@dslextreme.com>.
Did this change break the build?  Jenkins is showing test failures in TypeConverterRegistryTest.

Ralph

> Begin forwarded message:
> 
> From: ggregory@apache.org
> Subject: logging-log4j2 git commit: Make sure the value associated with the key is returned.
> Date: June 27, 2015 at 12:40:08 AM MST
> To: commits@logging.apache.org
> Reply-To: dev@logging.apache.org
> 
> Repository: logging-log4j2
> Updated Branches:
>  refs/heads/master a7e04f804 -> 70b72ad6f
> 
> 
> Make sure the value associated with the key is returned.
> 
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/70b72ad6
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/70b72ad6
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/70b72ad6
> 
> Branch: refs/heads/master
> Commit: 70b72ad6fabf976feb386715fa27d1b6061ca704
> Parents: a7e04f8
> Author: ggregory <gg...@apache.org>
> Authored: Sat Jun 27 00:40:06 2015 -0700
> Committer: ggregory <gg...@apache.org>
> Committed: Sat Jun 27 00:40:06 2015 -0700
> 
> ----------------------------------------------------------------------
> .../log4j/core/config/plugins/convert/TypeConverterRegistry.java  | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/70b72ad6/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> index d6e0670..12d364f 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/TypeConverterRegistry.java
> @@ -96,8 +96,7 @@ public class TypeConverterRegistry {
>             if (TypeUtil.isAssignable(type, key)) {
>                 LOGGER.debug("Found compatible TypeConverter<{}> for type [{}].", key, type);
>                 final TypeConverter<?> value = entry.getValue();
> -                registry.putIfAbsent(type, value);
> -                return value;
> +                return registry.putIfAbsent(type, value);
>             }
>         }
>         throw new UnknownFormatConversionException(type.toString());
>