spring-integration icon indicating copy to clipboard operation
spring-integration copied to clipboard

GH-3903: Improve AOT for gateway proxy beans

Open artembilan opened this issue 3 years ago • 1 comments

Fixes https://github.com/spring-projects/spring-integration/issues/3903

  • Fix MessagingGatewayRegistrar to set a targetType on the bean definition instead of FactoryBean.OBJECT_TYPE_ATTRIBUTE
  • Rework GatewayProxyBeanRegistrationAotProcessor to the GatewayProxyInitializationAotProcessor implements BeanFactoryInitializationAotProcessor to avoid custom code generation
  • Remove tests which rely on the FactoryBean.OBJECT_TYPE_ATTRIBUTE

artembilan avatar Oct 04 '22 17:10 artembilan

@snicoll ,

I have figured out that I can get rid off FactoryBean.OBJECT_TYPE_ATTRIBUTE in favor of setTargetType(). Works well in plain JVM. It fails directly on AOT compilation phase just because the generated code is wrong:

 /**
   * Bean definitions for {@link IntegrationApplication.ControlBusGateway}
   */
  public static class ControlBusGateway__BeanDefinitions {
    /**
     * Get the bean instance supplier for 'integrationApplication$ControlBusGateway'.
     */
    private static BeanInstanceSupplier<IntegrationApplication.ControlBusGateway> getControlBusGatewayInstanceSupplier(
        ) {
      return BeanInstanceSupplier.<IntegrationApplication.ControlBusGateway>forConstructor(Class.class)
              .withGenerator((registeredBean, args) -> new GatewayProxyFactoryBean(args.get(0)));
    }

    /**
     * Get the bean definition for 'controlBusGateway'
     */
    public static BeanDefinition getControlBusGatewayBeanDefinition() {
      Class<?> beanType = IntegrationApplication.ControlBusGateway.class;
      RootBeanDefinition beanDefinition = new RootBeanDefinition(beanType);
      beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(0, IntegrationApplication.ControlBusGateway.class);
      beanDefinition.getPropertyValues().addPropertyValue("defaultRequestChannelName", "controlBus.input");
      beanDefinition.getPropertyValues().addPropertyValue("proxyDefaultMethods", "false");
      beanDefinition.getPropertyValues().addPropertyValue("defaultRequestTimeoutExpressionString", "-9223372036854775808");
      beanDefinition.getPropertyValues().addPropertyValue("defaultReplyTimeoutExpressionString", "-9223372036854775808");
      beanDefinition.getPropertyValues().addPropertyValue("methodMetadataMap", null);
      beanDefinition.setInstanceSupplier(getControlBusGatewayInstanceSupplier());
      return beanDefinition;
    }
  }

The beanType must be a GatewayProxyFactoryBean.

So, I leave this as a Draft for future consideration since there is indeed no good support for FactoryBean in AOT mode.

The previous version, based on the FactoryBean.OBJECT_TYPE_ATTRIBUTE looks like this:

  /**
   * Get the bean instance supplier for 'integrationApplication$ControlBusGateway'.
   */
  private static BeanInstanceSupplier<GatewayProxyFactoryBean> getControlBusGatewayInstanceSupplier(
      ) {
    return BeanInstanceSupplier.<GatewayProxyFactoryBean>forConstructor(Class.class)
            .withGenerator((registeredBean, args) -> new GatewayProxyFactoryBean(args.get(0)));
  }

  /**
   * Get the bean definition for 'controlBusGateway'
   */
  private static BeanDefinition getControlBusGatewayBeanDefinition() {
    Class<?> beanType = GatewayProxyFactoryBean.class;
    RootBeanDefinition beanDefinition = new RootBeanDefinition(beanType);
    beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(0, IntegrationApplication.ControlBusGateway.class);
    beanDefinition.getPropertyValues().addPropertyValue("defaultRequestChannelName", "controlBus.input");
    beanDefinition.getPropertyValues().addPropertyValue("proxyDefaultMethods", "false");
    beanDefinition.getPropertyValues().addPropertyValue("defaultRequestTimeoutExpressionString", "-9223372036854775808");
    beanDefinition.getPropertyValues().addPropertyValue("defaultReplyTimeoutExpressionString", "-9223372036854775808");
    beanDefinition.getPropertyValues().addPropertyValue("methodMetadataMap", null);
    beanDefinition.setAttribute("factoryBeanObjectType", IntegrationApplication.ControlBusGateway.class);
    beanDefinition.setInstanceSupplier(getControlBusGatewayInstanceSupplier());
    return beanDefinition;
  }

Thanks for understanding

artembilan avatar Oct 04 '22 17:10 artembilan

It looks like with generic argument we now expose on the ProxyFactoryBean, it works with AOT even without the mentioned FactoryBean.OBJECT_TYPE_ATTRIBUTE and without custom code. In the end it looks like AOT generates a correct working code:

    private static BeanInstanceSupplier<GatewayProxyFactoryBean> getControlBusGatewayInstanceSupplier(
        ) {
      return BeanInstanceSupplier.<GatewayProxyFactoryBean>forConstructor(Class.class)
              .withGenerator((registeredBean, args) -> new GatewayProxyFactoryBean(args.get(0)));
    }

    public static BeanDefinition getControlBusGatewayBeanDefinition() {
      ResolvableType beanType = ResolvableType.forClassWithGenerics(GatewayProxyFactoryBean.class, IntegrationApplication.ControlBusGateway.class);
      RootBeanDefinition beanDefinition = new RootBeanDefinition(beanType);
      beanDefinition.getConstructorArgumentValues().addIndexedArgumentValue(0, IntegrationApplication.ControlBusGateway.class);
      beanDefinition.getPropertyValues().addPropertyValue("defaultRequestChannelName", "controlBus.input");
      beanDefinition.getPropertyValues().addPropertyValue("proxyDefaultMethods", "false");
      beanDefinition.getPropertyValues().addPropertyValue("defaultRequestTimeoutExpressionString", "-9223372036854775808");
      beanDefinition.getPropertyValues().addPropertyValue("defaultReplyTimeoutExpressionString", "-9223372036854775808");
      beanDefinition.getPropertyValues().addPropertyValue("methodMetadataMap", null);
      beanDefinition.setInstanceSupplier(getControlBusGatewayInstanceSupplier());
      return beanDefinition;
    }

artembilan avatar Oct 26 '22 19:10 artembilan

@garyrussell ,

I think this is good to go. Probably @snicoll is busy with other stuff. No worries: the fix is addressed exactly as it is expected in the issue description.

Thanks

artembilan avatar Nov 07 '22 16:11 artembilan