Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make clear which methods expect mapped vs. lower-level CORS config nodes #1700

Merged
merged 2 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ class CrossOriginFilter implements ContainerRequestFilter, ContainerResponseFilt
CrossOriginFilter() {
Config config = (Config) ConfigProvider.getConfig();

cors = CorsSupportMp.builder().config(config.get(CORS_CONFIG_KEY))
CorsSupportMp.Builder corsBuilder = CorsSupportMp.builder();
config.get(CORS_CONFIG_KEY).ifExists(corsBuilder::mappedConfig);
cors = corsBuilder
.secondaryLookupSupplier(this::crossOriginFromAnnotationSupplier)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,21 @@ public boolean isActive() {
return isEnabled && !crossOriginConfigMatchables.isEmpty();
}

Aggregator config(Config config) {
if (config.exists()) {
ConfigValue<CrossOriginConfig.Builder> configValue = config.as(CrossOriginConfig::builder);
if (configValue.isPresent()) {
CrossOriginConfig crossOriginConfig = configValue.get().build();
addPathlessCrossOrigin(crossOriginConfig);
}
}
return this;
}

/**
* Add cross-origin information from a {@link Config} node.
* Add mapped cross-origin information from a {@link Config} node.
*
* @param config {@code Config} node containing
* @param config {@code Config} node containing mapped {@code CrossOriginConfig} data
* @return updated builder
*/
Aggregator mappedConfig(Config config) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@
import io.helidon.webserver.ServerResponse;
import io.helidon.webserver.Service;

import static io.helidon.webserver.cors.Aggregator.PATHLESS_KEY;

/**
* SE implementation of {@link CorsSupportBase}.
*/
Expand Down Expand Up @@ -93,8 +91,21 @@ public static CorsSupport create(Config config) {
if (!config.exists()) {
throw MissingValueException.create(config.key());
}
Builder builder = builder().addCrossOrigin(PATHLESS_KEY, CrossOriginConfig.builder(config).build());
return builder.build();
return builder().config(config).build();
}

/**
* Creates a new {@code CorsSupport} instance based on the provided configuration expected to contain mapped cross-origin
* config information.
*
* @param config node containing the mapped cross-origin information
* @return initialized {@code CorsSupport} instance
*/
public static CorsSupport createMapped(Config config) {
if (!config.exists()) {
throw MissingValueException.create(config.key());
}
return builder().mappedConfig(config).build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,18 @@ public B config(Config config) {
return me();
}

/**
* Merges mapped CORS config information. Typically, the app or component will retrieve the provided {@code Config}
* instance from its own config.
*
* @param config the mapped CORS config information
* @return the updated builder
*/
public B mappedConfig(Config config) {
helperBuilder.mappedConfig(config);
return me();
}

/**
* Sets whether CORS support should be enabled or not.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,6 @@ public enum RequestType {
HelidonFeatures.register(HelidonFlavor.SE, "CORS");
}

/**
* Creates a new instance using CORS config in the provided {@link Config}.
*
* @param config Config node containing CORS set-up
* @return new instance based on the config
*/
public static <Q, R> CorsSupportHelper<Q, R> create(Config config) {
return CorsSupportHelper.<Q, R>builder().config(config).build();
}

/**
* Creates a new instance that is enabled but with no path mappings.
*
Expand Down Expand Up @@ -219,6 +209,17 @@ public Builder<Q, R> secondaryLookupSupplier(Supplier<Optional<CrossOriginConfig
* @return updated builder
*/
public Builder<Q, R> config(Config config) {
aggregator.config(config);
return this;
}

/**
* Adds mapped cross-origin information via config.
*
* @param config config node containing mapped CORS set-up information
* @return updated builder
*/
public Builder<Q, R> mappedConfig(Config config) {
aggregator.mappedConfig(config);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,28 @@ static Routing.Builder prepRouting() {
* Load a specific config for "/othergreet."
*/
Config twoCORSConfig = minimalConfig(ConfigSources.classpath("twoCORS.yaml"));
Config twoCORSMappedConfig = twoCORSConfig.get("cors-2-setup");
if (!twoCORSMappedConfig.exists()) {
throw new IllegalArgumentException("Expected config 'cors-2-setup' from twoCORS.yaml not found");
}
Config somewhatRestrictedConfig = twoCORSConfig.get("somewhat-restrictive");
if (!somewhatRestrictedConfig.exists()) {
throw new IllegalArgumentException("Expected config 'somewhat-restrictive' from twoCORS.yaml not found");
}
Config corsMappedSetupConfig = Config.create().get("cors-setup");
if (!corsMappedSetupConfig.exists()) {
throw new IllegalArgumentException("Expected config 'cors-setup' from default app config not found");
}

Routing.Builder builder = Routing.builder()
.register(GREETING_PATH,
CorsSupport.builder().config(Config.create().get("cors-setup")).build(),
CorsSupport.createMapped(corsMappedSetupConfig),
new GreetService())
.register(OTHER_GREETING_PATH,
CorsSupport.builder().config(twoCORSConfig.get("cors-2-setup")).build(),
CorsSupport.createMapped(twoCORSMappedConfig),
new GreetService("Other Hello"))
.any(TestHandlerRegistration.CORS4_CONTEXT_ROOT,
CorsSupport.create(twoCORSConfig.get("somewhat-restrictive")), // handler settings from config subnode
CorsSupport.create(somewhatRestrictedConfig), // handler settings from config subnode
(req, resp) -> resp.status(Http.Status.OK_200).send())
.get(TestHandlerRegistration.CORS4_CONTEXT_ROOT, // handler settings in-line
CorsSupport.builder()
Expand Down