Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spring inconsistently resolves an overloaded setter method [SPR-4931] #9606

Closed
spring-projects-issues opened this issue Jun 18, 2008 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jun 18, 2008

Fred Muhlenberg opened SPR-4931 and commented

Reference: http://forum.springframework.org/showthread.php?t=47051

I'm running into an issue with an overloaded set method.
I get different method resolutions depending on the environment in which I run.

I am using the MimeMessageHelper class and am setting the replyTo property and have sample code demonstrating my issue when configuring.

If I run in eclipse, the property requires the String version of the setter.
If I debug in eclipse, the property requires the InternetAddress version of the setter.

If I run from the command line, the property requires the InternetAddress version of the setter.
If I run on our (ancient) 1.4 Oracle App server I need the String version of the setter.

Same source files, same config files, different results.

I assert that if I pass in a bean reference, Spring ought to resolve to an overloaded method to the type of the reference.

My sample test:


Directory Hierarchy


./.classpath
./.project
./build.xml
./lib/activation.jar
./lib/commons-logging-1.0.4.jar
./lib/j2ee.jar
./lib/junit-3.8.1.jar
./lib/junit-4.4.jar
./lib/log4j-1.2.14.jar
./lib/mail.jar
./lib/spring-mock.jar
./lib/spring.jar
./test/java/context-one.xml
./test/java/context-two.xml
./test/java/log4j.properties
./test/java/mderf/MockMimeMessageHelper.java
./test/java/mderf/OneTest.java
./test/java/mderf/TwoTest.java


build.xml
<project name="mailproto" default="all" basedir="." >

<property name="lib.dir"         location="lib" />

<property name="build.dir"           location="build" />
<property name="build.classes.dir"   location="${build.dir}" />
<property name="build.report.dir"    location="${build.dir}/report" />

<property name="test.dir"         location="test" />
<property name="test.java.dir"    location="${test.dir}/java" />

<path id="compile.classpath">
    <fileset dir="${lib.dir}">
        <include name="**/*.jar"/>

<!-- <exclude name="**/junit-4.4.jar"/> -->

        <exclude name="**/junit-3.8.1.jar"/>
    </fileset>
</path>    

<path id="test.classpath">
    <path refid="compile.classpath" />
    <path location="${build.classes.dir}" />
</path>


<target name="init">
    <mkdir dir="${build.classes.dir}"/>
    <mkdir dir="${build.report.dir}"/>
</target>


<target name="all" depends="run">

</target>
    
<target name="compile" depends="init">
    
    <javac
        destdir="${build.classes.dir}"
        srcdir="${test.java.dir}"
    >
        <classpath refid="compile.classpath" />
    </javac>
    
    <copy todir="${build.classes.dir}">
        <fileset dir="${test.java.dir}">
            <include name="*.xml"/>
        </fileset>
    </copy>

</target>

<target name="run" depends="compile">
    
    <junit
        printsummary="true"
        fork="yes" 
        haltonerror="false"
        showoutput="true"
    >
        <classpath refid="test.classpath"/>
        <formatter type="xml"/>
        <batchtest todir="${build.report.dir}">
            <fileset dir="${build.dir}">
                <include name="**/*Test.class"/>
            </fileset>
        </batchtest>

    </junit>
    
    <junitreport todir="${build.report.dir}">
        <fileset dir="${build.report.dir}">
            <include name="TEST-*.xml"/>
        </fileset>
        <report format="frames" todir="${build.report.dir}"/>
    </junitreport>
    
    <echo>

The JUnit report can be found in ${build.report.dir}/index.html

    </echo>
    
</target>

<target name="clean">

    <delete dir="${build.dir}"/>
    
</target>

</project>


context-one.xml

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE beans PUBLIC "-//SPRING/DTD BEAN/EN" "http://www.springframework.org/dtd/spring-beans.dtd">
<beans>
<!--!-- **************************************** -->

<!-- Beans associated with email notification -->
<!-- **************************************** -->

<!-- Address of email server -->
<bean id="javaMailSender" class="org.springframework.mail.javamail.JavaMailSenderImpl">
    <property name="host"><value>localhost</value></property>
</bean>

<bean id="mimeMessage"
    factory-bean="javaMailSender"
    factory-method="createMimeMessage"/>

<bean id="toMimeMessageHelper" class="mderf.MockMimeMessageHelper"> 
    <constructor-arg type="javax.mail.internet.MimeMessage">
        <ref bean="mimeMessage"/>
    </constructor-arg>
    <property name="replyTo" ref="replyTo"/>

