Skip to content

Commit 40bb8e8

Browse files
odrotbohmodrotbohm
committed
DATAREST-573 - Polishing.
Removed RepositoryRestConfiguration.addCorsMapping(…) as we currently don't have any other shortcut methods for configuration like this. Tweaked the setup of (now Repository)CorsConfigurationAccessor to be created earlier so that we avoid recreation for every lookup. Introduced a NoOpStringValueResolver to be used by default so that we don't need to deal with the case of the resolver being null at the end of the call chain. Replaced constructor of RepositoryCorsConfigurationAccessor with corresponding Lombok annotation. Updated reference documentation accordingly. Original pull request: #233.
1 parent a3870ca commit 40bb8e8

File tree

8 files changed

+88
-87
lines changed

8 files changed

+88
-87
lines changed

‎spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/RepositoryRestConfiguration.java

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.springframework.http.MediaType;
2929
import org.springframework.util.Assert;
3030
import org.springframework.util.StringUtils;
31-
import org.springframework.web.cors.CorsConfiguration;
3231
import org.springframework.web.servlet.config.annotation.CorsRegistration;
3332

3433
/**
@@ -565,22 +564,6 @@ public RepositoryCorsRegistry getCorsRegistry() {
565564
return corsRegistry;
566565
}
567566

568-
/**
569-
* Configures Cross-origin resource sharing given a {@code path}.
570-
*
571-
* @param path path or path pattern, must not be {@literal null} or empty.
572-
* @return the {@link CorsRegistration} to build a CORS configuration.
573-
* @since 2.6
574-
* @see CorsConfiguration
575-
*/
576-
public CorsRegistration addCorsMapping(String path) {
577-
578-
Assert.notNull(path, "Path must not be null!");
579-
Assert.hasText(path, "Path must not be empty!");
580-
581-
return corsRegistry.addMapping(path);
582-
}
583-
584567
/**
585568
* Returns the {@link EntityLookupRegistrar} to create custom {@link EntityLookup} instances registered in the
586569
* configuration.

‎spring-data-rest-core/src/test/java/org/springframework/data/rest/core/RepositoryRestConfigurationUnitTests.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.springframework.data.rest.core.config.EnumTranslationConfiguration;
2727
import org.springframework.data.rest.core.config.MetadataConfiguration;
2828
import org.springframework.data.rest.core.config.ProjectionDefinitionConfiguration;
29+
import org.springframework.data.rest.core.config.RepositoryCorsRegistry;
2930
import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
3031
import org.springframework.data.rest.core.domain.Profile;
3132
import org.springframework.data.rest.core.domain.ProfileRepository;
@@ -149,9 +150,10 @@ public void considersDomainTypeOfValueRepositoryLookupTypes() {
149150
@Test
150151
public void configuresCorsProcessing() {
151152

152-
configuration.addCorsMapping("/hello").maxAge(1234);
153+
RepositoryCorsRegistry registry = configuration.getCorsRegistry();
154+
registry.addMapping("/hello").maxAge(1234);
153155

154-
Map<String, CorsConfiguration> corsConfigurations = configuration.getCorsRegistry().getCorsConfigurations();
156+
Map<String, CorsConfiguration> corsConfigurations = registry.getCorsConfigurations();
155157
assertThat(corsConfigurations, hasKey("/hello"));
156158

157159
CorsConfiguration corsConfiguration = corsConfigurations.get("/hello");

‎spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/AuthorRepository.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,17 @@
1515
*/
1616
package org.springframework.data.rest.webmvc.jpa;
1717

18+
import static org.springframework.web.bind.annotation.RequestMethod.*;
19+
1820
import org.springframework.data.repository.CrudRepository;
1921
import org.springframework.web.bind.annotation.CrossOrigin;
20-
import org.springframework.web.bind.annotation.RequestMethod;
2122

2223
/**
2324
* @author Oliver Gierke
2425
* @author Mark Paluch
2526
*/
26-
@CrossOrigin(origins = "http://not.so.far.away", allowCredentials = "true",
27-
methods = { RequestMethod.GET, RequestMethod.PATCH }, maxAge = 1234)
27+
@CrossOrigin(origins = "http://not.so.far.away", //
28+
allowCredentials = "true", //
29+
methods = { GET, PATCH }, //
30+
maxAge = 1234)
2831
public interface AuthorRepository extends CrudRepository<Author, Long> {}

‎spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/CorsIntegrationTests.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,9 @@
1515
*/
1616
package org.springframework.data.rest.webmvc.jpa;
1717

18-
import static org.hamcrest.Matchers.containsString;
19-
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
20-
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.options;
21-
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
22-
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
18+
import static org.hamcrest.Matchers.*;
19+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
20+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
2321

