Isolate scoped settings in default OData services

[habbes]

Issues

*This pull request fixes #3070 *

Description

Clones the reader, writer and URI parser settings when registering them as scoped services in AddDefaultODataServices() to ensure that different request scopes get different copies that can be independently modified.

Checklist (Uncheck if it is not completed)

• Test cases added
• Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

[marabooy]

question: Would there be an issue if we called configure reader action multiple times e.g. if we used a delegate instead of the cloned instance?

[habbes]

Are you referring to this approach:

services.AddScope(_ => {
    var readerSettings = new ODataMessageReaderSettings(...);
    configureReaderAction?.Invoke(readerSettings);
    return readerSettings;
});

If so, I considered it (and mentioned it in the attached issue). The reason I opted for cloning instead of that approach is because we can't predict the cost of the delegate since it's user defined, for all we know it could be reading from a file or database. On the other hand, since we control the implementation of the Clone() method, we can make better assumptions about it cost and we can optimize if it becomes a hotspot, so I think it's safer to err on the side of calling Clone() multiple times rather than calling the opaque user-defined delegate multiple times.

[marabooy]

LGTM

[syprieur]

When an ODataMessageWriterSettings is passed as argument to an ODataMessageWriter, the scoped ODataMessageWriterSettings service is being updated to that value?

[habbes]

@syprieur That's a very good question, apparently it does :( The settings from the ODataMessageWriterSettings are copied to the scoped ODataMessageWriterSettings service:

see:

• https://github.com/OData/odata.net/blob/main/src/Microsoft.OData.Core/ODataMessageWriter.cs#L110
• https://github.com/OData/odata.net/blob/main/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs#L485

which calls:

internal static ODataMessageWriterSettings CreateWriterSettings(
    IServiceProvider container,
    ODataMessageWriterSettings other)
{
    ODataMessageWriterSettings writerSettings;
    if (container == null)
    {
        writerSettings = new ODataMessageWriterSettings();
    }
    else
    {
       // we get the scoped settings
        writerSettings = container.GetRequiredService<ODataMessageWriterSettings>();
    }

    if (other != null)
    {
        // then overwrite the scoped service based on the settings passed to the writer
        writerSettings.CopyFrom(other);
    }

    return writerSettings;
}

[syprieur]

That explains the observed bug and confirms the hypothesis: the same settings instance is indeed re-used and updated concurrently.
Your change will fix it.