Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,21 @@ jobs:
- name: Replay decoders cover every live fixture operation
run: make check-replay-decoder-parity

# `smithy validate` checks `@examples` against the SMITHY model, where a
# member that `jsonAdd` later appends to a schema's `required` array is
# still natively optional. Nothing compared the projected examples to the
# projected schema, so #637 published two `GetTodolistOrGroup` examples
# that could not satisfy their own contract and a bot reviewer, not CI,
# caught it. The target also runs the gate's self-test, whose first case
# is that defect reproduced exactly.
#
# Under LC_ALL=C so CI exercises the non-UTF-8-locale path (the reads are
# pinned to UTF-8; this proves it stays that way).
- name: Published examples satisfy the schema the projection publishes
run: make check-projected-examples
env:
LC_ALL: C

# Advisory: prints and exits 0. The guarantee is the byte comparison that
# runs at the end of every job that installs anything. This is the early
# warning the byte comparison cannot give — it names the file and line at
Expand Down
18 changes: 16 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,7 @@ tools:
# Spec-shape lints
#------------------------------------------------------------------------------

.PHONY: check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-fixture-coverage check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged
.PHONY: check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-fixture-coverage check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged check-projected-examples

# Verify every bucket-scoped GET list operation has a flat-path counterpart
# (or is justified in spec/bucket-scoped-allowlist.txt). Cross-project SDK
Expand Down Expand Up @@ -1068,6 +1068,20 @@ check-fixture-coverage:
@./scripts/check-fixture-coverage.sh
@ruby ./scripts/test-check-fixture-coverage.rb

# Projected-example guard: every example openapi.json publishes must satisfy
# the schema it sits under IN THE PROJECTION. `smithy validate` checks
# `@examples` against the Smithy model, where a member that `jsonAdd` later
# appends to a schema's `required` array is still natively optional — so a
# projection-added required field can go missing from a published example
# with every other gate green. That shipped in #637 and a bot reviewer, not
# CI, caught it (#638). Reuses scripts/schema_instance_validator.rb, the same
# composition-aware walk check-fixture-coverage uses. The self-test asserts
# the gate rejects each crafted failure mode; the live check above only ever
# exercises the valid spec.
check-projected-examples:
@ruby ./scripts/check-projected-examples.rb
@ruby ./scripts/test-check-projected-examples.rb

# Presence contract for generated Kotlin ARRAY and PRIMITIVE SCALAR properties:
# optional -> `T? = null`, required -> `T`, required-and-nullable -> `T?` with no
# default, and no zero-value sentinel defaults (`= emptyList()`, `= 0`, `= false`,
Expand Down Expand Up @@ -1252,7 +1266,7 @@ check:
if [ $$rc -ne 0 ]; then exit $$rc; fi; \
echo "==> All checks passed"

check-targets: lint-actions sync-spec-version-check smithy-check smithy-mapper-test behavior-model-check provenance-check sync-api-version-check doc-constants-check url-routes-check bc3-route-parity test-bc3-route-parity go-check-drift go-check-wrapper-drift go-check-generated-drift auth-routable-check kt-check-drift swift-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps check-deprecation-parity check-fixture-coverage kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged
check-targets: lint-actions sync-spec-version-check smithy-check smithy-mapper-test behavior-model-check provenance-check sync-api-version-check doc-constants-check url-routes-check bc3-route-parity test-bc3-route-parity go-check-drift go-check-wrapper-drift go-check-generated-drift auth-routable-check kt-check-drift swift-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps check-deprecation-parity check-fixture-coverage kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged check-projected-examples
@:

