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;
> >> >
> >> >
> >> >
> >>
> >
>

Reply via email to