Skip to content

Keep the entries of a map keyed by ints when it is read as an object - #2459

Open
danielcweber wants to merge 2 commits into
fix/unknown-label-element-map-as-objectfrom
fix/int-keyed-map-as-object
Open

danielcweber wants to merge 2 commits into
fix/unknown-label-element-map-as-objectfrom
fix/int-keyed-map-as-object

Conversation

@danielcweber

@danielcweber danielcweber commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

A g:Map whose keys are not strings, such as ints, now keeps its entries when it is requested as an object. It used to come back as an empty dynamic object with every entry gone. It now comes back as a Dictionary<object, object> whose keys are still the ints they were sent as, the same entries a request for Dictionary<int, string> already got. Maps keyed by strings are unchanged and still become dynamic objects, and so are element maps whose id and label arrive under g:T keys.

The route to object copied the map into a JObject, and a JObject can only hold keys that are names, so any other key was dropped on the way.

Changes

  • Tests.Infrastructure: Map_of_typed_int_keys_as_object reads Map_of_Typed_Int_Keys_Typed_String_Values as an object. It records the result's type and its keys' types next to the value, because a snapshot writes the int key 1 and the string key "1" the same way, and a map that kept its entries by turning one into the other would not have kept them.
  • Tests.Infrastructure: Constructor_arguments_from_map_with_int_key pins the other half. A map with an int-keyed entry among its constructor arguments, read into a ClassWithFieldsAndConstructor, gives { StringArg: stringValue, IntArg: 42 }: the entry no member can be named after is left out, and what follows it is still read. Both implementations already answered this.
  • Support.NewtonsoftJson: when the target is object and some key is neither a string nor a g:T, MapDeferralConverterFactory asks for a Dictionary<object, object> instead of building a JObject. MapToDictionaryConverter already builds that type from a g:Map, converting each key like any other token. Its answer is taken as it is: it checks for the same g:Map this converter has just found, so there is no fallback to the JObject road.
  • Providers.GremlinServer.Tests: Group and Group_with_key_identity in ObjectQueryIntegrationTests update their snapshots. group() keys its map by the element itself, so these results have non-string keys too. Both had pinned [ {} ], with every group dropped. They now record the groups: each vertex keys the list of vertices it grouped.

Notes

  • Contains Say that an element map with an unknown label is still an element #2458, which it is branched from. Merge that first. Both are fast-forwards of 14.x.
  • Behaviour change. A caller that requested object for such a map used to get an empty object and now gets a dictionary with the entries. This includes real server output: a group() requested as object now comes back with its groups. Targets other than object are unaffected, since a POCO has no member a key of 1 could fill. No public API change, so version.json is left alone.
  • Measured before the fix: the test failed here with an empty DynamicDictionary and no keys at all. The System.Text.Json implementation in Gremlinq.Extensions gave the same answer. Its fix is Gremlinq.Extensions#207.
  • The contract test and the fix are separate commits, test first.
  • Support.NewtonsoftJson.Tests on net10.0: 273, up from 271, all green. Coverlet shows every changed line fully covered, branches included. dotnet build ExRam.Gremlinq.slnx succeeds with 0 warnings.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Syxmt77AL1HAfGM6p3LseP

@codecov

codecov Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.20%. Comparing base (531e3cc) to head (66ad4da).

Additional details and impacted files
@@           Coverage Diff           @@
##             14.x    #2459   +/-   ##
=======================================
  Coverage   93.19%   93.20%           
=======================================
  Files         279      279           
  Lines        7803     7809    +6     
  Branches      874      877    +3     
=======================================
+ Hits         7272     7278    +6     
  Misses        334      334           
  Partials      197      197           

☔ View full report in Codecov by Harness.
📢 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.

@danielcweber
danielcweber force-pushed the fix/int-keyed-map-as-object branch from a685e97 to 837bc3d Compare September 10, 2026 18:20
danielcweber and others added 2 commits September 11, 2026 12:06
Every shared test that reads a g:Map with non-string keys names the dictionary it
wants - Dictionary<int, string>, IImmutableDictionary<int, string> - and gets the
entries. Nothing said what the same map becomes when it is asked for as an object,
the type a caller has when it does not know, or care, what a query returns.

Map_of_typed_int_keys_as_object reads Map_of_Typed_Int_Keys_Typed_String_Values
that way. The answer it pins is a Dictionary<object, object> holding
{ 1: value1, 2: value2 }, keys still ints. A string-keyed map keeps becoming a
dynamic object; one whose keys are not names cannot, but it is still a map.

A snapshot writes an int key and a string key alike, so the test records the
result's type and its keys' types next to the value. Keeping the entries by
turning 1 into "1" would not keep them.

Both implementations currently fail it the same way: measured against each, the
answer is an empty dynamic object, every entry dropped, because the road to an
object runs through a string-keyed map and the int keys fall off on the way.

Constructor_arguments_from_map_with_int_key pins the other half: asked for as
anything but an object, a map with such a key among its entries is read the way a
map keyed by names is. Map_Of_Constructor_Arguments_With_Int_Key puts an int-keyed
entry between the two constructor arguments, and the answer is
{ StringArg: stringValue, IntArg: 42 } - the entry no member can be named after
left out, everything after it still read. Both implementations already answer it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Syxmt77AL1HAfGM6p3LseP
Asked for as an object, a g:Map went through MapDeferralConverterFactory, which
copies it into a JObject for the dynamic object behind it. A JObject only holds
names, so TryParseKey let string and g:T keys through and every other entry was
dropped - a map keyed by ints came back as an empty object.

When the target is object and a key is neither a string nor a g:T, the converter
now asks for a Dictionary<object, object> instead, which MapToDictionaryConverter
already builds from a g:Map, converting each key through the transformer like any
other token. Maps keyed by names, including the element maps whose id and label
arrive under g:T keys, take the JObject road as before.

Other targets are left alone: a POCO has no member a key of 1 could fill, so there
is nothing an int-keyed entry could become there.

The same holds for any key that is not a name, and group() is where a real server
sends them: its default key is the element itself. Group and
Group_with_key_identity in ObjectQueryIntegrationTests had pinned [ {} ] for it -
every group dropped. They now record the groups, each vertex keying the list of
vertices it grouped, the answer both implementations give against Gremlin Server.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Syxmt77AL1HAfGM6p3LseP
@danielcweber
danielcweber force-pushed the fix/int-keyed-map-as-object branch from 837bc3d to 66ad4da Compare September 11, 2026 11:16
@danielcweber
danielcweber changed the base branch from 14.x to fix/unknown-label-element-map-as-object September 11, 2026 13:26

This branch has not been deployed

No deployments
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.

1 participant