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

Add support for common value objects and records to ValueCodeGenerator #32263

Conversation

Christopher-Chianelli
Copy link

Add support for generating code to create common value Objects and Records. A common value Record is a public, accessible Record class that has common value objects for all of its components. A common value Object is a public, accessible Object class that has public setters for all of its non-public instance fields, with each field being assigned a common value.

The code generated for a common value Record calls the Record's canonical constructor with the generated code for each of its component. For instance, for record MyRecord(String a, int b), and instance MyRecord("Example", 1), it would generate the following code:

    new MyRecord("Example", 1)

The code generated for a common value Object calls the Object's public no-args constructor and sets its fields. Public fields are set by direct access. Non-public fields are set by calling their setters. The code will be wrapped by an inlined Supplier so the generated code can be used anywhere. For example, given the following class:

    class MyObject {
        public String name;
        private int age;

        public void setAge(int age) {
            this.age = age;
        }
    }

The code generated for MyObject("Example", 2) would be

    ((Supplier<MyObject>) () -> {
        MyObject $valueCodeGeneratorObject1 = new MyObject();
        $valueCodeGeneratorObject1.setAge(2);
        $valueCodeGeneratorObject1.name = "Example";
        return $valueCodeGeneratorObject1;
    }).get()

ValueCodeGenerator uses a ThreadLocal keeping track of the number of generateCode calls in the current Thread's stack. This allows the Delegate for Object to use an unused identifier for the variable holding the Object under construction.

Known limitations are that cyclic references cannot be handled. If a cyclic reference is detected, a ValueCodeGenerationException will be raised.

Additionally, if the same Object is used for multiple fields/components in Objects/Records, they will be different instances instead of the same instance.

This change will allow common value objects and records to be passed to the constructor of BeanDefinitions in an AOT native image. For instance, the following will now work in native image, provided myConfig is a common value object:

    MyConfig myConfig = new MyConfig();
    // ...Set fields of MyConfig ...
    RootBeanDefinition rootBeanDefinition = new RootBeanDefinition(
        MyBean.class);
    rootBeanDefinition.setFactoryMethodName("of");
    rootBeanDefinition.getConstructorArgumentValues()
       .addGenericArgumentValue(myConfig);
    registry.registerBeanDefinition(BEAN_NAME,
        rootBeanDefinition);

Add support for generating code to create common value Objects and
Records. A common value Record is a public, accessible Record class
that has common value objects for all of its components. A common
value Object is a public, accessible Object class that has public
setters for all of its non-public instance fields, with each field
being assigned a common value.

The code generated for a common value Record calls the Record's
canonical constructor with the generated code for each of its
component. For instance, for `record MyRecord(String a, int b)`,
and instance `MyRecord("Example", 1)`, it would generate the
following code:

    new MyRecord("Example", 1)

The code generated for a common value Object calls the Object's
public no-args constructor and sets its fields. Public fields are
set by direct access. Non-public fields are set by calling their
setters. The code will be wrapped by an inlined Supplier so the
generated code can be used anywhere. For example, given the
following class:

    class MyObject {
        public String name;
        private int age;

        public void setAge(int age) {
            this.age = age;
        }
    }

The code generated for MyObject("Example", 2) would be

    ((Supplier<MyObject>) () -> {
        MyObject $valueCodeGeneratorObject1 = new MyObject();
        $valueCodeGeneratorObject1.setAge(2);
        $valueCodeGeneratorObject1.name = "Example";
        return $valueCodeGeneratorObject1;
    }).get()

ValueCodeGenerator uses a ThreadLocal keeping track of
the number of generateCode calls in the current Thread's stack.
This allows the Delegate for Object to use an unused identifier
for the variable holding the Object under construction.

Known limitations are that cyclic references cannot be handled.
If a cyclic reference is detected, a ValueCodeGenerationException
will be raised.

Additionally, if the same Object is used for multiple
fields/components in Objects/Records, they will be different
instances instead of the same instance.

This change will allow common value objects and records to be
passed to the constructor of BeanDefinitions in an AOT native image.
For instance, the following will now work in native image, provided
`myConfig` is a common value object:

    MyConfig myConfig = new MyConfig();
    // ...Set fields of MyConfig ...
    RootBeanDefinition rootBeanDefinition = new RootBeanDefinition(
        MyBean.class);
    rootBeanDefinition.setFactoryMethodName("of");
    rootBeanDefinition.getConstructorArgumentValues()
       .addGenericArgumentValue(myConfig);
    registry.registerBeanDefinition(BEAN_NAME,
        rootBeanDefinition);
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 14, 2024
@sbrannen sbrannen added the theme: aot An issue related to Ahead-of-time processing label Feb 14, 2024
@sbrannen sbrannen changed the title Add support for common value Objects and Records to ValueCodeGenerator Add support for common value objects and records to ValueCodeGenerator Feb 14, 2024
@snicoll
Copy link
Member

