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

Allow ReadAsStreamFunc delegate to access property annotations #3104

Open
habbes opened this issue Nov 1, 2024 · 0 comments
Open

Allow ReadAsStreamFunc delegate to access property annotations #3104

habbes opened this issue Nov 1, 2024 · 0 comments
Assignees

Comments

@habbes
Copy link
Contributor

habbes commented Nov 1, 2024

A customer is facing issues handling deserialization of payloads with large string values, e.g.:

{
   "[email protected]": true,
   "Foo": "very large string"
}

Where Foo is a field with a very large value (e.g. 30MB). Changing the API is not an option form (e.g. changing the type to a stream property).

Furthermore, the payload is an open type, so the property names that can contain large values are not known in advance. They resort to using that is.Large custom annotation to tell which properties have large values.

Reading the entire value in memory causes large allocations in LOH (Large Object Heap) and subsequent performance issues. They are asking for a way to read such values using a stream.

ODL does provide such a solution via the ODataMessageReaderSettings.ReadAsStreamFunc property. The property accepts a predicate Func which will be called for each property, and should return true for properties which should be read as a stream instead of inline. For such properties, the OData reader will return an ODataReaderState.Stream state and they can use reader.CreateTextReaderAsync() or reader.CreateBinaryReaderAsync() to read string value or base64-encoded binary value respectively. This can allow them to read the data in chunks instead of buffering the entire value in memory. You can see an example of how this setting is used in this test: https://github.com/OData/odata.net/blob/release-7.x/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightReaderTests.cs#L846

Unfortunately, they cannot use ReadAsStreamFunc because they do not know the name of the properties in advance since they are dynamic properties. They also cannot provide a delegate that always returns true because they don't want to force streaming for all properties, even those with small values, the overhead would cause undesired performance issues.

They want to apply stream selectively, using the custom is.Large annotation. But ReadAsStreamFunc does not have access to the property annotations.

At the time where we call ReadAsStreamFunc we have access to the property annotations that have been read so far via the resourceState.PropertyAnnotationCollector(): https://github.com/OData/odata.net/blob/release-7.x/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs#L3597. But if we were to change the signature of the ReadAsStreamFunc to add new parameters, that would be a breaking change.

An non-breaking alternative we could consider is to add a new Func property to ODataMessageReaderSettings that accepts a "future-proof` context object that encapsulates relevant information, including property annotations. Then we can mark the current property for deprecation in ODL 9.

Assemblies affected

Microsoft.OData.Core 7.x and 8.x

Additional detail

Optional, details of the root cause if known. Delete this section if you have no additional details to add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants