CSHARP-4886: Mark API that will be removed in 3.0 as obsolete: MongoDB.Bson #1243

BorisDog · 2023-12-28T18:41:06Z

Additional entities to consider:
Utf8Helper
IBsonChunk
IBsonChunkSource
IByteBuffer
IChildSerializerConfigurable

…B.Bson

BorisDog · 2023-12-28T18:42:36Z

tests/BuildProps/Tests.Build.props

@@ -54,4 +54,25 @@
    </PackageReference>
  </ItemGroup>

+  <PropertyGroup>
+    <NoWarn>
+      1701;1702; <!--https://github.com/dotnet/roslyn/issues/19640-->


It seemed more efficient to define all warnings suppressions in single place for tests.

This is a sign that these changes will be painful for our users.

But hopefully significantly less painful than just removing API in 3.0 without prior warning.

BorisDog · 2023-12-28T18:52:12Z

src/MongoDB.Bson/IO/JsonWriterSettings.cs

@@ -47,6 +47,7 @@ public JsonWriterSettings()
        /// <summary>
        /// Gets or sets the default JsonWriterSettings.
        /// </summary>
+        [Obsolete("This property will be removed in later release.")]


The intention is also to remove all the static defaults and settings related to serialization.
We don't have new builders and configuration API defined yet, and for some time they will co-exist, but it's better to start obsoleting them as early as possible.

rstam

First: I think we are doing this way too prematurely. Am waiting for @JamesKovacs to get back to revisit this question with him.

Second: I think there is some uncertainty about what exactly is going to be deprecated. We won't really know for sure until we are close to being done with 3.0 and see what ended up changing and what didn't.

rstam · 2023-12-29T18:00:26Z

src/MongoDB.Bson/ObjectModel/BsonSymbol.cs

@@ -21,6 +21,7 @@ namespace MongoDB.Bson
    /// <summary>
    /// Represents a BSON symbol value.
    /// </summary>
+    [Obsolete("This class will be removed in later release.")]


Why is this being deprecated? What if a document in the database contains this type of BSON value?

Looks like Symbol was deprecated in 1.7.
I am not sure that we should remove this, but we do need to inform users that those classes are not expected to be used. I've changed the comment and added it for the related classes missed before.

rstam · 2023-12-29T18:00:43Z

src/MongoDB.Bson/ObjectModel/BsonType.cs

        /// <summary>
        /// A BSON undefined value.
        /// </summary>
+        [Obsolete("Undefined type is depricated and will be removed in later release.")]


Why is this being deprecated? What if a document in the database contains this type of BSON value?

It's depreciated, so users should be informed not to use it accidently.

rstam · 2023-12-29T18:00:51Z

src/MongoDB.Bson/ObjectModel/BsonType.cs

        /// <summary>
        /// A BSON symbol.
        /// </summary>
+        [Obsolete("Symbol type is depricated and will be removed in later release.")]


Why is this being deprecated? What if a document in the database contains this type of BSON value?

It's depreciated, so users should be informed not to use it accidently.

rstam · 2024-01-04T02:00:16Z

src/Directory.Build.props

+
+  <PropertyGroup>
+    <NoWarn>
+      CS0618 <!--Temporary disable obsolete warnings due the massive volume of warnings-->


This is a sign that these changes will be painful for our users.

Yes, but less painful than just API removal without prior warning. Which is the intention of these obsoletions PRs.

I think we should have a goal that users who stay on 2.x don't have to deal with these warnings at all.

rstam · 2024-01-04T02:04:10Z

src/MongoDB.Bson/Serialization/IdGenerators/BsonBinaryDataGuidGenerator.cs

+        /// <summary>
+        /// Gets an instance of BsonBinaryDataGuidGenerator for Unspecified GuidRepresentation.
+        /// </summary>
+        public static BsonBinaryDataGuidGenerator UnspecifiedInstance


Doesn't this duplicate the property of the same name on line 84?

Yes, fixing a typo.

rstam · 2024-01-04T18:53:26Z

src/MongoDB.Bson/Serialization/IdGenerators/ZeroIdChecker.cs

@@ -22,6 +22,7 @@ namespace MongoDB.Bson.Serialization.IdGenerators
    /// </summary>
    /// <typeparam name="T">The type of the Id.</typeparam>
    // TODO: is it worth trying to remove the dependency on IEquatable<T>?
+    [Obsolete("This class will be removed in later release.")]


rstam · 2024-01-04T18:53:31Z