# Clean all build artifacts
Expand Down
256 changes: 20 additions & 236 deletions scripts/check-fixture-coverage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,39 +27,31 @@
# components, and do not overlap `covered_schemas`.
#
# Uses conformance/runner/ruby/schema-walker.rb only for `find_response_schema`
# (operation-entry response-schema lookup). The required/type/nullability walk is
# implemented here because it must be composition-aware, which the walker is not.
# Wired into `make check` in the scripts/validate-api-gaps.rb style (stdlib only).
# (operation-entry response-schema lookup). The required/type/nullability walk
# cannot come from the walker because it must be composition-aware, which the
# walker is not; it lives in scripts/schema_instance_validator.rb, extracted so
# scripts/check-projected-examples.rb reuses the same walk instead of growing a
# parallel one. Wired into `make check` in the scripts/validate-api-gaps.rb style
# (stdlib only).
#
# Paths default to the repo layout but honour FIXTURE_MANIFEST / FIXTURE_OPENAPI
# / FIXTURE_DIR env overrides so the negative-case self-test
# (scripts/test-check-fixture-coverage.rb) can point it at crafted inputs.
#
# --- Scope: a deliberately PARTIAL structural validator ------------------------
#
# This is NOT a complete JSON-Schema / OpenAPI validator. It validates the subset
# that keeps a fixture structurally faithful to the generated types:
# * required-field presence
# * declared types (with integer/number integrality) and nullability
# * arrays and array-element typing/nullability
# * $ref (incl. 3.1 $ref-with-siblings) and allOf conjunction (required unioned;
# duplicate properties and array `items` conjoined; type constraints
# intersected; nullability = every part permits null)
# * anyOf/oneOf as AT-LEAST-ONE (the value must satisfy some branch; a group is
# nullable only if a branch is)
#
# Intentionally NOT implemented (out of scope unless a case is reached by the
# actual generated schemas): exact-one `oneOf` selection, `enum`, `const`,
# discriminators, `pattern`, `format`, numeric bounds, `additionalProperties`,
# `uniqueItems`, and other assertion keywords. Findings about these are declined
# unless they affect the current generated `openapi.json` or this documented
# subset.
# The instance walk is a deliberately PARTIAL structural validator — see the
# scope note in scripts/schema_instance_validator.rb for exactly what it does and
# does not assert.

require "json"
require "yaml"
require "set"

require_relative "../conformance/runner/ruby/schema-walker"
require_relative "schema_instance_validator"

# Receiverless `instance_errors(...)` / `merged_constraints(...)` / `ref_name(...)`
# below, exactly as when they were defined in this file.
include SchemaInstanceValidator # rubocop:disable Style/MixinUsage

PROJECT_ROOT = File.expand_path("..", __dir__)
MANIFEST_FILE = ENV.fetch("FIXTURE_MANIFEST", File.join(PROJECT_ROOT, "spec/fixtures/manifest.yaml"))
Expand Down Expand Up @@ -112,220 +104,12 @@ def resolve_pointer(doc, pointer)
end

# --- Schema helpers ------------------------------------------------------------

# Ruby name of a `$ref`, or nil.
def ref_name(schema)
return nil unless schema.is_a?(Hash) && schema["$ref"].is_a?(String)

schema["$ref"].match(%r{/components/schemas/(.+)\z})&.captures&.first
end

# Returns [Set(declared non-null json-type strings), nullable?] for a single
# schema node (no traversal). Handles OpenAPI 3.1 null-union (`type:[X,"null"]`)
# and 3.0 `nullable:true`. An empty set means the node declares no type.
def allowed_types(schema)
return [Set.new, false] unless schema.is_a?(Hash)

t = schema["type"]
case t
when Array
members = t.compact
non_null = members - ["null"]
# A union of only ["null"] still constrains the value to null — keep "null"
# as the type so a non-null value fails (an empty set would be unconstrained).
types = non_null.empty? ? Set.new(["null"]) : Set.new(non_null)
[types, members.include?("null")]
when String
# OpenAPI 3.1 scalar null type: the value must BE null — a real "null" type
# constraint (so a non-null value fails), and it is nullable. Returning an
# empty type set would wrongly impose no constraint at all.
return [Set.new(["null"]), true] if t == "null"

