fix: support primitive members in composed types

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
 

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)

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

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