src/MongoDB.Bson/Serialization/PrimitiveSerializationProvider.cs

@@ -26,6 +26,7 @@ namespace MongoDB.Bson.Serialization
    /// <summary>
    /// Provides serializers for primitive types.
    /// </summary>
+    [Obsolete("This class will be removed in later release.")]


Will our user be needing our built-in serializer providers?

rstam · 2024-01-04T18:53:53Z

src/MongoDB.Bson/Serialization/TypeMappingSerializationProvider.cs

@@ -22,6 +22,7 @@ namespace MongoDB.Bson.Serialization
    /// <summary>
    /// Represents a serialization provider based on a mapping from value types to serializer types.
    /// </summary>
+    [Obsolete("This class will be removed in later release.")]


rstam · 2024-01-04T18:54:13Z

tests/BuildProps/Tests.Build.props

@@ -54,4 +54,25 @@
    </PackageReference>
  </ItemGroup>

+  <PropertyGroup>
+    <NoWarn>
+      1701;1702; <!--https://github.com/dotnet/roslyn/issues/19640-->


This is a sign that these changes will be painful for our users.

rstam · 2024-01-04T18:54:23Z

src/MongoDB.Driver.Core/MongoDB.Driver.Core.csproj

+    <NoWarn>
+      NU5100;
+      CS0618 <!--Temporary disable obsolete warnings due the massive volume of warnings-->
+    </NoWarn>


This is a sign that these changes will be painful for our users.

But hopefully significantly less painful than just removing API in 3.0 without prior warning.

BorisDog

Addressed some of the comments.
Please advise what else needs to be deprecated.

BorisDog · 2024-03-01T18:28:00Z

src/Directory.Build.props

+
+  <PropertyGroup>
+    <NoWarn>
+      CS0618 <!--Temporary disable obsolete warnings due the massive volume of warnings-->


Yes, but less painful than just API removal without prior warning. Which is the intention of these obsoletions PRs.

BorisDog · 2024-03-01T18:54:40Z

src/MongoDB.Bson/ObjectModel/BsonSymbol.cs

@@ -21,6 +21,7 @@ namespace MongoDB.Bson
    /// <summary>
    /// Represents a BSON symbol value.
    /// </summary>
+    [Obsolete("This class will be removed in later release.")]


Looks like Symbol was deprecated in 1.7.
I am not sure that we should remove this, but we do need to inform users that those classes are not expected to be used. I've changed the comment and added it for the related classes missed before.

BorisDog · 2024-03-01T22:18:25Z

src/MongoDB.Bson/ObjectModel/BsonType.cs

        /// <summary>
        /// A BSON undefined value.
        /// </summary>
+        [Obsolete("Undefined type is depricated and will be removed in later release.")]


It's depreciated, so users should be informed not to use it accidently.

BorisDog · 2024-03-01T22:18:28Z

src/MongoDB.Bson/ObjectModel/BsonType.cs

        /// <summary>
        /// A BSON symbol.
        /// </summary>
+        [Obsolete("Symbol type is depricated and will be removed in later release.")]


It's depreciated, so users should be informed not to use it accidently.

BorisDog · 2024-03-06T20:55:27Z

src/MongoDB.Bson/Serialization/AttributedSerializationProvider.cs

@@ -23,6 +23,7 @@ namespace MongoDB.Bson.Serialization
    /// <summary>
    /// Provides serializers based on an attribute.
    /// </summary>
+    [Obsolete("This class will be removed in later release.")]


Do we need it as a pubic class?

BorisDog · 2024-03-06T21:05:42Z

src/MongoDB.Bson/Serialization/IdGenerators/ZeroIdChecker.cs

@@ -22,6 +22,7 @@ namespace MongoDB.Bson.Serialization.IdGenerators
    /// </summary>
    /// <typeparam name="T">The type of the Id.</typeparam>
    // TODO: is it worth trying to remove the dependency on IEquatable<T>?
+    [Obsolete("This class will be removed in later release.")]


BorisDog · 2024-03-06T21:08:58Z

src/MongoDB.Bson/Serialization/PrimitiveSerializationProvider.cs

@@ -26,6 +26,7 @@ namespace MongoDB.Bson.Serialization
    /// <summary>
    /// Provides serializers for primitive types.
    /// </summary>
+    [Obsolete("This class will be removed in later release.")]


Will our user be needing our built-in serializer providers?

BorisDog · 2024-03-06T21:09:21Z

