218163 - Simple properties not handled correctly

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Nicolas JOUVE]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1) Opera 7.11  [en]
Build Identifier: 

When you script a java bean, you cannot acces the bean properties for whom there 
are multiple methods having the name of the getter.
For instance if a bean has the property "name" and an additional getName(Locale 
locale) method, the "name" property won't be available when scripting the bean 
using Rhino.

Reproducible: Always

Steps to Reproduce:
Here is a TestCase exhibiting the problem:

package org.mozilla.javascript;

import java.io.StringReader;
import java.util.Locale;

import junit.framework.Assert;
import junit.framework.TestCase;

/**
 *
 * Tests Case
 * 
 * @author njouve
 * @since 3 sept. 03
 * 
 * $Log$
 */
public class TestPropertyWithMultipleGetters extends TestCase {

    /**
     * Constructor for TestPropertyWithMultipleGetters.
     * @param arg0
     */
    public TestPropertyWithMultipleGetters(String testName) {
        super(testName);
    }

    /**
     * Tests that a bean with a getter having an overloaded version 
     *
     */
    public void testPropertyHandlingWithMultipleGetters() {
        try {
            String propertyValueToSet = "John Doe";
            Context ctx = new Context();
            ctx = Context.enter(ctx);

            Script script = ctx.compileReader(null, new StringReader("this.
name=\"" + propertyValueToSet + "\""), "", 0, "");

            TestBean testBean = new TestBean();

            Scriptable thisObject = new WrapFactory().wrapNewObject(ctx, null, 
testBean);

            script.exec(ctx, thisObject);

            Assert.assertEquals(propertyValueToSet, testBean.getName());
        } catch (EvaluatorException e) {
            e.printStackTrace();
            fail("Expression should have been correctly evaluated" + e.
getMessage());
        } catch (Exception e) {
            e.printStackTrace();
            fail(e.getMessage());
        }
    }

    /**
     * This class is a bean that has a "name" property and an
     * additional method having the same name as the getter but with an 
additional
     * parameter
     * 
     *
     * 
     * @author njouve
     * @since 3 sept. 03
     * 
     * $Log$
     */
    public static class TestBean {

        public TestBean() {
        }

        private String name = null;
        /**
         * @return
         */
        public String getName() {
            return name;
        }

        /**
         * @param string
         */
        public void setName(String string) {
            name = string;
        }

        /**
         * @return
         */
        public String getName(Locale locale) {
            return null;
        }

    }

}

Actual Results:  
This test will fail with the current version, because an EvaluatorException is 
thrown when accessing the "name" property from javascript.

Expected Results:  
The test should succeed, that is no exception should be thrown

I 've found the offending lines in the core and I'd like to sublit the 
correction.
the offending class is org.mozilla.javascript.JavaMembers

 -- Line 398 -- 

 // Grab and inspect the getter method; does it have an empty parameter list,
// with a return value (eg. a getSomething() or isSomething())?
Class[] params;
Method[] getMethods = getJavaMethod.getMethods();
Class type;
if (getMethods != null &&
    getMethods.length == 1 &&
    (type = getMethods[0].getReturnType()) != null &&
    (params = getMethods[0].getParameterTypes()) != null &&
    params.length == 0)
{

It should be replaced by :

// Grab and inspect the getter method; does it have an empty parameter list,
// with a return value (eg. a getSomething() or isSomething())?
Class[] params;
Method[] getMethods = getJavaMethod.getMethods();
Class type = null;
// test each method in the set to found the only one having no parameter
for (int methodIdx = 0; methodIdx < getMethods.length; methodIdx++) {
    Method testedMethod = getMethods[methodIdx];
    if ((testedMethod.getReturnType() != null) &&
	(testedMethod.getParameterTypes() != null) &&
	(testedMethod.getParameterTypes().length == 0)) {
	    type = testedMethod.getReturnType();  
	    params = testedMethod.getParameterTypes();                          
	}
	
}
if (type != null)
{

Comment 1

[Nicolas JOUVE]

Attachment: This is the TestCase showing the bug

Comment 2

[Nicolas JOUVE]

Attachment: This is the modified version of the JavaMembers class that fixes the bug

Comment 3

[Igor Bukanov]

Nicolas, could you provide a patch against CVS tip as JavaMembers there changed
compared with Rhino 1.5R4.* ?

Comment 4

[Nicolas JOUVE]

Attachment: Patch for JavaMembers

cvs diff against latest version of JavaMembers (1.40 at time of writing)

Comment 5

[Igor Bukanov]

Few notes for the above patch:

diff -r1.40 JavaMembers.java

This diff generates a patch in the default format but patches in unified format
are more readable and resilient to source changes. Please use cvs diff -u
or even 
cvs diff -U 5 
to generate them.

>            if ((method.argTypes != null) && (method.argTypes.length == 0)) {
The check (method.argTypes != null) is redundant here since it always holds even
for 0-arity methods.

>                    if (method.argTypes.length == 0) {
This check is also redundant since it is already covered by the previous if.

Comment 6

[Nicolas JOUVE]

Attachment: Re-written patch (with redundant checks removed) in unified format

Redundant checks removed.
Unified format

Comment 7

[Igor Bukanov]

I committed Nicolas patch with some formating changes to make code to fit 80 colons.

Comment 8

[Igor Bukanov]

*** Bug 142100 has been marked as a duplicate of this bug. ***

Comment 9

[Igor Bukanov]

Trageting as resolved against 1.5R5