Skip to content

Commit

Permalink
Revert "Refactor ClientConfig defaults (#153)"
Browse files Browse the repository at this point in the history
This reverts commit 8a86ceb.
  • Loading branch information
mullermp committed Aug 4, 2023
1 parent 6343405 commit e2b1ae3
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 246 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import software.amazon.smithy.model.traits.HttpChecksumRequiredTrait;
import software.amazon.smithy.model.traits.HttpTrait;
import software.amazon.smithy.ruby.codegen.config.ClientConfig;
import software.amazon.smithy.ruby.codegen.config.ConfigProviderChain;
import software.amazon.smithy.ruby.codegen.middleware.Middleware;
import software.amazon.smithy.ruby.codegen.middleware.MiddlewareStackStep;
import software.amazon.smithy.ruby.codegen.util.Streaming;
Expand Down Expand Up @@ -79,42 +80,48 @@ public ApplicationTransport(
*/
public static ApplicationTransport createDefaultHttpApplicationTransport() {

ClientConfig endpoint = ClientConfig.builder()
ClientConfig endpoint = (new ClientConfig.Builder())
.name("endpoint")
.type("String")
.documentation("Endpoint of the service")
.allowOperationOverride()
.defaultDynamicValue("proc { |cfg| cfg[:stub_responses] ? 'http://localhost' : nil }")
.defaults(new ConfigProviderChain.Builder()
.dynamicProvider("proc { |cfg| cfg[:stub_responses] ? 'http://localhost' : nil }")
.build()
)
.build();

ClientFragment request = ClientFragment.builder()
ClientFragment request = (new ClientFragment.Builder())
.addConfig(endpoint)
// TODO: Replace URI with Endpoint middleware - should be a blank request
.render((self, ctx) -> "Hearth::HTTP::Request.new(uri: URI(" + endpoint.renderGetConfigValue() + "))")
.build();

ClientFragment response = ClientFragment.builder()
ClientFragment response = (new ClientFragment.Builder())
.render((self, ctx) -> "Hearth::HTTP::Response.new(body: response_body)")
.build();

ClientConfig httpClient = ClientConfig.builder()
ClientConfig httpClient = (new ClientConfig.Builder())
.name("http_client")
.type("Hearth::HTTP::Client")
.documentation("The HTTP Client to use for request transport.")
.documentationDefaultValue("Hearth::HTTP::Client.new")
.allowOperationOverride()
.defaultDynamicValue("proc { |cfg| Hearth::HTTP::Client.new(logger: cfg[:logger]) }")
.defaults(new ConfigProviderChain.Builder()
.dynamicProvider("proc { |cfg| Hearth::HTTP::Client.new(logger: cfg[:logger]) }")
.build()
)
.build();

ClientFragment client = ClientFragment.builder()
ClientFragment client = (new ClientFragment.Builder())
.addConfig(httpClient)
.render((self, ctx) -> httpClient.renderGetConfigValue())
.build();

MiddlewareList defaultMiddleware = (transport, context) -> {
List<Middleware> middleware = new ArrayList<>();

middleware.add(Middleware.builder()
middleware.add(new Middleware.Builder()
.klass(Hearth.BUILD_MIDDLEWARE)
.step(MiddlewareStackStep.BUILD)
.operationParams((ctx, operation) -> {
Expand All @@ -126,7 +133,7 @@ public static ApplicationTransport createDefaultHttpApplicationTransport() {
.build()
);

middleware.add(Middleware.builder()
middleware.add((new Middleware.Builder())
.klass("Hearth::HTTP::Middleware::ContentLength")
.operationPredicate(
(model, service, operation) ->
Expand All @@ -137,15 +144,15 @@ public static ApplicationTransport createDefaultHttpApplicationTransport() {
.build()
);

middleware.add(Middleware.builder()
middleware.add((new Middleware.Builder())
.klass("Hearth::HTTP::Middleware::ContentMD5")
.step(MiddlewareStackStep.AFTER_BUILD)
.operationPredicate(
(model, service, operation) -> operation.hasTrait(HttpChecksumRequiredTrait.class))
.build()
);

middleware.add(Middleware.builder()
middleware.add((new Middleware.Builder())
.klass(Hearth.PARSE_MIDDLEWARE)
.step(MiddlewareStackStep.PARSE)
.operationParams((ctx, operation) -> {
Expand All @@ -158,10 +165,10 @@ public static ApplicationTransport createDefaultHttpApplicationTransport() {
successCode = "" + httpTrait.get().getCode();
}
String errors = operation.getErrors()
.stream()
.map((error) -> "Errors::"
+ ctx.symbolProvider().toSymbol(ctx.model().expectShape(error)).getName())
.collect(Collectors.joining(", "));
.stream()
.map((error) -> "Errors::"
+ ctx.symbolProvider().toSymbol(ctx.model().expectShape(error)).getName())
.collect(Collectors.joining(", "));
params.put("error_parser",
"Hearth::HTTP::ErrorParser.new("
+ "error_module: Errors, success_status: " + successCode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ public ClientFragment(Builder builder) {
this.render = builder.render;
}

public static Builder builder() {
return new Builder();
}

/**
* @return set of client config to apply to support this fragment.
*/
Expand Down Expand Up @@ -84,6 +80,7 @@ public static class Builder implements SmithyBuilder<ClientFragment> {
};

/**
*
* @param config config to be added to support this fragment.
* @return this builder.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

import java.util.Objects;
import java.util.Set;
import software.amazon.smithy.ruby.codegen.ClientFragment;
import software.amazon.smithy.ruby.codegen.GenerationContext;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.SmithyUnstableApi;

Expand All @@ -31,8 +29,11 @@ public class ClientConfig {
private final String name;
private final String type;
private final String documentation;
private final String documentationDefaultValue;
private final String documentationType;
private final ConfigDefaults defaults;
private final ConfigProviderChain defaults;

private final String defaultLiteral;
private final boolean allowOperationOverride;

/**
Expand All @@ -42,22 +43,13 @@ public ClientConfig(Builder builder) {
this.name = builder.name;
this.type = builder.type;
this.documentation = builder.documentation;
this.documentationDefaultValue = builder.documentationDefaultValue;
this.documentationType = builder.documentationType;
if (builder.defaults != null) {
this.defaults = builder.defaults;
} else {
this.defaults = new DefaultLiteral("[]");
}
if (builder.documentationDefaultValue != null) {
this.defaults.setDocumentationDefault(builder.documentationDefaultValue);
}
this.defaults = builder.defaults;
this.defaultLiteral = builder.defaultLiteral;
this.allowOperationOverride = builder.allowOperationOverride;
}

public static Builder builder() {
return new Builder();
}

/**
* @return The name of the config - this will be the initialization parameter name as well
* as the member variable name.
Expand Down Expand Up @@ -87,7 +79,13 @@ public String getDocumentation() {
* @return Documented default value.
*/
public String getDocumentationDefaultValue() {
return defaults.getDocumentationDefault().orElse("");
if (documentationDefaultValue != null) {
return documentationDefaultValue;
}
if (defaults != null) {
return defaults.getDocumentationDefault().orElse("");
}
return "";
}

/**
Expand All @@ -101,12 +99,17 @@ public String getDocumentationType() {
}

/**
* Render the defaults for this config.
*
* @param context generation context
* @return chain of defaults to use.
*/
public ConfigProviderChain getDefaults() {
return defaults;
}

/**
* @return a literal default value to be rendered. Must result in an array of defaults in Ruby.
*/
public String renderDefaults(GenerationContext context) {
return defaults.renderDefault(context);
public String getDefaultLiteral() {
return defaultLiteral;
}

/**
Expand All @@ -133,7 +136,12 @@ public String renderGetConfigValue() {
public void addToConfigCollection(Set<ClientConfig> configCollection) {
if (!configCollection.contains(this)) {
configCollection.add(this);
defaults.addToConfigCollection(configCollection);
if (defaults != null) {
defaults.getProviders().forEach((p) -> {
p.providerFragment().getClientConfig()
.forEach((c) -> c.addToConfigCollection(configCollection));
});
}
}
}

Expand All @@ -155,6 +163,10 @@ public int hashCode() {
return Objects.hash(getName(), getType());
}

public static Builder builder() {
return new Builder();
}

/**
* Builder for ClientConfig.
*/
Expand All @@ -164,13 +176,10 @@ public static class Builder implements SmithyBuilder<ClientConfig> {
private String documentation;
private String documentationDefaultValue;
private String documentationType;
private ConfigDefaults defaults;
private ConfigProviderChain defaults;
private String defaultLiteral;
private boolean allowOperationOverride = false;

protected Builder() {

}

/**
* @param name name of the config option.
* @return this builder.
Expand Down Expand Up @@ -199,7 +208,7 @@ public Builder documentation(String documentation) {
}

/**
* @param defaultValue an optional default value to be use in documentation.
* @param defaultValue an optional default value
* @return this builder
*/
public Builder documentationDefaultValue(String defaultValue) {
Expand Down Expand Up @@ -233,20 +242,7 @@ public Builder allowOperationOverride() {
* @return this builder
*/
public Builder defaultPrimitiveValue(String value) {
validateDefaultNotSet();
this.defaults = ConfigProviderChain.builder().staticProvider(value).build();
return this;
}

public Builder defaultDynamicValue(String rubyDefaultBlock) {
validateDefaultNotSet();
this.defaults = ConfigProviderChain.builder().dynamicProvider(rubyDefaultBlock).build();
return this;
}

public Builder defaultDynamicValue(ClientFragment rubyDefaultBlock) {
validateDefaultNotSet();
this.defaults = ConfigProviderChain.builder().dynamicProvider(rubyDefaultBlock).build();
this.defaults = new ConfigProviderChain.Builder().staticProvider(value).build();
return this;
}

Expand All @@ -255,8 +251,7 @@ public Builder defaultDynamicValue(ClientFragment rubyDefaultBlock) {
* @return this builder
*/
public Builder defaultValue(String value) {
validateDefaultNotSet();
this.defaults = ConfigProviderChain.builder()
this.defaults = new ConfigProviderChain.Builder()
.dynamicProvider("proc { " + value + " }")
.build();
return this;
Expand All @@ -266,8 +261,11 @@ public Builder defaultValue(String value) {
* @param defaults chain of default providers to use.
* @return this builder.
*/
public Builder defaults(ConfigDefaults defaults) {
validateDefaultNotSet();
public Builder defaults(ConfigProviderChain defaults) {
if (defaultLiteral != null) {
throw new IllegalArgumentException("ConfigProviderChain defaults are incompatible "
+ "with defaultLiteral. You must provide only one.");
}
this.defaults = defaults;
return this;
}
Expand All @@ -277,20 +275,17 @@ public Builder defaults(ConfigDefaults defaults) {
* @return this builder
*/
public Builder defaultLiteral(String defaultLiteral) {
validateDefaultNotSet();
this.defaults = new DefaultLiteral(defaultLiteral);
if (defaults != null) {
throw new IllegalArgumentException("ConfigProviderChain defaults are incompatible "
+ "with defaultLiteral. You must provide only one.");
}
this.defaultLiteral = defaultLiteral;
return this;
}

@Override
public ClientConfig build() {
return new ClientConfig(this);
}

private void validateDefaultNotSet() {
if (defaults != null) {
throw new IllegalArgumentException("ConfigDefaults have already been set.");
}
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
public interface ConfigProvider {

/**
* Config defaults may depend on other config (eg a logger that uses log_level).
* The ClientFragment can be used to express these dependencies.
*
* @return the provider rendered into Ruby code
*/
ClientFragment providerFragment();
Expand Down
Loading

0 comments on commit e2b1ae3

Please sign in to comment.