Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

Handle init callbacks #1059

Closed

Conversation

christophstrobl
Copy link
Contributor

This PR introduces support for detecting and calling bean initialization (post construct) methods.
Public, protected and package private init methods will be called directly, however private ones require addition reflection configuration.

@christophstrobl christophstrobl linked an issue Sep 22, 2021 that may be closed by this pull request
Copy link
Contributor

@snicoll snicoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I've made a few comments. Looking at it a bit closed, I've realized what I've postponed should really happen now (injection of properties).

If we merge this, we'd call post construct before the properties are actually injected :(

@@ -47,12 +48,15 @@

private final List<PropertyDescriptor> properties;

private final List<InitializationCallback> initializationCallbacks;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having looked at the end result, I think this should rather be a MemberDescriptor<Method>. This way we can move the code generation to the writer and the reflection configuration can be provided based on the descriptor.

/**
* Wraps the code that's necessary to invoce initialization (post construct) methods if any.
*/
public static class InitializationCallback {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a result this could go away.


List<InitializationCallback> detectInstanceCallbacks(BeanDefinition beanDefinition) {

if(!(beanDefinition instanceof RootBeanDefinition)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about getInitMethodName? The idea is to honor all init method we know about. It should be this, and then any externally managed, if any.


private static final Field EXTERNALLY_MANAGED_INIT_METHODS;

static {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure the optimization is worth it. We could have a method that would give us back the list based on a bean definition and hide all that reflection hackery there.


List<InitializationCallback> callbacks = new ArrayList<>();
Class<?> targetType = beanDefinition.getResolvableType().toClass();
for (Object initMethodName : (Iterable<? extends Object>) field) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should rather put everything in a LinkedHashSet and iterate over it, to avoid cases where something is both detected by an initMethodName, and also annotated.

return callbacks;
}

private InitializationCallback getInitMethodCallback(Method initMethod) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code can move to DefaultBeanInstanceSupplierWriter.

@@ -44,16 +50,12 @@
import org.springframework.nativex.type.TypeSystem;
import org.springframework.util.ClassUtils;

import static org.springframework.context.bootstrap.generator.infrastructure.nativex.NativeConfigurationRegistry.InitializationConfiguration;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we don't have a consistent code style at the moment but can you please re-read the commit to avoid such unrelated changes?

@@ -62,6 +64,7 @@
@Override
public void process(BeanInstanceDescriptor descriptor, NativeConfigurationRegistry registry) {
findAndRegisterRelevantNativeHints(descriptor.getUserBeanClass(), registry);
registerReflectionForInitializtionMethodsIfNecessary(descriptor, registry);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right place for this. This should go in DefaultBeanNativeConfigurationProcessor.

*
* @author Christoph Strobl
*/
class InitializationCallbacksSupplierTests {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need more tests here, in particular with initMethodName.

@snicoll snicoll added the type: enhancement A general enhancement label Sep 22, 2021
@snicoll snicoll added this to the 0.11.0 milestone Sep 22, 2021
@snicoll
Copy link
Contributor

snicoll commented Sep 22, 2021

With the help of @jhoeller, we'll be able to reduce reflection access once spring-projects/spring-framework#27449 is done.

This commit introduces support for detecting and calling bean initialization (post construct) methods.
Public, protected and package private init methods will be called directly, however private ones require reflection and register addition reflection configuration.
@sdeleuze
Copy link
Contributor

I removed the version assigned since #1048 also covers it.

@snicoll snicoll added this to the 0.11.0-M1 milestone Sep 24, 2021
@snicoll
Copy link
Contributor

snicoll commented Sep 24, 2021

I am going to review and merge this hopefully by M1. The other one is (partially) superseded by this one anyway.

@snicoll snicoll mentioned this pull request Sep 24, 2021
@snicoll snicoll changed the title Handle bean initialization methods. Handle bean init callbacks Sep 27, 2021
@snicoll snicoll changed the title Handle bean init callbacks Handle init callbacks Sep 27, 2021
snicoll pushed a commit that referenced this pull request Sep 27, 2021
This commit introduces support for detecting and calling bean
initialization (post construct) methods.

See gh-1059
snicoll added a commit that referenced this pull request Sep 27, 2021
Rather than invoking the initialization callback as part of the instance
supplier, we should rather post-process the bean so that we can provide
the same ordering as the regular runtime.

See gh-1059
snicoll added a commit that referenced this pull request Sep 27, 2021
This commit rework the original proposal by writing the init and
destroy methods and using them to instantiate a dedicated bean post
processor that is similar to the original one used at runtime.

Both method names defined on the bean definition itself as well as
externally managed ones are supported. If the bean name is '(inferred)'
similar logic is applied at build time.

Closes gh-1048
See gh-1059
snicoll added a commit that referenced this pull request Sep 27, 2021
* pr/1059:
  Invoke init and destroy callbacks
  Polish "Handle bean initialization methods"
  Handle bean initialization methods

Closes gh-1059
@snicoll
Copy link
Contributor

snicoll commented Sep 27, 2021

Merged by 1f96ae9

@snicoll snicoll closed this Sep 27, 2021
snicoll pushed a commit that referenced this pull request Sep 28, 2021
This commit introduces support for detecting and calling bean
initialization (post construct) methods.

See gh-1059
snicoll added a commit that referenced this pull request Sep 28, 2021
Rather than invoking the initialization callback as part of the instance
supplier, we should rather post-process the bean so that we can provide
the same ordering as the regular runtime.

See gh-1059
snicoll added a commit that referenced this pull request Sep 28, 2021
This commit rework the original proposal by writing the init and
destroy methods and using them to instantiate a dedicated bean post
processor that is similar to the original one used at runtime.

Both method names defined on the bean definition itself as well as
externally managed ones are supported. If the bean name is '(inferred)'
similar logic is applied at build time.

Closes gh-1048
See gh-1059
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement A general enhancement
Development

Successfully merging this pull request may close these issues.

None yet

3 participants