Deprecate SpringApplication#refresh(ApplicationContext) and provide SpringApplication#refresh(ConfigurableApplicationContext) as a replacement

[justmehyp]

The method SpringApplication#refresh is only called by SpringApplication#refreshContext(ConfigurableApplicationContext).
So the application context instance passed to SpringApplication#refresh will always be a subclass of ConfigurableApplicationContext, which has a method 'refresh'.
No need to assert that using Assert.isInstanceOf(AbstractApplicationContext.class, applicationContext);.

It could be simplified like this:

protected void refresh(ConfigurableApplicationContext applicationContext) {
    applicationContext.refresh();
}

Further more, the field applicationContextClass of SpringApplication is also declared as Class<? extends ConfigurableApplicationContext>.

I think it is reasonable enough to modify the method signature.

[wilkinsona]

Thanks for the suggestion. While I agree that things could perhaps be simplified a little here, it would technically be a breaking API change. I'm not sure that the small benefit of the change justifies making it. May I ask what prompted you to open the issue?

[justmehyp]

Thank you for your reply.

Well, I am a number-one fan of Spring framework. Since I use it, I hope it gets better. I read the source code, then I feedback something I think is better. Sometimes I am stubborn about keeping the code clean, if it makes me confused ...

Yes, it is a breaking API change. But thankfully it is an internal API, not public. It is a protected method, but no subclasses inherit SpringApplication up to now.

I noticed that parameters of SpringApplication#afterRefresh changed for introducing ApplicationArguments and removing String[].
I think it is a good strategy. If you can't change it right away, I suggest you use this strategy again as a means of transition.

In 1.2.8.RELEASE, it is:

protected void afterRefresh(ConfigurableApplicationContext context, String[] args) {
    runCommandLineRunners(context, args);
}

In v1.3.8.RELEASE, it is deprecated:

@Deprecated
protected void afterRefresh(ConfigurableApplicationContext context, String[] args) {
}

and you add a new method:

protected void afterRefresh(ConfigurableApplicationContext context, ApplicationArguments args) {
    afterRefresh(context, args.getSourceArgs());
    callRunners(context, args);
}

in v1.4.0.RELEASE, SpringApplication#afterRefresh(ConfigurableApplicationContext, String[]) was removed. Only SpringApplication#afterRefresh(ConfigurableApplicationContext, ApplicationArguments) was retained.

[philwebb]

it is an internal API, not public. It is a protected method, but no subclasses inherit SpringApplication up to now.

I'm afraid we need to treat it as a public API. Just because we don't have a SpringApplication subclass in our code does not mean that someone is not relying on it.

I think it is a good strategy. If you can't change it right away, I suggest you use this strategy again as a means of transition.

I'm not sure that the additional argument would be in this case. We'd probably need to introduce a new method with a different name.

[justmehyp]

I think you are right. In this case, a new method such as doRefresh could be introduced:

protected void doRefresh(ConfigurableApplicationContext applicationContext) {
    refresh(applicationContext);
}

Then mark refresh as deprecated:

@Deprecated
protected void refresh(ApplicationContext applicationContext) {
    Assert.isInstanceOf(AbstractApplicationContext.class, applicationContext);
    ((AbstractApplicationContext) applicationContext).refresh();
}