You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by va...@apache.org on 2006/11/27 11:54:39 UTC
svn commit: r479586 -
/geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Author: vamsic007
Date: Mon Nov 27 02:54:38 2006
New Revision: 479586
URL: http://svn.apache.org/viewvc?view=rev&rev=479586
Log:
GERONIMO-2458 MapEditor does not work
Modified:
geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Modified: geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
==============================================================================
--- geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java (original)
+++ geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27 02:54:38 2006
@@ -19,17 +19,22 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
+import java.util.Iterator;
import java.util.Properties;
import java.util.Map;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
/**
- * A property editor for {@link java.util.Properties}.
+ * A property editor for {@link java.util.Map}.
*
* @version $Rev$ $Date$
*/
public class MapEditor
extends TextPropertyEditorSupport
{
+ private static final Log log = LogFactory.getLog(MapEditor.class);
/**
*
* @throws PropertyEditorException An IOException occured.
@@ -50,11 +55,30 @@
Map map = (Map) getValue();
if (!(map instanceof Properties)) {
Properties p = new Properties();
- if (map != null) {
- p.putAll(map);
+ if(map != null) {
+ if(!map.containsKey(null) && !map.containsValue(null)) {
+ p.putAll(map);
+ } else {
+ // Map contains null keys or values. Replace null with empty string.
+ log.warn("Map contains null keys or values. Replacing null values with empty string.");
+ for(Iterator itr = map.entrySet().iterator(); itr.hasNext(); ) {
+ Map.Entry entry = (Map.Entry) itr.next();
+ Object key = entry.getKey();
+ Object value = entry.getValue();
+ if(key == null) {
+ key = "";
+ }
+ if(value == null) {
+ value = "";
+ }
+ p.put(key, value);
+ }
+ }
+ map = p;
}
- map = p;
}
- return map.toString();
+ PropertiesEditor pe = new PropertiesEditor();
+ pe.setValue(map);
+ return pe.getAsText();
}
}
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Jason Dillon <ja...@planet57.com>.
On Nov 27, 2006, at 1:52 PM, Joe Bohn wrote:
> I agree with Jason that the change log should include a list of
> changes rather than problem statements. After all, it is a
> *change* log and not a *problem* log. :-) Just a brief summary of
> the change is all that's needed and I personally find this much
> more helpful than a one line description of a problem taken from
> the JIRA. Of course, the description should also include a
> reference to the JIRA.
Yup.
> I also agree with Donald (and I suspect Jason does too) that the
> JIRA is the right place for all the gory details on the problem and
> solution.
Yup.
> So, can we agree to keep the details in the JIRA but have a brief
> descriptive statement of what changed in the commit log rather than
> a statement of the problem?
Hope so ;-)
--jason
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Joe Bohn <jo...@earthlink.net>.
I agree with Jason that the change log should include a list of changes
rather than problem statements. After all, it is a *change* log and not
a *problem* log. :-) Just a brief summary of the change is all that's
needed and I personally find this much more helpful than a one line
description of a problem taken from the JIRA. Of course, the
description should also include a reference to the JIRA.
I also agree with Donald (and I suspect Jason does too) that the JIRA is
the right place for all the gory details on the problem and solution.
So, can we agree to keep the details in the JIRA but have a brief
descriptive statement of what changed in the commit log rather than a
statement of the problem?
Joe
Jason Dillon wrote:
> I completely disagree. The change log should contain context of the
> change, and not just the link to JIRA.
>
> --jason
>
>
> On Nov 27, 2006, at 12:56 PM, Donald Woods wrote:
>
>> Providing a link to the JIRA makes more sense, as it can contain the
>> full description of the fix, which releases it went in, any
>> coreq/prereqs like OpenEJB or TranQL changes, if multiple patches
>> and/or commits were required, .....
>>
>> As long as everyone adds real problem and solution descriptions in
>> the JIRAs, I'm fine with only providing a link back to it in a
>> commit, which lots of people before Vamsi have done in the past
>> without complaint :-)
>>
>>
>> -Donald
>>
>>
>> Jason Dillon wrote:
>>
>>> As I mentioned... I really do not want to have to bounce from commit
>>> messages to JIRA to provide oversight into the changes going into
>>> our SVN repo.
>>> Also, when something does break, I am going to want to look back
>>> through the SVN logs and find meaningful context to changes. In
>>> this case "GERONIMO-2458 MapEditor does not work" does not tell me
>>> what was changed, only tells me what was wrong.
>>> IMO SVN is the definitive location for changes... and those changes
>>> should be properly adorned with context of the change.
>>> --jason
>>> On Nov 27, 2006, at 12:08 PM, Donald Woods wrote:
>>>
>>>> The problem description and fix details were included in the JIRA
>>>> by Vamsi as they should be - http://issues.apache.org/jira/browse/
>>>> GERONIMO-2458
>>>>
>>>>
>>>> -Donald
>>>>
>>>>
>>>> Jason Dillon wrote:
>>>>
>>>>> Please... please... please... put more context into the commit log.
>>>>> "MapEditor does not work" tells me nothing about what was actually
>>>>> changed. When auditing changes it helps to have some context to
>>>>> the change in the commit log so that its easy to comprehend what
>>>>> the change was, with out having to dig around, or bounce into
>>>>> JIRA, etc.
>>>>> I've mentioned this a few times before... while its is good to
>>>>> include the JIRA issue ID, only including that ID, or in this case
>>>>> the ID and the issue subject, in the SVN commit message is not
>>>>> sufficient.
>>>>> Please... please... please... try to put some more meaningful
>>>>> context into commit messages.
>>>>> Thanks,
>>>>> --jason
>>>>> On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
>>>>>
>>>>>> Author: vamsic007
>>>>>> Date: Mon Nov 27 02:54:38 2006
>>>>>> New Revision: 479586
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=479586
>>>>>> Log:
>>>>>> GERONIMO-2458 MapEditor does not work
>>>>>>
>>>>>> Modified:
>>>>>> geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>>>>> org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>>>>
>>>>>> Modified: geronimo/server/trunk/modules/geronimo-common/src/
>>>>>> main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>>>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
>>>>>> geronimo-common/src/main/java/org/apache/geronimo/common/
>>>>>> propertyeditor/MapEditor.java?
>>>>>> view=diff&rev=479586&r1=479585&r2=479586
>>>>>> ==================================================================
>>>>>> ============
>>>>>> --- geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>>>>> org/apache/geronimo/common/propertyeditor/MapEditor.java (original)
>>>>>> +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>>>>> org/apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov
>>>>>> 27 02:54:38 2006
>>>>>> @@ -19,17 +19,22 @@
>>>>>>
>>>>>> import java.io.ByteArrayInputStream;
>>>>>> import java.io.IOException;
>>>>>> +import java.util.Iterator;
>>>>>> import java.util.Properties;
>>>>>> import java.util.Map;
>>>>>>
>>>>>> +import org.apache.commons.logging.Log;
>>>>>> +import org.apache.commons.logging.LogFactory;
>>>>>> +
>>>>>> /**
>>>>>> - * A property editor for {@link java.util.Properties}.
>>>>>> + * A property editor for {@link java.util.Map}.
>>>>>> *
>>>>>> * @version $Rev$ $Date$
>>>>>> */
>>>>>> public class MapEditor
>>>>>> extends TextPropertyEditorSupport
>>>>>> {
>>>>>> + private static final Log log = LogFactory.getLog
>>>>>> (MapEditor.class);
>>>>>> /**
>>>>>> *
>>>>>> * @throws PropertyEditorException An IOException occured.
>>>>>> @@ -50,11 +55,30 @@
>>>>>> Map map = (Map) getValue();
>>>>>> if (!(map instanceof Properties)) {
>>>>>> Properties p = new Properties();
>>>>>> - if (map != null) {
>>>>>> - p.putAll(map);
>>>>>> + if(map != null) {
>>>>>> + if(!map.containsKey(null) && !map.containsValue
>>>>>> (null)) {
>>>>>> + p.putAll(map);
>>>>>> + } else {
>>>>>> + // Map contains null keys or values.
>>>>>> Replace null with empty string.
>>>>>> + log.warn("Map contains null keys or values.
>>>>>> Replacing null values with empty string.");
>>>>>> + for(Iterator itr = map.entrySet().iterator
>>>>>> (); itr.hasNext(); ) {
>>>>>> + Map.Entry entry = (Map.Entry) itr.next();
>>>>>> + Object key = entry.getKey();
>>>>>> + Object value = entry.getValue();
>>>>>> + if(key == null) {
>>>>>> + key = "";
>>>>>> + }
>>>>>> + if(value == null) {
>>>>>> + value = "";
>>>>>> + }
>>>>>> + p.put(key, value);
>>>>>> + }
>>>>>> + }
>>>>>> + map = p;
>>>>>> }
>>>>>> - map = p;
>>>>>> }
>>>>>> - return map.toString();
>>>>>> + PropertiesEditor pe = new PropertiesEditor();
>>>>>> + pe.setValue(map);
>>>>>> + return pe.getAsText();
>>>>>> }
>>>>>> }
>>>>>>
>>>>>>
>
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Jason Dillon <ja...@planet57.com>.
I completely disagree. The change log should contain context of the
change, and not just the link to JIRA.
--jason
On Nov 27, 2006, at 12:56 PM, Donald Woods wrote:
> Providing a link to the JIRA makes more sense, as it can contain
> the full description of the fix, which releases it went in, any
> coreq/prereqs like OpenEJB or TranQL changes, if multiple patches
> and/or commits were required, .....
>
> As long as everyone adds real problem and solution descriptions in
> the JIRAs, I'm fine with only providing a link back to it in a
> commit, which lots of people before Vamsi have done in the past
> without complaint :-)
>
>
> -Donald
>
>
> Jason Dillon wrote:
>> As I mentioned... I really do not want to have to bounce from
>> commit messages to JIRA to provide oversight into the changes
>> going into our SVN repo.
>> Also, when something does break, I am going to want to look back
>> through the SVN logs and find meaningful context to changes. In
>> this case "GERONIMO-2458 MapEditor does not work" does not tell me
>> what was changed, only tells me what was wrong.
>> IMO SVN is the definitive location for changes... and those
>> changes should be properly adorned with context of the change.
>> --jason
>> On Nov 27, 2006, at 12:08 PM, Donald Woods wrote:
>>> The problem description and fix details were included in the JIRA
>>> by Vamsi as they should be - http://issues.apache.org/jira/browse/
>>> GERONIMO-2458
>>>
>>>
>>> -Donald
>>>
>>>
>>> Jason Dillon wrote:
>>>> Please... please... please... put more context into the commit log.
>>>> "MapEditor does not work" tells me nothing about what was
>>>> actually changed. When auditing changes it helps to have some
>>>> context to the change in the commit log so that its easy to
>>>> comprehend what the change was, with out having to dig around,
>>>> or bounce into JIRA, etc.
>>>> I've mentioned this a few times before... while its is good to
>>>> include the JIRA issue ID, only including that ID, or in this
>>>> case the ID and the issue subject, in the SVN commit message is
>>>> not sufficient.
>>>> Please... please... please... try to put some more meaningful
>>>> context into commit messages.
>>>> Thanks,
>>>> --jason
>>>> On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
>>>>> Author: vamsic007
>>>>> Date: Mon Nov 27 02:54:38 2006
>>>>> New Revision: 479586
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=479586
>>>>> Log:
>>>>> GERONIMO-2458 MapEditor does not work
>>>>>
>>>>> Modified:
>>>>> geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>>>> org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>>>
>>>>> Modified: geronimo/server/trunk/modules/geronimo-common/src/
>>>>> main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
>>>>> geronimo-common/src/main/java/org/apache/geronimo/common/
>>>>> propertyeditor/MapEditor.java?
>>>>> view=diff&rev=479586&r1=479585&r2=479586
>>>>> ==================================================================
>>>>> ============
>>>>> --- geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>>>> org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>>> (original)
>>>>> +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>>>> org/apache/geronimo/common/propertyeditor/MapEditor.java Mon
>>>>> Nov 27 02:54:38 2006
>>>>> @@ -19,17 +19,22 @@
>>>>>
>>>>> import java.io.ByteArrayInputStream;
>>>>> import java.io.IOException;
>>>>> +import java.util.Iterator;
>>>>> import java.util.Properties;
>>>>> import java.util.Map;
>>>>>
>>>>> +import org.apache.commons.logging.Log;
>>>>> +import org.apache.commons.logging.LogFactory;
>>>>> +
>>>>> /**
>>>>> - * A property editor for {@link java.util.Properties}.
>>>>> + * A property editor for {@link java.util.Map}.
>>>>> *
>>>>> * @version $Rev$ $Date$
>>>>> */
>>>>> public class MapEditor
>>>>> extends TextPropertyEditorSupport
>>>>> {
>>>>> + private static final Log log = LogFactory.getLog
>>>>> (MapEditor.class);
>>>>> /**
>>>>> *
>>>>> * @throws PropertyEditorException An IOException occured.
>>>>> @@ -50,11 +55,30 @@
>>>>> Map map = (Map) getValue();
>>>>> if (!(map instanceof Properties)) {
>>>>> Properties p = new Properties();
>>>>> - if (map != null) {
>>>>> - p.putAll(map);
>>>>> + if(map != null) {
>>>>> + if(!map.containsKey(null) && !map.containsValue
>>>>> (null)) {
>>>>> + p.putAll(map);
>>>>> + } else {
>>>>> + // Map contains null keys or values.
>>>>> Replace null with empty string.
>>>>> + log.warn("Map contains null keys or
>>>>> values. Replacing null values with empty string.");
>>>>> + for(Iterator itr = map.entrySet().iterator
>>>>> (); itr.hasNext(); ) {
>>>>> + Map.Entry entry = (Map.Entry) itr.next();
>>>>> + Object key = entry.getKey();
>>>>> + Object value = entry.getValue();
>>>>> + if(key == null) {
>>>>> + key = "";
>>>>> + }
>>>>> + if(value == null) {
>>>>> + value = "";
>>>>> + }
>>>>> + p.put(key, value);
>>>>> + }
>>>>> + }
>>>>> + map = p;
>>>>> }
>>>>> - map = p;
>>>>> }
>>>>> - return map.toString();
>>>>> + PropertiesEditor pe = new PropertiesEditor();
>>>>> + pe.setValue(map);
>>>>> + return pe.getAsText();
>>>>> }
>>>>> }
>>>>>
>>>>>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Donald Woods <dr...@yahoo.com>.
Providing a link to the JIRA makes more sense, as it can contain the
full description of the fix, which releases it went in, any
coreq/prereqs like OpenEJB or TranQL changes, if multiple patches and/or
commits were required, .....
As long as everyone adds real problem and solution descriptions in the
JIRAs, I'm fine with only providing a link back to it in a commit, which
lots of people before Vamsi have done in the past without complaint :-)
-Donald
Jason Dillon wrote:
> As I mentioned... I really do not want to have to bounce from commit
> messages to JIRA to provide oversight into the changes going into our
> SVN repo.
>
> Also, when something does break, I am going to want to look back through
> the SVN logs and find meaningful context to changes. In this case
> "GERONIMO-2458 MapEditor does not work" does not tell me what was
> changed, only tells me what was wrong.
>
> IMO SVN is the definitive location for changes... and those changes
> should be properly adorned with context of the change.
>
> --jason
>
>
> On Nov 27, 2006, at 12:08 PM, Donald Woods wrote:
>
>> The problem description and fix details were included in the JIRA by
>> Vamsi as they should be -
>> http://issues.apache.org/jira/browse/GERONIMO-2458
>>
>>
>> -Donald
>>
>>
>> Jason Dillon wrote:
>>> Please... please... please... put more context into the commit log.
>>> "MapEditor does not work" tells me nothing about what was actually
>>> changed. When auditing changes it helps to have some context to the
>>> change in the commit log so that its easy to comprehend what the
>>> change was, with out having to dig around, or bounce into JIRA, etc.
>>> I've mentioned this a few times before... while its is good to
>>> include the JIRA issue ID, only including that ID, or in this case
>>> the ID and the issue subject, in the SVN commit message is not
>>> sufficient.
>>> Please... please... please... try to put some more meaningful context
>>> into commit messages.
>>> Thanks,
>>> --jason
>>> On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
>>>> Author: vamsic007
>>>> Date: Mon Nov 27 02:54:38 2006
>>>> New Revision: 479586
>>>>
>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=479586
>>>> Log:
>>>> GERONIMO-2458 MapEditor does not work
>>>>
>>>> Modified:
>>>>
>>>> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>>
>>>>
>>>> Modified:
>>>> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>> (original)
>>>> +++
>>>> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>> Mon Nov 27 02:54:38 2006
>>>> @@ -19,17 +19,22 @@
>>>>
>>>> import java.io.ByteArrayInputStream;
>>>> import java.io.IOException;
>>>> +import java.util.Iterator;
>>>> import java.util.Properties;
>>>> import java.util.Map;
>>>>
>>>> +import org.apache.commons.logging.Log;
>>>> +import org.apache.commons.logging.LogFactory;
>>>> +
>>>> /**
>>>> - * A property editor for {@link java.util.Properties}.
>>>> + * A property editor for {@link java.util.Map}.
>>>> *
>>>> * @version $Rev$ $Date$
>>>> */
>>>> public class MapEditor
>>>> extends TextPropertyEditorSupport
>>>> {
>>>> + private static final Log log = LogFactory.getLog(MapEditor.class);
>>>> /**
>>>> *
>>>> * @throws PropertyEditorException An IOException occured.
>>>> @@ -50,11 +55,30 @@
>>>> Map map = (Map) getValue();
>>>> if (!(map instanceof Properties)) {
>>>> Properties p = new Properties();
>>>> - if (map != null) {
>>>> - p.putAll(map);
>>>> + if(map != null) {
>>>> + if(!map.containsKey(null) &&
>>>> !map.containsValue(null)) {
>>>> + p.putAll(map);
>>>> + } else {
>>>> + // Map contains null keys or values. Replace
>>>> null with empty string.
>>>> + log.warn("Map contains null keys or values.
>>>> Replacing null values with empty string.");
>>>> + for(Iterator itr = map.entrySet().iterator();
>>>> itr.hasNext(); ) {
>>>> + Map.Entry entry = (Map.Entry) itr.next();
>>>> + Object key = entry.getKey();
>>>> + Object value = entry.getValue();
>>>> + if(key == null) {
>>>> + key = "";
>>>> + }
>>>> + if(value == null) {
>>>> + value = "";
>>>> + }
>>>> + p.put(key, value);
>>>> + }
>>>> + }
>>>> + map = p;
>>>> }
>>>> - map = p;
>>>> }
>>>> - return map.toString();
>>>> + PropertiesEditor pe = new PropertiesEditor();
>>>> + pe.setValue(map);
>>>> + return pe.getAsText();
>>>> }
>>>> }
>>>>
>>>>
>
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Jason Dillon <ja...@planet57.com>.
As I mentioned... I really do not want to have to bounce from commit
messages to JIRA to provide oversight into the changes going into our
SVN repo.
Also, when something does break, I am going to want to look back
through the SVN logs and find meaningful context to changes. In this
case "GERONIMO-2458 MapEditor does not work" does not tell me what
was changed, only tells me what was wrong.
IMO SVN is the definitive location for changes... and those changes
should be properly adorned with context of the change.
--jason
On Nov 27, 2006, at 12:08 PM, Donald Woods wrote:
> The problem description and fix details were included in the JIRA
> by Vamsi as they should be - http://issues.apache.org/jira/browse/
> GERONIMO-2458
>
>
> -Donald
>
>
> Jason Dillon wrote:
>> Please... please... please... put more context into the commit log.
>> "MapEditor does not work" tells me nothing about what was actually
>> changed. When auditing changes it helps to have some context to
>> the change in the commit log so that its easy to comprehend what
>> the change was, with out having to dig around, or bounce into
>> JIRA, etc.
>> I've mentioned this a few times before... while its is good to
>> include the JIRA issue ID, only including that ID, or in this case
>> the ID and the issue subject, in the SVN commit message is not
>> sufficient.
>> Please... please... please... try to put some more meaningful
>> context into commit messages.
>> Thanks,
>> --jason
>> On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
>>> Author: vamsic007
>>> Date: Mon Nov 27 02:54:38 2006
>>> New Revision: 479586
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=479586
>>> Log:
>>> GERONIMO-2458 MapEditor does not work
>>>
>>> Modified:
>>> geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>> org/apache/geronimo/common/propertyeditor/MapEditor.java
>>>
>>> Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
>>> java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
>>> geronimo-common/src/main/java/org/apache/geronimo/common/
>>> propertyeditor/MapEditor.java?
>>> view=diff&rev=479586&r1=479585&r2=479586
>>> ====================================================================
>>> ==========
>>> --- geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>> org/apache/geronimo/common/propertyeditor/MapEditor.java (original)
>>> +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/
>>> org/apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov
>>> 27 02:54:38 2006
>>> @@ -19,17 +19,22 @@
>>>
>>> import java.io.ByteArrayInputStream;
>>> import java.io.IOException;
>>> +import java.util.Iterator;
>>> import java.util.Properties;
>>> import java.util.Map;
>>>
>>> +import org.apache.commons.logging.Log;
>>> +import org.apache.commons.logging.LogFactory;
>>> +
>>> /**
>>> - * A property editor for {@link java.util.Properties}.
>>> + * A property editor for {@link java.util.Map}.
>>> *
>>> * @version $Rev$ $Date$
>>> */
>>> public class MapEditor
>>> extends TextPropertyEditorSupport
>>> {
>>> + private static final Log log = LogFactory.getLog
>>> (MapEditor.class);
>>> /**
>>> *
>>> * @throws PropertyEditorException An IOException occured.
>>> @@ -50,11 +55,30 @@
>>> Map map = (Map) getValue();
>>> if (!(map instanceof Properties)) {
>>> Properties p = new Properties();
>>> - if (map != null) {
>>> - p.putAll(map);
>>> + if(map != null) {
>>> + if(!map.containsKey(null) && !map.containsValue
>>> (null)) {
>>> + p.putAll(map);
>>> + } else {
>>> + // Map contains null keys or values.
>>> Replace null with empty string.
>>> + log.warn("Map contains null keys or values.
>>> Replacing null values with empty string.");
>>> + for(Iterator itr = map.entrySet().iterator
>>> (); itr.hasNext(); ) {
>>> + Map.Entry entry = (Map.Entry) itr.next();
>>> + Object key = entry.getKey();
>>> + Object value = entry.getValue();
>>> + if(key == null) {
>>> + key = "";
>>> + }
>>> + if(value == null) {
>>> + value = "";
>>> + }
>>> + p.put(key, value);
>>> + }
>>> + }
>>> + map = p;
>>> }
>>> - map = p;
>>> }
>>> - return map.toString();
>>> + PropertiesEditor pe = new PropertiesEditor();
>>> + pe.setValue(map);
>>> + return pe.getAsText();
>>> }
>>> }
>>>
>>>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Donald Woods <dr...@yahoo.com>.
The problem description and fix details were included in the JIRA by
Vamsi as they should be - http://issues.apache.org/jira/browse/GERONIMO-2458
-Donald
Jason Dillon wrote:
> Please... please... please... put more context into the commit log.
>
> "MapEditor does not work" tells me nothing about what was actually
> changed. When auditing changes it helps to have some context to the
> change in the commit log so that its easy to comprehend what the change
> was, with out having to dig around, or bounce into JIRA, etc.
>
> I've mentioned this a few times before... while its is good to include
> the JIRA issue ID, only including that ID, or in this case the ID and
> the issue subject, in the SVN commit message is not sufficient.
>
> Please... please... please... try to put some more meaningful context
> into commit messages.
>
> Thanks,
>
> --jason
>
>
> On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
>
>> Author: vamsic007
>> Date: Mon Nov 27 02:54:38 2006
>> New Revision: 479586
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=479586
>> Log:
>> GERONIMO-2458 MapEditor does not work
>>
>> Modified:
>>
>> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>
>>
>> Modified:
>> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>>
>> URL:
>> http://svn.apache.org/viewvc/geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
>>
>> ==============================================================================
>>
>> ---
>> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>> (original)
>> +++
>> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>> Mon Nov 27 02:54:38 2006
>> @@ -19,17 +19,22 @@
>>
>> import java.io.ByteArrayInputStream;
>> import java.io.IOException;
>> +import java.util.Iterator;
>> import java.util.Properties;
>> import java.util.Map;
>>
>> +import org.apache.commons.logging.Log;
>> +import org.apache.commons.logging.LogFactory;
>> +
>> /**
>> - * A property editor for {@link java.util.Properties}.
>> + * A property editor for {@link java.util.Map}.
>> *
>> * @version $Rev$ $Date$
>> */
>> public class MapEditor
>> extends TextPropertyEditorSupport
>> {
>> + private static final Log log = LogFactory.getLog(MapEditor.class);
>> /**
>> *
>> * @throws PropertyEditorException An IOException occured.
>> @@ -50,11 +55,30 @@
>> Map map = (Map) getValue();
>> if (!(map instanceof Properties)) {
>> Properties p = new Properties();
>> - if (map != null) {
>> - p.putAll(map);
>> + if(map != null) {
>> + if(!map.containsKey(null) && !map.containsValue(null)) {
>> + p.putAll(map);
>> + } else {
>> + // Map contains null keys or values. Replace
>> null with empty string.
>> + log.warn("Map contains null keys or values.
>> Replacing null values with empty string.");
>> + for(Iterator itr = map.entrySet().iterator();
>> itr.hasNext(); ) {
>> + Map.Entry entry = (Map.Entry) itr.next();
>> + Object key = entry.getKey();
>> + Object value = entry.getValue();
>> + if(key == null) {
>> + key = "";
>> + }
>> + if(value == null) {
>> + value = "";
>> + }
>> + p.put(key, value);
>> + }
>> + }
>> + map = p;
>> }
>> - map = p;
>> }
>> - return map.toString();
>> + PropertiesEditor pe = new PropertiesEditor();
>> + pe.setValue(map);
>> + return pe.getAsText();
>> }
>> }
>>
>>
>
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Vamsavardhana Reddy <c1...@gmail.com>.
Jason,
Don't worry. I did not take it as personal. Infact I am glad that you
commented on my practice and it will help in improving things. This
discussion would be fruitful if we suggest some best practices for
Geronimo. As I have already mentioned, "all our efforts are to improve
things and make life easy (for a lot of others)". :o)
--vamsi
On 11/28/06, Jason Dillon <ja...@planet57.com> wrote:
>
> Aight... well, I still think that brief change context should be included
> in svn commits. I have said it before... and no doubt I will say it again
> sometime.
> Anyways, nothing personal, not picking on you... was just providing some
> comments to help improve the ability to provide oversight of svn via change
> log emails.
>
> --jason
>
>
> On Nov 27, 2006, at 10:08 PM, Vamsavardhana Reddy wrote:
>
> By copy+paste, I did not mean copying the JIRA content and pasting it into
> the change log. I meant to say, "anyway I am including the summary of
> change in JIRA comment while resolving the JIRA. I will copy+paste the same
> into svn change log."
>
> --vamsi
>
> On 11/28/06, Jason Dillon <ja...@planet57.com> wrote:
> >
> > I don't think it is appropriate to simply copy/paste the jira content
> > into the svn change comment. I expect the jira issue to contain a lot more
> > details and some comments about the change, where I expect the svn change
> > comment to briefly explain the change.
> > --jason
> >
> >
> > On Nov 27, 2006, at 9:57 PM, Vamsavardhana Reddy wrote:
> >
> > Hi Jason,
> >
> > Sometime ago I had difficulty in figuring out what revision(s) fixed a
> > particular JIRA and vice versa. So, I have made it a practice to include
> > the JIRA number in the change log and svn revision numbers in the JIRA
> > comments so that someone investigating the JIRA or the svn revision will be
> > able to figureout what is happening either from the JIRA or the svn change
> > log without much difficulty. I thought this is complete & sufficient and is
> > definitely an improvement on what was followed earlier. For those revisions
> > that are not related to a JIRA, I have included a description of the change
> > in the change log itself. Since no one had any complaints on this, I have
> > followed it till now. Adding full details (or a summary) of the change to
> > the change log would not be much of a overhead and it amounts to one more
> > Copy+Paste. I agree that adding a summary of changes along with a link to
> > where full details can be found (like the JIRA number) is definitely a
> > further improvement and will be followed from now on. After all, all our
> > efforts are to improve things and make life easy (for a lot of others).
> >
> > --vamsi
> >
> > On 11/28/06, Jason Dillon < jason@planet57.com> wrote:
> > >
> > > Please... please... please... put more context into the commit log.
> > >
> > > "MapEditor does not work" tells me nothing about what was actually
> > > changed. When auditing changes it helps to have some context to the
> > > change in the commit log so that its easy to comprehend what the
> > > change was, with out having to dig around, or bounce into JIRA, etc.
> > >
> > > I've mentioned this a few times before... while its is good to
> > > include the JIRA issue ID, only including that ID, or in this case
> > > the ID and the issue subject, in the SVN commit message is not
> > > sufficient.
> > >
> > > Please... please... please... try to put some more meaningful context
> > > into commit messages.
> > >
> > > Thanks,
> > >
> > > --jason
> > >
> > >
> > > On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
> > >
> > > > Author: vamsic007
> > > > Date: Mon Nov 27 02:54:38 2006
> > > > New Revision: 479586
> > > >
> > > > URL: http://svn.apache.org/viewvc?view=rev&rev=479586
> > > > Log:
> > > > GERONIMO-2458 MapEditor does not work
> > > >
> > > > Modified:
> > > > geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > >
> > > > apache/geronimo/common/propertyeditor/MapEditor.java
> > > >
> > > > Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
> > > > java/org/apache/geronimo/common/propertyeditor/MapEditor.java
> > > > URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> > > > geronimo-common/src/main/java/org/apache/geronimo/common/
> > > >
> > > propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
> > > >
> > > ======================================================================
> > > > ========
> > > > --- geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > >
> > > > apache/geronimo/common/propertyeditor/MapEditor.java (original)
> > > > +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > > > apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27
> > > > 02:54:38 2006
> > > > @@ -19,17 +19,22 @@
> > > >
> > > > import java.io.ByteArrayInputStream;
> > > > import java.io.IOException;
> > > > +import java.util.Iterator;
> > > > import java.util.Properties;
> > > > import java.util.Map;
> > > >
> > > > +import org.apache.commons.logging.Log ;
> > > > +import org.apache.commons.logging.LogFactory;
> > > > +
> > > > /**
> > > > - * A property editor for {@link java.util.Properties}.
> > > > + * A property editor for {@link java.util.Map}.
> > > > *
> > > > * @version $Rev$ $Date$
> > > > */
> > > > public class MapEditor
> > > > extends TextPropertyEditorSupport
> > > > {
> > > > + private static final Log log = LogFactory.getLog
> > > > (MapEditor.class);
> > > > /**
> > > > *
> > > > * @throws PropertyEditorException An IOException occured.
> > > > @@ -50,11 +55,30 @@
> > > > Map map = (Map) getValue();
> > > > if (!(map instanceof Properties)) {
> > > > Properties p = new Properties();
> > > > - if (map != null) {
> > > > - p.putAll(map);
> > > > + if(map != null) {
> > > > + if(!map.containsKey(null) && !map.containsValue
> > > > (null)) {
> > > > + p.putAll(map);
> > > > + } else {
> > > > + // Map contains null keys or values. Replace
> > > > null with empty string.
> > > > + log.warn("Map contains null keys or values.
> > > > Replacing null values with empty string.");
> > > > + for(Iterator itr = map.entrySet().iterator();
> > > > itr.hasNext(); ) {
> > > > + Map.Entry entry = (Map.Entry) itr.next();
> > > > + Object key = entry.getKey();
> > > > + Object value = entry.getValue();
> > > > + if(key == null) {
> > > > + key = "";
> > > > + }
> > > > + if(value == null) {
> > > > + value = "";
> > > > + }
> > > > + p.put(key, value);
> > > > + }
> > > > + }
> > > > + map = p;
> > > > }
> > > > - map = p;
> > > > }
> > > > - return map.toString();
> > > > + PropertiesEditor pe = new PropertiesEditor();
> > > > + pe.setValue(map);
> > > > + return pe.getAsText();
> > > > }
> > > > }
> > > >
> > > >
> > >
> > >
> >
> >
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Jason Dillon <ja...@planet57.com>.
Aight... well, I still think that brief change context should be
included in svn commits. I have said it before... and no doubt I
will say it again sometime.
Anyways, nothing personal, not picking on you... was just providing
some comments to help improve the ability to provide oversight of svn
via change log emails.
--jason
On Nov 27, 2006, at 10:08 PM, Vamsavardhana Reddy wrote:
> By copy+paste, I did not mean copying the JIRA content and pasting
> it into the change log. I meant to say, "anyway I am including the
> summary of change in JIRA comment while resolving the JIRA. I will
> copy+paste the same into svn change log."
>
> --vamsi
>
> On 11/28/06, Jason Dillon <ja...@planet57.com> wrote:
> I don't think it is appropriate to simply copy/paste the jira
> content into the svn change comment. I expect the jira issue to
> contain a lot more details and some comments about the change,
> where I expect the svn change comment to briefly explain the change.
>
> --jason
>
>
> On Nov 27, 2006, at 9:57 PM, Vamsavardhana Reddy wrote:
>
>> Hi Jason,
>>
>> Sometime ago I had difficulty in figuring out what revision(s)
>> fixed a particular JIRA and vice versa. So, I have made it a
>> practice to include the JIRA number in the change log and svn
>> revision numbers in the JIRA comments so that someone
>> investigating the JIRA or the svn revision will be able to
>> figureout what is happening either from the JIRA or the svn change
>> log without much difficulty. I thought this is complete &
>> sufficient and is definitely an improvement on what was followed
>> earlier. For those revisions that are not related to a JIRA, I
>> have included a description of the change in the change log
>> itself. Since no one had any complaints on this, I have followed
>> it till now. Adding full details (or a summary) of the change to
>> the change log would not be much of a overhead and it amounts to
>> one more Copy+Paste. I agree that adding a summary of changes
>> along with a link to where full details can be found (like the
>> JIRA number) is definitely a further improvement and will be
>> followed from now on. After all, all our efforts are to improve
>> things and make life easy (for a lot of others).
>>
>> --vamsi
>>
>> On 11/28/06, Jason Dillon < jason@planet57.com> wrote:
>> Please... please... please... put more context into the commit log.
>>
>> "MapEditor does not work" tells me nothing about what was actually
>> changed. When auditing changes it helps to have some context to the
>> change in the commit log so that its easy to comprehend what the
>> change was, with out having to dig around, or bounce into JIRA, etc.
>>
>> I've mentioned this a few times before... while its is good to
>> include the JIRA issue ID, only including that ID, or in this case
>> the ID and the issue subject, in the SVN commit message is not
>> sufficient.
>>
>> Please... please... please... try to put some more meaningful context
>> into commit messages.
>>
>> Thanks,
>>
>> --jason
>>
>>
>> On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
>>
>> > Author: vamsic007
>> > Date: Mon Nov 27 02:54:38 2006
>> > New Revision: 479586
>> >
>> > URL: http://svn.apache.org/viewvc?view=rev&rev=479586
>> > Log:
>> > GERONIMO-2458 MapEditor does not work
>> >
>> > Modified:
>> > geronimo/server/trunk/modules/geronimo-common/src/main/java/
>> org/
>> > apache/geronimo/common/propertyeditor/MapEditor.java
>> >
>> > Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
>> > java/org/apache/geronimo/common/propertyeditor/MapEditor.java
>> > URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
>> > geronimo-common/src/main/java/org/apache/geronimo/common/
>> > propertyeditor/MapEditor.java?
>> view=diff&rev=479586&r1=479585&r2=479586
>> >
>> =====================================================================
>> =
>> > ========
>> > --- geronimo/server/trunk/modules/geronimo-common/src/main/java/
>> org/
>> > apache/geronimo/common/propertyeditor/MapEditor.java (original)
>> > +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/
>> org/
>> > apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27
>> > 02:54:38 2006
>> > @@ -19,17 +19,22 @@
>> >
>> > import java.io.ByteArrayInputStream;
>> > import java.io.IOException;
>> > +import java.util.Iterator;
>> > import java.util.Properties;
>> > import java.util.Map;
>> >
>> > +import org.apache.commons.logging.Log ;
>> > +import org.apache.commons.logging.LogFactory;
>> > +
>> > /**
>> > - * A property editor for {@link java.util.Properties}.
>> > + * A property editor for {@link java.util.Map}.
>> > *
>> > * @version $Rev$ $Date$
>> > */
>> > public class MapEditor
>> > extends TextPropertyEditorSupport
>> > {
>> > + private static final Log log = LogFactory.getLog
>> > (MapEditor.class);
>> > /**
>> > *
>> > * @throws PropertyEditorException An IOException occured.
>> > @@ -50,11 +55,30 @@
>> > Map map = (Map) getValue();
>> > if (!(map instanceof Properties)) {
>> > Properties p = new Properties();
>> > - if (map != null) {
>> > - p.putAll(map);
>> > + if(map != null) {
>> > + if(!map.containsKey(null) && !map.containsValue
>> > (null)) {
>> > + p.putAll(map);
>> > + } else {
>> > + // Map contains null keys or values. Replace
>> > null with empty string.
>> > + log.warn("Map contains null keys or values.
>> > Replacing null values with empty string.");
>> > + for(Iterator itr = map.entrySet().iterator();
>> > itr.hasNext(); ) {
>> > + Map.Entry entry = (Map.Entry) itr.next();
>> > + Object key = entry.getKey();
>> > + Object value = entry.getValue();
>> > + if(key == null) {
>> > + key = "";
>> > + }
>> > + if(value == null) {
>> > + value = "";
>> > + }
>> > + p.put(key, value);
>> > + }
>> > + }
>> > + map = p;
>> > }
>> > - map = p;
>> > }
>> > - return map.toString();
>> > + PropertiesEditor pe = new PropertiesEditor();
>> > + pe.setValue(map);
>> > + return pe.getAsText();
>> > }
>> > }
>> >
>> >
>>
>>
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Vamsavardhana Reddy <c1...@gmail.com>.
By copy+paste, I did not mean copying the JIRA content and pasting it into
the change log. I meant to say, "anyway I am including the summary of
change in JIRA comment while resolving the JIRA. I will copy+paste the same
into svn change log."
--vamsi
On 11/28/06, Jason Dillon <ja...@planet57.com> wrote:
>
> I don't think it is appropriate to simply copy/paste the jira content into
> the svn change comment. I expect the jira issue to contain a lot more
> details and some comments about the change, where I expect the svn change
> comment to briefly explain the change.
> --jason
>
>
> On Nov 27, 2006, at 9:57 PM, Vamsavardhana Reddy wrote:
>
> Hi Jason,
>
> Sometime ago I had difficulty in figuring out what revision(s) fixed a
> particular JIRA and vice versa. So, I have made it a practice to include
> the JIRA number in the change log and svn revision numbers in the JIRA
> comments so that someone investigating the JIRA or the svn revision will be
> able to figureout what is happening either from the JIRA or the svn change
> log without much difficulty. I thought this is complete & sufficient and is
> definitely an improvement on what was followed earlier. For those revisions
> that are not related to a JIRA, I have included a description of the change
> in the change log itself. Since no one had any complaints on this, I have
> followed it till now. Adding full details (or a summary) of the change to
> the change log would not be much of a overhead and it amounts to one more
> Copy+Paste. I agree that adding a summary of changes along with a link to
> where full details can be found (like the JIRA number) is definitely a
> further improvement and will be followed from now on. After all, all our
> efforts are to improve things and make life easy (for a lot of others).
>
> --vamsi
>
> On 11/28/06, Jason Dillon <ja...@planet57.com> wrote:
> >
> > Please... please... please... put more context into the commit log.
> >
> > "MapEditor does not work" tells me nothing about what was actually
> > changed. When auditing changes it helps to have some context to the
> > change in the commit log so that its easy to comprehend what the
> > change was, with out having to dig around, or bounce into JIRA, etc.
> >
> > I've mentioned this a few times before... while its is good to
> > include the JIRA issue ID, only including that ID, or in this case
> > the ID and the issue subject, in the SVN commit message is not
> > sufficient.
> >
> > Please... please... please... try to put some more meaningful context
> > into commit messages.
> >
> > Thanks,
> >
> > --jason
> >
> >
> > On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
> >
> > > Author: vamsic007
> > > Date: Mon Nov 27 02:54:38 2006
> > > New Revision: 479586
> > >
> > > URL: http://svn.apache.org/viewvc?view=rev&rev=479586
> > > Log:
> > > GERONIMO-2458 MapEditor does not work
> > >
> > > Modified:
> > > geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > > apache/geronimo/common/propertyeditor/MapEditor.java
> > >
> > > Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
> > > java/org/apache/geronimo/common/propertyeditor/MapEditor.java
> > > URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> > > geronimo-common/src/main/java/org/apache/geronimo/common/
> > > propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
> >
> > > ======================================================================
> > > ========
> > > --- geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > > apache/geronimo/common/propertyeditor/MapEditor.java (original)
> > > +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > > apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27
> > > 02:54:38 2006
> > > @@ -19,17 +19,22 @@
> > >
> > > import java.io.ByteArrayInputStream;
> > > import java.io.IOException;
> > > +import java.util.Iterator;
> > > import java.util.Properties;
> > > import java.util.Map;
> > >
> > > +import org.apache.commons.logging.Log ;
> > > +import org.apache.commons.logging.LogFactory;
> > > +
> > > /**
> > > - * A property editor for {@link java.util.Properties}.
> > > + * A property editor for {@link java.util.Map}.
> > > *
> > > * @version $Rev$ $Date$
> > > */
> > > public class MapEditor
> > > extends TextPropertyEditorSupport
> > > {
> > > + private static final Log log = LogFactory.getLog
> > > (MapEditor.class);
> > > /**
> > > *
> > > * @throws PropertyEditorException An IOException occured.
> > > @@ -50,11 +55,30 @@
> > > Map map = (Map) getValue();
> > > if (!(map instanceof Properties)) {
> > > Properties p = new Properties();
> > > - if (map != null) {
> > > - p.putAll(map);
> > > + if(map != null) {
> > > + if(!map.containsKey(null) && !map.containsValue
> > > (null)) {
> > > + p.putAll(map);
> > > + } else {
> > > + // Map contains null keys or values. Replace
> > > null with empty string.
> > > + log.warn("Map contains null keys or values.
> > > Replacing null values with empty string.");
> > > + for(Iterator itr = map.entrySet().iterator();
> > > itr.hasNext(); ) {
> > > + Map.Entry entry = (Map.Entry) itr.next();
> > > + Object key = entry.getKey();
> > > + Object value = entry.getValue();
> > > + if(key == null) {
> > > + key = "";
> > > + }
> > > + if(value == null) {
> > > + value = "";
> > > + }
> > > + p.put(key, value);
> > > + }
> > > + }
> > > + map = p;
> > > }
> > > - map = p;
> > > }
> > > - return map.toString();
> > > + PropertiesEditor pe = new PropertiesEditor();
> > > + pe.setValue(map);
> > > + return pe.getAsText();
> > > }
> > > }
> > >
> > >
> >
> >
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Jason Dillon <ja...@planet57.com>.
I don't think it is appropriate to simply copy/paste the jira content
into the svn change comment. I expect the jira issue to contain a
lot more details and some comments about the change, where I expect
the svn change comment to briefly explain the change.
--jason
On Nov 27, 2006, at 9:57 PM, Vamsavardhana Reddy wrote:
> Hi Jason,
>
> Sometime ago I had difficulty in figuring out what revision(s)
> fixed a particular JIRA and vice versa. So, I have made it a
> practice to include the JIRA number in the change log and svn
> revision numbers in the JIRA comments so that someone investigating
> the JIRA or the svn revision will be able to figureout what is
> happening either from the JIRA or the svn change log without much
> difficulty. I thought this is complete & sufficient and is
> definitely an improvement on what was followed earlier. For those
> revisions that are not related to a JIRA, I have included a
> description of the change in the change log itself. Since no one
> had any complaints on this, I have followed it till now. Adding
> full details (or a summary) of the change to the change log would
> not be much of a overhead and it amounts to one more Copy+Paste. I
> agree that adding a summary of changes along with a link to where
> full details can be found (like the JIRA number) is definitely a
> further improvement and will be followed from now on. After all,
> all our efforts are to improve things and make life easy (for a lot
> of others).
>
> --vamsi
>
> On 11/28/06, Jason Dillon <ja...@planet57.com> wrote:
> Please... please... please... put more context into the commit log.
>
> "MapEditor does not work" tells me nothing about what was actually
> changed. When auditing changes it helps to have some context to the
> change in the commit log so that its easy to comprehend what the
> change was, with out having to dig around, or bounce into JIRA, etc.
>
> I've mentioned this a few times before... while its is good to
> include the JIRA issue ID, only including that ID, or in this case
> the ID and the issue subject, in the SVN commit message is not
> sufficient.
>
> Please... please... please... try to put some more meaningful context
> into commit messages.
>
> Thanks,
>
> --jason
>
>
> On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
>
> > Author: vamsic007
> > Date: Mon Nov 27 02:54:38 2006
> > New Revision: 479586
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=479586
> > Log:
> > GERONIMO-2458 MapEditor does not work
> >
> > Modified:
> > geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > apache/geronimo/common/propertyeditor/MapEditor.java
> >
> > Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
> > java/org/apache/geronimo/common/propertyeditor/MapEditor.java
> > URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> > geronimo-common/src/main/java/org/apache/geronimo/common/
> > propertyeditor/MapEditor.java?
> view=diff&rev=479586&r1=479585&r2=479586
> >
> ======================================================================
> > ========
> > --- geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > apache/geronimo/common/propertyeditor/MapEditor.java (original)
> > +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27
> > 02:54:38 2006
> > @@ -19,17 +19,22 @@
> >
> > import java.io.ByteArrayInputStream;
> > import java.io.IOException;
> > +import java.util.Iterator;
> > import java.util.Properties;
> > import java.util.Map;
> >
> > +import org.apache.commons.logging.Log ;
> > +import org.apache.commons.logging.LogFactory;
> > +
> > /**
> > - * A property editor for {@link java.util.Properties}.
> > + * A property editor for {@link java.util.Map}.
> > *
> > * @version $Rev$ $Date$
> > */
> > public class MapEditor
> > extends TextPropertyEditorSupport
> > {
> > + private static final Log log = LogFactory.getLog
> > (MapEditor.class);
> > /**
> > *
> > * @throws PropertyEditorException An IOException occured.
> > @@ -50,11 +55,30 @@
> > Map map = (Map) getValue();
> > if (!(map instanceof Properties)) {
> > Properties p = new Properties();
> > - if (map != null) {
> > - p.putAll(map);
> > + if(map != null) {
> > + if(!map.containsKey(null) && !map.containsValue
> > (null)) {
> > + p.putAll(map);
> > + } else {
> > + // Map contains null keys or values. Replace
> > null with empty string.
> > + log.warn("Map contains null keys or values.
> > Replacing null values with empty string.");
> > + for(Iterator itr = map.entrySet().iterator();
> > itr.hasNext(); ) {
> > + Map.Entry entry = (Map.Entry) itr.next();
> > + Object key = entry.getKey();
> > + Object value = entry.getValue();
> > + if(key == null) {
> > + key = "";
> > + }
> > + if(value == null) {
> > + value = "";
> > + }
> > + p.put(key, value);
> > + }
> > + }
> > + map = p;
> > }
> > - map = p;
> > }
> > - return map.toString();
> > + PropertiesEditor pe = new PropertiesEditor();
> > + pe.setValue(map);
> > + return pe.getAsText();
> > }
> > }
> >
> >
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Vamsavardhana Reddy <c1...@gmail.com>.
Hi Jason,
Sometime ago I had difficulty in figuring out what revision(s) fixed a
particular JIRA and vice versa. So, I have made it a practice to include
the JIRA number in the change log and svn revision numbers in the JIRA
comments so that someone investigating the JIRA or the svn revision will be
able to figureout what is happening either from the JIRA or the svn change
log without much difficulty. I thought this is complete & sufficient and is
definitely an improvement on what was followed earlier. For those revisions
that are not related to a JIRA, I have included a description of the change
in the change log itself. Since no one had any complaints on this, I have
followed it till now. Adding full details (or a summary) of the change to
the change log would not be much of a overhead and it amounts to one more
Copy+Paste. I agree that adding a summary of changes along with a link to
where full details can be found (like the JIRA number) is definitely a
further improvement and will be followed from now on. After all, all our
efforts are to improve things and make life easy (for a lot of others).
--vamsi
On 11/28/06, Jason Dillon <ja...@planet57.com> wrote:
>
> Please... please... please... put more context into the commit log.
>
> "MapEditor does not work" tells me nothing about what was actually
> changed. When auditing changes it helps to have some context to the
> change in the commit log so that its easy to comprehend what the
> change was, with out having to dig around, or bounce into JIRA, etc.
>
> I've mentioned this a few times before... while its is good to
> include the JIRA issue ID, only including that ID, or in this case
> the ID and the issue subject, in the SVN commit message is not
> sufficient.
>
> Please... please... please... try to put some more meaningful context
> into commit messages.
>
> Thanks,
>
> --jason
>
>
> On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
>
> > Author: vamsic007
> > Date: Mon Nov 27 02:54:38 2006
> > New Revision: 479586
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=479586
> > Log:
> > GERONIMO-2458 MapEditor does not work
> >
> > Modified:
> > geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > apache/geronimo/common/propertyeditor/MapEditor.java
> >
> > Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
> > java/org/apache/geronimo/common/propertyeditor/MapEditor.java
> > URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> > geronimo-common/src/main/java/org/apache/geronimo/common/
> > propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
> > ======================================================================
> > ========
> > --- geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > apache/geronimo/common/propertyeditor/MapEditor.java (original)
> > +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> > apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27
> > 02:54:38 2006
> > @@ -19,17 +19,22 @@
> >
> > import java.io.ByteArrayInputStream;
> > import java.io.IOException;
> > +import java.util.Iterator;
> > import java.util.Properties;
> > import java.util.Map;
> >
> > +import org.apache.commons.logging.Log;
> > +import org.apache.commons.logging.LogFactory;
> > +
> > /**
> > - * A property editor for {@link java.util.Properties}.
> > + * A property editor for {@link java.util.Map}.
> > *
> > * @version $Rev$ $Date$
> > */
> > public class MapEditor
> > extends TextPropertyEditorSupport
> > {
> > + private static final Log log = LogFactory.getLog
> > (MapEditor.class);
> > /**
> > *
> > * @throws PropertyEditorException An IOException occured.
> > @@ -50,11 +55,30 @@
> > Map map = (Map) getValue();
> > if (!(map instanceof Properties)) {
> > Properties p = new Properties();
> > - if (map != null) {
> > - p.putAll(map);
> > + if(map != null) {
> > + if(!map.containsKey(null) && !map.containsValue
> > (null)) {
> > + p.putAll(map);
> > + } else {
> > + // Map contains null keys or values. Replace
> > null with empty string.
> > + log.warn("Map contains null keys or values.
> > Replacing null values with empty string.");
> > + for(Iterator itr = map.entrySet().iterator();
> > itr.hasNext(); ) {
> > + Map.Entry entry = (Map.Entry) itr.next();
> > + Object key = entry.getKey();
> > + Object value = entry.getValue();
> > + if(key == null) {
> > + key = "";
> > + }
> > + if(value == null) {
> > + value = "";
> > + }
> > + p.put(key, value);
> > + }
> > + }
> > + map = p;
> > }
> > - map = p;
> > }
> > - return map.toString();
> > + PropertiesEditor pe = new PropertiesEditor();
> > + pe.setValue(map);
> > + return pe.getAsText();
> > }
> > }
> >
> >
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Jason Dillon <ja...@planet57.com>.
Please... please... please... put more context into the commit log.
"MapEditor does not work" tells me nothing about what was actually
changed. When auditing changes it helps to have some context to the
change in the commit log so that its easy to comprehend what the
change was, with out having to dig around, or bounce into JIRA, etc.
I've mentioned this a few times before... while its is good to
include the JIRA issue ID, only including that ID, or in this case
the ID and the issue subject, in the SVN commit message is not
sufficient.
Please... please... please... try to put some more meaningful context
into commit messages.
Thanks,
--jason
On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
> Author: vamsic007
> Date: Mon Nov 27 02:54:38 2006
> New Revision: 479586
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=479586
> Log:
> GERONIMO-2458 MapEditor does not work
>
> Modified:
> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> apache/geronimo/common/propertyeditor/MapEditor.java
>
> Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
> java/org/apache/geronimo/common/propertyeditor/MapEditor.java
> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> geronimo-common/src/main/java/org/apache/geronimo/common/
> propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
> ======================================================================
> ========
> --- geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> apache/geronimo/common/propertyeditor/MapEditor.java (original)
> +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27
> 02:54:38 2006
> @@ -19,17 +19,22 @@
>
> import java.io.ByteArrayInputStream;
> import java.io.IOException;
> +import java.util.Iterator;
> import java.util.Properties;
> import java.util.Map;
>
> +import org.apache.commons.logging.Log;
> +import org.apache.commons.logging.LogFactory;
> +
> /**
> - * A property editor for {@link java.util.Properties}.
> + * A property editor for {@link java.util.Map}.
> *
> * @version $Rev$ $Date$
> */
> public class MapEditor
> extends TextPropertyEditorSupport
> {
> + private static final Log log = LogFactory.getLog
> (MapEditor.class);
> /**
> *
> * @throws PropertyEditorException An IOException occured.
> @@ -50,11 +55,30 @@
> Map map = (Map) getValue();
> if (!(map instanceof Properties)) {
> Properties p = new Properties();
> - if (map != null) {
> - p.putAll(map);
> + if(map != null) {
> + if(!map.containsKey(null) && !map.containsValue
> (null)) {
> + p.putAll(map);
> + } else {
> + // Map contains null keys or values. Replace
> null with empty string.
> + log.warn("Map contains null keys or values.
> Replacing null values with empty string.");
> + for(Iterator itr = map.entrySet().iterator();
> itr.hasNext(); ) {
> + Map.Entry entry = (Map.Entry) itr.next();
> + Object key = entry.getKey();
> + Object value = entry.getValue();
> + if(key == null) {
> + key = "";
> + }
> + if(value == null) {
> + value = "";
> + }
> + p.put(key, value);
> + }
> + }
> + map = p;
> }
> - map = p;
> }
> - return map.toString();
> + PropertiesEditor pe = new PropertiesEditor();
> + pe.setValue(map);
> + return pe.getAsText();
> }
> }
>
>
Re: svn commit: r479586 - /geronimo/server/trunk/modules/geronimo-common/src/main/java/org/apache/geronimo/common/propertyeditor/MapEditor.java
Posted by Jason Dillon <ja...@planet57.com>.
Please... please... please... put more context into the commit log.
"MapEditor does not work" tells me nothing about what was actually
changed. When auditing changes it helps to have some context to the
change in the commit log so that its easy to comprehend what the
change was, with out having to dig around, or bounce into JIRA, etc.
I've mentioned this a few times before... while its is good to
include the JIRA issue ID, only including that ID, or in this case
the ID and the issue subject, in the SVN commit message is not
sufficient.
Please... please... please... try to put some more meaningful context
into commit messages.
Thanks,
--jason
On Nov 27, 2006, at 2:54 AM, vamsic007@apache.org wrote:
> Author: vamsic007
> Date: Mon Nov 27 02:54:38 2006
> New Revision: 479586
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=479586
> Log:
> GERONIMO-2458 MapEditor does not work
>
> Modified:
> geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> apache/geronimo/common/propertyeditor/MapEditor.java
>
> Modified: geronimo/server/trunk/modules/geronimo-common/src/main/
> java/org/apache/geronimo/common/propertyeditor/MapEditor.java
> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/modules/
> geronimo-common/src/main/java/org/apache/geronimo/common/
> propertyeditor/MapEditor.java?view=diff&rev=479586&r1=479585&r2=479586
> ======================================================================
> ========
> --- geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> apache/geronimo/common/propertyeditor/MapEditor.java (original)
> +++ geronimo/server/trunk/modules/geronimo-common/src/main/java/org/
> apache/geronimo/common/propertyeditor/MapEditor.java Mon Nov 27
> 02:54:38 2006
> @@ -19,17 +19,22 @@
>
> import java.io.ByteArrayInputStream;
> import java.io.IOException;
> +import java.util.Iterator;
> import java.util.Properties;
> import java.util.Map;
>
> +import org.apache.commons.logging.Log;
> +import org.apache.commons.logging.LogFactory;
> +
> /**
> - * A property editor for {@link java.util.Properties}.
> + * A property editor for {@link java.util.Map}.
> *
> * @version $Rev$ $Date$
> */
> public class MapEditor
> extends TextPropertyEditorSupport
> {
> + private static final Log log = LogFactory.getLog
> (MapEditor.class);
> /**
> *
> * @throws PropertyEditorException An IOException occured.
> @@ -50,11 +55,30 @@
> Map map = (Map) getValue();
> if (!(map instanceof Properties)) {
> Properties p = new Properties();
> - if (map != null) {
> - p.putAll(map);
> + if(map != null) {
> + if(!map.containsKey(null) && !map.containsValue
> (null)) {
> + p.putAll(map);
> + } else {
> + // Map contains null keys or values. Replace
> null with empty string.
> + log.warn("Map contains null keys or values.
> Replacing null values with empty string.");
> + for(Iterator itr = map.entrySet().iterator();
> itr.hasNext(); ) {
> + Map.Entry entry = (Map.Entry) itr.next();
> + Object key = entry.getKey();
> + Object value = entry.getValue();
> + if(key == null) {
> + key = "";
> + }
> + if(value == null) {
> + value = "";
> + }
> + p.put(key, value);
> + }
> + }
> + map = p;
> }
> - map = p;
> }
> - return map.toString();
> + PropertiesEditor pe = new PropertiesEditor();
> + pe.setValue(map);
> + return pe.getAsText();
> }
> }
>
>