From f238812cead5ab42a0f8ad8ca2e877823ebb9e91 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 31 Mar 2020 12:16:32 +0100 Subject: [PATCH] Polish "Improve handling of non-existent path in disk space health check" See gh-20580 --- .../DiskSpaceHealthIndicatorProperties.java | 2 +- ...althContributorAutoConfigurationTests.java | 9 +- .../system/DiskSpaceHealthIndicator.java | 10 +- .../DiskSpaceHealthIndicatorPathTests.java | 106 ------------------ .../system/DiskSpaceHealthIndicatorTests.java | 20 ++-- 5 files changed, 17 insertions(+), 130 deletions(-) delete mode 100644 spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthIndicatorProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthIndicatorProperties.java index b740ea79a3..f2b625463a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthIndicatorProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthIndicatorProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthContributorAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthContributorAutoConfigurationTests.java index 6b49b53601..6a349de9c5 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthContributorAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthContributorAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,8 +16,6 @@ package org.springframework.boot.actuate.autoconfigure.system; -import java.util.UUID; - import org.junit.jupiter.api.Test; import org.springframework.boot.actuate.autoconfigure.health.HealthContributorAutoConfiguration; @@ -61,9 +59,8 @@ class DiskSpaceHealthContributorAutoConfigurationTests { } @Test - void pathIsNotRequiredToExist() { - String randomPath = "IDoNOTeXiST" + UUID.randomUUID().toString(); - this.contextRunner.withPropertyValues("management.health.diskspace.path=" + randomPath) + void runWhenPathDoesNotExistShouldCreateIndicator() { + this.contextRunner.withPropertyValues("management.health.diskspace.path=does/not/exist") .run((context) -> assertThat(context).hasSingleBean(DiskSpaceHealthIndicator.class)); } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java index 50166c32a7..bf3919a5e5 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -59,9 +59,7 @@ public class DiskSpaceHealthIndicator extends AbstractHealthIndicator { @Override protected void doHealthCheck(Health.Builder builder) throws Exception { long diskFreeInBytes = this.path.getUsableSpace(); - // return value of 0L means "the abstract pathname does not name a - // partition" which for our purposes means it's not usable i.e DOWN - if (diskFreeInBytes >= this.threshold.toBytes() && diskFreeInBytes != 0L) { + if (diskFreeInBytes >= this.threshold.toBytes()) { builder.up(); } else { @@ -70,9 +68,7 @@ public class DiskSpaceHealthIndicator extends AbstractHealthIndicator { builder.down(); } builder.withDetail("total", this.path.getTotalSpace()).withDetail("free", diskFreeInBytes) - .withDetail("threshold", this.threshold.toBytes()).withDetail("exists", this.path.exists()) - .withDetail("canRead", this.path.canRead()).withDetail("canWrite", this.path.canWrite()) - .withDetail("canExecute", this.path.canExecute()); + .withDetail("threshold", this.threshold.toBytes()).withDetail("exists", this.path.exists()); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java deleted file mode 100644 index 44bb3b9f48..0000000000 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Copyright 2012-2019 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.actuate.system; - -import java.io.File; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; - -import org.springframework.boot.actuate.health.Health; -import org.springframework.boot.actuate.health.HealthIndicator; -import org.springframework.boot.actuate.health.Status; -import org.springframework.util.unit.DataSize; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.BDDMockito.given; - -/** - * Tests for the {@link DiskSpaceHealthIndicator} {@code path} parameter. - * - * @author Andreas Born - */ -class DiskSpaceHealthIndicatorPathTests { - - private static final DataSize THRESHOLD = DataSize.ofKilobytes(1); - - private static final DataSize ZERO_THRESHOLD = DataSize.ofBytes(0); - - private static final DataSize TOTAL_SPACE = DataSize.ofKilobytes(10); - - @Mock - private File fileMock; - - private HealthIndicator healthIndicator; - - @BeforeEach - void setUp() { - MockitoAnnotations.initMocks(this); - given(this.fileMock.exists()).willReturn(false); - given(this.fileMock.canRead()).willReturn(false); - given(this.fileMock.canWrite()).willReturn(false); - given(this.fileMock.canExecute()).willReturn(false); - this.healthIndicator = new DiskSpaceHealthIndicator(this.fileMock, THRESHOLD); - } - - @Test - void diskSpaceIsDown() { - Health health = this.healthIndicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.DOWN); - assertThat(health.getDetails().get("threshold")).isEqualTo(THRESHOLD.toBytes()); - assertThat(health.getDetails().get("free")).isEqualTo(0L); - assertThat(health.getDetails().get("total")).isEqualTo(0L); - assertThat(health.getDetails().get("exists")).isEqualTo(false); - assertThat(health.getDetails().get("canRead")).isEqualTo(false); - assertThat(health.getDetails().get("canWrite")).isEqualTo(false); - assertThat(health.getDetails().get("canExecute")).isEqualTo(false); - } - - @Test - void diskSpaceIsDownWhenThresholdIsZero() { - this.healthIndicator = new DiskSpaceHealthIndicator(this.fileMock, ZERO_THRESHOLD); - Health health = this.healthIndicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.DOWN); - assertThat(health.getDetails().get("threshold")).isEqualTo(ZERO_THRESHOLD.toBytes()); - assertThat(health.getDetails().get("free")).isEqualTo(0L); - assertThat(health.getDetails().get("total")).isEqualTo(0L); - assertThat(health.getDetails().get("exists")).isEqualTo(false); - assertThat(health.getDetails().get("canRead")).isEqualTo(false); - assertThat(health.getDetails().get("canWrite")).isEqualTo(false); - assertThat(health.getDetails().get("canExecute")).isEqualTo(false); - } - - @Test - void diskSpaceIsUpWhenPathOnlyExists() { - long freeSpace = THRESHOLD.toBytes() + 10; - given(this.fileMock.getUsableSpace()).willReturn(freeSpace); - given(this.fileMock.getTotalSpace()).willReturn(TOTAL_SPACE.toBytes()); - given(this.fileMock.exists()).willReturn(true); - Health health = this.healthIndicator.health(); - assertThat(health.getStatus()).isEqualTo(Status.UP); - assertThat(health.getDetails().get("threshold")).isEqualTo(THRESHOLD.toBytes()); - assertThat(health.getDetails().get("free")).isEqualTo(freeSpace); - assertThat(health.getDetails().get("total")).isEqualTo(TOTAL_SPACE.toBytes()); - assertThat(health.getDetails().get("exists")).isEqualTo(true); - assertThat(health.getDetails().get("canRead")).isEqualTo(false); - assertThat(health.getDetails().get("canWrite")).isEqualTo(false); - assertThat(health.getDetails().get("canExecute")).isEqualTo(false); - } - -} diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorTests.java index 6046faa2c5..42dd5388ad 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -52,9 +52,6 @@ class DiskSpaceHealthIndicatorTests { void setUp() { MockitoAnnotations.initMocks(this); given(this.fileMock.exists()).willReturn(true); - given(this.fileMock.canRead()).willReturn(true); - given(this.fileMock.canWrite()).willReturn(true); - given(this.fileMock.canExecute()).willReturn(true); this.healthIndicator = new DiskSpaceHealthIndicator(this.fileMock, THRESHOLD); } @@ -69,9 +66,6 @@ class DiskSpaceHealthIndicatorTests { assertThat(health.getDetails().get("free")).isEqualTo(freeSpace); assertThat(health.getDetails().get("total")).isEqualTo(TOTAL_SPACE.toBytes()); assertThat(health.getDetails().get("exists")).isEqualTo(true); - assertThat(health.getDetails().get("canRead")).isEqualTo(true); - assertThat(health.getDetails().get("canWrite")).isEqualTo(true); - assertThat(health.getDetails().get("canExecute")).isEqualTo(true); } @Test @@ -85,9 +79,15 @@ class DiskSpaceHealthIndicatorTests { assertThat(health.getDetails().get("free")).isEqualTo(freeSpace); assertThat(health.getDetails().get("total")).isEqualTo(TOTAL_SPACE.toBytes()); assertThat(health.getDetails().get("exists")).isEqualTo(true); - assertThat(health.getDetails().get("canRead")).isEqualTo(true); - assertThat(health.getDetails().get("canWrite")).isEqualTo(true); - assertThat(health.getDetails().get("canExecute")).isEqualTo(true); + } + + @Test + void whenPathDoesNotExistDiskSpaceIsDown() { + Health health = new DiskSpaceHealthIndicator(new File("does/not/exist"), THRESHOLD).health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails().get("free")).isEqualTo(0L); + assertThat(health.getDetails().get("total")).isEqualTo(0L); + assertThat(health.getDetails().get("exists")).isEqualTo(false); } }