I finaly reverted because of https://issues.apache.org/jira/browse/OFBIZ-4637.
I guess only a 1st detected symptom
I will see later what the problem was and will try to redo Patrick's work, it's
maybe just because
set(name, "");
was used, instead
element.setAttribute(name, "null");
Jacques
From: "Jacques Le Roux" <[email protected]>
I must say, I was also surprised by the GenericEntity.set(name, "") for String. As I know it fixed an issue with Entity Sync, as I
discussed with Patrick, it made sense to me to commit as is (though I did not had a chance to test it, which would be rather
complicated)
For the fall through cases, I don't see them as "unused switch statements", simply fall through cases using the same (final)
expression. They had an idea about which types correspond to cases, which I don't see as useless...
Sorry it's my last answer on this, I will leave for vacation tomorrow morning...
If you think it should be reverted, no pb with me, I will see that ater
BTW, maybe the 1st case should be handled by element.setAttribute(name, "") (preferred to element.setAttribute(name, "null") not
sure about that yet). But then I wonder why Patrick had to use set(name, ""). Because I know this is used in real world somewhere
for Entity Sync success...
I agree this needs to be clarified
Jacques
From: "Adrian Crum" <[email protected]>
No, it is not a syntax concern. The commit does not make sense.
For a String, you are calling
GenericEntity.set(name, "");
and for all other types you are calling:
Element.setAttribute(name, "null");
Why should a String data type be treated any differently than other data types? A null value is a null value - it doesn't have a
"type" other than "null".
And no, an empty unused switch statement does NOT help document anything - all
it does is add confusion.
-Adrian
On 12/22/2011 9:39 PM, Jacques Le Roux wrote:
Ha, it's only a syntax concern?
Actually I don't plenty agree. The String does not use element.setAttribute() and I wanted to make clear what the others types
are. Unlike Patrick, I used a fall through switch. Also if ever we need to distinguish cases later if will be easier (unlikely
but anyway the doc. aspect lead me to this prefer this syntax)
Jacques
From: "Adrian Crum" <[email protected]>
It looks like a hack because it has a long switch statement that doesn't do anything, and in one condition the set method is
being called while in another condition an element's set method is being called.
In other words, this could be easily expressed as:
element.setAttribute(name, value == null ? "null" : value);
-Adrian
On 12/22/2011 6:12 PM, Jacques Le Roux wrote:
Patrick crossed issues with Entity sync and null values and needed to address
them. I trust his work.
Why does "it looks like an ugly hack" to you, a feeling, or? Do you expect some surprises with this work? How would you
address the problem of null values, then ?
Jacques
From: "Adrian Crum" <[email protected]>
I don't like this commit - it looks like an ugly hack to me. What is the end result supposed to be? Or what is this supposed
to do?
-Adrian
On 12/22/2011 2:14 PM, [email protected] wrote:
Author: jleroux
Date: Thu Dec 22 14:14:57 2011
New Revision: 1222241
URL: http://svn.apache.org/viewvc?rev=1222241&view=rev
Log:
A modified patch from Patrick Antivackis "Null values are not synchronized in http mode"
https://issues.apache.org/jira/browse/OFBIZ-4602
In order to send over http the values to create, store and remove, Ofbiz is Xml serializing the values. GenericValue xml
serialization is managed in GenericEntity.makeXmlElement, unfortunately this method just don't serialized null valued
fields.
To solve this issue, I managed null value the same way GenericEntity.setString (which is used for the Xml deserializing). I
only managed until case 10, because i'm not sure the setString for the cases 11 to 15 are well managed (not taking care of
null value)
jleroux: After looking at GenericEntity.setString and initial commit (http://svn.ofbiz.org/viewcvs?rev=7779&view=rev) I see
no reasons to not handling cases under 10 the same way. So I added these cases as well using fall through.
Modified:
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java
Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java?rev=1222241&r1=1222240&r2=1222241&view=diff
==============================================================================
---
ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java Thu
Dec 22 14:14:57 2011
@@ -1077,6 +1077,42 @@ public class GenericEntity extends Obser
element.setAttribute(name, value);
}
}
+ else {
+ ModelFieldType type = null;
+ try {
+ type = getDelegator().getEntityFieldType(getModelEntity(),
modelField.getType());
+ } catch (GenericEntityException e) {
+ Debug.logWarning(e, module);
+ }
+ if (type == null) throw new IllegalArgumentException("Type " +
modelField.getType() + " not found");
+ String fieldType = type.getJavaType();
+
+ try {
+ switch (SqlJdbcUtil.getType(fieldType)) {
+ case 1: // String
+ set(name, "");
+ break;
+ case 2: // Timestamp
+ case 3: // Time
+ case 4: // java.sql.Date
+ case 5: // Integer
+ case 6: // Long
+ case 7: // Float
+ case 8: // Double
+ case 9: // BigDecimal
+ case 10:// Boolean
+ case 11:// Object
+ case 12:// Blob, byte[], ByteBuffer, HeapByteBuffer
+ case 13:// Clob
+ case 14:// java.util.Date
+ case 15:// Collection: ArrayList, HashSet, LinkedHashSet,
LinkedList
+ element.setAttribute(name, "null");
+ break;
+ }
+ } catch (GenericNotImplementedException ex) {
+ throw new IllegalArgumentException(ex.getMessage());
+ }
+ }
}
return element;
