hugoloza / gwt-platform

Automatically exported from code.google.com/p/gwt-platform
0 stars 0 forks source link

Spring Test Context Issue With Duplicate ActionHandlerValidatorMap (in 0.6, 0.7 and head) #420

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago

What steps will reproduce the problem?
======================================
1. Create a Spring Server module (by extending 
com.gwtplatform.dispatch.server.spring.HandlerModule) with at least TWO 
actions+handlers

2. Prepare a spring integration test::

  @RunWith(SpringJUnit4ClassRunner.class)
  @ContextConfiguration("/testContext.xml")
  public class SpringTestCase() {...}

3. Run it

What is the expected output? What do you see instead?
=====================================================
Regular run of the test. Instead I've got an error:
`java.lang.IllegalStateException: Failed to load ApplicationContext` with root 
cause:
`java.lang.IllegalStateException: Could not register object 
[com.gwtplatform.dispatch.server.actionhandlervalidator.ActionHandlerValidatorMa
pImpl@13785d3] under bean name 
'com.gwtplatform.dispatch.server.actionhandlervalidator.ActionHandlerValidatorMa
pImpl#0': there is already object 
[com.gwtplatform.dispatch.server.actionhandlervalidator.ActionHandlerValidatorMa
pImpl@32efa7] bound`

What version of the product are you using? On what operating system?
====================================================================
gwtp-dispatch-server-spring-{0.6, 0.7, 0.8-SNAPSHOT} on Win7 x86

::
  > java -version
  java version "1.6.0_31"
  Java(TM) SE Runtime Environment (build 1.6.0_31-b05)
  Java HotSpot(TM) Client VM (build 20.6-b01, mixed mode, sharing)

Please provide any additional information below.
================================================

It only occurs with two or more handler defined in 
`com.gwtplatform.dispatch.server.spring.HandlerModule`

The issue was already mentioned on the gwtp list:
https://groups.google.com/d/topic/gwt-platform/NHwPG0MWJVw/discussion

I'm attaching a test case (a patch to 0.7) - it modifies the basic-spring 
sample to include a second action + handler and a simple test. You can then run 
the test to reproduce the issue.

I'm also attaching a report of running the test in eclipse.

Thanks,
Igor Kupczyński

Original issue reported on code.google.com by puszczyk on 11 May 2012 at 9:29

Attachments:

GoogleCodeExporter commented 9 years ago
OK, I'm attaching a patch for the issue - it can be applied to the test cased 
posted with the issue to make it work.

The problem
===========

The problem is as follows. When binding a handler (via bindHandler in 
HandlerModule) a bean is registered for each pair of Handler-Action. The class 
of these beans is `ActionHandlerValidatorMapImpl`. By default spring gives a 
bean an id equal to its class name, so having more than one Handler-Action pair 
results in a collision of the generated id. When the spring context is run from 
a web app the SpringUtils class takes care of changing the bean name:

  public static <B> void registerBean(ApplicationContext applicationContext,
      B instance) throws BeansException {

    if (applicationContext instanceof GenericApplicationContext) {
      (...)
    } else if (applicationContext instanceof AbstractRefreshableWebApplicationContext) {
        ConfigurableListableBeanFactory beanFactory = ((AbstractRefreshableWebApplicationContext) applicationContext).getBeanFactory();
        beanFactory.registerSingleton(*generateName(beanFactory, createBeanDefinition(instance))*, instance);
    }
  }

private static String generateName(ConfigurableListableBeanFactory registry,
      RootBeanDefinition definition) {
    (...)

    String id = generatedBeanName;

    // Top-level bean: use plain class name.
    // Increase counter until the id is unique.
    int counter = -1;
    while (counter == -1 || (registry.containsSingleton(id))) {
      counter++;
      id = generatedBeanName + "#" + counter;
    }

    return id;
  }

Unfortunately, this does not apply to GenericApplicationContext.

A solution
==========

A solution is to add a counter value to both beans registered in 
`GenericApplicationContext` and `AbstractRefreshableWebApplicationContext`. 
I've created a class for this very reason:

public enum GwtpBeanNameGenerator {

    INSTANCE;