[Set.new([t]), schema["nullable"] == true]
else
[Set.new, schema["nullable"] == true]
end
end

def json_type(value)
case value
when Hash then "object"
when Array then "array"
when String then "string"
when true, false then "boolean"
when Integer then "integer"
when Float then "number"
else "null"
end
end

# True when `value`'s JSON type satisfies the declared `types`. integer/number
# interchange with one guard: a float supplied for an integer-only field passes
# only when it is mathematically integral (so FlexInt `1024.0` passes but `1.5`
# fails).
def type_matches?(types, value)
return true if types.empty?

actual = json_type(value)
return true if types.include?(actual)

if actual == "number" && types.include?("integer") && !types.include?("number")
return value.is_a?(Float) && value.finite? && value == value.truncate
end
return true if actual == "integer" && types.include?("number")

false
end

# Merges a schema's effective constraints across `$ref` (including 3.1
# `$ref`-with-siblings) and `allOf`, returning
# [required(Array), properties(Hash), types(Set), nullable(bool), items(schema),
# alt_groups(Array of branch-arrays), type_sets(Array of Sets)].
# `allOf` is a conjunction: required is unioned, and properties AND array `items`
# constrained by more than one branch are conjoined (allOf-wrapped) so a value
# must satisfy all of them; `type_sets` holds each part's declared type-set (a
# value must match every one). `anyOf`/`oneOf` are alternatives: their branches
# are NOT merged (that would over-require) but each group is collected so
# validation can require at least one branch to match — including groups
# inherited through `$ref` and `allOf`. `visited` (component names) + depth guard
# terminate reference/composition cycles.
def merged_constraints(schema, components, visited = Set.new, depth = 0)
req = []
props = {}
types = Set.new # union of all declared types (for messages + concrete_for?)
type_sets = [] # per-conjunctive-part declared type-sets — a value must
# satisfy EVERY one (allOf/$ref are a conjunction, so their
# type constraints INTERSECT, not union).
# `$ref`(+siblings) and allOf form a CONJUNCTION: null is allowed only if every
# part allows it, so a part that imposes a non-nullable type FORBIDS null. We
# accumulate `forbids_null` (OR) and return its negation as the nullable flag.
forbids_null = false
items = nil
alt_groups = []
return [req, props, types, true, items, alt_groups, type_sets] if depth > 40 || !schema.is_a?(Hash)

# When the same property (or array `items`) is constrained by more than one
# conjunctive part (e.g. declared in two allOf branches), conjoin the schemas
# so the value must satisfy ALL of them — not just the first seen.
add_prop = lambda do |k, v|
props[k] = props.key?(k) ? { "allOf" => [props[k], v] } : v
end
add_items = lambda do |i|
items = items ? { "allOf" => [items, i] } : i
end

absorb = lambda do |sub|
r2, p2, t2, sub_nullable, i2, a2, ts2 = merged_constraints(sub, components, visited, depth + 1)
req.concat(r2)
p2.each { |k, v| add_prop.call(k, v) }
types.merge(t2)
type_sets.concat(ts2)
forbids_null ||= !sub_nullable
add_items.call(i2) if i2
alt_groups.concat(a2)
end

name = ref_name(schema)
if name && !visited.include?(name)
visited << name
absorb.call(components[name])
# fall through to local keywords (OpenAPI 3.1 permits $ref siblings)
end

t, nn = allowed_types(schema)
types.merge(t)
type_sets << t unless t.empty?
# A node that imposes a concrete type but does not permit null forbids null.
forbids_null ||= (!t.empty? && !nn)
(schema["properties"] || {}).each { |k, v| add_prop.call(k, v) }
(schema["required"] || []).each { |r| req << r }
add_items.call(schema["items"]) if schema["items"]
(schema["allOf"] || []).each { |sub| absorb.call(sub) }
%w[anyOf oneOf].each do |key|
alt_groups << schema[key] if schema[key].is_a?(Array) && !schema[key].empty?
end

