From f5c5592c9f054b416cb4f35c863e6271e94fcfa9 Mon Sep 17 00:00:00 2001 From: Juli Tera <57973151+jterapin@users.noreply.github.com> Date: Wed, 9 Aug 2023 14:47:41 -0400 Subject: [PATCH] Implement config constraints (#155) --- .../lib/high_score_service/config.rb | 2 +- .../rails_json/lib/rails_json/config.rb | 2 +- .../projections/weather/lib/weather/config.rb | 2 +- .../white_label/lib/white_label/config.rb | 3 +- .../white_label/spec/compression_spec.rb | 7 ++- .../integration-specs/compression_spec.rb | 7 ++- .../ruby/codegen/config/ClientConfig.java | 44 ++++++++++------- .../ruby/codegen/config/ConfigConstraint.java | 30 ++++++++++++ .../ruby/codegen/config/RangeConstraint.java | 42 ++++++++++++++++ .../ruby/codegen/config/TypeConstraint.java | 42 ++++++++++++++++ .../codegen/generators/ConfigGenerator.java | 15 ++---- .../integrations/RequestCompression.java | 2 + hearth/lib/hearth/configuration.rb | 2 +- hearth/lib/hearth/validator.rb | 18 ++++++- hearth/spec/hearth/configuration_spec.rb | 2 +- hearth/spec/hearth/validator_spec.rb | 48 +++++++++++++++++++ 16 files changed, 231 insertions(+), 37 deletions(-) create mode 100644 codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/ConfigConstraint.java create mode 100644 codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/RangeConstraint.java create mode 100644 codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/TypeConstraint.java diff --git a/codegen/projections/high_score_service/lib/high_score_service/config.rb b/codegen/projections/high_score_service/lib/high_score_service/config.rb index 172cb510e..679f09037 100644 --- a/codegen/projections/high_score_service/lib/high_score_service/config.rb +++ b/codegen/projections/high_score_service/lib/high_score_service/config.rb @@ -90,7 +90,7 @@ module HighScoreService private - def validate_types! + def validate! Hearth::Validator.validate_types!(disable_host_prefix, TrueClass, FalseClass, context: 'config[:disable_host_prefix]') Hearth::Validator.validate_types!(endpoint, String, context: 'config[:endpoint]') Hearth::Validator.validate_types!(http_client, Hearth::HTTP::Client, context: 'config[:http_client]') diff --git a/codegen/projections/rails_json/lib/rails_json/config.rb b/codegen/projections/rails_json/lib/rails_json/config.rb index 609369a81..70ce3a1ad 100644 --- a/codegen/projections/rails_json/lib/rails_json/config.rb +++ b/codegen/projections/rails_json/lib/rails_json/config.rb @@ -90,7 +90,7 @@ module RailsJson private - def validate_types! + def validate! Hearth::Validator.validate_types!(disable_host_prefix, TrueClass, FalseClass, context: 'config[:disable_host_prefix]') Hearth::Validator.validate_types!(endpoint, String, context: 'config[:endpoint]') Hearth::Validator.validate_types!(http_client, Hearth::HTTP::Client, context: 'config[:http_client]') diff --git a/codegen/projections/weather/lib/weather/config.rb b/codegen/projections/weather/lib/weather/config.rb index 65b5a62d0..c03441e6e 100644 --- a/codegen/projections/weather/lib/weather/config.rb +++ b/codegen/projections/weather/lib/weather/config.rb @@ -90,7 +90,7 @@ module Weather private - def validate_types! + def validate! Hearth::Validator.validate_types!(disable_host_prefix, TrueClass, FalseClass, context: 'config[:disable_host_prefix]') Hearth::Validator.validate_types!(endpoint, String, context: 'config[:endpoint]') Hearth::Validator.validate_types!(http_client, Hearth::HTTP::Client, context: 'config[:http_client]') diff --git a/codegen/projections/white_label/lib/white_label/config.rb b/codegen/projections/white_label/lib/white_label/config.rb index f27c3eda3..bba64ae88 100644 --- a/codegen/projections/white_label/lib/white_label/config.rb +++ b/codegen/projections/white_label/lib/white_label/config.rb @@ -112,7 +112,7 @@ module WhiteLabel private - def validate_types! + def validate! Hearth::Validator.validate_types!(disable_host_prefix, TrueClass, FalseClass, context: 'config[:disable_host_prefix]') Hearth::Validator.validate_types!(disable_request_compression, TrueClass, FalseClass, context: 'config[:disable_request_compression]') Hearth::Validator.validate_types!(endpoint, String, context: 'config[:endpoint]') @@ -122,6 +122,7 @@ def validate_types! Hearth::Validator.validate_types!(logger, Logger, context: 'config[:logger]') Hearth::Validator.validate_types!(plugins, Hearth::PluginList, context: 'config[:plugins]') Hearth::Validator.validate_types!(request_min_compression_size_bytes, Integer, context: 'config[:request_min_compression_size_bytes]') + Hearth::Validator.validate_range!(request_min_compression_size_bytes, min: 0, max: 10485760, context: 'config[:request_min_compression_size_bytes]') Hearth::Validator.validate_types!(retry_strategy, Hearth::Retry::Strategy, context: 'config[:retry_strategy]') Hearth::Validator.validate_types!(stub_responses, TrueClass, FalseClass, context: 'config[:stub_responses]') Hearth::Validator.validate_types!(test_config, String, context: 'config[:test_config]') diff --git a/codegen/projections/white_label/spec/compression_spec.rb b/codegen/projections/white_label/spec/compression_spec.rb index cbbf23504..4cdd38046 100644 --- a/codegen/projections/white_label/spec/compression_spec.rb +++ b/codegen/projections/white_label/spec/compression_spec.rb @@ -17,7 +17,12 @@ module WhiteLabel context 'request_min_compression_size_bytes' do it 'raises error when given invalid integer' do - # TODO: after we implement constraints on configs + expect { Config.new(request_min_compression_size_bytes: -1 ) } + .to raise_error( + ArgumentError, + 'Expected config[:request_min_compression_size_bytes] ' \ + 'to be between 0 to 10485760, got -1.' + ) end end end diff --git a/codegen/smithy-ruby-codegen-test/integration-specs/compression_spec.rb b/codegen/smithy-ruby-codegen-test/integration-specs/compression_spec.rb index cbbf23504..108bb1592 100644 --- a/codegen/smithy-ruby-codegen-test/integration-specs/compression_spec.rb +++ b/codegen/smithy-ruby-codegen-test/integration-specs/compression_spec.rb @@ -17,7 +17,12 @@ module WhiteLabel context 'request_min_compression_size_bytes' do it 'raises error when given invalid integer' do - # TODO: after we implement constraints on configs + expect { Config.new(request_min_compression_size_bytes: -1) } + .to raise_error( + ArgumentError, + 'Expected config[:request_min_compression_size_bytes] ' \ + 'to be between 0 to 10485760, got -1.' + ) end end end diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/ClientConfig.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/ClientConfig.java index ee8654aa8..ffbd77d54 100644 --- a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/ClientConfig.java +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/ClientConfig.java @@ -15,6 +15,8 @@ package software.amazon.smithy.ruby.codegen.config; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; import java.util.Set; import software.amazon.smithy.ruby.codegen.ClientFragment; @@ -29,20 +31,19 @@ public class ClientConfig { private final String name; - private final String type; private final String documentation; private final String documentationType; private final ConfigDefaults defaults; private final boolean allowOperationOverride; + private final List constraints; /** * @param builder builder to construct from. */ public ClientConfig(Builder builder) { this.name = builder.name; - this.type = builder.type; this.documentation = builder.documentation; - this.documentationType = builder.documentationType; + this.documentationType = builder.documentationType != null ? builder.documentationType : builder.type; if (builder.defaults != null) { this.defaults = builder.defaults; } else { @@ -52,6 +53,7 @@ public ClientConfig(Builder builder) { this.defaults.setDocumentationDefault(builder.documentationDefaultValue); } this.allowOperationOverride = builder.allowOperationOverride; + this.constraints = List.copyOf(builder.constraints); } public static Builder builder() { @@ -66,13 +68,6 @@ public String getName() { return name; } - /** - * @return The Ruby type of the config (eg String, Integer, Boolean, ect). - */ - public String getType() { - return type; - } - /** * @return Documentation string to be added to the initialize method. */ @@ -94,10 +89,7 @@ public String getDocumentationDefaultValue() { * @return Documented type */ public String getDocumentationType() { - if (documentationType != null) { - return documentationType; - } - return type; + return documentationType; } /** @@ -109,6 +101,13 @@ public String renderDefaults(GenerationContext context) { return defaults.renderDefault(context); } + /** + * @return a list of ConfigConstraint objects. + */ + public List getConstraints() { + return constraints; + } + /** * If true, this config can be overridden * per operation. @@ -147,12 +146,12 @@ public boolean equals(Object o) { } ClientConfig that = (ClientConfig) o; return Objects.equals(getName(), that.getName()) - && Objects.equals(getType(), that.getType()); + && Objects.equals(getDocumentationType(), that.getDocumentationType()); } @Override public int hashCode() { - return Objects.hash(getName(), getType()); + return Objects.hash(getName(), getDocumentationType()); } /** @@ -166,9 +165,10 @@ public static class Builder implements SmithyBuilder { private String documentationType; private ConfigDefaults defaults; private boolean allowOperationOverride = false; + private final List constraints; protected Builder() { - + constraints = new ArrayList<>(); } /** @@ -186,6 +186,7 @@ public Builder name(String name) { */ public Builder type(String type) { this.type = type; + this.constraints.add(0, new TypeConstraint(type)); return this; } @@ -282,6 +283,15 @@ public Builder defaultLiteral(String defaultLiteral) { return this; } + /** + * @param constraint a ConfigConstraint object. + * @return this builder. + */ + public Builder constraint(ConfigConstraint constraint) { + this.constraints.add(constraint); + return this; + } + @Override public ClientConfig build() { return new ClientConfig(this); diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/ConfigConstraint.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/ConfigConstraint.java new file mode 100644 index 000000000..7d057ec35 --- /dev/null +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/ConfigConstraint.java @@ -0,0 +1,30 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.smithy.ruby.codegen.config; + +/** + * Represents the config value constraints for ClientConfig. + */ + +public interface ConfigConstraint { + /** + * Render the constraint validator for the config value. + * + * @param configName the name of the config. + * @return the string to be rendered into Ruby code. + */ + String render(String configName); +} diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/RangeConstraint.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/RangeConstraint.java new file mode 100644 index 000000000..e4949e8f4 --- /dev/null +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/RangeConstraint.java @@ -0,0 +1,42 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.smithy.ruby.codegen.config; + +/** + * Range Constraint for config value. + */ +public class RangeConstraint implements ConfigConstraint { + + private final long minValue; + private final long maxValue; + + /** + * @param minValue the minimum value + * @param maxValue the maximum value + */ + public RangeConstraint(long minValue, long maxValue) { + this.maxValue = maxValue; + this.minValue = minValue; + } + + @Override + public String render(String configName) { + return String.format( + "Hearth::Validator.validate_range!(%s, min: %d, max: %d, context: 'config[:%s]')", + configName, minValue, maxValue, configName); + } + +} diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/TypeConstraint.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/TypeConstraint.java new file mode 100644 index 000000000..a875e4657 --- /dev/null +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/config/TypeConstraint.java @@ -0,0 +1,42 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file 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 software.amazon.smithy.ruby.codegen.config; + +/** + * Type Constraint for config value. + */ +public class TypeConstraint implements ConfigConstraint { + + private final String type; + + /** + * @param type ruby type for the config. Used for validation, must be a valid Ruby class or Boolean. + */ + public TypeConstraint(String type) { + if (type.equals("Boolean")) { + this.type = "TrueClass, FalseClass"; + } else { + this.type = type; + } + } + + @Override + public String render(String configName) { + return String.format( + "Hearth::Validator.validate_types!(%s, %s, context: 'config[:%s]')", + configName, type, configName); + } +} diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/ConfigGenerator.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/ConfigGenerator.java index 95c685451..e1aa986f1 100644 --- a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/ConfigGenerator.java +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/ConfigGenerator.java @@ -100,7 +100,7 @@ private void renderConfigDocumentation(RubyCodeWriter writer) { writer.writeYardMethod("initialize(*options)", () -> { clientConfigList.forEach((clientConfig) -> { String member = RubyFormatter.asSymbol(RubySymbolProvider.toMemberName(clientConfig.getName())); - String returnType = clientConfig.getType(); + String returnType = clientConfig.getDocumentationType(); String defaultValue = clientConfig.getDocumentationDefaultValue(); String documentation = clientConfig.getDocumentation(); writer.writeYardOption("args", returnType, member, defaultValue, documentation); @@ -115,16 +115,11 @@ private void renderConfigDocumentation(RubyCodeWriter writer) { } private void renderValidateMethod(RubyCodeWriter writer) { - writer.openBlock("def validate_types!"); - clientConfigList.stream().forEach(clientConfig -> { + writer.openBlock("def validate!"); + clientConfigList.forEach(clientConfig -> { String member = RubySymbolProvider.toMemberName(clientConfig.getName()); - String type = clientConfig.getType(); - if (type.equals("Boolean")) { - type = "TrueClass, FalseClass"; - } - writer.write("$3T.validate_types!($1L, $2L, context: 'config[:$1L]')", - member, type, Hearth.VALIDATOR); - // TODO - add constraints here + clientConfig.getConstraints().forEach(c -> + writer.write(c.render(member))); }); writer.closeBlock("end"); } diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/integrations/RequestCompression.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/integrations/RequestCompression.java index 2fc54bca2..14277ccdf 100644 --- a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/integrations/RequestCompression.java +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/integrations/RequestCompression.java @@ -28,6 +28,7 @@ import software.amazon.smithy.ruby.codegen.GenerationContext; import software.amazon.smithy.ruby.codegen.RubyIntegration; import software.amazon.smithy.ruby.codegen.config.ClientConfig; +import software.amazon.smithy.ruby.codegen.config.RangeConstraint; import software.amazon.smithy.ruby.codegen.middleware.Middleware; import software.amazon.smithy.ruby.codegen.middleware.MiddlewareBuilder; import software.amazon.smithy.ruby.codegen.middleware.MiddlewareStackStep; @@ -62,6 +63,7 @@ public void modifyClientMiddleware(MiddlewareBuilder middlewareBuilder, Generati .documentation(minCompressionDocumentation) .allowOperationOverride() .defaultPrimitiveValue("10240") + .constraint(new RangeConstraint(0, 10485760)) .build(); Middleware compression = Middleware.builder() diff --git a/hearth/lib/hearth/configuration.rb b/hearth/lib/hearth/configuration.rb index e6bfd69d1..783913b76 100644 --- a/hearth/lib/hearth/configuration.rb +++ b/hearth/lib/hearth/configuration.rb @@ -6,7 +6,7 @@ module Configuration def initialize(**options) Hearth::Validator.validate_unknown!(self, options, context: 'config') Hearth::Config::Resolver.resolve(self, options, self.class.defaults) - validate_types! + validate! end def dup diff --git a/hearth/lib/hearth/validator.rb b/hearth/lib/hearth/validator.rb index e00dfab6e..75afb9c9c 100755 --- a/hearth/lib/hearth/validator.rb +++ b/hearth/lib/hearth/validator.rb @@ -3,7 +3,7 @@ module Hearth # Utility module for working with request parameters. # - # * Validate structure of parameters against the expected type. + # * Validate structure of parameters against the expected input. # * Raise errors with context when validation fails. # @api private module Validator @@ -13,7 +13,7 @@ module Validator # @param context [String] The context of the value being validated. # @raise [ArgumentError] Raises when the value is not one of given type(s). def self.validate_types!(value, *types, context:) - return if !value || types.any? { |type| value.is_a?(type) } + return if value.nil? || types.any? { |type| value.is_a?(type) } raise ArgumentError, "Expected #{context} to be in " \ @@ -41,5 +41,19 @@ def self.validate_unknown!(struct, params, context:) raise ArgumentError, "Unexpected members: [#{unknown.join(', ')}]" end + + # Validate the given value is within the expected range (inclusive). + # @param value [Object] The value to validate. + # @param min [Numeric] The minimum that the given value should be. + # @param max [Numeric] The maximum that the given value should be. + # @param context [String] The context of the value being validated. + # @raise [ArgumentError] Raises when the value is not within expected range. + def self.validate_range!(value, min:, max:, context:) + return if value.nil? || value.between?(min, max) + + raise ArgumentError, + "Expected #{context} to be between " \ + "#{min} to #{max}, got #{value}." + end end end diff --git a/hearth/spec/hearth/configuration_spec.rb b/hearth/spec/hearth/configuration_spec.rb index f13151cbb..cfc21aac4 100644 --- a/hearth/spec/hearth/configuration_spec.rb +++ b/hearth/spec/hearth/configuration_spec.rb @@ -12,7 +12,7 @@ module Hearth private - def validate_types! + def validate! Hearth::Validator.validate_types!( simple_option, String, context: 'config[:simple_option]' ) diff --git a/hearth/spec/hearth/validator_spec.rb b/hearth/spec/hearth/validator_spec.rb index 95416c708..6b461c200 100644 --- a/hearth/spec/hearth/validator_spec.rb +++ b/hearth/spec/hearth/validator_spec.rb @@ -42,6 +42,19 @@ module Hearth end end + context 'value is a boolean' do + let(:params) { { foo: false } } + + it 'raises an ArgumentError when type is non-boolean' do + expect do + subject.validate_types!(input[:foo], String, context: context) + end.to raise_error( + ArgumentError, + "Expected #{context} to be in [String], got FalseClass." + ) + end + end + context 'value is not set' do let(:params) { {} } @@ -132,5 +145,40 @@ module Hearth end end end + + describe '.validate_range!' do + context 'value is the within the expected range' do + let(:params) { { foo: 10 } } + + it 'does not raise an error' do + expect do + subject.validate_range!( + input[:foo], + min: 0, + max: 10, + context: context + ) + end.to_not raise_error + end + end + + context 'value is outside of the expected range' do + let(:params) { { foo: -1 } } + + it 'raises an ArgumentError' do + expect do + subject.validate_range!( + input[:foo], + min: 0, + max: 10, + context: context + ) + end.to raise_error( + ArgumentError, + "Expected #{context} to be between 0 to 10, got -1." + ) + end + end + end end end