diff --git a/CHANGELOG.md b/CHANGELOG.md index 89ac859..188b3e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.12.2] - 2024-08-23 + +### Changed + +- Fixed a bug where calls to ApiClientBuilder.EnableBackingStoreForParseNodeFactory and ApiClientBuilder.EnableBackingStoreForSerializationWriterFactory would enable a BackingStore around BackingStores. [#2563] (https://github.com/microsoftgraph/msgraph-sdk-dotnet/issues/2563) [#2588] (https://github.com/microsoftgraph/msgraph-sdk-dotnet/issues/2588) + ## [1.12.1] - 2024-08-21 ### Changed diff --git a/Directory.Build.props b/Directory.Build.props index 279d89b..d7c7528 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,7 +1,7 @@ - 1.12.1 + 1.12.2 false diff --git a/src/abstractions/ApiClientBuilder.cs b/src/abstractions/ApiClientBuilder.cs index 9f9bcee..e83e5f3 100644 --- a/src/abstractions/ApiClientBuilder.cs +++ b/src/abstractions/ApiClientBuilder.cs @@ -50,6 +50,9 @@ public static ISerializationWriterFactory EnableBackingStoreForSerializationWrit if(registry != SerializationWriterFactoryRegistry.DefaultInstance)// if the registry is the default instance, we already enabled it above. No need to do it twice EnableBackingStoreForSerializationRegistry(SerializationWriterFactoryRegistry.DefaultInstance); } + if(result is BackingStoreSerializationWriterProxyFactory) + //We are already enabled so use it. + return result; else result = new BackingStoreSerializationWriterProxyFactory(original); @@ -69,6 +72,9 @@ public static IParseNodeFactory EnableBackingStoreForParseNodeFactory(IParseNode if(registry != ParseNodeFactoryRegistry.DefaultInstance)// if the registry is the default instance, we already enabled it above. No need to do it twice EnableBackingStoreForParseNodeRegistry(ParseNodeFactoryRegistry.DefaultInstance); } + if(result is BackingStoreParseNodeFactory) + //We are already enabled so use it. + return result; else result = new BackingStoreParseNodeFactory(original); @@ -80,7 +86,7 @@ private static void EnableBackingStoreForParseNodeRegistry(ParseNodeFactoryRegis var keysToUpdate = new List(); foreach(var entry in registry.ContentTypeAssociatedFactories) { - if(entry.Value is not (BackingStoreSerializationWriterProxyFactory or SerializationWriterFactoryRegistry)) + if(entry.Value is not BackingStoreParseNodeFactory) { keysToUpdate.Add(entry.Key); } @@ -97,7 +103,7 @@ private static void EnableBackingStoreForSerializationRegistry(SerializationWrit var keysToUpdate = new List(); foreach(var entry in registry.ContentTypeAssociatedFactories) { - if(entry.Value is not (BackingStoreSerializationWriterProxyFactory or SerializationWriterFactoryRegistry)) + if(entry.Value is not BackingStoreSerializationWriterProxyFactory) { keysToUpdate.Add(entry.Key); } diff --git a/tests/abstractions/ApiClientBuilderTests.cs b/tests/abstractions/ApiClientBuilderTests.cs index e278537..09a977b 100644 --- a/tests/abstractions/ApiClientBuilderTests.cs +++ b/tests/abstractions/ApiClientBuilderTests.cs @@ -62,6 +62,45 @@ public void EnableBackingStoreForParseNodeFactory() Assert.IsType(parseNodeRegistry.ContentTypeAssociatedFactories[StreamContentType]); } + [Fact] + public void EnableBackingStoreForParseNodeFactoryMultipleCallsDoesNotDoubleWrap() + { + // Arrange + //it is not normal to test a private field, but it is the purpose of the test + var concreteFieldInfo = typeof(ParseNodeProxyFactory) + .GetField("_concrete", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + + + var parseNodeRegistry = new ParseNodeFactoryRegistry(); + var mockParseNodeFactory = new Mock(); + parseNodeRegistry.ContentTypeAssociatedFactories.TryAdd(StreamContentType, mockParseNodeFactory.Object); + + Assert.IsNotType(parseNodeRegistry.ContentTypeAssociatedFactories[StreamContentType]); + + // Act + var firstResult = ApiClientBuilder.EnableBackingStoreForParseNodeFactory(parseNodeRegistry); + var secondResult = ApiClientBuilder.EnableBackingStoreForParseNodeFactory(firstResult); + var thirdResult = ApiClientBuilder.EnableBackingStoreForParseNodeFactory(secondResult); + + + //make sure the original was not modifed + Assert.IsNotType(parseNodeRegistry); + + // Assert the type has changed due to backing store enabling + Assert.IsType(firstResult); + + //make sure the second call returned the original wrapper + Assert.Equal(firstResult, secondResult); + Assert.Equal(firstResult, thirdResult); + + //make sure what is in the registry is a BackingStore + var factory = parseNodeRegistry.ContentTypeAssociatedFactories[StreamContentType]; + Assert.IsType(factory); + + //make sure the concrete version of the factory is the same as the orginal + Assert.Equal(mockParseNodeFactory.Object, concreteFieldInfo!.GetValue(factory)); + } + [Fact] public void EnableBackingStoreForParseNodeFactoryAlsoEnablesForDefaultInstance() { @@ -80,5 +119,38 @@ public void EnableBackingStoreForParseNodeFactoryAlsoEnablesForDefaultInstance() Assert.IsType(parseNodeRegistry.ContentTypeAssociatedFactories[StreamContentType]); Assert.IsType(ParseNodeFactoryRegistry.DefaultInstance.ContentTypeAssociatedFactories[StreamContentType]); } + + [Fact] + public void EnableBackingStoreForParseNodeFactoryAlsoEnablesForDefaultInstanceMultipleCallsDoesNotDoubleWrap() + { + // Arrange + var parseNodeRegistry = new ParseNodeFactoryRegistry(); + var mockParseNodeFactory = new Mock(); + parseNodeRegistry.ContentTypeAssociatedFactories.TryAdd(StreamContentType, mockParseNodeFactory.Object); + ParseNodeFactoryRegistry.DefaultInstance.ContentTypeAssociatedFactories.TryAdd(StreamContentType, mockParseNodeFactory.Object); + + Assert.IsNotType(parseNodeRegistry.ContentTypeAssociatedFactories[StreamContentType]); + + // Act + var firstResult = ApiClientBuilder.EnableBackingStoreForParseNodeFactory(parseNodeRegistry); + var secondResult = ApiClientBuilder.EnableBackingStoreForParseNodeFactory(firstResult); + var thirdResult = ApiClientBuilder.EnableBackingStoreForParseNodeFactory(secondResult); + + + //make sure the original was not modifed + Assert.IsNotType(parseNodeRegistry); + + // Assert the type has changed due to backing store enabling + Assert.IsType(firstResult); + + //make sure the second call returned the original wrapper + Assert.Equal(firstResult, secondResult); + Assert.Equal(firstResult, thirdResult); + + //make sure what is in the registry is a BackingStore, it will be a new object so we can only check the type + var factory = ParseNodeFactoryRegistry.DefaultInstance.ContentTypeAssociatedFactories[StreamContentType]; + Assert.IsType(factory); + + } } }