diff --git a/.rubocop.yml b/.rubocop.yml index d3d3a4a9..94330a9c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -32,6 +32,11 @@ RSpec/EmptyExampleGroup: Exclude: - 'spec/lutaml/jsonld/transform_spec.rb' +# emits_nothing_extra carries the expectations for these examples. +RSpec/NoExpectationExample: + AllowedPatterns: + - '^emits_nothing_extra' + RSpec/NamedSubject: inherit_mode: merge: diff --git a/lib/compat/opal/lutaml_model_boot.rb b/lib/compat/opal/lutaml_model_boot.rb index 0f384ed5..cbef652d 100644 --- a/lib/compat/opal/lutaml_model_boot.rb +++ b/lib/compat/opal/lutaml_model_boot.rb @@ -371,6 +371,7 @@ require "lutaml/xml/serialization/model_import_ext" require "lutaml/xml/transformation/custom_method_wrapper" require "lutaml/xml/transformation/element_builder" +require "lutaml/xml/transformation/order_reconciler" require "lutaml/xml/transformation/ordered_applier" require "lutaml/xml/transformation/rule_applier" require "lutaml/xml/transformation/rule_compiler" diff --git a/lib/lutaml/model/attribute.rb b/lib/lutaml/model/attribute.rb index 3535f283..c7b14681 100644 --- a/lib/lutaml/model/attribute.rb +++ b/lib/lutaml/model/attribute.rb @@ -286,6 +286,32 @@ def cast_value(value, register) build_collection(value.map { |v| cast_element(v, register) }) end + # Cast the value a derived reader just recomputed. + # + # A derived attribute has no ivar and no writer. Its reader runs the + # source method and casts the result on every call, which makes the + # reader a casting entry point of its own — nothing a writer-side guard + # can reach. A collection still has to come back as a collection here, + # even when the source method returned nothing. + # + # @param value [Object] whatever the source method returned + # @param register [Symbol] register for type resolution + # @return [Object] the cast value, or an empty collection + def cast_derived(value, register) + return cast_element(value, register) unless collection? + return build_collection if no_data?(value) + return cast_value(value, register) if collection_instance?(value) + # A plain Array is a list of source elements even when the attribute + # declares its own collection class, which collection_instance? only + # recognises by that class. Without this it becomes one element whose + # value is the inspected Array. + if value.is_a?(::Array) + return build_collection(value.map { |v| cast_element(v, register) }) + end + + build_collection(cast_element(value, register)) + end + # Apply a value map to transform a value. # # value_map keys (:nil, :empty, :omitted) each map either to a symbolic @@ -331,8 +357,32 @@ def required_value_set?(value) true end + # True when the value is "nothing arrived" rather than data. + # + # nil and the uninitialized sentinel both mean the source carried no + # value. Neither is something a type can be asked to turn into an + # instance. + # + # @param value [Object] + # @return [Boolean] + def no_data?(value) + value.nil? || Utils.uninitialized?(value) + end + def cast_element(value, register) + # Resolve first: an undeclared type has to raise UnknownTypeError even + # when the value carries nothing, the way it did before the guard + # below existed. resolved_type = type(register) + + # Casting is for data. Every built-in type already hands nil and the + # uninitialized sentinel straight back, so this changes nothing for + # them. A type with its own `self.cast` returns a real instance + # instead, and that instance becomes a value nobody wrote — a phantom + # element in the document, or a one-item collection the source never + # contained. `cast` below guards the format entry point the same way. + return value if no_data?(value) + return cast_union(value, nil, register) if union? return resolved_type.new(value) if value.is_a?(::Hash) && !hash_type? @@ -623,6 +673,14 @@ def model_instance?(value) end def cast(value, format, register, options = {}) + # Same rule as cast_element, for the format entry point. from_json / + # from_yaml / from_toml / from_hash reach a missing key as nil, and a + # type with its own `self.cast` turns that nil into an instance the + # document never carried. Unlike cast_element this returns before + # resolving the type: resolution here depends on the format options + # below, and an undeclared type still raises through cast_element. + return value if no_data?(value) + # Namespace-aware type resolution: use type_with_namespace if namespace_uri provided namespace_uri = options[:namespace_uri] resolved_type = if options[:resolved_type] diff --git a/lib/lutaml/model/serialize.rb b/lib/lutaml/model/serialize.rb index 75c0f901..c83c3ff6 100644 --- a/lib/lutaml/model/serialize.rb +++ b/lib/lutaml/model/serialize.rb @@ -321,6 +321,65 @@ def resolve_reference_value(ref) ref.is_a?(Type::Reference) ? ref.object : ref end + + # Hand back a collection the caller can push onto. + # + # An unused collection points at LAZY_EMPTY_COLLECTION, one frozen Array + # shared by every instance of every class, so an untouched collection + # costs nothing. A reader cannot hand that array out: `model.items << x` + # raises FrozenError, and unfreezing it would leak one instance's items + # into all the others. So the first read swaps in this instance's own + # empty Array and stores it — which is what the sentinel's own comment + # has promised all along. A frozen model keeps the sentinel; there is + # nothing to store into. + # + # nil is left alone on purpose. A nil collection is a state of its own + # here, distinct from an empty one — `initialize_empty: false` is what + # produces it and `render_nil` renders it — so a reader that turned nil + # into [] would erase a documented distinction. + # + # @param attribute_name [Symbol] the collection attribute + # @return [Object] the stored collection + def materialize_lazy_collection(attribute_name) + current = instance_variable_get(:"@#{attribute_name}") + return current unless current.equal?(LAZY_EMPTY_COLLECTION) + return current if frozen? + + instance_variable_set(:"@#{attribute_name}", []) + end + + # Hand back a reference collection the caller can push onto. + # + # A reference attribute stores Type::Reference objects, and resolving + # them built a fresh Array per read — so `book.co_authors << author` + # landed on a throwaway and never reached the document. A Reference + # stays lazy until its target registers itself, so the reader can only + # store the resolved Array once every reference has actually resolved; + # until then it has to keep re-resolving and keeps handing back a copy. + # Once stored there is nothing left to resolve: resolve_reference_value + # passes a model instance straight through, and the generated key reader + # runs Attribute#reference_key over it to get the key back. + # + # Storing means `#{name}_ref` reports resolved objects from then on. It + # is the backing store, and after this the objects are what it holds. + # + # @param attribute_name [Symbol] the reference collection attribute + # @return [Object] the stored collection + def materialize_reference_collection(attribute_name) + refs = instance_variable_get(:"@#{attribute_name}_ref") + return resolve_reference_value(refs) unless refs.is_a?(::Array) + return refs if refs.none?(Type::Reference) + + resolved = resolve_reference_value(refs) + # Only a Reference that resolved to nothing keeps this lazy. A raw nil + # the caller put there is data, and waiting on it would never end. + dangling = refs.zip(resolved).any? do |ref, value| + ref.is_a?(Type::Reference) && value.nil? + end + return resolved if frozen? || dangling + + instance_variable_set(:"@#{attribute_name}_ref", resolved) + end end end end diff --git a/lib/lutaml/model/serialize/attribute_definition.rb b/lib/lutaml/model/serialize/attribute_definition.rb index c7281304..49fa0927 100644 --- a/lib/lutaml/model/serialize/attribute_definition.rb +++ b/lib/lutaml/model/serialize/attribute_definition.rb @@ -27,8 +27,11 @@ def define_attribute_methods(attr, register = nil) unless method_defined?(name, false) define_method(name) do value = public_send(attr.method_name) - # Cast the derived value to the specified type - attr.cast_element(value, register_id) + # Cast the derived value to the specified type. cast_derived, + # not cast_element: this reader is its own casting entry point, + # so it needs the same "nothing arrived, nothing to cast" rule + # the writers get, and a collection has to come back as one. + attr.cast_derived(value, register_id) end end elsif attr.unresolved_type == Lutaml::Model::Type::Reference @@ -64,14 +67,22 @@ def define_reference_methods(name, register) unless method_defined?(:"#{name}_#{key_method_name}", false) define_method("#{name}_#{key_method_name}") do ref = instance_variable_get(:"@#{name}_ref") - resolve_reference_key(ref) + # attr.reference_key first: once a collection reader has stored + # its resolved objects, the key has to come back off the object. + resolve_reference_key(attr.reference_key(ref)) end end unless method_defined?(name, false) - define_method(name) do - ref = instance_variable_get(:"@#{name}_ref") - resolve_reference_value(ref) + if attr.options[:collection] + define_method(name) do + materialize_reference_collection(name) + end + else + define_method(name) do + ref = instance_variable_get(:"@#{name}_ref") + resolve_reference_value(ref) + end end end @@ -101,7 +112,7 @@ def define_regular_attribute_methods(name, attr) if attr.collection? define_method(name) do |*args| if args.empty? - instance_variable_get(:"@#{name}") + materialize_lazy_collection(name) else # Builder-style: g.member(item) appends to collection value = args.first diff --git a/lib/lutaml/model/serialize/builder.rb b/lib/lutaml/model/serialize/builder.rb index 1300d7bf..14ce94c4 100644 --- a/lib/lutaml/model/serialize/builder.rb +++ b/lib/lutaml/model/serialize/builder.rb @@ -75,11 +75,10 @@ def ordered? mapping&.ordered? || false end - # Whether this instance was constructed via a builder block and - # therefore records mutations into element_order. Parsed models - # and instances constructed without a block do not track; their - # element_order (if any) comes from the parser and is treated as - # the complete source of truth by the serializer. + # Whether this instance was constructed via a builder block and so + # records one element_order entry per mutation. A parsed instance + # does not: its order comes from the document, where one entry can + # stand for several rules that share an element name. # @return [Boolean] def order_tracking_enabled? @__order_tracking__ ? true : false diff --git a/lib/lutaml/model/serialize/enum_handling.rb b/lib/lutaml/model/serialize/enum_handling.rb index 57123b3d..4d1e359a 100644 --- a/lib/lutaml/model/serialize/enum_handling.rb +++ b/lib/lutaml/model/serialize/enum_handling.rb @@ -49,15 +49,18 @@ def add_enum_methods_to_model(klass, enum_name, values, value_set_for(enum_name) enum_vals = public_send(:"#{enum_name}") + # `+` and `-` rather than `<<` and `delete`. The reader hands + # back the stored Array now, so mutating it here would reach an + # Array a caller already holds — and would land even when the + # store below raises on a frozen model. enum_vals = if !!val if collection - enum_vals << value + enum_vals.include?(value) ? enum_vals : enum_vals + [value] else [value] end elsif collection - enum_vals.delete(value) - enum_vals + enum_vals - [value] else instance_variable_get(:"@#{enum_name}") - [value] end @@ -78,13 +81,44 @@ def add_enum_methods_to_model(klass, enum_name, values, # @param collection [Boolean] Whether the enum is a collection def add_enum_getter_if_not_defined(klass, enum_name, collection) Utils.add_method_if_not_defined(klass, enum_name) do - i = instance_variable_get(:"@#{enum_name}") || [] + unless collection + i = instance_variable_get(:"@#{enum_name}") || [] + next i.is_a?(::Array) ? i.first : i + end - if !collection && i.is_a?(Array) - i.first - else - i.uniq + current = materialize_lazy_collection(enum_name) + + # An enum collection reads as an Array even when nothing was + # stored — the old `(ivar || []).uniq` never returned nil — so + # unlike a regular collection this one materializes nil too, and + # stores what it materialized so a push reaches the model. + if current.nil? + next [] if frozen? + + next instance_variable_set(:"@#{enum_name}", []) end + + # Not an Array, which means a model-defined writer stored + # something else. Read it the way the old reader did and leave + # what it stored alone — replacing it here would discard data. + next current.uniq unless current.is_a?(::Array) + + # A frozen Array cannot be deduped in place, and handing back a + # copy would lose the shared sentinel's identity, so only copy + # when there is actually something to remove. + if current.frozen? + deduped = current.uniq + next deduped.size == current.size ? current : deduped + end + + # Hand back the stored Array, not a copy. This used to be + # `i.uniq`, a fresh Array on every read, so `model.roles << "b"` + # pushed onto a throwaway and the value never reached the model — + # no error, no warning, the item simply never showed up in the + # document. It still has to read unique, which is what that `uniq` + # guaranteed, so dedupe in place rather than into a copy. + current.uniq! + current end end @@ -105,6 +139,11 @@ def add_enum_setter_if_not_defined(klass, enum_name, _values, if collection curr_value = public_send(:"#{enum_name}") + # Build a new Array rather than appending into the stored one. + # The reader hands back the real Array now, so `curr_value` may + # be this model's own collection — or, when the model defines + # its own reader, something shared between instances or not an + # Array at all. Duplicates are the reader's job either way. instance_variable_set(:"@#{enum_name}", curr_value + value) else instance_variable_set(:"@#{enum_name}", value) diff --git a/lib/lutaml/model/serialize/initialization.rb b/lib/lutaml/model/serialize/initialization.rb index 772e5e76..6cfd192f 100644 --- a/lib/lutaml/model/serialize/initialization.rb +++ b/lib/lutaml/model/serialize/initialization.rb @@ -387,8 +387,10 @@ def define_scalar_register_methods(name) def define_collection_register_methods(name) define_method(name) do |*args| if args.empty? - current = instance_variable_get(:"@#{name}") - current.equal?(LAZY_EMPTY_COLLECTION) ? [] : current + # Store the materialized Array instead of discarding it, so + # `model.items << x` on a register-scoped model reaches the model + # rather than a throwaway copy. + materialize_lazy_collection(name) else value = args.first current = instance_variable_get(:"@#{name}") diff --git a/lib/lutaml/xml/model_transform.rb b/lib/lutaml/xml/model_transform.rb index 95088306..a79d7ef3 100644 --- a/lib/lutaml/xml/model_transform.rb +++ b/lib/lutaml/xml/model_transform.rb @@ -359,7 +359,10 @@ def validate_document!(doc, options) def set_instance_ordering(instance, doc, ordered_option, mixed_content_option, xml_mapping = nil, instance_is_serialize = nil) - instance.element_order = doc.root.order + # dup: XmlElement#order hands back a frozen cache shared with the + # DOM. The model's copy has to stay mutable so callers can maintain + # element_order themselves. + instance.element_order = doc.root.order.dup if instance_is_serialize && doc.root.is_a?(::Lutaml::Xml::XmlElement) instance.attribute_order = doc.root.attribute_order end diff --git a/lib/lutaml/xml/transformation.rb b/lib/lutaml/xml/transformation.rb index f7f347e9..18f7b0db 100644 --- a/lib/lutaml/xml/transformation.rb +++ b/lib/lutaml/xml/transformation.rb @@ -14,11 +14,13 @@ module Xml # - TransformationSupport::ValueSerializer: Serializes values to XML strings # - TransformationSupport::ElementBuilder: Creates XML elements from values # - TransformationSupport::OrderedApplier: Applies rules in element order for round-trip + # - TransformationSupport::OrderReconciler: Aligns element_order with current values # - TransformationSupport::RuleApplier: Dispatches rule application to handlers class Transformation < Lutaml::Model::Transformation include TransformationSupport::RuleCompiler include TransformationSupport::RuleApplier include TransformationSupport::OrderedApplier + include TransformationSupport::OrderReconciler # Transform a model instance into XmlElement tree # diff --git a/lib/lutaml/xml/transformation/order_reconciler.rb b/lib/lutaml/xml/transformation/order_reconciler.rb new file mode 100644 index 00000000..09a46de9 --- /dev/null +++ b/lib/lutaml/xml/transformation/order_reconciler.rb @@ -0,0 +1,267 @@ +# frozen_string_literal: true + +module Lutaml + module Xml + module TransformationSupport + # Aligns a model's element_order with the values it currently holds. + # + # element_order describes the source document, not the model. Anything + # assigned after parsing has no entry in it, so OrderedApplier would + # never emit that value. Reconciliation inserts the missing entries + # before serialization starts. + # + # Only insertion is needed. A collection that shrank already emits the + # right count, because process_collection_item stops yielding once the + # index passes the value length. + # + # The model's own element_order is never modified: the reconciled array + # is a local view, so repeated to_xml calls stay idempotent. + module OrderReconciler + # @param model_instance [Object] The model instance + # @param compiled_rules [Array] The compiled rules + # @param options [Hash] Transformation options + # @return [Array(Array, Array)] the element order with + # any missing entries inserted, and the rules it could not + # reconcile that still hold an unemitted value. The caller must + # emit those the ordinary way or their value is lost. When nothing + # is missing the model's own order comes back untouched. + def reconciled_element_order(model_instance, compiled_rules, options) + order = model_instance.element_order + element_rules = element_typed_rules(compiled_rules) + + # Resolve every entry to its rule once. Coverage, insertion + # anchors and the final array all read from this, so no later + # step rescans the order or re-matches a rule. + resolved = order.map { |object| find_rule_for_element(object, compiled_rules) } + coverage = ::Hash.new(0) + resolved.each { |rule| coverage[rule] += 1 if rule } + + deficits, fallback = classify_rules(element_rules, coverage, + model_instance, options) + return [order, fallback] if deficits.empty? + + [insert_deficits(order, deficits, element_rules, resolved), fallback] + end + + private + + def element_typed_rules(compiled_rules) + compiled_rules.select do |rule| + rule.is_a?(::Lutaml::Model::CompiledRule) && + rule.option(:mapping_type) == :element + end + end + + # Custom-method element rules are compiled without a backing + # attribute, so they expose no value whose cardinality or + # explicitness could be measured. OrderedApplier invokes them + # directly and never reads their value either. + def reconcilable?(rule, options) + !attributeless_custom_rule?(rule) && valid_mapping?(rule, options) + end + + def attributeless_custom_rule?(rule) + rule.has_custom_methods? && rule.attribute_type.nil? + end + + # Split the element rules that are short of entries into the ones + # reconciliation can place and the ones it cannot. + # + # A rule it cannot place still holds a value nothing has emitted, so + # it comes back as a fallback for the caller to serialize the + # ordinary way. Dropping it would be the very data loss this file + # exists to stop. + # + # A rule that is neither short nor unemitted costs only the + # `using_default?` lookup inside expected_element_count, and the + # ambiguity scan runs solely for the ones that are short. + # + # @return [Array(Hash, Array)] + def classify_rules(element_rules, coverage, model_instance, options) + deficits = {} + fallback = [] + + element_rules.each do |rule| + missing = expected_element_count(rule, model_instance) - + coverage[rule] + next unless missing.positive? + + if reconcilable?(rule, options) && + unambiguous?(rule, element_rules) + deficits[rule] = missing + elsif coverage[rule].zero? && + emit_uncovered?(rule, element_rules, model_instance) + fallback << rule + end + end + + [deficits, fallback] + end + + # Whether a rule reconciliation could not place should still be + # emitted the ordinary way. + # + # Yes for an unambiguous rule: nothing else will emit its value. + # + # For rules sharing a serialized name, it depends on where + # element_order came from, because the dispatcher cannot tell them + # apart. A parsed order already stands for all of them — every one + # parsed from the same entry — so emitting the ones it did not + # resolve to would duplicate the element on a plain round-trip. An + # order built by a builder block records one entry per mutation, so + # a rule with no coverage genuinely has not been emitted. + def emit_uncovered?(rule, element_rules, model_instance) + return true if unambiguous?(rule, element_rules) + + model_instance.respond_to?(:order_tracking_enabled?) && + model_instance.order_tracking_enabled? + end + + # Entries to insert, keyed by the position in the original order + # they go before. Built in declaration order so several rules + # landing on one position keep their mapping order. + def insert_deficits(order, deficits, element_rules, resolved) + rule_index = {} + element_rules.each_with_index { |rule, i| rule_index[rule] = i } + + insertions = deficits.each_with_object({}) do |(rule, missing), acc| + at = insertion_index(resolved, rule, rule_index) + entries = ::Array.new(missing) { new_order_entry(rule) } + (acc[at] ||= []).concat(entries) + end + + rebuild_order(order, insertions) + end + + def rebuild_order(order, insertions) + result = [] + order.each_with_index do |object, index| + pending = insertions[index] + result.concat(pending) if pending + result << object + end + tail = insertions[order.length] + result.concat(tail) if tail + result + end + + # How many child elements the rule's current value should produce. + # + # Only values the caller explicitly set are counted. A parsed model + # keeps defaults and the uninitialized sentinel for elements absent + # from the source document, and turning those into new elements is + # exactly the over-emission that reverted an earlier attempt at this + # fix. + def expected_element_count(rule, model_instance) + owner = value_owner(rule, model_instance) + return 0 unless owner.respond_to?(:using_default?) + # A custom-method rule compiled without an attribute exposes no + # value to measure, and reading one would call a method that does + # not exist. + return 0 if attributeless_custom_rule?(rule) + + value = extract_ordered_rule_value(rule, model_instance) + return 0 if unmutated_default?(owner, rule, value) + return 0 if should_skip_delegated_value?(value, rule, owner) + # A custom `to:` method is handed the whole model and emits every + # value itself, and OrderedApplier calls it once per matching + # entry. One entry means one invocation, however many values the + # attribute holds. + return 1 if rule.custom_methods[:to] + return 1 unless rule.collection? + + collection_element_count(value, rule) + end + + def collection_element_count(value, rule) + # Mirror the applier's own dispatch. A value it will not iterate + # — nil, or a String — never reaches process_collection_item; it + # goes through apply_rule, which emits one element (xsi:nil under + # render_nil). + return 1 unless value.respond_to?(:each) && !value.is_a?(String) + + # Same length/size fallback process_collection_item uses, so the + # two cannot disagree about how many items a collection holds. + length = value.respond_to?(:length) ? value.length : value.size + return length if length.positive? + + empty_collection_renders_element?(rule) ? 1 : 0 + end + + # Whether the attribute still holds its default and nothing has put + # real data in it. + # + # This is the guard that keeps a parsed model from turning defaults + # and uninitialized sentinels into elements the source document + # never had. It follows RenderPolicy#should_skip_default? rather + # than inventing a second rule: a collection mutated in place is + # real data, because `items << "x"` never reaches the setter and so + # leaves `using_default?` true. + def unmutated_default?(owner, rule, value) + return false unless owner.using_default?(rule.attribute_name) + return false if rule.option(:render_default) + return false if ::Lutaml::Model::RenderPolicy + .derived_attribute_for?(owner, rule.attribute_name) + return false if rule.collection? && + !::Lutaml::Model::Utils.empty?(value) + + true + end + + def value_owner(rule, model_instance) + delegate = rule.option(:delegate_from) + return model_instance unless delegate + + model_instance.public_send(delegate) + end + + # find_rule_for_element resolves an inserted entry back to the FIRST + # element rule matching its name and namespace, and that match + # accepts a nil rule namespace, namespace aliases and name aliases. + # When more than one rule would claim the entry, reconciling would + # feed the value to the wrong rule, so leave the rule alone. + def unambiguous?(rule, element_rules) + matches = element_rules.select do |candidate| + matches_element_rule?(candidate, rule.serialized_name, + rule.namespace_class&.uri) + end + + matches == [rule] + end + + def new_order_entry(rule) + ::Lutaml::Xml::Element.new( + "Element", + rule.serialized_name, + node_type: :element, + namespace_uri: rule.namespace_class&.uri, + namespace_prefix: nil, + ) + end + + # Where a rule's new entries go: + # - after the last entry that already resolves to it, so extra + # collection items follow the existing ones and any interleaved + # element keeps its place + # - otherwise at the first entry declared after it, so a newly-set + # element lands in mapping-declaration order + def insertion_index(resolved, rule, rule_index) + own = rule_index[rule] + last_own = nil + first_later = nil + + resolved.each_with_index do |matched, index| + next unless matched + + last_own = index if matched.equal?(rule) + first_later ||= index if rule_index[matched] > own + end + + return last_own + 1 if last_own + + first_later || resolved.length + end + end + end + end +end diff --git a/lib/lutaml/xml/transformation/ordered_applier.rb b/lib/lutaml/xml/transformation/ordered_applier.rb index 77303d49..3037af29 100644 --- a/lib/lutaml/xml/transformation/ordered_applier.rb +++ b/lib/lutaml/xml/transformation/ordered_applier.rb @@ -24,10 +24,14 @@ module OrderedApplier # @yield Block to apply individual rules def apply_rules_in_order(root, model_instance, options, compiled_rules, model_class, register_id) - element_order = model_instance.element_order + element_order, fallback_rules = reconciled_element_order( + model_instance, compiled_rules, options + ) mapping = model_class.mappings_for(:xml, register_id) - # Track index per element type for collection attributes + # Track index per compiled rule for collection attributes. Keyed + # by rule, not by element name: one rule can appear under several + # names once aliases are in play. element_indices = ::Hash.new(0) # Track text node index for content-mapped attribute access. @@ -66,9 +70,11 @@ def apply_rules_in_order(root, model_instance, options, compiled_rules, processed_text_nodes = true if result == :text_node end - # Apply remaining rules that weren't in element_order (attributes only) - apply_remaining_rules(root, model_instance, options, compiled_rules, - mapping, processed_text_nodes) do |action, rule, value| + # Apply the rules element_order does not cover: attributes, + # content and raw. + apply_remaining_rules(model_instance, options, compiled_rules, + mapping, processed_text_nodes, + fallback_rules) do |action, rule, value| yield(action, rule, value) if block_given? end end @@ -183,8 +189,8 @@ def process_element_order_item(object, root, model_instance, options, # For collection attributes, get the specific item at the tracked index if rule.collection? && value.respond_to?(:each) && !value.is_a?(String) - process_collection_item(root, rule, value, object, element_indices, - options) do |action, r, v, xsi_nil_flag| + process_collection_item(rule, value, + element_indices) do |action, r, v, xsi_nil_flag| yield(action, r, v, xsi_nil_flag) if block_given? end elsif block_given? @@ -265,20 +271,21 @@ def extract_ordered_rule_value(rule, model_instance) # Process a single item from a collection # - # @param root [XmlElement] Root element + # Indices are keyed by the compiled rule, not the element name: a + # rule can appear in element_order under several names, because + # matches_name? accepts aliases and reconciliation inserts entries + # under the canonical name. One rule must mean one counter. + # # @param rule [CompiledRule] The rule # @param value [Array] The collection value - # @param object [Object] The element order object - # @param element_indices [Hash] Index tracker - # @param options [Hash] Options - def process_collection_item(_root, rule, value, object, element_indices, -_options) - index = element_indices[object.name] + # @param element_indices [Hash] Index tracker, keyed by rule + def process_collection_item(rule, value, element_indices) + index = element_indices[rule] value_length = value.respond_to?(:length) ? value.length : value.size if index < value_length single_value = value[index] - element_indices[object.name] += 1 + element_indices[rule] += 1 # Skip individual nil items when value_map says nil is omitted to_map = (rule.option(:value_map) || {})[:to] || {} @@ -286,36 +293,49 @@ def process_collection_item(_root, rule, value, object, element_indices, yield(:apply_single, rule, single_value) end elsif index.zero? && value_length.zero? - # Handle empty collections with value_map - to_map = (rule.option(:value_map) || {})[:to] || {} - if to_map[:empty] == :nil - yield(:apply_single, rule, nil, true) if block_given? - elsif to_map[:empty] == :blank - yield(:apply_single, rule, "") if block_given? + # 0 items means 0 child elements, unless value_map renders the + # empty collection as a nil or blank element. + case empty_collection_render_mode(rule) + when :nil then yield(:apply_single, rule, nil, true) if block_given? + when :blank then yield(:apply_single, rule, "") if block_given? end - # For :empty and :omitted: skip — 0 items = 0 child elements end end - # Apply remaining rules (attributes, content/raw, and any element - # rules not represented in element_order). + # What an empty collection serializes to, per value_map. # - # The element-type skip that used to live here caused silent data - # loss whenever an element-typed attribute was missing from - # element_order (e.g. when a direct setter forgot to call - # track_order). After the builder-side fix all mutation paths - # record into element_order, so this branch is a defense-in-depth - # safety net: emit any element-typed rule whose value would - # otherwise vanish. + # @param rule [CompiledRule] The rule + # @return [Symbol, nil] :nil, :blank, or the omitting mode + def empty_collection_render_mode(rule) + ((rule.option(:value_map) || {})[:to] || {})[:empty] + end + + # Whether an empty collection still produces one child element. + # + # @param rule [CompiledRule] The rule + # @return [Boolean] + def empty_collection_renders_element?(rule) + %i[nil blank].include?(empty_collection_render_mode(rule)) + end + + # Apply the rules element_order does not represent: attributes, + # content and raw. + # + # Element rules are applied here only when reconciliation handed + # them over. It covers every element value it can place, and + # emitting one of those again would duplicate it; the few it cannot + # place (ambiguous names, custom-method rules with no attribute) + # arrive as fallback_rules and would otherwise be dropped. # - # @param root [XmlElement] Root element # @param model_instance [Object] The model instance # @param options [Hash] Options # @param compiled_rules [Array] The compiled rules # @param mapping [Xml::Mapping] The mapping # @param processed_text_nodes [Boolean] Whether text nodes were processed - def apply_remaining_rules(_root, model_instance, options, -compiled_rules, mapping, processed_text_nodes) + # @param fallback_rules [Array] Element rules + # reconciliation could not place + def apply_remaining_rules(model_instance, options, compiled_rules, +mapping, processed_text_nodes, fallback_rules) attr_order = model_instance.respond_to?(:attribute_order) && model_instance.attribute_order @@ -326,10 +346,11 @@ def apply_remaining_rules(_root, model_instance, options, compiled_rules end - emitted_counts = element_order_coverage(model_instance, compiled_rules) - rules_to_apply.each do |rule| mapping_type = rule.option(:mapping_type) + if mapping_type == :element && !fallback_rules.include?(rule) + next + end # Skip content/raw if mixed or text nodes were processed if %i[content raw].include?(mapping_type) && @@ -337,95 +358,12 @@ def apply_remaining_rules(_root, model_instance, options, next end - if mapping_type == :element && - element_rule_already_emitted?(rule, model_instance, - emitted_counts) - next - end - next unless valid_mapping?(rule, options) yield(:apply_rule, rule, nil) if block_given? end end - # Count, per element-typed rule, how many entries in element_order - # already cover it. Returns a Hash keyed by CompiledRule identity. - # - # @param model_instance [Object] The model instance - # @param compiled_rules [Array] The compiled rules - # @return [Hash] Coverage counts - def element_order_coverage(model_instance, compiled_rules) - counts = ::Hash.new(0) - return counts unless model_instance.respond_to?(:element_order) - return counts unless (order = model_instance.element_order) - - element_rules = compiled_rules.select do |r| - r.is_a?(::Lutaml::Model::CompiledRule) && - r.option(:mapping_type) == :element - end - - order.each do |object| - next unless object.type == "Element" - - object_ns_uri = object.namespace_uri - matched = element_rules.find do |r| - matches_element_rule?(r, object.name, object_ns_uri) - end - counts[matched] += 1 if matched - end - - counts - end - - # Whether an element-typed rule has already been fully emitted - # via element_order (or should not be emitted at all by the safety - # net). Returns true when: - # - the model was not constructed via builder block - # (`@__order_tracking__` is nil/false): parsed models trust - # element_order as the complete source of truth, so the safety - # net must not second-guess it by emitting defaults/uninitialized - # values that the standard path would otherwise have skipped. - # - the standard skip logic says the value should be skipped - # (handles defaults, render_nil/render_empty, value_map, etc.) - # - or the rule was fully covered by element_order entries - # - # The safety net targets the bug class where a builder-block - # construction bypassed element_order tracking (e.g. via a - # mutation path that forgot to call record_mutation). After - # Option A, all setter/getter paths record into element_order, - # so this branch is defense-in-depth rather than the common - # path. - # - # @param rule [CompiledRule] The element rule - # @param model_instance [Object] The model instance - # @param emitted_counts [Hash] Coverage map - # @return [Boolean] - def element_rule_already_emitted?(rule, model_instance, -emitted_counts) - return true unless model_order_tracking_enabled?(model_instance) - - value = extract_ordered_rule_value(rule, model_instance) - return true if should_skip_value?(value, rule, model_instance) - - emitted = emitted_counts[rule] - if rule.collection? - value_length = value.respond_to?(:length) ? value.length : 0 - emitted >= value_length - else - emitted.positive? - end - end - - # Whether the model was constructed via a builder block and thus - # has order tracking enabled. Only such models are candidates for - # the safety net; parsed models trust element_order as-is. - def model_order_tracking_enabled?(model_instance) - return false unless model_instance.respond_to?(:order_tracking_enabled?) - - model_instance.order_tracking_enabled? - end - # Sort compiled rules so attribute rules follow the captured attribute_order. # Non-attribute rules (content, raw) maintain their original position. # diff --git a/lib/lutaml/xml/transformation/rule_compiler.rb b/lib/lutaml/xml/transformation/rule_compiler.rb index 92cbe65b..ca1d3a28 100644 --- a/lib/lutaml/xml/transformation/rule_compiler.rb +++ b/lib/lutaml/xml/transformation/rule_compiler.rb @@ -318,10 +318,12 @@ def compile_delegated_element_rule(mapping_rule, model_class, value_transformer = build_value_transformer(mapping_rule, attr) value_map = mapping_rule.raw_value_map rule_name = mapping_rule.multiple_mappings? ? mapping_rule.name.first : mapping_rule.name + alias_names = mapping_rule.multiple_mappings? ? mapping_rule.name[1..].map(&:to_s) : nil ::Lutaml::Model::CompiledRule.new( attribute_name: attr_name, serialized_name: rule_name.to_s, + alias_names: alias_names, attribute_type: attr_type, child_transformation: child_transformation, value_transformer: value_transformer, diff --git a/lib/lutaml/xml/transformation_support.rb b/lib/lutaml/xml/transformation_support.rb index 0a04b25d..ec343d40 100644 --- a/lib/lutaml/xml/transformation_support.rb +++ b/lib/lutaml/xml/transformation_support.rb @@ -8,6 +8,7 @@ module TransformationSupport autoload :ValueSerializer, "#{__dir__}/transformation/value_serializer" autoload :ElementBuilder, "#{__dir__}/transformation/element_builder" autoload :OrderedApplier, "#{__dir__}/transformation/ordered_applier" + autoload :OrderReconciler, "#{__dir__}/transformation/order_reconciler" autoload :RuleApplier, "#{__dir__}/transformation/rule_applier" end end diff --git a/spec/lutaml/model/collection_reader_liveness_spec.rb b/spec/lutaml/model/collection_reader_liveness_spec.rb new file mode 100644 index 00000000..4064620d --- /dev/null +++ b/spec/lutaml/model/collection_reader_liveness_spec.rb @@ -0,0 +1,516 @@ +# frozen_string_literal: true + +require "spec_helper" + +# A collection reader has to hand back a collection the caller can push onto. +# +# Four different readers broke that in four different ways, and only one of +# them made any noise: +# - the regular reader handed out LAZY_EMPTY_COLLECTION, one frozen Array +# shared by every instance, so `model.items << x` raised FrozenError; +# - the register-scoped reader substituted a fresh [] on every call, so the +# push landed on a throwaway and vanished without a word; +# - the enum reader returned `i.uniq`, also a fresh Array, same silence; +# - the reference reader resolved into a fresh Array, same silence again. +module CollectionReaderLivenessSpec + class Doc < Lutaml::Model::Serializable + attribute :name, :string + attribute :items, :string, collection: true + attribute :others, :string, collection: true + + xml do + root "doc" + map_element "name", to: :name + map_element "item", to: :items + map_element "other", to: :others + end + end + + class Filled < Lutaml::Model::Serializable + attribute :name, :string + attribute :items, :string, collection: true, initialize_empty: true + + xml do + root "doc" + map_element "name", to: :name + map_element "item", to: :items + end + + key_value do + map "name", to: :name + map "item", to: :items + end + end + + # A model is allowed to define its own writer, and the generated reader has + # to cope with whatever that writer stored. + class RolesOwnWriter < Lutaml::Model::Serializable + attribute :roles, :integer, values: [1, 2, 3], collection: true + + def roles=(value) + @roles = Set.new(value) + end + end + + # A model whose own writer keeps the value somewhere else, so the enum ivar + # is never written and the reader meets a bare nil. + class RolesLazyWriter < Lutaml::Model::Serializable + attribute :roles, :string, values: %w[reader writer admin], + collection: true + + def roles=(value) + @stashed = value + end + end + + # A reader the model defines itself can hand back anything at all, including + # one array shared by every instance. Deliberately not duplicated — sharing + # is the whole point, and a writer that mutated it would reach every model. + SHARED_ROLES = [] # rubocop:disable Style/MutableConstant -- shared on purpose + + class RolesOwnReader < Lutaml::Model::Serializable + attribute :roles, :string, values: %w[reader writer admin], + collection: true + + def roles + @roles ||= SHARED_ROLES + end + end + + class RolesSetReader < Lutaml::Model::Serializable + attribute :roles, :string, values: %w[reader writer admin], + collection: true + + def roles + @roles ||= Set.new + end + end + + class RolesFrozenDuplicates < Lutaml::Model::Serializable + attribute :roles, :string, values: %w[reader writer admin], + collection: true + + def roles=(_value) + @roles = %w[admin admin].freeze + end + end + + class RefAuthor < Lutaml::Model::Serializable + attribute :id, :string + attribute :name, :string + + xml do + root "author" + map_element "id", to: :id + map_element "name", to: :name + end + end + + class RefBook < Lutaml::Model::Serializable + attribute :id, :string + attribute :co_authors, { ref: ["CollectionReaderLivenessSpec::RefAuthor", :id] }, + collection: true, default: [] + + xml do + root "book" + map_element "id", to: :id + map_element "coAuthor", to: :co_authors + end + end + + class Roles < Lutaml::Model::Serializable + attribute :name, :string + attribute :roles, :string, values: %w[reader writer admin], collection: true + + xml do + root "roles" + map_element "name", to: :name + map_element "role", to: :roles + end + end +end + +RSpec.describe "what a collection reader hands back" do + let(:sentinel) { Lutaml::Model::Serialize::LAZY_EMPTY_COLLECTION } + let(:bare) { "n" } + + describe "the regular collection reader" do + it "keeps an item pushed onto a collection the document omitted" do + model = CollectionReaderLivenessSpec::Doc.from_xml(bare) + + model.items << "kept" + + expect(model.items).to eq(["kept"]) + end + + it "carries that item into the document" do + model = CollectionReaderLivenessSpec::Doc.from_xml(bare) + model.items << "kept" + + expect(model.to_xml.to_s).to include("kept") + end + + it "gives each instance its own array" do + one = CollectionReaderLivenessSpec::Doc.from_xml(bare) + two = CollectionReaderLivenessSpec::Doc.from_xml(bare) + + one.items << "mine" + + expect(two.items).to eq([]) + end + + it "leaves the collections nobody read on the shared sentinel" do + model = CollectionReaderLivenessSpec::Doc.from_xml(bare) + + model.items + + expect(model.instance_variable_get(:@others)).to be(sentinel) + end + + it "keeps the sentinel on a frozen model rather than raising" do + model = CollectionReaderLivenessSpec::Doc.from_xml(bare).freeze + + expect(model.items).to be(sentinel) + end + + # Reading must not be observable in the output. Every earlier round of + # this fix was refuted by a change in what got emitted. + %w[xml json yaml].each do |format| + it "does not change to_#{format} just because something read it" do + quiet = CollectionReaderLivenessSpec::Doc.from_xml(bare) + peeked = CollectionReaderLivenessSpec::Doc.from_xml(bare) + peeked.items + peeked.others + + expect(peeked.public_send(:"to_#{format}").to_s) + .to eq(quiet.public_send(:"to_#{format}").to_s) + end + end + end + + describe "the enum collection reader" do + it "keeps an item pushed onto an empty enum collection" do + model = CollectionReaderLivenessSpec::Roles.from_xml( + "n", + ) + + model.roles << "admin" + + expect(model.roles).to eq(["admin"]) + end + + it "keeps an item pushed onto a populated enum collection" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: ["reader"]) + + model.roles << "writer" + + expect(model.roles).to eq(%w[reader writer]) + end + + it "carries that item into the document" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: ["reader"]) + model.roles << "writer" + + expect(model.to_xml.to_s).to include("writer") + end + + it "still hides duplicates the way the reader used to" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n") + model.roles = ["reader"] + model.roles = %w[reader writer] + + expect(model.roles).to eq(%w[reader writer]) + end + + it "still answers the shorthand predicate" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: ["admin"]) + + expect(model.admin?).to be(true) + expect(model.reader?).to be(false) + end + + # Handing back the stored Array must not cost the guarantees the old + # `uniq` reader gave. + it "still reads unique after a duplicate is pushed through the reader" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: ["admin"]) + + model.roles << "admin" + + expect(model.roles).to eq(["admin"]) + end + + # The reader hands out the stored Array, so a writer must not mutate it. + # Every write builds a new Array, the way it did before the reader went + # live, which keeps a shared or frozen backing value safe. + it "does not write through into an array a caller already holds" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: ["admin"]) + held = model.roles + + model.roles = ["reader"] + + expect(held).to eq(["admin"]) + expect(model.roles).to eq(%w[admin reader]) + end + + it "does not duplicate into a held array through the shorthand writer" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: ["admin"]) + held = model.roles + + model.admin = true + + expect(held).to eq(["admin"]) + end + + it "hands back a stored array even when nothing ever wrote the ivar" do + model = CollectionReaderLivenessSpec::RolesLazyWriter.new + + model.roles << "admin" + + expect(model.roles).to eq(["admin"]) + end + + it "does not write through into a reader the model defines itself" do + CollectionReaderLivenessSpec::SHARED_ROLES.clear + one = CollectionReaderLivenessSpec::RolesOwnReader.new + two = CollectionReaderLivenessSpec::RolesOwnReader.new + + one.roles = ["admin"] + + expect(one.roles).to eq(["admin"]) + expect(two.roles).to eq([]) + expect(CollectionReaderLivenessSpec::SHARED_ROLES).to eq([]) + end + + it "assigns through a reader that hands back something other than an array" do + model = CollectionReaderLivenessSpec::RolesSetReader.new + + model.roles = ["admin"] + + expect(model.roles).to eq(Set.new(["admin"])) + end + + it "does not remove through into a held array via the shorthand writer" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: %w[admin reader]) + held = model.roles + + model.admin = false + + expect(held).to eq(%w[admin reader]) + expect(model.roles).to eq(["reader"]) + end + + it "does not mutate through the shorthand writer on a frozen model" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: ["admin"]) + held = model.roles + model.freeze + + expect { model.writer = true }.to raise_error(FrozenError) + expect(held).to eq(["admin"]) + end + + it "reads a frozen backing array as unique" do + model = CollectionReaderLivenessSpec::RolesFrozenDuplicates.new + model.roles = ["admin"] + + expect(model.roles).to eq(["admin"]) + end + + it "leaves a backing value its own writer stored alone" do + model = CollectionReaderLivenessSpec::RolesOwnWriter.new + model.roles = [1, 2] + + expect(model.roles).to eq([1, 2]) + expect(model.instance_variable_get(:@roles)).to eq(Set.new([1, 2])) + end + + it "does not touch the collection when a frozen model rejects the write" do + model = CollectionReaderLivenessSpec::Roles.new(name: "n", + roles: ["admin"]) + model.roles + model.freeze + + expect { model.roles = ["reader"] }.to raise_error(FrozenError) + expect(model.instance_variable_get(:@roles)).to eq(["admin"]) + end + end + + describe "the register-scoped collection reader" do + # initialization.rb defines its own pair of collection methods. Round 5 + # could not reach them through the public API, so drive the generator + # directly rather than assume the row is unreachable and therefore fine. + let(:host) do + Class.new(Lutaml::Model::Serializable) do + def self.name + "CollectionReaderLivenessSpec::RegisterScopedHost" + end + + attribute :notes, :string, collection: true + end + end + + let(:instance) do + host.send(:remove_method, :notes) + host.send(:remove_method, :notes=) + host.singleton_class.send(:public, :define_collection_register_methods) + host.define_collection_register_methods(:notes) + + inst = host.allocate + inst.send(:finalize_deserialization, nil) + inst + end + + it "is the reader under test" do + expect(instance.method(:notes).source_location.first) + .to end_with("serialize/initialization.rb") + end + + it "keeps an item pushed through it" do + instance.notes << "kept" + + expect(instance.notes).to eq(["kept"]) + end + + it "keeps an item appended builder-style" do + instance.notes("kept") + + expect(instance.notes).to eq(["kept"]) + end + end + + describe "the reference collection reader" do + let(:author) do + CollectionReaderLivenessSpec::RefAuthor.new(id: "a2", name: "Two") + end + + it "keeps an item pushed onto an empty reference collection" do + book = CollectionReaderLivenessSpec::RefBook.new(id: "b") + + book.co_authors << author + + expect(book.co_authors).to eq([author]) + end + + it "keeps an item pushed onto a populated reference collection" do + CollectionReaderLivenessSpec::RefAuthor.new(id: "a1", name: "One") + book = CollectionReaderLivenessSpec::RefBook.new(id: "b", + co_authors: ["a1"]) + + book.co_authors << author + + expect(book.co_authors.map(&:id)).to eq(%w[a1 a2]) + end + + it "carries that item into the document" do + book = CollectionReaderLivenessSpec::RefBook.new(id: "b") + book.co_authors << author + + expect(book.to_xml.to_s).to include("a2") + end + + it "still reads the keys back once the objects are stored" do + CollectionReaderLivenessSpec::RefAuthor.new(id: "a1", name: "One") + book = CollectionReaderLivenessSpec::RefBook.new(id: "b", + co_authors: ["a1"]) + + book.co_authors # resolves, and stores what it resolved + + expect(book.co_authors_ids).to eq(["a1"]) + end + + it "stays live when the collection also holds a raw nil" do + CollectionReaderLivenessSpec::RefAuthor.new(id: "a1", name: "One") + book = CollectionReaderLivenessSpec::RefBook.new( + id: "b", co_authors: ["a1", nil], + ) + + book.co_authors << author + + expect(book.co_authors.size).to eq(3) + expect(book.co_authors[1]).to be_nil + expect(book.co_authors.compact.map(&:id)).to eq(%w[a1 a2]) + end + + it "keeps re-resolving while any reference is still dangling" do + book = CollectionReaderLivenessSpec::RefBook.new(id: "b", + co_authors: ["late"]) + + expect(book.co_authors).to eq([nil]) + + CollectionReaderLivenessSpec::RefAuthor.new(id: "late", name: "Late") + + expect(book.co_authors.map(&:id)).to eq(["late"]) + end + end + + describe "every construction entry point" do + # The readers above are reached through from_xml. A collection that the + # model actually holds has to come back live from all of them, not just + # that one. + let(:entry_points) do + { + "new" => -> { CollectionReaderLivenessSpec::Filled.new }, + "new with attributes" => lambda { + CollectionReaderLivenessSpec::Filled.new(name: "n") + }, + "from_xml" => lambda { + CollectionReaderLivenessSpec::Filled.from_xml(bare) + }, + "from_json" => lambda { + CollectionReaderLivenessSpec::Filled.from_json('{"name":"n"}') + }, + "from_yaml" => lambda { + CollectionReaderLivenessSpec::Filled.from_yaml("name: n\n") + }, + "from_hash" => lambda { + CollectionReaderLivenessSpec::Filled.from_hash({ "name" => "n" }) + }, + "from_toml" => lambda { + CollectionReaderLivenessSpec::Filled.from_toml(%(name = "n"\n)) + }, + "dup" => lambda { + CollectionReaderLivenessSpec::Filled.from_xml(bare).dup + }, + "clone" => lambda { + CollectionReaderLivenessSpec::Filled.from_xml(bare).clone + }, + } + end + + it "hands back the same array on every read" do + results = entry_points.transform_values do |build| + model = build.call + model.items.equal?(model.items) + end + + expect(results).to all(satisfy { |_name, same| same }) + end + + it "keeps an item pushed through the reader" do + results = entry_points.transform_values do |build| + model = build.call + model.items << "kept" + model.items + end + + expect(results.values).to all(eq(["kept"])) + end + + it "carries that item into the document" do + results = entry_points.transform_values do |build| + model = build.call + model.items << "kept" + model.to_xml.to_s.include?("kept") + end + + expect(results).to all(satisfy { |_name, reached| reached }) + end + end +end diff --git a/spec/lutaml/model/lazy_collection_spec.rb b/spec/lutaml/model/lazy_collection_spec.rb index abb7be11..6e9702cf 100644 --- a/spec/lutaml/model/lazy_collection_spec.rb +++ b/spec/lutaml/model/lazy_collection_spec.rb @@ -78,14 +78,39 @@ class ParentWithManyCollections < Lutaml::Model::Serializable expect(ivar_c).to be(sentinel) end - it "returns sentinel for uninitialized collections (behaves like [])" do + it "hands out a real empty array for uninitialized collections" do xml = "test" instance = LazyCollectionTests::MultiCollection.from_xml(xml) items_a = instance.items_a expect(items_a).to eq([]) - expect(items_a).to be_frozen - expect(items_a).to be(sentinel) + # The sentinel is frozen and shared by every instance of every class, so + # a reader cannot hand it out: `instance.items_a << x` has to work, and + # has to stay on this instance. + expect(items_a).not_to be_frozen + expect(items_a).not_to be(sentinel) + expect(instance.instance_variable_get(:@items_a)).to be(items_a) + end + + it "keeps a pushed item and keeps it on this instance" do + xml = "test" + one = LazyCollectionTests::MultiCollection.from_xml(xml) + two = LazyCollectionTests::MultiCollection.from_xml(xml) + + one.items_a << "hello" + + expect(one.items_a).to eq(["hello"]) + expect(two.items_a).to eq([]) + end + + it "leaves the collections nobody read on the sentinel" do + xml = "test" + instance = LazyCollectionTests::MultiCollection.from_xml(xml) + + instance.items_a + + expect(instance.instance_variable_get(:@items_b)).to be(sentinel) + expect(instance.instance_variable_get(:@items_c)).to be(sentinel) end it "supports builder-style append on sentinel collections" do diff --git a/spec/lutaml/model/parsed_model_mutation_spec.rb b/spec/lutaml/model/parsed_model_mutation_spec.rb new file mode 100644 index 00000000..eea00b6a --- /dev/null +++ b/spec/lutaml/model/parsed_model_mutation_spec.rb @@ -0,0 +1,461 @@ +require "spec_helper" +require_relative "../../../lib/lutaml/model" + +# Regression coverage for the parsed-vs-built divergence: a model parsed from +# a document carries element_order, and mutations made after parsing used to +# be dropped during serialization. +# +# Every example here PARSES input, mutates, then serializes, and asserts on +# the emitted document. Asserting on attribute reads is what hid these bugs — +# the value always reads back correctly. +module ParsedModelMutationSpec + class Child < Lutaml::Model::Serializable + attribute :v, :string + + xml do + root "child" + map_attribute "v", to: :v + end + end + + class Mixed < Lutaml::Model::Serializable + attribute :a, Child + attribute :b, Child + + xml do + root "p" + mixed_content + map_element "a", to: :a + map_element "b", to: :b + end + end + + class MixColl < Lutaml::Model::Serializable + attribute :a, Child, collection: true + attribute :b, Child + + xml do + root "p" + mixed_content + map_element "a", to: :a + map_element "b", to: :b + end + end + + class EmptyColl < Lutaml::Model::Serializable + attribute :lead, :string + attribute :blank_topic, :string, collection: true + attribute :nil_topic, :string, collection: true + attribute :plain_topic, :string, collection: true + + xml do + root "s" + ordered + map_element "lead", to: :lead + map_element "blank", to: :blank_topic, render_empty: :as_blank + map_element "nil", to: :nil_topic, render_empty: :as_nil + map_element "plain", to: :plain_topic + end + end + + class NilColl < Lutaml::Model::Serializable + attribute :lead, :string + attribute :items, :string, collection: true + + xml do + root "s" + ordered + map_element "lead", to: :lead + map_element "item", to: :items, render_nil: :as_nil + end + end + + class DefaultColl < Lutaml::Model::Serializable + attribute :lead, :string + attribute :items, :string, collection: true, default: -> { [] } + + xml do + root "s" + ordered + map_element "lead", to: :lead + map_element "item", to: :items + end + end + + class DerivedOrdered < Lutaml::Model::Serializable + attribute :lead, :string + attribute :calc, :string, method: :calc_value + + xml do + root "s" + ordered + map_element "lead", to: :lead + map_element "calc", to: :calc + end + + def calc_value + "D" + end + end + + class Aliased < Lutaml::Model::Serializable + attribute :items, :string, collection: true + + xml do + root "p" + ordered + map_element %w[item old-item], to: :items + end + end + + class Glaze < Lutaml::Model::Serializable + attribute :color, :string + end + + class Delegating < Lutaml::Model::Serializable + attribute :glaze, Glaze + attribute :other, :string + + xml do + root "d" + ordered + map_element "color", to: :color, delegate: :glaze + map_element "other", to: :other + end + end + + class CustomColl < Lutaml::Model::Serializable + attribute :items, :string, collection: true + + xml do + root "p" + ordered + map_element "item", to: :items, with: { to: :items_to, from: :items_from } + end + + def items_to(model, parent, doc) + Array(model.items).each do |v| + el = doc.create_element("item") + doc.add_text(el, v) + parent.add_child(el) + end + end + + def items_from(model, values) + model.items = Array(values).map(&:to_s) + end + end + + class Inner < Lutaml::Model::Serializable + attribute :x, :string + end + + class DelegatingAlias < Lutaml::Model::Serializable + attribute :inner, Inner + attribute :other, :string + + xml do + root "d" + ordered + map_element %w[x old-x], to: :x, delegate: :inner + map_element "other", to: :other + end + end + + class SameNameA < Lutaml::Model::Serializable + attribute :v, :string + + xml do + root "sa" + map_attribute "v", to: :v + end + end + + class SameNameB < Lutaml::Model::Serializable + attribute :w, :string + + xml do + root "sb" + map_attribute "w", to: :w + end + end + + class AmbiguousNames < Lutaml::Model::Serializable + attribute :a, SameNameA + attribute :b, SameNameB + + xml do + root "p" + ordered + map_element "same", to: :a + map_element "same", to: :b + end + end + + class CustomMethod < Lutaml::Model::Serializable + attribute :name, :string + + xml do + root "c" + ordered + map_element "label", with: { to: :label_to_xml, from: :label_from_xml } + map_element "name", to: :name + end + + def label_to_xml(model, parent, doc) + el = doc.create_element("label") + doc.add_text(el, "L:#{model.name}") + parent.add_child(el) + end + + def label_from_xml(model, value) + model.name = Array(value).first.to_s.sub(/^L:/, "") + end + end +end + +RSpec.describe "parsed model mutation" do + # Defect 1: mixed_content dropped an attribute set after parse. + describe "singular element set after parse" do + it "emits the element that was set" do + model = ParsedModelMutationSpec::Mixed.from_xml('

') + model.b = ParsedModelMutationSpec::Child.new(v: "2") + + expect(model.to_xml).to include('').or include('') + end + + it "emits it in mapping-declaration order" do + model = ParsedModelMutationSpec::Mixed.from_xml('

') + model.a = ParsedModelMutationSpec::Child.new(v: "1") + + expect(model.to_xml.index("

') + model.b = ParsedModelMutationSpec::Child.new(v: "2") + + first = model.to_xml + second = model.to_xml + + expect(second).to eq(first) + expect(second.scan("

') + model.a = [ + ParsedModelMutationSpec::Child.new(v: "1"), + ParsedModelMutationSpec::Child.new(v: "2"), + ] + + expect(model.to_xml.scan("

') + model.a = [ParsedModelMutationSpec::Child.new(v: "9")] + + expect(model.to_xml.scan("

') + model.a = [ + ParsedModelMutationSpec::Child.new(v: "1"), + ParsedModelMutationSpec::Child.new(v: "2"), + ParsedModelMutationSpec::Child.new(v: "3"), + ] + + xml = model.to_xml + expect(xml.index('v="3"')).to be > xml.index('v="2"') + end + + it "shares one counter across canonical and alias element names" do + model = ParsedModelMutationSpec::Aliased + .from_xml("

x

") + model.items = %w[x y] + + xml = model.to_xml + expect(xml).to include("x") + expect(xml).to include("y") + end + end + + # Defect 3: element_order was frozen on parsed models. + describe "element_order on a parsed model" do + it "is mutable" do + model = ParsedModelMutationSpec::Mixed.from_xml('

') + + expect { model.element_order << "x" }.not_to raise_error + end + end + + describe "empty collection assigned after parse" do + it "emits a blank element under render_empty: :as_blank" do + model = ParsedModelMutationSpec::EmptyColl.from_xml("L") + model.blank_topic = [] + + expect(model.to_xml).to include("L") + model.nil_topic = [] + + expect(model.to_xml).to include("L") + model.plain_topic = [] + + expect(model.to_xml).not_to include("Lx") + model.items = nil + + expect(model.to_xml).to include("nil=\"true\"") + end + + it "emits nothing when the element was absent" do + model = ParsedModelMutationSpec::NilColl.from_xml("L") + model.items = nil + + expect(model.to_xml).not_to include("x") + end + + it "emits the pushed item on a parsed model" do + model = ParsedModelMutationSpec::DefaultColl + .from_xml("L") + model.items << "y" + + expect(model.to_xml).to include("y") + end + + it "emits nothing when the default collection stays empty" do + model = ParsedModelMutationSpec::DefaultColl + .from_xml("L") + + expect(model.to_xml).not_to include("") + with_order = ParsedModelMutationSpec::DerivedOrdered + .from_xml("L") + + expect(empty_order.to_xml).to include("D") + expect(with_order.to_xml).to include("D") + end + end + + describe "delegated element set after parse" do + it "emits the delegated value" do + model = ParsedModelMutationSpec::Delegating.from_xml("o") + model.glaze = ParsedModelMutationSpec::Glaze.new(color: "red") + + expect(model.to_xml).to include("red") + end + end + + describe "custom-method element rule on an ordered mapping" do + it "serializes without raising" do + model = ParsedModelMutationSpec::CustomMethod + .from_xml("x") + + expect { model.to_xml }.not_to raise_error + end + + # A custom `to:` emits the whole collection itself and is invoked once + # per matching entry, so one entry must mean one invocation. + it "invokes a custom collection transform once, not once per item" do + model = ParsedModelMutationSpec::CustomColl.from_xml("

