From 7f1799832d4533e4482bce07b3f46b14a8fbcf9f Mon Sep 17 00:00:00 2001
From: Oliver Gierke <ogierke@pivotal.io>
Date: Fri, 19 Jun 2015 14:38:08 +0200
Subject: [PATCH] DATAREST-327 - Polishing.

Moved Spring Security specific dependencies to a dedicated profile in pom.xml. JavaDoc polishing. Removed assertions on the response message.

Original pull request: #171.
---
 spring-data-rest-webmvc/pom.xml               |  93 +++++++---------
 .../data/rest/webmvc/security/Person.java     |  16 +--
 .../PreAuthorizedOrderRepository.java         |   4 +-
 .../security/SecureJpaConfiguration.java      |   4 +-
 .../security/SecuredPersonRepository.java     |  15 +++
 .../security/SecurityConfiguration.java       |  15 +++
 .../security/SecurityIntegrationTests.java    | 101 +++++++++---------
 .../data/rest/webmvc/solr/SolrWebTests.java   |   4 +-
 ...HandlingCustomizationIntegrationTests.java |  11 --
 9 files changed, 136 insertions(+), 127 deletions(-)

diff --git a/spring-data-rest-webmvc/pom.xml b/spring-data-rest-webmvc/pom.xml
index 7fb436d63..10bb4cd11 100644
--- a/spring-data-rest-webmvc/pom.xml
+++ b/spring-data-rest-webmvc/pom.xml
@@ -113,58 +113,6 @@
 			<scope>test</scope>
 		</dependency>
 
-		<dependency>
-			<groupId>org.apache.solr</groupId>
-			<artifactId>solr-core</artifactId>
-			<version>4.10.1</version>
-			<scope>test</scope>
-			<exclusions>
-				<exclusion>
-					<groupId>org.slf4j</groupId>
-					<artifactId>slf4j-api</artifactId>
-				</exclusion>
-				<exclusion>
-					<artifactId>jdk.tools</artifactId>
-					<groupId>jdk.tools</groupId>
-				</exclusion>
-			</exclusions>
-		</dependency>
-
-		<dependency>
-			<groupId>org.springframework.security</groupId>
-			<artifactId>spring-security-core</artifactId>
-			<version>${spring-security.version}</version>
-			<scope>test</scope>
-		</dependency>
-
-		<dependency>
-			<groupId>org.springframework.security</groupId>
-			<artifactId>spring-security-config</artifactId>
-			<version>${spring-security.version}</version>
-			<scope>test</scope>
-		</dependency>
-
-		<dependency>
-			<groupId>org.springframework.security</groupId>
-			<artifactId>spring-security-web</artifactId>
-			<version>${spring-security.version}</version>
-			<scope>test</scope>
-		</dependency>
-
-		<dependency>
-			<groupId>org.springframework.security</groupId>
-			<artifactId>spring-security-test</artifactId>
-			<version>${spring-security.version}</version>
-			<scope>test</scope>
-		</dependency>
-
-		<dependency>
-			<groupId>commons-lang</groupId>
-			<artifactId>commons-lang</artifactId>
-			<version>2.6</version>
-			<scope>test</scope>
-		</dependency>
-
 	</dependencies>
 
 	<profiles>
@@ -350,6 +298,47 @@
 
 		</profile>
 
+		<profile>
+
+			<id>spring-security</id>
+
+			<activation>
+				<activeByDefault>true</activeByDefault>
+			</activation>
+
+			<dependencies>
+
+				<dependency>
+					<groupId>org.springframework.security</groupId>
+					<artifactId>spring-security-core</artifactId>
+					<version>${spring-security.version}</version>
+					<scope>test</scope>
+				</dependency>
+
+				<dependency>
+					<groupId>org.springframework.security</groupId>
+					<artifactId>spring-security-config</artifactId>
+					<version>${spring-security.version}</version>
+					<scope>test</scope>
+				</dependency>
+
+				<dependency>
+					<groupId>org.springframework.security</groupId>
+					<artifactId>spring-security-web</artifactId>
+					<version>${spring-security.version}</version>
+					<scope>test</scope>
+				</dependency>
+
+				<dependency>
+					<groupId>org.springframework.security</groupId>
+					<artifactId>spring-security-test</artifactId>
+					<version>${spring-security.version}</version>
+					<scope>test</scope>
+				</dependency>
+
+			</dependencies>
+		</profile>
+
 	</profiles>
 
 	<build>
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/Person.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/Person.java
index 5e5d3efb5..3ba17509b 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/Person.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/Person.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2014 the original author or authors.
+ * Copyright 2012-2015 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.
@@ -15,6 +15,12 @@
  */
 package org.springframework.data.rest.webmvc.security;
 
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+
 import javax.persistence.Entity;
 import javax.persistence.GeneratedValue;
 import javax.persistence.Id;
@@ -22,15 +28,11 @@ import javax.persistence.ManyToMany;
 import javax.persistence.ManyToOne;
 import javax.persistence.PrePersist;
 import javax.validation.constraints.NotNull;
