Skip to content
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.

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

[Java] Untyped nodes #1070

Merged
merged 25 commits into from
Mar 20, 2024
Merged

[Java] Untyped nodes #1070

merged 25 commits into from
Mar 20, 2024

Conversation

andrueastman
Copy link
Member

@andrueastman andrueastman commented Feb 6, 2024

This PR is related to microsoft/kiota#2319

It adds a number of types that implement the Parsable interface to enable the representation of types that are unknown at generation time to unblock the serialization and deserialization of properties of unknown types.

It also adds functionality to the serialization library of Json to be able to serialize and deserialize the types as well as tests for the scenarios.

Generation of these types is unblocked in microsoft/kiota#4095

Copy link
Contributor

github-actions bot commented Feb 7, 2024

This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged.

Copy link
Contributor

github-actions bot commented Feb 8, 2024

Conflicts have been resolved. A maintainer will take a look shortly.

Copy link
Contributor

github-actions bot commented Feb 9, 2024

This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged.

Copy link
Contributor

Conflicts have been resolved. A maintainer will take a look shortly.

Copy link
Contributor

This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged.

Copy link
Contributor

github-actions bot commented Mar 4, 2024

Conflicts have been resolved. A maintainer will take a look shortly.

@andrueastman andrueastman changed the title WIP: Untyped nodes [Java] Untyped nodes Mar 4, 2024
@andrueastman andrueastman marked this pull request as ready for review March 4, 2024 18:37
Copy link
Contributor

github-actions bot commented Mar 4, 2024

This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged.

Copy link
Contributor

github-actions bot commented Mar 5, 2024

Conflicts have been resolved. A maintainer will take a look shortly.

final JsonPrimitive primitive = element.getAsJsonPrimitive();
if (primitive.isBoolean()) return new UntypedBoolean(primitive.getAsBoolean());
else if (primitive.isString()) return new UntypedString(primitive.getAsString());
else if (primitive.isNumber()) return new UntypedDouble(primitive.getAsDouble());
Copy link
Member

Choose a reason for hiding this comment

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

when are we effectively using demical/float/long/integer? I know it can be hard to tell without the requested type and solely from from the data.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe I did this because of the challenge you speak of. A similar implementation observed in the tryGetAnything() method at

else if (primitive.isNumber()) return primitive.getAsDouble();

I ended up keeping the other types as they can still be used during serialization though.

Copy link
Member

Choose a reason for hiding this comment

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

The only thing I'm a bit worried about is the symmetrical aspect of it.
If somebody serializes as int, they'd probably expect to get an int back.
But I guess as long as this choice is consistent across languages, we can re-evaluate later based on customer feedback.

@andrueastman andrueastman requested a review from baywet March 7, 2024 11:06
final JsonPrimitive primitive = element.getAsJsonPrimitive();
if (primitive.isBoolean()) return new UntypedBoolean(primitive.getAsBoolean());
else if (primitive.isString()) return new UntypedString(primitive.getAsString());
else if (primitive.isNumber()) return new UntypedDouble(primitive.getAsDouble());
Copy link
Member

Choose a reason for hiding this comment

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

The only thing I'm a bit worried about is the symmetrical aspect of it.
If somebody serializes as int, they'd probably expect to get an int back.
But I guess as long as this choice is consistent across languages, we can re-evaluate later based on customer feedback.

Copy link

sonarcloud bot commented Mar 19, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
63.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@andrueastman andrueastman merged commit c4e2406 into main Mar 20, 2024
8 of 9 checks passed
@andrueastman andrueastman deleted the andrueastman/untypedNodes branch March 20, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants