Add vajson as second serializer backend by JuriSchroeder · Pull Request #520 · eclipse-score/baselibs

JuriSchroeder · 2026-08-26T12:12:31Z

Adds vajson as a second json serialization backend. It's currently selectable by a CMake switch, but could replace the json_serializer implementation in the future. (See also https://github.com/orgs/eclipse-score/discussions/2390#discussioncomment-18160655)

This reverts commit ca5628f.

github-actions · 2026-08-26T12:27:57Z

Documentation preview for this pull request is available at:
pr-520: https://eclipse-score.github.io/baselibs/pr-520/

… into vajson-serializer

4og · 2026-09-01T12:27:24Z

@mihajlo-k, can you please review this PR?

mihajlo-k

I left a couple of comments. please note that this is a partial review, and I might post more comments.

mihajlo-k · 2026-09-04T08:46:23Z

+            std::array<char, 64> buffer{};
+            T value = static_cast<T>(number.GetValue());
+
+            const auto conversion_result = std::to_chars(buffer.data(), buffer.data() + buffer.size(), value);


According to RFC8259: https://www.rfc-editor.org/info/rfc8259/#section-6, infinities, NaNs or similar are not permitted. However, we don't guard against those values here. If we were to pass e.g. std::numeric_limits<double>::infinity() to the std::to_chars it will write (probably) a string "inf" into the buffer.

maybe guard it with std::isfinite?

I added a check for that.

mihajlo-k · 2026-09-04T08:49:46Z

+{
+    for (const char ch : string.GetValue())
+    {
+        switch (std::char_traits<char>::to_int_type(ch))


According to RFC8259: https://www.rfc-editor.org/info/rfc8259/#section-7 we should also escape control characters (U+0000 through U+001F).

mihajlo-k · 2026-09-04T09:34:39Z

+/// \brief Serializes a map of serializable elements
+/// \tparam Next type of serializer.
+/// \tparam Key Type of key. Must be convertible to a JKey.
+/// \tparam Value Type of value.
+/// \tparam Cmp Type of comparison function.
+/// \tparam Alloc Type of allocator.
+/// \param[in] serializer instance to write into.
+/// \param[in] map Map to serialize.
+/// \return The succeeding serializer.
+template <typename Next, typename Key, typename Value, typename Cmp, typename Alloc>
+auto operator<<(GenericValueSerializer<Next>&& serializer, const std::map<Key, Value, Cmp, Alloc>& map) noexcept ->
+    typename GenericValueSerializer<Next>::Next
+{
+    return std::move(serializer) << JObject(map);
+}


duplicate. please remove one

so, actually this should not compile. and now when I look at it, it seems it is indeed not compiled anywhere in the code. it's only included in score/json/internal/writer/vajson/writer/serializers.h but serializers.h itself is not included anywhere. @JuriSchroeder please check if this chunk of code is needed at all, and remove if not.

True. This was dead code because the adapter I built from S-Core to vaJSon, never actually uses these types. There was some more dead code I removed as well.

mihajlo-k

Generally a very good addition with a lot of improvements to existing code. However, there might be a couple of unnecessary additions, and a few potential improvements.

Additionally, please remove any commits that are out of this scope from the PR. e.g. Remove amsr namespace; Remove VCA annotations

mihajlo-k · 2026-09-10T09:58:50Z

Are these types used in VajsonSerialize? are they even related to JSON serialization?

Removed. (This was related to an extension, but since binary parsing is also removed it should also be removed from the serializer.)

mihajlo-k · 2026-09-10T10:02:59Z

Don't forget to add tests metadata to the tests (RecordProperty(...)). see some existing tests for this

mihajlo-k · 2026-09-10T10:09:03Z

+        return this->Serialize([this, number]() noexcept {
+            // Buffer size: max 24 chars for double, ~20 for int64, extra space for safety
+            std::array<char, 64> buffer{};
+            T value = static_cast<T>(number.GetValue());


what is the point of JNumberType? to me it seemed that it is "widening" the underlying type, but in this line it's narrowed back again.

mihajlo-k · 2026-09-10T10:13:50Z

+    else
+    {
+        const auto boolean = value.As<bool>();
+        serialized.emplace(std::move(serializer) << score::json::vajson::JBool(*boolean));


I know we currently exhaust all other possibilities before we go to bool, but still I think that we shouldn't simply dereference a Result type before checking it's validity.

mihajlo-k · 2026-09-10T10:33:00Z

+        result = score::Result<void>{
+            score::unexpect,
+            score::json::MakeError(score::json::Error::kUnknownError, "vaJSON serializer failed to write to stream")};


this can be simplified

Suggested change

result = score::Result<void>{

score::unexpect,

score::json::MakeError(score::json::Error::kUnknownError, "vaJSON serializer failed to write to stream")};

result = MakeUnexpected(score::json::Error::kUnknownError, "vaJSON serializer failed to write to stream");

mihajlo-k · 2026-09-10T10:39:05Z

+`json_serialize` (the default, a custom implementation) and `vajson` (the vector json library). The parser flag
+`base_library` has no influence on serialization.
+
+bazel test --config=spp_host_clang //score/json/... --//platform/aas/lib/json:writer_library="vajson"


Suggested change

bazel test --config=spp_host_clang //score/json/... --//platform/aas/lib/json:writer_library="vajson"

bazel test --config=spp_host_clang //score/json/... --//score/json:writer_library="vajson"

github-advanced-security

ClangTidy found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

JuriSchroeder added 8 commits May 22, 2026 16:27

Adapt bazel infrastructure to enable choosing json library

98a7073

Add serializer files

8891022

Move serializer

1a15f81

Add adapter

ba4035b

Merge remote-tracking branch 'origin/main' into vajson-serializer

7fa7701

Move vajson serializer to vajson parser directory

ca5628f

Revert "Move vajson serializer to vajson parser directory"

a5961be

This reverts commit ca5628f.

Add vajson as second serializer backend

21d0e52

github-project-automation Bot added this to BAS - Baselibs FT Aug 26, 2026

github-project-automation Bot moved this to In Progress in BAS - Baselibs FT Aug 26, 2026

JuriSchroeder deployed to workflow-approval August 26, 2026 12:12 — with GitHub Actions Active

github-actions Bot added comp-json Related to score/json component c++ C++ code bazel Bazel and Starlark build files labels Aug 26, 2026

Remove amsr namespace; Remove VCA annotations

755faa9

JuriSchroeder requested a deployment to workflow-approval August 26, 2026 12:42 — with GitHub Actions Waiting

Merge branch 'main' into vajson-serializer

a4a6486

JuriSchroeder requested a deployment to workflow-approval August 26, 2026 14:16 — with GitHub Actions Waiting

JuriSchroeder added 2 commits August 26, 2026 16:33

Make functions single return

c6d1dee

Merge branch 'vajson-serializer' of github.com:JuriSchroeder/baselibs…

84b0580

… into vajson-serializer

JuriSchroeder deployed to workflow-approval August 27, 2026 06:36 — with GitHub Actions Active

JuriSchroeder marked this pull request as ready for review August 27, 2026 08:34

JuriSchroeder requested review from 4og, antonkri, arkjedrz, fbaeuerle and pawelrutkaq as code owners August 27, 2026 08:34

4og requested a review from mihajlo-k September 1, 2026 12:26

mihajlo-k suggested changes Sep 4, 2026

View reviewed changes

mihajlo-k suggested changes Sep 10, 2026

View reviewed changes

JuriSchroeder added 6 commits September 17, 2026 14:37

RFC compliance for JSON serializer

3a5bfcc

Remove dead code

60d52d7

Handle bool differently

42976cc

Merge remote-tracking branch 'origin/main' into vajson-serializer

7750a5e

Remove more dead code

ba1c8ff

Coverage scope fix

35394f5

JuriSchroeder requested a deployment to workflow-approval September 17, 2026 14:13 — with GitHub Actions Waiting

Update README.md

42bcca8

JuriSchroeder requested a deployment to workflow-approval September 17, 2026 14:19 — with GitHub Actions Waiting

github-advanced-security Bot found potential problems Sep 17, 2026

View reviewed changes

	bazel test --config=spp_host_clang //score/json/... --//platform/aas/lib/json:writer_library="vajson"
	bazel test --config=spp_host_clang //score/json/... --//score/json:writer_library="vajson"

Conversation

JuriSchroeder commented Aug 26, 2026

Uh oh!

github-actions Bot commented Aug 26, 2026

Uh oh!

4og commented Sep 1, 2026

Uh oh!

mihajlo-k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihajlo-k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-advanced-security AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants