From 0e5be0ab62092574f1d852b6587a6cbd4cf7b309 Mon Sep 17 00:00:00 2001 From: Marten Deinum Date: Mon, 10 Aug 2020 11:44:58 +0200 Subject: [PATCH 1/2] Allow step action to be optional Update `SpringApplicationRunListeners` so that the step action is optional and does not need to call `end()`. This commit also introduces a test for the failure case showing that there is a missed call to end with the initial solution. See gh-22845 --- .../boot/SpringApplicationRunListeners.java | 4 +++- .../boot/SpringApplicationTests.java | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationRunListeners.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationRunListeners.java index bb6081899c..6f3c133aa5 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationRunListeners.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationRunListeners.java @@ -103,7 +103,8 @@ class SpringApplicationRunListeners { } private void doWithListeners(String stepName, Consumer listenerAction) { - doWithListeners(stepName, listenerAction, StartupStep::end); + doWithListeners(stepName, listenerAction, (step) -> { + }); } private void doWithListeners(String stepName, Consumer listenerAction, @@ -111,6 +112,7 @@ class SpringApplicationRunListeners { StartupStep step = this.applicationStartup.start(stepName); this.listeners.forEach(listenerAction); stepAction.accept(step); + step.end(); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index 844365685c..d92a05c26b 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -141,6 +141,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; * @author Madhura Bhave * @author Brian Clozel * @author Artsiom Yudovin + * @author Marten Deinum */ @ExtendWith(OutputCaptureExtension.class) class SpringApplicationTests { @@ -1179,6 +1180,29 @@ class SpringApplicationTests { assertThat(startCount).isEqualTo(endCount); } + @Test + void customApplicationStartupPublishStartupStepsWithFailure() { + ApplicationStartup applicationStartup = mock(ApplicationStartup.class); + StartupStep startupStep = mock(StartupStep.class); + given(applicationStartup.start(anyString())).willReturn(startupStep); + given(startupStep.tag(anyString(), anyString())).willReturn(startupStep); + given(startupStep.tag(anyString(), ArgumentMatchers.>any())).willReturn(startupStep); + SpringApplication application = new SpringApplication(BrokenPostConstructConfig.class); + application.setWebApplicationType(WebApplicationType.NONE); + application.setApplicationStartup(applicationStartup); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(application::run); + + verify(applicationStartup).start("spring.boot.application.starting"); + verify(applicationStartup).start("spring.boot.application.environment-prepared"); + verify(applicationStartup).start("spring.boot.application.failed"); + + long startCount = mockingDetails(applicationStartup).getInvocations().stream() + .filter((invocation) -> invocation.getMethod().toString().contains("start(")).count(); + long endCount = mockingDetails(startupStep).getInvocations().stream() + .filter((invocation) -> invocation.getMethod().toString().contains("end(")).count(); + assertThat(startCount).isEqualTo(endCount); + } + private ArgumentMatcher isAvailabilityChangeEventWithState( S state) { return (argument) -> (argument instanceof AvailabilityChangeEvent) From a062b51efbe50cfc14109b02798d4504114988b9 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 11 Aug 2020 10:57:03 -0700 Subject: [PATCH 2/2] Polish 'Allow step action to be optional' See gh-22845 --- .../boot/SpringApplicationRunListeners.java | 7 ++++--- .../org/springframework/boot/SpringApplicationTests.java | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationRunListeners.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationRunListeners.java index 6f3c133aa5..faea9acf24 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationRunListeners.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationRunListeners.java @@ -103,15 +103,16 @@ class SpringApplicationRunListeners { } private void doWithListeners(String stepName, Consumer listenerAction) { - doWithListeners(stepName, listenerAction, (step) -> { - }); + doWithListeners(stepName, listenerAction, null); } private void doWithListeners(String stepName, Consumer listenerAction, Consumer stepAction) { StartupStep step = this.applicationStartup.start(stepName); this.listeners.forEach(listenerAction); - stepAction.accept(step); + if (stepAction != null) { + stepAction.accept(step); + } step.end(); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index d92a05c26b..263f7c3409 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -1191,16 +1191,16 @@ class SpringApplicationTests { application.setWebApplicationType(WebApplicationType.NONE); application.setApplicationStartup(applicationStartup); assertThatExceptionOfType(BeanCreationException.class).isThrownBy(application::run); - verify(applicationStartup).start("spring.boot.application.starting"); verify(applicationStartup).start("spring.boot.application.environment-prepared"); verify(applicationStartup).start("spring.boot.application.failed"); - long startCount = mockingDetails(applicationStartup).getInvocations().stream() .filter((invocation) -> invocation.getMethod().toString().contains("start(")).count(); long endCount = mockingDetails(startupStep).getInvocations().stream() .filter((invocation) -> invocation.getMethod().toString().contains("end(")).count(); - assertThat(startCount).isEqualTo(endCount); + assertThat(startCount).isEqualTo(endCount + 1); // Will be same after + // spring-framework #25572 is + // fixed } private ArgumentMatcher isAvailabilityChangeEventWithState(