Skip to content

Update JS driver to GraphBinary4#3312

Open
kirill-stepanishin wants to merge 3 commits intoapache:masterfrom
kirill-stepanishin:graph-binary-4-JS
Open

Update JS driver to GraphBinary4#3312
kirill-stepanishin wants to merge 3 commits intoapache:masterfrom
kirill-stepanishin:graph-binary-4-JS

Conversation

@kirill-stepanishin
Copy link
Contributor

Update JS driver to GraphBinary v4

Migrates the gremlin-javascript GraphBinary serialization layer from v1 to v4 and replaces the test suite.

Serializer changes

Removed serializer files: Traverser, P, TextP, Lambda, TraversalStrategy, BulkSet, OffsetDateTime, LongSerializerNg

Removed DataType constants (had serializer implementations): Traverser (0x21), P (0x1e), TextP (0x28), Lambda (0x1d), TraversalStrategy (0x29), BulkSet (0x2a), Timestamp (0x05), Class (0x06), Barrier (0x13), Binding (0x14), Bytecode (0x15), Cardinality (0x16), Column (0x17), Operator (0x19), Order (0x1a), Pick (0x1b), Pop (0x1c), Scope (0x1f), OffsetDateTime (0x88)

Removed DataType constants (no serializer implementations existed): Metrics (0x2c), TraversalMetrics (0x2d), Merge (0x2e), DT (0x2f), GType (0x30), InetAddress (0x82), Instant (0x83), LocalDate (0x84), LocalDateTime (0x85), LocalTime (0x86), MonthDay (0x87), OffsetTime (0x89), Period (0x8a), Year (0x8b), YearMonth (0x8c), ZonedDateTime (0x8d), ZoneOffset (0x8e), Custom (0x00)

Added: Marker (0xfd), StubSerializer for Tree (0x2b), Graph (0x10), CompositePDT (0xf0), PrimitivePDT (0xf1)

Renamed: ByteBufferSerializer → BinarySerializer (0x25), DATE constant → DATETIME (same code 0x04)

Replaced: DateSerializer and OffsetDateTimeSerializer both removed, replaced by new DateTimeSerializer (0x04) with component format (year/month/day/nanos/offset — 18 bytes) instead of epoch millis

Modified:

  • Byte: unsigned → signed (writeUInt8 → writeInt8)
  • EnumSerializer: narrowed to Direction (0x18) and T (0x20) only, uses fully-qualified string values
  • List/Set: value_flag=0x02 indicates bulked format
  • Map: value_flag=0x02 indicates ordered, value_flag=0x01 returns null
  • Edge/Vertex/VertexProperty: labels serialize as List<String> on the wire. Properties field always writes empty List instead of null — properties on graph objects are lost during serialization.

Test rewrite

Replaced per-serializer tests with a model-based approach:

  • model.js / model-test.js — validates .gbin reference files with three modes: run (byte-exact), runWriteRead (double round-trip idempotency), runRead (deserialize-only)
  • error-cases-test.js — malformed input cases consolidated from old per-serializer tests
  • type-detection-test.js — AnySerializer dispatch, canBeUsedFor, number routing boundaries, round-trip extras
  • null-handling-test.js — null/undefined per type in both fully-qualified and bare formats

Note: GraphBinaryReader and GraphBinaryWriter are not updated in this PR. They will be migrated to the v4 message framing format in a future PR, as they depend on other changes.

@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.83%. Comparing base (cfd6889) to head (5402938).
⚠️ Report is 829 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3312      +/-   ##
============================================
- Coverage     77.87%   75.83%   -2.04%     
+ Complexity    13578    13283     -295     
============================================
  Files          1015     1020       +5     
  Lines         59308    59873     +565     
  Branches       6835     7037     +202     
============================================
- Hits          46184    45403     -781     
- Misses        10817    11812     +995     
- Partials       2307     2658     +351     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Cole-Greer
Copy link
Contributor

Cole-Greer commented Mar 4, 2026

I left some new comments which I think are worth considering but non-blocking. Overall LGTM
VOTE +1

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.

4 participants