Cwikius-Spring
/
spring-security
mirror of https://github.com/spring-projects/spring-security.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Polish Tests and Error Messages 

MockMvc matchers are best matched with the MockMvc execution API -
it's a little odd to try and use them inside of an AssertJ assertion
since they do their own asserting.

It's more readable to place "this." in front of member variables.

It's best to test just one class at a time in a unit test.

Issue: gh-4187
This commit is contained in:
Josh Cummings 2019-02-28 10:51:04 -07:00
parent 82d527ed42
commit d86550f64b
No known key found for this signature in database
GPG Key ID: 49EF60DD7FF83443
4 changed files with 26 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
web/src/main/java/org/springframework/security/web/authentication/logout/HeaderWriterLogoutHandler.java
View File

@ -38,7 +38,7 @@ public final class HeaderWriterLogoutHandler implements LogoutHandler {
* @throws {@link IllegalArgumentException} if headerWriter is null.
*/
public HeaderWriterLogoutHandler(HeaderWriter headerWriter) {
Assert.notNull(headerWriter, "headerWriter cannot be null.");
Assert.notNull(headerWriter, "headerWriter cannot be null");
this.headerWriter = headerWriter;
}

2
web/src/main/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriter.java
View File

@ -67,7 +67,7 @@ public final class ClearSiteDataHeaderWriter implements HeaderWriter {
* @throws {@link IllegalArgumentException} if sources is null or empty.
*/
public ClearSiteDataHeaderWriter(String ...sources) {
Assert.notEmpty(sources, "Sources cannot be empty or null.");
Assert.notEmpty(sources, "sources cannot be empty or null");
this.requestMatcher = new SecureRequestMatcher();
this.headerValue = Stream.of(sources).map(this::quote).collect(Collectors.joining(", "));
}

58
web/src/test/java/org/springframework/security/web/authentication/logout/HeaderWriterLogoutHandlerTests.java
View File

@ -20,24 +20,23 @@ import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.core.Authentication;
import org.springframework.security.web.header.writers.ClearSiteDataHeaderWriter;
import org.springframework.security.web.header.HeaderWriter;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
import static org.mockito.Mockito.verify;
/**
*
* @author Rafiullah Hamedy
* @author Josh Cummings
*
* @see {@link HeaderWriterLogoutHandler}
*/
public class HeaderWriterLogoutHandlerTests {
private static final String HEADER_NAME = "Clear-Site-Data";
private MockHttpServletResponse response;
private MockHttpServletRequest request;
@ -51,54 +50,19 @@ public class HeaderWriterLogoutHandlerTests {
}
@Test
public void createInstanceWhenHeaderWriterIsNullThenThrowsException() {
public void constructorWhenHeaderWriterIsNullThenThrowsException() {
this.thrown.expect(IllegalArgumentException.class);
this.thrown.expectMessage("headerWriter cannot be null.");
this.thrown.expectMessage("headerWriter cannot be null");
new HeaderWriterLogoutHandler(null);
}
@Test
public void createInstanceWhenSourceIsNullThenThrowsException() {
this.thrown.expect(IllegalArgumentException.class);
this.thrown.expectMessage("Sources cannot be empty or null.");
public void logoutWhenHasHeaderWriterThenInvoked() {
HeaderWriter headerWriter = mock(HeaderWriter.class);
HeaderWriterLogoutHandler handler = new HeaderWriterLogoutHandler(headerWriter);
handler.logout(this.request, this.response, mock(Authentication.class));
new HeaderWriterLogoutHandler(new ClearSiteDataHeaderWriter());
}
@Test
public void logoutWhenRequestIsNotSecureThenHeaderIsNotPresent() {
HeaderWriterLogoutHandler handler = new HeaderWriterLogoutHandler(
new ClearSiteDataHeaderWriter("cache"));
handler.logout(request, response, mock(Authentication.class));
assertThat(header().doesNotExist(HEADER_NAME));
}
@Test
public void logoutWhenRequestIsSecureThenHeaderIsPresentMatchesWildCardSource() {
HeaderWriterLogoutHandler handler = new HeaderWriterLogoutHandler(
new ClearSiteDataHeaderWriter("*"));
this.request.setSecure(true);
handler.logout(request, response, mock(Authentication.class));
assertThat(header().stringValues(HEADER_NAME, "\"*\""));
}
@Test
public void logoutWhenRequestIsSecureThenHeaderValueMatchesSource() {
HeaderWriterLogoutHandler handler = new HeaderWriterLogoutHandler(
new ClearSiteDataHeaderWriter("cache", "cookies", "storage",
"executionContexts"));
this.request.setSecure(true);
handler.logout(request, response, mock(Authentication.class));
assertThat(header().stringValues(HEADER_NAME, "\"cache\", \"cookies\", \"storage\", "
+ "\"executionContexts\""));
verify(headerWriter).writeHeaders(this.request, this.response);
}
}

35
web/src/test/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriterTests.java
View File

@ -20,15 +20,16 @@ import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
/**
*
* @author Rafiullah Hamedy
* @author Josh Cummings
*
* @see {@link ClearSiteDataHeaderWriter}
*/
@ -43,53 +44,43 @@ public class ClearSiteDataHeaderWriterTests {
@Before
public void setup() {
request = new MockHttpServletRequest();
request.setSecure(true);
response = new MockHttpServletResponse();
this.request = new MockHttpServletRequest();
this.request.setSecure(true);
this.response = new MockHttpServletResponse();
}
@Test
public void createInstanceWhenMissingSourceThenThrowsException() {
this.thrown.expect(Exception.class);
this.thrown.expectMessage("Sources cannot be empty or null.");
this.thrown.expectMessage("sources cannot be empty or null");
new ClearSiteDataHeaderWriter();
}
@Test
public void createInstanceWhenEmptySourceThenThrowsException() {
this.thrown.expect(Exception.class);
this.thrown.expectMessage("Sources cannot be empty or null.");
new ClearSiteDataHeaderWriter(new String[] {});
}
@Test
public void writeHeaderWhenRequestNotSecureThenHeaderIsNotPresent() {
this.request.setSecure(false);
ClearSiteDataHeaderWriter headerWriter = new ClearSiteDataHeaderWriter("cache");
headerWriter.writeHeaders(request, response);
headerWriter.writeHeaders(this.request, this.response);
assertThat(header().doesNotExist(HEADER_NAME));
assertThat(this.response.getHeader(HEADER_NAME)).isNull();
}
@Test
public void writeHeaderWhenRequestIsSecureThenHeaderValueMatchesPassedSource() {
ClearSiteDataHeaderWriter headerWriter = new ClearSiteDataHeaderWriter("storage");
headerWriter.writeHeaders(request, response);
headerWriter.writeHeaders(this.request, this.response);
assertThat(header().stringValues(HEADER_NAME, "\"storage\""));
assertThat(this.response.getHeader(HEADER_NAME)).isEqualTo("\"storage\"");
}
@Test
public void writeHeaderWhenRequestIsSecureThenHeaderValueMatchesPassedSources() {
ClearSiteDataHeaderWriter headerWriter =
new ClearSiteDataHeaderWriter("cache", "cookies", "storage", "executionContexts");
headerWriter.writeHeaders(this.request, this.response);
headerWriter.writeHeaders(request, response);
assertThat(header().stringValues(HEADER_NAME, "\"cache\", \"cookies\", \"storage\","
+ " \"executionContexts\""));
assertThat(this.response.getHeader(HEADER_NAME))
.isEqualTo("\"cache\", \"cookies\", \"storage\", \"executionContexts\"");
}
}