-import java.util.ArrayList;
-import java.util.Calendar;
-import java.util.Collections;
-import java.util.Date;
-import java.util.List;
+
+import org.springframework.data.rest.core.annotation.Description;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
-import org.springframework.data.rest.core.annotation.Description;
 
 /**
  * An entity that represents a person.
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/PreAuthorizedOrderRepository.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/PreAuthorizedOrderRepository.java
index 442e73369..cc0733336 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/PreAuthorizedOrderRepository.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/PreAuthorizedOrderRepository.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2013-2015 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.
@@ -42,4 +42,4 @@ public interface PreAuthorizedOrderRepository extends CrudRepository<Order, Long
 	@Override
 	void deleteAll();
 }
-// end::code[]
\ No newline at end of file
+// end::code[]
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecureJpaConfiguration.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecureJpaConfiguration.java
index aaf38e1dd..83ab02409 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecureJpaConfiguration.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecureJpaConfiguration.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 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.
@@ -64,6 +64,4 @@ public class SecureJpaConfiguration {
 	public PlatformTransactionManager transactionManager() {
 		return new JpaTransactionManager();
 	}
-
 }
-
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecuredPersonRepository.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecuredPersonRepository.java
index 7f0131a34..e83d5fd61 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecuredPersonRepository.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecuredPersonRepository.java
@@ -1,3 +1,18 @@
+/*
+ * Copyright 2014-2015 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
+ *
+ *      http://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.data.rest.webmvc.security;
 
 import org.springframework.data.repository.CrudRepository;
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecurityConfiguration.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecurityConfiguration.java
index 82f65840b..eab21a5ba 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecurityConfiguration.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecurityConfiguration.java
@@ -1,3 +1,18 @@
+/*
+ * Copyright 2014-2015 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
+ *
+ *      http://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.data.rest.webmvc.security;
 
 import org.springframework.beans.factory.annotation.Autowired;
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecurityIntegrationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecurityIntegrationTests.java
index 4f2c0a348..8f95efbba 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecurityIntegrationTests.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/security/SecurityIntegrationTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 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.
@@ -15,50 +15,40 @@
  */
 package org.springframework.data.rest.webmvc.security;
 
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.*;
-import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
+import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.*;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
 
-import java.util.Arrays;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.rest.webmvc.AbstractWebIntegrationTests;
 import org.springframework.data.rest.webmvc.TestMvcClient;
-import org.springframework.hateoas.Link;
 import org.springframework.hateoas.config.EnableHypermediaSupport;
 import org.springframework.http.HttpStatus;
 import org.springframework.mock.web.MockHttpServletResponse;
-import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor;
-import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
-import org.springframework.test.context.web.WebAppConfiguration;
-import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.web.context.WebApplicationContext;
-import org.springframework.web.util.NestedServletException;
 
 /**
  * Test Spring Data REST in the context of being locked down by Spring Security. Uses MockMvc to simulate HTTP-based
- * interactions. Testing is possible on the repository level, but that doesn't align with the mission
- * of Spring Data REST.
+ * interactions. Testing is possible on the repository level, but that doesn't align with the mission of Spring Data
+ * REST.
  *
  * @author Greg Turnquist
  * @author Rob Winch
  */
 @RunWith(SpringJUnit4ClassRunner.class)
-@ContextConfiguration(classes = {SecureJpaConfiguration.class, SecurityConfiguration.class})
+@ContextConfiguration(classes = { SecureJpaConfiguration.class, SecurityConfiguration.class })
 @Transactional
 @EnableHypermediaSupport(type = EnableHypermediaSupport.HypermediaType.HAL)
 public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
@@ -100,8 +90,9 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 				build();
 	}
 
-	//=================================================================
-
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void deletePersonAccessDeniedForNoCredentials() throws Exception {
 
@@ -116,12 +107,12 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 		// Clear any side effects of logging in to get the URI from security.
 		SecurityContextHolder.clearContext();
 
-		MockHttpServletResponse deleteResponse = mvc.perform(delete(href)).//
-				andExpect(status().isUnauthorized()).//
-				andReturn().getResponse();
-		assertThat(deleteResponse.getErrorMessage(), is("Full authentication is required to access this resource"));
+		mvc.perform(delete(href)).andExpect(status().isUnauthorized());
 	}
 
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void deletePersonAccessDeniedForUsers() throws Exception {
 
@@ -133,13 +124,13 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 		// Clear any side effects of logging in to get the URI from security.
 		SecurityContextHolder.clearContext();
 
-		MockHttpServletResponse deleteResponse = mvc.perform(delete(href).//
-				with(user("user").roles("USER"))).//
-				andExpect(status().isForbidden()).//
-				andReturn().getResponse();
-		assertThat(deleteResponse.getErrorMessage(), is("Access is denied"));
+		mvc.perform(delete(href).with(user("user").roles("USER"))).//
+				andExpect(status().isForbidden());
 	}
 
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void deletePersonAccessGrantedForAdmins() throws Exception {
 
@@ -155,17 +146,19 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 			.andExpect(status().is(HttpStatus.NO_CONTENT.value()));
 	}
 
