From a40cc8bbe09e2d21f2abea522d1a9fc09b83c21d Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 23 Oct 2020 18:27:49 +0100 Subject: [PATCH] Polishing contribution Closes gh-25647 --- .../http/ContentDisposition.java | 120 ++++++++---------- .../http/ContentDispositionTests.java | 34 +++-- 2 files changed, 68 insertions(+), 86 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java index 0a80807ecf..8b735a22ee 100644 --- a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java +++ b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java @@ -48,18 +48,6 @@ public final class ContentDisposition { private static final String INVALID_HEADER_FIELD_PARAMETER_FORMAT = "Invalid header field parameter format (as defined in RFC 5987)"; - /** - * The {@literal attachment} content-disposition type. - */ - private static final String ATTACHMENT = "attachment"; - /** - * The {@literal form-data} content-disposition type. - */ - private static final String FORM_DATA = "form-data"; - /** - * The {@literal inline} content-disposition type. - */ - private static final String INLINE = "inline"; @Nullable private final String type; @@ -105,8 +93,34 @@ public final class ContentDisposition { /** - * Return the disposition type, like for example {@literal inline}, {@literal attachment}, - * {@literal form-data}, or {@code null} if not defined. + * Return whether the {@link #getType() type} is {@literal "attachment"}. + * @since 5.3 + */ + public boolean isAttachment() { + return (this.type != null && this.type.equalsIgnoreCase("attachment")); + } + + /** + * Return whether the {@link #getType() type} is {@literal "form-data"}. + * @since 5.3 + */ + public boolean isFormData() { + return (this.type != null && this.type.equalsIgnoreCase("form-data")); + } + + /** + * Return whether the {@link #getType() type} is {@literal "inline"}. + * @since 5.3 + */ + public boolean isInline() { + return (this.type != null && this.type.equalsIgnoreCase("inline")); + } + + /** + * Return the disposition type. + * @see #isAttachment() + * @see #isFormData() + * @see #isInline() */ @Nullable public String getType() { @@ -186,30 +200,6 @@ public final class ContentDisposition { return this.readDate; } - /** - * Return whether the {@link #getType() type} is {@literal attachment}. - * @since 5.3 - */ - public boolean isAttachment() { - return ATTACHMENT.equals(this.type); - } - - /** - * Return whether the {@link #getType() type} is {@literal form-data}. - * @since 5.3 - */ - public boolean isFormData() { - return FORM_DATA.equals(this.type); - } - - /** - * Return whether the {@link #getType() type} is {@literal inline}. - * @since 5.3 - */ - public boolean isInline() { - return INLINE.equals(this.type); - } - @Override public boolean equals(@Nullable Object other) { if (this == other) { @@ -289,6 +279,30 @@ public final class ContentDisposition { } + /** + * Return a builder for a {@code ContentDisposition} of type {@literal "attachment"}. + * @since 5.3 + */ + public static Builder attachment() { + return builder("attachment"); + } + + /** + * Return a builder for a {@code ContentDisposition} of type {@literal "form-data"}. + * @since 5.3 + */ + public static Builder formData() { + return builder("form-data"); + } + + /** + * Return a builder for a {@code ContentDisposition} of type {@literal "inline"}. + * @since 5.3 + */ + public static Builder inline() { + return builder("inline"); + } + /** * Return a builder for a {@code ContentDisposition}. * @param type the disposition type like for example {@literal inline}, @@ -299,36 +313,6 @@ public final class ContentDisposition { return new BuilderImpl(type); } - /** - * Return a builder for a {@code ContentDisposition} with - * the {@link #ATTACHMENT attachment} type. - * @return the builder - * @since 5.3 - */ - public static Builder attachment() { - return builder(ATTACHMENT); - } - - /** - * Return a builder for a {@code ContentDisposition} with - * the {@link #FORM_DATA form-data} type. - * @return the builder - * @since 5.3 - */ - public static Builder formData() { - return builder(FORM_DATA); - } - - /** - * Return a builder for a {@code ContentDisposition} with - * the {@link #INLINE inline} type. - * @return the builder - * @since 5.3 - */ - public static Builder inline() { - return builder(INLINE); - } - /** * Return an empty content disposition. */ diff --git a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java index e2fb0aa6e1..00b49d2fc0 100644 --- a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java +++ b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java @@ -25,8 +25,6 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; -import static org.springframework.http.ContentDisposition.attachment; -import static org.springframework.http.ContentDisposition.formData; /** * Unit tests for {@link ContentDisposition} @@ -42,7 +40,7 @@ class ContentDispositionTests { @SuppressWarnings("deprecation") void parse() { assertThat(parse("form-data; name=\"foo\"; filename=\"foo.txt\"; size=123")) - .isEqualTo(formData() + .isEqualTo(ContentDisposition.formData() .name("foo") .filename("foo.txt") .size(123L) @@ -52,7 +50,7 @@ class ContentDispositionTests { @Test void parseFilenameUnquoted() { assertThat(parse("form-data; filename=unquoted")) - .isEqualTo(formData() + .isEqualTo(ContentDisposition.formData() .filename("unquoted") .build()); } @@ -60,7 +58,7 @@ class ContentDispositionTests { @Test // SPR-16091 void parseFilenameWithSemicolon() { assertThat(parse("attachment; filename=\"filename with ; semicolon.txt\"")) - .isEqualTo(attachment() + .isEqualTo(ContentDisposition.attachment() .filename("filename with ; semicolon.txt") .build()); } @@ -68,7 +66,7 @@ class ContentDispositionTests { @Test void parseEncodedFilename() { assertThat(parse("form-data; name=\"name\"; filename*=UTF-8''%E4%B8%AD%E6%96%87.txt")) - .isEqualTo(formData() + .isEqualTo(ContentDisposition.formData() .name("name") .filename("中文.txt", StandardCharsets.UTF_8) .build()); @@ -77,7 +75,7 @@ class ContentDispositionTests { @Test // gh-24112 void parseEncodedFilenameWithPaddedCharset() { assertThat(parse("attachment; filename*= UTF-8''some-file.zip")) - .isEqualTo(attachment() + .isEqualTo(ContentDisposition.attachment() .filename("some-file.zip", StandardCharsets.UTF_8) .build()); } @@ -85,7 +83,7 @@ class ContentDispositionTests { @Test void parseEncodedFilenameWithoutCharset() { assertThat(parse("form-data; name=\"name\"; filename*=test.txt")) - .isEqualTo(formData() + .isEqualTo(ContentDisposition.formData() .name("name") .filename("test.txt") .build()); @@ -112,7 +110,7 @@ class ContentDispositionTests { BiConsumer tester = (description, filename) -> assertThat(parse("form-data; name=\"file\"; filename=\"" + filename + "\"; size=123")) .as(description) - .isEqualTo(formData().name("file").filename(filename).size(123L).build()); + .isEqualTo(ContentDisposition.formData().name("file").filename(filename).size(123L).build()); tester.accept("Escaped quotes should be ignored", "\\\"The Twilight Zone\\\".txt"); @@ -131,7 +129,7 @@ class ContentDispositionTests { @SuppressWarnings("deprecation") void parseWithExtraSemicolons() { assertThat(parse("form-data; name=\"foo\";; ; filename=\"foo.txt\"; size=123")) - .isEqualTo(formData() + .isEqualTo(ContentDisposition.formData() .name("foo") .filename("foo.txt") .size(123L) @@ -150,7 +148,7 @@ class ContentDispositionTests { "creation-date=\"" + creationTime.format(formatter) + "\"; " + "modification-date=\"" + modificationTime.format(formatter) + "\"; " + "read-date=\"" + readTime.format(formatter) + "\"")).isEqualTo( - attachment() + ContentDisposition.attachment() .creationDate(creationTime) .modificationDate(modificationTime) .readDate(readTime) @@ -167,7 +165,7 @@ class ContentDispositionTests { "creation-date=\"-1\"; " + "modification-date=\"-1\"; " + "read-date=\"" + readTime.format(formatter) + "\"")).isEqualTo( - attachment() + ContentDisposition.attachment() .readDate(readTime) .build()); } @@ -196,7 +194,7 @@ class ContentDispositionTests { @SuppressWarnings("deprecation") void format() { assertThat( - formData() + ContentDisposition.formData() .name("foo") .filename("foo.txt") .size(123L) @@ -207,7 +205,7 @@ class ContentDispositionTests { @Test void formatWithEncodedFilename() { assertThat( - formData() + ContentDisposition.formData() .name("name") .filename("中文.txt", StandardCharsets.UTF_8) .build().toString()) @@ -217,7 +215,7 @@ class ContentDispositionTests { @Test void formatWithEncodedFilenameUsingUsAscii() { assertThat( - formData() + ContentDisposition.formData() .name("name") .filename("test.txt", StandardCharsets.US_ASCII) .build() @@ -230,10 +228,10 @@ class ContentDispositionTests { BiConsumer tester = (input, output) -> { - assertThat(formData().filename(input).build().toString()) + assertThat(ContentDisposition.formData().filename(input).build().toString()) .isEqualTo("form-data; filename=\"" + output + "\""); - assertThat(formData().filename(input, StandardCharsets.US_ASCII).build().toString()) + assertThat(ContentDisposition.formData().filename(input, StandardCharsets.US_ASCII).build().toString()) .isEqualTo("form-data; filename=\"" + output + "\""); }; @@ -263,7 +261,7 @@ class ContentDispositionTests { @Test void formatWithEncodedFilenameUsingInvalidCharset() { assertThatIllegalArgumentException().isThrownBy(() -> - formData() + ContentDisposition.formData() .name("name") .filename("test.txt", StandardCharsets.UTF_16) .build()