<!-- <property name="replyTo" value="no-reply@localhost"/> -->

    <property name="subject" value="Attention:  Change Request"/>
</bean>

<!-- Reply address for automated messages -->
<bean id="replyTo" class="javax.mail.internet.InternetAddress">
    <property name="address" value="no-reply@localhost"/>
    <property name="personal" value="Do Not Reply (Automated Message)"/>
</bean>

</beans>


context-two.xml

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE beans PUBLIC "-//SPRING/DTD BEAN/EN" "http://www.springframework.org/dtd/spring-beans.dtd">
<beans>
<!--!-- **************************************** -->

<!-- Beans associated with email notification -->
<!-- **************************************** -->

<!-- Address of email server -->
<bean id="javaMailSender" class="org.springframework.mail.javamail.JavaMailSenderImpl">
    <property name="host"><value>localhost</value></property>
</bean>

<bean id="mimeMessage"
    factory-bean="javaMailSender"
    factory-method="createMimeMessage"/>

<bean id="toMimeMessageHelper" class="mderf.MockMimeMessageHelper"> 
    <constructor-arg type="javax.mail.internet.MimeMessage">
        <ref bean="mimeMessage"/>
    </constructor-arg>

<!-- <property name="replyTo" ref="replyTo"/> -->

       <property name="replyTo" value="no-reply@localhost"/>
    <property name="subject" value="Attention:  Change Request"/>
</bean>

<!-- Reply address for automated messages -->
<bean id="replyTo" class="javax.mail.internet.InternetAddress">
    <property name="address" value="no-reply@localhost"/>
    <property name="personal" value="Do Not Reply (Automated Message)"/>
</bean>

</beans>


log4j.properties

1. direct log messages to stdout ###

log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.Target=System.out
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=%d{ABSOLUTE} %5p %c{1}:%L - %m%n

1. direct messages to file hibernate.log ###

#log4j.appender.file=org.apache.log4j.FileAppender
#log4j.appender.file.File=hibernate.log
#log4j.appender.file.layout=org.apache.log4j.PatternLayout
#log4j.appender.file.layout.ConversionPattern=%d{ABSOLUTE} %5p %c{1}:%L - %m%n

1. set log levels - for more verbose logging change 'info' to 'debug' ###

log4j.rootLogger=warn, stdout


mderf/MockMimeMessageHelper.java
/*

  • MockMimeMessageHelper.java
  • Created: Dec 11, 2007
  • Version: 1.0
    */
    package mderf;

import javax.mail.MessagingException;
import javax.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage;

import org.springframework.mail.javamail.MimeMessageHelper;

