[Dependencies]: Removes direct reference for Newtonfoft package and add a target build.

[aavasthy]

Pull Request Template

Description

The intent of this change is to ensure that the Newtonsoft.Json dependency, which is currently used within the SDK, does not propagate to downstream consumers as Package dependency. The SDK references Newtonsoft.Json for its internal functionality, but this package should not be exposed to users of the SDK. This measure also avoids the risk of consumers relying on a specific version of Newtonsoft.Json that might have vulnerabilities or other limitations.

Type of change

• Dependency Management Update:
  • Make Newtonsoft.Json package reference as private asset.
  • Updated Microsoft.Azure.Cosmos.targets to ensure the Newtonsoft.Json dependency is not passed to consumer apps and there is a build failure when the consumer app do not have Newtonsoft package installed.
  • Provide an option to consumer app to disable this newtonsoft package build failure by setting DisableNewtonsoftJsonCheck to true.

  	<PropertyGroup>
		<DisableNewtonsoftJsonCheck>true</DisableNewtonsoftJsonCheck>
	</PropertyGroup>

Testing the change on a consumer console app with updated dotnet SDK package without installing newtonsoft package in consumer app:

• Installing the Microsoft.Azure.Cosmos package in the console app:
  • No Newtonsoft package dependency getting added in consumer app.
  • Build failure when no newtonsoft package is available. Failure message "The Newtonsoft.Json package must be explicitly referenced with version >= 10.0.2. Please add a reference to Newtonsoft.Json, or set the 'AzureCosmosDisableNewtonsoftJsonCheck' property to 'true' to bypass this check."

Once the Newtonsoft nuget package is included the build succeeds:

Testing the change on a consumer console app with updated dotnet SDK package when consumer app installs any other version of newtonsoft package on their end:

Testing the change on a consumer console app with updated dotnet SDK package when consumer app uses packages.config:

Testing the change on a consumer console app with updated dotnet SDK package and Newtonsoft available as transitive dependency:

Testing the change on a consumer console app with updated dotnet SDK package and build failure and allowing consumer app the option to opt out this build failure due to missing newtonsoft:

Testing the change on a consumer console app by installing updated Microsoft.Azure.Cosmos.Encryption which has dependency on updated Microsoft.Azure.Cosmos package. The build failure happens if Newtonsoft is missing:

Please delete options that are not relevant.

• [] Bug fix (non-breaking change which fixes an issue)
• [] New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [] This change requires a documentation update

Closing issues

To automatically close an issue: closes #4674

[bartelink]

Really appreciate the time you took to write the overview (and the fact that it's also very well written!)

[Pilchie]

Slight nit in the overview - you should probably check "Breaking Change" instead of non-breaking bug fix.

[eerhardt]

LGTM. Thanks!

[roji]

What would be the guidance here for a layer on top of the Cosmos SDK, e.g. the EF provider for Cosmos? Following the logic in this issue, the EF provider shouldn't reference a specific version of Newtonsoft.JSON for the exact same reason as Microsoft.Azure.Cosmos shouldn't. But if I understand correctly, only the project directly referencing Microsoft.Azure.Cosmos gets the build error about the lack of Newtonsoft.JSON, so that wouldn't work...

In other words, it seems like the technique here doesn't work well for transitive scenarios where Microsoft.Azure.Cosmos isn't referenced directly by the user's end application, but rather via another layer.

(This isn't an immediate concern for EF since the EF provider uses Newtonsoft.JSON directly for some things, and so must reference that in any case at the moment; but that may change. See dotnet/efcore#35171.)

[eerhardt]

What would be the guidance here for a layer on top of the Cosmos SDK, e.g. the EF provider for Cosmos?

The .NET Aspire CosmosDB integration fits into this. My plan for it is to set AzureCosmosDisableNewtonsoftJsonCheck=true in Aspire.Microsoft.Azure.CosmosDB's .csproj.

But if I understand correctly, only the project directly referencing Microsoft.Azure.Cosmos gets the build error about the lack of Newtonsoft.JSON, so that wouldn't work...

This isn't correct. Since the MSBuild logic is going into the buildTransitive folder, it will apply for any project that references Microsoft.Azure.Cosmos (directly or transitively).

With this plan, .NET Aspire's CosmosDB integration doesn't need to set a Newtonsoft.Json dependency, but anyone referencing it will.

[roji]

This isn't correct. Since the MSBuild logic is going into the buildTransitive folder, it will apply for any project that references Microsoft.Azure.Cosmos (directly or transitively).

I see, thanks - I missed that part. So the EF provider will be able to do the same then.

[ElisaPijnAvanade]

I'm having my own internal library using Microsoft.Azure.Cosmos and a different project relying on my internal library.
In my project I'm not using Newtonsoft.Json but using System.Text.Json. So I don't want to install Newtonsoft.Json on my own library as I don't need it.
Do I understand it correctly that on my internal library I can set the AzureCosmosDisableNewtonsoftJsonCheck=true and that way I can still use the Microsoft.Azure.Cosmos, as Microsoft.Azure.Cosmos will handle the dependency internally?
Is it true that I need to set this AzureCosmosDisableNewtonsoftJsonCheck=true on all the projects that are relying on my internal library?

Update:
the projects build correctly with the AzureCosmosDisableNewtonsoftJsonCheck=true, however when using CosmosClientBuilder("someconnectionstring").Build() I get an "Unhandled exception: System.IO.FileNotFoundException: Could not load file or assembly 'Newtonsoft.Json, Version=10.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed'. The system cannot find the file specified."
It looks like I'm forced to install Newtonsoft.Json on my own project, is that correct? In that case what is the purpose of the AzureCosmosDisableNewtonsoftJsonCheck=true setting?

[Pilchie]

Do I understand it correctly that on my internal library I can set the AzureCosmosDisableNewtonsoftJsonCheck=true and that way I can still use the Microsoft.Azure.Cosmos, as Microsoft.Azure.Cosmos will handle the dependency internally?

No, this is not correct. Someone, somewhere still needs to include a reference to Newtonsoft.Json, and it needs to be deployed with your applications. Microsoft.Azure.Cosmos still depends on it internally, but we want to get out of the business of being the package that dictates the version. So, you have two choices:

1. Either your library adds the reference.
2. You add the AzureCosmosDisableNewtonsoftJsonCheck=true and pass that responsibility on to consumers of your library.