From 8d6c020eb0436be096569abae09f6e7a64bee053 Mon Sep 17 00:00:00 2001 From: Alan Wong Date: Mon, 4 Jun 2018 17:24:43 -0700 Subject: [PATCH 1/3] Removing model container and maintaining only one Edm model structure at a time --- .../Microsoft.AspNet.OData.Shared.projitems | 1 - .../Query/Expressions/ModelContainer.cs | 32 --------------- .../Query/Expressions/SelectExpandBinder.cs | 9 ++--- .../Query/Expressions/SelectExpandWrapper.cs | 17 +++----- ...crosoft.AspNet.OData.Test.Shared.projitems | 1 - .../Query/Expressions/ModelContainerTest.cs | 38 ------------------ .../Expressions/SelectExpandBinderTest.cs | 4 +- .../Expressions/SelectExpandWrapperTest.cs | 40 +++++++++---------- 8 files changed, 29 insertions(+), 113 deletions(-) delete mode 100644 src/Microsoft.AspNet.OData.Shared/Query/Expressions/ModelContainer.cs delete mode 100644 test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/ModelContainerTest.cs diff --git a/src/Microsoft.AspNet.OData.Shared/Microsoft.AspNet.OData.Shared.projitems b/src/Microsoft.AspNet.OData.Shared/Microsoft.AspNet.OData.Shared.projitems index a5e0cf84f3..8ac32cf9ee 100644 --- a/src/Microsoft.AspNet.OData.Shared/Microsoft.AspNet.OData.Shared.projitems +++ b/src/Microsoft.AspNet.OData.Shared/Microsoft.AspNet.OData.Shared.projitems @@ -228,7 +228,6 @@ - diff --git a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/ModelContainer.cs b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/ModelContainer.cs deleted file mode 100644 index 6d90862071..0000000000 --- a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/ModelContainer.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. See License.txt in the project root for license information. - -using System; -using System.Collections.Concurrent; -using Microsoft.OData.Edm; - -namespace Microsoft.AspNet.OData.Query.Expressions -{ - /// - /// EntityFramework does not let you inject non primitive constant values (like IEdmModel) in Select queries. Primitives like strings and guids can be - /// injected as they can be translated into a SQL query. This container associates a unique string with each EDM model, so that, given the string the model - /// can be retrieved later. - /// - internal static class ModelContainer - { - private static ConcurrentDictionary _map = new ConcurrentDictionary(); - private static ConcurrentDictionary _reverseMap = new ConcurrentDictionary(); - - public static string GetModelID(IEdmModel model) - { - string index = _map.GetOrAdd(model, m => Guid.NewGuid().ToString()); - _reverseMap.TryAdd(index, model); - return index; - } - - public static IEdmModel GetModel(string id) - { - return _reverseMap[id]; - } - } -} diff --git a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandBinder.cs b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandBinder.cs index 457ba18339..29e7342a1d 100644 --- a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandBinder.cs +++ b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandBinder.cs @@ -29,7 +29,6 @@ internal class SelectExpandBinder private ODataQueryContext _context; private IEdmModel _model; private ODataQuerySettings _settings; - private string _modelID; public SelectExpandBinder(ODataQuerySettings settings, SelectExpandQueryOption selectExpandQuery) { @@ -42,7 +41,6 @@ public SelectExpandBinder(ODataQuerySettings settings, SelectExpandQueryOption s _selectExpandQuery = selectExpandQuery; _context = selectExpandQuery.Context; _model = _context.Model; - _modelID = ModelContainer.GetModelID(_model); _settings = settings; } @@ -313,10 +311,9 @@ private Expression ProjectElement(Expression source, SelectExpandClause selectEx bool isTypeNamePropertySet = false; bool isContainerPropertySet = false; - // Initialize property 'ModelID' on the wrapper class. - // source = new Wrapper { ModelID = 'some-guid-id' } - wrapperProperty = wrapperType.GetProperty("ModelID"); - wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, Expression.Constant(_modelID))); + // Initialize property 'Model' on the wrapper class + wrapperProperty = wrapperType.GetProperty("Model"); + wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, Expression.Constant(_model))); // Initialize property 'Instance' on the wrapper class // source => new Wrapper { Instance = element } diff --git a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandWrapper.cs b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandWrapper.cs index c0e0fdc338..a2a4dccc21 100644 --- a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandWrapper.cs +++ b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandWrapper.cs @@ -26,9 +26,9 @@ internal abstract class SelectExpandWrapper : IEdmEntityObject, ISelectExpandWra public PropertyContainer Container { get; set; } /// - /// An ID to uniquely identify the model in the . + /// Edm model corresponding to request. /// - public string ModelID { get; set; } + public IEdmModel Model { get; set; } /// public object UntypedInstance { get; set; } @@ -41,7 +41,7 @@ internal abstract class SelectExpandWrapper : IEdmEntityObject, ISelectExpandWra /// public IEdmTypeReference GetEdmType() { - IEdmModel model = GetModel(); + IEdmModel model = Model; Type elementType = GetElementType(); return model.GetEdmTypeReference(elementType); } @@ -64,7 +64,7 @@ public bool TryGetPropertyValue(string propertyName, out object value) if (UseInstanceForProperties && UntypedInstance != null) { _typedEdmEntityObject = _typedEdmEntityObject ?? - new TypedEdmEntityObject(UntypedInstance, GetEdmType() as IEdmEntityTypeReference, GetModel()); + new TypedEdmEntityObject(UntypedInstance, GetEdmType() as IEdmEntityTypeReference, Model); return _typedEdmEntityObject.TryGetPropertyValue(propertyName, out value); } @@ -88,7 +88,7 @@ public IDictionary ToDictionary(Func dictionary = new Dictionary(); IEdmStructuredType type = GetEdmType().AsStructured().StructuredDefinition(); - IPropertyMapper mapper = mapperProvider(GetModel(), type); + IPropertyMapper mapper = mapperProvider(Model, type); if (mapper == null) { throw Error.InvalidOperation(SRResources.InvalidPropertyMapper, typeof(IPropertyMapper).FullName, @@ -123,12 +123,5 @@ public IDictionary ToDictionary(Func - diff --git a/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/ModelContainerTest.cs b/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/ModelContainerTest.cs deleted file mode 100644 index 169fb482ef..0000000000 --- a/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/ModelContainerTest.cs +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. See License.txt in the project root for license information. - -using Microsoft.AspNet.OData.Query.Expressions; -using Microsoft.OData.Edm; -using Xunit; - -namespace Microsoft.AspNet.OData.Test.Query.Expressions -{ - public class ModelContainerTest - { - [Fact] - public void GetModelID_Returns_DifferentIDForDifferentModels() - { - EdmModel model1 = new EdmModel(); - EdmModel model2 = new EdmModel(); - - Assert.NotEqual(ModelContainer.GetModelID(model1), ModelContainer.GetModelID(model2)); - } - - [Fact] - public void GetModelID_Returns_SameIDForSameModel() - { - EdmModel model = new EdmModel(); - - Assert.Equal(ModelContainer.GetModelID(model), ModelContainer.GetModelID(model)); - } - - [Fact] - public void GetModelID_AndThen_GetModel_ReturnsOriginalModel() - { - EdmModel model = new EdmModel(); - string modelID = ModelContainer.GetModelID(model); - - Assert.Same(model, ModelContainer.GetModel(modelID)); - } - } -} diff --git a/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandBinderTest.cs b/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandBinderTest.cs index 01cbeb368a..4b27775180 100644 --- a/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandBinderTest.cs +++ b/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandBinderTest.cs @@ -299,8 +299,8 @@ public void ProjectAsWrapper_Element_ProjectedValueContainsModelID() // Assert SelectExpandWrapper customerWrapper = Expression.Lambda(projection).Compile().DynamicInvoke() as SelectExpandWrapper; - Assert.NotNull(customerWrapper.ModelID); - Assert.Same(_model.Model, ModelContainer.GetModel(customerWrapper.ModelID)); + Assert.NotNull(customerWrapper.Model); + Assert.Same(_model.Model, customerWrapper.Model); } [Theory] diff --git a/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandWrapperTest.cs b/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandWrapperTest.cs index 7aaca0ff8e..779736958c 100644 --- a/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandWrapperTest.cs +++ b/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandWrapperTest.cs @@ -16,12 +16,10 @@ namespace Microsoft.AspNet.OData.Test.Query.Expressions public class SelectExpandWrapperTest { private CustomersModelWithInheritance _model; - private string _modelID; public SelectExpandWrapperTest() { _model = new CustomersModelWithInheritance(); - _modelID = ModelContainer.GetModelID(_model.Model); } [Fact] @@ -45,7 +43,7 @@ public void GetEdmType_Returns_InstanceType() { _model.Model.SetAnnotationValue(_model.Customer, new ClrTypeAnnotation(typeof(TestEntity))); _model.Model.SetAnnotationValue(_model.SpecialCustomer, new ClrTypeAnnotation(typeof(DerivedEntity))); - SelectExpandWrapper wrapper = new SelectExpandWrapper { ModelID = _modelID }; + SelectExpandWrapper wrapper = new SelectExpandWrapper { Model = _model.Model }; wrapper.Instance = new DerivedEntity(); IEdmTypeReference edmType = wrapper.GetEdmType(); @@ -58,7 +56,7 @@ public void GetEdmType_Returns_ElementTypeIfInstanceIsNull() { _model.Model.SetAnnotationValue(_model.Customer, new ClrTypeAnnotation(typeof(TestEntity))); _model.Model.SetAnnotationValue(_model.SpecialCustomer, new ClrTypeAnnotation(typeof(DerivedEntity))); - SelectExpandWrapper wrapper = new SelectExpandWrapper { ModelID = _modelID }; + SelectExpandWrapper wrapper = new SelectExpandWrapper { Model = _model.Model }; IEdmTypeReference edmType = wrapper.GetEdmType(); @@ -72,10 +70,10 @@ public void TryGetValue_ReturnsValueFromPropertyContainer_IfPresent() MockPropertyContainer container = new MockPropertyContainer(); container.Properties.Add("SampleProperty", expectedPropertyValue); SelectExpandWrapper wrapper = new SelectExpandWrapper - { - ModelID = _modelID, - Container = container - }; + { + Model = _model.Model, + Container = container + }; wrapper.Instance = new TestEntity(); object value; @@ -91,10 +89,10 @@ public void TryGetValue_ReturnsValueFromInstance_IfNotPresentInContainer() object expectedPropertyValue = new object(); MockPropertyContainer container = new MockPropertyContainer(); SelectExpandWrapper wrapper = new SelectExpandWrapper - { - ModelID = _modelID, - Container = container - }; + { + Model = _model.Model, + Container = container + }; wrapper.Instance = new TestEntity { SampleProperty = expectedPropertyValue }; wrapper.UseInstanceForProperties = true; @@ -109,7 +107,7 @@ public void TryGetValue_ReturnsValueFromInstance_IfNotPresentInContainer() public void TryGetValue_ReturnsValueFromInstance_IfContainerIsNull() { object expectedPropertyValue = new object(); - SelectExpandWrapper wrapper = new SelectExpandWrapper { ModelID = _modelID }; + SelectExpandWrapper wrapper = new SelectExpandWrapper { Model = _model.Model }; wrapper.Instance = new TestEntity { SampleProperty = expectedPropertyValue }; wrapper.UseInstanceForProperties = true; @@ -129,7 +127,7 @@ public void TryGetValue_PropertyAliased_IfAnnotationSet() _model.CustomerName, new ClrPropertyInfoAnnotation(typeof(TestEntityWithAlias).GetProperty("SampleProperty"))); object expectedPropertyValue = new object(); - SelectExpandWrapper wrapper = new SelectExpandWrapper { ModelID = _modelID }; + SelectExpandWrapper wrapper = new SelectExpandWrapper { Model = _model.Model }; wrapper.Instance = new TestEntityWithAlias { SampleProperty = expectedPropertyValue }; wrapper.UseInstanceForProperties = true; @@ -145,7 +143,7 @@ public void TryGetValue_PropertyAliased_IfAnnotationSet() [Fact] public void TryGetValue_ReturnsFalse_IfContainerAndInstanceAreNull() { - SelectExpandWrapper wrapper = new SelectExpandWrapper { ModelID = _modelID }; + SelectExpandWrapper wrapper = new SelectExpandWrapper { Model = _model.Model }; object value; bool result = wrapper.TryGetPropertyValue("SampleProperty", out value); @@ -156,7 +154,7 @@ public void TryGetValue_ReturnsFalse_IfContainerAndInstanceAreNull() [Fact] public void TryGetValue_ReturnsFalse_IfPropertyNotPresentInElement() { - SelectExpandWrapper wrapper = new SelectExpandWrapper { ModelID = _modelID }; + SelectExpandWrapper wrapper = new SelectExpandWrapper { Model = _model.Model }; object value; bool result = wrapper.TryGetPropertyValue("SampleNotPresentProperty", out value); @@ -177,7 +175,7 @@ public void ToDictionary_ContainsAllStructuralProperties_IfInstanceIsNotNull() SelectExpandWrapper testWrapper = new SelectExpandWrapper { Instance = new TestEntity { SampleProperty = 42 }, - ModelID = ModelContainer.GetModelID(model), + Model = _model.Model, UseInstanceForProperties = true, }; @@ -202,7 +200,7 @@ public void ToDictionary_ContainsAllProperties_FromContainer() SelectExpandWrapper wrapper = new SelectExpandWrapper { Container = container, - ModelID = ModelContainer.GetModelID(model) + Model = _model.Model }; // Act @@ -237,7 +235,7 @@ public void ToDictionary_Throws_IfMapperProvider_ReturnsNullPropertyMapper() SelectExpandWrapper wrapper = new SelectExpandWrapper { Instance = new TestEntity { SampleProperty = 42 }, - ModelID = ModelContainer.GetModelID(model) + Model = model }; Func mapperProvider = @@ -266,7 +264,7 @@ public void ToDictionary_Throws_IfMappingIsNullOrEmpty_ForAGivenProperty(string SelectExpandWrapper testWrapper = new SelectExpandWrapper { Instance = new TestEntity { SampleProperty = 42 }, - ModelID = ModelContainer.GetModelID(model), + Model = _model.Model, UseInstanceForProperties = true, }; @@ -296,7 +294,7 @@ public void ToDictionary_AppliesMappingToAllProperties_IfInstanceIsNotNull() SelectExpandWrapper testWrapper = new SelectExpandWrapper { Instance = new TestEntity { SampleProperty = 42 }, - ModelID = ModelContainer.GetModelID(model), + Model = _model.Model, UseInstanceForProperties = true, }; From 24270713b16c4714940e0204320cddfed1f36e7c Mon Sep 17 00:00:00 2001 From: Alan Wong Date: Tue, 5 Jun 2018 12:57:45 -0700 Subject: [PATCH 2/3] Dummy commit to kick off tests --- build.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.ps1 b/build.ps1 index 19faf298ee..59b25a0f2e 100644 --- a/build.ps1 +++ b/build.ps1 @@ -31,7 +31,7 @@ if ($args.Count -eq 0) $TestType = 'Nightly' $Configuration = 'Release' } -elseif ($args[0] -match 'quick' -or ($args[0] -match '-q')) +elseif ($args[0] -match 'quick' -or ($args[0] -match '-q')) { $TestType = "Quick" } From 8c9087f55ee8cf2f0ae548ca320a0ec3983e93f5 Mon Sep 17 00:00:00 2001 From: Alan Wong Date: Tue, 5 Jun 2018 17:04:06 -0700 Subject: [PATCH 3/3] Dummy commit to kick off test --- build.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.ps1 b/build.ps1 index 59b25a0f2e..14fdf590df 100644 --- a/build.ps1 +++ b/build.ps1 @@ -43,7 +43,7 @@ elseif ($args[0] -match 'EnableSkipStrongName') { $TestType = "EnableSkipStrongName" } -else +else { Error("Unknown input ""$args"". It can be empty or ""quick|DisableSkipStrongName|EnableSkipStrongName"".") exit