Nice one, thanks Swapnil! Regards Scott
On Thu, 21 Mar 2019, 01:25 Swapnil M Mane, <swapnilmm...@apache.org> wrote: > Done with the suggested changes Scott. > > trunk at rev 1855898 > release18.12 at rev 1855899 > release17.12 at rev 1855900 > release16.11 at rev 1855901 > > Thanks again for valuable inputs. > > - Best Regards, > Swapnil M Mane, > ofbiz.apache.org > > > > On Fri, Mar 15, 2019 at 9:52 AM Swapnil M Mane <swapnilmm...@apache.org> > wrote: > > > Makes perfect sense, thanks so much for the input Scott. > > I will do the suggested changes. > > > > > > - Best Regards, > > Swapnil M Mane, > > ofbiz.apache.org > > > > > > > > On Fri, Mar 15, 2019 at 5:36 AM Scott Gray <scott.g...@hotwaxsystems.com > > > > wrote: > > > >> Hi Swapnil, > >> > >> A minor thing, but I wonder if it would be better to move this check > >> underneath node.getTextContent()? If we assume that most nodes will > >> contain text then it might be more efficient to do a null check after > >> retrieving nodeValue. > >> > >> e.g. > >> Node node = (Node) obj; > >> String nodeValue = node.getTextContent(); > >> if (nodeValue == null) { > >> // Check node type and return obj; > >> } > >> if ("String".equals(type) || "java.lang.String".equals(type)) { > >> return nodeValue; > >> ... > >> > >> Regards > >> Scott > >> > >> > >> On Thu, 14 Mar 2019, 01:27 , <swapnilmm...@apache.org> wrote: > >> > >> > Author: swapnilmmane > >> > Date: Wed Mar 13 12:27:13 2019 > >> > New Revision: 1855403 > >> > > >> > URL: http://svn.apache.org/viewvc?rev=1855403&view=rev > >> > Log: > >> > Fixed: simpleTypeConvert always returns Null for Document, Document > Type > >> > and Notation Node (OFBIZ-10832) > >> > > >> > As per the code, getTextContent() method is used get text content of > the > >> > node and its descendants but the node.getTextContent() always return > >> Null > >> > for the following Node type > >> > DOCUMENT_NODE, DOCUMENT_TYPE_NODE, NOTATION_NODE [1] > >> > > >> > Since we can't get the text value of Document, Document Type and > >> Notation > >> > Node, thus simply return the same object. > >> > > >> > [1] > >> > > >> > https://docs.oracle.com/javase/7/docs/api/org/w3c/dom/Node.html#getTextContent() > >> > > >> > Thanks: Deepak Dixit for reviewing the code. > >> > > >> > Modified: > >> > > >> > > >> > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java > >> > > >> > Modified: > >> > > >> > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java > >> > URL: > >> > > >> > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java?rev=1855403&r1=1855402&r2=1855403&view=diff > >> > > >> > > >> > ============================================================================== > >> > --- > >> > > >> > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java > >> > (original) > >> > +++ > >> > > >> > ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java > >> > Wed Mar 13 12:27:13 2019 > >> > @@ -289,6 +289,17 @@ public class ObjectType { > >> > } > >> > if (obj instanceof Node) { > >> > Node node = (Node) obj; > >> > + > >> > + /* We can't get the text value of Document, Document Type > >> and > >> > Notation Node, > >> > + * thus simply returning the same object from > >> > simpleTypeOrObjectConvert method. > >> > + * Please refer to OFBIZ-10832 Jira and > >> > + * > >> > > >> > https://docs.oracle.com/javase/7/docs/api/org/w3c/dom/Node.html#getTextContent() > >> > for more details. > >> > + */ > >> > + short nodeType = node.getNodeType(); > >> > + if (Node.DOCUMENT_NODE == nodeType || > >> Node.DOCUMENT_TYPE_NODE > >> > == nodeType || Node.NOTATION_NODE == nodeType) { > >> > + return obj; > >> > + } > >> > + > >> > String nodeValue = node.getTextContent(); > >> > if ("String".equals(type) || > >> "java.lang.String".equals(type)) > >> > { > >> > return nodeValue; > >> > > >> > > >> > > >> > > >