From c95426a616ddb0309cc6160f0593e246cd060d8c Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 30 Jun 2023 13:55:34 +0200 Subject: [PATCH 1/3] Polishing --- .../springframework/core/io/PathResource.java | 26 ++++++++-------- .../core/io/PathResourceTests.java | 30 ++++++++++--------- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/io/PathResource.java b/spring-core/src/main/java/org/springframework/core/io/PathResource.java index 2a31bbfe4e..c4a4168b2e 100644 --- a/spring-core/src/main/java/org/springframework/core/io/PathResource.java +++ b/spring-core/src/main/java/org/springframework/core/io/PathResource.java @@ -62,7 +62,7 @@ public class PathResource extends AbstractResource implements WritableResource { /** - * Create a new PathResource from a Path handle. + * Create a new {@code PathResource} from a {@link Path} handle. *

Note: Unlike {@link FileSystemResource}, when building relative resources * via {@link #createRelative}, the relative path will be built underneath * the given root: e.g. Paths.get("C:/dir1/"), relative path "dir2" → "C:/dir1/dir2"! @@ -74,7 +74,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * Create a new PathResource from a Path handle. + * Create a new {@code PathResource} from a path string. *

Note: Unlike {@link FileSystemResource}, when building relative resources * via {@link #createRelative}, the relative path will be built underneath * the given root: e.g. Paths.get("C:/dir1/"), relative path "dir2" → "C:/dir1/dir2"! @@ -87,7 +87,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * Create a new PathResource from a Path handle. + * Create a new {@code PathResource} from a {@link URI}. *

Note: Unlike {@link FileSystemResource}, when building relative resources * via {@link #createRelative}, the relative path will be built underneath * the given root: e.g. Paths.get("C:/dir1/"), relative path "dir2" → "C:/dir1/dir2"! @@ -128,7 +128,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation opens a InputStream for the underlying file. + * This implementation opens an {@link InputStream} for the underlying file. * @see java.nio.file.spi.FileSystemProvider#newInputStream(Path, OpenOption...) */ @Override @@ -174,7 +174,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation opens a OutputStream for the underlying file. + * This implementation opens an {@link OutputStream} for the underlying file. * @see java.nio.file.spi.FileSystemProvider#newOutputStream(Path, OpenOption...) */ @Override @@ -186,7 +186,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation returns a URL for the underlying file. + * This implementation returns a {@link URL} for the underlying file. * @see java.nio.file.Path#toUri() * @see java.net.URI#toURL() */ @@ -196,7 +196,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation returns a URI for the underlying file. + * This implementation returns a {@link URI} for the underlying file. * @see java.nio.file.Path#toUri() */ @Override @@ -213,7 +213,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation returns the underlying File reference. + * This implementation returns the underlying {@link File} reference. */ @Override public File getFile() throws IOException { @@ -228,7 +228,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation opens a Channel for the underlying file. + * This implementation opens a {@link ReadableByteChannel} for the underlying file. * @see Files#newByteChannel(Path, OpenOption...) */ @Override @@ -242,7 +242,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation opens a Channel for the underlying file. + * This implementation opens a {@link WritableByteChannel} for the underlying file. * @see Files#newByteChannel(Path, OpenOption...) */ @Override @@ -259,7 +259,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation returns the underlying File's timestamp. + * This implementation returns the underlying file's timestamp. * @see java.nio.file.Files#getLastModifiedTime(Path, java.nio.file.LinkOption...) */ @Override @@ -270,7 +270,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation creates a PathResource, applying the given path + * This implementation creates a {@link PathResource}, applying the given path * relative to the path of the underlying file of this resource descriptor. * @see java.nio.file.Path#resolve(String) */ @@ -295,7 +295,7 @@ public class PathResource extends AbstractResource implements WritableResource { /** - * This implementation compares the underlying Path references. + * This implementation compares the underlying {@link Path} references. */ @Override public boolean equals(@Nullable Object obj) { diff --git a/spring-core/src/test/java/org/springframework/core/io/PathResourceTests.java b/spring-core/src/test/java/org/springframework/core/io/PathResourceTests.java index cee4841845..df00b8e8d5 100644 --- a/spring-core/src/test/java/org/springframework/core/io/PathResourceTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/PathResourceTests.java @@ -142,7 +142,7 @@ class PathResourceTests { } @Test - void doesNotExistIsNotReadable() { + void nonExistingFileIsNotReadable() { PathResource resource = new PathResource(NON_EXISTING_FILE); assertThat(resource.isReadable()).isFalse(); } @@ -157,7 +157,7 @@ class PathResourceTests { void getInputStream() throws IOException { PathResource resource = new PathResource(TEST_FILE); byte[] bytes = FileCopyUtils.copyToByteArray(resource.getInputStream()); - assertThat(bytes.length).isGreaterThan(0); + assertThat(bytes).hasSizeGreaterThan(0); } @Test @@ -167,7 +167,7 @@ class PathResourceTests { } @Test - void getInputStreamDoesNotExist() throws IOException { + void getInputStreamForNonExistingFile() throws IOException { PathResource resource = new PathResource(NON_EXISTING_FILE); assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(resource::getInputStream); } @@ -260,24 +260,26 @@ class PathResourceTests { @Test void equalsAndHashCode() { - Resource mr1 = new PathResource(TEST_FILE); - Resource mr2 = new PathResource(TEST_FILE); - Resource mr3 = new PathResource(TEST_DIR); - assertThat(mr1).isEqualTo(mr2); - assertThat(mr1).isNotEqualTo(mr3); - assertThat(mr1).hasSameHashCodeAs(mr2); - assertThat(mr1).doesNotHaveSameHashCodeAs(mr3); + Resource resource1 = new PathResource(TEST_FILE); + Resource resource2 = new PathResource(TEST_FILE); + Resource resource3 = new PathResource(TEST_DIR); + assertThat(resource1).isEqualTo(resource1); + assertThat(resource1).isEqualTo(resource2); + assertThat(resource2).isEqualTo(resource1); + assertThat(resource1).isNotEqualTo(resource3); + assertThat(resource1).hasSameHashCodeAs(resource2); + assertThat(resource1).doesNotHaveSameHashCodeAs(resource3); } @Test - void outputStream(@TempDir Path temporaryFolder) throws IOException { + void getOutputStreamForExistingFile(@TempDir Path temporaryFolder) throws IOException { PathResource resource = new PathResource(temporaryFolder.resolve("test")); FileCopyUtils.copy("test".getBytes(StandardCharsets.UTF_8), resource.getOutputStream()); assertThat(resource.contentLength()).isEqualTo(4L); } @Test - void doesNotExistOutputStream(@TempDir Path temporaryFolder) throws IOException { + void getOutputStreamForNonExistingFile(@TempDir Path temporaryFolder) throws IOException { File file = temporaryFolder.resolve("test").toFile(); file.delete(); PathResource resource = new PathResource(file.toPath()); @@ -286,7 +288,7 @@ class PathResourceTests { } @Test - void directoryOutputStream() throws IOException { + void getOutputStreamForDirectory() { PathResource resource = new PathResource(TEST_DIR); assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(resource::getOutputStream); } @@ -314,7 +316,7 @@ class PathResourceTests { } @Test - void getReadableByteChannelDoesNotExist() throws IOException { + void getReadableByteChannelForNonExistingFile() throws IOException { PathResource resource = new PathResource(NON_EXISTING_FILE); assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(resource::readableChannel); } From d8729a7c67be34f78ec7352992d5933c8ca9bc8b Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Wed, 28 Jun 2023 10:10:53 +0800 Subject: [PATCH 2/3] Polish variable name in ReschedulingRunnable As a variable name, `initDelay` is applicable for `scheduleAtFixedRate` and `scheduleWithFixedDelay` but not for `schedule`. Closes gh-30762 --- .../scheduling/concurrent/ReschedulingRunnable.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ReschedulingRunnable.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ReschedulingRunnable.java index 10fbaaab23..c14be23fd5 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ReschedulingRunnable.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ReschedulingRunnable.java @@ -79,8 +79,8 @@ class ReschedulingRunnable extends DelegatingErrorHandlingRunnable implements Sc if (this.scheduledExecutionTime == null) { return null; } - Duration initialDelay = Duration.between(this.triggerContext.getClock().instant(), this.scheduledExecutionTime); - this.currentFuture = this.executor.schedule(this, initialDelay.toNanos(), TimeUnit.NANOSECONDS); + Duration delay = Duration.between(this.triggerContext.getClock().instant(), this.scheduledExecutionTime); + this.currentFuture = this.executor.schedule(this, delay.toNanos(), TimeUnit.NANOSECONDS); return this; } } From 3c05679a978f085fbad5556e2a4ee5b6c6cf7e85 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 30 Jun 2023 14:04:37 +0200 Subject: [PATCH 3/3] Polishing See gh-30762 --- .../scheduling/concurrent/ConcurrentTaskScheduler.java | 5 ++--- .../scheduling/concurrent/ThreadPoolTaskScheduler.java | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java index eb2f29a4c8..9b3e65b33f 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java @@ -212,10 +212,9 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T @Override public ScheduledFuture schedule(Runnable task, Instant startTime) { - Duration initialDelay = Duration.between(this.clock.instant(), startTime); + Duration delay = Duration.between(this.clock.instant(), startTime); try { - return this.scheduledExecutor.schedule(decorateTask(task, false), - NANO.convert(initialDelay), NANO); + return this.scheduledExecutor.schedule(decorateTask(task, false), NANO.convert(delay), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex); diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java index bffdc53bef..bad07b6ee8 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java @@ -383,10 +383,9 @@ public class ThreadPoolTaskScheduler extends ExecutorConfigurationSupport @Override public ScheduledFuture schedule(Runnable task, Instant startTime) { ScheduledExecutorService executor = getScheduledExecutor(); - Duration initialDelay = Duration.between(this.clock.instant(), startTime); + Duration delay = Duration.between(this.clock.instant(), startTime); try { - return executor.schedule(errorHandlingTask(task, false), - NANO.convert(initialDelay), NANO); + return executor.schedule(errorHandlingTask(task, false), NANO.convert(delay), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex);