From 3b1efbd05c85206461c251b07b4df8c430eab81e Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Fri, 21 Aug 2026 11:08:03 +0900 Subject: [PATCH 01/12] feat: add OpenAPI schema drift check --- README.md | 26 ++ README_ja.md | 26 ++ docs/openapi-drift-check.md | 96 +++++ lib/structured_params.rb | 6 + .../open_api/configuration.rb | 57 +++ lib/structured_params/open_api/drift_check.rb | 330 ++++++++++++++++++ lib/structured_params/open_api/registry.rb | 63 ++++ lib/tasks/structured_params_open_api.rake | 12 + .../open_api/configuration.rbs | 35 ++ .../open_api/drift_check.rbs | 138 ++++++++ sig/structured_params/open_api/registry.rbs | 33 ++ spec/open_api/configuration_spec.rb | 46 +++ spec/open_api/drift_check_spec.rb | 131 +++++++ spec/open_api/registry_spec.rb | 53 +++ spec/tasks/structured_params_open_api_spec.rb | 29 ++ structured_params.gemspec | 1 + 16 files changed, 1082 insertions(+) create mode 100644 docs/openapi-drift-check.md create mode 100644 lib/structured_params/open_api/configuration.rb create mode 100644 lib/structured_params/open_api/drift_check.rb create mode 100644 lib/structured_params/open_api/registry.rb create mode 100644 lib/tasks/structured_params_open_api.rake create mode 100644 sig/structured_params/open_api/configuration.rbs create mode 100644 sig/structured_params/open_api/drift_check.rbs create mode 100644 sig/structured_params/open_api/registry.rbs create mode 100644 spec/open_api/configuration_spec.rb create mode 100644 spec/open_api/drift_check_spec.rb create mode 100644 spec/open_api/registry_spec.rb create mode 100644 spec/tasks/structured_params_open_api_spec.rb diff --git a/README.md b/README.md index c0b525a..c942f80 100644 --- a/README.md +++ b/README.md @@ -107,11 +107,37 @@ def create end ``` +### 3. OpenAPI Drift Check + +Configure an OpenAPI document once and select the request parameter namespace. +The default convention maps `CreateUserParams` to `#/components/schemas/CreateUser`. + +```ruby +# config/initializers/structured_params.rb +StructuredParams::OpenApi.configure do |config| + config.document = Rails.root.join('openapi.yml').to_s + config.parameter_class_filter = ->(parameter_class) do + parameter_class.name&.start_with?('Api::') + end +end +``` + +Run the check in CI: + +```sh +bundle exec rake structured_params:openapi:drift_check +``` + +See [OpenAPI Drift Check](docs/openapi-drift-check.md) for component-name +overrides, property mappings, and supported schema types. New classes matching +the filter are checked automatically; register only exceptional mappings. + ## Documentation - **[Installation and Setup](docs/installation.md)** - Getting started with StructuredParams - **[Basic Usage](docs/basic-usage.md)** - Parameter classes, nested objects, and arrays - **[Validation](docs/validation.md)** - Using ActiveModel validations with nested structures +- **[OpenAPI Drift Check](docs/openapi-drift-check.md)** - Detect differences between request schemas and Params classes - **[Strong Parameters](docs/strong-parameters.md)** - Automatic permit list generation - **[Error Handling](docs/error-handling.md)** - Flat and structured error formats - **[Serialization](docs/serialization.md)** - Converting parameters to hashes and JSON diff --git a/README_ja.md b/README_ja.md index e3be3ad..d1dd0d3 100644 --- a/README_ja.md +++ b/README_ja.md @@ -107,11 +107,37 @@ def create end ``` +### 3. OpenAPI drift check + +OpenAPI ドキュメントを一度設定し、各リクエスト用パラメータクラスを登録します。 +既定では `CreateUserParams` を `#/components/schemas/CreateUser` に対応付けます。 + +```ruby +# config/initializers/structured_params.rb +StructuredParams::OpenApi.configure do |config| + config.document = Rails.root.join('openapi.yml').to_s + config.parameter_class_filter = ->(parameter_class) do + parameter_class.name&.start_with?('Api::') + end +end +``` + +CI では次を実行します。 + +```sh +bundle exec rake structured_params:openapi:drift_check +``` + +filter に一致する新しい class は自動で検査されます。component 名の例外指定、 +property mapping、対応する schema 型の詳細は +[OpenAPI Drift Check](docs/openapi-drift-check.md) を参照してください。 + ## ドキュメント - **[インストールとセットアップ](docs/installation.md)** - StructuredParams の始め方 - **[基本的な使い方](docs/basic-usage.md)** - パラメータクラス、ネストオブジェクト、配列 - **[バリデーション](docs/validation.md)** - ネスト構造での ActiveModel バリデーション +- **[OpenAPI Drift Check](docs/openapi-drift-check.md)** - OpenAPI schema と Params class の乖離検出 - **[Strong Parameters](docs/strong-parameters.md)** - permit リストの自動生成 - **[エラーハンドリング](docs/error-handling.md)** - フラット形式と構造化形式のエラー - **[シリアライゼーション](docs/serialization.md)** - パラメータのハッシュと JSON 変換 diff --git a/docs/openapi-drift-check.md b/docs/openapi-drift-check.md new file mode 100644 index 0000000..145ee20 --- /dev/null +++ b/docs/openapi-drift-check.md @@ -0,0 +1,96 @@ +# OpenAPI Drift Check + +Configure the application-wide OpenAPI document once and select the namespace of +request parameter classes to check. The Rake task discovers these classes after +Rails eager loads the application. + +```ruby +StructuredParams::OpenApi.configure do |config| + config.document = Rails.root.join('openapi.yml').to_s + config.parameter_class_filter = ->(parameter_class) do + parameter_class.name&.start_with?('Api::') + end +end +``` + +```sh +bundle exec rake structured_params:openapi:drift_check +``` + +By default, `CreateUserParams` is matched with +`#/components/schemas/CreateUser`: the class's namespace and a trailing +`Params`, `Parameter`, `Parameters`, or `Form` are removed. Override the +convention globally when necessary. + +```ruby +StructuredParams::OpenApi.configure do |config| + config.schema_name_resolver = ->(parameter_class) do + "#{parameter_class.name.demodulize.delete_suffix('Params')}Request" + end +end +``` + +For an exceptional class, pass only its component name rather than another +file path. + +```ruby +StructuredParams::OpenApi::DriftCheck.call( + parameter_class: CreateUserParams, + schema: 'CreateUserRequest' +) +``` + +The checker compares schema properties with `attribute` declarations, scalar and +nested object/array types, and schema `required` properties with unconditional +`validates ..., presence: true` declarations. Use `property_names` when the JSON +property name differs from the Ruby attribute name. + +```ruby +StructuredParams::OpenApi::DriftCheck.call( + parameter_class: CreateUserParams, + property_names: { user_id: 'userId' } +) +``` + +In RSpec, use the result as the failure message so a CI failure lists every +detected difference. + +```ruby +it 'matches the request schema' do + result = StructuredParams::OpenApi::DriftCheck.call( + parameter_class: CreateUserParams + ) + + expect(result).to be_valid, result.to_message +end +``` + +`required` means that a JSON key must be present, whereas ActiveModel's `presence` +validator also rejects blank values (including `false`). For APIs where those +semantics are intentionally different, pass `required_validation: :none` and check +required-key behavior separately. + +## Rake task + +The gem provides `structured_params:openapi:drift_check` in Rails applications. +The task automatically checks classes selected by `parameter_class_filter`, so a +new `Api::*Params` class needs no registration. Use the Registry only for an +exceptional component name or property mapping; explicit registrations override +auto-discovery for the same class. + +```ruby +StructuredParams::OpenApi::Registry.register( + LegacyUserParams, + schema: 'LegacyCreateUserRequest' +) +``` + +Run it in CI with: + +```sh +bundle exec rake structured_params:openapi:drift_check +``` + +The current scope deliberately skips `oneOf`, `anyOf`, `allOf`, multiple-type +schemas, and external references. Skips are reported in `result.skipped` so they +remain visible for manual review without being reported as a false type match. diff --git a/lib/structured_params.rb b/lib/structured_params.rb index 622d85b..0683bcf 100644 --- a/lib/structured_params.rb +++ b/lib/structured_params.rb @@ -18,6 +18,12 @@ require_relative 'structured_params/type/object' require_relative 'structured_params/type/array' +# OpenAPI contract checks +require_relative 'structured_params/open_api/configuration' +require_relative 'structured_params/open_api/drift_check' +require_relative 'structured_params/open_api/registry' +load File.expand_path('tasks/structured_params_open_api.rake', __dir__) if defined?(Rake) + # params (load after type definitions) require_relative 'structured_params/params' diff --git a/lib/structured_params/open_api/configuration.rb b/lib/structured_params/open_api/configuration.rb new file mode 100644 index 0000000..3160d09 --- /dev/null +++ b/lib/structured_params/open_api/configuration.rb @@ -0,0 +1,57 @@ +# rbs_inline: enabled +# frozen_string_literal: true + +module StructuredParams + # OpenAPI contract configuration and drift-check utilities. + module OpenApi + # Global OpenAPI document and component-name convention for drift checks. + class Configuration + attr_accessor :document #: Hash[untyped, untyped]|String? + attr_writer :schema_name_resolver #: untyped + attr_writer :parameter_class_filter #: untyped + + #: () -> void + def initialize + @schema_name_resolver = lambda do |parameter_class| + parameter_class.name.split('::').last.sub(/(?:Params?|Parameters?|Form)$/, '') + end + end + + #: () -> Array[singleton(StructuredParams::Params)] + def parameter_classes + return [] unless @parameter_class_filter + + ObjectSpace.each_object(Class).select do |parameter_class| + parameter_class < StructuredParams::Params && @parameter_class_filter.call(parameter_class) + end + end + + #: (singleton(StructuredParams::Params), String?) -> String + def pointer_for(parameter_class, schema_name = nil) + name = schema_name || @schema_name_resolver.call(parameter_class) + return name if name.start_with?('/', '#/') + + "/components/schemas/#{name.gsub('~', '~0').gsub('/', '~1')}" + end + end + + class << self + # @rbs @configuration: Configuration? + + #: () -> Configuration + def configuration + @configuration ||= Configuration.new + end + + #: () { (Configuration) -> void } -> void + def configure + yield configuration + end + + #: () -> void + def reset_configuration! + @configuration = Configuration.new + end + end + end +end diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb new file mode 100644 index 0000000..cff7c6b --- /dev/null +++ b/lib/structured_params/open_api/drift_check.rb @@ -0,0 +1,330 @@ +# rbs_inline: enabled +# frozen_string_literal: true + +require 'yaml' + +module StructuredParams + # Schema comparison types for OpenAPI drift checks. + module OpenApi + # Compares OpenAPI object schemas with StructuredParams declarations. + # rubocop:disable Metrics/ClassLength + class DriftCheck + # @rbs @openapi_source: Hash[untyped, untyped]|String? + # @rbs @schema_source: Hash[untyped, untyped]|String? + # @rbs @parameter_class: singleton(::StructuredParams::Params) + # @rbs @pointer: String? + # @rbs @property_names: Hash[Symbol, String] + # @rbs @required_validation: :presence|:none + # @rbs @document: Hash[untyped, untyped]? + # @rbs @errors: Array[Issue] + # @rbs @skipped: Array[Issue] + # @rbs @seen_pairs: Array[Array[untyped]] + + Issue = Struct.new(:kind, :path, :message, keyword_init: true) + + # Result of a single schema/class comparison. + class Result + attr_reader :errors, :skipped #: Array[Issue] + + #: (Array[Issue], Array[Issue]) -> void + def initialize(errors, skipped) + @errors = errors + @skipped = skipped + end + + #: () -> bool + def valid? + errors.empty? + end + + #: () -> String + def to_message + sections = [] + sections << "OpenAPI drift detected:\n#{format_issues(errors)}" unless valid? + sections << "OpenAPI checks skipped:\n#{format_issues(skipped)}" unless skipped.empty? + sections.join("\n") + end + + private + + #: (Array[Issue]) -> String + def format_issues(issues) + issues.map { |issue| "- #{issue.path}: #{issue.message}" }.join("\n") + end + end + + class << self + #: (**untyped) -> Result + # rubocop:disable Metrics/ParameterLists + def call( + parameter_class:, + openapi: nil, + schema: nil, + pointer: nil, + property_names: {}, + required_validation: :presence + ) + new( + openapi: openapi, + schema: schema, + parameter_class: parameter_class, + pointer: pointer, + property_names: property_names, + required_validation: required_validation + ).call + end + # rubocop:enable Metrics/ParameterLists + end + + #: (**untyped) -> void + # rubocop:disable Metrics/ParameterLists + def initialize( + parameter_class:, + openapi: nil, + schema: nil, + pointer: nil, + property_names: {}, + required_validation: :presence + ) + @openapi_source = openapi || OpenApi.configuration.document + @schema_source = schema + @parameter_class = parameter_class + @pointer = pointer + @property_names = property_names.transform_keys(&:to_sym).transform_values(&:to_s) + @required_validation = required_validation + @errors = [] + @skipped = [] + @seen_pairs = [] #: Array[Array[untyped]] + + unless %i[presence none].include?(required_validation) + raise ArgumentError, "required_validation must be :presence or :none, got #{required_validation.inspect}" + end + + return if @schema_source.is_a?(Hash) || @openapi_source + + raise ArgumentError, 'configure an OpenAPI document or pass schema as a Hash' + end + # rubocop:enable Metrics/ParameterLists + + #: () -> Result + def call + schema, @document = load_schema + check_object(resolve_references(schema), @parameter_class, '') + Result.new(@errors, @skipped) + end + + private + + #: () -> Array[Hash[untyped, untyped]] + def load_schema + if @schema_source.is_a?(Hash) + return [@schema_source, @schema_source] if selected_pointer.nil? + + return [resolve_pointer(@schema_source, selected_pointer), @schema_source] + end + + document = load_document + pointer = selected_pointer || OpenApi.configuration.pointer_for(@parameter_class, @schema_source) + [resolve_pointer(document, pointer), document] + end + + #: () -> Hash[untyped, untyped] + def load_document + return @openapi_source if @openapi_source.is_a?(Hash) + + YAML.safe_load_file(@openapi_source, aliases: true) + end + + #: () -> String? + def selected_pointer + return @pointer unless @pointer.nil? || @pointer.empty? + return unless @schema_source.is_a?(String) + + OpenApi.configuration.pointer_for(@parameter_class, @schema_source) + end + + #: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void + # rubocop:disable Metrics/AbcSize + def check_object(schema, parameter_class, path) + return if skip_union?(schema, path) + return if seen?(schema, parameter_class) + + properties = value_for(schema, 'properties') + unless properties.is_a?(Hash) + skip(path, 'schema has no properties; only object schemas are supported') + return + end + + schema_attributes = properties.keys.to_h { |name| [attribute_name(name), name.to_s] } + attribute_types = parameter_class.attribute_types + compare_attribute_names(schema_attributes, attribute_types, path) + compare_required_attributes(schema, schema_attributes, parameter_class, path) + + (schema_attributes.keys & attribute_types.keys.map(&:to_sym)).each do |attribute| + property_schema = resolve_references(value_for(properties, schema_attributes.fetch(attribute))) + compare_type(property_schema, attribute_types.fetch(attribute.to_s), join_path(path, attribute)) + end + end + # rubocop:enable Metrics/AbcSize + + #: (Hash[Symbol, String], Hash[String, ActiveModel::Type::Value], String) -> void + def compare_attribute_names(schema_attributes, attribute_types, path) + schema_attributes.each do |attribute, property| + next if attribute_types.key?(attribute.to_s) + + error(join_path(path, property), :missing_attribute, 'schema property has no matching attribute declaration') + end + + attribute_types.each_key do |attribute| + next if schema_attributes.key?(attribute.to_sym) + + error(join_path(path, attribute), :unexpected_attribute, + 'attribute declaration has no matching schema property') + end + end + + #: (Hash[untyped, untyped], Hash[Symbol, String], singleton(StructuredParams::Params), String) -> void + def compare_required_attributes(schema, schema_attributes, parameter_class, path) + return if @required_validation == :none + + Array(value_for(schema, 'required')).each do |property| + attribute = attribute_name(property) + next unless schema_attributes.key?(attribute) + next if presence_validator?(parameter_class, attribute) + + error(join_path(path, property), :missing_presence_validation, + 'required schema property has no unconditional presence validation') + end + end + + #: (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void + # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + def compare_type(schema, attribute_type, path) + return if skip_union?(schema, path) + + if value_for(schema, 'type').is_a?(Array) + skip(path, 'multiple schema types are outside the supported drift-check scope') + return + end + + expected_type = schema_type(schema) + actual_type = attribute_type.type + unless compatible_types?(expected_type, actual_type) + error(path, :type_mismatch, + "schema type #{expected_type.inspect} does not match attribute type #{actual_type.inspect}") + return + end + + if expected_type == :object && attribute_type.is_a?(Type::Object) + check_object(schema, attribute_type.value_class, path) + elsif expected_type == :array && attribute_type.is_a?(Type::Array) + compare_array_item_type(schema, attribute_type, path) + end + end + # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + # rubocop:enable Metrics/AbcSize + + #: (Hash[untyped, untyped], Type::Array, String) -> void + def compare_array_item_type(schema, attribute_type, path) + items = value_for(schema, 'items') + unless items.is_a?(Hash) + skip(path, 'array schema has no items declaration') + return + end + + compare_type(resolve_references(items), attribute_type.item_type, "#{path}[]") + end + + #: (Hash[untyped, untyped]) -> Symbol? + def schema_type(schema) + type = value_for(schema, 'type') + return type.to_sym if type + return :object if value_for(schema, 'properties').is_a?(Hash) + + nil + end + + #: (Symbol?, Symbol) -> bool + def compatible_types?(schema_type, attribute_type) + return false if schema_type.nil? + return %i[float decimal].include?(attribute_type) if schema_type == :number + + schema_type == attribute_type + end + + #: (singleton(StructuredParams::Params), Symbol) -> bool + def presence_validator?(parameter_class, attribute) + parameter_class._validators.fetch(attribute.to_sym, []).any? do |validator| + validator.kind == :presence && validator.options.slice(:if, :unless).empty? + end + end + + #: (Hash[untyped, untyped], String) -> bool + def skip_union?(schema, path) + union_key = %w[oneOf anyOf allOf].find { |key| value_for(schema, key) } + return false unless union_key + + skip(path, "#{union_key} is outside the supported drift-check scope") + true + end + + #: (Hash[untyped, untyped]) -> Hash[untyped, untyped] + def resolve_references(schema) + reference = value_for(schema, '$ref') + return schema unless reference + + unless reference.start_with?('#/') + skip('', "external reference #{reference.inspect} is outside the supported drift-check scope") + return schema + end + + referenced_schema = resolve_pointer(@document, reference.delete_prefix('#')) + overrides = schema.reject { |key, _| key.to_s == '$ref' } + resolve_references(referenced_schema.merge(overrides)) + end + + #: (Hash[untyped, untyped], String) -> Hash[untyped, untyped] + def resolve_pointer(document, pointer) + pointer.delete_prefix('#').delete_prefix('/').split('/').reduce(document) do |value, segment| + value.fetch(segment.gsub('~1', '/').gsub('~0', '~')) + end + end + + #: (Hash[untyped, untyped], String) -> untyped + def value_for(hash, key) + hash[key] || hash[key.to_sym] + end + + #: (untyped) -> Symbol + def attribute_name(property) + @property_names.invert.fetch(property.to_s, property).to_sym + end + + #: (Hash[untyped, untyped], singleton(StructuredParams::Params)) -> bool + def seen?(schema, parameter_class) + pair = [schema.object_id, parameter_class] + return true if @seen_pairs.include?(pair) + + @seen_pairs << pair + false + end + + #: (String, String) -> String + def join_path(path, name) + path.empty? ? name.to_s : "#{path}.#{name}" + end + + #: (String, Symbol, String) -> void + def error(path, kind, message) + @errors << Issue.new(kind: kind, path: path, message: message) + end + + #: (String, String) -> void + def skip(path, message) + @skipped << Issue.new(kind: :unsupported, path: path.empty? ? '(root)' : path, message: message) + end + end + # rubocop:enable Metrics/ClassLength + end +end diff --git a/lib/structured_params/open_api/registry.rb b/lib/structured_params/open_api/registry.rb new file mode 100644 index 0000000..efdb195 --- /dev/null +++ b/lib/structured_params/open_api/registry.rb @@ -0,0 +1,63 @@ +# rbs_inline: enabled +# frozen_string_literal: true + +module StructuredParams + # OpenAPI contract registration and bulk execution utilities. + module OpenApi + # Raised by Registry.check_all! when one or more registered contracts drift. + class DriftError < StandardError; end + + # Collects exceptional mappings and runs all configured drift checks. + class Registry + class << self + # @rbs @entries: Array[Array[untyped]]? + + #: (singleton(StructuredParams::Params), **untyped) -> void + def register(parameter_class, schema: nil, pointer: nil, property_names: {}, required_validation: :presence) + entries << [ + parameter_class, + { + schema: schema, + pointer: pointer, + property_names: property_names, + required_validation: required_validation + } + ] + end + + #: (?auto_discover: bool) -> Array[DriftCheck::Result] + def check_all(auto_discover: false) + checks = auto_discover ? auto_discovered_checks : {} + entries.each { |parameter_class, options| checks[parameter_class] = options } + checks.map { |parameter_class, options| DriftCheck.call(parameter_class: parameter_class, **options) } + end + + #: (?auto_discover: bool) -> Array[DriftCheck::Result] + def check_all!(auto_discover: false) + results = check_all(auto_discover: auto_discover) + errors = results.reject(&:valid?).map(&:to_message) + raise DriftError, errors.join("\n\n") unless errors.empty? + + results + end + + #: () -> void + def clear + @entries = [] + end + + private + + #: () -> Hash[singleton(StructuredParams::Params), Hash[untyped, untyped]] + def auto_discovered_checks + OpenApi.configuration.parameter_classes.to_h { |parameter_class| [parameter_class, {}] } + end + + #: () -> Array[Array[untyped]] + def entries + @entries ||= [] + end + end + end + end +end diff --git a/lib/tasks/structured_params_open_api.rake b/lib/tasks/structured_params_open_api.rake new file mode 100644 index 0000000..453fdc8 --- /dev/null +++ b/lib/tasks/structured_params_open_api.rake @@ -0,0 +1,12 @@ +# rbs_inline: enabled +# frozen_string_literal: true + +namespace :structured_params do + namespace :openapi do + desc 'Check registered and auto-discovered StructuredParams classes against the OpenAPI document' + task drift_check: :environment do + Rails.application.eager_load! if defined?(Rails) && Rails.respond_to?(:application) + StructuredParams::OpenApi::Registry.check_all!(auto_discover: true) + end + end +end diff --git a/sig/structured_params/open_api/configuration.rbs b/sig/structured_params/open_api/configuration.rbs new file mode 100644 index 0000000..bfef6d3 --- /dev/null +++ b/sig/structured_params/open_api/configuration.rbs @@ -0,0 +1,35 @@ +# Generated from lib/structured_params/open_api/configuration.rb with RBS::Inline + +module StructuredParams + # OpenAPI contract configuration and drift-check utilities. + module OpenApi + # Global OpenAPI document and component-name convention for drift checks. + class Configuration + attr_accessor document: Hash[untyped, untyped] | String? + + attr_writer schema_name_resolver: untyped + + attr_writer parameter_class_filter: untyped + + # : () -> void + def initialize: () -> void + + # : () -> Array[singleton(StructuredParams::Params)] + def parameter_classes: () -> Array[singleton(StructuredParams::Params)] + + # : (singleton(StructuredParams::Params), String?) -> String + def pointer_for: (singleton(StructuredParams::Params), String?) -> String + end + + @configuration: Configuration? + + # : () -> Configuration + def self.configuration: () -> Configuration + + # : () { (Configuration) -> void } -> void + def self.configure: () { (Configuration) -> void } -> void + + # : () -> void + def self.reset_configuration!: () -> void + end +end diff --git a/sig/structured_params/open_api/drift_check.rbs b/sig/structured_params/open_api/drift_check.rbs new file mode 100644 index 0000000..9097a4b --- /dev/null +++ b/sig/structured_params/open_api/drift_check.rbs @@ -0,0 +1,138 @@ +# Generated from lib/structured_params/open_api/drift_check.rb with RBS::Inline + +module StructuredParams + # Schema comparison types for OpenAPI drift checks. + module OpenApi + # Compares OpenAPI object schemas with StructuredParams declarations. + # rubocop:disable Metrics/ClassLength + class DriftCheck + @seen_pairs: Array[Array[untyped]] + + @skipped: Array[Issue] + + @errors: Array[Issue] + + @document: Hash[untyped, untyped]? + + @required_validation: :presence | :none + + @property_names: Hash[Symbol, String] + + @pointer: String? + + @parameter_class: singleton(::StructuredParams::Params) + + @schema_source: Hash[untyped, untyped] | String? + + @openapi_source: Hash[untyped, untyped] | String? + + class Issue < Struct[untyped] + attr_accessor kind(): untyped + + attr_accessor path(): untyped + + attr_accessor message(): untyped + + def self.new: (?kind: untyped, ?path: untyped, ?message: untyped) -> instance + | ({ ?kind: untyped, ?path: untyped, ?message: untyped }) -> instance + end + + # Result of a single schema/class comparison. + class Result + attr_reader errors: Array[Issue] + + attr_reader skipped: Array[Issue] + + # : (Array[Issue], Array[Issue]) -> void + def initialize: (Array[Issue], Array[Issue]) -> void + + # : () -> bool + def valid?: () -> bool + + # : () -> String + def to_message: () -> String + + private + + # : (Array[Issue]) -> String + def format_issues: (Array[Issue]) -> String + end + + # : (**untyped) -> Result + # rubocop:disable Metrics/ParameterLists + def self.call: (**untyped) -> Result + + # : (**untyped) -> void + # rubocop:disable Metrics/ParameterLists + def initialize: (**untyped) -> void + + # : () -> Result + def call: () -> Result + + private + + # : () -> Array[Hash[untyped, untyped]] + def load_schema: () -> Array[Hash[untyped, untyped]] + + # : () -> Hash[untyped, untyped] + def load_document: () -> Hash[untyped, untyped] + + # : () -> String? + def selected_pointer: () -> String? + + # : (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void + # rubocop:disable Metrics/AbcSize + def check_object: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void + + # : (Hash[Symbol, String], Hash[String, ActiveModel::Type::Value], String) -> void + def compare_attribute_names: (Hash[Symbol, String], Hash[String, ActiveModel::Type::Value], String) -> void + + # : (Hash[untyped, untyped], Hash[Symbol, String], singleton(StructuredParams::Params), String) -> void + def compare_required_attributes: (Hash[untyped, untyped], Hash[Symbol, String], singleton(StructuredParams::Params), String) -> void + + # : (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void + # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + def compare_type: (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void + + # : (Hash[untyped, untyped], Type::Array, String) -> void + def compare_array_item_type: (Hash[untyped, untyped], Type::Array, String) -> void + + # : (Hash[untyped, untyped]) -> Symbol? + def schema_type: (Hash[untyped, untyped]) -> Symbol? + + # : (Symbol?, Symbol) -> bool + def compatible_types?: (Symbol?, Symbol) -> bool + + # : (singleton(StructuredParams::Params), Symbol) -> bool + def presence_validator?: (singleton(StructuredParams::Params), Symbol) -> bool + + # : (Hash[untyped, untyped], String) -> bool + def skip_union?: (Hash[untyped, untyped], String) -> bool + + # : (Hash[untyped, untyped]) -> Hash[untyped, untyped] + def resolve_references: (Hash[untyped, untyped]) -> Hash[untyped, untyped] + + # : (Hash[untyped, untyped], String) -> Hash[untyped, untyped] + def resolve_pointer: (Hash[untyped, untyped], String) -> Hash[untyped, untyped] + + # : (Hash[untyped, untyped], String) -> untyped + def value_for: (Hash[untyped, untyped], String) -> untyped + + # : (untyped) -> Symbol + def attribute_name: (untyped) -> Symbol + + # : (Hash[untyped, untyped], singleton(StructuredParams::Params)) -> bool + def seen?: (Hash[untyped, untyped], singleton(StructuredParams::Params)) -> bool + + # : (String, String) -> String + def join_path: (String, String) -> String + + # : (String, Symbol, String) -> void + def error: (String, Symbol, String) -> void + + # : (String, String) -> void + def skip: (String, String) -> void + end + end +end diff --git a/sig/structured_params/open_api/registry.rbs b/sig/structured_params/open_api/registry.rbs new file mode 100644 index 0000000..093142b --- /dev/null +++ b/sig/structured_params/open_api/registry.rbs @@ -0,0 +1,33 @@ +# Generated from lib/structured_params/open_api/registry.rb with RBS::Inline + +module StructuredParams + # OpenAPI contract registration and bulk execution utilities. + module OpenApi + # Raised by Registry.check_all! when one or more registered contracts drift. + class DriftError < StandardError + end + + # Collects exceptional mappings and runs all configured drift checks. + class Registry + @entries: Array[Array[untyped]]? + + # : (singleton(StructuredParams::Params), **untyped) -> void + def self.register: (singleton(StructuredParams::Params), **untyped) -> void + + # : (?auto_discover: bool) -> Array[DriftCheck::Result] + def self.check_all: (?auto_discover: bool) -> Array[DriftCheck::Result] + + # : (?auto_discover: bool) -> Array[DriftCheck::Result] + def self.check_all!: (?auto_discover: bool) -> Array[DriftCheck::Result] + + # : () -> void + def self.clear: () -> void + + # : () -> Hash[singleton(StructuredParams::Params), Hash[untyped, untyped]] + private def self.auto_discovered_checks: () -> Hash[singleton(StructuredParams::Params), Hash[untyped, untyped]] + + # : () -> Array[Array[untyped]] + private def self.entries: () -> Array[Array[untyped]] + end + end +end diff --git a/spec/open_api/configuration_spec.rb b/spec/open_api/configuration_spec.rb new file mode 100644 index 0000000..4fc249e --- /dev/null +++ b/spec/open_api/configuration_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe StructuredParams::OpenApi::Configuration do + after { StructuredParams::OpenApi.reset_configuration! } + + let(:parameter_class) do + stub_const('OpenApiRequestParams', Class.new(StructuredParams::Params)).tap do |klass| + klass.attribute :name, :string + klass.validates :name, presence: true + end + end + + let(:document) do + { + 'components' => { + 'schemas' => { + 'OpenApiRequest' => { + 'type' => 'object', + 'properties' => { 'name' => { 'type' => 'string' } }, + 'required' => ['name'] + } + } + } + } + end + + it 'uses the configured document and default class-name convention' do + StructuredParams::OpenApi.configure { |config| config.document = document } + + result = StructuredParams::OpenApi::DriftCheck.call(parameter_class: parameter_class) + + expect(result).to be_valid + end + + it 'discovers only classes selected by the configured filter' do + stub_const('Api', Module.new) + parameter_class = stub_const('Api::CreateUserParams', Class.new(StructuredParams::Params)) + StructuredParams::OpenApi.configure do |config| + config.parameter_class_filter = ->(klass) { klass.name&.start_with?('Api::') } + end + + expect(StructuredParams::OpenApi.configuration.parameter_classes).to include(parameter_class) + end +end diff --git a/spec/open_api/drift_check_spec.rb b/spec/open_api/drift_check_spec.rb new file mode 100644 index 0000000..fc89376 --- /dev/null +++ b/spec/open_api/drift_check_spec.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'tempfile' + +RSpec.describe StructuredParams::OpenApi::DriftCheck do + describe '.call' do + let(:schema) do + { + 'type' => 'object', + 'properties' => { + 'name' => { 'type' => 'string' }, + 'age' => { 'type' => 'integer' }, + 'enabled' => { 'type' => 'boolean' }, + 'tags' => { 'type' => 'array', 'items' => { 'type' => 'string' } }, + 'address' => { + 'type' => 'object', + 'properties' => { 'city' => { 'type' => 'string' } }, + 'required' => ['city'] + } + }, + 'required' => %w[name age] + } + end + + let(:address_class) do + Class.new(StructuredParams::Params) do + attribute :city, :string + validates :city, presence: true + end + end + let(:parameter_class) do + address = address_class + Class.new(StructuredParams::Params) do + attribute :name, :string + attribute :age, :integer + attribute :enabled, :boolean + attribute :tags, :array, value_type: :string + attribute :address, :object, value_class: address + validates :name, :age, presence: true + end + end + + it 'accepts matching attributes, types, nested objects, and required validations' do + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result).to be_valid + expect(result.skipped).to be_empty + end + + it 'reports missing and unexpected attributes, types, and required validations' do + parameter_class.attribute :extra, :string + parameter_class.attribute :age, :string + parameter_class._validators[:age].clear + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result.errors.map(&:kind)).to contain_exactly( + :unexpected_attribute, + :type_mismatch, + :missing_presence_validation + ) + expect(result.to_message).to include('extra', 'schema type :integer does not match attribute type :string') + end + + it 'maps differently named schema properties to attributes' do + schema['properties']['userId'] = schema['properties'].delete('name') + schema['required'][0] = 'userId' + + result = described_class.call( + schema: schema, + parameter_class: parameter_class, + property_names: { name: 'userId' } + ) + + expect(result).to be_valid + end + + it 'reports union schemas as skipped instead of treating them as compatible' do + schema['properties']['age'] = { 'oneOf' => [{ 'type' => 'integer' }, { 'type' => 'string' }] } + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result).to be_valid + expect(result.skipped.first.message).to include('oneOf') + end + + it 'allows required validation checking to be disabled' do + schema['required'] << 'enabled' + + result = described_class.call( + schema: schema, + parameter_class: parameter_class, + required_validation: :none + ) + + expect(result).to be_valid + end + + it 'loads a YAML schema selected by JSON Pointer and resolves local references' do + address_schema = schema['properties'].delete('address') + schema['properties']['address'] = { '$ref' => '#/components/schemas/Address' } + document = { 'components' => { 'schemas' => { 'Request' => schema, 'Address' => address_schema } } } + + Tempfile.create(['openapi', '.yml']) do |file| + file.write(document.to_yaml) + file.flush + + result = described_class.call( + openapi: file.path, + schema: 'Request', + parameter_class: parameter_class + ) + + expect(result).to be_valid + end + end + + it 'selects a schema from a hash with a JSON Pointer' do + document = { 'components' => { 'schemas' => { 'Request' => schema } } } + + result = described_class.call( + openapi: document, + schema: 'Request', + parameter_class: parameter_class + ) + + expect(result).to be_valid + end + end +end diff --git a/spec/open_api/registry_spec.rb b/spec/open_api/registry_spec.rb new file mode 100644 index 0000000..0baa550 --- /dev/null +++ b/spec/open_api/registry_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe StructuredParams::OpenApi::Registry do + after do + described_class.clear + StructuredParams::OpenApi.reset_configuration! + end + + it 'runs registered checks and raises when any of them drift' do + parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } + schema = { 'type' => 'object', 'properties' => { 'age' => { 'type' => 'integer' } } } + described_class.register(parameter_class, schema: schema) + + expect { described_class.check_all! }.to raise_error(StructuredParams::OpenApi::DriftError, /drift detected/) + end + + context 'with an auto-discovery filter' do + let(:parameter_class) do + stub_const('Api', Module.new) + stub_const('Api::CreateUserParams', Class.new(StructuredParams::Params)).tap do |klass| + klass.attribute :name, :string + klass.validates :name, presence: true + end + end + + before do + document = { + 'components' => { + 'schemas' => { + 'CreateUser' => { + 'type' => 'object', + 'properties' => { 'name' => { 'type' => 'string' } }, + 'required' => ['name'] + } + } + } + } + StructuredParams::OpenApi.configure do |config| + config.document = document + config.parameter_class_filter = ->(klass) { klass.equal?(parameter_class) } + end + end + + it 'auto-discovers parameter classes selected by the configured filter' do + results = described_class.check_all(auto_discover: true) + + expect(results).to all(be_valid) + expect(results).to have_attributes(size: 1) + end + end +end diff --git a/spec/tasks/structured_params_open_api_spec.rb b/spec/tasks/structured_params_open_api_spec.rb new file mode 100644 index 0000000..37532ad --- /dev/null +++ b/spec/tasks/structured_params_open_api_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rake' + +RSpec.describe Rake::Task do + subject(:drift_check_task) { described_class['structured_params:openapi:drift_check'] } + + around do |example| + original_application = Rake.application + Rake.application = Rake::Application.new + load File.expand_path('../../lib/tasks/structured_params_open_api.rake', __dir__) + example.run + ensure + Rake.application = original_application + end + + after { StructuredParams::OpenApi::Registry.clear } + + it 'runs all registered drift checks after the Rails environment task' do + described_class.define_task(:environment) + parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } + schema = { 'type' => 'object', 'properties' => { 'age' => { 'type' => 'integer' } } } + StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) + + expect { drift_check_task.invoke } + .to raise_error(StructuredParams::OpenApi::DriftError, /drift detected/) + end +end diff --git a/structured_params.gemspec b/structured_params.gemspec index 0489492..85465c9 100644 --- a/structured_params.gemspec +++ b/structured_params.gemspec @@ -23,6 +23,7 @@ Gem::Specification.new do |spec| 'LICENSE.txt', 'CHANGELOG.md', 'lib/**/*.rb', + 'lib/tasks/**/*.rake', 'sig/**/*.rbs' ] From 4e0bfca28a993a53ecc9a63576bedf0e636a2ec6 Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Fri, 21 Aug 2026 11:09:17 +0900 Subject: [PATCH 02/12] chore: manage Ruby version with mise --- .mise.toml | 2 ++ .ruby-version | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 .mise.toml delete mode 100644 .ruby-version diff --git a/.mise.toml b/.mise.toml new file mode 100644 index 0000000..f3d14c8 --- /dev/null +++ b/.mise.toml @@ -0,0 +1,2 @@ +[tools] +ruby = "4.0.5" diff --git a/.ruby-version b/.ruby-version deleted file mode 100644 index 7636e75..0000000 --- a/.ruby-version +++ /dev/null @@ -1 +0,0 @@ -4.0.5 From dd854f4e6f9d14afcb538a6f110480cf2d1e7026 Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Fri, 21 Aug 2026 22:02:27 +0900 Subject: [PATCH 03/12] fix: harden OpenAPI drift check for real-world use in a Rails host app The drift check feature (added in 3b1efbd) crashed any Rails boot (rails runner/console) when Thor::Actions partially loaded rake internals without mixing Rake::DSL into main. Loading the rake task now happens through a Rails::Railtie#rake_tasks hook instead of an unconditional require-time load. Also, dogfooding the feature against a real app's OpenAPI document and StructuredParams classes surfaced several rough edges, fixed here: - DriftCheck::Result#to_message is now prefixed with the checked class's name, so Registry.check_all!'s combined DriftError message reads as one block per class instead of an unattributed flat list of paths. - A single-entry `allOf` (the common TypeSpec/OpenAPI idiom for attaching a description/default next to a $ref, since $ref forbids sibling keys) is unwrapped and compared instead of being skipped as unsupported composition. - A schema pointer that can't be resolved (e.g. a resolver-guessed name with no matching component) is now recorded as a skip instead of raising KeyError and aborting every other class's check. - Registry.register accepts a class-name String, resolved lazily in check_all, so registering from a plain Rails initializer no longer needs a to_prepare workaround for Zeitwerk autoloading timing. - Configuration#ignore_drift(path:, kind:) suppresses one specific finding (by path and, optionally, kind) across every check, for cases where the Ruby side is correct but the check's structural heuristics can't see it (e.g. presence enforced only through conditional validators). Ignored findings move to Result#ignored rather than disappearing, so they stay auditable. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018ypnVJKdZdSTiJP8NDXgc8 --- docs/openapi-drift-check.md | 40 +++++++++++++ lib/structured_params.rb | 16 +++++- .../open_api/configuration.rb | 20 +++++++ lib/structured_params/open_api/drift_check.rb | 46 ++++++++++++--- lib/structured_params/open_api/registry.rb | 15 ++++- sig/structured_params.rbs | 8 +++ .../open_api/configuration.rbs | 15 +++++ .../open_api/drift_check.rbs | 18 +++++- sig/structured_params/open_api/registry.rbs | 9 ++- spec/open_api/configuration_spec.rb | 21 +++++++ spec/open_api/drift_check_spec.rb | 56 +++++++++++++++++++ spec/open_api/railtie_loading_spec.rb | 23 ++++++++ spec/open_api/registry_spec.rb | 13 +++++ 13 files changed, 285 insertions(+), 15 deletions(-) create mode 100644 spec/open_api/railtie_loading_spec.rb diff --git a/docs/openapi-drift-check.md b/docs/openapi-drift-check.md index 145ee20..eb17c58 100644 --- a/docs/openapi-drift-check.md +++ b/docs/openapi-drift-check.md @@ -70,6 +70,25 @@ validator also rejects blank values (including `false`). For APIs where those semantics are intentionally different, pass `required_validation: :none` and check required-key behavior separately. +## Ignoring a specific finding + +`required_validation: :none` and the Registry both operate on a whole class, so they +aren't precise enough to accept one known finding while still checking everything else +at that path. For that, configure an ignore rule instead: + +```ruby +StructuredParams::OpenApi.configure do |config| + config.ignore_drift(path: 'wallet_information.wallet_type', kind: :missing_presence_validation) +end +``` + +This applies wherever that path occurs, including once per parent class for a nested +object reused by several top-level parameter classes. Omit `kind:` to ignore every +finding at that path. Ignored findings move to `result.ignored` instead of +`result.errors`, so `valid?` treats them as resolved and they don't appear in +`to_message` — but they stay available on the result object for anyone who wants to +audit what's being suppressed and why, rather than disappearing silently. + ## Rake task The gem provides `structured_params:openapi:drift_check` in Rails applications. @@ -85,6 +104,18 @@ StructuredParams::OpenApi::Registry.register( ) ``` +`parameter_class` may also be a class name String, which is resolved lazily when +the task runs. This is useful for registering from a plain `config/initializers/*.rb` +file, where Rails hasn't finished setting up autoloading yet and referencing the +class constant directly would raise `NameError`. + +```ruby +StructuredParams::OpenApi::Registry.register( + 'LegacyUserParams', + schema: 'LegacyCreateUserRequest' +) +``` + Run it in CI with: ```sh @@ -94,3 +125,12 @@ bundle exec rake structured_params:openapi:drift_check The current scope deliberately skips `oneOf`, `anyOf`, `allOf`, multiple-type schemas, and external references. Skips are reported in `result.skipped` so they remain visible for manual review without being reported as a false type match. + +A class whose resolved schema name has no matching component is also skipped +rather than raising, so one misconfigured or intentionally out-of-scope class +(for example, one backed by query parameters instead of a requestBody schema) +doesn't stop the whole task from reporting every other class's drift. + +`result.to_message` is prefixed with the checked class's name, so `check_all!`'s +combined `DriftError` message reads as one block per class instead of a flat, +unattributed list of paths. diff --git a/lib/structured_params.rb b/lib/structured_params.rb index 0683bcf..173f430 100644 --- a/lib/structured_params.rb +++ b/lib/structured_params.rb @@ -22,7 +22,6 @@ require_relative 'structured_params/open_api/configuration' require_relative 'structured_params/open_api/drift_check' require_relative 'structured_params/open_api/registry' -load File.expand_path('tasks/structured_params_open_api.rake', __dir__) if defined?(Rake) # params (load after type definitions) require_relative 'structured_params/params' @@ -100,4 +99,19 @@ def register_types_as(object_name:, array_name:) ActiveModel::Type.register(array_name, StructuredParams::Type::Array) end end + + RAKE_TASK_PATH = File.expand_path('tasks/structured_params_open_api.rake', __dir__) #: String + + if defined?(Rails::Railtie) + # Defers loading the OpenAPI drift-check rake task until Rails actually + # loads rake tasks, so requiring this gem never depends on the Rake DSL + # being mixed into +main+ (e.g. `rails runner`, `rails console`, app boot). + class Railtie < Rails::Railtie + rake_tasks do + load StructuredParams::RAKE_TASK_PATH + end + end + elsif defined?(Rake::DSL) + load RAKE_TASK_PATH + end end diff --git a/lib/structured_params/open_api/configuration.rb b/lib/structured_params/open_api/configuration.rb index 3160d09..116fa5d 100644 --- a/lib/structured_params/open_api/configuration.rb +++ b/lib/structured_params/open_api/configuration.rb @@ -6,6 +6,8 @@ module StructuredParams module OpenApi # Global OpenAPI document and component-name convention for drift checks. class Configuration + # @rbs @ignored_issues: Array[Hash[Symbol, untyped]] + attr_accessor :document #: Hash[untyped, untyped]|String? attr_writer :schema_name_resolver #: untyped attr_writer :parameter_class_filter #: untyped @@ -15,6 +17,24 @@ def initialize @schema_name_resolver = lambda do |parameter_class| parameter_class.name.split('::').last.sub(/(?:Params?|Parameters?|Form)$/, '') end + @ignored_issues = [] + end + + # Suppresses a specific finding (matched by +path+ and, optionally, +kind+) across + # every drift check, instead of raising it as an error. Applies wherever that path + # occurs, including once per parent class for a nested object reused by several + # top-level parameter classes. Omit +kind+ to ignore every kind at that path. + # + # Ignored findings are moved to DriftCheck::Result#ignored rather than dropped, so + # they stay auditable; #valid? and #to_message treat them as resolved. + #: (path: String, ?kind: Symbol?) -> void + def ignore_drift(path:, kind: nil) + @ignored_issues << { path: path, kind: kind } + end + + #: (Symbol, String) -> bool + def ignored?(kind, path) + @ignored_issues.any? { |rule| rule.fetch(:path) == path && [nil, kind].include?(rule.fetch(:kind)) } end #: () -> Array[singleton(StructuredParams::Params)] diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb index cff7c6b..42aafa8 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/lib/structured_params/open_api/drift_check.rb @@ -18,18 +18,23 @@ class DriftCheck # @rbs @document: Hash[untyped, untyped]? # @rbs @errors: Array[Issue] # @rbs @skipped: Array[Issue] + # @rbs @ignored: Array[Issue] # @rbs @seen_pairs: Array[Array[untyped]] Issue = Struct.new(:kind, :path, :message, keyword_init: true) # Result of a single schema/class comparison. class Result - attr_reader :errors, :skipped #: Array[Issue] + # @rbs @parameter_class: singleton(::StructuredParams::Params) - #: (Array[Issue], Array[Issue]) -> void - def initialize(errors, skipped) + attr_reader :errors, :skipped, :ignored #: Array[Issue] + + #: (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void + def initialize(parameter_class, errors, skipped, ignored) + @parameter_class = parameter_class @errors = errors @skipped = skipped + @ignored = ignored end #: () -> bool @@ -42,7 +47,9 @@ def to_message sections = [] sections << "OpenAPI drift detected:\n#{format_issues(errors)}" unless valid? sections << "OpenAPI checks skipped:\n#{format_issues(skipped)}" unless skipped.empty? - sections.join("\n") + return '' if sections.empty? + + "#{@parameter_class.name || @parameter_class.inspect}\n#{sections.join("\n")}" end private @@ -94,6 +101,7 @@ def initialize( @required_validation = required_validation @errors = [] @skipped = [] + @ignored = [] @seen_pairs = [] #: Array[Array[untyped]] unless %i[presence none].include?(required_validation) @@ -108,9 +116,16 @@ def initialize( #: () -> Result def call - schema, @document = load_schema + begin + schema, @document = load_schema + rescue KeyError => e + skip('', "no matching OpenAPI schema found (#{e.message}); " \ + 'check the schema_name_resolver convention or register an explicit schema name') + return Result.new(@parameter_class, @errors, @skipped, @ignored) + end + check_object(resolve_references(schema), @parameter_class, '') - Result.new(@errors, @skipped) + Result.new(@parameter_class, @errors, @skipped, @ignored) end private @@ -271,6 +286,7 @@ def skip_union?(schema, path) #: (Hash[untyped, untyped]) -> Hash[untyped, untyped] def resolve_references(schema) + schema = unwrap_single_all_of(schema) reference = value_for(schema, '$ref') return schema unless reference @@ -284,6 +300,20 @@ def resolve_references(schema) resolve_references(referenced_schema.merge(overrides)) end + # OpenAPI/JSON Schema forbid sibling keys next to `$ref`, so generators such as TypeSpec + # commonly wrap a single `$ref` in `allOf` to attach a `description`, `default`, etc. + # That's equivalent to the ref carrying those siblings directly, so unwrap and merge it + # instead of treating it as unsupported schema composition. `allOf` with more than one + # entry is genuine composition and stays out of scope (reported via skip_union?). + #: (Hash[untyped, untyped]) -> Hash[untyped, untyped] + def unwrap_single_all_of(schema) + all_of = value_for(schema, 'allOf') + return schema unless all_of.is_a?(Array) && all_of.size == 1 + + overrides = schema.reject { |key, _| key.to_s == 'allOf' } + all_of.first.merge(overrides) + end + #: (Hash[untyped, untyped], String) -> Hash[untyped, untyped] def resolve_pointer(document, pointer) pointer.delete_prefix('#').delete_prefix('/').split('/').reduce(document) do |value, segment| @@ -317,7 +347,9 @@ def join_path(path, name) #: (String, Symbol, String) -> void def error(path, kind, message) - @errors << Issue.new(kind: kind, path: path, message: message) + issue = Issue.new(kind: kind, path: path, message: message) + target = OpenApi.configuration.ignored?(kind, path) ? @ignored : @errors + target << issue end #: (String, String) -> void diff --git a/lib/structured_params/open_api/registry.rb b/lib/structured_params/open_api/registry.rb index efdb195..1dd7bf6 100644 --- a/lib/structured_params/open_api/registry.rb +++ b/lib/structured_params/open_api/registry.rb @@ -12,7 +12,9 @@ class Registry class << self # @rbs @entries: Array[Array[untyped]]? - #: (singleton(StructuredParams::Params), **untyped) -> void + # +parameter_class+ may be a class name String (e.g. from a Rails initializer, + # before Zeitwerk autoloading is set up); it is resolved lazily in +check_all+. + #: (singleton(StructuredParams::Params) | String, **untyped) -> void def register(parameter_class, schema: nil, pointer: nil, property_names: {}, required_validation: :presence) entries << [ parameter_class, @@ -28,7 +30,7 @@ def register(parameter_class, schema: nil, pointer: nil, property_names: {}, req #: (?auto_discover: bool) -> Array[DriftCheck::Result] def check_all(auto_discover: false) checks = auto_discover ? auto_discovered_checks : {} - entries.each { |parameter_class, options| checks[parameter_class] = options } + entries.each { |parameter_class, options| checks[resolve(parameter_class)] = options } checks.map { |parameter_class, options| DriftCheck.call(parameter_class: parameter_class, **options) } end @@ -36,7 +38,9 @@ def check_all(auto_discover: false) def check_all!(auto_discover: false) results = check_all(auto_discover: auto_discover) errors = results.reject(&:valid?).map(&:to_message) - raise DriftError, errors.join("\n\n") unless errors.empty? + # Leading blank line so the first class's message doesn't get glued onto the + # "ClassName: " prefix Ruby/Rake print ahead of an exception's message. + raise DriftError, "\n\n#{errors.join("\n\n")}" unless errors.empty? results end @@ -57,6 +61,11 @@ def auto_discovered_checks def entries @entries ||= [] end + + #: (singleton(StructuredParams::Params) | String) -> singleton(StructuredParams::Params) + def resolve(parameter_class) + parameter_class.is_a?(String) ? Object.const_get(parameter_class) : parameter_class + end end end end diff --git a/sig/structured_params.rbs b/sig/structured_params.rbs index 1a9debe..9d0110a 100644 --- a/sig/structured_params.rbs +++ b/sig/structured_params.rbs @@ -50,4 +50,12 @@ module StructuredParams # Helper method to register types with custom names # : (object_name: Symbol, array_name: Symbol) -> void def self.register_types_as: (object_name: Symbol, array_name: Symbol) -> void + + RAKE_TASK_PATH: String + + # Defers loading the OpenAPI drift-check rake task until Rails actually + # loads rake tasks, so requiring this gem never depends on the Rake DSL + # being mixed into +main+ (e.g. `rails runner`, `rails console`, app boot). + class Railtie < Rails::Railtie + end end diff --git a/sig/structured_params/open_api/configuration.rbs b/sig/structured_params/open_api/configuration.rbs index bfef6d3..c3a2989 100644 --- a/sig/structured_params/open_api/configuration.rbs +++ b/sig/structured_params/open_api/configuration.rbs @@ -5,6 +5,8 @@ module StructuredParams module OpenApi # Global OpenAPI document and component-name convention for drift checks. class Configuration + @ignored_issues: Array[Hash[Symbol, untyped]] + attr_accessor document: Hash[untyped, untyped] | String? attr_writer schema_name_resolver: untyped @@ -14,6 +16,19 @@ module StructuredParams # : () -> void def initialize: () -> void + # Suppresses a specific finding (matched by +path+ and, optionally, +kind+) across + # every drift check, instead of raising it as an error. Applies wherever that path + # occurs, including once per parent class for a nested object reused by several + # top-level parameter classes. Omit +kind+ to ignore every kind at that path. + # + # Ignored findings are moved to DriftCheck::Result#ignored rather than dropped, so + # they stay auditable; #valid? and #to_message treat them as resolved. + # : (path: String, ?kind: Symbol?) -> void + def ignore_drift: (path: String, ?kind: Symbol?) -> void + + # : (Symbol, String) -> bool + def ignored?: (Symbol, String) -> bool + # : () -> Array[singleton(StructuredParams::Params)] def parameter_classes: () -> Array[singleton(StructuredParams::Params)] diff --git a/sig/structured_params/open_api/drift_check.rbs b/sig/structured_params/open_api/drift_check.rbs index 9097a4b..c2d60d6 100644 --- a/sig/structured_params/open_api/drift_check.rbs +++ b/sig/structured_params/open_api/drift_check.rbs @@ -8,6 +8,8 @@ module StructuredParams class DriftCheck @seen_pairs: Array[Array[untyped]] + @ignored: Array[Issue] + @skipped: Array[Issue] @errors: Array[Issue] @@ -39,12 +41,16 @@ module StructuredParams # Result of a single schema/class comparison. class Result + @parameter_class: singleton(::StructuredParams::Params) + attr_reader errors: Array[Issue] attr_reader skipped: Array[Issue] - # : (Array[Issue], Array[Issue]) -> void - def initialize: (Array[Issue], Array[Issue]) -> void + attr_reader ignored: Array[Issue] + + # : (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void + def initialize: (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void # : () -> bool def valid?: () -> bool @@ -113,6 +119,14 @@ module StructuredParams # : (Hash[untyped, untyped]) -> Hash[untyped, untyped] def resolve_references: (Hash[untyped, untyped]) -> Hash[untyped, untyped] + # OpenAPI/JSON Schema forbid sibling keys next to `$ref`, so generators such as TypeSpec + # commonly wrap a single `$ref` in `allOf` to attach a `description`, `default`, etc. + # That's equivalent to the ref carrying those siblings directly, so unwrap and merge it + # instead of treating it as unsupported schema composition. `allOf` with more than one + # entry is genuine composition and stays out of scope (reported via skip_union?). + # : (Hash[untyped, untyped]) -> Hash[untyped, untyped] + def unwrap_single_all_of: (Hash[untyped, untyped]) -> Hash[untyped, untyped] + # : (Hash[untyped, untyped], String) -> Hash[untyped, untyped] def resolve_pointer: (Hash[untyped, untyped], String) -> Hash[untyped, untyped] diff --git a/sig/structured_params/open_api/registry.rbs b/sig/structured_params/open_api/registry.rbs index 093142b..ea5da78 100644 --- a/sig/structured_params/open_api/registry.rbs +++ b/sig/structured_params/open_api/registry.rbs @@ -11,8 +11,10 @@ module StructuredParams class Registry @entries: Array[Array[untyped]]? - # : (singleton(StructuredParams::Params), **untyped) -> void - def self.register: (singleton(StructuredParams::Params), **untyped) -> void + # +parameter_class+ may be a class name String (e.g. from a Rails initializer, + # before Zeitwerk autoloading is set up); it is resolved lazily in +check_all+. + # : (singleton(StructuredParams::Params) | String, **untyped) -> void + def self.register: (singleton(StructuredParams::Params) | String, **untyped) -> void # : (?auto_discover: bool) -> Array[DriftCheck::Result] def self.check_all: (?auto_discover: bool) -> Array[DriftCheck::Result] @@ -28,6 +30,9 @@ module StructuredParams # : () -> Array[Array[untyped]] private def self.entries: () -> Array[Array[untyped]] + + # : (singleton(StructuredParams::Params) | String) -> singleton(StructuredParams::Params) + private def self.resolve: (singleton(StructuredParams::Params) | String) -> singleton(StructuredParams::Params) end end end diff --git a/spec/open_api/configuration_spec.rb b/spec/open_api/configuration_spec.rb index 4fc249e..200f293 100644 --- a/spec/open_api/configuration_spec.rb +++ b/spec/open_api/configuration_spec.rb @@ -43,4 +43,25 @@ expect(StructuredParams::OpenApi.configuration.parameter_classes).to include(parameter_class) end + + describe '#ignore_drift and #ignored?' do + it 'matches only the given kind at the given path when kind is specified' do + StructuredParams::OpenApi.configure do |config| + config.ignore_drift(path: 'wallet_information.wallet_type', kind: :missing_presence_validation) + end + + expect(StructuredParams::OpenApi.configuration.ignored?(:missing_presence_validation, + 'wallet_information.wallet_type')).to be true + expect(StructuredParams::OpenApi.configuration.ignored?(:type_mismatch, + 'wallet_information.wallet_type')).to be false + expect(StructuredParams::OpenApi.configuration.ignored?(:missing_presence_validation, 'other_path')).to be false + end + + it 'matches every kind at the path when kind is omitted' do + StructuredParams::OpenApi.configure { |config| config.ignore_drift(path: 'legacy_field') } + + expect(StructuredParams::OpenApi.configuration.ignored?(:type_mismatch, 'legacy_field')).to be true + expect(StructuredParams::OpenApi.configuration.ignored?(:missing_attribute, 'legacy_field')).to be true + end + end end diff --git a/spec/open_api/drift_check_spec.rb b/spec/open_api/drift_check_spec.rb index fc89376..1998050 100644 --- a/spec/open_api/drift_check_spec.rb +++ b/spec/open_api/drift_check_spec.rb @@ -63,6 +63,23 @@ expect(result.to_message).to include('extra', 'schema type :integer does not match attribute type :string') end + it 'moves a finding matching a configured Configuration#ignore rule to result.ignored' do + parameter_class._validators[:age].clear + StructuredParams::OpenApi.configure do |config| + config.ignore_drift(path: 'age', kind: :missing_presence_validation) + end + + begin + result = described_class.call(schema: schema, parameter_class: parameter_class) + ensure + StructuredParams::OpenApi.reset_configuration! + end + + expect(result).to be_valid + expect(result.errors).to be_empty + expect(result.ignored.map(&:path)).to contain_exactly('age') + end + it 'maps differently named schema properties to attributes' do schema['properties']['userId'] = schema['properties'].delete('name') schema['required'][0] = 'userId' @@ -85,6 +102,27 @@ expect(result.skipped.first.message).to include('oneOf') end + it 'unwraps a single-entry allOf instead of treating it as unsupported composition' do + # Generators such as TypeSpec commonly wrap a single $ref in allOf because + # OpenAPI/JSON Schema forbid sibling keys (e.g. description) directly next to $ref. + schema['properties']['age'] = { 'allOf' => [{ 'type' => 'string' }], 'description' => 'wrapped' } + parameter_class.attribute :age, :string + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result).to be_valid + expect(result.skipped).to be_empty + end + + it 'still reports multi-entry allOf as skipped composition' do + schema['properties']['age'] = { 'allOf' => [{ 'type' => 'integer' }, { 'minimum' => 0 }] } + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result).to be_valid + expect(result.skipped.first.message).to include('allOf') + end + it 'allows required validation checking to be disabled' do schema['required'] << 'enabled' @@ -127,5 +165,23 @@ expect(result).to be_valid end + + it 'skips instead of raising when no schema matches the requested name' do + document = { 'components' => { 'schemas' => { 'Other' => schema } } } + + result = described_class.call(openapi: document, schema: 'Missing', parameter_class: parameter_class) + + expect(result).to be_valid + expect(result.skipped.first.message).to include('no matching OpenAPI schema found') + end + + it 'prefixes the message with the parameter class name' do + stub_const('DriftCheckSampleParameter', Class.new(StructuredParams::Params) { attribute :name, :string }) + minimal_schema = { 'type' => 'object', 'properties' => { 'other' => { 'type' => 'string' } } } + + result = described_class.call(schema: minimal_schema, parameter_class: DriftCheckSampleParameter) + + expect(result.to_message).to start_with('DriftCheckSampleParameter') + end end end diff --git a/spec/open_api/railtie_loading_spec.rb b/spec/open_api/railtie_loading_spec.rb new file mode 100644 index 0000000..7bba69e --- /dev/null +++ b/spec/open_api/railtie_loading_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'open3' + +# rubocop:disable RSpec/DescribeClass +RSpec.describe 'loading the OpenAPI drift-check rake task' do + it 'does not raise when Rake is defined without its DSL mixed into main' do + # Thor::Actions (used by Rails::Command, e.g. `rails runner`/`rails console`) + # requires 'rake/file_utils_ext' directly. That defines the top-level + # `Rake` constant without loading rake/dsl_definition.rb, so `namespace` + # and `task` are never mixed into `main`. Reproduce that exact condition + # instead of a full `require 'rake'`, which mixes the DSL in as a side effect. + lib_path = File.expand_path('../../lib', __dir__) + script = "require 'rake/file_utils_ext'; require 'structured_params'; puts 'ok'" + + output, status = Open3.capture2e('bundle', 'exec', 'ruby', '-I', lib_path, '-e', script) + + expect(status).to be_success, output + expect(output).to include('ok') + end +end +# rubocop:enable RSpec/DescribeClass diff --git a/spec/open_api/registry_spec.rb b/spec/open_api/registry_spec.rb index 0baa550..63404a4 100644 --- a/spec/open_api/registry_spec.rb +++ b/spec/open_api/registry_spec.rb @@ -50,4 +50,17 @@ expect(results).to have_attributes(size: 1) end end + + it 'resolves a class-name String registered before autoloading has run' do + stub_const('Api', Module.new) + stub_const('Api::CreateUserParams', Class.new(StructuredParams::Params)).tap do |klass| + klass.attribute :name, :string + end + schema = { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string' } } } + described_class.register('Api::CreateUserParams', schema: schema) + + results = described_class.check_all + + expect(results).to all(be_valid) + end end From 8af6e826503110b439f14d69a2e3dcb9452c280c Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 09:15:21 +0900 Subject: [PATCH 04/12] fix: satisfy current RuboCop checks --- lib/structured_params/errors.rb | 4 ++-- .../open_api/configuration.rb | 2 +- lib/structured_params/open_api/drift_check.rb | 24 +++++++++---------- lib/structured_params/open_api/registry.rb | 2 +- lib/structured_params/params.rb | 3 +-- sig/structured_params/errors.rbs | 3 ++- .../open_api/configuration.rbs | 2 +- .../open_api/drift_check.rbs | 23 +++++++++--------- sig/structured_params/open_api/registry.rbs | 2 +- sig/structured_params/params.rbs | 2 +- spec/errors_spec.rb | 3 +-- spec/form_object_spec.rb | 3 +-- spec/open_api/railtie_loading_spec.rb | 3 +-- spec/permit_spec.rb | 12 ++++------ 14 files changed, 39 insertions(+), 49 deletions(-) diff --git a/lib/structured_params/errors.rb b/lib/structured_params/errors.rb index cac5360..1284e66 100644 --- a/lib/structured_params/errors.rb +++ b/lib/structured_params/errors.rb @@ -1,7 +1,6 @@ # rbs_inline: enabled # frozen_string_literal: true -# rubocop:disable Style/OptionalBooleanParameter module StructuredParams # Custom errors collection that handles nested attribute names class Errors < ActiveModel::Errors @@ -9,6 +8,7 @@ class Errors < ActiveModel::Errors # Add structured option to get nested structure for dot-notation attributes #: (?bool, ?structured: false) -> Hash[Symbol, String] #: (?bool, structured: bool) -> Hash[Symbol, untyped] + # rubocop:disable-next Style/OptionalBooleanParameter def to_hash(full_messages = false, structured: false) if structured attribute_messages_hash = build_attribute_messages_hash(full_messages) @@ -43,6 +43,7 @@ def messages(structured: false) # Build a hash with attribute names as keys and their error messages as values # This is used for to_hash(structured: true) #: (bool) -> Hash[Symbol, Array[String]] + # rubocop:disable-next Style/OptionalBooleanParameter def build_attribute_messages_hash(full_messages = false) message_method = full_messages ? :full_message : :message @@ -65,4 +66,3 @@ def build_nested_hash(target_hash, flat_hash, separator = '.') end end end -# rubocop:enable Style/OptionalBooleanParameter diff --git a/lib/structured_params/open_api/configuration.rb b/lib/structured_params/open_api/configuration.rb index 116fa5d..04ed99e 100644 --- a/lib/structured_params/open_api/configuration.rb +++ b/lib/structured_params/open_api/configuration.rb @@ -56,7 +56,7 @@ def pointer_for(parameter_class, schema_name = nil) end class << self - # @rbs @configuration: Configuration? + # @rbs self.@configuration: Configuration? #: () -> Configuration def configuration diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb index 42aafa8..b1c4f60 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/lib/structured_params/open_api/drift_check.rb @@ -7,7 +7,7 @@ module StructuredParams # Schema comparison types for OpenAPI drift checks. module OpenApi # Compares OpenAPI object schemas with StructuredParams declarations. - # rubocop:disable Metrics/ClassLength + # rubocop:disable-next Metrics/ClassLength class DriftCheck # @rbs @openapi_source: Hash[untyped, untyped]|String? # @rbs @schema_source: Hash[untyped, untyped]|String? @@ -21,7 +21,12 @@ class DriftCheck # @rbs @ignored: Array[Issue] # @rbs @seen_pairs: Array[Array[untyped]] - Issue = Struct.new(:kind, :path, :message, keyword_init: true) + Issue = Struct.new( + :kind, #: Symbol + :path, #: String + :message, #: String + keyword_init: true + ) # Result of a single schema/class comparison. class Result @@ -62,7 +67,7 @@ def format_issues(issues) class << self #: (**untyped) -> Result - # rubocop:disable Metrics/ParameterLists + # rubocop:disable-next Metrics/ParameterLists def call( parameter_class:, openapi: nil, @@ -80,11 +85,10 @@ def call( required_validation: required_validation ).call end - # rubocop:enable Metrics/ParameterLists end #: (**untyped) -> void - # rubocop:disable Metrics/ParameterLists + # rubocop:disable-next Metrics/ParameterLists def initialize( parameter_class:, openapi: nil, @@ -112,7 +116,6 @@ def initialize( raise ArgumentError, 'configure an OpenAPI document or pass schema as a Hash' end - # rubocop:enable Metrics/ParameterLists #: () -> Result def call @@ -159,7 +162,7 @@ def selected_pointer end #: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void - # rubocop:disable Metrics/AbcSize + # rubocop:disable-next Metrics/AbcSize def check_object(schema, parameter_class, path) return if skip_union?(schema, path) return if seen?(schema, parameter_class) @@ -180,7 +183,6 @@ def check_object(schema, parameter_class, path) compare_type(property_schema, attribute_types.fetch(attribute.to_s), join_path(path, attribute)) end end - # rubocop:enable Metrics/AbcSize #: (Hash[Symbol, String], Hash[String, ActiveModel::Type::Value], String) -> void def compare_attribute_names(schema_attributes, attribute_types, path) @@ -213,8 +215,7 @@ def compare_required_attributes(schema, schema_attributes, parameter_class, path end #: (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void - # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + # rubocop:disable-next Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity def compare_type(schema, attribute_type, path) return if skip_union?(schema, path) @@ -237,8 +238,6 @@ def compare_type(schema, attribute_type, path) compare_array_item_type(schema, attribute_type, path) end end - # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity - # rubocop:enable Metrics/AbcSize #: (Hash[untyped, untyped], Type::Array, String) -> void def compare_array_item_type(schema, attribute_type, path) @@ -357,6 +356,5 @@ def skip(path, message) @skipped << Issue.new(kind: :unsupported, path: path.empty? ? '(root)' : path, message: message) end end - # rubocop:enable Metrics/ClassLength end end diff --git a/lib/structured_params/open_api/registry.rb b/lib/structured_params/open_api/registry.rb index 1dd7bf6..e48bf01 100644 --- a/lib/structured_params/open_api/registry.rb +++ b/lib/structured_params/open_api/registry.rb @@ -10,7 +10,7 @@ class DriftError < StandardError; end # Collects exceptional mappings and runs all configured drift checks. class Registry class << self - # @rbs @entries: Array[Array[untyped]]? + # @rbs self.@entries: Array[Array[untyped]]? # +parameter_class+ may be a class name String (e.g. from a Rails initializer, # before Zeitwerk autoloading is set up); it is resolved lazily in +check_all+. diff --git a/lib/structured_params/params.rb b/lib/structured_params/params.rb index 02ab255..73465ab 100644 --- a/lib/structured_params/params.rb +++ b/lib/structured_params/params.rb @@ -50,7 +50,7 @@ module StructuredParams # <%= f.text_field :name %> # <%= f.text_field :email %> # <% end %> - # rubocop:disable Metrics/ClassLength + # rubocop:disable-next Metrics/ClassLength class Params include ActiveModel::Model include ActiveModel::Attributes @@ -347,5 +347,4 @@ def import_structured_errors(structured_errors, prefix) end end end - # rubocop:enable Metrics/ClassLength end diff --git a/sig/structured_params/errors.rbs b/sig/structured_params/errors.rbs index b415827..747b309 100644 --- a/sig/structured_params/errors.rbs +++ b/sig/structured_params/errors.rbs @@ -1,6 +1,5 @@ # Generated from lib/structured_params/errors.rb with RBS::Inline -# rubocop:disable Style/OptionalBooleanParameter module StructuredParams # Custom errors collection that handles nested attribute names class Errors < ActiveModel::Errors @@ -8,6 +7,7 @@ module StructuredParams # Add structured option to get nested structure for dot-notation attributes # : (?bool, ?structured: false) -> Hash[Symbol, String] # : (?bool, structured: bool) -> Hash[Symbol, untyped] + # rubocop:disable-next Style/OptionalBooleanParameter def to_hash: (?bool, ?structured: false) -> Hash[Symbol, String] | (?bool, structured: bool) -> Hash[Symbol, untyped] @@ -26,6 +26,7 @@ module StructuredParams # Build a hash with attribute names as keys and their error messages as values # This is used for to_hash(structured: true) # : (bool) -> Hash[Symbol, Array[String]] + # rubocop:disable-next Style/OptionalBooleanParameter def build_attribute_messages_hash: (bool) -> Hash[Symbol, Array[String]] # Build a nested hash structure from flat dot-notation keys diff --git a/sig/structured_params/open_api/configuration.rbs b/sig/structured_params/open_api/configuration.rbs index c3a2989..5412a82 100644 --- a/sig/structured_params/open_api/configuration.rbs +++ b/sig/structured_params/open_api/configuration.rbs @@ -36,7 +36,7 @@ module StructuredParams def pointer_for: (singleton(StructuredParams::Params), String?) -> String end - @configuration: Configuration? + self.@configuration: Configuration? # : () -> Configuration def self.configuration: () -> Configuration diff --git a/sig/structured_params/open_api/drift_check.rbs b/sig/structured_params/open_api/drift_check.rbs index c2d60d6..2829f62 100644 --- a/sig/structured_params/open_api/drift_check.rbs +++ b/sig/structured_params/open_api/drift_check.rbs @@ -4,7 +4,7 @@ module StructuredParams # Schema comparison types for OpenAPI drift checks. module OpenApi # Compares OpenAPI object schemas with StructuredParams declarations. - # rubocop:disable Metrics/ClassLength + # rubocop:disable-next Metrics/ClassLength class DriftCheck @seen_pairs: Array[Array[untyped]] @@ -28,15 +28,15 @@ module StructuredParams @openapi_source: Hash[untyped, untyped] | String? - class Issue < Struct[untyped] - attr_accessor kind(): untyped + class Issue < Struct[Symbol | String] + attr_accessor kind(): Symbol - attr_accessor path(): untyped + attr_accessor path(): String - attr_accessor message(): untyped + attr_accessor message(): String - def self.new: (?kind: untyped, ?path: untyped, ?message: untyped) -> instance - | ({ ?kind: untyped, ?path: untyped, ?message: untyped }) -> instance + def self.new: (?kind: Symbol, ?path: String, ?message: String) -> instance + | ({ ?kind: Symbol, ?path: String, ?message: String }) -> instance end # Result of a single schema/class comparison. @@ -65,11 +65,11 @@ module StructuredParams end # : (**untyped) -> Result - # rubocop:disable Metrics/ParameterLists + # rubocop:disable-next Metrics/ParameterLists def self.call: (**untyped) -> Result # : (**untyped) -> void - # rubocop:disable Metrics/ParameterLists + # rubocop:disable-next Metrics/ParameterLists def initialize: (**untyped) -> void # : () -> Result @@ -87,7 +87,7 @@ module StructuredParams def selected_pointer: () -> String? # : (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void - # rubocop:disable Metrics/AbcSize + # rubocop:disable-next Metrics/AbcSize def check_object: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void # : (Hash[Symbol, String], Hash[String, ActiveModel::Type::Value], String) -> void @@ -97,8 +97,7 @@ module StructuredParams def compare_required_attributes: (Hash[untyped, untyped], Hash[Symbol, String], singleton(StructuredParams::Params), String) -> void # : (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void - # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity + # rubocop:disable-next Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity def compare_type: (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void # : (Hash[untyped, untyped], Type::Array, String) -> void diff --git a/sig/structured_params/open_api/registry.rbs b/sig/structured_params/open_api/registry.rbs index ea5da78..640df0f 100644 --- a/sig/structured_params/open_api/registry.rbs +++ b/sig/structured_params/open_api/registry.rbs @@ -9,7 +9,7 @@ module StructuredParams # Collects exceptional mappings and runs all configured drift checks. class Registry - @entries: Array[Array[untyped]]? + self.@entries: Array[Array[untyped]]? # +parameter_class+ may be a class name String (e.g. from a Rails initializer, # before Zeitwerk autoloading is set up); it is resolved lazily in +check_all+. diff --git a/sig/structured_params/params.rbs b/sig/structured_params/params.rbs index 55aa1e7..42a93a3 100644 --- a/sig/structured_params/params.rbs +++ b/sig/structured_params/params.rbs @@ -49,7 +49,7 @@ module StructuredParams # <%= f.text_field :name %> # <%= f.text_field :email %> # <% end %> - # rubocop:disable Metrics/ClassLength + # rubocop:disable-next Metrics/ClassLength class Params include ActiveModel::Model diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index c0fba93..6ffa985 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -106,7 +106,7 @@ errors.add('items.1.subitems.0.description', 'is too long') end - # rubocop:disable RSpec/ExampleLength + # rubocop:disable-next RSpec/ExampleLength it 'creates deep nested structure' do expect(errors.to_hash(false, structured: true)).to eq({ items: { @@ -127,7 +127,6 @@ } }) end - # rubocop:enable RSpec/ExampleLength end context 'with mixed flat and nested attributes' do diff --git a/spec/form_object_spec.rb b/spec/form_object_spec.rb index 4d318c7..608320f 100644 --- a/spec/form_object_spec.rb +++ b/spec/form_object_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -# rubocop:disable RSpec/DescribeClass +# rubocop:disable-next RSpec/DescribeClass RSpec.describe 'StructuredParams::Params as Form Object' do describe '.form_class?' do it 'returns true for classes with a Form suffix' do @@ -258,4 +258,3 @@ end end end -# rubocop:enable RSpec/DescribeClass diff --git a/spec/open_api/railtie_loading_spec.rb b/spec/open_api/railtie_loading_spec.rb index 7bba69e..3e8ac61 100644 --- a/spec/open_api/railtie_loading_spec.rb +++ b/spec/open_api/railtie_loading_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require 'open3' -# rubocop:disable RSpec/DescribeClass +# rubocop:disable-next RSpec/DescribeClass RSpec.describe 'loading the OpenAPI drift-check rake task' do it 'does not raise when Rake is defined without its DSL mixed into main' do # Thor::Actions (used by Rails::Command, e.g. `rails runner`/`rails console`) @@ -20,4 +20,3 @@ expect(output).to include('ok') end end -# rubocop:enable RSpec/DescribeClass diff --git a/spec/permit_spec.rb b/spec/permit_spec.rb index 9cc314b..4bb3de6 100644 --- a/spec/permit_spec.rb +++ b/spec/permit_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -# rubocop:disable RSpec/DescribeClass +# rubocop:disable-next RSpec/DescribeClass RSpec.describe 'StructuredParams::Params.permit' do describe 'Form Object context (with nested params structure)' do let(:params) do @@ -79,7 +79,7 @@ ) end - # rubocop:disable RSpec/MultipleExpectations + # rubocop:disable-next RSpec/MultipleExpectations it 'permits array parameters' do permitted = UserParameter.permit(params) @@ -90,7 +90,6 @@ expect(permitted[:hobbies][0][:extra]).to be_nil expect(permitted[:tags]).to eq(%w[tag1 tag2 tag3]) end - # rubocop:enable RSpec/MultipleExpectations end context 'with namespaced form' do @@ -150,7 +149,7 @@ ) end - # rubocop:disable RSpec/MultipleExpectations + # rubocop:disable-next RSpec/MultipleExpectations it 'permits nested object parameters without require' do permitted = UserParameter.permit(params, require: false) @@ -161,7 +160,6 @@ expect(permitted[:address][:extra]).to be_nil expect(permitted[:extra_field]).to be_nil end - # rubocop:enable RSpec/MultipleExpectations end context 'with arrays' do @@ -179,7 +177,7 @@ ) end - # rubocop:disable RSpec/MultipleExpectations + # rubocop:disable-next RSpec/MultipleExpectations it 'permits array parameters without require' do permitted = UserParameter.permit(params, require: false) @@ -191,7 +189,6 @@ expect(permitted[:tags]).to eq(%w[tag1 tag2 tag3]) expect(permitted[:extra_field]).to be_nil end - # rubocop:enable RSpec/MultipleExpectations end context 'when user manually extracts nested params' do @@ -217,4 +214,3 @@ end end end -# rubocop:enable RSpec/DescribeClass From 6fc5d9da365752c0eb0ca50330f4926b88083f7b Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 09:17:09 +0900 Subject: [PATCH 05/12] fix: align RBS struct annotations --- lib/structured_params/open_api/drift_check.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb index b1c4f60..178dea3 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/lib/structured_params/open_api/drift_check.rb @@ -22,8 +22,8 @@ class DriftCheck # @rbs @seen_pairs: Array[Array[untyped]] Issue = Struct.new( - :kind, #: Symbol - :path, #: String + :kind, #: Symbol + :path, #: String :message, #: String keyword_init: true ) From b8e346e2db0ade61389ee5441478b38edfffc55d Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 09:18:57 +0900 Subject: [PATCH 06/12] fix: align RBS struct annotations --- lib/structured_params/open_api/drift_check.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb index 178dea3..536e79d 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/lib/structured_params/open_api/drift_check.rb @@ -22,9 +22,9 @@ class DriftCheck # @rbs @seen_pairs: Array[Array[untyped]] Issue = Struct.new( - :kind, #: Symbol - :path, #: String - :message, #: String + :kind, #: Symbol + :path, #: String + :message, #: String keyword_init: true ) From 9ce8b6fe37acc0d78c7f009811d2b52b5425d3df Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 10:18:41 +0900 Subject: [PATCH 07/12] fix: address review findings for OpenAPI drift check - restore block-style rubocop directives (disable-next is invalid) - report missing schemas and unresolved/circular \$refs as errors - print skipped/ignored checks in the rake task; add strict mode - guard against self-referencing schemas, array pointers, nil schemas - accept date/time strings; cache parsed YAML and permit Date/Time - scope ignore_drift to a class and validate its kind - add specs and update docs Co-Authored-By: Claude Sonnet 5.5 --- .rubocop_rbs.yml | 2 + README_ja.md | 2 +- docs/openapi-drift-check.md | 68 ++++-- lib/structured_params/errors.rb | 4 +- .../open_api/configuration.rb | 63 ++++- lib/structured_params/open_api/drift_check.rb | 206 +++++++++++++---- lib/structured_params/open_api/registry.rb | 21 +- lib/structured_params/params.rb | 3 +- lib/tasks/structured_params_open_api.rake | 7 +- sig/structured_params/errors.rbs | 3 +- .../open_api/configuration.rbs | 34 ++- .../open_api/drift_check.rbs | 81 +++++-- sig/structured_params/open_api/registry.rbs | 9 +- sig/structured_params/params.rbs | 2 +- spec/errors_spec.rb | 3 +- spec/form_object_spec.rb | 3 +- spec/open_api/configuration_spec.rb | 88 +++++++ spec/open_api/drift_check_spec.rb | 217 +++++++++++++++++- spec/open_api/railtie_loading_spec.rb | 3 +- spec/open_api/registry_spec.rb | 60 +++++ spec/permit_spec.rb | 12 +- spec/tasks/structured_params_open_api_spec.rb | 29 +++ 22 files changed, 792 insertions(+), 128 deletions(-) diff --git a/.rubocop_rbs.yml b/.rubocop_rbs.yml index 30f1222..c5ec528 100644 --- a/.rubocop_rbs.yml +++ b/.rubocop_rbs.yml @@ -13,6 +13,8 @@ Style/RbsInline/UntypedInstanceVariable: Exclude: - 'lib/structured_params/params.rb' - 'lib/structured_params.rb' + - 'lib/structured_params/open_api/configuration.rb' + - 'lib/structured_params/open_api/registry.rb' Style/RbsInline/RequireRbsInlineComment: Exclude: diff --git a/README_ja.md b/README_ja.md index d1dd0d3..1d7ef64 100644 --- a/README_ja.md +++ b/README_ja.md @@ -109,7 +109,7 @@ end ### 3. OpenAPI drift check -OpenAPI ドキュメントを一度設定し、各リクエスト用パラメータクラスを登録します。 +OpenAPI ドキュメントを一度設定し、filter でリクエスト用パラメータクラスの namespace を選択します。 既定では `CreateUserParams` を `#/components/schemas/CreateUser` に対応付けます。 ```ruby diff --git a/docs/openapi-drift-check.md b/docs/openapi-drift-check.md index eb17c58..8bb3152 100644 --- a/docs/openapi-drift-check.md +++ b/docs/openapi-drift-check.md @@ -19,7 +19,7 @@ bundle exec rake structured_params:openapi:drift_check By default, `CreateUserParams` is matched with `#/components/schemas/CreateUser`: the class's namespace and a trailing -`Params`, `Parameter`, `Parameters`, or `Form` are removed. Override the +`Param`, `Params`, `Parameter`, `Parameters`, or `Form` are removed. Override the convention globally when necessary. ```ruby @@ -30,8 +30,10 @@ StructuredParams::OpenApi.configure do |config| end ``` -For an exceptional class, pass only its component name rather than another -file path. +For an exceptional class, pass its component name as `schema:`. It also accepts a +JSON Pointer (`'/components/schemas/CreateUserRequest'`, `'#/…'`) or an inline Hash. +`pointer:` selects a pointer explicitly, and `openapi:` overrides the configured +document for one call (a file path or a Hash). ```ruby StructuredParams::OpenApi::DriftCheck.call( @@ -40,9 +42,22 @@ StructuredParams::OpenApi::DriftCheck.call( ) ``` +Scalar types are compared as follows: + +| Schema `type` | Accepted attribute types | +|---------------|--------------------------| +| `string` | `:string`, `:date`, `:datetime`, `:time` (`format` is not inspected) | +| `integer` | `:integer`, `:big_integer` | +| `number` | `:float`, `:decimal` | +| `boolean`, `object`, `array` | the same type | + +A schema with neither `type` nor `properties` (for example an enum-only schema) is +reported as a `type_mismatch`. + The checker compares schema properties with `attribute` declarations, scalar and nested object/array types, and schema `required` properties with unconditional -`validates ..., presence: true` declarations. Use `property_names` when the JSON +`validates ..., presence: true` declarations ("unconditional" means no `if:` or +`unless:` option; `on:` and `allow_nil` are not considered). Use `property_names` when the JSON property name differs from the Ruby attribute name. ```ruby @@ -84,7 +99,12 @@ end This applies wherever that path occurs, including once per parent class for a nested object reused by several top-level parameter classes. Omit `kind:` to ignore every -finding at that path. Ignored findings move to `result.ignored` instead of +error kind at that path. Pass `parameter_class:` (a class or its name) to limit a rule +to one class. Valid kinds are `missing_attribute`, `unexpected_attribute`, +`missing_presence_validation`, `type_mismatch`, `missing_schema` and +`unresolved_reference`; anything else raises `ArgumentError`. Paths use attribute names +joined with `.`, with `[]` for array items (for example `items[].sku`), and a schema +that cannot be found at all is reported at `(root)`. Skips are never ignored. Ignored findings move to `result.ignored` instead of `result.errors`, so `valid?` treats them as resolved and they don't appear in `to_message` — but they stay available on the result object for anyone who wants to audit what's being suppressed and why, rather than disappearing silently. @@ -92,8 +112,10 @@ audit what's being suppressed and why, rather than disappearing silently. ## Rake task The gem provides `structured_params:openapi:drift_check` in Rails applications. -The task automatically checks classes selected by `parameter_class_filter`, so a -new `Api::*Params` class needs no registration. Use the Registry only for an +The task automatically checks classes selected by `parameter_class_filter` (without a +filter, only registered classes are checked, and the task warns when nothing was +checked), so a new `Api::*Params` class needs no registration. Anonymous classes are +never auto-discovered, which is also why the filter example guards with `name&.`. Use the Registry only for an exceptional component name or property mapping; explicit registrations override auto-discovery for the same class. @@ -122,15 +144,31 @@ Run it in CI with: bundle exec rake structured_params:openapi:drift_check ``` -The current scope deliberately skips `oneOf`, `anyOf`, `allOf`, multiple-type -schemas, and external references. Skips are reported in `result.skipped` so they -remain visible for manual review without being reported as a false type match. +The current scope deliberately skips `oneOf`, `anyOf`, multi-entry `allOf`, schemas +with a `type` array (every OpenAPI 3.1 nullable such as `[string, 'null']`; 3.0 +`nullable: true` is compared normally), and external references. A single-entry +`allOf` is treated as the schema it wraps, as TypeSpec emits for `$ref` plus a +`description`. Recursive schemas are compared once per ancestor chain; the repeated visit +is dropped silently. + +Skips never fail a run, but they are not silent: the Rake task prints every skipped +and ignored entry with a summary line, and `result.skipped` / `result.to_report` expose +them in RSpec. Set `STRUCTURED_PARAMS_STRICT=1` (or pass `strict: true` to +`Registry.check_all!`) to fail on skipped checks too. + +A class whose resolved schema name has no matching component is reported as a +`missing_schema` error, as is a dangling or circular local `$ref` +(`unresolved_reference`), so a renamed component or a typo cannot silently drop +coverage. For a class that intentionally has no schema (for example one backed by query +parameters), ignore it explicitly: -A class whose resolved schema name has no matching component is also skipped -rather than raising, so one misconfigured or intentionally out-of-scope class -(for example, one backed by query parameters instead of a requestBody schema) -doesn't stop the whole task from reporting every other class's drift. +```ruby +StructuredParams::OpenApi.configure do |config| + config.ignore_drift(path: '(root)', kind: :missing_schema, parameter_class: 'SearchParams') +end +``` `result.to_message` is prefixed with the checked class's name, so `check_all!`'s combined `DriftError` message reads as one block per class instead of a flat, -unattributed list of paths. +unattributed list of paths. A later registration for the same class replaces an earlier +one. diff --git a/lib/structured_params/errors.rb b/lib/structured_params/errors.rb index 1284e66..cac5360 100644 --- a/lib/structured_params/errors.rb +++ b/lib/structured_params/errors.rb @@ -1,6 +1,7 @@ # rbs_inline: enabled # frozen_string_literal: true +# rubocop:disable Style/OptionalBooleanParameter module StructuredParams # Custom errors collection that handles nested attribute names class Errors < ActiveModel::Errors @@ -8,7 +9,6 @@ class Errors < ActiveModel::Errors # Add structured option to get nested structure for dot-notation attributes #: (?bool, ?structured: false) -> Hash[Symbol, String] #: (?bool, structured: bool) -> Hash[Symbol, untyped] - # rubocop:disable-next Style/OptionalBooleanParameter def to_hash(full_messages = false, structured: false) if structured attribute_messages_hash = build_attribute_messages_hash(full_messages) @@ -43,7 +43,6 @@ def messages(structured: false) # Build a hash with attribute names as keys and their error messages as values # This is used for to_hash(structured: true) #: (bool) -> Hash[Symbol, Array[String]] - # rubocop:disable-next Style/OptionalBooleanParameter def build_attribute_messages_hash(full_messages = false) message_method = full_messages ? :full_message : :message @@ -66,3 +65,4 @@ def build_nested_hash(target_hash, flat_hash, separator = '.') end end end +# rubocop:enable Style/OptionalBooleanParameter diff --git a/lib/structured_params/open_api/configuration.rb b/lib/structured_params/open_api/configuration.rb index 04ed99e..1c7a913 100644 --- a/lib/structured_params/open_api/configuration.rb +++ b/lib/structured_params/open_api/configuration.rb @@ -1,12 +1,16 @@ # rbs_inline: enabled # frozen_string_literal: true +require 'date' +require 'yaml' + module StructuredParams # OpenAPI contract configuration and drift-check utilities. module OpenApi # Global OpenAPI document and component-name convention for drift checks. class Configuration # @rbs @ignored_issues: Array[Hash[Symbol, untyped]] + # @rbs @documents: Hash[String, Array[untyped]] attr_accessor :document #: Hash[untyped, untyped]|String? attr_writer :schema_name_resolver #: untyped @@ -18,23 +22,57 @@ def initialize parameter_class.name.split('::').last.sub(/(?:Params?|Parameters?|Form)$/, '') end @ignored_issues = [] + @documents = {} + end + + # Parses an OpenAPI file once per modification time, so a bulk check across many + # parameter classes does not re-read the same document for each of them. JSON files + # work too, since JSON is valid YAML. Only the mtime invalidates the cache, so two writes + # within the filesystem's timestamp granularity can serve a stale parse. YAML dates such as + # `example: 2020-01-01` load as Date, hence +permitted_classes+. + #: (String) -> Hash[untyped, untyped] + def load_document(path) + mtime = File.mtime(path) + cached_mtime, document = @documents[path] + return document if cached_mtime == mtime + + document = YAML.safe_load_file(path, aliases: true, permitted_classes: [Date, Time]) + unless document.is_a?(Hash) + raise ArgumentError, + "OpenAPI document #{path} must contain a mapping at its top level" + end + + @documents[path] = [mtime, document] + document end - # Suppresses a specific finding (matched by +path+ and, optionally, +kind+) across - # every drift check, instead of raising it as an error. Applies wherever that path - # occurs, including once per parent class for a nested object reused by several - # top-level parameter classes. Omit +kind+ to ignore every kind at that path. + # Suppresses a specific finding (matched by exact +path+ and, optionally, +kind+) instead + # of raising it as an error. Without +parameter_class+ it applies to every drift check, + # including once per parent class for a nested object reused by several top-level + # parameter classes. Omit +kind+ to ignore every error kind at that path. Skips + # (unsupported schema features) are never ignored; they only appear in Result#skipped. + # + # Paths use attribute names joined with +.+ and +[]+ for array items, e.g. + # +items[].sku+; a schema that cannot be found at all is reported at +(root)+. # # Ignored findings are moved to DriftCheck::Result#ignored rather than dropped, so # they stay auditable; #valid? and #to_message treat them as resolved. - #: (path: String, ?kind: Symbol?) -> void - def ignore_drift(path:, kind: nil) - @ignored_issues << { path: path, kind: kind } + #: (path: String, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void + def ignore_drift(path:, kind: nil, parameter_class: nil) + if kind && !DriftCheck::ERROR_KINDS.include?(kind) + raise ArgumentError, "unknown kind #{kind.inspect}; expected one of #{DriftCheck::ERROR_KINDS.inspect}" + end + + @ignored_issues << { path: path, kind: kind, parameter_class: parameter_class&.to_s } end - #: (Symbol, String) -> bool - def ignored?(kind, path) - @ignored_issues.any? { |rule| rule.fetch(:path) == path && [nil, kind].include?(rule.fetch(:kind)) } + #: (Symbol, String, ?untyped) -> bool + def ignored?(kind, path, parameter_class = nil) + @ignored_issues.any? do |rule| + rule.fetch(:path) == path && + [nil, kind].include?(rule.fetch(:kind)) && + [nil, parameter_class&.to_s].include?(rule.fetch(:parameter_class)) + end end #: () -> Array[singleton(StructuredParams::Params)] @@ -42,11 +80,12 @@ def parameter_classes return [] unless @parameter_class_filter ObjectSpace.each_object(Class).select do |parameter_class| - parameter_class < StructuredParams::Params && @parameter_class_filter.call(parameter_class) + parameter_class < StructuredParams::Params && parameter_class.name && + @parameter_class_filter.call(parameter_class) end end - #: (singleton(StructuredParams::Params), String?) -> String + #: (singleton(StructuredParams::Params), ?String?) -> String def pointer_for(parameter_class, schema_name = nil) name = schema_name || @schema_name_resolver.call(parameter_class) return name if name.start_with?('/', '#/') diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb index 536e79d..b4741a7 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/lib/structured_params/open_api/drift_check.rb @@ -1,13 +1,14 @@ # rbs_inline: enabled # frozen_string_literal: true +require 'date' require 'yaml' module StructuredParams # Schema comparison types for OpenAPI drift checks. module OpenApi # Compares OpenAPI object schemas with StructuredParams declarations. - # rubocop:disable-next Metrics/ClassLength + # rubocop:disable Metrics/ClassLength class DriftCheck # @rbs @openapi_source: Hash[untyped, untyped]|String? # @rbs @schema_source: Hash[untyped, untyped]|String? @@ -19,7 +20,13 @@ class DriftCheck # @rbs @errors: Array[Issue] # @rbs @skipped: Array[Issue] # @rbs @ignored: Array[Issue] - # @rbs @seen_pairs: Array[Array[untyped]] + # @rbs @active_pairs: Array[Array[untyped]] + + # Finding kinds that can be raised as errors (and therefore matched by +ignore_drift+). + ERROR_KINDS = %i[ + missing_attribute unexpected_attribute missing_presence_validation type_mismatch + missing_schema unresolved_reference + ].freeze #: Array[Symbol] Issue = Struct.new( :kind, #: Symbol @@ -30,9 +37,8 @@ class DriftCheck # Result of a single schema/class comparison. class Result - # @rbs @parameter_class: singleton(::StructuredParams::Params) - attr_reader :errors, :skipped, :ignored #: Array[Issue] + attr_reader :parameter_class #: singleton(::StructuredParams::Params) #: (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void def initialize(parameter_class, errors, skipped, ignored) @@ -54,11 +60,28 @@ def to_message sections << "OpenAPI checks skipped:\n#{format_issues(skipped)}" unless skipped.empty? return '' if sections.empty? - "#{@parameter_class.name || @parameter_class.inspect}\n#{sections.join("\n")}" + "#{class_label}\n#{sections.join("\n")}" + end + + # Lists what a passing check still left unverified (skipped) or suppressed (ignored), + # so those stay visible in CI output instead of only on the result object. + #: () -> String + def to_report + sections = [] + sections << "OpenAPI checks skipped:\n#{format_issues(skipped)}" unless skipped.empty? + sections << "OpenAPI findings ignored:\n#{format_issues(ignored)}" unless ignored.empty? + return '' if sections.empty? + + "#{class_label}\n#{sections.join("\n")}" end private + #: () -> String + def class_label + @parameter_class.name || @parameter_class.inspect + end + #: (Array[Issue]) -> String def format_issues(issues) issues.map { |issue| "- #{issue.path}: #{issue.message}" }.join("\n") @@ -66,8 +89,8 @@ def format_issues(issues) end class << self + # rubocop:disable Metrics/ParameterLists #: (**untyped) -> Result - # rubocop:disable-next Metrics/ParameterLists def call( parameter_class:, openapi: nil, @@ -85,10 +108,11 @@ def call( required_validation: required_validation ).call end + # rubocop:enable Metrics/ParameterLists end + # rubocop:disable Metrics/ParameterLists #: (**untyped) -> void - # rubocop:disable-next Metrics/ParameterLists def initialize( parameter_class:, openapi: nil, @@ -106,7 +130,7 @@ def initialize( @errors = [] @skipped = [] @ignored = [] - @seen_pairs = [] #: Array[Array[untyped]] + @active_pairs = [] #: Array[Array[untyped]] unless %i[presence none].include?(required_validation) raise ArgumentError, "required_validation must be :presence or :none, got #{required_validation.inspect}" @@ -116,24 +140,43 @@ def initialize( raise ArgumentError, 'configure an OpenAPI document or pass schema as a Hash' end + # rubocop:enable Metrics/ParameterLists #: () -> Result def call - begin - schema, @document = load_schema - rescue KeyError => e - skip('', "no matching OpenAPI schema found (#{e.message}); " \ - 'check the schema_name_resolver convention or register an explicit schema name') - return Result.new(@parameter_class, @errors, @skipped, @ignored) - end + @errors = [] + @skipped = [] + @ignored = [] + @active_pairs = [] - check_object(resolve_references(schema), @parameter_class, '') - Result.new(@parameter_class, @errors, @skipped, @ignored) + schema = load_root_schema + check_object(resolve_references(schema, ''), @parameter_class, '') if schema + Result.new(@parameter_class, @errors.dup.freeze, @skipped.dup.freeze, @ignored.dup.freeze) end private - #: () -> Array[Hash[untyped, untyped]] + # A missing or mistyped schema means the class silently lost its contract coverage, + # so it is an error (suppressible with +ignore_drift+) rather than a skip. + #: () -> Hash[untyped, untyped]? + def load_root_schema + schema, @document = load_schema + return schema if schema.is_a?(Hash) + + report_missing_schema('the selected schema is not an object') + rescue IndexError => e # also covers KeyError + report_missing_schema(e.message) + end + + #: (String) -> nil + def report_missing_schema(detail) + error('(root)', :missing_schema, + "no matching OpenAPI schema found (#{detail}); " \ + 'check the schema_name_resolver convention or register an explicit schema name') + nil + end + + #: () -> [Hash[untyped, untyped], Hash[untyped, untyped]] def load_schema if @schema_source.is_a?(Hash) return [@schema_source, @schema_source] if selected_pointer.nil? @@ -150,7 +193,7 @@ def load_schema def load_document return @openapi_source if @openapi_source.is_a?(Hash) - YAML.safe_load_file(@openapi_source, aliases: true) + OpenApi.configuration.load_document(@openapi_source) end #: () -> String? @@ -162,11 +205,26 @@ def selected_pointer end #: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void - # rubocop:disable-next Metrics/AbcSize def check_object(schema, parameter_class, path) - return if skip_union?(schema, path) - return if seen?(schema, parameter_class) + return if unresolved_reference?(schema) || skip_union?(schema, path) + + # Only ancestors are tracked (compared by structural equality), so a schema reused by + # sibling properties is still compared each time while a self-referencing schema + # stops silently at its second visit. + pair = [schema, parameter_class] + return if @active_pairs.include?(pair) + @active_pairs.push(pair) + begin + check_object_properties(schema, parameter_class, path) + ensure + @active_pairs.pop + end + end + + #: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void + # rubocop:disable Metrics/AbcSize + def check_object_properties(schema, parameter_class, path) properties = value_for(schema, 'properties') unless properties.is_a?(Hash) skip(path, 'schema has no properties; only object schemas are supported') @@ -179,10 +237,21 @@ def check_object(schema, parameter_class, path) compare_required_attributes(schema, schema_attributes, parameter_class, path) (schema_attributes.keys & attribute_types.keys.map(&:to_sym)).each do |attribute| - property_schema = resolve_references(value_for(properties, schema_attributes.fetch(attribute))) - compare_type(property_schema, attribute_types.fetch(attribute.to_s), join_path(path, attribute)) + compare_property(value_for(properties, schema_attributes.fetch(attribute)), + attribute_types.fetch(attribute.to_s), join_path(path, attribute)) end end + # rubocop:enable Metrics/AbcSize + + #: (untyped, ActiveModel::Type::Value, String) -> void + def compare_property(property_schema, attribute_type, path) + unless property_schema.is_a?(Hash) + skip(path, 'property schema is not an object') + return + end + + compare_type(resolve_references(property_schema, path), attribute_type, path) + end #: (Hash[Symbol, String], Hash[String, ActiveModel::Type::Value], String) -> void def compare_attribute_names(schema_attributes, attribute_types, path) @@ -214,10 +283,10 @@ def compare_required_attributes(schema, schema_attributes, parameter_class, path end end + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity #: (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void - # rubocop:disable-next Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity def compare_type(schema, attribute_type, path) - return if skip_union?(schema, path) + return if unresolved_reference?(schema) || skip_union?(schema, path) if value_for(schema, 'type').is_a?(Array) skip(path, 'multiple schema types are outside the supported drift-check scope') @@ -238,6 +307,7 @@ def compare_type(schema, attribute_type, path) compare_array_item_type(schema, attribute_type, path) end end + # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity #: (Hash[untyped, untyped], Type::Array, String) -> void def compare_array_item_type(schema, attribute_type, path) @@ -247,7 +317,7 @@ def compare_array_item_type(schema, attribute_type, path) return end - compare_type(resolve_references(items), attribute_type.item_type, "#{path}[]") + compare_type(resolve_references(items, "#{path}[]"), attribute_type.item_type, "#{path}[]") end #: (Hash[untyped, untyped]) -> Symbol? @@ -263,6 +333,8 @@ def schema_type(schema) def compatible_types?(schema_type, attribute_type) return false if schema_type.nil? return %i[float decimal].include?(attribute_type) if schema_type == :number + return %i[date datetime time string].include?(attribute_type) if schema_type == :string + return %i[integer big_integer].include?(attribute_type) if schema_type == :integer schema_type == attribute_type end @@ -283,27 +355,57 @@ def skip_union?(schema, path) true end - #: (Hash[untyped, untyped]) -> Hash[untyped, untyped] - def resolve_references(schema) + #: (Hash[untyped, untyped], String, ?Array[String]) -> Hash[untyped, untyped] + def resolve_references(schema, path, followed = []) schema = unwrap_single_all_of(schema) reference = value_for(schema, '$ref') return schema unless reference unless reference.start_with?('#/') - skip('', "external reference #{reference.inspect} is outside the supported drift-check scope") + skip(path, "external reference #{reference.inspect} is outside the supported drift-check scope") return schema end - referenced_schema = resolve_pointer(@document, reference.delete_prefix('#')) + referenced_schema = fetch_reference(reference, path, followed) + return schema unless referenced_schema + overrides = schema.reject { |key, _| key.to_s == '$ref' } - resolve_references(referenced_schema.merge(overrides)) + resolve_references(referenced_schema.merge(overrides), path, followed + [reference]) + end + + # A dangling or circular local reference is a broken contract, not an unsupported feature. + #: (String, String, Array[String]) -> Hash[untyped, untyped]? + def fetch_reference(reference, path, followed) + return unresolved_reference(path, reference, 'it refers back to itself') if followed.include?(reference) + + target = resolve_pointer(@document, reference.delete_prefix('#')) + return target if target.is_a?(Hash) + + unresolved_reference(path, reference, 'it does not point to a schema object') + rescue IndexError => e # also covers KeyError + unresolved_reference(path, reference, e.message) + end + + #: (String, String, String) -> nil + def unresolved_reference(path, reference, detail) + error(display_path(path), :unresolved_reference, + "reference #{reference.inspect} could not be resolved (#{detail})") + nil + end + + # A `$ref` left after resolve_references was already reported (skipped if external, an + # error if unresolvable), so comparing it further would only add misleading findings. + #: (Hash[untyped, untyped]) -> bool + def unresolved_reference?(schema) + !value_for(schema, '$ref').nil? end # OpenAPI/JSON Schema forbid sibling keys next to `$ref`, so generators such as TypeSpec # commonly wrap a single `$ref` in `allOf` to attach a `description`, `default`, etc. - # That's equivalent to the ref carrying those siblings directly, so unwrap and merge it - # instead of treating it as unsupported schema composition. `allOf` with more than one - # entry is genuine composition and stays out of scope (reported via skip_union?). + # That's equivalent to the ref carrying those siblings directly, so unwrap and merge a + # single-entry `allOf` (typically a `$ref`, though an inline schema is unwrapped the same + # way) instead of treating it as unsupported schema composition. `allOf` with more than + # one entry is genuine composition and stays out of scope (reported via skip_union?). #: (Hash[untyped, untyped]) -> Hash[untyped, untyped] def unwrap_single_all_of(schema) all_of = value_for(schema, 'allOf') @@ -313,13 +415,24 @@ def unwrap_single_all_of(schema) all_of.first.merge(overrides) end - #: (Hash[untyped, untyped], String) -> Hash[untyped, untyped] + #: (untyped, String) -> untyped def resolve_pointer(document, pointer) pointer.delete_prefix('#').delete_prefix('/').split('/').reduce(document) do |value, segment| - value.fetch(segment.gsub('~1', '/').gsub('~0', '~')) + dig_pointer_segment(value, segment.gsub('~1', '/').gsub('~0', '~')) end end + #: (untyped, String) -> untyped + def dig_pointer_segment(value, segment) + case value + when Hash then value.fetch(segment) + when Array then value.fetch(Integer(segment)) + else raise KeyError, "key not found: #{segment.inspect}" + end + rescue ArgumentError + raise KeyError, "key not found: #{segment.inspect}" + end + #: (Hash[untyped, untyped], String) -> untyped def value_for(hash, key) hash[key] || hash[key.to_sym] @@ -330,15 +443,6 @@ def attribute_name(property) @property_names.invert.fetch(property.to_s, property).to_sym end - #: (Hash[untyped, untyped], singleton(StructuredParams::Params)) -> bool - def seen?(schema, parameter_class) - pair = [schema.object_id, parameter_class] - return true if @seen_pairs.include?(pair) - - @seen_pairs << pair - false - end - #: (String, String) -> String def join_path(path, name) path.empty? ? name.to_s : "#{path}.#{name}" @@ -347,14 +451,20 @@ def join_path(path, name) #: (String, Symbol, String) -> void def error(path, kind, message) issue = Issue.new(kind: kind, path: path, message: message) - target = OpenApi.configuration.ignored?(kind, path) ? @ignored : @errors + target = OpenApi.configuration.ignored?(kind, path, @parameter_class) ? @ignored : @errors target << issue end + #: (String) -> String + def display_path(path) + path.empty? ? '(root)' : path + end + #: (String, String) -> void def skip(path, message) - @skipped << Issue.new(kind: :unsupported, path: path.empty? ? '(root)' : path, message: message) + @skipped << Issue.new(kind: :unsupported, path: display_path(path), message: message) end end + # rubocop:enable Metrics/ClassLength end end diff --git a/lib/structured_params/open_api/registry.rb b/lib/structured_params/open_api/registry.rb index e48bf01..f0fa0dc 100644 --- a/lib/structured_params/open_api/registry.rb +++ b/lib/structured_params/open_api/registry.rb @@ -34,10 +34,12 @@ def check_all(auto_discover: false) checks.map { |parameter_class, options| DriftCheck.call(parameter_class: parameter_class, **options) } end - #: (?auto_discover: bool) -> Array[DriftCheck::Result] - def check_all!(auto_discover: false) + # With +strict+, a check that skipped part of a schema also fails the run. + #: (?auto_discover: bool, ?strict: bool) -> Array[DriftCheck::Result] + def check_all!(auto_discover: false, strict: false) results = check_all(auto_discover: auto_discover) - errors = results.reject(&:valid?).map(&:to_message) + failing = results.reject { |result| result.valid? && (!strict || result.skipped.empty?) } + errors = failing.map(&:to_message) # Leading blank line so the first class's message doesn't get glued onto the # "ClassName: " prefix Ruby/Rake print ahead of an exception's message. raise DriftError, "\n\n#{errors.join("\n\n")}" unless errors.empty? @@ -45,6 +47,19 @@ def check_all!(auto_discover: false) results end + # Describes what passing checks skipped or ignored, plus a one-line summary. + #: (Array[DriftCheck::Result]) -> String + def report(results) + return 'No parameter classes were checked; set parameter_class_filter or register classes.' if results.empty? + + skipped = results.count { |result| !result.skipped.empty? } + ignored = results.count { |result| !result.ignored.empty? } + sections = results.map(&:to_report).reject(&:empty?) + sections << "Checked #{results.size} classes: #{skipped} with skipped checks, " \ + "#{ignored} with ignored findings." + sections.join("\n\n") + end + #: () -> void def clear @entries = [] diff --git a/lib/structured_params/params.rb b/lib/structured_params/params.rb index 73465ab..02ab255 100644 --- a/lib/structured_params/params.rb +++ b/lib/structured_params/params.rb @@ -50,7 +50,7 @@ module StructuredParams # <%= f.text_field :name %> # <%= f.text_field :email %> # <% end %> - # rubocop:disable-next Metrics/ClassLength + # rubocop:disable Metrics/ClassLength class Params include ActiveModel::Model include ActiveModel::Attributes @@ -347,4 +347,5 @@ def import_structured_errors(structured_errors, prefix) end end end + # rubocop:enable Metrics/ClassLength end diff --git a/lib/tasks/structured_params_open_api.rake b/lib/tasks/structured_params_open_api.rake index 453fdc8..321652a 100644 --- a/lib/tasks/structured_params_open_api.rake +++ b/lib/tasks/structured_params_open_api.rake @@ -5,8 +5,11 @@ namespace :structured_params do namespace :openapi do desc 'Check registered and auto-discovered StructuredParams classes against the OpenAPI document' task drift_check: :environment do - Rails.application.eager_load! if defined?(Rails) && Rails.respond_to?(:application) - StructuredParams::OpenApi::Registry.check_all!(auto_discover: true) + Rails.application&.eager_load! if defined?(Rails) && Rails.respond_to?(:application) + registry = StructuredParams::OpenApi::Registry + # Set STRUCTURED_PARAMS_STRICT=1 to fail the task when any check was skipped. + results = registry.check_all!(auto_discover: true, strict: ENV['STRUCTURED_PARAMS_STRICT'] == '1') + puts registry.report(results) end end end diff --git a/sig/structured_params/errors.rbs b/sig/structured_params/errors.rbs index 747b309..b415827 100644 --- a/sig/structured_params/errors.rbs +++ b/sig/structured_params/errors.rbs @@ -1,5 +1,6 @@ # Generated from lib/structured_params/errors.rb with RBS::Inline +# rubocop:disable Style/OptionalBooleanParameter module StructuredParams # Custom errors collection that handles nested attribute names class Errors < ActiveModel::Errors @@ -7,7 +8,6 @@ module StructuredParams # Add structured option to get nested structure for dot-notation attributes # : (?bool, ?structured: false) -> Hash[Symbol, String] # : (?bool, structured: bool) -> Hash[Symbol, untyped] - # rubocop:disable-next Style/OptionalBooleanParameter def to_hash: (?bool, ?structured: false) -> Hash[Symbol, String] | (?bool, structured: bool) -> Hash[Symbol, untyped] @@ -26,7 +26,6 @@ module StructuredParams # Build a hash with attribute names as keys and their error messages as values # This is used for to_hash(structured: true) # : (bool) -> Hash[Symbol, Array[String]] - # rubocop:disable-next Style/OptionalBooleanParameter def build_attribute_messages_hash: (bool) -> Hash[Symbol, Array[String]] # Build a nested hash structure from flat dot-notation keys diff --git a/sig/structured_params/open_api/configuration.rbs b/sig/structured_params/open_api/configuration.rbs index 5412a82..c0142c6 100644 --- a/sig/structured_params/open_api/configuration.rbs +++ b/sig/structured_params/open_api/configuration.rbs @@ -7,6 +7,8 @@ module StructuredParams class Configuration @ignored_issues: Array[Hash[Symbol, untyped]] + @documents: Hash[String, Array[untyped]] + attr_accessor document: Hash[untyped, untyped] | String? attr_writer schema_name_resolver: untyped @@ -16,24 +18,36 @@ module StructuredParams # : () -> void def initialize: () -> void - # Suppresses a specific finding (matched by +path+ and, optionally, +kind+) across - # every drift check, instead of raising it as an error. Applies wherever that path - # occurs, including once per parent class for a nested object reused by several - # top-level parameter classes. Omit +kind+ to ignore every kind at that path. + # Parses an OpenAPI file once per modification time, so a bulk check across many + # parameter classes does not re-read the same document for each of them. JSON files + # work too, since JSON is valid YAML. Only the mtime invalidates the cache, so two writes + # within the filesystem's timestamp granularity can serve a stale parse. YAML dates such as + # `example: 2020-01-01` load as Date, hence +permitted_classes+. + # : (String) -> Hash[untyped, untyped] + def load_document: (String) -> Hash[untyped, untyped] + + # Suppresses a specific finding (matched by exact +path+ and, optionally, +kind+) instead + # of raising it as an error. Without +parameter_class+ it applies to every drift check, + # including once per parent class for a nested object reused by several top-level + # parameter classes. Omit +kind+ to ignore every error kind at that path. Skips + # (unsupported schema features) are never ignored; they only appear in Result#skipped. + # + # Paths use attribute names joined with +.+ and +[]+ for array items, e.g. + # +items[].sku+; a schema that cannot be found at all is reported at +(root)+. # # Ignored findings are moved to DriftCheck::Result#ignored rather than dropped, so # they stay auditable; #valid? and #to_message treat them as resolved. - # : (path: String, ?kind: Symbol?) -> void - def ignore_drift: (path: String, ?kind: Symbol?) -> void + # : (path: String, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void + def ignore_drift: (path: String, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void - # : (Symbol, String) -> bool - def ignored?: (Symbol, String) -> bool + # : (Symbol, String, ?untyped) -> bool + def ignored?: (Symbol, String, ?untyped) -> bool # : () -> Array[singleton(StructuredParams::Params)] def parameter_classes: () -> Array[singleton(StructuredParams::Params)] - # : (singleton(StructuredParams::Params), String?) -> String - def pointer_for: (singleton(StructuredParams::Params), String?) -> String + # : (singleton(StructuredParams::Params), ?String?) -> String + def pointer_for: (singleton(StructuredParams::Params), ?String?) -> String end self.@configuration: Configuration? diff --git a/sig/structured_params/open_api/drift_check.rbs b/sig/structured_params/open_api/drift_check.rbs index 2829f62..d2a5b0a 100644 --- a/sig/structured_params/open_api/drift_check.rbs +++ b/sig/structured_params/open_api/drift_check.rbs @@ -4,9 +4,9 @@ module StructuredParams # Schema comparison types for OpenAPI drift checks. module OpenApi # Compares OpenAPI object schemas with StructuredParams declarations. - # rubocop:disable-next Metrics/ClassLength + # rubocop:disable Metrics/ClassLength class DriftCheck - @seen_pairs: Array[Array[untyped]] + @active_pairs: Array[Array[untyped]] @ignored: Array[Issue] @@ -28,6 +28,9 @@ module StructuredParams @openapi_source: Hash[untyped, untyped] | String? + # Finding kinds that can be raised as errors (and therefore matched by +ignore_drift+). + ERROR_KINDS: Array[Symbol] + class Issue < Struct[Symbol | String] attr_accessor kind(): Symbol @@ -41,14 +44,14 @@ module StructuredParams # Result of a single schema/class comparison. class Result - @parameter_class: singleton(::StructuredParams::Params) - attr_reader errors: Array[Issue] attr_reader skipped: Array[Issue] attr_reader ignored: Array[Issue] + attr_reader parameter_class: singleton(::StructuredParams::Params) + # : (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void def initialize: (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void @@ -58,18 +61,26 @@ module StructuredParams # : () -> String def to_message: () -> String + # Lists what a passing check still left unverified (skipped) or suppressed (ignored), + # so those stay visible in CI output instead of only on the result object. + # : () -> String + def to_report: () -> String + private + # : () -> String + def class_label: () -> String + # : (Array[Issue]) -> String def format_issues: (Array[Issue]) -> String end + # rubocop:disable Metrics/ParameterLists # : (**untyped) -> Result - # rubocop:disable-next Metrics/ParameterLists def self.call: (**untyped) -> Result + # rubocop:disable Metrics/ParameterLists # : (**untyped) -> void - # rubocop:disable-next Metrics/ParameterLists def initialize: (**untyped) -> void # : () -> Result @@ -77,8 +88,16 @@ module StructuredParams private - # : () -> Array[Hash[untyped, untyped]] - def load_schema: () -> Array[Hash[untyped, untyped]] + # A missing or mistyped schema means the class silently lost its contract coverage, + # so it is an error (suppressible with +ignore_drift+) rather than a skip. + # : () -> Hash[untyped, untyped]? + def load_root_schema: () -> Hash[untyped, untyped]? + + # : (String) -> nil + def report_missing_schema: (String) -> nil + + # : () -> [Hash[untyped, untyped], Hash[untyped, untyped]] + def load_schema: () -> [ Hash[untyped, untyped], Hash[untyped, untyped] ] # : () -> Hash[untyped, untyped] def load_document: () -> Hash[untyped, untyped] @@ -87,17 +106,23 @@ module StructuredParams def selected_pointer: () -> String? # : (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void - # rubocop:disable-next Metrics/AbcSize def check_object: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void + # : (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void + # rubocop:disable Metrics/AbcSize + def check_object_properties: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void + + # : (untyped, ActiveModel::Type::Value, String) -> void + def compare_property: (untyped, ActiveModel::Type::Value, String) -> void + # : (Hash[Symbol, String], Hash[String, ActiveModel::Type::Value], String) -> void def compare_attribute_names: (Hash[Symbol, String], Hash[String, ActiveModel::Type::Value], String) -> void # : (Hash[untyped, untyped], Hash[Symbol, String], singleton(StructuredParams::Params), String) -> void def compare_required_attributes: (Hash[untyped, untyped], Hash[Symbol, String], singleton(StructuredParams::Params), String) -> void + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity # : (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void - # rubocop:disable-next Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity def compare_type: (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void # : (Hash[untyped, untyped], Type::Array, String) -> void @@ -115,19 +140,35 @@ module StructuredParams # : (Hash[untyped, untyped], String) -> bool def skip_union?: (Hash[untyped, untyped], String) -> bool - # : (Hash[untyped, untyped]) -> Hash[untyped, untyped] - def resolve_references: (Hash[untyped, untyped]) -> Hash[untyped, untyped] + # : (Hash[untyped, untyped], String, ?Array[String]) -> Hash[untyped, untyped] + def resolve_references: (Hash[untyped, untyped], String, ?Array[String]) -> Hash[untyped, untyped] + + # A dangling or circular local reference is a broken contract, not an unsupported feature. + # : (String, String, Array[String]) -> Hash[untyped, untyped]? + def fetch_reference: (String, String, Array[String]) -> Hash[untyped, untyped]? + + # : (String, String, String) -> nil + def unresolved_reference: (String, String, String) -> nil + + # A `$ref` left after resolve_references was already reported (skipped if external, an + # error if unresolvable), so comparing it further would only add misleading findings. + # : (Hash[untyped, untyped]) -> bool + def unresolved_reference?: (Hash[untyped, untyped]) -> bool # OpenAPI/JSON Schema forbid sibling keys next to `$ref`, so generators such as TypeSpec # commonly wrap a single `$ref` in `allOf` to attach a `description`, `default`, etc. - # That's equivalent to the ref carrying those siblings directly, so unwrap and merge it - # instead of treating it as unsupported schema composition. `allOf` with more than one - # entry is genuine composition and stays out of scope (reported via skip_union?). + # That's equivalent to the ref carrying those siblings directly, so unwrap and merge a + # single-entry `allOf` (typically a `$ref`, though an inline schema is unwrapped the same + # way) instead of treating it as unsupported schema composition. `allOf` with more than + # one entry is genuine composition and stays out of scope (reported via skip_union?). # : (Hash[untyped, untyped]) -> Hash[untyped, untyped] def unwrap_single_all_of: (Hash[untyped, untyped]) -> Hash[untyped, untyped] - # : (Hash[untyped, untyped], String) -> Hash[untyped, untyped] - def resolve_pointer: (Hash[untyped, untyped], String) -> Hash[untyped, untyped] + # : (untyped, String) -> untyped + def resolve_pointer: (untyped, String) -> untyped + + # : (untyped, String) -> untyped + def dig_pointer_segment: (untyped, String) -> untyped # : (Hash[untyped, untyped], String) -> untyped def value_for: (Hash[untyped, untyped], String) -> untyped @@ -135,15 +176,15 @@ module StructuredParams # : (untyped) -> Symbol def attribute_name: (untyped) -> Symbol - # : (Hash[untyped, untyped], singleton(StructuredParams::Params)) -> bool - def seen?: (Hash[untyped, untyped], singleton(StructuredParams::Params)) -> bool - # : (String, String) -> String def join_path: (String, String) -> String # : (String, Symbol, String) -> void def error: (String, Symbol, String) -> void + # : (String) -> String + def display_path: (String) -> String + # : (String, String) -> void def skip: (String, String) -> void end diff --git a/sig/structured_params/open_api/registry.rbs b/sig/structured_params/open_api/registry.rbs index 640df0f..e0717e3 100644 --- a/sig/structured_params/open_api/registry.rbs +++ b/sig/structured_params/open_api/registry.rbs @@ -19,8 +19,13 @@ module StructuredParams # : (?auto_discover: bool) -> Array[DriftCheck::Result] def self.check_all: (?auto_discover: bool) -> Array[DriftCheck::Result] - # : (?auto_discover: bool) -> Array[DriftCheck::Result] - def self.check_all!: (?auto_discover: bool) -> Array[DriftCheck::Result] + # With +strict+, a check that skipped part of a schema also fails the run. + # : (?auto_discover: bool, ?strict: bool) -> Array[DriftCheck::Result] + def self.check_all!: (?auto_discover: bool, ?strict: bool) -> Array[DriftCheck::Result] + + # Describes what passing checks skipped or ignored, plus a one-line summary. + # : (Array[DriftCheck::Result]) -> String + def self.report: (Array[DriftCheck::Result]) -> String # : () -> void def self.clear: () -> void diff --git a/sig/structured_params/params.rbs b/sig/structured_params/params.rbs index 42a93a3..55aa1e7 100644 --- a/sig/structured_params/params.rbs +++ b/sig/structured_params/params.rbs @@ -49,7 +49,7 @@ module StructuredParams # <%= f.text_field :name %> # <%= f.text_field :email %> # <% end %> - # rubocop:disable-next Metrics/ClassLength + # rubocop:disable Metrics/ClassLength class Params include ActiveModel::Model diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index 6ffa985..c0fba93 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -106,7 +106,7 @@ errors.add('items.1.subitems.0.description', 'is too long') end - # rubocop:disable-next RSpec/ExampleLength + # rubocop:disable RSpec/ExampleLength it 'creates deep nested structure' do expect(errors.to_hash(false, structured: true)).to eq({ items: { @@ -127,6 +127,7 @@ } }) end + # rubocop:enable RSpec/ExampleLength end context 'with mixed flat and nested attributes' do diff --git a/spec/form_object_spec.rb b/spec/form_object_spec.rb index 608320f..4d318c7 100644 --- a/spec/form_object_spec.rb +++ b/spec/form_object_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -# rubocop:disable-next RSpec/DescribeClass +# rubocop:disable RSpec/DescribeClass RSpec.describe 'StructuredParams::Params as Form Object' do describe '.form_class?' do it 'returns true for classes with a Form suffix' do @@ -258,3 +258,4 @@ end end end +# rubocop:enable RSpec/DescribeClass diff --git a/spec/open_api/configuration_spec.rb b/spec/open_api/configuration_spec.rb index 200f293..0d56cf0 100644 --- a/spec/open_api/configuration_spec.rb +++ b/spec/open_api/configuration_spec.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require 'spec_helper' +require 'tempfile' +require 'fileutils' RSpec.describe StructuredParams::OpenApi::Configuration do after { StructuredParams::OpenApi.reset_configuration! } @@ -64,4 +66,90 @@ expect(StructuredParams::OpenApi.configuration.ignored?(:missing_attribute, 'legacy_field')).to be true end end + + describe '#ignore_drift' do + it 'rejects an unknown kind so a typo cannot silently never match' do + expect { StructuredParams::OpenApi.configuration.ignore_drift(path: 'a', kind: :type_mismtach) } + .to raise_error(ArgumentError, /unknown kind/) + end + + it 'scopes a rule to the given parameter class' do + configuration = StructuredParams::OpenApi.configuration + configuration.ignore_drift(path: 'a', parameter_class: 'ScopedParams') + + expect(configuration.ignored?(:type_mismatch, 'a', 'ScopedParams')).to be true + expect(configuration.ignored?(:type_mismatch, 'a', 'OtherParams')).to be false + expect(configuration.ignored?(:type_mismatch, 'a')).to be false + end + end + + describe '#pointer_for' do + subject(:configuration) { described_class.new } + + it 'maps class names to component pointers by the default convention' do + expect(configuration.pointer_for(stub_const('Api::UserParams', Class.new))).to eq('/components/schemas/User') + expect(configuration.pointer_for(stub_const('CreateUserForm', Class.new))).to eq('/components/schemas/CreateUser') + expect(configuration.pointer_for(stub_const('Plain', Class.new))).to eq('/components/schemas/Plain') + end + + it 'passes JSON Pointers through and escapes component names' do + expect(configuration.pointer_for(Class.new, '/paths/x')).to eq('/paths/x') + expect(configuration.pointer_for(Class.new, '#/paths/x')).to eq('#/paths/x') + expect(configuration.pointer_for(Class.new, 'a/b~c')).to eq('/components/schemas/a~1b~0c') + end + + it 'uses a custom schema_name_resolver' do + configuration.schema_name_resolver = ->(klass) { "#{klass.name}Request" } + + expect(configuration.pointer_for(stub_const('Thing', Class.new))).to eq('/components/schemas/ThingRequest') + end + end + + describe '#parameter_classes' do + it 'is empty without a filter and ignores anonymous classes' do + configuration = described_class.new + expect(configuration.parameter_classes).to eq([]) + + Class.new(StructuredParams::Params) + configuration.parameter_class_filter = ->(klass) { klass.name.start_with?('Api::') } + + expect { configuration.parameter_classes }.not_to raise_error + end + end + + describe '#load_document' do + subject(:configuration) { described_class.new } + + it 'parses a file once per modification time' do + Tempfile.create(['openapi', '.yml']) do |file| + file.write("a: 1\n") + file.flush + allow(YAML).to receive(:safe_load_file).and_call_original + + 2.times { configuration.load_document(file.path) } + expect(YAML).to have_received(:safe_load_file).once + + file.write("b: 2\n") + file.flush + FileUtils.touch(file.path, mtime: Time.now + 5) + expect(configuration.load_document(file.path)).to eq('a' => 1, 'b' => 2) + end + end + + it 'resolves YAML aliases and rejects a document that is not a mapping' do + Tempfile.create(['openapi', '.yml']) do |file| + file.write("base: &base\n a: 1\ncopy: *base\n") + file.flush + expect(configuration.load_document(file.path)['copy']).to eq('a' => 1) + end + + Tempfile.create(['empty', '.yml']) do |file| + expect { configuration.load_document(file.path) }.to raise_error(ArgumentError, /mapping/) + end + end + + it 'raises for a missing file' do + expect { configuration.load_document('/nonexistent/openapi.yml') }.to raise_error(Errno::ENOENT) + end + end end diff --git a/spec/open_api/drift_check_spec.rb b/spec/open_api/drift_check_spec.rb index 1998050..ba87368 100644 --- a/spec/open_api/drift_check_spec.rb +++ b/spec/open_api/drift_check_spec.rb @@ -166,13 +166,226 @@ expect(result).to be_valid end - it 'skips instead of raising when no schema matches the requested name' do + it 'reports a missing schema as an error instead of raising' do document = { 'components' => { 'schemas' => { 'Other' => schema } } } result = described_class.call(openapi: document, schema: 'Missing', parameter_class: parameter_class) + expect(result.errors).to contain_exactly( + have_attributes(kind: :missing_schema, path: '(root)', + message: a_string_including('no matching OpenAPI schema')) + ) + end + + it 'lets a missing schema be ignored for one class only' do + document = { 'components' => { 'schemas' => { 'Other' => schema } } } + StructuredParams::OpenApi.configure do |config| + config.ignore_drift(path: '(root)', kind: :missing_schema, parameter_class: parameter_class) + end + + result = described_class.call(openapi: document, schema: 'Missing', parameter_class: parameter_class) + other = described_class.call(openapi: document, schema: 'Missing', parameter_class: Class.new(StructuredParams::Params)) + + expect(result).to be_valid + expect(result.ignored.map(&:kind)).to eq([:missing_schema]) + expect(other).not_to be_valid + ensure + StructuredParams::OpenApi.reset_configuration! + end + + it 'reports a pointer that runs past the end of an array as a missing schema' do + document = { 'items' => [schema] } + + result = described_class.call(openapi: document, schema: '/items/5', parameter_class: parameter_class) + + expect(result.errors.map(&:kind)).to eq([:missing_schema]) + end + + it 'terminates on a self-referencing schema' do + node_class = Class.new(StructuredParams::Params) + node_class.attribute :name, :string + node_class.attribute :children, :array, value_class: node_class + document = { + 'components' => { 'schemas' => { 'Node' => { + 'type' => 'object', + 'properties' => { + 'name' => { 'type' => 'string' }, + 'children' => { 'type' => 'array', 'items' => { '$ref' => '#/components/schemas/Node' } } + } + } } } + } + + result = described_class.call(openapi: document, schema: 'Node', parameter_class: node_class) + + expect(result).to be_valid + end + + it 'reports a dangling nested reference as an error without extra findings' do + schema['properties']['address'] = { '$ref' => '#/components/schemas/Missing' } + document = { 'components' => { 'schemas' => { 'Request' => schema } } } + + result = described_class.call(openapi: document, schema: 'Request', parameter_class: parameter_class) + + expect(result.errors).to contain_exactly( + have_attributes(kind: :unresolved_reference, path: 'address', message: a_string_including('Missing')) + ) + expect(result.skipped).to be_empty + end + + it 'reports a circular reference chain instead of recursing forever' do + schema['properties']['address'] = { '$ref' => '#/components/schemas/Loop' } + document = { 'components' => { 'schemas' => { 'Request' => schema, + 'Loop' => { '$ref' => '#/components/schemas/Loop' } } } } + + result = described_class.call(openapi: document, schema: 'Request', parameter_class: parameter_class) + + expect(result.errors.map(&:kind)).to eq([:unresolved_reference]) + end + + it 'skips an external reference without extra findings' do + schema['properties']['address'] = { '$ref' => 'other.yml#/Address' } + + result = described_class.call(schema: schema, parameter_class: parameter_class) + expect(result).to be_valid - expect(result.skipped.first.message).to include('no matching OpenAPI schema found') + expect(result.skipped).to contain_exactly(have_attributes(path: 'address', + message: a_string_including('external'))) + end + + it 'merges sibling keys next to a reference and follows chained references' do + schema['properties']['address'] = { '$ref' => '#/components/schemas/Alias', 'description' => 'Where to ship' } + document = { + 'components' => { 'schemas' => { + 'Request' => schema, + 'Alias' => { '$ref' => '#/components/schemas/Address' }, + 'Address' => { 'type' => 'object', 'properties' => { 'city' => { 'type' => 'string' } }, + 'required' => ['city'] } + } } + } + + result = described_class.call(openapi: document, schema: 'Request', parameter_class: parameter_class) + + expect(result).to be_valid + expect(result.skipped).to be_empty + end + + it 'reports a property that has no matching attribute' do + schema['properties']['nickname'] = { 'type' => 'string' } + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result.errors).to contain_exactly(have_attributes(kind: :missing_attribute, path: 'nickname')) + end + + it 'does not accumulate findings when the same instance is called twice' do + schema['properties']['nickname'] = { 'type' => 'string' } + check = described_class.new(schema: schema, parameter_class: parameter_class) + + check.call + result = check.call + + expect(result.errors.size).to eq(1) + expect(result.errors).to be_frozen + end + + it 'skips nullable type arrays, objects without properties and arrays without items' do + schema['properties']['name'] = { 'type' => %w[string null] } + schema['properties']['address'] = { 'type' => 'object' } + schema['properties']['tags'] = { 'type' => 'array' } + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result.skipped.map(&:path)).to contain_exactly('name', 'address', 'tags') + end + + it 'skips a property whose schema is not an object' do + schema['properties']['name'] = nil + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result.skipped).to contain_exactly(have_attributes(path: 'name')) + end + + it 'reports a type mismatch for a nested object property and a required nested attribute' do + schema['properties']['address']['properties']['city'] = { 'type' => 'integer' } + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result.errors).to contain_exactly(have_attributes(kind: :type_mismatch, path: 'address.city')) + end + + it 'does not treat a conditional presence validator as unconditional' do + conditional = Class.new(StructuredParams::Params) do + attribute :name, :string + validates :name, presence: true, if: -> { true } + end + name_schema = { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string' } }, 'required' => ['name'] } + + result = described_class.call(schema: name_schema, parameter_class: conditional) + + expect(result.errors.map(&:kind)).to eq([:missing_presence_validation]) + end + + it 'accepts compatible numeric types and rejects an integer attribute for a number schema' do + numbers = Class.new(StructuredParams::Params) do + attribute :price, :decimal + attribute :count, :integer + end + number_schema = { + 'type' => 'object', + 'properties' => { 'price' => { 'type' => 'number' }, 'count' => { 'type' => 'number' } } + } + + result = described_class.call(schema: number_schema, parameter_class: numbers) + + expect(result.errors).to contain_exactly(have_attributes(kind: :type_mismatch, path: 'count')) + end + + it 'validates constructor arguments' do + expect { described_class.new(schema: schema, parameter_class: parameter_class, required_validation: :foo) } + .to raise_error(ArgumentError, /required_validation/) + expect { described_class.new(parameter_class: parameter_class) } + .to raise_error(ArgumentError, /configure an OpenAPI document/) + end + + it 'resolves array indexes in JSON Pointers' do + document = { 'components' => { 'schemas' => { 'Request' => { 'allOf' => [schema] } } } } + + result = described_class.call(openapi: document, schema: '/components/schemas/Request/allOf/0', + parameter_class: parameter_class) + + expect(result).to be_valid + end + + it 'accepts string schemas for date and time attributes' do + date_class = Class.new(StructuredParams::Params) do + attribute :born_on, :date + attribute :seen_at, :datetime + end + date_schema = { + 'type' => 'object', + 'properties' => { + 'born_on' => { 'type' => 'string', 'format' => 'date' }, + 'seen_at' => { 'type' => 'string', 'format' => 'date-time' } + } + } + + result = described_class.call(schema: date_schema, parameter_class: date_class) + + expect(result).to be_valid + end + + it 'loads YAML documents containing unquoted dates' do + document = { 'components' => { 'schemas' => { 'Request' => schema } } } + + Tempfile.create(['openapi', '.yml']) do |file| + file.write(document.to_yaml.sub('components:', "info:\n released: 2024-01-01\ncomponents:")) + file.flush + + result = described_class.call(openapi: file.path, schema: 'Request', parameter_class: parameter_class) + + expect(result).to be_valid + end end it 'prefixes the message with the parameter class name' do diff --git a/spec/open_api/railtie_loading_spec.rb b/spec/open_api/railtie_loading_spec.rb index 3e8ac61..7bba69e 100644 --- a/spec/open_api/railtie_loading_spec.rb +++ b/spec/open_api/railtie_loading_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require 'open3' -# rubocop:disable-next RSpec/DescribeClass +# rubocop:disable RSpec/DescribeClass RSpec.describe 'loading the OpenAPI drift-check rake task' do it 'does not raise when Rake is defined without its DSL mixed into main' do # Thor::Actions (used by Rails::Command, e.g. `rails runner`/`rails console`) @@ -20,3 +20,4 @@ expect(output).to include('ok') end end +# rubocop:enable RSpec/DescribeClass diff --git a/spec/open_api/registry_spec.rb b/spec/open_api/registry_spec.rb index 63404a4..9025d9d 100644 --- a/spec/open_api/registry_spec.rb +++ b/spec/open_api/registry_spec.rb @@ -63,4 +63,64 @@ expect(results).to all(be_valid) end + + describe 'skipped and ignored checks' do + let(:parameter_class) { Class.new(StructuredParams::Params) { attribute :name, :string } } + let(:skipping_schema) do + { 'type' => 'object', 'properties' => { 'name' => { 'type' => %w[string null] } } } + end + + before { described_class.register(parameter_class, schema: skipping_schema) } + + it 'does not raise for a skipped-only result but reports it' do + results = described_class.check_all! + + expect(results).to all(be_valid) + expect(described_class.report(results)).to include('OpenAPI checks skipped', 'name', 'with skipped checks') + end + + it 'raises for a skipped-only result in strict mode' do + expect { described_class.check_all!(strict: true) } + .to raise_error(StructuredParams::OpenApi::DriftError, /checks skipped/) + end + + it 'includes skips in the message when the class also has errors' do + skipping_schema['properties']['age'] = { 'type' => 'integer' } + + expect { described_class.check_all! } + .to raise_error(StructuredParams::OpenApi::DriftError, /drift detected.*checks skipped/m) + end + + it 'joins the messages of every failing class' do + other = Class.new(StructuredParams::Params) { attribute :id, :integer } + described_class.register(other, schema: { 'type' => 'object', 'properties' => { 'x' => { 'type' => 'string' } } }) + skipping_schema['properties']['age'] = { 'type' => 'integer' } + + expect { described_class.check_all! } + .to raise_error(StructuredParams::OpenApi::DriftError) { |error| expect(error.message.scan('drift detected').size).to eq(2) } + end + end + + describe '.report' do + it 'warns when nothing was checked and lists ignored findings' do + expect(described_class.report([])).to include('No parameter classes were checked') + + parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } + StructuredParams::OpenApi.configure { |config| config.ignore_drift(path: 'age') } + described_class.register(parameter_class, + schema: { 'type' => 'object', 'properties' => { 'age' => { 'type' => 'integer' } } }) + + report = described_class.report(described_class.check_all) + + expect(report).to include('OpenAPI findings ignored', 'age') + end + end + + it 'passes register options through to the check' do + parameter_class = Class.new(StructuredParams::Params) { attribute :user_id, :integer } + schema = { 'type' => 'object', 'properties' => { 'userId' => { 'type' => 'integer' } } } + described_class.register(parameter_class, schema: schema, property_names: { user_id: 'userId' }) + + expect(described_class.check_all).to all(be_valid) + end end diff --git a/spec/permit_spec.rb b/spec/permit_spec.rb index 4bb3de6..9cc314b 100644 --- a/spec/permit_spec.rb +++ b/spec/permit_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -# rubocop:disable-next RSpec/DescribeClass +# rubocop:disable RSpec/DescribeClass RSpec.describe 'StructuredParams::Params.permit' do describe 'Form Object context (with nested params structure)' do let(:params) do @@ -79,7 +79,7 @@ ) end - # rubocop:disable-next RSpec/MultipleExpectations + # rubocop:disable RSpec/MultipleExpectations it 'permits array parameters' do permitted = UserParameter.permit(params) @@ -90,6 +90,7 @@ expect(permitted[:hobbies][0][:extra]).to be_nil expect(permitted[:tags]).to eq(%w[tag1 tag2 tag3]) end + # rubocop:enable RSpec/MultipleExpectations end context 'with namespaced form' do @@ -149,7 +150,7 @@ ) end - # rubocop:disable-next RSpec/MultipleExpectations + # rubocop:disable RSpec/MultipleExpectations it 'permits nested object parameters without require' do permitted = UserParameter.permit(params, require: false) @@ -160,6 +161,7 @@ expect(permitted[:address][:extra]).to be_nil expect(permitted[:extra_field]).to be_nil end + # rubocop:enable RSpec/MultipleExpectations end context 'with arrays' do @@ -177,7 +179,7 @@ ) end - # rubocop:disable-next RSpec/MultipleExpectations + # rubocop:disable RSpec/MultipleExpectations it 'permits array parameters without require' do permitted = UserParameter.permit(params, require: false) @@ -189,6 +191,7 @@ expect(permitted[:tags]).to eq(%w[tag1 tag2 tag3]) expect(permitted[:extra_field]).to be_nil end + # rubocop:enable RSpec/MultipleExpectations end context 'when user manually extracts nested params' do @@ -214,3 +217,4 @@ end end end +# rubocop:enable RSpec/DescribeClass diff --git a/spec/tasks/structured_params_open_api_spec.rb b/spec/tasks/structured_params_open_api_spec.rb index 37532ad..0113ee2 100644 --- a/spec/tasks/structured_params_open_api_spec.rb +++ b/spec/tasks/structured_params_open_api_spec.rb @@ -26,4 +26,33 @@ expect { drift_check_task.invoke } .to raise_error(StructuredParams::OpenApi::DriftError, /drift detected/) end + + context 'when every check passes' do + before do + described_class.define_task(:environment) + parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } + schema = { 'type' => 'object', 'properties' => { 'name' => { 'type' => %w[string null] } } } + StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) + end + + it 'prints skipped checks without failing' do + expect { drift_check_task.invoke }.to output(/OpenAPI checks skipped.*Checked 1 classes/m).to_stdout + end + + it 'fails on skipped checks when STRUCTURED_PARAMS_STRICT=1' do + stub_const('ENV', ENV.to_hash.merge('STRUCTURED_PARAMS_STRICT' => '1')) + + expect { drift_check_task.invoke }.to raise_error(StructuredParams::OpenApi::DriftError, /checks skipped/) + end + end + + it 'eager loads the Rails application before checking' do + described_class.define_task(:environment) + application = double(eager_load!: nil) + stub_const('Rails', Module.new) + allow(Rails).to receive(:application).and_return(application) + + expect { drift_check_task.invoke }.to output(/No parameter classes were checked/).to_stdout + expect(application).to have_received(:eager_load!) + end end From 5c309f225e08c0026eddd3dc7975aa7ff00100ca Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 10:25:25 +0900 Subject: [PATCH 08/12] fix: treat conditional presence validators as not covering required; bump mise Ruby - on:, allow_nil and allow_blank no longer count as unconditional presence - pin mise Ruby to 4.0.7 to match CI Co-Authored-By: Claude Sonnet 5.5 --- .mise.toml | 2 +- docs/openapi-drift-check.md | 4 ++-- lib/structured_params/open_api/drift_check.rb | 5 ++++- .../open_api/drift_check.rbs | 3 +++ spec/open_api/drift_check_spec.rb | 21 +++++++++++-------- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/.mise.toml b/.mise.toml index f3d14c8..6327a9e 100644 --- a/.mise.toml +++ b/.mise.toml @@ -1,2 +1,2 @@ [tools] -ruby = "4.0.5" +ruby = "4.0.7" diff --git a/docs/openapi-drift-check.md b/docs/openapi-drift-check.md index 8bb3152..9983eff 100644 --- a/docs/openapi-drift-check.md +++ b/docs/openapi-drift-check.md @@ -56,8 +56,8 @@ reported as a `type_mismatch`. The checker compares schema properties with `attribute` declarations, scalar and nested object/array types, and schema `required` properties with unconditional -`validates ..., presence: true` declarations ("unconditional" means no `if:` or -`unless:` option; `on:` and `allow_nil` are not considered). Use `property_names` when the JSON +`validates ..., presence: true` declarations ("unconditional" means none of the +`if:`, `unless:`, `on:`, `allow_nil:` or `allow_blank:` options). Use `property_names` when the JSON property name differs from the Ruby attribute name. ```ruby diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb index b4741a7..e493374 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/lib/structured_params/open_api/drift_check.rb @@ -28,6 +28,9 @@ class DriftCheck missing_schema unresolved_reference ].freeze #: Array[Symbol] + # Options that stop a presence validator from enforcing presence on every validation. + CONDITIONAL_PRESENCE_OPTIONS = %i[if unless on allow_nil allow_blank].freeze #: Array[Symbol] + Issue = Struct.new( :kind, #: Symbol :path, #: String @@ -342,7 +345,7 @@ def compatible_types?(schema_type, attribute_type) #: (singleton(StructuredParams::Params), Symbol) -> bool def presence_validator?(parameter_class, attribute) parameter_class._validators.fetch(attribute.to_sym, []).any? do |validator| - validator.kind == :presence && validator.options.slice(:if, :unless).empty? + validator.kind == :presence && validator.options.slice(*CONDITIONAL_PRESENCE_OPTIONS).empty? end end diff --git a/sig/structured_params/open_api/drift_check.rbs b/sig/structured_params/open_api/drift_check.rbs index d2a5b0a..b47a258 100644 --- a/sig/structured_params/open_api/drift_check.rbs +++ b/sig/structured_params/open_api/drift_check.rbs @@ -31,6 +31,9 @@ module StructuredParams # Finding kinds that can be raised as errors (and therefore matched by +ignore_drift+). ERROR_KINDS: Array[Symbol] + # Options that stop a presence validator from enforcing presence on every validation. + CONDITIONAL_PRESENCE_OPTIONS: Array[Symbol] + class Issue < Struct[Symbol | String] attr_accessor kind(): Symbol diff --git a/spec/open_api/drift_check_spec.rb b/spec/open_api/drift_check_spec.rb index ba87368..0f68c13 100644 --- a/spec/open_api/drift_check_spec.rb +++ b/spec/open_api/drift_check_spec.rb @@ -314,16 +314,19 @@ expect(result.errors).to contain_exactly(have_attributes(kind: :type_mismatch, path: 'address.city')) end - it 'does not treat a conditional presence validator as unconditional' do - conditional = Class.new(StructuredParams::Params) do - attribute :name, :string - validates :name, presence: true, if: -> { true } + [{ if: -> { true } }, { on: :create }, { allow_nil: true }, { allow_blank: true }].each do |options| + it "does not treat a presence validator with #{options.keys.first}: as unconditional" do + conditional = Class.new(StructuredParams::Params) do + attribute :name, :string + validates :name, presence: true, **options + end + name_schema = { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string' } }, + 'required' => ['name'] } + + result = described_class.call(schema: name_schema, parameter_class: conditional) + + expect(result.errors.map(&:kind)).to eq([:missing_presence_validation]) end - name_schema = { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string' } }, 'required' => ['name'] } - - result = described_class.call(schema: name_schema, parameter_class: conditional) - - expect(result.errors.map(&:kind)).to eq([:missing_presence_validation]) end it 'accepts compatible numeric types and rejects an integer attribute for a number schema' do From 04352c8ec2a27537c48e0600b0c5186ecc518627 Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 10:26:47 +0900 Subject: [PATCH 09/12] style: use disable-next for single-statement RuboCop directives Newer RuboCop (Style/DirectiveScope) requires disable-next where a disable/enable pair wraps a single statement. Co-Authored-By: Claude Sonnet 5.5 --- lib/structured_params/errors.rb | 3 +-- lib/structured_params/open_api/drift_check.rb | 6 ++---- lib/structured_params/params.rb | 3 +-- sig/structured_params/errors.rbs | 2 +- sig/structured_params/open_api/drift_check.rbs | 4 ++-- sig/structured_params/params.rbs | 2 +- spec/errors_spec.rb | 3 +-- spec/form_object_spec.rb | 3 +-- spec/open_api/railtie_loading_spec.rb | 3 +-- spec/permit_spec.rb | 12 ++++-------- 10 files changed, 15 insertions(+), 26 deletions(-) diff --git a/lib/structured_params/errors.rb b/lib/structured_params/errors.rb index cac5360..0e39904 100644 --- a/lib/structured_params/errors.rb +++ b/lib/structured_params/errors.rb @@ -1,7 +1,7 @@ # rbs_inline: enabled # frozen_string_literal: true -# rubocop:disable Style/OptionalBooleanParameter +# rubocop:disable-next Style/OptionalBooleanParameter module StructuredParams # Custom errors collection that handles nested attribute names class Errors < ActiveModel::Errors @@ -65,4 +65,3 @@ def build_nested_hash(target_hash, flat_hash, separator = '.') end end end -# rubocop:enable Style/OptionalBooleanParameter diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb index e493374..8109abe 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/lib/structured_params/open_api/drift_check.rb @@ -8,7 +8,7 @@ module StructuredParams # Schema comparison types for OpenAPI drift checks. module OpenApi # Compares OpenAPI object schemas with StructuredParams declarations. - # rubocop:disable Metrics/ClassLength + # rubocop:disable-next Metrics/ClassLength class DriftCheck # @rbs @openapi_source: Hash[untyped, untyped]|String? # @rbs @schema_source: Hash[untyped, untyped]|String? @@ -226,7 +226,7 @@ def check_object(schema, parameter_class, path) end #: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void - # rubocop:disable Metrics/AbcSize + # rubocop:disable-next Metrics/AbcSize def check_object_properties(schema, parameter_class, path) properties = value_for(schema, 'properties') unless properties.is_a?(Hash) @@ -244,7 +244,6 @@ def check_object_properties(schema, parameter_class, path) attribute_types.fetch(attribute.to_s), join_path(path, attribute)) end end - # rubocop:enable Metrics/AbcSize #: (untyped, ActiveModel::Type::Value, String) -> void def compare_property(property_schema, attribute_type, path) @@ -468,6 +467,5 @@ def skip(path, message) @skipped << Issue.new(kind: :unsupported, path: display_path(path), message: message) end end - # rubocop:enable Metrics/ClassLength end end diff --git a/lib/structured_params/params.rb b/lib/structured_params/params.rb index 02ab255..73465ab 100644 --- a/lib/structured_params/params.rb +++ b/lib/structured_params/params.rb @@ -50,7 +50,7 @@ module StructuredParams # <%= f.text_field :name %> # <%= f.text_field :email %> # <% end %> - # rubocop:disable Metrics/ClassLength + # rubocop:disable-next Metrics/ClassLength class Params include ActiveModel::Model include ActiveModel::Attributes @@ -347,5 +347,4 @@ def import_structured_errors(structured_errors, prefix) end end end - # rubocop:enable Metrics/ClassLength end diff --git a/sig/structured_params/errors.rbs b/sig/structured_params/errors.rbs index b415827..a738cf4 100644 --- a/sig/structured_params/errors.rbs +++ b/sig/structured_params/errors.rbs @@ -1,6 +1,6 @@ # Generated from lib/structured_params/errors.rb with RBS::Inline -# rubocop:disable Style/OptionalBooleanParameter +# rubocop:disable-next Style/OptionalBooleanParameter module StructuredParams # Custom errors collection that handles nested attribute names class Errors < ActiveModel::Errors diff --git a/sig/structured_params/open_api/drift_check.rbs b/sig/structured_params/open_api/drift_check.rbs index b47a258..fd94f34 100644 --- a/sig/structured_params/open_api/drift_check.rbs +++ b/sig/structured_params/open_api/drift_check.rbs @@ -4,7 +4,7 @@ module StructuredParams # Schema comparison types for OpenAPI drift checks. module OpenApi # Compares OpenAPI object schemas with StructuredParams declarations. - # rubocop:disable Metrics/ClassLength + # rubocop:disable-next Metrics/ClassLength class DriftCheck @active_pairs: Array[Array[untyped]] @@ -112,7 +112,7 @@ module StructuredParams def check_object: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void # : (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void - # rubocop:disable Metrics/AbcSize + # rubocop:disable-next Metrics/AbcSize def check_object_properties: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void # : (untyped, ActiveModel::Type::Value, String) -> void diff --git a/sig/structured_params/params.rbs b/sig/structured_params/params.rbs index 55aa1e7..42a93a3 100644 --- a/sig/structured_params/params.rbs +++ b/sig/structured_params/params.rbs @@ -49,7 +49,7 @@ module StructuredParams # <%= f.text_field :name %> # <%= f.text_field :email %> # <% end %> - # rubocop:disable Metrics/ClassLength + # rubocop:disable-next Metrics/ClassLength class Params include ActiveModel::Model diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index c0fba93..6ffa985 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -106,7 +106,7 @@ errors.add('items.1.subitems.0.description', 'is too long') end - # rubocop:disable RSpec/ExampleLength + # rubocop:disable-next RSpec/ExampleLength it 'creates deep nested structure' do expect(errors.to_hash(false, structured: true)).to eq({ items: { @@ -127,7 +127,6 @@ } }) end - # rubocop:enable RSpec/ExampleLength end context 'with mixed flat and nested attributes' do diff --git a/spec/form_object_spec.rb b/spec/form_object_spec.rb index 4d318c7..608320f 100644 --- a/spec/form_object_spec.rb +++ b/spec/form_object_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -# rubocop:disable RSpec/DescribeClass +# rubocop:disable-next RSpec/DescribeClass RSpec.describe 'StructuredParams::Params as Form Object' do describe '.form_class?' do it 'returns true for classes with a Form suffix' do @@ -258,4 +258,3 @@ end end end -# rubocop:enable RSpec/DescribeClass diff --git a/spec/open_api/railtie_loading_spec.rb b/spec/open_api/railtie_loading_spec.rb index 7bba69e..3e8ac61 100644 --- a/spec/open_api/railtie_loading_spec.rb +++ b/spec/open_api/railtie_loading_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require 'open3' -# rubocop:disable RSpec/DescribeClass +# rubocop:disable-next RSpec/DescribeClass RSpec.describe 'loading the OpenAPI drift-check rake task' do it 'does not raise when Rake is defined without its DSL mixed into main' do # Thor::Actions (used by Rails::Command, e.g. `rails runner`/`rails console`) @@ -20,4 +20,3 @@ expect(output).to include('ok') end end -# rubocop:enable RSpec/DescribeClass diff --git a/spec/permit_spec.rb b/spec/permit_spec.rb index 9cc314b..4bb3de6 100644 --- a/spec/permit_spec.rb +++ b/spec/permit_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -# rubocop:disable RSpec/DescribeClass +# rubocop:disable-next RSpec/DescribeClass RSpec.describe 'StructuredParams::Params.permit' do describe 'Form Object context (with nested params structure)' do let(:params) do @@ -79,7 +79,7 @@ ) end - # rubocop:disable RSpec/MultipleExpectations + # rubocop:disable-next RSpec/MultipleExpectations it 'permits array parameters' do permitted = UserParameter.permit(params) @@ -90,7 +90,6 @@ expect(permitted[:hobbies][0][:extra]).to be_nil expect(permitted[:tags]).to eq(%w[tag1 tag2 tag3]) end - # rubocop:enable RSpec/MultipleExpectations end context 'with namespaced form' do @@ -150,7 +149,7 @@ ) end - # rubocop:disable RSpec/MultipleExpectations + # rubocop:disable-next RSpec/MultipleExpectations it 'permits nested object parameters without require' do permitted = UserParameter.permit(params, require: false) @@ -161,7 +160,6 @@ expect(permitted[:address][:extra]).to be_nil expect(permitted[:extra_field]).to be_nil end - # rubocop:enable RSpec/MultipleExpectations end context 'with arrays' do @@ -179,7 +177,7 @@ ) end - # rubocop:disable RSpec/MultipleExpectations + # rubocop:disable-next RSpec/MultipleExpectations it 'permits array parameters without require' do permitted = UserParameter.permit(params, require: false) @@ -191,7 +189,6 @@ expect(permitted[:tags]).to eq(%w[tag1 tag2 tag3]) expect(permitted[:extra_field]).to be_nil end - # rubocop:enable RSpec/MultipleExpectations end context 'when user manually extracts nested params' do @@ -217,4 +214,3 @@ end end end -# rubocop:enable RSpec/DescribeClass From 90c12d3c10a94d2263c981c9a0f2273de270ae4a Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 19:33:21 +0900 Subject: [PATCH 10/12] feat: support nullable/oneOf, several OpenAPI files and finer ignore rules OpenAPI drift check improvements: - document_resolver and Registry.register(openapi:) check classes against several OpenAPI files - ignore_drift accepts any of path:/kind:/parameter_class: and validates kind - required properties that are nullable need no presence validation - OpenAPI 3.1 nullable forms (oneOf/anyOf with null, type arrays with null) and scalar oneOf/anyOf are compared instead of skipped - presence validators with on:/allow_nil/allow_blank no longer count as unconditional - missing schemas and unresolved references are errors; skips are printed by the Rake task, with a strict mode - split DriftCheck::Issue and DriftCheck::Result into their own files - reorganize the guide into sections with a table of contents, add a Japanese version and a realistic quick start Co-Authored-By: Claude Sonnet 5.5 --- README_ja.md | 4 +- docs/openapi-drift-check.md | 343 ++++++++++++---- docs/openapi-drift-check_ja.md | 379 ++++++++++++++++++ .../open_api/configuration.rb | 31 +- lib/structured_params/open_api/drift_check.rb | 183 ++++++--- .../open_api/drift_check/issue.rb | 16 + .../open_api/drift_check/result.rb | 61 +++ lib/structured_params/open_api/registry.rb | 7 +- .../open_api/configuration.rbs | 24 +- .../open_api/drift_check.rbs | 92 +++-- .../open_api/drift_check/issue.rbs | 19 + .../open_api/drift_check/result.rbs | 40 ++ sig/structured_params/open_api/registry.rbs | 3 + spec/open_api/configuration_spec.rb | 40 ++ spec/open_api/drift_check/result_spec.rb | 76 ++++ spec/open_api/drift_check_spec.rb | 81 +++- spec/open_api/registry_spec.rb | 38 +- spec/tasks/structured_params_open_api_spec.rb | 2 +- 18 files changed, 1239 insertions(+), 200 deletions(-) create mode 100644 docs/openapi-drift-check_ja.md create mode 100644 lib/structured_params/open_api/drift_check/issue.rb create mode 100644 lib/structured_params/open_api/drift_check/result.rb create mode 100644 sig/structured_params/open_api/drift_check/issue.rbs create mode 100644 sig/structured_params/open_api/drift_check/result.rbs create mode 100644 spec/open_api/drift_check/result_spec.rb diff --git a/README_ja.md b/README_ja.md index 1d7ef64..d39cd16 100644 --- a/README_ja.md +++ b/README_ja.md @@ -130,14 +130,14 @@ bundle exec rake structured_params:openapi:drift_check filter に一致する新しい class は自動で検査されます。component 名の例外指定、 property mapping、対応する schema 型の詳細は -[OpenAPI Drift Check](docs/openapi-drift-check.md) を参照してください。 +[OpenAPI Drift Check](docs/openapi-drift-check_ja.md) を参照してください。 ## ドキュメント - **[インストールとセットアップ](docs/installation.md)** - StructuredParams の始め方 - **[基本的な使い方](docs/basic-usage.md)** - パラメータクラス、ネストオブジェクト、配列 - **[バリデーション](docs/validation.md)** - ネスト構造での ActiveModel バリデーション -- **[OpenAPI Drift Check](docs/openapi-drift-check.md)** - OpenAPI schema と Params class の乖離検出 +- **[OpenAPI Drift Check](docs/openapi-drift-check_ja.md)** - OpenAPI schema と Params class の乖離検出 - **[Strong Parameters](docs/strong-parameters.md)** - permit リストの自動生成 - **[エラーハンドリング](docs/error-handling.md)** - フラット形式と構造化形式のエラー - **[シリアライゼーション](docs/serialization.md)** - パラメータのハッシュと JSON 変換 diff --git a/docs/openapi-drift-check.md b/docs/openapi-drift-check.md index 9983eff..b515d3d 100644 --- a/docs/openapi-drift-check.md +++ b/docs/openapi-drift-check.md @@ -1,35 +1,148 @@ # OpenAPI Drift Check -Configure the application-wide OpenAPI document once and select the namespace of -request parameter classes to check. The Rake task discovers these classes after -Rails eager loads the application. +English | [日本語](openapi-drift-check_ja.md) + +Detects differences between OpenAPI request schemas and `StructuredParams::Params` +classes, so a change on one side cannot silently diverge from the other. + +## Table of contents + +- [Terms](#terms) +- [Quick start](#quick-start) +- [Matching classes to schemas](#matching-classes-to-schemas) + - [Default naming convention](#default-naming-convention) + - [Custom naming convention](#custom-naming-convention) + - [Exceptions for a single class](#exceptions-for-a-single-class) +- [What is compared](#what-is-compared) + - [Scalar types](#scalar-types) + - [Attributes and required properties](#attributes-and-required-properties) + - [Property names](#property-names) +- [Using it in RSpec](#using-it-in-rspec) +- [Ignoring a specific finding](#ignoring-a-specific-finding) +- [Rake task and Registry](#rake-task-and-registry) + - [Auto-discovery](#auto-discovery) + - [Registering exceptions](#registering-exceptions) + - [Multiple OpenAPI files](#multiple-openapi-files) + - [Running in CI](#running-in-ci) +- [Scope, skips and missing schemas](#scope-skips-and-missing-schemas) + +## Terms + +In this guide, a **component** means a named schema under `components.schemas` in the +OpenAPI document. Each `Params` class is checked against one component, found by name +(for example `CreateUserParams` → `CreateUser`). + +```yaml +# openapi.yml +components: + schemas: + CreateUser: # <- a "component" (its JSON Pointer is #/components/schemas/CreateUser) + type: object + properties: + name: { type: string } +``` + +## Quick start + +Add an initializer and keep only the parts you need. The OpenAPI document here is +generated from TypeSpec, so its components are named like `V1.Orders.CreateRequest`, for +classes named like `Api::V1::Orders::CreateRequestParameter`. ```ruby +# config/initializers/structured_params_openapi.rb StructuredParams::OpenApi.configure do |config| - config.document = Rails.root.join('openapi.yml').to_s - config.parameter_class_filter = ->(parameter_class) do - parameter_class.name&.start_with?('Api::') + # Required: the OpenAPI document and the classes to check. + config.document = Rails.root.join('docs/api/openapi.yaml').to_s + config.parameter_class_filter = ->(klass) { klass.name&.start_with?('Api::') } + + # Only if your component names differ from the default convention + # (CreateUserParams -> CreateUser): + # Api::V1::Orders::CreateRequestParameter -> "V1.Orders.CreateRequest" + config.schema_name_resolver = lambda do |klass| + klass.name.delete_prefix('Api::').delete_suffix('Parameter').gsub('::', '.') + end + + # Only for classes that should have a schema but do not yet: + # record them instead of failing. + %w[ + Api::V1::Reports::ExportRequestParameter + Api::V1::Reports::SummaryRequestParameter + ].each do |name| + config.ignore_drift(kind: :missing_schema, parameter_class: name) end end + +# Only for classes whose component name does not follow the convention above. +{ + 'Api::V1::Orders::KonbiniRequestParameter' => 'V1.Orders.CreateKonbiniRequest', + 'Api::V1::Orders::BankTransferRequestParameter' => 'V1.Orders.CreateBankTransferRequest' +}.each do |parameter_class, schema| + StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) +end ``` +- `parameter_class_filter` selects the classes to check, so a new `Api::*` class is + picked up without registration. +- `schema_name_resolver` maps every class to its component by one rule. +- `ignore_drift` records a class that has no schema yet, and it stays listed in the Rake + output. A class that is never meant to be checked is better left out of the filter + (see [Scope, skips and missing schemas](#scope-skips-and-missing-schemas)). +- `Registry.register` covers the few classes that break the naming rule. + +Then run the Rake task to see what is left to fix: + ```sh bundle exec rake structured_params:openapi:drift_check ``` -By default, `CreateUserParams` is matched with -`#/components/schemas/CreateUser`: the class's namespace and a trailing -`Param`, `Params`, `Parameter`, `Parameters`, or `Form` are removed. Override the -convention globally when necessary. +## Matching classes to schemas + +### Default naming convention + +By default, `CreateUserParams` is matched with `#/components/schemas/CreateUser`: the +class's namespace and a trailing `Param`, `Params`, `Parameter`, `Parameters`, or `Form` +are removed. + +### Custom naming convention + +If your components follow a different naming convention, override it globally with +`schema_name_resolver`. For example, when the OpenAPI document names the components +for `Api::V1::CreateUserParams` and `Api::V2::CreateUserParams` as `V1CreateUserRequest` +and `V2CreateUserRequest`: + +```yaml +# openapi.yml +components: + schemas: + V1CreateUserRequest: # <- the default convention would look for CreateUser + type: object + properties: + name: { type: string } + V2CreateUserRequest: ... +``` ```ruby StructuredParams::OpenApi.configure do |config| config.schema_name_resolver = ->(parameter_class) do - "#{parameter_class.name.demodulize.delete_suffix('Params')}Request" + # Api::V1::CreateUserParams -> "V1CreateUser" -> "V1CreateUserRequest" + name = parameter_class.name.delete_prefix('Api::').delete_suffix('Params').gsub('::', '') + "#{name}Request" end end ``` +The resolver returns a component name, which replaces the default convention for every +class. Only the suffix `Params` is removed here, so a `CreateUserForm` would become +`CreateUserFormRequest`. + +Keep the namespace in the returned name. If you drop it (for example with `demodulize`), +`Api::V1::CreateUserParams` and `Api::V2::CreateUserParams` both look for +`CreateUserRequest`, which does not exist, and report `missing_schema`. If each version +lives in its own OpenAPI file instead, keep the component name as is and switch files with +`document_resolver` (see [Multiple OpenAPI files](#multiple-openapi-files)). + +### Exceptions for a single class + For an exceptional class, pass its component name as `schema:`. It also accepts a JSON Pointer (`'/components/schemas/CreateUserRequest'`, `'#/…'`) or an inline Hash. `pointer:` selects a pointer explicitly, and `openapi:` overrides the configured @@ -42,7 +155,9 @@ StructuredParams::OpenApi::DriftCheck.call( ) ``` -Scalar types are compared as follows: +## What is compared + +### Scalar types | Schema `type` | Accepted attribute types | |---------------|--------------------------| @@ -52,13 +167,34 @@ Scalar types are compared as follows: | `boolean`, `object`, `array` | the same type | A schema with neither `type` nor `properties` (for example an enum-only schema) is -reported as a `type_mismatch`. +reported as a `type_mismatch`. Nested object and array types are compared recursively. + +### Attributes and required properties + +Schema properties are compared with `attribute` declarations. + +Schema `required` is compared with presence validation, and this mapping is only an +approximation. OpenAPI's `required` means one thing: the key must exist. In Rails, +a missing key and a `nil` value both leave the attribute `nil`, so no validator can +express "the key exists", and nothing says whether `presence`, `allow_nil` or +`allow_blank` was intended. The check therefore uses an unconditional +`validates ..., presence: true` as a stand-in ("unconditional" means none of the `if:`, +`unless:`, `on:`, `allow_nil:` or `allow_blank:` options). + +A required property that the schema allows to be null (`nullable: true`, a `type` array +including `'null'`, or a `oneOf`/`anyOf` with a `null` option) needs no `presence`, since +`presence` would reject the null the contract permits, so it is not reported. + +A `presence` that carries one of those options may let a missing key through, so the +check cannot confirm it satisfies `required` and reports `missing_presence_validation`. +That is not necessarily a bug. When the Ruby side is intentional, accept it per +property with [`ignore_drift`](#ignoring-a-specific-finding). When `presence` is +too strict for the whole API (it also rejects `false` and `""`, which `required` allows), +pass `required_validation: :none` and check required-key behavior separately. -The checker compares schema properties with `attribute` declarations, scalar and -nested object/array types, and schema `required` properties with unconditional -`validates ..., presence: true` declarations ("unconditional" means none of the -`if:`, `unless:`, `on:`, `allow_nil:` or `allow_blank:` options). Use `property_names` when the JSON -property name differs from the Ruby attribute name. +### Property names + +Use `property_names` when the JSON property name differs from the Ruby attribute name. ```ruby StructuredParams::OpenApi::DriftCheck.call( @@ -67,8 +203,9 @@ StructuredParams::OpenApi::DriftCheck.call( ) ``` -In RSpec, use the result as the failure message so a CI failure lists every -detected difference. +## Using it in RSpec + +Use the result as the failure message so a CI failure lists every detected difference. ```ruby it 'matches the request schema' do @@ -80,11 +217,6 @@ it 'matches the request schema' do end ``` -`required` means that a JSON key must be present, whereas ActiveModel's `presence` -validator also rejects blank values (including `false`). For APIs where those -semantics are intentionally different, pass `required_validation: :none` and check -required-key behavior separately. - ## Ignoring a specific finding `required_validation: :none` and the Registry both operate on a whole class, so they @@ -97,27 +229,42 @@ StructuredParams::OpenApi.configure do |config| end ``` -This applies wherever that path occurs, including once per parent class for a nested -object reused by several top-level parameter classes. Omit `kind:` to ignore every -error kind at that path. Pass `parameter_class:` (a class or its name) to limit a rule -to one class. Valid kinds are `missing_attribute`, `unexpected_attribute`, -`missing_presence_validation`, `type_mismatch`, `missing_schema` and -`unresolved_reference`; anything else raises `ArgumentError`. Paths use attribute names -joined with `.`, with `[]` for array items (for example `items[].sku`), and a schema -that cannot be found at all is reported at `(root)`. Skips are never ignored. Ignored findings move to `result.ignored` instead of -`result.errors`, so `valid?` treats them as resolved and they don't appear in -`to_message` — but they stay available on the result object for anyone who wants to -audit what's being suppressed and why, rather than disappearing silently. - -## Rake task - -The gem provides `structured_params:openapi:drift_check` in Rails applications. -The task automatically checks classes selected by `parameter_class_filter` (without a -filter, only registered classes are checked, and the task warns when nothing was -checked), so a new `Api::*Params` class needs no registration. Anonymous classes are -never auto-discovered, which is also why the filter example guards with `name&.`. Use the Registry only for an -exceptional component name or property mapping; explicit registrations override -auto-discovery for the same class. +- The rule applies wherever that path occurs, including once per parent class for a + nested object reused by several top-level parameter classes. +- Omit `kind:` to ignore every error kind at that path. Valid kinds are + `missing_attribute`, `unexpected_attribute`, `missing_presence_validation`, + `type_mismatch`, `missing_schema` and `unresolved_reference`; anything else raises + `ArgumentError`. +- Pass `parameter_class:` (a class or its name) to limit a rule to one class. +- Paths use attribute names joined with `.`, with `[]` for array items (for example + `items[].sku`). Each of `path:`, `kind:` and `parameter_class:` narrows a rule and an + omitted one matches everything, but at least one is required. A missing schema + (`missing_schema`) concerns the whole class rather than a property, so omit `path:`. +- Skips are never ignored. + +Ignored findings move to `result.ignored` instead of `result.errors`, so `valid?` +treats them as resolved and they don't appear in `to_message` — but they stay available +on the result object for anyone who wants to audit what's being suppressed and why, +rather than disappearing silently. + +## Rake task and Registry + +### Auto-discovery + +The gem provides `structured_params:openapi:drift_check` in Rails applications. The task +automatically checks classes selected by `parameter_class_filter`, so a new +`Api::*Params` class needs no registration. + +- Without a filter, only registered classes are checked, and the task warns when + nothing was checked. +- Anonymous classes are never auto-discovered, which is also why the filter example + guards with `name&.`. + +### Registering exceptions + +Use the Registry only for an exceptional component name or property mapping; explicit +registrations override auto-discovery for the same class, and a later registration for +the same class replaces an earlier one. ```ruby StructuredParams::OpenApi::Registry.register( @@ -126,10 +273,10 @@ StructuredParams::OpenApi::Registry.register( ) ``` -`parameter_class` may also be a class name String, which is resolved lazily when -the task runs. This is useful for registering from a plain `config/initializers/*.rb` -file, where Rails hasn't finished setting up autoloading yet and referencing the -class constant directly would raise `NameError`. +`parameter_class` may also be a class name String, which is resolved lazily when the +task runs. This is useful for registering from a plain `config/initializers/*.rb` file, +where Rails hasn't finished setting up autoloading yet and referencing the class +constant directly would raise `NameError`. ```ruby StructuredParams::OpenApi::Registry.register( @@ -138,37 +285,95 @@ StructuredParams::OpenApi::Registry.register( ) ``` -Run it in CI with: +### Multiple OpenAPI files + +When an application has several OpenAPI files (for example `admin_api.yml` and +`front_api.yml`), set `document_resolver` so each auto-discovered class is checked +against its own file. Return `nil` to fall back to `config.document`. + +```ruby +StructuredParams::OpenApi.configure do |config| + config.parameter_class_filter = ->(klass) { klass.name&.match?(/\A(AdminApi|FrontApi)::/) } + config.document_resolver = ->(klass) do + case klass.name + when /\AAdminApi::/ then Rails.root.join('openapi/admin_api.yml').to_s + when /\AFrontApi::/ then Rails.root.join('openapi/front_api.yml').to_s + end + end +end +``` + +For a one-off class, `Registry.register(..., openapi:)` (a file path or Hash) overrides +the resolver. Without either, the configured `document` is used. + +```ruby +StructuredParams::OpenApi::Registry.register( + AdminUserParams, + openapi: Rails.root.join('openapi/admin.yml').to_s +) +``` + +### Running in CI ```sh bundle exec rake structured_params:openapi:drift_check ``` -The current scope deliberately skips `oneOf`, `anyOf`, multi-entry `allOf`, schemas -with a `type` array (every OpenAPI 3.1 nullable such as `[string, 'null']`; 3.0 -`nullable: true` is compared normally), and external references. A single-entry -`allOf` is treated as the schema it wraps, as TypeSpec emits for `$ref` plus a -`description`. Recursive schemas are compared once per ancestor chain; the repeated visit -is dropped silently. +Set `STRUCTURED_PARAMS_STRICT=1` to also fail on skipped checks (see below). -Skips never fail a run, but they are not silent: the Rake task prints every skipped -and ignored entry with a summary line, and `result.skipped` / `result.to_report` expose -them in RSpec. Set `STRUCTURED_PARAMS_STRICT=1` (or pass `strict: true` to +## Scope, skips and missing schemas + +Handled as if they were plain schemas: + +- OpenAPI 3.1 nullable forms: `oneOf`/`anyOf` with a `{type: null}` option and a `type` + array with `'null'` (for example `[string, 'null']`). Once the null option is removed + they are compared like 3.0 `nullable: true`; +- `oneOf`/`anyOf` of plain scalars (for example `string` or `integer`), which match when + the attribute fits any option; +- a single-entry `allOf`, as TypeSpec emits for `$ref` plus a `description`. + +The current scope deliberately skips: + +- `oneOf`/`anyOf` of objects, and multi-entry `allOf`; +- a `type` array of several non-null types; +- external references. + +Recursive schemas are compared once per ancestor chain; the repeated visit is dropped +silently. + +Skips never fail a run, but they are not silent: the Rake task prints every skipped and +ignored entry with a summary line, and `result.skipped` / `result.to_report` expose them +in RSpec. Set `STRUCTURED_PARAMS_STRICT=1` (or pass `strict: true` to `Registry.check_all!`) to fail on skipped checks too. A class whose resolved schema name has no matching component is reported as a `missing_schema` error, as is a dangling or circular local `$ref` (`unresolved_reference`), so a renamed component or a typo cannot silently drop -coverage. For a class that intentionally has no schema (for example one backed by query -parameters), ignore it explicitly: +coverage. A class that intentionally has no schema (for example one backed by query +parameters) can be handled in two ways: -```ruby -StructuredParams::OpenApi.configure do |config| - config.ignore_drift(path: '(root)', kind: :missing_schema, parameter_class: 'SearchParams') -end -``` +- If it is never meant to be checked, leave it out of `parameter_class_filter`. The + class is not checked at all, and nothing is recorded about it. + + ```ruby + NO_SCHEMA = %w[Api::V1::SearchParams].freeze + + StructuredParams::OpenApi.configure do |config| + config.parameter_class_filter = ->(klass) do + klass.name&.start_with?('Api::') && NO_SCHEMA.exclude?(klass.name) + end + end + ``` + +- If a schema should exist but does not yet, ignore the finding for that class. It moves + to `result.ignored` and is listed by the Rake task, so it can be revisited later. + + ```ruby + StructuredParams::OpenApi.configure do |config| + config.ignore_drift(kind: :missing_schema, parameter_class: 'Api::V1::SearchParams') + end + ``` `result.to_message` is prefixed with the checked class's name, so `check_all!`'s combined `DriftError` message reads as one block per class instead of a flat, -unattributed list of paths. A later registration for the same class replaces an earlier -one. +unattributed list of paths. diff --git a/docs/openapi-drift-check_ja.md b/docs/openapi-drift-check_ja.md new file mode 100644 index 0000000..37bcbca --- /dev/null +++ b/docs/openapi-drift-check_ja.md @@ -0,0 +1,379 @@ +# OpenAPI Drift Check + +[English](openapi-drift-check.md) | 日本語 + +OpenAPI のリクエスト schema と `StructuredParams::Params` クラスの差分を検出し、 +片方だけが変更されて乖離することを防ぎます。 + +## 目次 + +- [用語](#用語) +- [クイックスタート](#クイックスタート) +- [クラスと schema の対応付け](#クラスと-schema-の対応付け) + - [既定の命名規則](#既定の命名規則) + - [命名規則を変更する](#命名規則を変更する) + - [特定のクラスだけ例外にする](#特定のクラスだけ例外にする) +- [比較する内容](#比較する内容) + - [スカラー型](#スカラー型) + - [attribute と必須 property](#attribute-と必須-property) + - [property 名の対応](#property-名の対応) +- [RSpec で使う](#rspec-で使う) +- [特定の検出結果を無視する](#特定の検出結果を無視する) +- [Rake タスクと Registry](#rake-タスクと-registry) + - [自動検出](#自動検出) + - [例外を登録する](#例外を登録する) + - [複数の OpenAPI ファイル](#複数の-openapi-ファイル) + - [CI で実行する](#ci-で実行する) +- [対象範囲、skip、schema の欠落](#対象範囲skipschema-の欠落) + +## 用語 + +このドキュメントで **component** と呼ぶのは、OpenAPI ドキュメントの +`components.schemas` 配下にある、名前付きの schema のことです。各 `Params` クラスは、 +名前で見つけた 1 つの component と照合されます(たとえば `CreateUserParams` → +`CreateUser`)。 + +```yaml +# openapi.yml +components: + schemas: + CreateUser: # <- これが「component」(JSON Pointer は #/components/schemas/CreateUser) + type: object + properties: + name: { type: string } +``` + +## クイックスタート + +initializer を追加し、必要な部分だけ残してください。この例の OpenAPI ドキュメントは +TypeSpec から生成されているため、component 名は `V1.Orders.CreateRequest` のような形で、 +クラス名は `Api::V1::Orders::CreateRequestParameter` のような形です。 + +```ruby +# config/initializers/structured_params_openapi.rb +StructuredParams::OpenApi.configure do |config| + # 必須: OpenAPI ドキュメントと、検査するクラス + config.document = Rails.root.join('docs/api/openapi.yaml').to_s + config.parameter_class_filter = ->(klass) { klass.name&.start_with?('Api::') } + + # component 名が既定の規則(CreateUserParams -> CreateUser)と異なる場合だけ: + # Api::V1::Orders::CreateRequestParameter -> "V1.Orders.CreateRequest" + config.schema_name_resolver = lambda do |klass| + klass.name.delete_prefix('Api::').delete_suffix('Parameter').gsub('::', '.') + end + + # schema があるはずだがまだないクラスがある場合だけ: + # 失敗にせず、記録だけ残す + %w[ + Api::V1::Reports::ExportRequestParameter + Api::V1::Reports::SummaryRequestParameter + ].each do |name| + config.ignore_drift(kind: :missing_schema, parameter_class: name) + end +end + +# 上の規則に従わない component 名を持つクラスがある場合だけ +{ + 'Api::V1::Orders::KonbiniRequestParameter' => 'V1.Orders.CreateKonbiniRequest', + 'Api::V1::Orders::BankTransferRequestParameter' => 'V1.Orders.CreateBankTransferRequest' +}.each do |parameter_class, schema| + StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) +end +``` + +- `parameter_class_filter` は検査するクラスを選びます。新しい `Api::*` クラスは、登録なしで + 対象になります。 +- `schema_name_resolver` は、1 つの規則で全クラスを component に対応付けます。 +- `ignore_drift` は、まだ schema がないクラスを記録します。Rake の出力には一覧表示され + 続けます。検査する予定のないクラスは、filter から外すほうが適切です + ([対象範囲、skip、schema の欠落](#対象範囲skipschema-の欠落)を参照)。 +- `Registry.register` は、命名の規則に合わない少数のクラスを個別に対応付けます。 + +あとは Rake タスクを実行すると、直すべき残りが分かります。 + +```sh +bundle exec rake structured_params:openapi:drift_check +``` + +## クラスと schema の対応付け + +### 既定の命名規則 + +既定では、`CreateUserParams` を `#/components/schemas/CreateUser` に対応付けます。 +クラスの namespace と、末尾の `Param`、`Params`、`Parameter`、`Parameters`、`Form` を +取り除いた名前が component 名になります。 + +### 命名規則を変更する + +component の命名規則が異なる場合は、`schema_name_resolver` で全体の規則を変更します。 +たとえば OpenAPI 側で、`Api::V1::CreateUserParams` と `Api::V2::CreateUserParams` に +対応する component が `V1CreateUserRequest` と `V2CreateUserRequest` という名前の場合は、 +次のようにします。 + +```yaml +# openapi.yml +components: + schemas: + V1CreateUserRequest: # <- 既定の規則では CreateUser を探してしまう + type: object + properties: + name: { type: string } + V2CreateUserRequest: ... +``` + +```ruby +StructuredParams::OpenApi.configure do |config| + config.schema_name_resolver = ->(parameter_class) do + # Api::V1::CreateUserParams -> "V1CreateUser" -> "V1CreateUserRequest" + name = parameter_class.name.delete_prefix('Api::').delete_suffix('Params').gsub('::', '') + "#{name}Request" + end +end +``` + +resolver が返すのは component 名で、すべてのクラスで既定の規則の代わりに使われます。 +この例が取り除くのは `Params` だけなので、`CreateUserForm` は `CreateUserFormRequest` +になります。 + +返す名前には namespace を残してください。残さない(たとえば `demodulize` で外す)と、 +`Api::V1::CreateUserParams` も `Api::V2::CreateUserParams` も存在しない +`CreateUserRequest` を探してしまい、`missing_schema` になります。バージョンごとに +OpenAPI ファイル自体が分かれている場合は、component 名はそのままにして、 +`document_resolver` でファイルを切り替えます +([複数の OpenAPI ファイル](#複数の-openapi-ファイル)を参照)。 + +### 特定のクラスだけ例外にする + +例外的なクラスには、`schema:` で component 名を渡します。JSON Pointer +(`'/components/schemas/CreateUserRequest'`、`'#/…'`)や、インラインの Hash も渡せます。 +`pointer:` で Pointer を明示でき、`openapi:` で 1 回の呼び出しに限って設定済みの +ドキュメントを上書きできます(ファイルパスまたは Hash)。 + +```ruby +StructuredParams::OpenApi::DriftCheck.call( + parameter_class: CreateUserParams, + schema: 'CreateUserRequest' +) +``` + +## 比較する内容 + +### スカラー型 + +| schema の `type` | 許容する attribute の型 | +|------------------|--------------------------| +| `string` | `:string`、`:date`、`:datetime`、`:time`(`format` は見ません) | +| `integer` | `:integer`、`:big_integer` | +| `number` | `:float`、`:decimal` | +| `boolean`、`object`、`array` | 同じ型 | + +`type` も `properties` もない schema(たとえば enum だけの schema)は、 +`type_mismatch` として報告されます。ネストした object と array の型は再帰的に比較します。 + +### attribute と必須 property + +schema の property は `attribute` 宣言と比較します。 + +schema の `required` は presence 検証と比較しますが、この対応は近似です。OpenAPI の +`required` が意味するのは「キーが存在すること」だけです。Rails では、キーが無い場合も値が +`nil` の場合も、属性は `nil` になります。そのため「キーが存在する」ことを表現できる +バリデータはなく、`presence`、`allow_nil`、`allow_blank` のどれが意図なのかも分かりません。 +そこでこのチェックは、無条件の `validates ..., presence: true` を代わりの目安として使います +(「無条件」とは、`if:`、`unless:`、`on:`、`allow_nil:`、`allow_blank:` のいずれも指定されて +いないことです)。 + +schema が null を許す required の property(`nullable: true`、`'null'` を含む `type` 配列、 +`null` を選択肢に持つ `oneOf`/`anyOf`)には、`presence` は不要です。`presence` を付けると、 +契約が許している null を拒否してしまうため、報告されません。 + +これらのオプションが付いた `presence` は、キーが無くても通る可能性があるため、`required` +を満たすか確認できず、`missing_presence_validation` として報告されます。これは必ずしも +バグではありません。Ruby 側が意図的な書き方なら、 +[`ignore_drift`](#特定の検出結果を無視する)で property ごとに許容してください。`presence` +が API 全体にとって厳しすぎる場合(`required` が許す `false` や `""` も拒否します)は、 +`required_validation: :none` を指定し、必須キーの挙動は別に検証してください。 + +### property 名の対応 + +JSON の property 名と Ruby の attribute 名が異なる場合は `property_names` を使います。 + +```ruby +StructuredParams::OpenApi::DriftCheck.call( + parameter_class: CreateUserParams, + property_names: { user_id: 'userId' } +) +``` + +## RSpec で使う + +結果をそのまま失敗メッセージに使うと、CI の失敗に検出した差分がすべて並びます。 + +```ruby +it 'matches the request schema' do + result = StructuredParams::OpenApi::DriftCheck.call( + parameter_class: CreateUserParams + ) + + expect(result).to be_valid, result.to_message +end +``` + +## 特定の検出結果を無視する + +`required_validation: :none` と Registry はクラス全体に作用するため、既知の 1 件だけを +許容して、同じパスの他の検査は続けたい場合には粗すぎます。その場合は ignore 規則を +設定します。 + +```ruby +StructuredParams::OpenApi.configure do |config| + config.ignore_drift(path: 'wallet_information.wallet_type', kind: :missing_presence_validation) +end +``` + +- 規則は、そのパスが現れるすべての場所に適用されます。複数のトップレベルクラスが + 再利用するネストした object では、親クラスごとに 1 回ずつ適用されます。 +- `kind:` を省略すると、そのパスのすべてのエラー種別を無視します。指定できる種別は + `missing_attribute`、`unexpected_attribute`、`missing_presence_validation`、 + `type_mismatch`、`missing_schema`、`unresolved_reference` で、それ以外は + `ArgumentError` になります。 +- `parameter_class:`(クラスまたはその名前)を渡すと、規則を 1 つのクラスに限定できます。 +- パスは attribute 名を `.` でつなぎ、配列の要素には `[]` を付けます + (例: `items[].sku`)。`path:`、`kind:`、`parameter_class:` はどれも規則を絞り込み、 + 省略したものは「すべて」に一致しますが、少なくとも 1 つは必要です。schema が見つからない + 場合(`missing_schema`)は、property ではなくクラス全体についての指摘なので、`path:` + を省略します。 +- skip は無視できません。 + +無視した検出結果は `result.errors` ではなく `result.ignored` に移ります。そのため +`valid?` は解決済みとして扱い、`to_message` にも表示されません。ただし結果オブジェクト +には残るので、何をなぜ抑制しているかを後から確認でき、黙って消えることはありません。 + +## Rake タスクと Registry + +### 自動検出 + +gem は Rails アプリケーションに `structured_params:openapi:drift_check` を提供します。 +このタスクは `parameter_class_filter` で選択されたクラスを自動で検査するため、新しい +`Api::*Params` クラスに登録は不要です。 + +- filter がなければ登録済みのクラスだけが対象で、何も検査されなかった場合は + タスクが警告します。 +- 無名クラスは自動検出されません。filter の例が `name&.` で守られているのはそのためです。 + +### 例外を登録する + +Registry は、例外的な component 名や property の対応付けが必要な場合にだけ使って +ください。同じクラスに対しては、明示的な登録が自動検出より優先されます。同じクラスを +後から登録すると、先の登録を置き換えます。 + +```ruby +StructuredParams::OpenApi::Registry.register( + LegacyUserParams, + schema: 'LegacyCreateUserRequest' +) +``` + +`parameter_class` にはクラス名の文字列も渡せます。この場合はタスクの実行時に解決 +されます。Rails の autoload がまだ設定されていない `config/initializers/*.rb` から +登録するときに便利です。クラス定数を直接参照すると `NameError` になるためです。 + +```ruby +StructuredParams::OpenApi::Registry.register( + 'LegacyUserParams', + schema: 'LegacyCreateUserRequest' +) +``` + +### 複数の OpenAPI ファイル + +アプリケーションに複数の OpenAPI ファイルがある場合(たとえば `admin_api.yml` と +`front_api.yml`)は、`document_resolver` を設定すると、自動検出された各クラスを +それぞれのファイルに対して検査できます。`nil` を返したクラスは `config.document` に +フォールバックします。 + +```ruby +StructuredParams::OpenApi.configure do |config| + config.parameter_class_filter = ->(klass) { klass.name&.match?(/\A(AdminApi|FrontApi)::/) } + config.document_resolver = ->(klass) do + case klass.name + when /\AAdminApi::/ then Rails.root.join('openapi/admin_api.yml').to_s + when /\AFrontApi::/ then Rails.root.join('openapi/front_api.yml').to_s + end + end +end +``` + +個別のクラスには `Registry.register(..., openapi:)`(ファイルパスまたは Hash)で +resolver を上書きできます。どちらもなければ、設定済みの `document` を使います。 + +```ruby +StructuredParams::OpenApi::Registry.register( + AdminUserParams, + openapi: Rails.root.join('openapi/admin.yml').to_s +) +``` + +### CI で実行する + +```sh +bundle exec rake structured_params:openapi:drift_check +``` + +skip も失敗扱いにしたい場合は、`STRUCTURED_PARAMS_STRICT=1` を設定します(下記参照)。 + +## 対象範囲、skip、schema の欠落 + +通常の schema と同じように扱うもの: + +- OpenAPI 3.1 の nullable の書き方: `{type: null}` を選択肢に持つ `oneOf`/`anyOf` と、 + `'null'` を含む `type` 配列(たとえば `[string, 'null']`)。null の選択肢を除くと、 + 3.0 の `nullable: true` と同じように比較されます。 +- スカラーだけの `oneOf`/`anyOf`(たとえば `string` か `integer`)。attribute がどれか 1 つの + 選択肢に合えば一致します。 +- 要素が 1 つだけの `allOf`。TypeSpec が `$ref` に `description` を付けるときに出力する形です。 + +現在の対象範囲では、次のものを意図的に skip します。 + +- object の `oneOf`/`anyOf`、複数要素の `allOf` +- null 以外の型を複数持つ `type` 配列 +- 外部参照 + +再帰する schema は、祖先の連鎖ごとに 1 回だけ比較され、繰り返しの訪問は黙って打ち切られます。 + +skip は実行を失敗させませんが、黙ってもいません。Rake タスクは skip と ignore の +すべての項目をサマリ行とともに表示し、RSpec では `result.skipped` と +`result.to_report` で確認できます。skip も失敗扱いにしたい場合は、 +`STRUCTURED_PARAMS_STRICT=1` を設定するか、`Registry.check_all!` に `strict: true` を +渡してください。 + +解決した schema 名に対応する component がないクラスは、`missing_schema` エラーとして +報告されます。壊れた、または循環するローカルの `$ref` も同様です +(`unresolved_reference`)。component 名の変更や打ち間違いで、検査が黙って外れること +はありません。意図的に schema を持たないクラス(たとえば query parameter が元の +クラス)は、次の 2 通りで扱えます。 + +- 検査する予定がないなら、`parameter_class_filter` から外します。そのクラスはまったく + 検査されず、記録も残りません。 + + ```ruby + NO_SCHEMA = %w[Api::V1::SearchParams].freeze + + StructuredParams::OpenApi.configure do |config| + config.parameter_class_filter = ->(klass) do + klass.name&.start_with?('Api::') && NO_SCHEMA.exclude?(klass.name) + end + end + ``` + +- 本来は schema があるはずだが、まだない場合は、そのクラスの指摘を無視します。指摘は + `result.ignored` に移り、Rake タスクにも一覧表示されるので、後から見直せます。 + + ```ruby + StructuredParams::OpenApi.configure do |config| + config.ignore_drift(kind: :missing_schema, parameter_class: 'Api::V1::SearchParams') + end + ``` + +`result.to_message` の先頭には検査したクラス名が付くため、`check_all!` がまとめて +投げる `DriftError` のメッセージは、パスをただ並べたものではなく、クラスごとの +ブロックになります。 diff --git a/lib/structured_params/open_api/configuration.rb b/lib/structured_params/open_api/configuration.rb index 1c7a913..08944ab 100644 --- a/lib/structured_params/open_api/configuration.rb +++ b/lib/structured_params/open_api/configuration.rb @@ -14,6 +14,7 @@ class Configuration attr_accessor :document #: Hash[untyped, untyped]|String? attr_writer :schema_name_resolver #: untyped + attr_writer :document_resolver #: untyped attr_writer :parameter_class_filter #: untyped #: () -> void @@ -25,6 +26,14 @@ def initialize @documents = {} end + # The OpenAPI document for +parameter_class+: the +document_resolver+ result when it + # returns one, otherwise the global +document+. Lets one app check classes from several + # API namespaces (e.g. admin and front) against separate files. + #: (singleton(StructuredParams::Params)) -> (Hash[untyped, untyped] | String)? + def document_for(parameter_class) + @document_resolver&.call(parameter_class) || document + end + # Parses an OpenAPI file once per modification time, so a bulk check across many # parameter classes does not re-read the same document for each of them. JSON files # work too, since JSON is valid YAML. Only the mtime invalidates the cache, so two writes @@ -46,19 +55,25 @@ def load_document(path) document end - # Suppresses a specific finding (matched by exact +path+ and, optionally, +kind+) instead - # of raising it as an error. Without +parameter_class+ it applies to every drift check, + # Suppresses matching findings instead of raising them as errors. Each of +path+, +kind+ + # and +parameter_class+ narrows the rule; an omitted one matches everything, but at least + # one must be given. Without +parameter_class+ the rule applies to every drift check, # including once per parent class for a nested object reused by several top-level - # parameter classes. Omit +kind+ to ignore every error kind at that path. Skips - # (unsupported schema features) are never ignored; they only appear in Result#skipped. + # parameter classes. Skips (unsupported schema features) are never ignored; they only + # appear in Result#skipped. # # Paths use attribute names joined with +.+ and +[]+ for array items, e.g. - # +items[].sku+; a schema that cannot be found at all is reported at +(root)+. + # +items[].sku+. A schema that cannot be found at all (+:missing_schema+) concerns the + # whole class rather than a property, so omit +path+ for it. # # Ignored findings are moved to DriftCheck::Result#ignored rather than dropped, so # they stay auditable; #valid? and #to_message treat them as resolved. - #: (path: String, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void - def ignore_drift(path:, kind: nil, parameter_class: nil) + #: (?path: String?, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void + def ignore_drift(path: nil, kind: nil, parameter_class: nil) + if path.nil? && kind.nil? && parameter_class.nil? + raise ArgumentError, 'give at least one of path:, kind: or parameter_class: to ignore_drift' + end + if kind && !DriftCheck::ERROR_KINDS.include?(kind) raise ArgumentError, "unknown kind #{kind.inspect}; expected one of #{DriftCheck::ERROR_KINDS.inspect}" end @@ -69,7 +84,7 @@ def ignore_drift(path:, kind: nil, parameter_class: nil) #: (Symbol, String, ?untyped) -> bool def ignored?(kind, path, parameter_class = nil) @ignored_issues.any? do |rule| - rule.fetch(:path) == path && + [nil, path].include?(rule.fetch(:path)) && [nil, kind].include?(rule.fetch(:kind)) && [nil, parameter_class&.to_s].include?(rule.fetch(:parameter_class)) end diff --git a/lib/structured_params/open_api/drift_check.rb b/lib/structured_params/open_api/drift_check.rb index 8109abe..0280466 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/lib/structured_params/open_api/drift_check.rb @@ -3,6 +3,8 @@ require 'date' require 'yaml' +require_relative 'drift_check/issue' +require_relative 'drift_check/result' module StructuredParams # Schema comparison types for OpenAPI drift checks. @@ -31,65 +33,8 @@ class DriftCheck # Options that stop a presence validator from enforcing presence on every validation. CONDITIONAL_PRESENCE_OPTIONS = %i[if unless on allow_nil allow_blank].freeze #: Array[Symbol] - Issue = Struct.new( - :kind, #: Symbol - :path, #: String - :message, #: String - keyword_init: true - ) - - # Result of a single schema/class comparison. - class Result - attr_reader :errors, :skipped, :ignored #: Array[Issue] - attr_reader :parameter_class #: singleton(::StructuredParams::Params) - - #: (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void - def initialize(parameter_class, errors, skipped, ignored) - @parameter_class = parameter_class - @errors = errors - @skipped = skipped - @ignored = ignored - end - - #: () -> bool - def valid? - errors.empty? - end - - #: () -> String - def to_message - sections = [] - sections << "OpenAPI drift detected:\n#{format_issues(errors)}" unless valid? - sections << "OpenAPI checks skipped:\n#{format_issues(skipped)}" unless skipped.empty? - return '' if sections.empty? - - "#{class_label}\n#{sections.join("\n")}" - end - - # Lists what a passing check still left unverified (skipped) or suppressed (ignored), - # so those stay visible in CI output instead of only on the result object. - #: () -> String - def to_report - sections = [] - sections << "OpenAPI checks skipped:\n#{format_issues(skipped)}" unless skipped.empty? - sections << "OpenAPI findings ignored:\n#{format_issues(ignored)}" unless ignored.empty? - return '' if sections.empty? - - "#{class_label}\n#{sections.join("\n")}" - end - - private - - #: () -> String - def class_label - @parameter_class.name || @parameter_class.inspect - end - - #: (Array[Issue]) -> String - def format_issues(issues) - issues.map { |issue| "- #{issue.path}: #{issue.message}" }.join("\n") - end - end + # Schema types that a +oneOf+/+anyOf+ of plain scalars may offer. + SCALAR_SCHEMA_TYPES = %i[string integer number boolean].freeze #: Array[Symbol] class << self # rubocop:disable Metrics/ParameterLists @@ -124,7 +69,7 @@ def initialize( property_names: {}, required_validation: :presence ) - @openapi_source = openapi || OpenApi.configuration.document + @openapi_source = openapi || OpenApi.configuration.document_for(parameter_class) @schema_source = schema @parameter_class = parameter_class @pointer = pointer @@ -279,16 +224,58 @@ def compare_required_attributes(schema, schema_attributes, parameter_class, path attribute = attribute_name(property) next unless schema_attributes.key?(attribute) next if presence_validator?(parameter_class, attribute) + next if nullable_property?(schema, property, join_path(path, property)) error(join_path(path, property), :missing_presence_validation, 'required schema property has no unconditional presence validation') end end + # A required property that the schema allows to be null must not demand +presence+, + # which would reject the null the contract permits. + #: (Hash[untyped, untyped], String, String) -> bool + def nullable_property?(schema, property, path) + property_schema = value_for(value_for(schema, 'properties') || {}, property) + return false unless property_schema.is_a?(Hash) + + resolved = without_reporting { resolve_references(property_schema, path) } + nullable_schema?(resolved) + end + + #: (Hash[untyped, untyped]) -> bool + def nullable_schema?(schema) + return true if value_for(schema, 'nullable') == true + return true if null_type?(value_for(schema, 'type')) + + %w[oneOf anyOf].any? do |key| + Array(value_for(schema, key)).any? { |option| option.is_a?(Hash) && nullable_schema?(option) } + end + end + + #: (untyped) -> bool + def null_type?(type) + type == 'null' || (type.is_a?(Array) && type.include?('null')) + end + + # Runs a lookup whose findings are reported elsewhere, so they are not recorded twice. + #: [T] () { () -> T } -> T + def without_reporting + sizes = [@errors.size, @skipped.size, @ignored.size] + yield + ensure + @errors.slice!(sizes[0]..) + @skipped.slice!(sizes[1]..) + @ignored.slice!(sizes[2]..) + end + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity #: (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void def compare_type(schema, attribute_type, path) - return if unresolved_reference?(schema) || skip_union?(schema, path) + return if unresolved_reference?(schema) + + scalar_types = scalar_union_types(schema, path) + return compare_scalar_union(scalar_types, attribute_type, path) if scalar_types + return if skip_union?(schema, path) if value_for(schema, 'type').is_a?(Array) skip(path, 'multiple schema types are outside the supported drift-check scope') @@ -348,6 +335,78 @@ def presence_validator?(parameter_class, attribute) end end + # A +oneOf+/+anyOf+ of plain scalars matches when the attribute fits any of them. + #: (Array[Symbol], ActiveModel::Type::Value, String) -> void + def compare_scalar_union(types, attribute_type, path) + actual_type = attribute_type.type + return if types.any? { |type| compatible_types?(type, actual_type) } + + error(path, :type_mismatch, "schema types #{types.inspect} do not match attribute type #{actual_type.inspect}") + end + + # The scalar types offered by a +oneOf+/+anyOf+, or nil when it is not a plain scalar union. + #: (Hash[untyped, untyped], String) -> Array[Symbol]? + def scalar_union_types(schema, path) + key = union_key(schema) + return unless key + + options = value_for(schema, key).reject { |option| null_option?(option) } + types = options.map { |option| scalar_option_type(option, path) } + types if types.size > 1 && types.all? { |type| SCALAR_SCHEMA_TYPES.include?(type) } + end + + #: (untyped, String) -> Symbol? + def scalar_option_type(option, path) + return unless option.is_a?(Hash) + + resolved = without_reporting { resolve_references(option, path) } + schema_type(resolved) unless value_for(resolved, 'properties') + end + + #: (Hash[untyped, untyped]) -> String? + def union_key(schema) + %w[oneOf anyOf].find { |name| value_for(schema, name).is_a?(Array) } + end + + # +oneOf: [X, {type: null}]+ and +type: [T, 'null']+ are how OpenAPI 3.1 says "X or null". + # Collapse them to X and mark the result +nullable+, so they are compared like a 3.0 + # +nullable: true+ schema instead of being treated as unsupported unions. + #: (Hash[untyped, untyped]) -> Hash[untyped, untyped] + def collapse_nullable(schema) + collapse_null_type(schema) || collapse_null_union(schema) || schema + end + + #: (Hash[untyped, untyped]) -> Hash[untyped, untyped]? + def collapse_null_type(schema) + type = value_for(schema, 'type') + return unless type.is_a?(Array) && type.include?('null') + + others = type - ['null'] + schema.merge('type' => others.first, 'nullable' => true) if others.size == 1 + end + + #: (Hash[untyped, untyped]) -> Hash[untyped, untyped]? + def collapse_null_union(schema) + key = union_key(schema) + return unless key + + options = value_for(schema, key) + present = options.reject { |option| null_option?(option) } + return unless present.size == 1 && present.size < options.size + + nullable_option(present.first, schema.reject { |name, _| name.to_s == key }) + end + + #: (untyped, Hash[untyped, untyped]) -> Hash[untyped, untyped]? + def nullable_option(option, siblings) + option.merge(siblings).merge('nullable' => true) if option.is_a?(Hash) + end + + #: (untyped) -> bool + def null_option?(option) + option.is_a?(Hash) && null_type?(value_for(option, 'type')) && !value_for(option, 'type').is_a?(Array) + end + #: (Hash[untyped, untyped], String) -> bool def skip_union?(schema, path) union_key = %w[oneOf anyOf allOf].find { |key| value_for(schema, key) } @@ -359,7 +418,7 @@ def skip_union?(schema, path) #: (Hash[untyped, untyped], String, ?Array[String]) -> Hash[untyped, untyped] def resolve_references(schema, path, followed = []) - schema = unwrap_single_all_of(schema) + schema = collapse_nullable(unwrap_single_all_of(schema)) reference = value_for(schema, '$ref') return schema unless reference diff --git a/lib/structured_params/open_api/drift_check/issue.rb b/lib/structured_params/open_api/drift_check/issue.rb new file mode 100644 index 0000000..ea6384f --- /dev/null +++ b/lib/structured_params/open_api/drift_check/issue.rb @@ -0,0 +1,16 @@ +# rbs_inline: enabled +# frozen_string_literal: true + +module StructuredParams + module OpenApi + class DriftCheck + # A single finding (error, skipped check, or ignored finding) from a drift check. + Issue = Struct.new( + :kind, #: Symbol + :path, #: String + :message, #: String + keyword_init: true + ) + end + end +end diff --git a/lib/structured_params/open_api/drift_check/result.rb b/lib/structured_params/open_api/drift_check/result.rb new file mode 100644 index 0000000..96ffa61 --- /dev/null +++ b/lib/structured_params/open_api/drift_check/result.rb @@ -0,0 +1,61 @@ +# rbs_inline: enabled +# frozen_string_literal: true + +module StructuredParams + module OpenApi + class DriftCheck + # Result of a single schema/class comparison. + class Result + attr_reader :errors, :skipped, :ignored #: Array[Issue] + attr_reader :parameter_class #: singleton(::StructuredParams::Params) + + #: (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void + def initialize(parameter_class, errors, skipped, ignored) + @parameter_class = parameter_class + @errors = errors + @skipped = skipped + @ignored = ignored + end + + #: () -> bool + def valid? + errors.empty? + end + + #: () -> String + def to_message + sections = [] + sections << "OpenAPI drift detected:\n#{format_issues(errors)}" unless valid? + sections << "OpenAPI checks skipped:\n#{format_issues(skipped)}" unless skipped.empty? + return '' if sections.empty? + + "#{class_label}\n#{sections.join("\n")}" + end + + # Lists what a passing check still left unverified (skipped) or suppressed (ignored), + # so those stay visible in CI output instead of only on the result object. + #: () -> String + def to_report + sections = [] + sections << "OpenAPI checks skipped:\n#{format_issues(skipped)}" unless skipped.empty? + sections << "OpenAPI findings ignored:\n#{format_issues(ignored)}" unless ignored.empty? + return '' if sections.empty? + + "#{class_label}\n#{sections.join("\n")}" + end + + private + + #: () -> String + def class_label + @parameter_class.name || @parameter_class.inspect + end + + #: (Array[Issue]) -> String + def format_issues(issues) + issues.map { |issue| "- #{issue.path}: #{issue.message}" }.join("\n") + end + end + end + end +end diff --git a/lib/structured_params/open_api/registry.rb b/lib/structured_params/open_api/registry.rb index f0fa0dc..86c0b94 100644 --- a/lib/structured_params/open_api/registry.rb +++ b/lib/structured_params/open_api/registry.rb @@ -14,11 +14,16 @@ class << self # +parameter_class+ may be a class name String (e.g. from a Rails initializer, # before Zeitwerk autoloading is set up); it is resolved lazily in +check_all+. + # +openapi+ (a file path or Hash) overrides the configured document for this class, so + # one app can check classes against several OpenAPI files. #: (singleton(StructuredParams::Params) | String, **untyped) -> void - def register(parameter_class, schema: nil, pointer: nil, property_names: {}, required_validation: :presence) + # rubocop:disable-next Metrics/ParameterLists + def register(parameter_class, openapi: nil, schema: nil, pointer: nil, property_names: {}, + required_validation: :presence) entries << [ parameter_class, { + openapi: openapi, schema: schema, pointer: pointer, property_names: property_names, diff --git a/sig/structured_params/open_api/configuration.rbs b/sig/structured_params/open_api/configuration.rbs index c0142c6..485495e 100644 --- a/sig/structured_params/open_api/configuration.rbs +++ b/sig/structured_params/open_api/configuration.rbs @@ -13,11 +13,19 @@ module StructuredParams attr_writer schema_name_resolver: untyped + attr_writer document_resolver: untyped + attr_writer parameter_class_filter: untyped # : () -> void def initialize: () -> void + # The OpenAPI document for +parameter_class+: the +document_resolver+ result when it + # returns one, otherwise the global +document+. Lets one app check classes from several + # API namespaces (e.g. admin and front) against separate files. + # : (singleton(StructuredParams::Params)) -> (Hash[untyped, untyped] | String)? + def document_for: (singleton(StructuredParams::Params)) -> (Hash[untyped, untyped] | String)? + # Parses an OpenAPI file once per modification time, so a bulk check across many # parameter classes does not re-read the same document for each of them. JSON files # work too, since JSON is valid YAML. Only the mtime invalidates the cache, so two writes @@ -26,19 +34,21 @@ module StructuredParams # : (String) -> Hash[untyped, untyped] def load_document: (String) -> Hash[untyped, untyped] - # Suppresses a specific finding (matched by exact +path+ and, optionally, +kind+) instead - # of raising it as an error. Without +parameter_class+ it applies to every drift check, + # Suppresses matching findings instead of raising them as errors. Each of +path+, +kind+ + # and +parameter_class+ narrows the rule; an omitted one matches everything, but at least + # one must be given. Without +parameter_class+ the rule applies to every drift check, # including once per parent class for a nested object reused by several top-level - # parameter classes. Omit +kind+ to ignore every error kind at that path. Skips - # (unsupported schema features) are never ignored; they only appear in Result#skipped. + # parameter classes. Skips (unsupported schema features) are never ignored; they only + # appear in Result#skipped. # # Paths use attribute names joined with +.+ and +[]+ for array items, e.g. - # +items[].sku+; a schema that cannot be found at all is reported at +(root)+. + # +items[].sku+. A schema that cannot be found at all (+:missing_schema+) concerns the + # whole class rather than a property, so omit +path+ for it. # # Ignored findings are moved to DriftCheck::Result#ignored rather than dropped, so # they stay auditable; #valid? and #to_message treat them as resolved. - # : (path: String, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void - def ignore_drift: (path: String, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void + # : (?path: String?, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void + def ignore_drift: (?path: String?, ?kind: Symbol?, ?parameter_class: (Class | String)?) -> void # : (Symbol, String, ?untyped) -> bool def ignored?: (Symbol, String, ?untyped) -> bool diff --git a/sig/structured_params/open_api/drift_check.rbs b/sig/structured_params/open_api/drift_check.rbs index fd94f34..e6c7ccc 100644 --- a/sig/structured_params/open_api/drift_check.rbs +++ b/sig/structured_params/open_api/drift_check.rbs @@ -34,49 +34,8 @@ module StructuredParams # Options that stop a presence validator from enforcing presence on every validation. CONDITIONAL_PRESENCE_OPTIONS: Array[Symbol] - class Issue < Struct[Symbol | String] - attr_accessor kind(): Symbol - - attr_accessor path(): String - - attr_accessor message(): String - - def self.new: (?kind: Symbol, ?path: String, ?message: String) -> instance - | ({ ?kind: Symbol, ?path: String, ?message: String }) -> instance - end - - # Result of a single schema/class comparison. - class Result - attr_reader errors: Array[Issue] - - attr_reader skipped: Array[Issue] - - attr_reader ignored: Array[Issue] - - attr_reader parameter_class: singleton(::StructuredParams::Params) - - # : (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void - def initialize: (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void - - # : () -> bool - def valid?: () -> bool - - # : () -> String - def to_message: () -> String - - # Lists what a passing check still left unverified (skipped) or suppressed (ignored), - # so those stay visible in CI output instead of only on the result object. - # : () -> String - def to_report: () -> String - - private - - # : () -> String - def class_label: () -> String - - # : (Array[Issue]) -> String - def format_issues: (Array[Issue]) -> String - end + # Schema types that a +oneOf+/+anyOf+ of plain scalars may offer. + SCALAR_SCHEMA_TYPES: Array[Symbol] # rubocop:disable Metrics/ParameterLists # : (**untyped) -> Result @@ -124,6 +83,21 @@ module StructuredParams # : (Hash[untyped, untyped], Hash[Symbol, String], singleton(StructuredParams::Params), String) -> void def compare_required_attributes: (Hash[untyped, untyped], Hash[Symbol, String], singleton(StructuredParams::Params), String) -> void + # A required property that the schema allows to be null must not demand +presence+, + # which would reject the null the contract permits. + # : (Hash[untyped, untyped], String, String) -> bool + def nullable_property?: (Hash[untyped, untyped], String, String) -> bool + + # : (Hash[untyped, untyped]) -> bool + def nullable_schema?: (Hash[untyped, untyped]) -> bool + + # : (untyped) -> bool + def null_type?: (untyped) -> bool + + # Runs a lookup whose findings are reported elsewhere, so they are not recorded twice. + # : [T] () { () -> T } -> T + def without_reporting: [T] () { () -> T } -> T + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity # : (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void def compare_type: (Hash[untyped, untyped], ActiveModel::Type::Value, String) -> void @@ -140,6 +114,38 @@ module StructuredParams # : (singleton(StructuredParams::Params), Symbol) -> bool def presence_validator?: (singleton(StructuredParams::Params), Symbol) -> bool + # A +oneOf+/+anyOf+ of plain scalars matches when the attribute fits any of them. + # : (Array[Symbol], ActiveModel::Type::Value, String) -> void + def compare_scalar_union: (Array[Symbol], ActiveModel::Type::Value, String) -> void + + # The scalar types offered by a +oneOf+/+anyOf+, or nil when it is not a plain scalar union. + # : (Hash[untyped, untyped], String) -> Array[Symbol]? + def scalar_union_types: (Hash[untyped, untyped], String) -> Array[Symbol]? + + # : (untyped, String) -> Symbol? + def scalar_option_type: (untyped, String) -> Symbol? + + # : (Hash[untyped, untyped]) -> String? + def union_key: (Hash[untyped, untyped]) -> String? + + # +oneOf: [X, {type: null}]+ and +type: [T, 'null']+ are how OpenAPI 3.1 says "X or null". + # Collapse them to X and mark the result +nullable+, so they are compared like a 3.0 + # +nullable: true+ schema instead of being treated as unsupported unions. + # : (Hash[untyped, untyped]) -> Hash[untyped, untyped] + def collapse_nullable: (Hash[untyped, untyped]) -> Hash[untyped, untyped] + + # : (Hash[untyped, untyped]) -> Hash[untyped, untyped]? + def collapse_null_type: (Hash[untyped, untyped]) -> Hash[untyped, untyped]? + + # : (Hash[untyped, untyped]) -> Hash[untyped, untyped]? + def collapse_null_union: (Hash[untyped, untyped]) -> Hash[untyped, untyped]? + + # : (untyped, Hash[untyped, untyped]) -> Hash[untyped, untyped]? + def nullable_option: (untyped, Hash[untyped, untyped]) -> Hash[untyped, untyped]? + + # : (untyped) -> bool + def null_option?: (untyped) -> bool + # : (Hash[untyped, untyped], String) -> bool def skip_union?: (Hash[untyped, untyped], String) -> bool diff --git a/sig/structured_params/open_api/drift_check/issue.rbs b/sig/structured_params/open_api/drift_check/issue.rbs new file mode 100644 index 0000000..73c5dda --- /dev/null +++ b/sig/structured_params/open_api/drift_check/issue.rbs @@ -0,0 +1,19 @@ +# Generated from lib/structured_params/open_api/drift_check/issue.rb with RBS::Inline + +module StructuredParams + module OpenApi + class DriftCheck + # A single finding (error, skipped check, or ignored finding) from a drift check. + class Issue < Struct[Symbol | String] + attr_accessor kind(): Symbol + + attr_accessor path(): String + + attr_accessor message(): String + + def self.new: (?kind: Symbol, ?path: String, ?message: String) -> instance + | ({ ?kind: Symbol, ?path: String, ?message: String }) -> instance + end + end + end +end diff --git a/sig/structured_params/open_api/drift_check/result.rbs b/sig/structured_params/open_api/drift_check/result.rbs new file mode 100644 index 0000000..98e029f --- /dev/null +++ b/sig/structured_params/open_api/drift_check/result.rbs @@ -0,0 +1,40 @@ +# Generated from lib/structured_params/open_api/drift_check/result.rb with RBS::Inline + +module StructuredParams + module OpenApi + class DriftCheck + # Result of a single schema/class comparison. + class Result + attr_reader errors: Array[Issue] + + attr_reader skipped: Array[Issue] + + attr_reader ignored: Array[Issue] + + attr_reader parameter_class: singleton(::StructuredParams::Params) + + # : (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void + def initialize: (singleton(::StructuredParams::Params), Array[Issue], Array[Issue], Array[Issue]) -> void + + # : () -> bool + def valid?: () -> bool + + # : () -> String + def to_message: () -> String + + # Lists what a passing check still left unverified (skipped) or suppressed (ignored), + # so those stay visible in CI output instead of only on the result object. + # : () -> String + def to_report: () -> String + + private + + # : () -> String + def class_label: () -> String + + # : (Array[Issue]) -> String + def format_issues: (Array[Issue]) -> String + end + end + end +end diff --git a/sig/structured_params/open_api/registry.rbs b/sig/structured_params/open_api/registry.rbs index e0717e3..06cd24b 100644 --- a/sig/structured_params/open_api/registry.rbs +++ b/sig/structured_params/open_api/registry.rbs @@ -13,7 +13,10 @@ module StructuredParams # +parameter_class+ may be a class name String (e.g. from a Rails initializer, # before Zeitwerk autoloading is set up); it is resolved lazily in +check_all+. + # +openapi+ (a file path or Hash) overrides the configured document for this class, so + # one app can check classes against several OpenAPI files. # : (singleton(StructuredParams::Params) | String, **untyped) -> void + # rubocop:disable-next Metrics/ParameterLists def self.register: (singleton(StructuredParams::Params) | String, **untyped) -> void # : (?auto_discover: bool) -> Array[DriftCheck::Result] diff --git a/spec/open_api/configuration_spec.rb b/spec/open_api/configuration_spec.rb index 0d56cf0..a1a120e 100644 --- a/spec/open_api/configuration_spec.rb +++ b/spec/open_api/configuration_spec.rb @@ -73,6 +73,20 @@ .to raise_error(ArgumentError, /unknown kind/) end + it 'matches every path when path is omitted' do + configuration = StructuredParams::OpenApi.configuration + configuration.ignore_drift(kind: :missing_schema, parameter_class: 'ScopedParams') + + expect(configuration.ignored?(:missing_schema, '(root)', 'ScopedParams')).to be true + expect(configuration.ignored?(:missing_schema, 'anything', 'ScopedParams')).to be true + expect(configuration.ignored?(:missing_schema, '(root)', 'OtherParams')).to be false + expect(configuration.ignored?(:type_mismatch, '(root)', 'ScopedParams')).to be false + end + + it 'requires at least one narrowing argument so a rule cannot ignore everything' do + expect { StructuredParams::OpenApi.configuration.ignore_drift }.to raise_error(ArgumentError, /at least one/) + end + it 'scopes a rule to the given parameter class' do configuration = StructuredParams::OpenApi.configuration configuration.ignore_drift(path: 'a', parameter_class: 'ScopedParams') @@ -152,4 +166,30 @@ expect { configuration.load_document('/nonexistent/openapi.yml') }.to raise_error(Errno::ENOENT) end end + + describe '#document_for' do + subject(:configuration) { described_class.new } + + it 'falls back to the global document without a resolver or when it returns nil' do + configuration.document = 'openapi.yml' + expect(configuration.document_for(Class.new)).to eq('openapi.yml') + + configuration.document_resolver = ->(_klass) {} + expect(configuration.document_for(Class.new)).to eq('openapi.yml') + end + + it 'uses the resolver result and lets each class pick its own file' do + configuration.document = 'openapi.yml' + configuration.document_resolver = lambda do |klass| + case klass.name + when /\AAdminApi::/ then 'admin_api.yml' + when /\AFrontApi::/ then 'front_api.yml' + end + end + + expect(configuration.document_for(stub_const('AdminApi::UserParams', Class.new))).to eq('admin_api.yml') + expect(configuration.document_for(stub_const('FrontApi::UserParams', Class.new))).to eq('front_api.yml') + expect(configuration.document_for(stub_const('Other::UserParams', Class.new))).to eq('openapi.yml') + end + end end diff --git a/spec/open_api/drift_check/result_spec.rb b/spec/open_api/drift_check/result_spec.rb new file mode 100644 index 0000000..6099730 --- /dev/null +++ b/spec/open_api/drift_check/result_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe StructuredParams::OpenApi::DriftCheck::Result do + def issue(kind, path, message) + StructuredParams::OpenApi::DriftCheck::Issue.new(kind: kind, path: path, message: message) + end + + let(:parameter_class) { Class.new(StructuredParams::Params) } + let(:error) { issue(:type_mismatch, 'name', 'wrong type') } + let(:skipped_issue) { issue(:unsupported, 'tags', 'oneOf is outside the supported drift-check scope') } + let(:ignored_issue) { issue(:missing_attribute, 'extra', 'schema property has no matching attribute declaration') } + + before { stub_const('ResultSpecParameter', parameter_class) } + + describe '#valid?' do + it 'is true without errors, even with skipped or ignored findings' do + expect(described_class.new(parameter_class, [], [skipped_issue], [ignored_issue])).to be_valid + end + + it 'is false with errors' do + expect(described_class.new(parameter_class, [error], [], [])).not_to be_valid + end + end + + describe '#to_message' do + it 'returns an empty string when nothing is reported' do + expect(described_class.new(parameter_class, [], [], []).to_message).to eq('') + end + + it 'lists errors and skipped findings under the class name' do + result = described_class.new(parameter_class, [error], [skipped_issue], [ignored_issue]) + + expect(result.to_message).to eq(<<~MESSAGE.chomp) + ResultSpecParameter + OpenAPI drift detected: + - name: wrong type + OpenAPI checks skipped: + - tags: oneOf is outside the supported drift-check scope + MESSAGE + end + + it 'omits the errors section when valid' do + result = described_class.new(parameter_class, [], [skipped_issue], []) + + expect(result.to_message).not_to include('drift detected') + expect(result.to_message).to include('OpenAPI checks skipped') + end + + it 'falls back to inspect for anonymous classes' do + anonymous = Class.new(StructuredParams::Params) + result = described_class.new(anonymous, [error], [], []) + + expect(result.to_message).to start_with(anonymous.inspect) + end + end + + describe '#to_report' do + it 'returns an empty string without skipped or ignored findings' do + expect(described_class.new(parameter_class, [error], [], []).to_report).to eq('') + end + + it 'lists skipped and ignored findings but not errors' do + result = described_class.new(parameter_class, [error], [skipped_issue], [ignored_issue]) + + expect(result.to_report).to eq(<<~REPORT.chomp) + ResultSpecParameter + OpenAPI checks skipped: + - tags: oneOf is outside the supported drift-check scope + OpenAPI findings ignored: + - extra: schema property has no matching attribute declaration + REPORT + end + end +end diff --git a/spec/open_api/drift_check_spec.rb b/spec/open_api/drift_check_spec.rb index 0f68c13..5d6a748 100644 --- a/spec/open_api/drift_check_spec.rb +++ b/spec/open_api/drift_check_spec.rb @@ -93,13 +93,42 @@ expect(result).to be_valid end - it 'reports union schemas as skipped instead of treating them as compatible' do - schema['properties']['age'] = { 'oneOf' => [{ 'type' => 'integer' }, { 'type' => 'string' }] } + it 'reports object unions as skipped instead of treating them as compatible' do + schema['properties']['address'] = { 'oneOf' => [{ 'type' => 'object', 'properties' => {} }, + { 'type' => 'object', 'properties' => {} }] } result = described_class.call(schema: schema, parameter_class: parameter_class) expect(result).to be_valid - expect(result.skipped.first.message).to include('oneOf') + expect(result.skipped).to contain_exactly(have_attributes(path: 'address', message: a_string_including('oneOf'))) + end + + it 'accepts a scalar oneOf/anyOf when the attribute fits one option and rejects it otherwise' do + schema['properties']['age'] = { 'oneOf' => [{ 'type' => 'integer' }, { 'type' => 'string' }] } + schema['properties']['name'] = { 'anyOf' => [{ 'type' => 'integer' }, { 'type' => 'boolean' }] } + + result = described_class.call(schema: schema, parameter_class: parameter_class) + + expect(result.errors).to contain_exactly(have_attributes(kind: :type_mismatch, path: 'name')) + expect(result.skipped).to be_empty + end + + it 'compares oneOf/anyOf with a null option and a type array with null as nullable schemas' do + schema['properties']['age'] = { 'oneOf' => [{ 'type' => 'string' }, { 'type' => 'null' }] } + schema['properties']['name'] = { 'type' => %w[string null] } + address_or_null = [{ '$ref' => '#/components/schemas/Address' }, { 'type' => 'null' }] + schema['properties']['address'] = { 'anyOf' => address_or_null, 'description' => 'optional' } + document = { 'components' => { 'schemas' => { 'Request' => schema, 'Address' => { + 'type' => 'object', 'properties' => { 'city' => { 'type' => 'integer' } }, 'required' => ['city'] + } } } } + + result = described_class.call(openapi: document, schema: 'Request', parameter_class: parameter_class) + + expect(result.skipped).to be_empty + expect(result.errors).to contain_exactly( + have_attributes(kind: :type_mismatch, path: 'age'), + have_attributes(kind: :type_mismatch, path: 'address.city') + ) end it 'unwraps a single-entry allOf instead of treating it as unsupported composition' do @@ -180,7 +209,7 @@ it 'lets a missing schema be ignored for one class only' do document = { 'components' => { 'schemas' => { 'Other' => schema } } } StructuredParams::OpenApi.configure do |config| - config.ignore_drift(path: '(root)', kind: :missing_schema, parameter_class: parameter_class) + config.ignore_drift(kind: :missing_schema, parameter_class: parameter_class) end result = described_class.call(openapi: document, schema: 'Missing', parameter_class: parameter_class) @@ -288,8 +317,8 @@ expect(result.errors).to be_frozen end - it 'skips nullable type arrays, objects without properties and arrays without items' do - schema['properties']['name'] = { 'type' => %w[string null] } + it 'skips multiple-type arrays, objects without properties and arrays without items' do + schema['properties']['name'] = { 'type' => %w[string integer] } schema['properties']['address'] = { 'type' => 'object' } schema['properties']['tags'] = { 'type' => 'array' } @@ -314,6 +343,46 @@ expect(result.errors).to contain_exactly(have_attributes(kind: :type_mismatch, path: 'address.city')) end + describe 'required properties that the schema allows to be null' do + let(:nickname_class) { Class.new(StructuredParams::Params) { attribute :nickname, :string } } + let(:components) do + { 'Nullable' => { 'type' => 'string', 'nullable' => true }, 'Plain' => { 'type' => 'string' } } + end + + def check(property_schema, parameter_class: nickname_class) + document = { + 'components' => { 'schemas' => components.merge( + 'Request' => { 'type' => 'object', 'properties' => { 'nickname' => property_schema }, + 'required' => ['nickname'] } + ) } + } + described_class.call(openapi: document, schema: 'Request', parameter_class: parameter_class) + end + + [ + ['3.0 nullable', { 'type' => 'string', 'nullable' => true }], + ['3.0 nullable wrapped in allOf', + { 'allOf' => [{ '$ref' => '#/components/schemas/Plain' }], 'nullable' => true }], + ['3.1 type array', { 'type' => %w[string null] }], + ['3.1 oneOf with null', { 'oneOf' => [{ 'type' => 'string' }, { 'type' => 'null' }] }], + ['nullable behind a $ref', { '$ref' => '#/components/schemas/Nullable' }] + ].each do |label, property_schema| + it "does not require presence for #{label}" do + expect(check(property_schema).errors).to be_empty + end + end + + it 'still requires presence when the property is not nullable' do + expect(check({ 'type' => 'string' }).errors.map(&:kind)).to eq([:missing_presence_validation]) + end + + it 'does not record findings twice while looking up nullability' do + result = check({ '$ref' => '#/components/schemas/Missing' }) + + expect(result.errors.map(&:kind)).to contain_exactly(:unresolved_reference, :missing_presence_validation) + end + end + [{ if: -> { true } }, { on: :create }, { allow_nil: true }, { allow_blank: true }].each do |options| it "does not treat a presence validator with #{options.keys.first}: as unconditional" do conditional = Class.new(StructuredParams::Params) do diff --git a/spec/open_api/registry_spec.rb b/spec/open_api/registry_spec.rb index 9025d9d..e257dc7 100644 --- a/spec/open_api/registry_spec.rb +++ b/spec/open_api/registry_spec.rb @@ -67,7 +67,7 @@ describe 'skipped and ignored checks' do let(:parameter_class) { Class.new(StructuredParams::Params) { attribute :name, :string } } let(:skipping_schema) do - { 'type' => 'object', 'properties' => { 'name' => { 'type' => %w[string null] } } } + { 'type' => 'object', 'properties' => { 'name' => { 'type' => %w[string integer] } } } end before { described_class.register(parameter_class, schema: skipping_schema) } @@ -123,4 +123,40 @@ expect(described_class.check_all).to all(be_valid) end + + it 'checks a class against its own OpenAPI document instead of the configured one' do + parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } + other_document = { 'components' => { 'schemas' => { 'Admin' => { + 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string' } } + } } } } + StructuredParams::OpenApi.configure { |config| config.document = { 'components' => { 'schemas' => {} } } } + described_class.register(parameter_class, openapi: other_document, schema: 'Admin') + + expect(described_class.check_all).to all(be_valid) + end + + describe 'with a document_resolver' do + def document_with(property) + props = { property => { 'type' => 'string' } } + { 'components' => { 'schemas' => { 'User' => { 'type' => 'object', 'properties' => props } } } } + end + + it 'checks auto-discovered classes against the document chosen per class' do + stub_const('AdminApi', Module.new) + stub_const('FrontApi', Module.new) + classes = { 'AdminApi::UserParams' => :role, 'FrontApi::UserParams' => :nickname }.map do |name, attribute| + stub_const(name, Class.new(StructuredParams::Params)).tap { |klass| klass.attribute attribute, :string } + end + documents = { 'AdminApi::UserParams' => document_with('role'), 'FrontApi::UserParams' => document_with('nickname') } + StructuredParams::OpenApi.configure do |config| + config.parameter_class_filter = ->(klass) { classes.include?(klass) } + config.document_resolver = ->(klass) { documents.fetch(klass.name) } + end + + results = described_class.check_all(auto_discover: true) + + expect(results.size).to eq(2) + expect(results).to all(be_valid) + end + end end diff --git a/spec/tasks/structured_params_open_api_spec.rb b/spec/tasks/structured_params_open_api_spec.rb index 0113ee2..f0882df 100644 --- a/spec/tasks/structured_params_open_api_spec.rb +++ b/spec/tasks/structured_params_open_api_spec.rb @@ -31,7 +31,7 @@ before do described_class.define_task(:environment) parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } - schema = { 'type' => 'object', 'properties' => { 'name' => { 'type' => %w[string null] } } } + schema = { 'type' => 'object', 'properties' => { 'name' => { 'type' => %w[string integer] } } } StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) end From 1f13330b904f5a51cfcf489f1701970a0a2f29b5 Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 19:33:36 +0900 Subject: [PATCH 11/12] refactor: extract the OpenAPI drift check into the structured_params-openapi gem Move the drift check out of the core gem into structured_params-openapi, kept in this repository as a subdirectory with its own gemspec, sig, specs and docs. The core no longer loads any OpenAPI code or Rake task. - gem structured_params-openapi (require 'structured_params/openapi'), which depends on structured_params >= 0.9.5 - module is StructuredParams::OpenAPI; the Railtie is StructuredParams::OpenAPI::Railtie - Gemfile, Rakefile, Steepfile, lefthook, AGENTS.md and RuboCop config cover both gems; the core README points to the new gem Co-Authored-By: Claude Sonnet 5.5 --- .rubocop_rbs.yml | 8 ++-- AGENTS.md | 8 +++- Gemfile | 1 + README.md | 23 +++-------- README_ja.md | 23 +++-------- Rakefile | 7 ++++ Steepfile | 10 +++++ lefthook.yml | 4 +- lib/structured_params.rb | 20 ---------- sig/structured_params.rbs | 8 ---- structured_params-openapi/.rspec | 2 + structured_params-openapi/LICENSE.txt | 21 ++++++++++ structured_params-openapi/README.md | 34 ++++++++++++++++ structured_params-openapi/README_ja.md | 34 ++++++++++++++++ .../docs/drift-check.md | 28 ++++++------- .../docs/drift-check_ja.md | 28 ++++++------- .../lib/structured_params/openapi.rb | 28 +++++++++++++ .../openapi}/configuration.rb | 2 +- .../structured_params/openapi}/drift_check.rb | 12 +++--- .../openapi}/drift_check/issue.rb | 2 +- .../openapi}/drift_check/result.rb | 2 +- .../structured_params/openapi}/registry.rb | 4 +- .../lib/structured_params/openapi/version.rb | 8 ++++ .../lib/tasks/structured_params_openapi.rake | 2 +- .../sig/structured_params/openapi.rbs | 14 +++++++ .../openapi}/configuration.rbs | 4 +- .../openapi}/drift_check.rbs | 4 +- .../openapi}/drift_check/issue.rbs | 4 +- .../openapi}/drift_check/result.rbs | 4 +- .../structured_params/openapi}/registry.rbs | 4 +- .../sig/structured_params/openapi/version.rbs | 7 ++++ .../spec/openapi}/configuration_spec.rb | 34 ++++++++-------- .../spec/openapi}/drift_check/result_spec.rb | 4 +- .../spec/openapi}/drift_check_spec.rb | 10 ++--- .../spec/openapi}/railtie_loading_spec.rb | 2 +- .../spec/openapi}/registry_spec.rb | 22 +++++------ structured_params-openapi/spec/spec_helper.rb | 18 +++++++++ .../tasks/structured_params_openapi_spec.rb | 12 +++--- .../structured_params-openapi.gemspec | 39 +++++++++++++++++++ structured_params.gemspec | 1 - 40 files changed, 340 insertions(+), 162 deletions(-) create mode 100644 structured_params-openapi/.rspec create mode 100644 structured_params-openapi/LICENSE.txt create mode 100644 structured_params-openapi/README.md create mode 100644 structured_params-openapi/README_ja.md rename docs/openapi-drift-check.md => structured_params-openapi/docs/drift-check.md (95%) rename docs/openapi-drift-check_ja.md => structured_params-openapi/docs/drift-check_ja.md (96%) create mode 100644 structured_params-openapi/lib/structured_params/openapi.rb rename {lib/structured_params/open_api => structured_params-openapi/lib/structured_params/openapi}/configuration.rb (99%) rename {lib/structured_params/open_api => structured_params-openapi/lib/structured_params/openapi}/drift_check.rb (98%) rename {lib/structured_params/open_api => structured_params-openapi/lib/structured_params/openapi}/drift_check/issue.rb (95%) rename {lib/structured_params/open_api => structured_params-openapi/lib/structured_params/openapi}/drift_check/result.rb (99%) rename {lib/structured_params/open_api => structured_params-openapi/lib/structured_params/openapi}/registry.rb (97%) create mode 100644 structured_params-openapi/lib/structured_params/openapi/version.rb rename lib/tasks/structured_params_open_api.rake => structured_params-openapi/lib/tasks/structured_params_openapi.rake (91%) create mode 100644 structured_params-openapi/sig/structured_params/openapi.rbs rename {sig/structured_params/open_api => structured_params-openapi/sig/structured_params/openapi}/configuration.rbs (97%) rename {sig/structured_params/open_api => structured_params-openapi/sig/structured_params/openapi}/drift_check.rbs (98%) rename {sig/structured_params/open_api => structured_params-openapi/sig/structured_params/openapi}/drift_check/issue.rbs (82%) rename {sig/structured_params/open_api => structured_params-openapi/sig/structured_params/openapi}/drift_check/result.rbs (91%) rename {sig/structured_params/open_api => structured_params-openapi/sig/structured_params/openapi}/registry.rbs (95%) create mode 100644 structured_params-openapi/sig/structured_params/openapi/version.rbs rename {spec/open_api => structured_params-openapi/spec/openapi}/configuration_spec.rb (86%) rename {spec/open_api => structured_params-openapi/spec/openapi}/drift_check/result_spec.rb (95%) rename {spec/open_api => structured_params-openapi/spec/openapi}/drift_check_spec.rb (98%) rename {spec/open_api => structured_params-openapi/spec/openapi}/railtie_loading_spec.rb (97%) rename {spec/open_api => structured_params-openapi/spec/openapi}/registry_spec.rb (90%) create mode 100644 structured_params-openapi/spec/spec_helper.rb rename spec/tasks/structured_params_open_api_spec.rb => structured_params-openapi/spec/tasks/structured_params_openapi_spec.rb (85%) create mode 100644 structured_params-openapi/structured_params-openapi.gemspec diff --git a/.rubocop_rbs.yml b/.rubocop_rbs.yml index c5ec528..704a13a 100644 --- a/.rubocop_rbs.yml +++ b/.rubocop_rbs.yml @@ -8,14 +8,14 @@ plugins: Style/RbsInline/MissingTypeAnnotation: EnforcedStyle: method_type_signature Exclude: - - 'spec/**/*' + - '**/spec/**/*' Style/RbsInline/UntypedInstanceVariable: Exclude: - 'lib/structured_params/params.rb' - 'lib/structured_params.rb' - - 'lib/structured_params/open_api/configuration.rb' - - 'lib/structured_params/open_api/registry.rb' + - 'structured_params-openapi/lib/structured_params/openapi/configuration.rb' + - 'structured_params-openapi/lib/structured_params/openapi/registry.rb' Style/RbsInline/RequireRbsInlineComment: Exclude: - - 'spec/**/*' + - '**/spec/**/*' diff --git a/AGENTS.md b/AGENTS.md index f85e0a8..a66deac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,6 +11,10 @@ ## 主要ディレクトリ +- `structured_params-openapi/`: OpenAPI drift check の別 gem(`structured_params-openapi`)。 + 独自の `lib/`・`sig/`・`spec/`・`docs/` と gemspec を持ちます。コアとは同じリポジトリで管理し、 + テストは `cd structured_params-openapi && bundle exec rspec` で実行します。 + - `lib/structured_params.rb`: エントリーポイント - `lib/structured_params/params.rb`: コアの `Params` 実装 - `lib/structured_params/type/`: `array` / `object` 型ハンドラ @@ -61,9 +65,9 @@ bundle exec rubocop lib/structured_params/params.rb `lefthook.yml` によりコミット前に以下が走ります。 -- `bundle exec rbs-inline --output=sig lib/**/*.rb` +- `bundle exec rbs-inline --output=sig lib/**/*.rb`(`structured_params-openapi/` でも同様) - `bundle exec rubocop` -- `bundle exec rspec` +- `bundle exec rspec`(`structured_params-openapi/` でも実行) - `bundle exec steep check` フックで失敗しない状態まで揃えてから完了扱いにしてください。 diff --git a/Gemfile b/Gemfile index 838cf75..d70f085 100644 --- a/Gemfile +++ b/Gemfile @@ -4,6 +4,7 @@ source 'https://rubygems.org' gemspec +gemspec path: 'structured_params-openapi' # Allow testing different Rails versions via environment variable rails_version = ENV['RAILS_VERSION'] || '~> 8.0.0' diff --git a/README.md b/README.md index c942f80..51f4aa9 100644 --- a/README.md +++ b/README.md @@ -107,37 +107,26 @@ def create end ``` -### 3. OpenAPI Drift Check +### 3. OpenAPI Drift Check (optional gem) -Configure an OpenAPI document once and select the request parameter namespace. -The default convention maps `CreateUserParams` to `#/components/schemas/CreateUser`. +Detecting differences between your OpenAPI schemas and Params classes is provided by a +separate gem, [`structured_params-openapi`](structured_params-openapi/README.md): ```ruby -# config/initializers/structured_params.rb -StructuredParams::OpenApi.configure do |config| - config.document = Rails.root.join('openapi.yml').to_s - config.parameter_class_filter = ->(parameter_class) do - parameter_class.name&.start_with?('Api::') - end -end +# Gemfile +gem 'structured_params-openapi' ``` -Run the check in CI: - ```sh bundle exec rake structured_params:openapi:drift_check ``` -See [OpenAPI Drift Check](docs/openapi-drift-check.md) for component-name -overrides, property mappings, and supported schema types. New classes matching -the filter are checked automatically; register only exceptional mappings. - ## Documentation - **[Installation and Setup](docs/installation.md)** - Getting started with StructuredParams - **[Basic Usage](docs/basic-usage.md)** - Parameter classes, nested objects, and arrays - **[Validation](docs/validation.md)** - Using ActiveModel validations with nested structures -- **[OpenAPI Drift Check](docs/openapi-drift-check.md)** - Detect differences between request schemas and Params classes +- **[OpenAPI Drift Check](structured_params-openapi/docs/drift-check.md)** - Detect differences between request schemas and Params classes (`structured_params-openapi` gem) - **[Strong Parameters](docs/strong-parameters.md)** - Automatic permit list generation - **[Error Handling](docs/error-handling.md)** - Flat and structured error formats - **[Serialization](docs/serialization.md)** - Converting parameters to hashes and JSON diff --git a/README_ja.md b/README_ja.md index d39cd16..8e85ef4 100644 --- a/README_ja.md +++ b/README_ja.md @@ -107,37 +107,26 @@ def create end ``` -### 3. OpenAPI drift check +### 3. OpenAPI drift check(別 gem) -OpenAPI ドキュメントを一度設定し、filter でリクエスト用パラメータクラスの namespace を選択します。 -既定では `CreateUserParams` を `#/components/schemas/CreateUser` に対応付けます。 +OpenAPI の schema と Params クラスの差分検出は、別 gem の +[`structured_params-openapi`](structured_params-openapi/README_ja.md) として提供しています。 ```ruby -# config/initializers/structured_params.rb -StructuredParams::OpenApi.configure do |config| - config.document = Rails.root.join('openapi.yml').to_s - config.parameter_class_filter = ->(parameter_class) do - parameter_class.name&.start_with?('Api::') - end -end +# Gemfile +gem 'structured_params-openapi' ``` -CI では次を実行します。 - ```sh bundle exec rake structured_params:openapi:drift_check ``` -filter に一致する新しい class は自動で検査されます。component 名の例外指定、 -property mapping、対応する schema 型の詳細は -[OpenAPI Drift Check](docs/openapi-drift-check_ja.md) を参照してください。 - ## ドキュメント - **[インストールとセットアップ](docs/installation.md)** - StructuredParams の始め方 - **[基本的な使い方](docs/basic-usage.md)** - パラメータクラス、ネストオブジェクト、配列 - **[バリデーション](docs/validation.md)** - ネスト構造での ActiveModel バリデーション -- **[OpenAPI Drift Check](docs/openapi-drift-check_ja.md)** - OpenAPI schema と Params class の乖離検出 +- **[OpenAPI Drift Check](structured_params-openapi/docs/drift-check_ja.md)** - OpenAPI schema と Params class の乖離検出(`structured_params-openapi` gem) - **[Strong Parameters](docs/strong-parameters.md)** - permit リストの自動生成 - **[エラーハンドリング](docs/error-handling.md)** - フラット形式と構造化形式のエラー - **[シリアライゼーション](docs/serialization.md)** - パラメータのハッシュと JSON 変換 diff --git a/Rakefile b/Rakefile index af5a5d4..0b01eab 100644 --- a/Rakefile +++ b/Rakefile @@ -28,4 +28,11 @@ if RUBY_VERSION >= '3.3.0' default_tasks << :steep end +# The OpenAPI plugin lives in its own gem directory with its own spec suite. +desc 'Run the structured_params-openapi specs' +task :spec_openapi do + Dir.chdir('structured_params-openapi') { sh 'bundle exec rspec' } +end +default_tasks << :spec_openapi + task default: default_tasks diff --git a/Steepfile b/Steepfile index 5cd3edb..fb68fbe 100644 --- a/Steepfile +++ b/Steepfile @@ -12,3 +12,13 @@ target :lib do hash[Steep::Diagnostic::Ruby::LibraryRBSError] = nil end end + +target :openapi do + signature 'sig', 'structured_params-openapi/sig' + + check 'structured_params-openapi/lib' + + configure_code_diagnostics do |hash| + hash[Steep::Diagnostic::Ruby::LibraryRBSError] = nil + end +end diff --git a/lefthook.yml b/lefthook.yml index 9bf5464..2ddee2e 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -1,7 +1,7 @@ prepare-commit-msg: commands: rbs-inline: - run: bundle exec rbs-inline --output=sig lib/**/*.rb && git add sig/ + run: bundle exec rbs-inline --output=sig lib/**/*.rb && (cd structured_params-openapi && bundle exec rbs-inline --output=sig lib/**/*.rb) && git add sig/ structured_params-openapi/sig/ pre-commit: commands: @@ -9,5 +9,7 @@ pre-commit: run: ./bin/rubocop rspec: run: bundle exec rspec + rspec-open-api: + run: cd structured_params-openapi && bundle exec rspec steep: run: bundle exec steep check diff --git a/lib/structured_params.rb b/lib/structured_params.rb index 173f430..622d85b 100644 --- a/lib/structured_params.rb +++ b/lib/structured_params.rb @@ -18,11 +18,6 @@ require_relative 'structured_params/type/object' require_relative 'structured_params/type/array' -# OpenAPI contract checks -require_relative 'structured_params/open_api/configuration' -require_relative 'structured_params/open_api/drift_check' -require_relative 'structured_params/open_api/registry' - # params (load after type definitions) require_relative 'structured_params/params' @@ -99,19 +94,4 @@ def register_types_as(object_name:, array_name:) ActiveModel::Type.register(array_name, StructuredParams::Type::Array) end end - - RAKE_TASK_PATH = File.expand_path('tasks/structured_params_open_api.rake', __dir__) #: String - - if defined?(Rails::Railtie) - # Defers loading the OpenAPI drift-check rake task until Rails actually - # loads rake tasks, so requiring this gem never depends on the Rake DSL - # being mixed into +main+ (e.g. `rails runner`, `rails console`, app boot). - class Railtie < Rails::Railtie - rake_tasks do - load StructuredParams::RAKE_TASK_PATH - end - end - elsif defined?(Rake::DSL) - load RAKE_TASK_PATH - end end diff --git a/sig/structured_params.rbs b/sig/structured_params.rbs index 9d0110a..1a9debe 100644 --- a/sig/structured_params.rbs +++ b/sig/structured_params.rbs @@ -50,12 +50,4 @@ module StructuredParams # Helper method to register types with custom names # : (object_name: Symbol, array_name: Symbol) -> void def self.register_types_as: (object_name: Symbol, array_name: Symbol) -> void - - RAKE_TASK_PATH: String - - # Defers loading the OpenAPI drift-check rake task until Rails actually - # loads rake tasks, so requiring this gem never depends on the Rake DSL - # being mixed into +main+ (e.g. `rails runner`, `rails console`, app boot). - class Railtie < Rails::Railtie - end end diff --git a/structured_params-openapi/.rspec b/structured_params-openapi/.rspec new file mode 100644 index 0000000..83e16f8 --- /dev/null +++ b/structured_params-openapi/.rspec @@ -0,0 +1,2 @@ +--color +--require spec_helper diff --git a/structured_params-openapi/LICENSE.txt b/structured_params-openapi/LICENSE.txt new file mode 100644 index 0000000..0626645 --- /dev/null +++ b/structured_params-openapi/LICENSE.txt @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2024 Mizuki Yamamoto + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/structured_params-openapi/README.md b/structured_params-openapi/README.md new file mode 100644 index 0000000..dffd7e4 --- /dev/null +++ b/structured_params-openapi/README.md @@ -0,0 +1,34 @@ +# structured_params-openapi + +English | [日本語](README_ja.md) + +OpenAPI drift check for [StructuredParams](https://github.com/Syati/structured_params). +It detects differences between OpenAPI request schemas and `StructuredParams::Params` +classes, so a change on one side cannot silently diverge from the other. + +## Installation + +```ruby +# Gemfile +gem 'structured_params-openapi' +``` + +Bundler requires it automatically (`structured_params/openapi`). In a Rails application +the Rake task `structured_params:openapi:drift_check` is registered for you. + +## Usage + +```ruby +# config/initializers/structured_params_openapi.rb +StructuredParams::OpenAPI.configure do |config| + config.document = Rails.root.join('docs/api/openapi.yaml').to_s + config.parameter_class_filter = ->(klass) { klass.name&.start_with?('Api::') } +end +``` + +```sh +bundle exec rake structured_params:openapi:drift_check +``` + +See the [guide](docs/drift-check.md) for naming conventions, multiple OpenAPI files, +ignoring findings, and the supported schema features. diff --git a/structured_params-openapi/README_ja.md b/structured_params-openapi/README_ja.md new file mode 100644 index 0000000..c470209 --- /dev/null +++ b/structured_params-openapi/README_ja.md @@ -0,0 +1,34 @@ +# structured_params-openapi + +[English](README.md) | 日本語 + +[StructuredParams](https://github.com/Syati/structured_params) 用の OpenAPI drift check です。 +OpenAPI のリクエスト schema と `StructuredParams::Params` クラスの差分を検出し、片方だけが +変更されて乖離することを防ぎます。 + +## インストール + +```ruby +# Gemfile +gem 'structured_params-openapi' +``` + +Bundler が自動で require します(`structured_params/openapi`)。Rails アプリケーションでは、 +Rake タスク `structured_params:openapi:drift_check` も自動で登録されます。 + +## 使い方 + +```ruby +# config/initializers/structured_params_openapi.rb +StructuredParams::OpenAPI.configure do |config| + config.document = Rails.root.join('docs/api/openapi.yaml').to_s + config.parameter_class_filter = ->(klass) { klass.name&.start_with?('Api::') } +end +``` + +```sh +bundle exec rake structured_params:openapi:drift_check +``` + +命名規則、複数の OpenAPI ファイル、検出結果の無視、対応している schema の機能については、 +[ガイド](docs/drift-check_ja.md)を参照してください。 diff --git a/docs/openapi-drift-check.md b/structured_params-openapi/docs/drift-check.md similarity index 95% rename from docs/openapi-drift-check.md rename to structured_params-openapi/docs/drift-check.md index b515d3d..2c6b10d 100644 --- a/docs/openapi-drift-check.md +++ b/structured_params-openapi/docs/drift-check.md @@ -1,6 +1,6 @@ # OpenAPI Drift Check -English | [日本語](openapi-drift-check_ja.md) +English | [日本語](drift-check_ja.md) Detects differences between OpenAPI request schemas and `StructuredParams::Params` classes, so a change on one side cannot silently diverge from the other. @@ -50,7 +50,7 @@ classes named like `Api::V1::Orders::CreateRequestParameter`. ```ruby # config/initializers/structured_params_openapi.rb -StructuredParams::OpenApi.configure do |config| +StructuredParams::OpenAPI.configure do |config| # Required: the OpenAPI document and the classes to check. config.document = Rails.root.join('docs/api/openapi.yaml').to_s config.parameter_class_filter = ->(klass) { klass.name&.start_with?('Api::') } @@ -77,7 +77,7 @@ end 'Api::V1::Orders::KonbiniRequestParameter' => 'V1.Orders.CreateKonbiniRequest', 'Api::V1::Orders::BankTransferRequestParameter' => 'V1.Orders.CreateBankTransferRequest' }.each do |parameter_class, schema| - StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) + StructuredParams::OpenAPI::Registry.register(parameter_class, schema: schema) end ``` @@ -122,7 +122,7 @@ components: ``` ```ruby -StructuredParams::OpenApi.configure do |config| +StructuredParams::OpenAPI.configure do |config| config.schema_name_resolver = ->(parameter_class) do # Api::V1::CreateUserParams -> "V1CreateUser" -> "V1CreateUserRequest" name = parameter_class.name.delete_prefix('Api::').delete_suffix('Params').gsub('::', '') @@ -149,7 +149,7 @@ JSON Pointer (`'/components/schemas/CreateUserRequest'`, `'#/…'`) or an inline document for one call (a file path or a Hash). ```ruby -StructuredParams::OpenApi::DriftCheck.call( +StructuredParams::OpenAPI::DriftCheck.call( parameter_class: CreateUserParams, schema: 'CreateUserRequest' ) @@ -197,7 +197,7 @@ pass `required_validation: :none` and check required-key behavior separately. Use `property_names` when the JSON property name differs from the Ruby attribute name. ```ruby -StructuredParams::OpenApi::DriftCheck.call( +StructuredParams::OpenAPI::DriftCheck.call( parameter_class: CreateUserParams, property_names: { user_id: 'userId' } ) @@ -209,7 +209,7 @@ Use the result as the failure message so a CI failure lists every detected diffe ```ruby it 'matches the request schema' do - result = StructuredParams::OpenApi::DriftCheck.call( + result = StructuredParams::OpenAPI::DriftCheck.call( parameter_class: CreateUserParams ) @@ -224,7 +224,7 @@ aren't precise enough to accept one known finding while still checking everythin at that path. For that, configure an ignore rule instead: ```ruby -StructuredParams::OpenApi.configure do |config| +StructuredParams::OpenAPI.configure do |config| config.ignore_drift(path: 'wallet_information.wallet_type', kind: :missing_presence_validation) end ``` @@ -267,7 +267,7 @@ registrations override auto-discovery for the same class, and a later registrati the same class replaces an earlier one. ```ruby -StructuredParams::OpenApi::Registry.register( +StructuredParams::OpenAPI::Registry.register( LegacyUserParams, schema: 'LegacyCreateUserRequest' ) @@ -279,7 +279,7 @@ where Rails hasn't finished setting up autoloading yet and referencing the class constant directly would raise `NameError`. ```ruby -StructuredParams::OpenApi::Registry.register( +StructuredParams::OpenAPI::Registry.register( 'LegacyUserParams', schema: 'LegacyCreateUserRequest' ) @@ -292,7 +292,7 @@ When an application has several OpenAPI files (for example `admin_api.yml` and against its own file. Return `nil` to fall back to `config.document`. ```ruby -StructuredParams::OpenApi.configure do |config| +StructuredParams::OpenAPI.configure do |config| config.parameter_class_filter = ->(klass) { klass.name&.match?(/\A(AdminApi|FrontApi)::/) } config.document_resolver = ->(klass) do case klass.name @@ -307,7 +307,7 @@ For a one-off class, `Registry.register(..., openapi:)` (a file path or Hash) ov the resolver. Without either, the configured `document` is used. ```ruby -StructuredParams::OpenApi::Registry.register( +StructuredParams::OpenAPI::Registry.register( AdminUserParams, openapi: Rails.root.join('openapi/admin.yml').to_s ) @@ -358,7 +358,7 @@ parameters) can be handled in two ways: ```ruby NO_SCHEMA = %w[Api::V1::SearchParams].freeze - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.parameter_class_filter = ->(klass) do klass.name&.start_with?('Api::') && NO_SCHEMA.exclude?(klass.name) end @@ -369,7 +369,7 @@ parameters) can be handled in two ways: to `result.ignored` and is listed by the Rake task, so it can be revisited later. ```ruby - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.ignore_drift(kind: :missing_schema, parameter_class: 'Api::V1::SearchParams') end ``` diff --git a/docs/openapi-drift-check_ja.md b/structured_params-openapi/docs/drift-check_ja.md similarity index 96% rename from docs/openapi-drift-check_ja.md rename to structured_params-openapi/docs/drift-check_ja.md index 37bcbca..800ff7b 100644 --- a/docs/openapi-drift-check_ja.md +++ b/structured_params-openapi/docs/drift-check_ja.md @@ -1,6 +1,6 @@ # OpenAPI Drift Check -[English](openapi-drift-check.md) | 日本語 +[English](drift-check.md) | 日本語 OpenAPI のリクエスト schema と `StructuredParams::Params` クラスの差分を検出し、 片方だけが変更されて乖離することを防ぎます。 @@ -51,7 +51,7 @@ TypeSpec から生成されているため、component 名は `V1.Orders.CreateR ```ruby # config/initializers/structured_params_openapi.rb -StructuredParams::OpenApi.configure do |config| +StructuredParams::OpenAPI.configure do |config| # 必須: OpenAPI ドキュメントと、検査するクラス config.document = Rails.root.join('docs/api/openapi.yaml').to_s config.parameter_class_filter = ->(klass) { klass.name&.start_with?('Api::') } @@ -77,7 +77,7 @@ end 'Api::V1::Orders::KonbiniRequestParameter' => 'V1.Orders.CreateKonbiniRequest', 'Api::V1::Orders::BankTransferRequestParameter' => 'V1.Orders.CreateBankTransferRequest' }.each do |parameter_class, schema| - StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) + StructuredParams::OpenAPI::Registry.register(parameter_class, schema: schema) end ``` @@ -122,7 +122,7 @@ components: ``` ```ruby -StructuredParams::OpenApi.configure do |config| +StructuredParams::OpenAPI.configure do |config| config.schema_name_resolver = ->(parameter_class) do # Api::V1::CreateUserParams -> "V1CreateUser" -> "V1CreateUserRequest" name = parameter_class.name.delete_prefix('Api::').delete_suffix('Params').gsub('::', '') @@ -150,7 +150,7 @@ OpenAPI ファイル自体が分かれている場合は、component 名はそ ドキュメントを上書きできます(ファイルパスまたは Hash)。 ```ruby -StructuredParams::OpenApi::DriftCheck.call( +StructuredParams::OpenAPI::DriftCheck.call( parameter_class: CreateUserParams, schema: 'CreateUserRequest' ) @@ -198,7 +198,7 @@ schema が null を許す required の property(`nullable: true`、`'null'` を JSON の property 名と Ruby の attribute 名が異なる場合は `property_names` を使います。 ```ruby -StructuredParams::OpenApi::DriftCheck.call( +StructuredParams::OpenAPI::DriftCheck.call( parameter_class: CreateUserParams, property_names: { user_id: 'userId' } ) @@ -210,7 +210,7 @@ StructuredParams::OpenApi::DriftCheck.call( ```ruby it 'matches the request schema' do - result = StructuredParams::OpenApi::DriftCheck.call( + result = StructuredParams::OpenAPI::DriftCheck.call( parameter_class: CreateUserParams ) @@ -225,7 +225,7 @@ end 設定します。 ```ruby -StructuredParams::OpenApi.configure do |config| +StructuredParams::OpenAPI.configure do |config| config.ignore_drift(path: 'wallet_information.wallet_type', kind: :missing_presence_validation) end ``` @@ -267,7 +267,7 @@ Registry は、例外的な component 名や property の対応付けが必要 後から登録すると、先の登録を置き換えます。 ```ruby -StructuredParams::OpenApi::Registry.register( +StructuredParams::OpenAPI::Registry.register( LegacyUserParams, schema: 'LegacyCreateUserRequest' ) @@ -278,7 +278,7 @@ StructuredParams::OpenApi::Registry.register( 登録するときに便利です。クラス定数を直接参照すると `NameError` になるためです。 ```ruby -StructuredParams::OpenApi::Registry.register( +StructuredParams::OpenAPI::Registry.register( 'LegacyUserParams', schema: 'LegacyCreateUserRequest' ) @@ -292,7 +292,7 @@ StructuredParams::OpenApi::Registry.register( フォールバックします。 ```ruby -StructuredParams::OpenApi.configure do |config| +StructuredParams::OpenAPI.configure do |config| config.parameter_class_filter = ->(klass) { klass.name&.match?(/\A(AdminApi|FrontApi)::/) } config.document_resolver = ->(klass) do case klass.name @@ -307,7 +307,7 @@ end resolver を上書きできます。どちらもなければ、設定済みの `document` を使います。 ```ruby -StructuredParams::OpenApi::Registry.register( +StructuredParams::OpenAPI::Registry.register( AdminUserParams, openapi: Rails.root.join('openapi/admin.yml').to_s ) @@ -358,7 +358,7 @@ skip は実行を失敗させませんが、黙ってもいません。Rake タ ```ruby NO_SCHEMA = %w[Api::V1::SearchParams].freeze - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.parameter_class_filter = ->(klass) do klass.name&.start_with?('Api::') && NO_SCHEMA.exclude?(klass.name) end @@ -369,7 +369,7 @@ skip は実行を失敗させませんが、黙ってもいません。Rake タ `result.ignored` に移り、Rake タスクにも一覧表示されるので、後から見直せます。 ```ruby - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.ignore_drift(kind: :missing_schema, parameter_class: 'Api::V1::SearchParams') end ``` diff --git a/structured_params-openapi/lib/structured_params/openapi.rb b/structured_params-openapi/lib/structured_params/openapi.rb new file mode 100644 index 0000000..ea04ac3 --- /dev/null +++ b/structured_params-openapi/lib/structured_params/openapi.rb @@ -0,0 +1,28 @@ +# rbs_inline: enabled +# frozen_string_literal: true + +require 'structured_params' +require_relative 'openapi/version' +require_relative 'openapi/configuration' +require_relative 'openapi/drift_check' +require_relative 'openapi/registry' + +module StructuredParams + # OpenAPI drift check for StructuredParams classes. + module OpenAPI + RAKE_TASK_PATH = File.expand_path('../tasks/structured_params_openapi.rake', __dir__) #: String + + if defined?(Rails::Railtie) + # Defers loading the drift-check rake task until Rails actually loads rake tasks, + # so requiring this gem never depends on the Rake DSL being mixed into +main+ + # (e.g. `rails runner`, `rails console`, app boot). + class Railtie < Rails::Railtie + rake_tasks do + load StructuredParams::OpenAPI::RAKE_TASK_PATH + end + end + elsif defined?(Rake::DSL) + load RAKE_TASK_PATH + end + end +end diff --git a/lib/structured_params/open_api/configuration.rb b/structured_params-openapi/lib/structured_params/openapi/configuration.rb similarity index 99% rename from lib/structured_params/open_api/configuration.rb rename to structured_params-openapi/lib/structured_params/openapi/configuration.rb index 08944ab..1bbb95b 100644 --- a/lib/structured_params/open_api/configuration.rb +++ b/structured_params-openapi/lib/structured_params/openapi/configuration.rb @@ -6,7 +6,7 @@ module StructuredParams # OpenAPI contract configuration and drift-check utilities. - module OpenApi + module OpenAPI # Global OpenAPI document and component-name convention for drift checks. class Configuration # @rbs @ignored_issues: Array[Hash[Symbol, untyped]] diff --git a/lib/structured_params/open_api/drift_check.rb b/structured_params-openapi/lib/structured_params/openapi/drift_check.rb similarity index 98% rename from lib/structured_params/open_api/drift_check.rb rename to structured_params-openapi/lib/structured_params/openapi/drift_check.rb index 0280466..fe2ea32 100644 --- a/lib/structured_params/open_api/drift_check.rb +++ b/structured_params-openapi/lib/structured_params/openapi/drift_check.rb @@ -8,7 +8,7 @@ module StructuredParams # Schema comparison types for OpenAPI drift checks. - module OpenApi + module OpenAPI # Compares OpenAPI object schemas with StructuredParams declarations. # rubocop:disable-next Metrics/ClassLength class DriftCheck @@ -69,7 +69,7 @@ def initialize( property_names: {}, required_validation: :presence ) - @openapi_source = openapi || OpenApi.configuration.document_for(parameter_class) + @openapi_source = openapi || OpenAPI.configuration.document_for(parameter_class) @schema_source = schema @parameter_class = parameter_class @pointer = pointer @@ -133,7 +133,7 @@ def load_schema end document = load_document - pointer = selected_pointer || OpenApi.configuration.pointer_for(@parameter_class, @schema_source) + pointer = selected_pointer || OpenAPI.configuration.pointer_for(@parameter_class, @schema_source) [resolve_pointer(document, pointer), document] end @@ -141,7 +141,7 @@ def load_schema def load_document return @openapi_source if @openapi_source.is_a?(Hash) - OpenApi.configuration.load_document(@openapi_source) + OpenAPI.configuration.load_document(@openapi_source) end #: () -> String? @@ -149,7 +149,7 @@ def selected_pointer return @pointer unless @pointer.nil? || @pointer.empty? return unless @schema_source.is_a?(String) - OpenApi.configuration.pointer_for(@parameter_class, @schema_source) + OpenAPI.configuration.pointer_for(@parameter_class, @schema_source) end #: (Hash[untyped, untyped], singleton(StructuredParams::Params), String) -> void @@ -512,7 +512,7 @@ def join_path(path, name) #: (String, Symbol, String) -> void def error(path, kind, message) issue = Issue.new(kind: kind, path: path, message: message) - target = OpenApi.configuration.ignored?(kind, path, @parameter_class) ? @ignored : @errors + target = OpenAPI.configuration.ignored?(kind, path, @parameter_class) ? @ignored : @errors target << issue end diff --git a/lib/structured_params/open_api/drift_check/issue.rb b/structured_params-openapi/lib/structured_params/openapi/drift_check/issue.rb similarity index 95% rename from lib/structured_params/open_api/drift_check/issue.rb rename to structured_params-openapi/lib/structured_params/openapi/drift_check/issue.rb index ea6384f..7a93181 100644 --- a/lib/structured_params/open_api/drift_check/issue.rb +++ b/structured_params-openapi/lib/structured_params/openapi/drift_check/issue.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true module StructuredParams - module OpenApi + module OpenAPI class DriftCheck # A single finding (error, skipped check, or ignored finding) from a drift check. Issue = Struct.new( diff --git a/lib/structured_params/open_api/drift_check/result.rb b/structured_params-openapi/lib/structured_params/openapi/drift_check/result.rb similarity index 99% rename from lib/structured_params/open_api/drift_check/result.rb rename to structured_params-openapi/lib/structured_params/openapi/drift_check/result.rb index 96ffa61..e9fad97 100644 --- a/lib/structured_params/open_api/drift_check/result.rb +++ b/structured_params-openapi/lib/structured_params/openapi/drift_check/result.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true module StructuredParams - module OpenApi + module OpenAPI class DriftCheck # Result of a single schema/class comparison. class Result diff --git a/lib/structured_params/open_api/registry.rb b/structured_params-openapi/lib/structured_params/openapi/registry.rb similarity index 97% rename from lib/structured_params/open_api/registry.rb rename to structured_params-openapi/lib/structured_params/openapi/registry.rb index 86c0b94..8f6e6ea 100644 --- a/lib/structured_params/open_api/registry.rb +++ b/structured_params-openapi/lib/structured_params/openapi/registry.rb @@ -3,7 +3,7 @@ module StructuredParams # OpenAPI contract registration and bulk execution utilities. - module OpenApi + module OpenAPI # Raised by Registry.check_all! when one or more registered contracts drift. class DriftError < StandardError; end @@ -74,7 +74,7 @@ def clear #: () -> Hash[singleton(StructuredParams::Params), Hash[untyped, untyped]] def auto_discovered_checks - OpenApi.configuration.parameter_classes.to_h { |parameter_class| [parameter_class, {}] } + OpenAPI.configuration.parameter_classes.to_h { |parameter_class| [parameter_class, {}] } end #: () -> Array[Array[untyped]] diff --git a/structured_params-openapi/lib/structured_params/openapi/version.rb b/structured_params-openapi/lib/structured_params/openapi/version.rb new file mode 100644 index 0000000..531f6a5 --- /dev/null +++ b/structured_params-openapi/lib/structured_params/openapi/version.rb @@ -0,0 +1,8 @@ +# rbs_inline: enabled +# frozen_string_literal: true + +module StructuredParams + module OpenAPI + VERSION = '0.1.0' #: String + end +end diff --git a/lib/tasks/structured_params_open_api.rake b/structured_params-openapi/lib/tasks/structured_params_openapi.rake similarity index 91% rename from lib/tasks/structured_params_open_api.rake rename to structured_params-openapi/lib/tasks/structured_params_openapi.rake index 321652a..d3d7f28 100644 --- a/lib/tasks/structured_params_open_api.rake +++ b/structured_params-openapi/lib/tasks/structured_params_openapi.rake @@ -6,7 +6,7 @@ namespace :structured_params do desc 'Check registered and auto-discovered StructuredParams classes against the OpenAPI document' task drift_check: :environment do Rails.application&.eager_load! if defined?(Rails) && Rails.respond_to?(:application) - registry = StructuredParams::OpenApi::Registry + registry = StructuredParams::OpenAPI::Registry # Set STRUCTURED_PARAMS_STRICT=1 to fail the task when any check was skipped. results = registry.check_all!(auto_discover: true, strict: ENV['STRUCTURED_PARAMS_STRICT'] == '1') puts registry.report(results) diff --git a/structured_params-openapi/sig/structured_params/openapi.rbs b/structured_params-openapi/sig/structured_params/openapi.rbs new file mode 100644 index 0000000..48a26cc --- /dev/null +++ b/structured_params-openapi/sig/structured_params/openapi.rbs @@ -0,0 +1,14 @@ +# Generated from lib/structured_params/openapi.rb with RBS::Inline + +module StructuredParams + # OpenAPI drift check for StructuredParams classes. + module OpenAPI + RAKE_TASK_PATH: String + + # Defers loading the drift-check rake task until Rails actually loads rake tasks, + # so requiring this gem never depends on the Rake DSL being mixed into +main+ + # (e.g. `rails runner`, `rails console`, app boot). + class Railtie < Rails::Railtie + end + end +end diff --git a/sig/structured_params/open_api/configuration.rbs b/structured_params-openapi/sig/structured_params/openapi/configuration.rbs similarity index 97% rename from sig/structured_params/open_api/configuration.rbs rename to structured_params-openapi/sig/structured_params/openapi/configuration.rbs index 485495e..301c69e 100644 --- a/sig/structured_params/open_api/configuration.rbs +++ b/structured_params-openapi/sig/structured_params/openapi/configuration.rbs @@ -1,8 +1,8 @@ -# Generated from lib/structured_params/open_api/configuration.rb with RBS::Inline +# Generated from lib/structured_params/openapi/configuration.rb with RBS::Inline module StructuredParams # OpenAPI contract configuration and drift-check utilities. - module OpenApi + module OpenAPI # Global OpenAPI document and component-name convention for drift checks. class Configuration @ignored_issues: Array[Hash[Symbol, untyped]] diff --git a/sig/structured_params/open_api/drift_check.rbs b/structured_params-openapi/sig/structured_params/openapi/drift_check.rbs similarity index 98% rename from sig/structured_params/open_api/drift_check.rbs rename to structured_params-openapi/sig/structured_params/openapi/drift_check.rbs index e6c7ccc..6942ff4 100644 --- a/sig/structured_params/open_api/drift_check.rbs +++ b/structured_params-openapi/sig/structured_params/openapi/drift_check.rbs @@ -1,8 +1,8 @@ -# Generated from lib/structured_params/open_api/drift_check.rb with RBS::Inline +# Generated from lib/structured_params/openapi/drift_check.rb with RBS::Inline module StructuredParams # Schema comparison types for OpenAPI drift checks. - module OpenApi + module OpenAPI # Compares OpenAPI object schemas with StructuredParams declarations. # rubocop:disable-next Metrics/ClassLength class DriftCheck diff --git a/sig/structured_params/open_api/drift_check/issue.rbs b/structured_params-openapi/sig/structured_params/openapi/drift_check/issue.rbs similarity index 82% rename from sig/structured_params/open_api/drift_check/issue.rbs rename to structured_params-openapi/sig/structured_params/openapi/drift_check/issue.rbs index 73c5dda..9149311 100644 --- a/sig/structured_params/open_api/drift_check/issue.rbs +++ b/structured_params-openapi/sig/structured_params/openapi/drift_check/issue.rbs @@ -1,7 +1,7 @@ -# Generated from lib/structured_params/open_api/drift_check/issue.rb with RBS::Inline +# Generated from lib/structured_params/openapi/drift_check/issue.rb with RBS::Inline module StructuredParams - module OpenApi + module OpenAPI class DriftCheck # A single finding (error, skipped check, or ignored finding) from a drift check. class Issue < Struct[Symbol | String] diff --git a/sig/structured_params/open_api/drift_check/result.rbs b/structured_params-openapi/sig/structured_params/openapi/drift_check/result.rbs similarity index 91% rename from sig/structured_params/open_api/drift_check/result.rbs rename to structured_params-openapi/sig/structured_params/openapi/drift_check/result.rbs index 98e029f..f7208fb 100644 --- a/sig/structured_params/open_api/drift_check/result.rbs +++ b/structured_params-openapi/sig/structured_params/openapi/drift_check/result.rbs @@ -1,7 +1,7 @@ -# Generated from lib/structured_params/open_api/drift_check/result.rb with RBS::Inline +# Generated from lib/structured_params/openapi/drift_check/result.rb with RBS::Inline module StructuredParams - module OpenApi + module OpenAPI class DriftCheck # Result of a single schema/class comparison. class Result diff --git a/sig/structured_params/open_api/registry.rbs b/structured_params-openapi/sig/structured_params/openapi/registry.rbs similarity index 95% rename from sig/structured_params/open_api/registry.rbs rename to structured_params-openapi/sig/structured_params/openapi/registry.rbs index 06cd24b..2f48834 100644 --- a/sig/structured_params/open_api/registry.rbs +++ b/structured_params-openapi/sig/structured_params/openapi/registry.rbs @@ -1,8 +1,8 @@ -# Generated from lib/structured_params/open_api/registry.rb with RBS::Inline +# Generated from lib/structured_params/openapi/registry.rb with RBS::Inline module StructuredParams # OpenAPI contract registration and bulk execution utilities. - module OpenApi + module OpenAPI # Raised by Registry.check_all! when one or more registered contracts drift. class DriftError < StandardError end diff --git a/structured_params-openapi/sig/structured_params/openapi/version.rbs b/structured_params-openapi/sig/structured_params/openapi/version.rbs new file mode 100644 index 0000000..be20ff9 --- /dev/null +++ b/structured_params-openapi/sig/structured_params/openapi/version.rbs @@ -0,0 +1,7 @@ +# Generated from lib/structured_params/openapi/version.rb with RBS::Inline + +module StructuredParams + module OpenAPI + VERSION: String + end +end diff --git a/spec/open_api/configuration_spec.rb b/structured_params-openapi/spec/openapi/configuration_spec.rb similarity index 86% rename from spec/open_api/configuration_spec.rb rename to structured_params-openapi/spec/openapi/configuration_spec.rb index a1a120e..8000dcd 100644 --- a/spec/open_api/configuration_spec.rb +++ b/structured_params-openapi/spec/openapi/configuration_spec.rb @@ -4,8 +4,8 @@ require 'tempfile' require 'fileutils' -RSpec.describe StructuredParams::OpenApi::Configuration do - after { StructuredParams::OpenApi.reset_configuration! } +RSpec.describe StructuredParams::OpenAPI::Configuration do + after { StructuredParams::OpenAPI.reset_configuration! } let(:parameter_class) do stub_const('OpenApiRequestParams', Class.new(StructuredParams::Params)).tap do |klass| @@ -29,9 +29,9 @@ end it 'uses the configured document and default class-name convention' do - StructuredParams::OpenApi.configure { |config| config.document = document } + StructuredParams::OpenAPI.configure { |config| config.document = document } - result = StructuredParams::OpenApi::DriftCheck.call(parameter_class: parameter_class) + result = StructuredParams::OpenAPI::DriftCheck.call(parameter_class: parameter_class) expect(result).to be_valid end @@ -39,42 +39,42 @@ it 'discovers only classes selected by the configured filter' do stub_const('Api', Module.new) parameter_class = stub_const('Api::CreateUserParams', Class.new(StructuredParams::Params)) - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.parameter_class_filter = ->(klass) { klass.name&.start_with?('Api::') } end - expect(StructuredParams::OpenApi.configuration.parameter_classes).to include(parameter_class) + expect(StructuredParams::OpenAPI.configuration.parameter_classes).to include(parameter_class) end describe '#ignore_drift and #ignored?' do it 'matches only the given kind at the given path when kind is specified' do - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.ignore_drift(path: 'wallet_information.wallet_type', kind: :missing_presence_validation) end - expect(StructuredParams::OpenApi.configuration.ignored?(:missing_presence_validation, + expect(StructuredParams::OpenAPI.configuration.ignored?(:missing_presence_validation, 'wallet_information.wallet_type')).to be true - expect(StructuredParams::OpenApi.configuration.ignored?(:type_mismatch, + expect(StructuredParams::OpenAPI.configuration.ignored?(:type_mismatch, 'wallet_information.wallet_type')).to be false - expect(StructuredParams::OpenApi.configuration.ignored?(:missing_presence_validation, 'other_path')).to be false + expect(StructuredParams::OpenAPI.configuration.ignored?(:missing_presence_validation, 'other_path')).to be false end it 'matches every kind at the path when kind is omitted' do - StructuredParams::OpenApi.configure { |config| config.ignore_drift(path: 'legacy_field') } + StructuredParams::OpenAPI.configure { |config| config.ignore_drift(path: 'legacy_field') } - expect(StructuredParams::OpenApi.configuration.ignored?(:type_mismatch, 'legacy_field')).to be true - expect(StructuredParams::OpenApi.configuration.ignored?(:missing_attribute, 'legacy_field')).to be true + expect(StructuredParams::OpenAPI.configuration.ignored?(:type_mismatch, 'legacy_field')).to be true + expect(StructuredParams::OpenAPI.configuration.ignored?(:missing_attribute, 'legacy_field')).to be true end end describe '#ignore_drift' do it 'rejects an unknown kind so a typo cannot silently never match' do - expect { StructuredParams::OpenApi.configuration.ignore_drift(path: 'a', kind: :type_mismtach) } + expect { StructuredParams::OpenAPI.configuration.ignore_drift(path: 'a', kind: :type_mismtach) } .to raise_error(ArgumentError, /unknown kind/) end it 'matches every path when path is omitted' do - configuration = StructuredParams::OpenApi.configuration + configuration = StructuredParams::OpenAPI.configuration configuration.ignore_drift(kind: :missing_schema, parameter_class: 'ScopedParams') expect(configuration.ignored?(:missing_schema, '(root)', 'ScopedParams')).to be true @@ -84,11 +84,11 @@ end it 'requires at least one narrowing argument so a rule cannot ignore everything' do - expect { StructuredParams::OpenApi.configuration.ignore_drift }.to raise_error(ArgumentError, /at least one/) + expect { StructuredParams::OpenAPI.configuration.ignore_drift }.to raise_error(ArgumentError, /at least one/) end it 'scopes a rule to the given parameter class' do - configuration = StructuredParams::OpenApi.configuration + configuration = StructuredParams::OpenAPI.configuration configuration.ignore_drift(path: 'a', parameter_class: 'ScopedParams') expect(configuration.ignored?(:type_mismatch, 'a', 'ScopedParams')).to be true diff --git a/spec/open_api/drift_check/result_spec.rb b/structured_params-openapi/spec/openapi/drift_check/result_spec.rb similarity index 95% rename from spec/open_api/drift_check/result_spec.rb rename to structured_params-openapi/spec/openapi/drift_check/result_spec.rb index 6099730..b67a41b 100644 --- a/spec/open_api/drift_check/result_spec.rb +++ b/structured_params-openapi/spec/openapi/drift_check/result_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' -RSpec.describe StructuredParams::OpenApi::DriftCheck::Result do +RSpec.describe StructuredParams::OpenAPI::DriftCheck::Result do def issue(kind, path, message) - StructuredParams::OpenApi::DriftCheck::Issue.new(kind: kind, path: path, message: message) + StructuredParams::OpenAPI::DriftCheck::Issue.new(kind: kind, path: path, message: message) end let(:parameter_class) { Class.new(StructuredParams::Params) } diff --git a/spec/open_api/drift_check_spec.rb b/structured_params-openapi/spec/openapi/drift_check_spec.rb similarity index 98% rename from spec/open_api/drift_check_spec.rb rename to structured_params-openapi/spec/openapi/drift_check_spec.rb index 5d6a748..653ff66 100644 --- a/spec/open_api/drift_check_spec.rb +++ b/structured_params-openapi/spec/openapi/drift_check_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' require 'tempfile' -RSpec.describe StructuredParams::OpenApi::DriftCheck do +RSpec.describe StructuredParams::OpenAPI::DriftCheck do describe '.call' do let(:schema) do { @@ -65,14 +65,14 @@ it 'moves a finding matching a configured Configuration#ignore rule to result.ignored' do parameter_class._validators[:age].clear - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.ignore_drift(path: 'age', kind: :missing_presence_validation) end begin result = described_class.call(schema: schema, parameter_class: parameter_class) ensure - StructuredParams::OpenApi.reset_configuration! + StructuredParams::OpenAPI.reset_configuration! end expect(result).to be_valid @@ -208,7 +208,7 @@ it 'lets a missing schema be ignored for one class only' do document = { 'components' => { 'schemas' => { 'Other' => schema } } } - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.ignore_drift(kind: :missing_schema, parameter_class: parameter_class) end @@ -219,7 +219,7 @@ expect(result.ignored.map(&:kind)).to eq([:missing_schema]) expect(other).not_to be_valid ensure - StructuredParams::OpenApi.reset_configuration! + StructuredParams::OpenAPI.reset_configuration! end it 'reports a pointer that runs past the end of an array as a missing schema' do diff --git a/spec/open_api/railtie_loading_spec.rb b/structured_params-openapi/spec/openapi/railtie_loading_spec.rb similarity index 97% rename from spec/open_api/railtie_loading_spec.rb rename to structured_params-openapi/spec/openapi/railtie_loading_spec.rb index 3e8ac61..72065a0 100644 --- a/spec/open_api/railtie_loading_spec.rb +++ b/structured_params-openapi/spec/openapi/railtie_loading_spec.rb @@ -12,7 +12,7 @@ # and `task` are never mixed into `main`. Reproduce that exact condition # instead of a full `require 'rake'`, which mixes the DSL in as a side effect. lib_path = File.expand_path('../../lib', __dir__) - script = "require 'rake/file_utils_ext'; require 'structured_params'; puts 'ok'" + script = "require 'rake/file_utils_ext'; require 'structured_params/openapi'; puts 'ok'" output, status = Open3.capture2e('bundle', 'exec', 'ruby', '-I', lib_path, '-e', script) diff --git a/spec/open_api/registry_spec.rb b/structured_params-openapi/spec/openapi/registry_spec.rb similarity index 90% rename from spec/open_api/registry_spec.rb rename to structured_params-openapi/spec/openapi/registry_spec.rb index e257dc7..9b76bd7 100644 --- a/spec/open_api/registry_spec.rb +++ b/structured_params-openapi/spec/openapi/registry_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe StructuredParams::OpenApi::Registry do +RSpec.describe StructuredParams::OpenAPI::Registry do after do described_class.clear - StructuredParams::OpenApi.reset_configuration! + StructuredParams::OpenAPI.reset_configuration! end it 'runs registered checks and raises when any of them drift' do @@ -13,11 +13,11 @@ schema = { 'type' => 'object', 'properties' => { 'age' => { 'type' => 'integer' } } } described_class.register(parameter_class, schema: schema) - expect { described_class.check_all! }.to raise_error(StructuredParams::OpenApi::DriftError, /drift detected/) + expect { described_class.check_all! }.to raise_error(StructuredParams::OpenAPI::DriftError, /drift detected/) end context 'with an auto-discovery filter' do - let(:parameter_class) do + let!(:parameter_class) do stub_const('Api', Module.new) stub_const('Api::CreateUserParams', Class.new(StructuredParams::Params)).tap do |klass| klass.attribute :name, :string @@ -37,7 +37,7 @@ } } } - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.document = document config.parameter_class_filter = ->(klass) { klass.equal?(parameter_class) } end @@ -81,14 +81,14 @@ it 'raises for a skipped-only result in strict mode' do expect { described_class.check_all!(strict: true) } - .to raise_error(StructuredParams::OpenApi::DriftError, /checks skipped/) + .to raise_error(StructuredParams::OpenAPI::DriftError, /checks skipped/) end it 'includes skips in the message when the class also has errors' do skipping_schema['properties']['age'] = { 'type' => 'integer' } expect { described_class.check_all! } - .to raise_error(StructuredParams::OpenApi::DriftError, /drift detected.*checks skipped/m) + .to raise_error(StructuredParams::OpenAPI::DriftError, /drift detected.*checks skipped/m) end it 'joins the messages of every failing class' do @@ -97,7 +97,7 @@ skipping_schema['properties']['age'] = { 'type' => 'integer' } expect { described_class.check_all! } - .to raise_error(StructuredParams::OpenApi::DriftError) { |error| expect(error.message.scan('drift detected').size).to eq(2) } + .to raise_error(StructuredParams::OpenAPI::DriftError) { |error| expect(error.message.scan('drift detected').size).to eq(2) } end end @@ -106,7 +106,7 @@ expect(described_class.report([])).to include('No parameter classes were checked') parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } - StructuredParams::OpenApi.configure { |config| config.ignore_drift(path: 'age') } + StructuredParams::OpenAPI.configure { |config| config.ignore_drift(path: 'age') } described_class.register(parameter_class, schema: { 'type' => 'object', 'properties' => { 'age' => { 'type' => 'integer' } } }) @@ -129,7 +129,7 @@ other_document = { 'components' => { 'schemas' => { 'Admin' => { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string' } } } } } } - StructuredParams::OpenApi.configure { |config| config.document = { 'components' => { 'schemas' => {} } } } + StructuredParams::OpenAPI.configure { |config| config.document = { 'components' => { 'schemas' => {} } } } described_class.register(parameter_class, openapi: other_document, schema: 'Admin') expect(described_class.check_all).to all(be_valid) @@ -148,7 +148,7 @@ def document_with(property) stub_const(name, Class.new(StructuredParams::Params)).tap { |klass| klass.attribute attribute, :string } end documents = { 'AdminApi::UserParams' => document_with('role'), 'FrontApi::UserParams' => document_with('nickname') } - StructuredParams::OpenApi.configure do |config| + StructuredParams::OpenAPI.configure do |config| config.parameter_class_filter = ->(klass) { classes.include?(klass) } config.document_resolver = ->(klass) { documents.fetch(klass.name) } end diff --git a/structured_params-openapi/spec/spec_helper.rb b/structured_params-openapi/spec/spec_helper.rb new file mode 100644 index 0000000..bdced2f --- /dev/null +++ b/structured_params-openapi/spec/spec_helper.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'structured_params' +require 'structured_params/openapi' + +StructuredParams.register_types + +RSpec.configure do |config| + # Enable flags like --only-failures and --next-failure + config.example_status_persistence_file_path = '.rspec_status' + + # Disable RSpec exposing methods globally on `Module` and `main` + config.disable_monkey_patching! + + config.expect_with :rspec do |c| + c.syntax = :expect + end +end diff --git a/spec/tasks/structured_params_open_api_spec.rb b/structured_params-openapi/spec/tasks/structured_params_openapi_spec.rb similarity index 85% rename from spec/tasks/structured_params_open_api_spec.rb rename to structured_params-openapi/spec/tasks/structured_params_openapi_spec.rb index f0882df..eb02013 100644 --- a/spec/tasks/structured_params_open_api_spec.rb +++ b/structured_params-openapi/spec/tasks/structured_params_openapi_spec.rb @@ -9,22 +9,22 @@ around do |example| original_application = Rake.application Rake.application = Rake::Application.new - load File.expand_path('../../lib/tasks/structured_params_open_api.rake', __dir__) + load File.expand_path('../../lib/tasks/structured_params_openapi.rake', __dir__) example.run ensure Rake.application = original_application end - after { StructuredParams::OpenApi::Registry.clear } + after { StructuredParams::OpenAPI::Registry.clear } it 'runs all registered drift checks after the Rails environment task' do described_class.define_task(:environment) parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } schema = { 'type' => 'object', 'properties' => { 'age' => { 'type' => 'integer' } } } - StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) + StructuredParams::OpenAPI::Registry.register(parameter_class, schema: schema) expect { drift_check_task.invoke } - .to raise_error(StructuredParams::OpenApi::DriftError, /drift detected/) + .to raise_error(StructuredParams::OpenAPI::DriftError, /drift detected/) end context 'when every check passes' do @@ -32,7 +32,7 @@ described_class.define_task(:environment) parameter_class = Class.new(StructuredParams::Params) { attribute :name, :string } schema = { 'type' => 'object', 'properties' => { 'name' => { 'type' => %w[string integer] } } } - StructuredParams::OpenApi::Registry.register(parameter_class, schema: schema) + StructuredParams::OpenAPI::Registry.register(parameter_class, schema: schema) end it 'prints skipped checks without failing' do @@ -42,7 +42,7 @@ it 'fails on skipped checks when STRUCTURED_PARAMS_STRICT=1' do stub_const('ENV', ENV.to_hash.merge('STRUCTURED_PARAMS_STRICT' => '1')) - expect { drift_check_task.invoke }.to raise_error(StructuredParams::OpenApi::DriftError, /checks skipped/) + expect { drift_check_task.invoke }.to raise_error(StructuredParams::OpenAPI::DriftError, /checks skipped/) end end diff --git a/structured_params-openapi/structured_params-openapi.gemspec b/structured_params-openapi/structured_params-openapi.gemspec new file mode 100644 index 0000000..cc78c25 --- /dev/null +++ b/structured_params-openapi/structured_params-openapi.gemspec @@ -0,0 +1,39 @@ +# frozen_string_literal: true +# rbs_inline: enabled + +require_relative 'lib/structured_params/openapi/version' + +Gem::Specification.new do |spec| + spec.name = 'structured_params-openapi' + spec.version = StructuredParams::OpenAPI::VERSION + spec.authors = ['Mizuki Yamamoto'] + spec.email = ['mizuki-y@syati.info'] + + spec.summary = 'OpenAPI drift check for StructuredParams.' + spec.description = ( + 'Detects differences between OpenAPI request schemas and StructuredParams classes, ' \ + 'with a Rake task for CI, JSON Pointer and component naming conventions, and ' \ + 'support for several OpenAPI documents.' + ) + spec.homepage = 'https://github.com/Syati/structured_params' + spec.license = 'MIT' + spec.required_ruby_version = '>= 3.2.0' + spec.require_paths = ['lib'] + spec.files = Dir[ + 'LICENSE.txt', + 'README*.md', + 'docs/**/*.md', + 'lib/**/*.rb', + 'lib/tasks/**/*.rake', + 'sig/**/*.rbs' + ] + + spec.metadata = { + 'homepage_uri' => 'https://github.com/Syati/structured_params/tree/main/structured_params-openapi#readme', + 'bug_tracker_uri' => 'https://github.com/Syati/structured_params/issues', + 'source_code_uri' => 'https://github.com/Syati/structured_params/tree/main/structured_params-openapi', + 'rubygems_mfa_required' => 'true' + } + + spec.add_dependency 'structured_params', '>= 0.9.5' +end diff --git a/structured_params.gemspec b/structured_params.gemspec index 85465c9..0489492 100644 --- a/structured_params.gemspec +++ b/structured_params.gemspec @@ -23,7 +23,6 @@ Gem::Specification.new do |spec| 'LICENSE.txt', 'CHANGELOG.md', 'lib/**/*.rb', - 'lib/tasks/**/*.rake', 'sig/**/*.rbs' ] From 5191e688b71e086f45cee5144ee1d1320f59428a Mon Sep 17 00:00:00 2001 From: mizuki-y Date: Mon, 12 Oct 2026 21:17:48 +0900 Subject: [PATCH 12/12] fix: address review comments on the OpenAPI drift check - report a dangling $ref inside a scalar oneOf/anyOf as :unresolved_reference instead of silencing it and only skipping the union - scope the Style/OptionalBooleanParameter suppression in errors.rb to the two methods that need it instead of the whole module Co-Authored-By: Claude Sonnet 5.5 --- lib/structured_params/errors.rb | 3 ++- sig/structured_params/errors.rbs | 3 ++- .../lib/structured_params/openapi/drift_check.rb | 3 ++- .../spec/openapi/drift_check_spec.rb | 10 ++++++++++ 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/structured_params/errors.rb b/lib/structured_params/errors.rb index 0e39904..1284e66 100644 --- a/lib/structured_params/errors.rb +++ b/lib/structured_params/errors.rb @@ -1,7 +1,6 @@ # rbs_inline: enabled # frozen_string_literal: true -# rubocop:disable-next Style/OptionalBooleanParameter module StructuredParams # Custom errors collection that handles nested attribute names class Errors < ActiveModel::Errors @@ -9,6 +8,7 @@ class Errors < ActiveModel::Errors # Add structured option to get nested structure for dot-notation attributes #: (?bool, ?structured: false) -> Hash[Symbol, String] #: (?bool, structured: bool) -> Hash[Symbol, untyped] + # rubocop:disable-next Style/OptionalBooleanParameter def to_hash(full_messages = false, structured: false) if structured attribute_messages_hash = build_attribute_messages_hash(full_messages) @@ -43,6 +43,7 @@ def messages(structured: false) # Build a hash with attribute names as keys and their error messages as values # This is used for to_hash(structured: true) #: (bool) -> Hash[Symbol, Array[String]] + # rubocop:disable-next Style/OptionalBooleanParameter def build_attribute_messages_hash(full_messages = false) message_method = full_messages ? :full_message : :message diff --git a/sig/structured_params/errors.rbs b/sig/structured_params/errors.rbs index a738cf4..747b309 100644 --- a/sig/structured_params/errors.rbs +++ b/sig/structured_params/errors.rbs @@ -1,6 +1,5 @@ # Generated from lib/structured_params/errors.rb with RBS::Inline -# rubocop:disable-next Style/OptionalBooleanParameter module StructuredParams # Custom errors collection that handles nested attribute names class Errors < ActiveModel::Errors @@ -8,6 +7,7 @@ module StructuredParams # Add structured option to get nested structure for dot-notation attributes # : (?bool, ?structured: false) -> Hash[Symbol, String] # : (?bool, structured: bool) -> Hash[Symbol, untyped] + # rubocop:disable-next Style/OptionalBooleanParameter def to_hash: (?bool, ?structured: false) -> Hash[Symbol, String] | (?bool, structured: bool) -> Hash[Symbol, untyped] @@ -26,6 +26,7 @@ module StructuredParams # Build a hash with attribute names as keys and their error messages as values # This is used for to_hash(structured: true) # : (bool) -> Hash[Symbol, Array[String]] + # rubocop:disable-next Style/OptionalBooleanParameter def build_attribute_messages_hash: (bool) -> Hash[Symbol, Array[String]] # Build a nested hash structure from flat dot-notation keys diff --git a/structured_params-openapi/lib/structured_params/openapi/drift_check.rb b/structured_params-openapi/lib/structured_params/openapi/drift_check.rb index fe2ea32..c8cb011 100644 --- a/structured_params-openapi/lib/structured_params/openapi/drift_check.rb +++ b/structured_params-openapi/lib/structured_params/openapi/drift_check.rb @@ -359,7 +359,8 @@ def scalar_union_types(schema, path) def scalar_option_type(option, path) return unless option.is_a?(Hash) - resolved = without_reporting { resolve_references(option, path) } + # Not silenced: a dangling $ref in an option must surface as :unresolved_reference. + resolved = resolve_references(option, path) schema_type(resolved) unless value_for(resolved, 'properties') end diff --git a/structured_params-openapi/spec/openapi/drift_check_spec.rb b/structured_params-openapi/spec/openapi/drift_check_spec.rb index 653ff66..0d0238b 100644 --- a/structured_params-openapi/spec/openapi/drift_check_spec.rb +++ b/structured_params-openapi/spec/openapi/drift_check_spec.rb @@ -113,6 +113,16 @@ expect(result.skipped).to be_empty end + it 'reports a dangling $ref inside a scalar oneOf once instead of only skipping' do + options = [{ '$ref' => '#/components/schemas/Missing' }, { 'type' => 'string' }] + schema['properties']['age'] = { 'oneOf' => options } + document = { 'components' => { 'schemas' => { 'Request' => schema } } } + + result = described_class.call(openapi: document, schema: 'Request', parameter_class: parameter_class) + + expect(result.errors).to contain_exactly(have_attributes(kind: :unresolved_reference, path: 'age')) + end + it 'compares oneOf/anyOf with a null option and a type array with null as nullable schemas' do schema['properties']['age'] = { 'oneOf' => [{ 'type' => 'string' }, { 'type' => 'null' }] } schema['properties']['name'] = { 'type' => %w[string null] }