src/MongoDB.Bson/Serialization/Serializers/TupleSerializers.cs

@@ -26,7 +26,7 @@ namespace MongoDB.Bson.Serialization.Serializers
    public interface IBsonTupleSerializer
    {
        /// <summary>
-        /// Gets ths serializer for an item.
+        /// Gets this serializer for an item.


Thanks, done.

BorisDog · 2024-03-06T21:10:43Z

src/MongoDB.Driver.Core/MongoDB.Driver.Core.csproj

+    <NoWarn>
+      NU5100;
+      CS0618 <!--Temporary disable obsolete warnings due the massive volume of warnings-->
+    </NoWarn>


But hopefully significantly less painful than just removing API in 3.0 without prior warning.

BorisDog · 2024-03-06T21:12:03Z

tests/BuildProps/Tests.Build.props

@@ -54,4 +54,25 @@
    </PackageReference>
  </ItemGroup>

+  <PropertyGroup>
+    <NoWarn>
+      1701;1702; <!--https://github.com/dotnet/roslyn/issues/19640-->


But hopefully significantly less painful than just removing API in 3.0 without prior warning.

rstam · 2024-03-13T17:39:08Z

src/Directory.Build.props

+
+  <PropertyGroup>
+    <NoWarn>
+      CS0618 <!--Temporary disable obsolete warnings due the massive volume of warnings-->


I think we should have a goal that users who stay on 2.x don't have to deal with these warnings at all.

rstam · 2024-03-13T17:42:57Z

src/MongoDB.Bson/Serialization/CollectionsSerializationProvider.cs

@@ -28,6 +28,7 @@ namespace MongoDB.Bson.Serialization
    /// <summary>
    /// Provides serializers for collections.
    /// </summary>
+    [Obsolete("This class will be removed in later release.")]


Users can write custom serialization providers and in general configure any mix of serialization providers in any order. To do so the providers need to be public.

Removing these classes (or making them internal) removes a feature.

rstam · 2024-03-13T17:43:38Z

src/MongoDB.Bson/Serialization/CreatorMapDelegateCompiler.cs

@@ -24,6 +24,7 @@ namespace MongoDB.Bson.Serialization
    /// <summary>
    /// A helper class used to create and compile delegates for creator maps.
    /// </summary>
+    [Obsolete("This class will be removed in later release.")]


I thought when you said "removed" you meant "removed".

It probably can be made internal.

rstam · 2024-03-13T17:46:45Z

src/MongoDB.Bson/ObjectModel/BsonSymbol.cs

-        }
-    }
-}
+/* Copyright 2010-present MongoDB Inc.


What happened here?

Why does the entire file appear to be modified?

Same for several other files below.

rstam · 2024-03-13T17:47:08Z

src/MongoDB.Bson/Serialization/Serializers/BsonValueSerializer.cs

-        }
-    }
-}
+/* Copyright 2010-present MongoDB Inc.


Hard to tell if you changed anything in this file or not...

Same for several other files.

BorisDog added 2 commits December 27, 2023 11:37

CSHARP-4886: Mark API that will be removed in 3.0 as obsolete: MongoD…

fb35b8e

…B.Bson

- Revert unrelated changes

58ccbce

BorisDog requested a review from a team as a code owner December 28, 2023 18:41

BorisDog requested review from adelinowona, rstam and sanych-sun and removed request for a team and adelinowona December 28, 2023 18:41

BorisDog added 2 commits December 28, 2023 10:46

- Revert unrelated changes

b3136da

- Revert unrelated changes

3f876e3

BorisDog commented Dec 28, 2023

View reviewed changes

rstam requested changes Jan 4, 2024

View reviewed changes

BorisDog changed the title ~~CSHARP4886: Mark API that will be removed in 3.0 as obsolete: MongoDB.Bson~~ CSHARP-4886: Mark API that will be removed in 3.0 as obsolete: MongoDB.Bson Mar 1, 2024

- PR comments

fd43e74

BorisDog commented Mar 6, 2024

View reviewed changes

rstam self-requested a review March 7, 2024 17:10

rstam requested changes Mar 13, 2024

View reviewed changes

CSHARP-4886: Mark API that will be removed in 3.0 as obsolete: MongoDB.Bson #1243

Are you sure you want to change the base?

CSHARP-4886: Mark API that will be removed in 3.0 as obsolete: MongoDB.Bson #1243

Conversation

BorisDog commented Dec 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rstam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BorisDog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment