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

ODL 9 Release Planning Document #3185

Open
wants to merge 1 commit into
base: dev-9.x
Choose a base branch
from

Conversation

gathogojr
Copy link
Contributor

Description

Working document for ODL 9 release planning to facilitate discussion around goal, scope and milestones.

The scope of ODL 9 includes evaluating and potentially incorporating the issues in the table below. Each item will be assessed based on its impact, feasibility, and alignment with the project's goals.

## .NET Framework Support
ODL 9 will align with modern .NET platform support to ensure compatibility and long-term maintainability. The release will support:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get an agreement that we need to pick a date and ship on that date if for no other reason than to take the latest .NET version (and drop support where applicable)?

The major implication is that if code doesn't get merged to main by that date, we need to go ahead and ship anyway so that we aren't sitting on a release that supports for out-dated .NET version without an alternative.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this in the meeting and had several thoughts about it:

  1. My opinion is that we first implement the add support for the new .NET version and remove support for any out-of-support old versions, and then when we hit our release date, we can always at least fall back to this change.
  2. Another proposal was to have two lists of items, the ones that we must do for release, and the ones that would be nice to have for release. The idea is that we should push the release date until the "must do"s are completed.
  3. Another proposal was to have a release candidate cutoff where any features that are not merged by the time of that RC will be pushed to the next release and we focus on making sure that RC is usable as the final release version.
  4. Try to scope the work so that we are feature complete early and can therefore focus on addressing any major issues in the new changes, though being open to last minute breaking changes and potentially pushing the release date as a result.

At least one aspect we all seem agreed on is that prioritization is important so that we don't miss breaking change opportunities. Making sure that breaks are prioritized higher than non-breaks is a good way to take advantage of the major release.

| Item | Details | Library | Status | Remark |
| ---- | ----------- | ------- | ------ | ------ |
| **[2908](https://github.com/OData/odata.net/issues/2908): Remove `OdataMessageInfo` from the DI container** | It is pointless to have this injectable as a service since we overwrite all the properties of the resolved instance. | Core | In scope | |
| **[2907](https://github.com/OData/odata.net/issues/2907): Remove `IContainerProvider` and pass the `IServiceProvider` directly to the message reader or writer** | | Core | In scope | Consult with Robert McLaws to gather his input on how best to redesign dependency injection in the writers and readers |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Robert's comments in the github issue. That said, even if we can't overhaul the DI patterns that we use, I think this particular area is a massive step in the right direction for improved DI practices. So, I think we should do this work even if we find it's out of scope to overhaul all of our DI usage.

| **[2816](https://github.com/OData/odata.net/issues/2816): Use `ValueTask<T>` instead of `Task<T>` for async I/O APIs** | | Core | | |
| **[2657](https://github.com/OData/odata.net/issues/2657): Resolve untyped numeric value as `Edm.Double` (not `Edm.Decimal` or `Edm.Int32`)** | Currently, untyped/dynamic numeric property is resolved into `Edm.Decimal` if the property is single-valued or `Edm.Int32` if the property is collection-valued. Per the protocol, if there’s no type information, we have to determine the type heuristically – resolve it as an `Edm.Double`. **NOTE**: Change should be behind a feature flag that customers can use to retain the old behaviour | Core | | |
| **[2881](https://github.com/OData/odata.net/issues/2881): Implement cleaner approach to use of character array pool in `JsonWriter`** | Redesign logic around use of character array pool to eliminate coupling of char array pool with the `ODataJsonWriter` | Core | | |
| **[2430](https://github.com/OData/odata.net/issues/2430): `ODataJsonWriter` should use `Encoding.CreateTranscodingStream` in .NET 6+** | Swap `TranscodingStream` with built-in `Encoding.CreateTranscodingStream` used to convert UTF-8 to other encodings | Core | | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is anyone asking for additional encodings? If not, I think this is lower priority.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corranrogue9 The thing is, @habbes had grabbed a whole class from the .NET Core source code and added it to our repo, because 7.x was targeting frameworks below .NET 6. Because the effort involved in getting rid of the class is very minimal, I don't see a reason to retain it in the repo. It's unnecessary tech debt and future maintainers of the library might not have the context about why the class was added in the first place and the cumulative effort of maintaining the class plus tests might continue to grow with time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, yes, please get rid of this technical debt if it's just moving a source reference to a dll reference

| **[2894](https://github.com/OData/odata.net/issues/2894): EdmLib successfully parses type names with unbalanced parens** | Bug in Edm lib causes the parser to parse type names with unbalance parens, e.g., `“Collection(NS.MyType”` | Edm | | |
| **[2420](https://github.com/OData/odata.net/issues/2420): Use `Utf8JsonWriter.WriteRawValue` to implement `ODataUtf8JsonWriter.WriteRawValue` in .NET 6+** | Current implementation of `ODataUtf8JsonWriter.WriteRawValue` manually writes to the stream. Refactor to use `Utf8JsonWriter.WriteRawValue` available in .NET 6+ | Core | | |
| **[3085](https://github.com/OData/odata.net/issues/3085): Change type of `ReturnType` of `IEdmOperation` to use `IEdmOperationReturn`** | Use `IEdmOperationReturn` introduced in 7.x as the type for `IEdmOperation.ReturnType` | Edm | | |
| **[2911](https://github.com/OData/odata.net/issues/2911): `CustomUriFunctions` should not be static or irreversible** | | Core | | Related to [3158](https://github.com/OData/odata.net/issues/3158), [2712](https://github.com/OData/odata.net/issues/2712)? |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one speaks to our overall practices around dependency injection. Do we want to track this "larger" change in DI practices? I don't know if it has value to track something like that, just wondering...

| **[2395](https://github.com/OData/odata.net/issues/2395): Swapping Microsoft.Spatial with .NET Topology Suite** | AzDO: [2981062](https://identitydivision.visualstudio.com/Engineering/_workitems/edit/2981062). Support for spatial types was introduced in EF Core 2.2 via the **.NET Topology Suite** library. EF Core maps the **.NET Topology Suite** spatial types onto SQL database spatial types. **Microsoft.Spatial** types on the other hand have no direct mapping to SQL database spatial types. To improve end-to-end user experience when working with spatial types, it makes sense to replace **Microsoft.Spatial** library with the **.NET Topology Suite** | Core | | |
| **[2696](https://github.com/OData/odata.net/issues/2696): Rationalize `ODataUri` and `ODataUriSlim` into a single type** | AzDO [2671720](https://identitydivision.visualstudio.com/Engineering/_workitems/edit/2671720) | Core | | |
| **[2714](https://github.com/OData/odata.net/issues/2714): `ODataReader` fails reading count for a non-expanded navigation property** | Support reading/writing control information (i.e., count) and annotations on non-expanded collections | Core | | |
| **[3158](https://github.com/OData/odata.net/issues/3158): `CustomUriLiteralParsers` and `CustomUriLiteralPrefixes` should not be static or irreversible** | | Core | | Related to [2911](https://github.com/OData/odata.net/issues/2911), [2712](https://github.com/OData/odata.net/issues/2712)? |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are going to track DI practices, this would be another one

| **[3181](https://github.com/OData/odata.net/issues/3181): Remove obsolete `ODataMessageReaderSettings.ReadUntypedAsString property`** | | Core | | |
| **[3182](https://github.com/OData/odata.net/issues/3182): Remove obsolete `DataServiceContext.Timeout` property** | | Client | | |
| **[3183](https://github.com/OData/odata.net/issues/3183): Resolve the `SYSLIB0051` obsoletion warning reported on `DataServiceClientException`** | | Client | | |

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are on lines 43 and 44

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@habbes Those two are already included :)

| **[3182](https://github.com/OData/odata.net/issues/3182): Remove obsolete `DataServiceContext.Timeout` property** | | Client | | |
| **[3183](https://github.com/OData/odata.net/issues/3183): Resolve the `SYSLIB0051` obsoletion warning reported on `DataServiceClientException`** | | Client | | |

[//]: # "**[](https://github.com/OData/odata.net/issues/): ** | | Core | | |"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are investigations we're making around re-architecting the UriParser and JsonSerializer. Those are still high-level proposals being investigated, but I'm wondering whether they could make the case for ODL 9 (assuming they would be fleshed out and implemented in time) or whether they're too "disruptive".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe they could be separate components that could be integrated once they've been tested extensively in some preview, potentially standalone, packages.

| **[3104](https://github.com/OData/odata.net/issues/3104): Allow `ReadAsStreamFunc` delegate to access property annotations** | To make it possible to check for particular property annotation and read the property as a stream | Core | | Internal? |
| **[3100](https://github.com/OData/odata.net/issues/3100): Allow `ODataUtf8JsonWriter` buffer size to be configurable by the user** | To enable buffer size to be configurable so a customer can control how frequent stream I/O is invoked | Core | | Internal? |
| **[2882](https://github.com/OData/odata.net/issues/2882): Implement a cleaner approach to support for buffering in `JsonWriter`** | To eliminate a hack in `ODataJsonLightOutputContext` where we check if the writer is an `ODataUtfJsonWriter` before using a buffering stream | Core | | |
| **[2727](https://github.com/OData/odata.net/issues/2727): `DataServiceQuerySingle<T>.GetValue` throws `InvalidOperationException`** | Because `GetValue/GetValueAsync` internally calls `Single()` instead or `SingleOrDefault()`, an `InvalidOperationException` exception is thrown if an entity is not found. | Client | | Consider changing the behavior? Would there be any ramifications? |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should do this as proposed. We should change the design so that actions with a single-valued, nullable return type return null when in the right cases. Notice that this is slightly different behavior. For example, if the action with a nullable return type successfully returns null, we should return null. But if the action with a nullable return type fails, we should throw. The proposal as written would suggest that both cases should return null, but this would mean that callers have no way to differentiate between successful null values, and unsuccessful calls.

| **[2727](https://github.com/OData/odata.net/issues/2727): `DataServiceQuerySingle<T>.GetValue` throws `InvalidOperationException`** | Because `GetValue/GetValueAsync` internally calls `Single()` instead or `SingleOrDefault()`, an `InvalidOperationException` exception is thrown if an entity is not found. | Client | | Consider changing the behavior? Would there be any ramifications? |
| **[2717](https://github.com/OData/odata.net/issues/2717): Action and function with the same name in the CSDL cause problems in MS Graph** | | Core | | Should this be implemented and controlled by a setting? |
| **[2662](https://github.com/OData/odata.net/issues/2662): ODL can not read the top-level untyped collection** | | Core | | Resolve numeric values in untyped collection as double ([2657](https://github.com/OData/odata.net/issues/2657))? |
| **[2562](https://github.com/OData/odata.net/issues/2562): Fix logic for `FindNavigationTarget` in major release** | Fix for how binding path works for contained navigation properties was included in 7.4.2. For backward compatibility, the legacy behavior was retained. This needs clean up in a major release. See [`EdmContainedEntitySet.FindNavigationTarget`](https://github.com/OData/odata.net/blob/aba9a63ec5ca1cf8180ab83df8fe953f018e4785/src/Microsoft.OData.Edm/Schema/EdmContainedEntitySet.cs#L147) and [`ExtensionMethods.TryGetRelativeEntitySetPath`](https://github.com/OData/odata.net/blob/aba9a63ec5ca1cf8180ab83df8fe953f018e4785/src/Microsoft.OData.Edm/ExtensionMethods/ExtensionMethods.cs#L3277). Relevant pull request [1109](https://github.com/OData/odata.net/pull/1109), relevant test [`CollectionOfExpandedEntities_Version741AndBefore`](https://github.com/OData/odata.net/blob/aba9a63ec5ca1cf8180ab83df8fe953f018e4785/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/Roundtrip/ContextUrlWriterReaderTests.cs#L787) | Core | | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this issue. I don't necessarily need to understand it, but maybe it's good if we had more details in the issue.

| **[3066](https://github.com/OData/odata.net/issues/3066): Default `EnableCaseInsensitive` to true when initializing a new instance of `ODataUriResolver`** | The call to `AddRouteComponents` in ASP.NET Core OData configures an `ODataUriResolver` with `EnableCaseInsensitive` set to true but when `AddDefaultODataServices` is called, it configures the same property with the value false | Core | In scope | Default behaviour for creating an `ODataUriResolver` should be `EnableCaseInsensitive = true`. Customers can change behaviour by injecting an `ODataUriResolver` with `EnableCaseInsensitive` set to false. Expand this to align the defaults for ASP.NET Core OData with the defaults for ODL. Also align with the OData standard where possible. Default constructor for should initialize the properties to the right defaults |
| **[2801](https://github.com/OData/odata.net/issues/2801): Proposal to create model.Find methods that accept `ReadOnlySpan<&lt;>char>`** | To reduce allocations resulting from splitting path into segments | Core | | |
| **[2816](https://github.com/OData/odata.net/issues/2816): Use `ValueTask<T>` instead of `Task<T>` for async I/O APIs** | | Core | | |
| **[2657](https://github.com/OData/odata.net/issues/2657): Resolve untyped numeric value as `Edm.Double` (not `Edm.Decimal` or `Edm.Int32`)** | Currently, untyped/dynamic numeric property is resolved into `Edm.Decimal` if the property is single-valued or `Edm.Int32` if the property is collection-valued. Per the protocol, if there’s no type information, we have to determine the type heuristically – resolve it as an `Edm.Double`. **NOTE**: Change should be behind a feature flag that customers can use to retain the old behaviour | Core | | |
Copy link
Contributor

@paulodero paulodero Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are considering ODL 9 to be a major release opportunity for breaking changes, why do we still need to have this behind a feature flag? When will we have a better opportunity to default to the correct behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulodero The thing is, this is the kind of behaviour that some particular customers might want to be toggled forever. If you were to review the issue as articulated by Sam, he also expected that a numeric value without the decimal part is resolved as an integer. I don't expect he'd be the only one who'd expect that behaviour. For that reason, to conform with the standard, we should go ahead and fix the behaviour, but have a feature flag (that we could retain in perpetuity) that can accommodate those customers with a similar expectation. I think we have do that before. @mikepizzo can weigh in with on this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have been trying to use the language "feature flag" to mean "keep a breaking semantic change behind an if statement so that customers can maintain the current behavior and we will deprecate the current behavior eventually" and the language "behavior flag" to mean "a configurable behavior that customers will want to set to accommodate different scenarios"

- Review each proposed item to determine its relevance and priority for the ODL 9 release.
- Conduct feasibility analysis to assess the complexity and resources required for implementation.

### 2. Development Phase (March - September 2025)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we checked if our CI pipeline will allow us to take .NET 10 preview versions? Will they be available starting in march?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corranrogue9 That is a good question. We can find out how a team like efcore is able to synchronize their releases with the dotnetcore team. I'd assume they target the preview versions when working on their latest release

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the ADO pipelines do allow pulling the latest preview versions, but I think we have to target it specifically. I don't think that's a problem though.

| **[3085](https://github.com/OData/odata.net/issues/3085): Change type of `ReturnType` of `IEdmOperation` to use `IEdmOperationReturn`** | Use `IEdmOperationReturn` introduced in 7.x as the type for `IEdmOperation.ReturnType` | Edm | | |
| **[2911](https://github.com/OData/odata.net/issues/2911): `CustomUriFunctions` should not be static or irreversible** | | Core | | Related to [3158](https://github.com/OData/odata.net/issues/3158), [2712](https://github.com/OData/odata.net/issues/2712)? |
| **[3064](https://github.com/OData/odata.net/issues/3064): POST payload generated by OData client contains `@odata.type` property annotations for both declared and dynamic enum properties** | Fix bug where OData client generates `@odata.type` property annotations for declared enum properties which are not required and cause the payload to be verbose | Client | | |
| **[3082](https://github.com/OData/odata.net/issues/3082): Implement full support for `DateOnly` and `TimeOnly` in OData Client and deprecate `Date` and `TimeOfDay`** | Support for `DateOnly` and `TimeOnly` is already implemented in OData Core. For OData Client to achieve parity, **OData Connected Service** should be refactored to emit `DateOnly` and `TimeOnly` properties (controlled by a setting to toggle legacy behaviour?) and OData Client adapted to work with the new types | Client | | |
Copy link
Contributor

@paulodero paulodero Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything that prevents us from shipping this in ODL 8 and wait for ODL 9? I would rather focus on items that can only be released in ODL 9 and continue working on other features/bugs under ODL 8 with the current release cadence.

@gathogojr gathogojr force-pushed the odl9-release-planning branch from b347970 to 0630350 Compare February 18, 2025 18:19
- Release a beta version incorporating feedback from the preview phase.
- Continue to monitor and address any reported issues.

### 5. Release Candidate (October 2025)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is the cutoff for new features/changes?

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