snicoll commented Feb 14, 2024

Thanks for the PR.

Add support for generating code to create common value Objects and Records.

Do I understand that what this does is taking an existing class structure and attempt to restore its state in a generic fashion? If so, we have no intention to bring that complexity to the core framework. Assuming that the record has been initialized via some code of yours, is there something specific that prevents you from getting the expected result?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Feb 14, 2024
@snicoll snicoll changed the title Add support for common value objects and records to ValueCodeGenerator Add support for common value objects and records to ValueCodeGenerator Feb 14, 2024
@sbrannen sbrannen changed the title Add support for common value objects and records to ValueCodeGenerator Add support for common value objects and records to ValueCodeGenerator Feb 14, 2024
@bclozel bclozel changed the title Add support for common value objects and records to ValueCodeGenerator Add support for common value objects and records to ValueCodeGenerator Feb 14, 2024
@snicoll
Copy link
Member

snicoll commented Feb 14, 2024

Do I understand that what this does is taking an existing class structure and attempt to restore its state in a generic fashion?

Actually, I can answer my own question. @Christopher-Chianelli before spending time to submit such a large PR, please consider raising an issue first to discuss the merit of the change, especially as we already had a conversation where I thought I made it clear that we didn't want to do that.

If you're still having an issue with the record re-created with your own code, please raise an issue with more details.

@snicoll snicoll closed this Feb 14, 2024
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 14, 2024
@Christopher-Chianelli
Copy link
Author

Christopher-Chianelli commented Feb 14, 2024

Thanks for the PR.

Add support for generating code to create common value Objects and Records.

Do I understand that what this does is taking an existing class structure and attempt to restore its state in a generic fashion? If so, we have no intention to bring that complexity to the core framework. Assuming that the record has been initialized via some code of yours, is there something specific that prevents you from getting the expected result?

JAX-B seems to be unsupported in native image (GraalVM issue) without either including a 9000+ lines reflect-config.json file or running native-image with the agent (see https://stackoverflow.com/questions/77969978/how-to-use-jaxb-in-native-image-without-using-agent). A part for why reflect-config.json is so large is that @RegisterReflectionForBinding does not seem to pick up library classes outside the application (did not registers implementations of interfaces inside lists). Cannot contribute the hints via AOTContribution either, since EntityScanner will also not pick up the implementations. Said file cannot be easily generated, since classes from the integration test it is generated from and unneeded classes should be removed from the agent output. Since users of our library should work with the default spring boot configuration, that removes the option of enabling the agent. The code path used to generate the bean in question involves parsing an XML file, and hence, a special route need to be taken in native if a record/value class cannot be passed to a constructor to avoid the largely unmaintainable reflect-config.json file.

Do I understand that what this does is taking an existing class structure and attempt to restore its state in a generic fashion?

Actually, I can answer my own question. @Christopher-Chianelli before spending time to submit such a large PR, please consider raising an issue first to discuss the merit of the change, especially as we already had a conversation where I thought I made it clear that we didn't want to do that.

If you're still having an issue with the record re-created with your own code, please raise an issue with more details.

Per the second point of https://github.com/spring-projects/spring-framework/blob/main/CONTRIBUTING.md#submit-a-pull-request , I figured I might as create a PR since the majority of the work was already done when submitting the equivalent of this PR upstream to JavaPoet (square/javapoet#999), and I figured other users of Spring stands to benefit from it, since it allows use cases such as

    @Override
    public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException {
            MyRecord myRecord = generateRecord();
            RootBeanDefinition rootBeanDefinition = new RootBeanDefinition(MyBean.class);
            rootBeanDefinition.setFactoryMethodName("from");
            rootBeanDefinition.getConstructorArgumentValues().addGenericArgumentValue(
                    myRecord);
            registry.registerBeanDefinition("MyBean", rootBeanDefinition);
    }

to also work in native image (currently fails with ValueCodeGenerationException when generating myRecord).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply theme: aot An issue related to Ahead-of-time processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants