Skip to content

[22202] JSON to DynamicData deserializer#5553

Merged
rsanchez15 merged 11 commits intomasterfrom
feature/json-deserialize
Jul 9, 2025
Merged

[22202] JSON to DynamicData deserializer#5553
rsanchez15 merged 11 commits intomasterfrom
feature/json-deserialize

Conversation

@juanlofer-eprosima
Copy link
Copy Markdown
Contributor

@juanlofer-eprosima juanlofer-eprosima commented Jan 9, 2025

Description

This PR is a follow-up of #4780, in which a DynamicData to JSON serializer was introduced. Here a new utility method is introduced to perform the inverse transformation, i.e. from JSON to DynamicData. In addition, json_serialize method has been modified to:

  • Handle enums with base type other that int32
  • Improve wide characters and strings handling
  • Increase overall robustness
  • Decrease 'complexity' through higher encapsulation

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • N/A Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • Changes are API compatible.
  • New feature has been added to the versions.md file (if applicable).
  • New feature has been documented/Current behavior is correctly described in the documentation.
  • N/A Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@rsanchez15 rsanchez15 added this to the v3.3.0 milestone Apr 4, 2025
@EugenioCollado EugenioCollado force-pushed the feature/json-deserialize branch from b4d628c to 14023a4 Compare April 10, 2025 13:42
@EugenioCollado EugenioCollado force-pushed the feature/json-deserialize branch from 9dbd70e to d221bcc Compare May 27, 2025 13:25
@juanlofer-eprosima juanlofer-eprosima force-pushed the feature/json-deserialize branch from d221bcc to a0db432 Compare June 13, 2025 07:16
@juanlofer-eprosima juanlofer-eprosima force-pushed the feature/json-deserialize branch 2 times, most recently from f0476d3 to dc161bb Compare July 2, 2025 13:57
@juanlofer-eprosima juanlofer-eprosima marked this pull request as ready for review July 2, 2025 14:10
@github-actions github-actions bot added the ci-pending PR which CI is running label Jul 2, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a JSON-to-DynamicData deserializer (json_deserialize), integrates it into the public utils API, and adds round-trip tests to verify JSON serialization and deserialization.

  • Add json_deserialize entry point in utils.hpp/utils.cpp and implement logic in json_dynamic_data.hpp/.cpp.
  • Update CMakeLists and versions.md to include the new deserializer.
  • Extend unit and feature tests to cover JSON → DynamicData deserialization and round-trip equality.

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/cpp/fastdds/xtypes/utils.hpp / utils.cpp Expose and implement json_deserialize wrapper
src/cpp/fastdds/xtypes/serializers/json/json_dynamic_data.hpp / .cpp Core JSON → DynamicData logic
src/cpp/fastdds/xtypes/serializers/json/dynamic_data_json.hpp / .cpp Adjust array/element parameter naming and include fixes
test/unittest/dds/xtypes/serializers/json/DynDataJSONTests.cpp Add deserialization and round-trip assertions
test/feature/dynamic_types/DynamicTypesTests.cpp Include JSON deserialization in feature tests
src/cpp/CMakeLists.txt / versions.md Register new source file and document new feature
Comments suppressed due to low confidence (2)

test/unittest/dds/xtypes/serializers/json/DynDataJSONTests.cpp:83

  • Consider adding negative tests for json_deserialize, such as invalid JSON input and type-mismatch cases, to verify error handling and return codes.
            EXPECT_TRUE(dyn_data->equals(dyn_data_from_json));

versions.md:25

  • [nitpick] For consistency with other list entries, add a space after the leading asterisk: * Add new JSON to DynamicData deserializer....
* Add new JSON to DynamicData deserializer (`json_deserialize`).

@juanlofer-eprosima juanlofer-eprosima requested review from richiprosima and removed request for richiprosima July 2, 2025 14:46
@juanlofer-eprosima juanlofer-eprosima force-pushed the feature/json-deserialize branch from 80dfa1b to f7f40ac Compare July 4, 2025 06:12
@juanlofer-eprosima juanlofer-eprosima requested review from richiprosima and removed request for richiprosima July 4, 2025 06:16
@juanlofer-eprosima juanlofer-eprosima requested review from richiprosima and removed request for richiprosima July 4, 2025 12:14
@juanlofer-eprosima juanlofer-eprosima force-pushed the feature/json-deserialize branch from 0c4797a to 51c10bb Compare July 7, 2025 09:06
@juanlofer-eprosima juanlofer-eprosima requested review from richiprosima and removed request for richiprosima July 8, 2025 08:32
@Carlosespicur
Copy link
Copy Markdown
Contributor

LGTM

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
@juanlofer-eprosima juanlofer-eprosima force-pushed the feature/json-deserialize branch from b79ebf9 to 9cf2e87 Compare July 8, 2025 08:45
@juanlofer-eprosima juanlofer-eprosima requested review from richiprosima and removed request for richiprosima July 8, 2025 08:46
@juanlofer-eprosima juanlofer-eprosima force-pushed the feature/json-deserialize branch from e7e2849 to 0699742 Compare July 9, 2025 06:32
@juanlofer-eprosima juanlofer-eprosima requested review from richiprosima and removed request for richiprosima July 9, 2025 06:32
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
@juanlofer-eprosima juanlofer-eprosima force-pushed the feature/json-deserialize branch from 0699742 to 161e0f2 Compare July 9, 2025 06:53
@juanlofer-eprosima juanlofer-eprosima requested review from richiprosima and removed request for richiprosima July 9, 2025 06:53
@rsanchez15 rsanchez15 merged commit 9a524da into master Jul 9, 2025
35 of 37 checks passed
@rsanchez15 rsanchez15 deleted the feature/json-deserialize branch July 9, 2025 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-pending PR which CI is running

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants