Skip to content

Commit 7b6be90

Browse files
authored
fix(c++): fix c++ duration serialization (#3598)
## Why? ## What does this PR do? ## Related issues ## AI Contribution Checklist - [ ] Substantial AI assistance was used in this PR: `yes` / `no` - [ ] If `yes`, I included a completed [AI Contribution Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs) in this PR description and the required `AI Usage Disclosure`. - [ ] If `yes`, my PR description includes the required `ai_review` summary and screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes. ## Does this PR introduce any user-facing change? - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark
1 parent f5646ab commit 7b6be90

3 files changed

Lines changed: 100 additions & 8 deletions

File tree

cpp/fory/serialization/serialization_test.cc

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919

2020
#include "fory/serialization/fory.h"
2121
#include "fory/serialization/ref_resolver.h"
22+
#include "fory/serialization/skip.h"
2223
#include "gtest/gtest.h"
2324
#include <atomic>
25+
#include <chrono>
2426
#include <cstdint>
2527
#include <cstring>
2628
#include <map>
@@ -98,6 +100,11 @@ inline void register_test_types(Fory &fory) {
98100
fory.register_enum<OldStatus>(type_id++);
99101
}
100102

103+
inline std::vector<uint8_t> buffer_bytes(Buffer &buffer) {
104+
return std::vector<uint8_t>(buffer.data(),
105+
buffer.data() + buffer.writer_index());
106+
}
107+
101108
template <typename T>
102109
void test_roundtrip(const T &original, bool should_equal = true) {
103110
auto fory = Fory::builder().xlang(true).track_ref(false).build();
@@ -182,6 +189,79 @@ TEST(SerializationTest, StringRoundtrip) {
182189
test_roundtrip(std::string("UTF-8: 你好世界"));
183190
}
184191

192+
TEST(SerializationTest, DurationRoundtrip) {
193+
auto fory = Fory::builder().xlang(true).track_ref(false).build();
194+
std::vector<Duration> values = {
195+
Duration(0),
196+
std::chrono::seconds(12) + Duration(345678901),
197+
-std::chrono::seconds(7) - std::chrono::milliseconds(45) - Duration(67),
198+
Duration(-1),
199+
};
200+
201+
for (const Duration &original : values) {
202+
auto serialize_result = fory.serialize(original);
203+
ASSERT_TRUE(serialize_result.ok())
204+
<< "Serialization failed: " << serialize_result.error().to_string();
205+
206+
std::vector<uint8_t> bytes = std::move(serialize_result).value();
207+
auto deserialize_result =
208+
fory.deserialize<Duration>(bytes.data(), bytes.size());
209+
ASSERT_TRUE(deserialize_result.ok())
210+
<< "Deserialization failed: " << deserialize_result.error().to_string();
211+
EXPECT_EQ(deserialize_result.value(), original);
212+
}
213+
}
214+
215+
TEST(SerializationTest, DurationUsesSecondsAndNanosecondsPayload) {
216+
struct TestCase {
217+
Duration value;
218+
int64_t expected_seconds;
219+
int32_t expected_nanos;
220+
};
221+
222+
auto fory = Fory::builder().xlang(true).track_ref(false).build();
223+
std::vector<TestCase> cases = {
224+
{Duration(1234567890), 1, 234567890},
225+
{Duration(-1234567890), -1, -234567890},
226+
{Duration(-1), 0, -1},
227+
};
228+
229+
for (const TestCase &test_case : cases) {
230+
WriteContext write_ctx(fory.config(), fory.type_resolver().clone());
231+
Serializer<Duration>::write_data(test_case.value, write_ctx);
232+
ASSERT_FALSE(write_ctx.has_error()) << write_ctx.error().to_string();
233+
234+
Buffer expected;
235+
expected.write_var_int64(test_case.expected_seconds);
236+
expected.write_int32(test_case.expected_nanos);
237+
EXPECT_EQ(buffer_bytes(write_ctx.buffer()), buffer_bytes(expected));
238+
239+
ReadContext read_ctx(fory.config(), fory.type_resolver().clone());
240+
read_ctx.attach(write_ctx.buffer());
241+
Duration decoded = Serializer<Duration>::read_data(read_ctx);
242+
ASSERT_FALSE(read_ctx.has_error()) << read_ctx.error().to_string();
243+
EXPECT_EQ(decoded, test_case.value);
244+
EXPECT_EQ(read_ctx.buffer().reader_index(),
245+
write_ctx.buffer().writer_index());
246+
}
247+
}
248+
249+
TEST(SerializationTest, DurationSkipConsumesSecondsAndNanosecondsPayload) {
250+
auto fory = Fory::builder().xlang(true).track_ref(false).build();
251+
WriteContext write_ctx(fory.config(), fory.type_resolver().clone());
252+
Serializer<Duration>::write_data(Duration(-1), write_ctx);
253+
ASSERT_FALSE(write_ctx.has_error()) << write_ctx.error().to_string();
254+
255+
ReadContext read_ctx(fory.config(), fory.type_resolver().clone());
256+
read_ctx.attach(write_ctx.buffer());
257+
skip_field_value(read_ctx,
258+
FieldType(static_cast<uint32_t>(TypeId::DURATION), false),
259+
RefMode::None);
260+
ASSERT_FALSE(read_ctx.has_error()) << read_ctx.error().to_string();
261+
EXPECT_EQ(read_ctx.buffer().reader_index(),
262+
write_ctx.buffer().writer_index());
263+
}
264+
185265
// ============================================================================
186266
// Character Type Tests (C++ native only)
187267
// ============================================================================

cpp/fory/serialization/skip.cc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,13 @@ void skip_field_value(ReadContext &ctx, const FieldType &field_type,
549549
return;
550550

551551
case TypeId::DURATION: {
552-
// Duration is stored as fixed 8-byte nanosecond count.
553-
constexpr uint32_t k_bytes = static_cast<uint32_t>(sizeof(int64_t));
552+
// Duration is stored as signed varint64 seconds + signed int32
553+
// nanoseconds.
554+
skip_varint(ctx);
555+
if (FORY_PREDICT_FALSE(ctx.has_error())) {
556+
return;
557+
}
558+
constexpr uint32_t k_bytes = static_cast<uint32_t>(sizeof(int32_t));
554559
ctx.buffer().increase_reader_index(k_bytes, ctx.error());
555560
return;
556561
}

cpp/fory/serialization/temporal_serializers.h

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ struct Date {
5555
// ============================================================================
5656

5757
/// Serializer for Duration (std::chrono::nanoseconds)
58-
/// Per xlang spec: serialized as int64 nanosecond count
58+
/// Per xlang spec: serialized as signed varint64 seconds + signed int32
59+
/// nanoseconds
5960
template <> struct Serializer<Duration> {
6061
static constexpr TypeId type_id = TypeId::DURATION;
6162

@@ -85,8 +86,10 @@ template <> struct Serializer<Duration> {
8586
}
8687

8788
static inline void write_data(const Duration &duration, WriteContext &ctx) {
88-
int64_t nanos = duration.count();
89-
ctx.write_bytes(&nanos, sizeof(int64_t));
89+
auto seconds = std::chrono::duration_cast<std::chrono::seconds>(duration);
90+
auto remainder = duration - seconds;
91+
ctx.write_var_int64(seconds.count());
92+
ctx.buffer().write_int32(static_cast<int32_t>(remainder.count()));
9093
}
9194

9295
static inline void write_data_generic(const Duration &duration,
@@ -115,9 +118,13 @@ template <> struct Serializer<Duration> {
115118
}
116119

117120
static inline Duration read_data(ReadContext &ctx) {
118-
int64_t nanos;
119-
ctx.read_bytes(&nanos, sizeof(int64_t), ctx.error());
120-
return Duration(nanos);
121+
int64_t seconds = ctx.read_var_int64(ctx.error());
122+
if (FORY_PREDICT_FALSE(ctx.has_error())) {
123+
return Duration(0);
124+
}
125+
int32_t nanos = ctx.read_int32(ctx.error());
126+
return std::chrono::duration_cast<Duration>(std::chrono::seconds(seconds)) +
127+
Duration(nanos);
121128
}
122129

123130
static inline Duration read_with_type_info(ReadContext &ctx, RefMode ref_mode,

0 commit comments

Comments
 (0)