a

") + model.items = %w[a b] + + expect(model.to_xml.scan("").size).to eq(2) + end + end + + # Two rules mapping one element name cannot be told apart by the ordered + # dispatcher, which resolves an entry to the first matching rule. That is + # a pre-existing limitation, so these specs pin the boundaries of what + # reconciliation may do around it rather than the dispatcher's output. + describe "two element rules sharing one serialized name" do + it "emits both values when built" do + model = ParsedModelMutationSpec::AmbiguousNames.new do |m| + m.a = ParsedModelMutationSpec::SameNameA.new(v: "1") + m.b = ParsedModelMutationSpec::SameNameB.new(w: "2") + end + + expect(model.to_xml).to include('v="1"') + expect(model.to_xml).to include('w="2"') + end + + # A parsed entry stands for every rule that shares its name, so + # reconciliation must not emit the ones the dispatcher skipped — + # doing so duplicates the element on a plain round-trip. + it "does not duplicate the element on a round-trip" do + xml = '

' + model = ParsedModelMutationSpec::AmbiguousNames.from_xml(xml) + + expect(model.to_xml.scan("oV") + + xml = model.to_xml + expect(xml).to include("V") + expect(xml.index("other")).to be < xml.index("V") + end + end + + describe "builder-block construction" do + it "emits attributes supplied by hash in declaration order" do + model = ParsedModelMutationSpec::EmptyColl.new(lead: "L") do |m| + m.plain_topic = ["t"] + end + + xml = model.to_xml + expect(xml.index(" came from. + class RegularCollection < Lutaml::Model::Serializable + attribute :lead, :string + attribute :things, Eager, collection: true + + xml do + root "r" + ordered + map_element "lead", to: :lead + map_element "things", to: :things + end + + key_value do + map "lead", to: :lead + map "things", to: :things + end + end + + # Same shape without `ordered`, because rounds 1 and 2 both mistook this for + # an ordering problem. + class RegularCollectionUnordered < Lutaml::Model::Serializable + attribute :lead, :string + attribute :things, Eager, collection: true + + xml do + root "r" + map_element "lead", to: :lead + map_element "things", to: :things + end + end + + # S12b — the control. Identical but for the element type. + class RegularCollectionBuiltIn < Lutaml::Model::Serializable + attribute :lead, :string + attribute :things, :string, collection: true + + xml do + root "r" + ordered + map_element "lead", to: :lead + map_element "things", to: :things + end + + key_value do + map "lead", to: :lead + map "things", to: :things + end + end + + # S13 — regular attribute whose writer the model already defines. The value + # is cast before it reaches that writer, so a phantom can be laundered + # through third-party code. + class RegularPredefinedWriter < Lutaml::Model::Serializable + def thing=(value) + @thing = "seen(#{value.inspect})" + end + + attribute :lead, :string + attribute :thing, Eager + + xml do + root "r" + map_element "lead", to: :lead + map_element "thing", to: :thing + end + end + + # S14 — regular attribute whose name collides with an enum value string, so + # the writer is regenerated even though a setter already exists. + class RegularEnumShorthandCollision < Lutaml::Model::Serializable + attribute :lead, :string + attribute :align, :string, values: %w[char left] + attribute :char, Eager + + xml do + root "r" + map_element "lead", to: :lead + map_element "align", to: :align + map_element "char", to: :char + end + end + + # S17 — regular collection with a custom Collection class. + class ThingCollection < Lutaml::Model::Collection + instances :items, Eager + end + + class RegularCustomCollection < Lutaml::Model::Serializable + attribute :lead, :string + attribute :things, Eager, collection: ThingCollection + + xml do + root "r" + map_element "lead", to: :lead + map_element "things", to: :things + end + end + + # S18 — a collection with a real default. The fix must not eat these. + class SeededCollection < Lutaml::Model::Serializable + attribute :lead, :string + attribute :things, Eager, collection: true, default: -> { ["seed"] } + + xml do + root "r" + map_element "lead", to: :lead + map_element "things", to: :things + end + end + + # S20 — a scalar default the mapping asks for explicitly. + class RenderedDefault < Lutaml::Model::Serializable + attribute :lead, :string + attribute :thing, Eager, default: -> { "dflt" } + + xml do + root "r" + map_element "lead", to: :lead + map_element "thing", to: :thing, render_default: true + end + end +end + +RSpec.describe "a value nobody wrote" do + def compact(xml) + xml.to_s.gsub(/<\?xml[^>]*\?>/, "").gsub(/\s*\n\s*/, "").strip + end + + let(:register) { Lutaml::Model::Config.default_register } + let(:uninitialized) { Lutaml::Model::UninitializedClass.instance } + let(:bare_xml) { "L" } + let(:bare_json) { '{"lead":"L"}' } + + # Each format is asserted against its own output. A single combined + # assertion would hide the shapes where the formats disagree. + def emits_nothing_extra(klass, attr_name) + aggregate_failures do + expect(compact(klass.new(lead: "L").to_xml)).to eq("L") + expect(klass.new(lead: "L").to_json).to eq('{"lead":"L"}') + expect(klass.new(lead: "L").to_yaml).to eq("---\nlead: L\n") + expect(compact(klass.from_xml(bare_xml).to_xml)) + .to eq("L") + expect(klass.from_json(bare_json).to_json).to eq('{"lead":"L"}') + expect(klass.from_yaml("lead: L\n").to_yaml).to eq("---\nlead: L\n") + expect(klass.attributes[attr_name].cast_element(uninitialized, register)) + .to be(uninitialized) + end + end + + describe "the root cause" do + # The module claims to protect self.cast for every subclass. It prepends + # instance methods, so it never sees a class-level cast, which is why the + # guard has to live in the attribute layer instead. + it "is not covered by UninitializedClassGuard" do + expect(Lutaml::Model::Type::Value.singleton_class.ancestors) + .not_to include(Lutaml::Model::Type::UninitializedClassGuard) + expect(PhantomValueSpec::Eager.cast(uninitialized)) + .to be_a(PhantomValueSpec::Eager) + end + + it "leaves the sentinel alone at the one place a type is handed a value" do + attr = PhantomValueSpec::RegularCollection.attributes[:things] + + expect(attr.cast_element(uninitialized, register)).to be(uninitialized) + expect(attr.cast_value(uninitialized, register)).to be(uninitialized) + expect(attr.default(register)).to be(uninitialized) + end + + it "leaves nil alone there too" do + attr = PhantomValueSpec::RegularCollection.attributes[:things] + + expect(attr.cast_element(nil, register)).to be_nil + expect(attr.cast(nil, :json, register)).to be_nil + end + + it "still casts real data" do + attr = PhantomValueSpec::RegularCollection.attributes[:things] + + expect(attr.cast_element("x", register)).to be_a(PhantomValueSpec::Eager) + expect(attr.cast_value(%w[x y], register).size).to eq(2) + end + end + + describe "S1c enum scalar, custom element type" do + it "emits no element and no key for a source that carried neither" do + emits_nothing_extra(PhantomValueSpec::EnumScalar, :role) + end + + it "does not fail its own validator over a value nobody set" do + expect { PhantomValueSpec::EnumScalar.new(lead: "L").validate! } + .not_to raise_error + end + end + + describe "S1 enum scalar, built-in type (control)" do + it "stays clean" do + emits_nothing_extra(PhantomValueSpec::EnumScalarBuiltIn, :role) + end + end + + describe "S2c enum collection, custom element type" do + it "emits no element and no key for a source that carried neither" do + emits_nothing_extra(PhantomValueSpec::EnumCollection, :roles) + end + end + + describe "S2 enum collection, built-in type" do + it "keeps an item pushed through the reader" do + model = PhantomValueSpec::EnumCollectionBuiltIn.new(lead: "L", + roles: ["a"]) + model.roles << "b" + + expect(model.roles).to eq(%w[a b]) + end + + it "carries that item into the document" do + model = PhantomValueSpec::EnumCollectionBuiltIn.new(lead: "L", + roles: ["a"]) + model.roles << "b" + + expect(compact(model.to_xml)) + .to eq("Lab") + end + + it "keeps an item pushed onto a collection the document omitted" do + model = PhantomValueSpec::EnumCollectionBuiltIn.from_xml(bare_xml) + model.roles << "a" + + expect(model.roles).to eq(["a"]) + expect(compact(model.to_xml)) + .to eq("La") + end + + it "still keeps duplicates out" do + model = PhantomValueSpec::EnumCollectionBuiltIn.new(lead: "L") + model.roles = ["a"] + model.roles = %w[a b] + + expect(model.roles).to eq(%w[a b]) + end + + it "still keeps duplicates out through the shorthand writer" do + model = PhantomValueSpec::EnumCollectionBuiltIn.new(lead: "L") + model.a! + model.a! + model.b! + + expect(model.roles).to eq(%w[a b]) + end + + it "still removes a value through the shorthand writer" do + model = PhantomValueSpec::EnumCollectionBuiltIn.new(lead: "L", + roles: %w[a b]) + model.a = false + + expect(model.roles).to eq(["b"]) + end + end + + describe "S3 enum whose reader the model already defines" do + it "leaves the model's own reader in charge" do + expect(PhantomValueSpec::EnumPredefinedReader.new(lead: "L").role) + .to eq("mine") + expect(compact(PhantomValueSpec::EnumPredefinedReader.new(lead: "L").to_xml)) + .to eq("Lmine") + end + + it "does not fabricate at the attribute layer behind it" do + attr = PhantomValueSpec::EnumPredefinedReader.attributes[:role] + + expect(attr.cast_element(uninitialized, register)).to be(uninitialized) + end + end + + describe "S4 enum with non-String values (control)" do + it "stays clean" do + emits_nothing_extra(PhantomValueSpec::EnumNonString, :level) + end + + it "generates no shorthand methods" do + expect(PhantomValueSpec::EnumNonString.new(lead: "L")).not_to respond_to(:"1?") + end + end + + describe "S5 derived scalar" do + # The reader calls into the cast on every call, with no writer in the + # path, so a writer-side guard cannot reach this shape at all. + it "emits no element and no key when the source method returns nothing" do + emits_nothing_extra(PhantomValueSpec::DerivedScalar, :thing) + end + + it "reads as nil rather than as a manufactured instance" do + expect(PhantomValueSpec::DerivedScalar.new(lead: "L").thing).to be_nil + end + + it "still casts what the source method does return" do + model = PhantomValueSpec::DerivedScalarWithValue.new(lead: "L") + + expect(model.thing).to be_a(PhantomValueSpec::Eager) + expect(compact(model.to_xml)) + .to eq(%(LE("real"))) + end + end + + describe "S6 derived collection" do + it "emits no element and no key when the source method returns nothing" do + emits_nothing_extra(PhantomValueSpec::DerivedCollection, :things) + end + + it "reads as a collection, not as a bare instance" do + expect(PhantomValueSpec::DerivedCollection.new(lead: "L").things).to eq([]) + end + + it "still casts every item the source method returns" do + model = PhantomValueSpec::DerivedCollectionWithValues.new(lead: "L") + + expect(model.things.size).to eq(2) + expect(model.things).to all(be_a(PhantomValueSpec::Eager)) + expect(compact(model.to_xml)) + .to eq(%(LE("x")E("y"))) + end + + it "reads an array source as many elements even with its own collection class" do + model = PhantomValueSpec::DerivedCustomCollection.new(lead: "L") + + expect(model.things.size).to eq(2) + expect(model.things.map(&:to_s)).to eq(['E("x")', 'E("y")']) + end + + it "reads a single source value as a one-item collection, not a bare instance" do + model = PhantomValueSpec::DerivedCollectionScalarSource.new(lead: "L") + + expect(model.things).to be_a(Array) + expect(model.things.size).to eq(1) + expect(model.things.first).to be_a(PhantomValueSpec::Eager) + expect(compact(model.to_xml)) + .to eq(%(LE("x"))) + end + end + + describe "S7 derived where the name equals the method name" do + it "falls through to the regular generator and stays clean" do + attr = PhantomValueSpec::DerivedSameName.attributes[:thing] + + expect(attr.cast_element(uninitialized, register)).to be(uninitialized) + expect(compact(PhantomValueSpec::DerivedSameName.new(lead: "L").to_xml)) + .to eq("L") + end + end + + describe "S8 derived whose reader the model already defines" do + it "leaves the model's own reader in charge" do + expect(compact(PhantomValueSpec::DerivedPredefinedReader.new(lead: "L").to_xml)) + .to eq("Lmine") + end + + it "does not fabricate at the attribute layer behind it" do + attr = PhantomValueSpec::DerivedPredefinedReader.attributes[:thing] + + expect(attr.cast_element(uninitialized, register)).to be(uninitialized) + end + end + + describe "S9 Reference scalar" do + # cast_element returns before validate_attr_type!, so this shape + # fabricated a live Reference even with a built-in type. + it "does not build a Reference out of the sentinel" do + attr = PhantomValueSpec::ReferenceScalar.attributes[:thing] + + expect(attr.cast_element(uninitialized, register)).to be(uninitialized) + expect(attr.cast_value(uninitialized, register)).to be(uninitialized) + expect(attr.default(register)).to be(uninitialized) + end + + it "still builds one out of a real key" do + attr = PhantomValueSpec::ReferenceScalar.attributes[:thing] + + expect(attr.cast_element("k1", register)) + .to be_a(Lutaml::Model::Type::Reference) + end + end + + describe "S10 Reference collection" do + it "does not build a Reference out of the sentinel" do + attr = PhantomValueSpec::ReferenceCollection.attributes[:things] + + expect(attr.cast_element(uninitialized, register)).to be(uninitialized) + expect(attr.default(register)).to be(uninitialized) + end + end + + describe "the cast guard itself" do + it "still reports an undeclared type when the value carries nothing" do + klass = Class.new(Lutaml::Model::Serializable) do + attribute :thing, :no_such_type_is_registered + end + + expect { klass.new(thing: nil) } + .to raise_error(Lutaml::Model::UnknownTypeError) + end + end + + describe "S11 regular scalar" do + it "emits no element and no key for a source that carried neither" do + emits_nothing_extra(PhantomValueSpec::RegularScalar, :thing) + end + + it "reads as nil on a model built with no value for it" do + expect(PhantomValueSpec::RegularScalar.new(lead: "L").thing).to be_nil + end + end + + describe "S12 regular collection, custom element type" do + it "emits no element and no key for a source that carried neither" do + emits_nothing_extra(PhantomValueSpec::RegularCollection, :things) + end + + it "emits nothing on a plain mapping either" do + expect(compact(PhantomValueSpec::RegularCollectionUnordered.from_xml(bare_xml).to_xml)) + .to eq("L") + end + + it "leaves the collection empty rather than holding a made-up item" do + expect(PhantomValueSpec::RegularCollection.from_xml(bare_xml).things) + .to eq([]) + end + + it "still emits what the document did contain" do + model = PhantomValueSpec::RegularCollection + .from_xml("LP") + + expect(compact(model.to_xml)) + .to eq(%(LE("P"))) + end + + it "appends through the builder argument instead of replacing" do + model = PhantomValueSpec::RegularCollection.new(lead: "L") + model.things("x") + model.things("y") + + expect(model.things.size).to eq(2) + end + end + + describe "S12b regular collection, built-in element type (control)" do + it "was clean before and stays clean" do + emits_nothing_extra(PhantomValueSpec::RegularCollectionBuiltIn, :things) + end + end + + describe "S13 regular attribute whose writer the model already defines" do + # The value is cast before it reaches the model's own writer, so a + # manufactured one gets laundered through code the library does not own. + it "hands that writer nothing rather than a made-up value" do + model = PhantomValueSpec::RegularPredefinedWriter.from_json(bare_json) + + expect(model.thing).to eq("seen(nil)") + end + + it "does not fabricate at the attribute layer behind it" do + attr = PhantomValueSpec::RegularPredefinedWriter.attributes[:thing] + + expect(attr.cast_element(uninitialized, register)).to be(uninitialized) + end + end + + describe "S14 regular attribute colliding with an enum value name" do + it "emits no element for a source that carried none" do + expect(compact(PhantomValueSpec::RegularEnumShorthandCollision.new(lead: "L").to_xml)) + .to eq("L") + expect(compact(PhantomValueSpec::RegularEnumShorthandCollision.from_xml(bare_xml).to_xml)) + .to eq("L") + end + end + + describe "S17 regular collection with a custom Collection class" do + it "emits no element for a source that carried none" do + expect(compact(PhantomValueSpec::RegularCustomCollection.from_xml(bare_xml).to_xml)) + .to eq("L") + end + + it "does not fail its own validator over a collection nobody filled" do + expect { PhantomValueSpec::RegularCustomCollection.new(lead: "L").validate! } + .not_to raise_error + end + end + + describe "S18 collection with a real default" do + it "still emits the default" do + expect(compact(PhantomValueSpec::SeededCollection.new(lead: "L").to_xml)) + .to eq(%(LE("seed"))) + expect(compact(PhantomValueSpec::SeededCollection.from_xml(bare_xml).to_xml)) + .to eq(%(LE("seed"))) + end + end + + describe "S20 scalar with render_default on the mapping" do + it "still emits the default" do + expect(compact(PhantomValueSpec::RenderedDefault.new(lead: "L").to_xml)) + .to include("dflt") + expect(compact(PhantomValueSpec::RenderedDefault.from_xml(bare_xml).to_xml)) + .to include("dflt") + end + end +end diff --git a/spec/lutaml/model/serialize_perf_guard_spec.rb b/spec/lutaml/model/serialize_perf_guard_spec.rb index b8b061db..89c385b7 100644 --- a/spec/lutaml/model/serialize_perf_guard_spec.rb +++ b/spec/lutaml/model/serialize_perf_guard_spec.rb @@ -35,9 +35,61 @@ def self.name instance.send(:init_deserialization_state, nil) # Collections are initialized with LAZY_EMPTY_COLLECTION (frozen shared []) # instead of per-instance Array.new — avoids allocation overhead + expect(instance.instance_variable_get(:@tags)) + .to be(Lutaml::Model::Serialize::LAZY_EMPTY_COLLECTION) + end + + it "allocates a per-instance array only once the collection is read" do + instance = model_class.allocate + instance.send(:init_deserialization_state, nil) + tags = instance.tags + expect(tags).to eq([]) - expect(tags).to be(Lutaml::Model::Serialize::LAZY_EMPTY_COLLECTION) + expect(tags).not_to be(Lutaml::Model::Serialize::LAZY_EMPTY_COLLECTION) + expect(instance.instance_variable_get(:@tags)).to be(tags) + end + end + + describe "enum collection assignment cost" do + # Duplicates are the collection reader's job, so this writer stays a plain + # `+`. The tempting alternative — deduping here with an Array#include? per + # appended item — makes assignment quadratic, and an enum collection has no + # size limit. Measured where this guard was written: the rescan form takes + # 6.35s at this size, against well under a millisecond for the `+`. The + # budget is deliberately coarse; it exists to catch a quadratic writer, not + # to police small regressions. + let(:enum_values) { (1..64_000).to_a } + + let(:enum_model_class) do + values = enum_values + Class.new(Lutaml::Model::Serializable) do + attribute :codes, :integer, values: values, collection: true + + def self.name + "LargeEnumCollectionModel" + end + end + end + + it "assigns a large collection without rescanning what it already holds" do + model = enum_model_class.new + + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + model.codes = enum_values + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started + + expect(model.codes.size).to eq(64_000) + expect(elapsed).to be < 0.75 + end + + it "still reads back without duplicates after repeated assignment" do + model = enum_model_class.new + + model.codes = [1, 2, 2, 3] + model.codes = [3, 4] + + expect(model.codes).to eq([1, 2, 3, 4]) end end