    private static AtomicInteger counter = new AtomicInteger(0);

    public String generateBeanName(BeanDefinition definition,
            BeanFactory factory) {
        String generatedBeanName = definition.getBeanClassName();
        if (generatedBeanName == null) {
            if (definition.getParentName() != null) {
                generatedBeanName = definition.getParentName() + "$child";
            } else if (definition.getFactoryBeanName() != null) {
                generatedBeanName = definition.getFactoryBeanName()
                        + "$created";
            }
        }
        if (!StringUtils.hasText(generatedBeanName)) {
            throw new BeanDefinitionStoreException(
                    "Unnamed bean definition specifies neither "
                            + "'class' nor 'parent' nor 'factory-bean' - can't generate bean name");
        }

        return generatedBeanName + "#" + counter.getAndIncrement();
    }

}

It can be used as follows (in SpringUtils):

    public static <B> void registerBean(ApplicationContext applicationContext,
            B instance) throws BeansException {

        if (applicationContext instanceof GenericApplicationContext) {
            DefaultListableBeanFactory beanFactory = ((GenericApplicationContext) applicationContext)
                    .getDefaultListableBeanFactory();
            beanFactory.registerSingleton(GwtpBeanNameGenerator.INSTANCE
                    .generateBeanName(createBeanDefinition(instance),
                            beanFactory), instance);
        } else if (applicationContext instanceof AbstractRefreshableWebApplicationContext) {
            ConfigurableListableBeanFactory beanFactory = ((AbstractRefreshableWebApplicationContext) applicationContext)
                    .getBeanFactory();
            beanFactory.registerSingleton(GwtpBeanNameGenerator.INSTANCE
                    .generateBeanName(createBeanDefinition(instance),
                            beanFactory), instance);
        }
    }

For convenience I'm attaching both classes as well. I've tested in unit tests 
and my two apps and it seems to work.

Hope it can help someone,
Igor Kupczynki

Original comment by puszczyk on 11 May 2012 at 4:15

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks!

Before merging this, could you ask for a code review on codereview.appspot.com?

Original comment by goudreau...@gmail.com on 23 May 2012 at 2:23

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hi, I've added the patch for a review:
https://codereview.appspot.com/6256073/

Thanks,
Igor

Original comment by puszczyk on 30 May 2012 at 9:59

GoogleCodeExporter commented 9 years ago
I've run into this issue as well. The root cause of the bug is that in 
SpringUtils.registerBean() the beans are registered as singletons 
(beanFactory.registerSingleton()) but for GenericApplicationContext the 
DefaultBeanNameGenerator is used. The DefaultBeanNameGenerator looks for beans 
in the registry as normal beans rather than singletons, so it doesn't find the 
pre-existing bean and increment the suffix count.

The generateBeanName() method in SpringUtils looks like a copy of 
BeanDefinitionReaderUtils.generateBeanName() but instead of doing the bean 
lookup as a normal bean is looks them up as a singleton bean instead (as it 
should since it registers them as singletons).

The fix should be to simply use SpringUtils.generateBeanName() for both the web 
context and generic context. If you want to compare SpringUtils.generateName() 
is here:

http://code.google.com/p/gwt-platform/source/browse/gwtp-core/gwtp-dispatch-serv
er-spring/src/main/java/com/gwtplatform/dispatch/server/spring/utils/SpringUtils
.java?r=27fb7e23931e91c46532fefb48c55a9163ebbdd6#84

And the DefaultBeanNameGenerator version (which just calls 
BeanDefinitionReaderUtils.generateBeanName()) is here:

http://grepcode.com/file/repo1.maven.org/maven2/org.springframework/spring-beans
/3.0.5.RELEASE/org/springframework/beans/factory/support/BeanDefinitionReaderUti
ls.java#BeanDefinitionReaderUtils.generateBeanName%28org.springframework.beans.f
actory.config.BeanDefinition%2Corg.springframework.beans.factory.support.BeanDef
initionRegistry%2Cboolean%29

Have you found a good way to work around this issue or did you just patch your 
GWTP install?

Original comment by jeraym...@gmail.com on 30 Aug 2012 at 3:42