From e1ee16134c9f66f429303a9b0d23fae08f956e36 Mon Sep 17 00:00:00 2001 From: andreatp Date: Wed, 26 Aug 2026 15:38:01 +0100 Subject: [PATCH] fix!: read JSON scalars by type instead of coercing - scalar readers return nil on a type mismatch, matching dotnet and Python - get_collection_of_primitive_values dispatched on the class, so no branch ever matched - write_*_value(nil, value) discarded the scalar instead of storing it --- .../json_parse_node.rb | 62 +++++----- .../json_serialization_writer.rb | 44 ++++--- .../spec/json_composed_type_primitive_spec.rb | 114 ++++++++++++++++++ ...microsoft_kiota_serialization_json_spec.rb | 4 +- 4 files changed, 180 insertions(+), 44 deletions(-) create mode 100644 components/serialization/json/spec/json_composed_type_primitive_spec.rb diff --git a/components/serialization/json/lib/microsoft_kiota_serialization_json/json_parse_node.rb b/components/serialization/json/lib/microsoft_kiota_serialization_json/json_parse_node.rb index b1a8ca0..616a55b 100644 --- a/components/serialization/json/lib/microsoft_kiota_serialization_json/json_parse_node.rb +++ b/components/serialization/json/lib/microsoft_kiota_serialization_json/json_parse_node.rb @@ -14,20 +14,25 @@ def initialize(node) @current_node = node end + # The scalar readers answer only for the type they are named after and return nil otherwise, + # matching the dotnet and Python runtimes. Coercing instead would make every reader answer for + # every payload, which leaves a composed type unable to tell which member it holds. def get_string_value - @current_node.to_s + @current_node.is_a?(String) ? @current_node : nil end def get_boolean_value - @current_node + [true, false].include?(@current_node) ? @current_node : nil end def get_number_value - @current_node.to_i + @current_node.is_a?(Integer) ? @current_node : nil end + # Widened to Numeric because JSON writes a whole number without a fraction, so a float field can + # legitimately arrive as an Integer. def get_float_value - @current_node.to_f + @current_node.is_a?(Numeric) ? @current_node.to_f : nil end def get_guid_value @@ -50,35 +55,34 @@ def get_duration_value MicrosoftKiotaAbstractions::ISODuration.new(@current_node) end + # The generator passes the type as a class, except for booleans which it passes as a plain + # string. A `case` cannot dispatch on that: `when String` asks whether the type is an instance + # of String, and a class is an instance of Class, so every branch fell through to the string + # reader. A hash keys on the class object itself. + PRIMITIVE_READERS = { + String => :get_string_value, + Float => :get_float_value, + Integer => :get_number_value, + Date => :get_date_value, + DateTime => :get_date_time_value, + Time => :get_time_value, + MicrosoftKiotaAbstractions::ISODuration => :get_duration_value, + UUIDTools::UUID => :get_guid_value, + 'boolean' => :get_boolean_value, + 'Boolean' => :get_boolean_value + }.freeze + def get_collection_of_primitive_values(type) + reader = PRIMITIVE_READERS[type] @current_node.map do |object| next if object.nil? + # an untyped collection is generated as Object, which has no reader of its own; the parsed + # JSON scalar is already the value, so it passes through rather than being stringified + next object if reader.nil? - current_parse_node = JsonParseNode.new(object) - case type - when String - current_parse_node.get_string_value - when Float - current_parse_node.get_float_value - when Integer - current_parse_node.get_float_value - when 'Boolean' - current_parse_node.get_float_value - when DateTime - current_parse_node.get_date_time_value - when Time - current_parse_node.get_time_value - when Date - current_parse_node.get_date_value - when MicrosoftKiotaAbstractions::ISODuration - current_parse_node.get_duration_value - when UUIDTools::UUID - current_parse_node.get_guid_value - else - current_parse_node.get_string_value - end + JsonParseNode.new(object).public_send(reader) rescue StandardError => e - raise e.class, `Failed to fetch #{type} type` + raise e.class, "Failed to fetch #{type} type: #{e.message}" end end @@ -121,6 +125,8 @@ def assign_field_values(item) def get_enum_values(_type) raw_values = get_string_value + return [] if raw_values.nil? + raw_values.split(',').map(&:strip) end diff --git a/components/serialization/json/lib/microsoft_kiota_serialization_json/json_serialization_writer.rb b/components/serialization/json/lib/microsoft_kiota_serialization_json/json_serialization_writer.rb index f354d89..4e341e4 100644 --- a/components/serialization/json/lib/microsoft_kiota_serialization_json/json_serialization_writer.rb +++ b/components/serialization/json/lib/microsoft_kiota_serialization_json/json_serialization_writer.rb @@ -12,34 +12,48 @@ class JsonSerializationWriter def initialize @writer = {} + @root_value = nil + @has_root_value = false end - attr_reader :writer + attr_reader :writer, :root_value + + # A composed type whose selected member is a primitive serializes the scalar as the whole + # document rather than as a member of an object, so a nil key means "this is the root". + def set_root_value(value) + @root_value = value + @has_root_value = true + value + end + + def root_value? + @has_root_value + end def write_string_value(key, value) - raise StandardError, 'no key or value included in write_string_value(key, value)' if !key && !value - return value.to_s unless key + raise StandardError, 'no key or value included in write_string_value(key, value)' if key.nil? && value.nil? + return set_root_value(value) if key.nil? - @writer[key] = (value || nil) + @writer[key] = value end def write_boolean_value(key, value) - raise StandardError, 'no key or value included in write_boolean_value(key, value)' if !key && !value - return value unless key + raise StandardError, 'no key or value included in write_boolean_value(key, value)' if key.nil? && value.nil? + return set_root_value(value) if key.nil? @writer[key] = value end def write_number_value(key, value) - raise StandardError, 'no key or value included in write_number_value(key, value)' if !key && !value - return value unless key + raise StandardError, 'no key or value included in write_number_value(key, value)' if key.nil? && value.nil? + return set_root_value(value) if key.nil? @writer[key] = value end def write_float_value(key, value) - raise StandardError, 'no key or value included in write_float_value(key, value)' if !key && !value - return value unless key + raise StandardError, 'no key or value included in write_float_value(key, value)' if key.nil? && value.nil? + return set_root_value(value) if key.nil? @writer[key] = value end @@ -99,13 +113,13 @@ def write_collection_of_object_values(key, values) end def write_object_value(key, value, *additional_values_to_merge) - return unless value + values = [value, *additional_values_to_merge].compact + return if values.empty? if key - @writer[key] = object_value_hash(value, *additional_values_to_merge) + @writer[key] = object_value_hash(*values) else - value.serialize(self) - additional_values_to_merge.each { |v| v&.serialize(self) } + values.each { |v| v.serialize(self) } end end @@ -114,7 +128,7 @@ def write_enum_value(key, values) end def get_serialized_content - @writer.to_json # TODO: encode to byte array to stay content type agnostic + (@has_root_value ? @root_value : @writer).to_json # TODO: encode to byte array to stay content type agnostic end def write_additional_data(value) diff --git a/components/serialization/json/spec/json_composed_type_primitive_spec.rb b/components/serialization/json/spec/json_composed_type_primitive_spec.rb new file mode 100644 index 0000000..3bed87f --- /dev/null +++ b/components/serialization/json/spec/json_composed_type_primitive_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require_relative 'spec_helper' +require 'microsoft_kiota_abstractions' + +module PrimitiveComposedModels + class Simple + include MicrosoftKiotaAbstractions::Parsable + + attr_accessor :email + + def get_field_deserializers = { 'email' => ->(n) { @email = n.get_string_value } } + def serialize(writer) = writer.write_string_value('email', @email) + def self.create_from_discriminator_value(_parse_node) = Simple.new + end +end + +RSpec.describe 'composed types with primitive members' do + let(:writer) { MicrosoftKiotaSerializationJson::JsonSerializationWriter.new } + + describe 'write_object_value with a leading nil member' do + it 'still serializes the remaining members' do + obj = PrimitiveComposedModels::Simple.new + obj.email = 'x@y.z' + writer.write_object_value(nil, nil, obj) + expect(writer.writer).to eq({ 'email' => 'x@y.z' }) + end + end + + describe 'scalar values written with a nil key' do + it 'serializes a string as the root of the document' do + writer.write_string_value(nil, 'hello') + expect(JSON.parse(writer.get_serialized_content)).to eq('hello') + end + + it 'serializes a number as the root of the document' do + writer.write_number_value(nil, 42) + expect(JSON.parse(writer.get_serialized_content)).to eq(42) + end + + it 'serializes false as the root of the document rather than raising' do + expect { writer.write_boolean_value(nil, false) }.not_to raise_error + expect(JSON.parse(writer.get_serialized_content)).to be(false) + end + end + + describe 'type-strict parse node getters' do + def node_for(json) = MicrosoftKiotaSerializationJson::JsonParseNode.new(JSON.parse(json)) + + it 'answers only for the matching type' do + s = node_for('"hello"') + expect(s.get_string_value).to eq('hello') + expect(s.get_number_value).to be_nil + expect(s.get_boolean_value).to be_nil + expect(s.get_float_value).to be_nil + end + + it 'does not coerce a number into a string' do + n = node_for('42') + expect(n.get_string_value).to be_nil + expect(n.get_number_value).to eq(42) + end + + it 'keeps a float out of the integer reader' do + expect(node_for('1.5').get_number_value).to be_nil + expect(node_for('1.5').get_float_value).to eq(1.5) + end + + it 'reads a whole number through the float reader, since JSON omits the fraction' do + expect(node_for('1').get_float_value).to eq(1.0) + end + + it 'returns false for a false boolean rather than nil' do + expect(node_for('false').get_boolean_value).to be(false) + end + + it 'does not answer the boolean reader for a lookalike' do + expect(node_for('"true"').get_boolean_value).to be_nil + expect(node_for('1').get_boolean_value).to be_nil + end + + it 'does not stringify a structure' do + expect(node_for('{"a": 1}').get_string_value).to be_nil + end + end + + describe 'collections of primitive values' do + def collection_for(json) = MicrosoftKiotaSerializationJson::JsonParseNode.new(JSON.parse(json)) + + # The generator passes the boolean type as a plain string and every other type as a constant, + # so both shapes have to reach the right reader. + it 'reads booleans as booleans' do + expect(collection_for('[true, false]').get_collection_of_primitive_values('boolean')).to eq([true, false]) + end + + it 'reads whole numbers as integers' do + expect(collection_for('[1, 2]').get_collection_of_primitive_values(Integer)).to eq([1, 2]) + end + + it 'reads floats as floats' do + expect(collection_for('[1.5, 2.5]').get_collection_of_primitive_values(Float)).to eq([1.5, 2.5]) + end + + it 'reads strings as strings' do + expect(collection_for('["a", "b"]').get_collection_of_primitive_values(String)).to eq(%w[a b]) + end + + # an untyped array is generated as Object, so its values must survive rather than be forced + # through the string reader + it 'passes an untyped collection through unchanged' do + expect(collection_for('[1, "a", true]').get_collection_of_primitive_values(Object)).to eq([1, 'a', true]) + end + end +end diff --git a/components/serialization/json/spec/microsoft_kiota_serialization_json_spec.rb b/components/serialization/json/spec/microsoft_kiota_serialization_json_spec.rb index 1b852ff..97116d0 100644 --- a/components/serialization/json/spec/microsoft_kiota_serialization_json_spec.rb +++ b/components/serialization/json/spec/microsoft_kiota_serialization_json_spec.rb @@ -13,7 +13,9 @@ it 'can build jsonParseNode' do json_parse_node = MicrosoftKiotaSerializationJson::JsonParseNode.new(JSON.parse('{"value": [{"hasAttachments": false}] }')) expect(json_parse_node).not_to be nil - expect(json_parse_node.get_string_value.gsub(/\s+/, '')).to eq('{"value"=>[{"hasAttachments"=>false}]}') + # an object node is not a string, so the string reader declines it rather than dumping its inspect form + expect(json_parse_node.get_string_value).to be_nil + expect(json_parse_node.get_child_node('value')).not_to be nil end it 'can deserialize payload' do