2422
import org.junit.Test;
2523
import org.springframework.context.annotation.Bean;
@@ -58,7 +56,7 @@ RepositoryRestConfigurer repositoryRestConfigurer() {
5856
@Override
5957
public void configureRepositoryRestConfiguration(RepositoryRestConfiguration config) {
6058

61-
config.addCorsMapping("/books/**") //
59+
config.getCorsRegistry().addMapping("/books/**") //
6260
.allowedMethods("GET", "PUT", "POST") //
6361
.allowedOrigins("http://far.far.away");
6462
}
@@ -79,7 +77,8 @@ public void appliesSelectiveDefaultCorsConfiguration() throws Exception {
7977
.header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "POST")) //
8078
.andExpect(status().isOk()) //
8179
.andExpect(header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, "http://far.far.away")) //
82-
.andExpect(header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS, "GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS,TRACE"));
80+
.andExpect(
81+
header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS, "GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS,TRACE"));
8382
}
8483

8584
/**

‎spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMapping.java

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
*/
1616
package org.springframework.data.rest.webmvc;
1717

18+
import lombok.NonNull;
19+
import lombok.RequiredArgsConstructor;
20+
1821
import java.util.Arrays;
1922
import java.util.HashSet;
2023
import java.util.LinkedHashSet;
@@ -66,7 +69,7 @@ public class RepositoryRestHandlerMapping extends BasePathAwareHandlerMapping {
6669
private final RepositoryRestConfiguration configuration;
6770
private final Repositories repositories;
6871

69-
private StringValueResolver embeddedValueResolver;
72+
private RepositoryCorsConfigurationAccessor corsConfigurationAccessor;
7073
private JpaHelper jpaHelper;
7174

7275
/**
@@ -99,6 +102,8 @@ public RepositoryRestHandlerMapping(ResourceMappings mappings, RepositoryRestCon
99102
this.mappings = mappings;
100103
this.configuration = config;
101104
this.repositories = repositories;
105+
this.corsConfigurationAccessor = new RepositoryCorsConfigurationAccessor(mappings, repositories,
106+
NoOpStringValueResolver.INSTANCE);
102107
}
103108

104109
/**
@@ -108,14 +113,17 @@ public void setJpaHelper(JpaHelper jpaHelper) {
108113
this.jpaHelper = jpaHelper;
109114
}
110115

111-
/* (non-Javadoc)
116+
/*
117+
* (non-Javadoc)
112118
* @see org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping#setEmbeddedValueResolver(org.springframework.util.StringValueResolver)
113119
*/
114120
@Override
115121
public void setEmbeddedValueResolver(StringValueResolver resolver) {
116122

117-
embeddedValueResolver = resolver;
118123
super.setEmbeddedValueResolver(resolver);
124+
125+
this.corsConfigurationAccessor = new RepositoryCorsConfigurationAccessor(mappings, repositories,
126+
resolver == null ? NoOpStringValueResolver.INSTANCE : resolver);
119127
}
120128

121129
/*
@@ -207,15 +215,10 @@ protected CorsConfiguration getCorsConfiguration(Object handler, HttpServletRequ
207215
return corsConfiguration;
208216
}
209217

210-
// Repository root resource
211-
CorsConfiguration repositoryConfiguration = new CorsConfigurationAccessor(mappings, repositories,
212-
embeddedValueResolver).findCorsConfiguration(lookupPath);
213-
214-
if (repositoryConfiguration != null) {
215-
return corsConfiguration != null ? corsConfiguration.combine(repositoryConfiguration) : repositoryConfiguration;
216-
}
218+
CorsConfiguration repositoryCorsConfiguration = corsConfigurationAccessor.findCorsConfiguration(lookupPath);
217219

218-
return corsConfiguration;
220+
return corsConfiguration == null ? repositoryCorsConfiguration
221+
: corsConfiguration.combine(repositoryCorsConfiguration);
219222
}
220223

221224
/**
@@ -231,37 +234,40 @@ private static String getRepositoryBasePath(String repositoryLookupPath) {
231234
}
232235

233236
/**
234-
* Accessor to obtain {@link CorsConfiguration} for exposed repositories.
235-
* <p>
236-
* Exported Repository classes can be annotated with {@link CrossOrigin} to configure CORS for a specific repository.
237+
* No-op {@link StringValueResolver} that returns the given {@link String} value as is.
237238
*
238-
* @author Mark Paluch
239+
* @author Oliver Gierke
239240
* @since 2.6
240241
*/
241-
static class CorsConfigurationAccessor {
242+
enum NoOpStringValueResolver implements StringValueResolver {
242243

243-
private final ResourceMappings mappings;
244-
private final Repositories repositories;
245-
private final StringValueResolver embeddedValueResolver;
244+
INSTANCE;
246245

247-
/**
248-
* Creates a new {@link CorsConfigurationAccessor} given {@link ResourceMappings}, {@link Repositories} and
249-
* {@link StringValueResolver}.
250-
*
251-
* @param mappings must not be {@literal null}.
252-
* @param repositories must not be {@literal null}.
253-
* @param embeddedValueResolver may be {@literal null} if not present.
246+
/*
247+
* (non-Javadoc)
248+
* @see org.springframework.util.StringValueResolver#resolveStringValue(java.lang.String)
254249
*/
255-
CorsConfigurationAccessor(ResourceMappings mappings, Repositories repositories,
256-
StringValueResolver embeddedValueResolver) {
250+
@Override
251+
public String resolveStringValue(String value) {
252+
return value;
253+
}
254+
}
257255

258-
Assert.notNull(mappings, "ResourceMappings must not be null!");
259-
Assert.notNull(repositories, "Repositories must not be null!");
256+
/**
257+
* Accessor to obtain {@link CorsConfiguration} for exposed repositories.
258+
* <p>
259+
* Exported repository classes can be annotated with {@link CrossOrigin} to configure CORS for a specific repository.
260+
*
261+
* @author Mark Paluch
262+
* @author Oliver Gierke
263+
* @since 2.6
264+
*/
265+
@RequiredArgsConstructor
266+
static class RepositoryCorsConfigurationAccessor {
260267

261-
this.mappings = mappings;
262-
this.repositories = repositories;
263-
this.embeddedValueResolver = embeddedValueResolver;
264-
}
268+
private final @NonNull ResourceMappings mappings;
269+
private final @NonNull Repositories repositories;
270+
private final @NonNull StringValueResolver embeddedValueResolver;
265271

266272
CorsConfiguration findCorsConfiguration(String lookupPath) {
267273

@@ -305,7 +311,7 @@ protected CorsConfiguration createConfiguration(Class<?> repositoryInterface) {
305311
if (CollectionUtils.isEmpty(config.getAllowedOrigins())) {
306312
config.setAllowedOrigins(Arrays.asList(CrossOrigin.DEFAULT_ORIGINS));
307313
}
308-
314+
309315
if (CollectionUtils.isEmpty(config.getAllowedMethods())) {
310316
for (HttpMethod httpMethod : HttpMethod.values()) {
311317
config.addAllowedMethod(httpMethod);
@@ -362,7 +368,7 @@ private void updateCorsConfig(CorsConfiguration config, CrossOrigin annotation)
362368
}
363369

364370
private String resolveCorsAnnotationValue(String value) {
365-
return (this.embeddedValueResolver != null ? this.embeddedValueResolver.resolveStringValue(value) : value);
371+
return this.embeddedValueResolver.resolveStringValue(value);
366372
}
367373
}
368374
}

‎spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/CorsConfigurationAccessorUnitTests.java renamed to ‎spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryCorsConfigurationAccessorUnitTests.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
package org.springframework.data.rest.webmvc;
1717

18-
import static org.hamcrest.MatcherAssert.assertThat;
18+
import static org.hamcrest.MatcherAssert.*;
1919
import static org.hamcrest.Matchers.*;
2020

2121
import org.junit.Before;
@@ -25,28 +25,31 @@
2525
import org.mockito.runners.MockitoJUnitRunner;
2626
import org.springframework.data.repository.support.Repositories;
2727
import org.springframework.data.rest.core.mapping.ResourceMappings;
28-
import org.springframework.data.rest.webmvc.RepositoryRestHandlerMapping.CorsConfigurationAccessor;
28+
import org.springframework.data.rest.webmvc.RepositoryRestHandlerMapping.NoOpStringValueResolver;
29+
import org.springframework.data.rest.webmvc.RepositoryRestHandlerMapping.RepositoryCorsConfigurationAccessor;
2930
import org.springframework.web.bind.annotation.CrossOrigin;
3031
import org.springframework.web.bind.annotation.RequestMethod;
3132
import org.springframework.web.cors.CorsConfiguration;
3233

3334
/**
34-
* Unit tests for {@link CorsConfigurationAccessor}.
35+
* Unit tests for {@link RepositoryCorsConfigurationAccessor}.
3536
*
3637
* @author Mark Paluch
38+
* @author Oliver Gierke
3739
* @soundtrack Aso Mamiko - Drive Me Crazy (Club Mix)
40+
* @since 2.6
3841
*/
3942
@RunWith(MockitoJUnitRunner.class)
40-
public class CorsConfigurationAccessorUnitTests {
43+
public class RepositoryCorsConfigurationAccessorUnitTests {
4144

42-
CorsConfigurationAccessor accessor;
45+
RepositoryCorsConfigurationAccessor accessor;
4346

4447
@Mock ResourceMappings mappings;
4548
@Mock Repositories repositories;
4649

4750
@Before
4851
public void before() throws Exception {
49-
accessor = new CorsConfigurationAccessor(mappings, repositories, null);
52+
accessor = new RepositoryCorsConfigurationAccessor(mappings, repositories, NoOpStringValueResolver.INSTANCE);
5053
}
5154

5255
/**
@@ -90,7 +93,10 @@ interface PlainRepository {}
9093
@CrossOrigin
9194
interface AnnotatedRepository {}
9295

93-
@CrossOrigin(origins = "http://far.far.away", allowedHeaders = "Content-type", maxAge = 1234,
94-
exposedHeaders = "Accept", methods = RequestMethod.PATCH, allowCredentials = "true")
96+
@CrossOrigin(origins = "http://far.far.away", //
97+
allowedHeaders = "Content-type", //
98+
maxAge = 1234, exposedHeaders = "Accept", //
99+
methods = RequestMethod.PATCH, //
100+
allowCredentials = "true")
95101
interface FullyConfiguredCorsRepository {}
96102
}

‎spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestHandlerMappingUnitTests.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.mockito.Mock;
2828
import org.mockito.runners.MockitoJUnitRunner;
2929
import org.springframework.data.domain.Sort;
30+
import org.springframework.data.repository.support.Repositories;
3031
import org.springframework.data.rest.core.config.EnumTranslationConfiguration;
3132
import org.springframework.data.rest.core.config.MetadataConfiguration;
3233
import org.springframework.data.rest.core.config.ProjectionDefinitionConfiguration;
@@ -57,6 +58,7 @@ public class RepositoryRestHandlerMappingUnitTests {
5758
}
5859

5960
@Mock ResourceMappings mappings;
61+
@Mock Repositories repositories;
6062

6163
RepositoryRestConfiguration configuration;
6264
RepositoryRestHandlerMapping handlerMapping;
@@ -69,7 +71,7 @@ public void setUp() throws Exception {
6971
configuration = new RepositoryRestConfiguration(new ProjectionDefinitionConfiguration(),
7072
new MetadataConfiguration(), mock(EnumTranslationConfiguration.class));
7173

72-
handlerMapping = new RepositoryRestHandlerMapping(mappings, configuration);
74+
handlerMapping = new RepositoryRestHandlerMapping(mappings, configuration, repositories);
7375
handlerMapping.setApplicationContext(CONTEXT);
7476

7577
mockRequest = new MockHttpServletRequest();

‎src/main/asciidoc/configuring-cors.adoc

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ In the above example CORS support is enabled for the whole `PersonRepository`. `
2121
[source, java]
2222
----
2323
@CrossOrigin(origins = "http://domain2.com",
24-
methods = { RequestMethod.GET, RequestMethod.POST, RequestMethod.DELETE }, maxAge = 3600)
24+
methods = { RequestMethod.GET, RequestMethod.POST, RequestMethod.DELETE },
25+
maxAge = 3600)
2526
interface PersonRepository extends CrudRepository<Person, Long> {}
2627
----
2728

@@ -34,14 +35,13 @@ Spring Data REST fully supports http://docs.spring.io/spring/docs/current/spring
3435
[source, java]
3536
----
3637
@RepositoryRestController
37-
@RequestMapping("/person")
3838
public class PersonController {
3939
40-
@CrossOrigin(maxAge = 3600)
41-
@RequestMapping(method = RequestMethod.GET, "/xml/{id}", produces = MediaType.APPLICATION_XML_VALUE)
42-
public Person retrieve(@PathVariable Long id) {
43-
// ...
44-
}
40+
@CrossOrigin(maxAge = 3600)
41+
@RequestMapping(path = "/people/xml/{id}", method = RequestMethod.GET, produces = MediaType.APPLICATION_XML_VALUE)
42+
public Person retrieve(@PathVariable Long id) {
43+
// …
44+
}
4545
}
4646
----
4747

@@ -61,12 +61,12 @@ public class SpringDataRestCustomization extends RepositoryRestConfigurerAdapter
6161
@Override
6262
public void configureRepositoryRestConfiguration(RepositoryRestConfiguration config) {
6363
64-
config.addCorsMapping("/person/**")
65-
.allowedOrigins("http://domain2.com")
66-
.allowedMethods("PUT", "DELETE")
67-
.allowedHeaders("header1", "header2", "header3")
68-
.exposedHeaders("header1", "header2")
69-
.allowCredentials(false).maxAge(3600);
64+
config.getCorsRegistry().addCorsMapping("/person/**")
65+
.allowedOrigins("http://domain2.com")
66+
.allowedMethods("PUT", "DELETE")
67+
.allowedHeaders("header1", "header2", "header3")
68+
.exposedHeaders("header1", "header2")
69+
.allowCredentials(false).maxAge(3600);
7070
}
7171
}
7272
----

0 commit comments

Comments
 (0)