# An anyOf/oneOf group (local or inherited via $ref/allOf) permits null only if
# at least one branch does; if every branch forbids null, the group forbids it
# (the value must satisfy some branch). Conjoin that with the surrounding
# $ref/allOf constraints. Branch nullability is computed with a FRESH visited
# set so outer traversal state can't short-circuit it.
alt_groups.each do |branches|
group_allows_null = branches.any? do |branch|
_, _, _, branch_nullable, = merged_constraints(branch, components, Set.new, depth + 1)
branch_nullable
end
forbids_null ||= !group_allows_null
end

[req, props, types, !forbids_null, items, alt_groups, type_sets]
end

# Composition-aware validation of `value` against `schema`. Reports
# (path-tagged) errors for a missing required field, a required field present as
# null against a non-nullable schema, a null array element against a non-nullable
# item schema, and a present value whose JSON type contradicts the declared type.
# Optional object-field nulls are tolerated: the Smithy-derived OpenAPI
# under-marks some nullable optionals (e.g. Person.bio/location are `type:string`
# yet the wire sends null), so flagging them would be a false positive.
def instance_errors(prefix, value, schema, components, depth = 0)
return [] if depth > 60
return [] if value.nil? # optional-null tolerated; required-/element-null handled in context

req, props, _types, _nullable, items, alt_groups, type_sets = merged_constraints(schema, components)

# The value must satisfy EVERY conjunctive part's declared type (allOf/$ref
# intersect their type constraints — a value matching only one contradictory
# branch fails).
type_sets.each do |ts|
next if type_matches?(ts, value)

label = prefix.empty? ? "(root)" : prefix
return ["#{label}: expected #{ts.to_a.sort.join('|')}, got #{json_type(value)}"]
end

errs = []

# anyOf/oneOf: the value must satisfy at least one branch of each group
# (oneOf is validated as "at least one" — enforcing exactly-one would need full
# discriminator/enum/const validation to avoid false positives).
alt_groups.each do |branches|
next if branches.any? { |branch| instance_errors(prefix, value, branch, components, depth + 1).empty? }

label = prefix.empty? ? "(root)" : prefix
errs << "#{label}: value matches none of the #{branches.length} allowed alternatives (anyOf/oneOf)"
end

if value.is_a?(Hash)
req.uniq.each do |rk|
field = prefix.empty? ? rk : "#{prefix}/#{rk}"
if !value.key?(rk)
errs << "missing required field `#{field}`"
elsif value[rk].nil?
_, _, _, field_nullable, = merged_constraints(props[rk] || {}, components)
errs << "#{field}: required field is null but its schema is not nullable" unless field_nullable
end
end
value.each do |k, v|
next unless props.key?(k)

child = prefix.empty? ? k : "#{prefix}/#{k}"
errs.concat(instance_errors(child, v, props[k], components, depth + 1))
end
elsif value.is_a?(Array) && items
_, _, _, item_nullable, = merged_constraints(items, components)
value.each_with_index do |item, i|
ip = "#{prefix}[#{i}]"
if item.nil?
errs << "#{ip}: null array element but the item schema is not nullable" unless item_nullable
next
end
errs.concat(instance_errors(ip, item, items, components, depth + 1))
end
end

errs
end
#
# The generic walk (`ref_name`, `allowed_types`, `json_type`, `type_matches?`,
# `merged_constraints`, `instance_errors`) lives in
# scripts/schema_instance_validator.rb, included above. What remains here is the
# part that is about THIS guard: the concrete-instance rule and the #408
# rich-text-emitter inventory.

def concrete_for?(schema_name, instance, components)
_, _, types, = merged_constraints({ "$ref" => "#/components/schemas/#{schema_name}" }, components)
Expand Down
Loading
Loading