From ce86f4e4b51ade58b25782562ceda54d073dce20 Mon Sep 17 00:00:00 2001 From: Marcus Da Coregio Date: Fri, 6 May 2022 09:42:30 -0300 Subject: [PATCH] Polish ServerWebExchangeDelegatingServerHttpHeadersWriter Issue gh-11073 --- ...angeDelegatingServerHttpHeadersWriter.java | 6 +- ...elegatingServerHttpHeadersWriterTests.java | 56 ++++++++++--------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/server/header/ServerWebExchangeDelegatingServerHttpHeadersWriter.java b/web/src/main/java/org/springframework/security/web/server/header/ServerWebExchangeDelegatingServerHttpHeadersWriter.java index a62f4677d0..6c4b008b5c 100644 --- a/web/src/main/java/org/springframework/security/web/server/header/ServerWebExchangeDelegatingServerHttpHeadersWriter.java +++ b/web/src/main/java/org/springframework/security/web/server/header/ServerWebExchangeDelegatingServerHttpHeadersWriter.java @@ -43,6 +43,8 @@ public final class ServerWebExchangeDelegatingServerHttpHeadersWriter implements public ServerWebExchangeDelegatingServerHttpHeadersWriter( ServerWebExchangeMatcherEntry headersWriter) { Assert.notNull(headersWriter, "headersWriter cannot be null"); + Assert.notNull(headersWriter.getMatcher(), "webExchangeMatcher cannot be null"); + Assert.notNull(headersWriter.getEntry(), "delegateHeadersWriter cannot be null"); this.headersWriter = headersWriter; } @@ -55,9 +57,7 @@ public final class ServerWebExchangeDelegatingServerHttpHeadersWriter implements */ public ServerWebExchangeDelegatingServerHttpHeadersWriter(ServerWebExchangeMatcher webExchangeMatcher, ServerHttpHeadersWriter delegateHeadersWriter) { - Assert.notNull(webExchangeMatcher, "webExchangeMatcher cannot be null"); - Assert.notNull(delegateHeadersWriter, "delegateHeadersWriter cannot be null"); - this.headersWriter = new ServerWebExchangeMatcherEntry<>(webExchangeMatcher, delegateHeadersWriter); + this(new ServerWebExchangeMatcherEntry<>(webExchangeMatcher, delegateHeadersWriter)); } @Override diff --git a/web/src/test/java/org/springframework/security/web/server/header/ServerWebExchangeDelegatingServerHttpHeadersWriterTests.java b/web/src/test/java/org/springframework/security/web/server/header/ServerWebExchangeDelegatingServerHttpHeadersWriterTests.java index d2858e7585..c3c38ff0a3 100644 --- a/web/src/test/java/org/springframework/security/web/server/header/ServerWebExchangeDelegatingServerHttpHeadersWriterTests.java +++ b/web/src/test/java/org/springframework/security/web/server/header/ServerWebExchangeDelegatingServerHttpHeadersWriterTests.java @@ -17,7 +17,6 @@ package org.springframework.security.web.server.header; import java.util.Collections; -import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -49,9 +48,6 @@ public class ServerWebExchangeDelegatingServerHttpHeadersWriterTests { @Mock private ServerHttpHeadersWriter delegate; - @Mock - private ServerWebExchangeMatcherEntry matcherEntry; - private ServerWebExchange exchange; private ServerWebExchangeDelegatingServerHttpHeadersWriter headerWriter; @@ -63,49 +59,57 @@ public class ServerWebExchangeDelegatingServerHttpHeadersWriterTests { } @Test - public void constructorNullWebExchangeMatcher() { + public void constructorWhenNullWebExchangeMatcherThenException() { assertThatIllegalArgumentException() - .isThrownBy(() -> new ServerWebExchangeDelegatingServerHttpHeadersWriter(null, this.delegate)); + .isThrownBy(() -> new ServerWebExchangeDelegatingServerHttpHeadersWriter(null, this.delegate)) + .withMessage("webExchangeMatcher cannot be null"); } @Test - public void constructorNullWebExchangeMatcherEntry() { + public void constructorWhenNullWebExchangeMatcherEntryThenException() { assertThatIllegalArgumentException() - .isThrownBy(() -> new ServerWebExchangeDelegatingServerHttpHeadersWriter(null)); + .isThrownBy(() -> new ServerWebExchangeDelegatingServerHttpHeadersWriter(null)) + .withMessage("headersWriter cannot be null"); } @Test - public void constructorNullDelegate() { + public void constructorWhenNullDelegateHeadersWriterThenException() { assertThatIllegalArgumentException() - .isThrownBy(() -> new ServerWebExchangeDelegatingServerHttpHeadersWriter(this.matcher, null)); + .isThrownBy(() -> new ServerWebExchangeDelegatingServerHttpHeadersWriter(this.matcher, null)) + .withMessage("delegateHeadersWriter cannot be null"); } @Test - public void writeHeadersOnMatch() { - Map params = Collections.singletonMap("foo", "bar"); - given(this.matcher.matches(this.exchange)).willReturn(ServerWebExchangeMatcher.MatchResult.match(params)); + public void constructorWhenEntryWithNullMatcherThenException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new ServerWebExchangeDelegatingServerHttpHeadersWriter( + new ServerWebExchangeMatcherEntry<>(null, this.delegate))) + .withMessage("webExchangeMatcher cannot be null"); + } + + @Test + public void constructorWhenEntryWithNullEntryThenException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new ServerWebExchangeDelegatingServerHttpHeadersWriter( + new ServerWebExchangeMatcherEntry<>(this.matcher, null))) + .withMessage("delegateHeadersWriter cannot be null"); + } + + @Test + public void writeHeadersWhenMatchThenDelegateWriteHttpHeaders() { + given(this.matcher.matches(this.exchange)) + .willReturn(ServerWebExchangeMatcher.MatchResult.match(Collections.emptyMap())); given(this.delegate.writeHttpHeaders(this.exchange)).willReturn(Mono.empty()); this.headerWriter.writeHttpHeaders(this.exchange).block(); verify(this.delegate).writeHttpHeaders(this.exchange); } @Test - public void writeHeadersOnNoMatch() { + public void writeHeadersWhenNoMatchThenDelegateNotCalled() { given(this.matcher.matches(this.exchange)).willReturn(ServerWebExchangeMatcher.MatchResult.notMatch()); this.headerWriter.writeHttpHeaders(this.exchange).block(); + verify(this.matcher).matches(this.exchange); verify(this.delegate, times(0)).writeHttpHeaders(this.exchange); } - @Test - public void writeHeadersOnMatchWithServerWebExchangeMatcherEntry() { - this.headerWriter = new ServerWebExchangeDelegatingServerHttpHeadersWriter(this.matcherEntry); - given(this.matcherEntry.getMatcher()).willReturn(this.matcher); - given(this.matcherEntry.getEntry()).willReturn(this.delegate); - Map params = Collections.singletonMap("foo", "bar"); - given(this.matcher.matches(this.exchange)).willReturn(ServerWebExchangeMatcher.MatchResult.match(params)); - given(this.delegate.writeHttpHeaders(this.exchange)).willReturn(Mono.empty()); - this.headerWriter.writeHttpHeaders(this.exchange).block(); - verify(this.delegate).writeHttpHeaders(this.exchange); - } - }