Skip to content

fix setting root element and don't return empty values #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
olafura wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix setting root element and don't return empty values #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd2162e
fix setting root element and don't return empty values
olafura Feb 6, 2024
aa69aa1
update version and changelog
olafura Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

## [0.9.2] - 2024-01-06

### Changed

- Fix setting the root element and don't return empty values

## [0.9.1] - 2023-08-18

### Changed
Expand Down
7 changes: 4 additions & 3 deletions lib/microsoft_kiota_serialization_json/json_serialization_writer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,17 @@ def write_collection_of_object_values(key, values)
end
end
@writer[key] = values.map do |v|
self.write_object_value(nil, v).writer
self.write_object_value(nil, v, true).writer
end
end
end

def write_object_value(key, value)
Copy link
Member

@baywet baywet Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I don't think we can accept this pull request as is.
This change effectively modifies the interface (updated needed in abstractions and other implementations) and would require changes in the generator.
The main problem with that approach is that it diverges from other languages

Can you create an issue where you provide more context about what you are doing and what's not working?
(snippet code for the request, stack trace if any, sent and received payload, etc...)

Copy link
Author

@olafura olafura Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is here and can also be addressed there if more appropriate:
https://github.com/microsoft/kiota-abstractions-ruby/blob/main/lib/microsoft_kiota_abstractions/request_information.rb#L99-L101

Since with both objects and arrays we don't modify the writer only return a temp version. We don't actually set anything there.

I was going by the assumption that if you pass nil as the first param you want to add the value to the root. If that is the wrong assumption the I can move my PR to kiota-abstractions-ruby

Copy link
Member

@baywet baywet Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the assumption is correct.
I think the issue is that having any of the "write" methods returning a new writer. This was a temporary workaround done by the intern years ago so his demo could work at the time, but it diverges from the other language implementations and this is what we should look to correct here.

def write_object_value(key, value, array = false)
if value
if !key
temp = JsonSerializationWriter.new()
value.serialize(temp)
@writer = @writer.merge(temp.writer) unless array
return temp
end
begin
Expand All @@ -179,7 +180,7 @@ def write_enum_value(key, values)
end

def get_serialized_content()
return @writer.to_json #TODO encode to byte array to stay content type agnostic
return @writer.compact.to_json #TODO encode to byte array to stay content type agnostic
end

def write_additional_data(value)
Expand Down
2 changes: 1 addition & 1 deletion lib/microsoft_kiota_serialization_json/version.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true

module MicrosoftKiotaSerializationJson
VERSION = "0.9.1"
VERSION = "0.9.2"
end