/**

  • This class

  • @author Fred Muhlenberg, High Performance Technologies, Inc.
    */
    public class MockMimeMessageHelper extends MimeMessageHelper
    {
    /**

    • @param arg0
      */
      public MockMimeMessageHelper( MimeMessage arg0 )
      {
      super( arg0 );
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setFrom(javax.mail.internet.InternetAddress)
      */
      @Override
      public void setFrom( InternetAddress arg0 ) throws MessagingException
      {
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setFrom(java.lang.String)
      */
      @Override
      public void setFrom( String arg0 ) throws MessagingException
      {
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setReplyTo(javax.mail.internet.InternetAddress)
      */
      @Override
      public void setReplyTo( InternetAddress arg0 ) throws MessagingException
      {
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setReplyTo(java.lang.String)
      */
      @Override
      public void setReplyTo( String arg0 ) throws MessagingException
      {
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setSubject(java.lang.String)
      */
      @Override
      public void setSubject( String arg0 ) throws MessagingException
      {
      }

    /**

    • @param arg0
    • @param arg1
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setText(java.lang.String, boolean)
      */
      @Override
      public void setText( String arg0, boolean arg1 ) throws MessagingException
      {
      setText( arg0 );
      }

    /**

    • @param arg0
    • @param arg1
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setText(java.lang.String, java.lang.String)
      */
      @Override
      public void setText( String arg0, String arg1 ) throws MessagingException
      {
      setText( arg0 );
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setText(java.lang.String)
      */
      @Override
      public void setText( String arg0 ) throws MessagingException
      {
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setTo(javax.mail.internet.InternetAddress)
      */
      @Override
      public void setTo( InternetAddress arg0 ) throws MessagingException
      {
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setTo(javax.mail.internet.InternetAddress[])
      */
      @Override
      public void setTo( InternetAddress[] arg0 ) throws MessagingException
      {
      for( InternetAddress ia : arg0 )
      {
      setTo( ia );
      }
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setTo(java.lang.String)
      */
      @Override
      public void setTo( String arg0 ) throws MessagingException
      {
      }

    /**

    • @param arg0
    • @throws MessagingException
    • @see org.springframework.mail.javamail.MimeMessageHelper#setTo(java.lang.String[])
      */
      @Override
      public void setTo( String[] arg0 ) throws MessagingException
      {
      for( String str : arg0 )
      {
      setTo( str );
      }
      }

}


mderf/OneTest.java
package mderf;

import org.junit.Test;
import org.springframework.test.AbstractSingleSpringContextTests;

public class OneTest extends AbstractSingleSpringContextTests
{

protected String[] getConfigLocations()
{
    return new String[]{ "context-one.xml" };
}
        
@Test
public void testDummy()
{
    System.out.println( "Dummy test" );
}

}


mderf/TwoTest.java
package mderf;

import org.junit.Test;
import org.springframework.context.support.ClassPathXmlApplicationContext;

public class TwoTest
{

protected String[] getConfigLocations()
{
    return new String[]{ "context-two.xml" };
}
        
@Test
public void testDummy()
{
    new ClassPathXmlApplicationContext( getConfigLocations() );
    System.out.println( "Dummy output" );
}

}



Affects: 2.0.2, 2.5 final, 2.5.4

Attachments:

Issue Links:

Referenced from: commits 4a25e2d

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 18, 2008

Fred Muhlenberg commented

Didn't see this option earlier.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 19, 2008

Juergen Hoeller commented

The problem here is that the JavaBeans convention does not support overloaded setter methods to begin with. Each bean property must have exactly one (or none at all) setter method associated with. We'll reconsider our JavaBeans support there for Spring 3.0, possibly going beyond the standard JavaBeans conventions...

For that reason, MimeMessageHelper isn't a valid JavaBean - and it's not intended to be one in the first place. Note that MimeMessageHelper wraps a JavaMail MimeMessage object - one at a time - so it's supposed to be used as a programmatic wrapper object only. In particular, do not access a MimeMessageHelper concurrently - MimeMessage and hence MimeMessageHelper are not thread-safe!

Juergen

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 19, 2008

Fred Muhlenberg commented

Given that the MimeMessageHelper isn't a valid JavaBean, should the resolution of the set method at least produce a consistent result? Instead of the situation I keep running into where deployment to the app server resolves to one set method and unit testing through Ant resolves to a different set method, all while using the same JVM.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Oct 7, 2009

john commented

Preferred - infer the setter type using the same rules the javac compiler does
... Sane and adheres to principal of least surprise

Backdoor - provide a means for the user to indicate the specific setter type to use
.... A hack really because if the approach above is taken then there's generally no surprise

At the very least - refuse to inject a property where there is ambiguity
.... Breaking change for spring.
.... my code might break and need some fixes but at least I'd have confidence the app will remain stable over time.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Nov 9, 2009

Costin Leau commented

Adding a bit of context to what Juergen said, Spring relies internally on java.beans.Introspector class for determining the accessors/mutators (getters/setters) of a particular bean, which is both a blessing and a curse since we're as complaint as possible, with respect to the JavaBeans spec yet limited to the Introspector functionality.
Any updates/replacement to its behaviour need to be thoroughly tested and they will likely break some clients depending on the Introspector behaviour.

@john - I'm not that familiar with the javac compiler but it does have access to the sources. Things at runtime are different - for example the methods are returned (through the reflection API) out of order. Thus the same class, can look differently depending on the JVM or the OS on which it runs (something which actually does happen in practice).
This impacts the way the Introspector works - and w/o any rules in place on how the sorting should be done (such as alphabetically), one is bound to either keep the unspecified behaviour in place or break the spec.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Nov 11, 2009

Juergen Hoeller commented

We're spitting out a warning on first access to such an invalid property now, in case of ambiguous setter methods found. We can't throw an exception since existing applications might rely on the present choice in which case they should softly learn about it through a warning instead.

Note that the introspection process may only result in arbitrary setter methods if no getter method has been found: Otherwise the setter whose parameter type matches the getter's return type would have been chosen upfront. This is also a way to make a custom bean property valid: If you really need overloaded setters on a class that is supposed to be used as a regular JavaBean, make sure that there is a getter as well - matching the type of the 'primary' setter.

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants