[ 
https://issues.apache.org/jira/browse/OGNL-102?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Lukasz Lenart updated OGNL-102:
-------------------------------
    Description: 
In the "for" loop, we did not check the result is null or not. if it's null 
(returned "result = children[i].getValue( context, result );

"), then next time when execute this "result = children[i].getValue( context, 
result );", it will throw one exception. if this method is used frequently, 
then one performance issue here.

suggestion to check the result here like this:


{code:java}
    protected Object getValueBody(OgnlContext context, Object source)
            throws OgnlException
    {
        Object result = source;

        for(int i = 0, ilast = _children.length - 1; i <= ilast; ++i)
        {

            if(result == null) {
                break;
            }

            boolean handled = false;

            if (i < ilast) {
                if (_children[i] instanceof ASTProperty) {
                    ASTProperty propertyNode = (ASTProperty) _children[i];
                    int indexType = 
propertyNode.getIndexedPropertyType(context, result);

                    if ((indexType != OgnlRuntime.INDEXED_PROPERTY_NONE) && 
(_children[i + 1] instanceof ASTProperty)) {
                        ASTProperty indexNode = (ASTProperty) _children[i + 1];

                        if (indexNode.isIndexedAccess()) {
                            Object index = indexNode.getProperty(context, 
result);

                            if (index instanceof DynamicSubscript) {
                                if (indexType == 
OgnlRuntime.INDEXED_PROPERTY_INT) {
                                    Object array = 
propertyNode.getValue(context, result);
                                    int len = Array.getLength(array);

                                    switch(((DynamicSubscript) 
index).getFlag()) {
                                        case DynamicSubscript.ALL:
                                            result = 
Array.newInstance(array.getClass().getComponentType(), len);
                                            System.arraycopy(array, 0, result, 
0, len);
                                            handled = true;
                                            i++;
                                            break;
                                        case DynamicSubscript.FIRST:
                                            index = new Integer((len > 0) ? 0 : 
-1);
                                            break;
                                        case DynamicSubscript.MID:
                                            index = new Integer((len > 0) ? 
(len / 2) : -1);
                                            break;
                                        case DynamicSubscript.LAST:
                                            index = new Integer((len > 0) ? 
(len - 1) : -1);
                                            break;
                                    }
                                } else {
                                    if (indexType == 
OgnlRuntime.INDEXED_PROPERTY_OBJECT) { throw new OgnlException(
                                            "DynamicSubscript '" + indexNode
                                            + "' not allowed for object indexed 
property '" + propertyNode
                                            + "'"); }
                                }
                            }
                            if (!handled) 
                            {
                                result = 
OgnlRuntime.getIndexedProperty(context, result,
                                                                        
propertyNode.getProperty(context, result).toString(),
                                                                        index);
                                handled = true;
                                i++;
                            }
                        }
                    }
                }
            }
            if (!handled)
            {
                result = _children[i].getValue(context, result);
            }
        }
        return result;
    }

{code}
 

  was:
In the "for" loop, we did not check the result is null or not. if it's null 
(returned "result = children[i].getValue( context, result );

"), then next time when execute this "result = children[i].getValue( context, 
result );", it will throw one exception. if this method is used frequently, 
then one performanece issue here.



suggestion to check the result here like this:



for ( int i = 0, ilast = children.length - 1; i <= ilast; ++i ) {

           

            if(result == null)

            {

                break;

            }

            

            boolean         handled = false;



            if (i < ilast) {

                if (children[i] instanceof ASTProperty) {

                    ASTProperty     propertyNode = (ASTProperty)children[i];

                    int             indexType = 
propertyNode.getIndexedPropertyType(context, result);



                    if ((indexType != OgnlRuntime.INDEXED_PROPERTY_NONE) && 
(children[i + 1] instanceof ASTProperty)) {

                        ASTProperty     indexNode = (ASTProperty)children[i + 
1];



                        if (indexNode.isIndexedAccess()) {

                            Object      index = indexNode.getProperty(context, 
result);



                            if (index instanceof DynamicSubscript) {

                                if (indexType == 
OgnlRuntime.INDEXED_PROPERTY_INT) {

                                    Object      array = 
propertyNode.getValue(context, result);

                                    int         len = Array.getLength(array);



                                    switch 
(((DynamicSubscript)index).getFlag()) {

                                        case DynamicSubscript.ALL:

                                            result = Array.newInstance( 
array.getClass().getComponentType(), len );

                                            System.arraycopy( array, 0, result, 
0, len );

                                            handled = true;

                                            i++;

                                            break;

                                        case DynamicSubscript.FIRST:

                                            index = new Integer((len > 0) ? 0 : 
-1);

                                            break;

                                        case DynamicSubscript.MID:

                                            index = new Integer((len > 0) ? 
(len / 2) : -1);

                                            break;

                                        case DynamicSubscript.LAST:

                                            index = new Integer((len > 0) ? 
(len - 1) : -1);

                                            break;

                                    }

                                } else {

                                    if (indexType == 
OgnlRuntime.INDEXED_PROPERTY_OBJECT) {

                                        throw new 
OgnlException("DynamicSubscript '" + indexNode + "' not allowed for object 
indexed property '" + propertyNode + "'");

                                    }

                                }

                            }

                            if (!handled) {

                                result = 
OgnlRuntime.getIndexedProperty(context, result, 
propertyNode.getProperty(context, result).toString(), index);

                                handled = true;

                                i++;

                            }

                        }

                    }

                }

            }

            if (!handled) {

                result = children[i].getValue( context, result );

            }

        }


> performance for the ASTChain.getValueBody() method.
> ---------------------------------------------------
>
>                 Key: OGNL-102
>                 URL: https://issues.apache.org/jira/browse/OGNL-102
>             Project: Commons OGNL
>          Issue Type: Improvement
>    Affects Versions: 2.7
>            Reporter: Torr
>            Assignee: Jesse Kuhnert
>            Priority: Major
>
> In the "for" loop, we did not check the result is null or not. if it's null 
> (returned "result = children[i].getValue( context, result );
> "), then next time when execute this "result = children[i].getValue( context, 
> result );", it will throw one exception. if this method is used frequently, 
> then one performance issue here.
> suggestion to check the result here like this:
> {code:java}
>     protected Object getValueBody(OgnlContext context, Object source)
>             throws OgnlException
>     {
>         Object result = source;
>         for(int i = 0, ilast = _children.length - 1; i <= ilast; ++i)
>         {
>             if(result == null) {
>                 break;
>             }
>             boolean handled = false;
>             if (i < ilast) {
>                 if (_children[i] instanceof ASTProperty) {
>                     ASTProperty propertyNode = (ASTProperty) _children[i];
>                     int indexType = 
> propertyNode.getIndexedPropertyType(context, result);
>                     if ((indexType != OgnlRuntime.INDEXED_PROPERTY_NONE) && 
> (_children[i + 1] instanceof ASTProperty)) {
>                         ASTProperty indexNode = (ASTProperty) _children[i + 
> 1];
>                         if (indexNode.isIndexedAccess()) {
>                             Object index = indexNode.getProperty(context, 
> result);
>                             if (index instanceof DynamicSubscript) {
>                                 if (indexType == 
> OgnlRuntime.INDEXED_PROPERTY_INT) {
>                                     Object array = 
> propertyNode.getValue(context, result);
>                                     int len = Array.getLength(array);
>                                     switch(((DynamicSubscript) 
> index).getFlag()) {
>                                         case DynamicSubscript.ALL:
>                                             result = 
> Array.newInstance(array.getClass().getComponentType(), len);
>                                             System.arraycopy(array, 0, 
> result, 0, len);
>                                             handled = true;
>                                             i++;
>                                             break;
>                                         case DynamicSubscript.FIRST:
>                                             index = new Integer((len > 0) ? 0 
> : -1);
>                                             break;
>                                         case DynamicSubscript.MID:
>                                             index = new Integer((len > 0) ? 
> (len / 2) : -1);
>                                             break;
>                                         case DynamicSubscript.LAST:
>                                             index = new Integer((len > 0) ? 
> (len - 1) : -1);
>                                             break;
>                                     }
>                                 } else {
>                                     if (indexType == 
> OgnlRuntime.INDEXED_PROPERTY_OBJECT) { throw new OgnlException(
>                                             "DynamicSubscript '" + indexNode
>                                             + "' not allowed for object 
> indexed property '" + propertyNode
>                                             + "'"); }
>                                 }
>                             }
>                             if (!handled) 
>                             {
>                                 result = 
> OgnlRuntime.getIndexedProperty(context, result,
>                                                                         
> propertyNode.getProperty(context, result).toString(),
>                                                                         
> index);
>                                 handled = true;
>                                 i++;
>                             }
>                         }
>                     }
>                 }
>             }
>             if (!handled)
>             {
>                 result = _children[i].getValue(context, result);
>             }
>         }
>         return result;
>     }
> {code}
>  



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to