-	//=================================================================
-
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void findAllPeopleAccessDeniedForNoCredentials() throws Throwable {
 
-		MockHttpServletResponse response = mvc.perform(get(client.discoverUnique("people").expand().getHref())).//
-				andExpect(status().isUnauthorized()).//
-				andReturn().getResponse();
-		assertThat(response.getErrorMessage(), is("Full authentication is required to access this resource"));
+		mvc.perform(get(client.discoverUnique("people").expand().getHref())).//
+				andExpect(status().isUnauthorized());
 	}
 
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void findAllPeopleAccessGrantedForUsers() throws Throwable {
 
@@ -174,6 +167,9 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 				andExpect(status().isOk());
 	}
 
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void findAllPeopleAccessGrantedForAdmins() throws Throwable {
 
@@ -182,9 +178,9 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 				andExpect(status().isOk());
 	}
 
-	//=================================================================
-
-
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void deleteOrderAccessDeniedForNoCredentials() throws Exception {
 
@@ -197,12 +193,12 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 		// Clear any side effects of logging in to get the URI from security.
 		SecurityContextHolder.clearContext();
 
-		MockHttpServletResponse deleteResponse = mvc.perform(delete(href)).//
-				andExpect(status().isUnauthorized()).//
-				andReturn().getResponse();
-		assertThat(deleteResponse.getErrorMessage(), is("Full authentication is required to access this resource"));
+		mvc.perform(delete(href)).andExpect(status().isUnauthorized());
 	}
 
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void deleteOrderAccessDeniedForUsers() throws Exception {
 
@@ -211,12 +207,13 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 				andReturn().getResponse();
 		String href = assertHasJsonPathValue("$._embedded.orders[0]._links.self.href", response);
 
-		MockHttpServletResponse deleteResponse = mvc.perform(delete(href).with(user("user").roles("USER"))).//
-				andExpect(status().isForbidden()).//
-				andReturn().getResponse();
-		assertThat(deleteResponse.getErrorMessage(), is("Access is denied"));
+		mvc.perform(delete(href).with(user("user").roles("USER"))).//
+				andExpect(status().isForbidden());
 	}
 
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void deleteOrderAccessGrantedForAdmins() throws Exception {
 
@@ -232,17 +229,19 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 				.andExpect(status().is(HttpStatus.NO_CONTENT.value()));
 	}
 
-	//=================================================================
-
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void findAllOrdersAccessDeniedForNoCredentials() throws Throwable {
 
-		MockHttpServletResponse response = mvc.perform(get(client.discoverUnique("orders").expand().getHref())).//
-			andExpect(status().isUnauthorized()).//
-			andReturn().getResponse();
-		assertThat(response.getErrorMessage(), is("Full authentication is required to access this resource"));
+		mvc.perform(get(client.discoverUnique("orders").expand().getHref())).//
+				andExpect(status().isUnauthorized());
 	}
 
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void findAllOrdersAccessGrantedForUsers() throws Throwable {
 
@@ -251,6 +250,9 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 				andExpect(status().isOk());
 	}
 
+	/**
+	 * @see DATAREST-327
+	 */
 	@Test
 	public void findAllOrdersAccessGrantedForAdmins() throws Throwable {
 
@@ -258,5 +260,4 @@ public class SecurityIntegrationTests extends AbstractWebIntegrationTests {
 				with(user("user").roles("USER", "ADMIN"))).//
 				andExpect(status().isOk());
 	}
-
 }
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/solr/SolrWebTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/solr/SolrWebTests.java
index 81393317f..bf93cd922 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/solr/SolrWebTests.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/solr/SolrWebTests.java
@@ -29,7 +29,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
-import org.springframework.data.rest.webmvc.AbstractWebIntegrationTests;
+import org.springframework.data.rest.webmvc.CommonWebTests;
 import org.springframework.data.solr.repository.config.EnableSolrRepositories;
 import org.springframework.hateoas.Link;
 import org.springframework.http.MediaType;
@@ -42,7 +42,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
  * @author Christoph Strobl
  */
 @ContextConfiguration(classes = { SolrWebTests.MyConf.class })
-public class SolrWebTests extends AbstractWebIntegrationTests {
+public class SolrWebTests extends CommonWebTests {
 
 	public static @ClassRule TemporaryFolder TEMP_FOLDER = new TemporaryFolder();
 
diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ExceptionHandlingCustomizationIntegrationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ExceptionHandlingCustomizationIntegrationTests.java
index a15e2b35a..da2d84b27 100644
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ExceptionHandlingCustomizationIntegrationTests.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ExceptionHandlingCustomizationIntegrationTests.java
@@ -18,8 +18,6 @@ package org.springframework.data.rest.webmvc.support;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
 
-import java.util.Collections;
-
 import org.junit.Test;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
@@ -72,13 +70,4 @@ public class ExceptionHandlingCustomizationIntegrationTests extends AbstractWebI
 		mvc.perform(get(link.getHref())).//
 				andExpect(status().isInternalServerError());
 	}
-
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.data.rest.webmvc.AbstractWebIntegrationTests#expectedRootLinkRels()
-	 */
-	@Override
-	protected Iterable<String> expectedRootLinkRels() {
-		return Collections.emptySet();
-	}
 }