Skip to content

Should Schema copying have an explicit graph-aware ownership contract? #131

Description

@numbata

Audited main at e62a44c142537c08eb144cd844ada6b893107b25 with Ruby 3.3.6. This is an open design question supported by a reproduction, not a request for a broad rewrite.

What and where

lib/grape_oas/api_model/schema.rb overrides initialize_copy to recursively duplicate properties, copy required, and shallow-copy defs. Other mutable edges (items, composition arrays, extensions, enum/default/example values) remain shared.

The comment promises an independent copy, but behavior depends on how a child is attached. Property recursion has no identity memo, so a cyclic property graph overflows the stack. In contrast, lib/grape_oas/exporter/concerns/schema_indexer.rb already treats schemas as a graph and tracks object identity.

Copying is used by lib/grape_oas/introspectors/entity_introspector_support/exposure_processor.rb to isolate changes to cached entity schemas. This makes the contract operationally relevant, although the reproduction below does not establish that every normal Grape declaration reaches the cycle failure.

Minimal reproductions

Run with bundle exec ruby -Ilib:

require "grape_oas"
s = GrapeOAS::ApiModel::Schema
original = s.new(type: "array", items: s.new(type: "string"))
copy = original.dup
copy.items.type = "integer"
p original.items.type # "integer" -- shared child

recursive = s.new(type: "object")
recursive.add_property(:self, recursive)
recursive.dup # SystemStackError

Both behaviors were reproduced. Ordinary property-tree copying is covered by existing tests; those tests do not establish independence for all graph edges or termination on cycles.

Why investigate

The DTO is a composite graph with shared named schemas, not necessarily a tree. A partially deep copy obscures aliasing and can make metadata updates affect a supposedly separate schema. Blind deep copying can also destroy deliberate sharing, duplicate work, or recurse forever. This is an ownership/identity design decision, not simply “add more .dup calls.”

Open questions and options

  • Should ordinary Ruby dup remain shallow, with a separately named copy_for_override operation that copies only fields the caller changes?
  • Alternatively, should a dedicated graph copier memoize by object identity, preserve repeated references/cycles within the copy, and copy all owned mutable values consistently?
  • Could immutable schema nodes plus explicit replacement simplify this later? That is a larger redesign and should not be assumed necessary for this issue.
  • What should happen to Node#id and canonical names when copying: preserve identity or create a distinct schema? Today inherited instance variables, including the ID, are copied.

Document the chosen semantics and cover properties, array items, composition alternatives, nested metadata, shared children, and cycles. Release triage: investigate reachable cached-schema cases first; the demonstrated low-level failure does not alone justify a large pre-release immutability rewrite.

References

Ruby Object#dup documents shallow copying and class-specific customization. The issue is the inconsistent custom contract, not a claim that Ruby requires deep copying.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions