Skip to content

RDBC-1037 RDBC-1038 Fix load_into_stream and implement stream_into/to_stream#278

Open
redknightlois wants to merge 1 commit intoravendb:v7.1from
redknightlois:RDBC-1037
Open

RDBC-1037 RDBC-1038 Fix load_into_stream and implement stream_into/to_stream#278
redknightlois wants to merge 1 commit intoravendb:v7.1from
redknightlois:RDBC-1037

Conversation

@redknightlois
Copy link
Member

Issue link

https://issues.hibernatingrhinos.com/issue/RDBC-1037
https://issues.hibernatingrhinos.com/issue/RDBC-1038

Additional description

RDBC-1037session.advanced.load_into_stream() and load_starting_with_into_stream() were broken: the implementation was reading from the output stream instead of writing to it, so callers always received an empty result. Fixed to serialize documents and write them to the provided stream.

RDBC-1038 – Adds session.advanced.stream_into(query, output), which streams query results incrementally to an output stream without buffering the full result set in memory. Also adds DocumentQuery.to_stream() and RawDocumentQuery.to_stream() as convenience wrappers. The output format matches the C# StreamInto wire format ({"Results":[...]}).

Type of change

  • Bug fix
  • Regression bug fix
  • Optimization
  • New feature

How risky is the change?

  • Low
  • Moderate
  • High
  • Not relevant

Backward compatibility

  • Non breaking change
  • Ensured. Please explain how has it been implemented?
  • Breaking change
  • Not relevant

Is it platform specific issue?

  • Yes. Please list the affected platforms.
  • No

Documentation update

  • This change requires a documentation update. Please mark the issue on YouTrack using Documentation Required tag.
  • No documentation update is needed

Testing by Contributor

  • Tests have been added that prove the fix is effective or that the feature works
  • Internal classes added to the test class (e.g. entity or index definition classes) have the lowest possible access modifier (preferable private)
  • It has been verified by manual testing
  • Existing tests verify the correct behavior

Testing by RavenDB QA team

  • This change requires a special QA testing due to possible performance or resources usage implications (CPU, memory, IO). Please mark the issue on YouTrack using QA Required tag.
  • No special testing by RavenDB QA team is needed

Is there any existing behavior change of other features due to this change?

  • Yes. Please list the affected features/subsystems and provide appropriate explanation
  • No

UI work

  • It requires further work in the Studio. Please mark the issue on YouTrack using Studio Required tag.
  • No UI work is needed

…ssion streaming APIs

- _load_internal_stream: write serialized result to output stream instead of reading from it; broaden exception handler from IOError to Exception and use proper raise-from chaining; filter None fields from to_json() output so the stream matches the server wire format (only fields the server actually sent)
- load_starting_with_into_stream: add required output parameter, validate it, fix grammar error in id_prefix guard message, delegate to internal helper; filter None fields at write site
- stream_into: implemented; writes {"Results":[...]} incrementally to output as JSONL items arrive — no full in-memory buffering; matches C# StreamInto format expected by JObject.Load / json.GetValue("Results") tests
- DocumentQuery.to_stream / RawDocumentQuery.to_stream: convenience wrappers over session.advanced.stream_into

Tests:
- dotnet migrated test (RDBC-1037): load_into_stream and load_starting_with_into_stream write valid JSON to BytesIO; every integration test asserts exact count and field values
- dotnet migrated test (RDBC-1038): stream_into and to_stream write Results JSON; each integration test asserts count and field values for every result; unit test verifies the method rejects non-query arguments at runtime
- JVM migrated test: adds test_can_load_by_ids_into_stream (port of C# CanLoadByIdsIntoStream) alongside the existing load_starting_with test
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