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

Add JsonSerializable XXStruct classes #129

Merged
merged 3 commits into from
Jul 31, 2023
Merged

Add JsonSerializable XXStruct classes #129

merged 3 commits into from
Jul 31, 2023

Conversation

7hong13
Copy link
Contributor

@7hong13 7hong13 commented Jul 28, 2023

What this PR does / why we need it?

  • Cleaned up test-related terminology by renaming getStructureAsString() to toTestString().
  • Introduced JsonSerializableStruct
    • TimeTicket has a JSON serialization issue due to its lamport property being of type Long, which is not directly supported in JSON serialization. It can be problematic when CrdtTreePos and RgaTreeSplitPos, which have a createdAt property, need be serialized into JSON strings and passed through Presence between platforms for selection handling.
    • To properly serialize TimeTicket, its lamport type should be String, so I added TimeTicketStruct and other XXStruct data types based on the JS-SDK implementation. The postfix Struct is used to explicitly indicate that these classes are exclusively designed for JSON serialization.

Any background context you want to provide?

This PR only includes works released in v0.4.4.

What are the relevant tickets?

Fixes #

Checklist

  • Added relevant tests or not required
  • Didn't break anything

@7hong13 7hong13 requested review from hackerwins and skhugh July 28, 2023 09:15
@7hong13 7hong13 self-assigned this Jul 28, 2023
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #129 (462dc47) into main (ddb5aa3) will decrease coverage by 0.65%.
Report is 2 commits behind head on main.
The diff coverage is 68.49%.

@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
- Coverage   83.70%   83.05%   -0.65%     
==========================================
  Files          62       63       +1     
  Lines        3620     3654      +34     
  Branches      536      534       -2     
==========================================
+ Hits         3030     3035       +5     
- Misses        297      325      +28     
- Partials      293      294       +1     
Files Changed Coverage Δ
...c/main/kotlin/dev/yorkie/document/json/JsonTree.kt 79.56% <37.50%> (-0.14%) ⬇️
...tlin/dev/yorkie/document/JsonSerializableStruct.kt 52.94% <52.94%> (ø)
...c/main/kotlin/dev/yorkie/document/json/JsonText.kt 75.78% <54.54%> (-3.34%) ⬇️
...n/dev/yorkie/document/operation/SelectOperation.kt 33.33% <66.66%> (-27.78%) ⬇️
...in/kotlin/dev/yorkie/document/crdt/RgaTreeSplit.kt 79.77% <71.42%> (-0.46%) ⬇️
...src/main/kotlin/dev/yorkie/api/ElementConverter.kt 76.89% <100.00%> (ø)
...c/main/kotlin/dev/yorkie/document/crdt/CrdtText.kt 89.70% <100.00%> (-2.95%) ⬇️
...c/main/kotlin/dev/yorkie/document/crdt/CrdtTree.kt 75.27% <100.00%> (+0.36%) ⬆️
...c/main/kotlin/dev/yorkie/document/crdt/TextInfo.kt 87.87% <100.00%> (ø)
...lin/dev/yorkie/document/operation/EditOperation.kt 77.14% <100.00%> (-2.86%) ⬇️
... and 2 more

... and 3 files with indirect coverage changes

@7hong13 7hong13 merged commit 5290bdb into main Jul 31, 2023
@7hong13 7hong13 deleted the xx_struct branch July 31, 2023 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants