Skip to content

Commit

Permalink
Fix ArgumentException in ProjectPlanCompiler caused by missing materi…
Browse files Browse the repository at this point in the history
…alizerContext argument in dynamic calls (#2535)

* Add failing test

* Fix issue

* Use nameof instead of literals for dynamic method names

* Refactoring

* Use base.MaterializerCache consistently
  • Loading branch information
habbes authored Oct 24, 2022
1 parent ed9810a commit 1eadadf
Show file tree
Hide file tree
Showing 18 changed files with 90 additions and 54 deletions.
3 changes: 2 additions & 1 deletion src/Microsoft.OData.Client/AtomMaterializerLog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ internal class AtomMaterializerLog
/// <summary>
/// The materializer context.
/// </summary>
private IODataMaterializerContext materializerContext;
private readonly IODataMaterializerContext materializerContext;

#endregion Private fields

Expand All @@ -69,6 +69,7 @@ internal AtomMaterializerLog(MergeOption mergeOption, ClientEdmModel model, Enti
{
Debug.Assert(model != null, "model != null");
Debug.Assert(entityTracker != null, "entityTracker != null");
Debug.Assert(materializerContext != null, "materializerContext != null");

this.appendOnlyEntries = new Dictionary<Uri, ODataResource>(EqualityComparer<Uri>.Default);
this.mergeOption = mergeOption;
Expand Down
10 changes: 5 additions & 5 deletions src/Microsoft.OData.Client/BaseAsyncResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ internal abstract class BaseAsyncResult : IAsyncResult
/// <summary>wrapped request</summary>
protected PerRequest perRequest;

/// <summary>
/// Cache used to store temporary metadata used for materialization of OData items.
/// </summary>
protected MaterializerCache materializerCache = new MaterializerCache();

/// <summary>
/// The int equivalent for true.
/// </summary>
Expand Down Expand Up @@ -94,6 +89,11 @@ internal BaseAsyncResult(object source, string method, AsyncCallback callback, o
this.userState = state;
}

/// <summary>
/// Cache used to store temporary metadata used for materialization of OData items.
/// </summary>
protected MaterializerCache MaterializerCache { get; } = new MaterializerCache();

/// <summary>
/// This delegate exists to workaround limitations in the WP7 runtime.
/// When limitations on the number of parameters to Func&lt;&gt; are resolved, this can be subsumed by the following:
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.OData.Client/BatchSaveResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ protected override MaterializeAtom GetMaterializer(EntityDescriptor entityDescri
/*projectionPlan*/ null,
this.currentOperationResponse.CreateResponseMessage(),
ODataPayloadKind.Resource,
this.materializerCache);
base.MaterializerCache);
}

/// <summary>
Expand Down Expand Up @@ -690,7 +690,7 @@ private IEnumerable<OperationResponse> HandleBatchResponse(ODataBatchReader batc
this.currentOperationResponse.Headers.GetHeader(XmlConstants.HttpContentType),
this.currentOperationResponse.CreateResponseMessage(),
query.PayloadKind,
this.materializerCache);
base.MaterializerCache);
qresponse = QueryOperationResponse.GetInstance(query.ElementType, this.currentOperationResponse.Headers, query, materializer);
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/Microsoft.OData.Client/GroupByProjectionPlanCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,12 @@ private Expression CallValueForPath(Expression entry, Expression entryType, Proj
Debug.Assert(entry != null, "entry != null");
Debug.Assert(path != null, "path != null");

Expression result = CallMaterializer("ProjectionValueForPath", this.materializerExpression, entry, entryType, Expression.Constant(path, typeof(object)));
Expression result = CallMaterializer(
nameof(ODataEntityMaterializerInvoker.ProjectionValueForPath),
this.materializerExpression,
entry,
entryType,
Expression.Constant(path, typeof(object)));
this.annotations.Add(result, new ExpressionAnnotation() { Segment = path[path.Count - 1] });
return result;
}
Expand Down Expand Up @@ -523,7 +528,7 @@ private Expression RebindMethodCallForAggregationMethod(MethodCallExpression met
annotation.Segment.StartPath.Add(memberSegment);

Expression value = CallMaterializer(
"ProjectionDynamicValueForPath",
nameof(ODataEntityMaterializerInvoker.ProjectionDynamicValueForPath),
this.materializerExpression,
annotation.Segment.StartPath.RootEntry,
Expression.Constant(targetType, typeof(Type)),
Expand Down
32 changes: 22 additions & 10 deletions src/Microsoft.OData.Client/ProjectionPlanCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ internal class ProjectionPlanCompiler : ALinqExpressionVisitor
/// <param name="materializerContext">The materializer context.</param>
private ProjectionPlanCompiler(Dictionary<Expression, Expression> normalizerRewrites, IODataMaterializerContext materializerContext)
{
Debug.Assert(materializerContext != null, "materializerContext != null");

this.annotations = new Dictionary<Expression, ExpressionAnnotation>(ReferenceEqualityComparer<Expression>.Instance);
this.materializerExpression = Expression.Parameter(typeof(object), "mat");
this.normalizerRewrites = normalizerRewrites;
Expand Down Expand Up @@ -504,7 +506,12 @@ private static Expression RebindConstructor(ConstructorInfo info, params Express
/// <returns>A new expression with the call instance.</returns>
private Expression CallCheckValueForPathIsNull(Expression entry, Expression entryType, ProjectionPath path)
{
Expression result = CallMaterializer("ProjectionCheckValueForPathIsNull", entry, entryType, Expression.Constant(path, typeof(object)));
Expression result = CallMaterializer(
nameof(ODataEntityMaterializerInvoker.ProjectionCheckValueForPathIsNull),
entry,
entryType,
Expression.Constant(path, typeof(object)),
Expression.Constant(this.materializerContext));
this.annotations.Add(result, new ExpressionAnnotation() { Segment = path[path.Count - 1] });
return result;
}
Expand All @@ -519,7 +526,12 @@ private Expression CallValueForPath(Expression entry, Expression entryType, Proj
Debug.Assert(entry != null, "entry != null");
Debug.Assert(path != null, "path != null");

Expression result = CallMaterializer("ProjectionValueForPath", this.materializerExpression, entry, entryType, Expression.Constant(path, typeof(object)));
Expression result = CallMaterializer(
nameof(ODataEntityMaterializerInvoker.ProjectionValueForPath),
this.materializerExpression,
entry,
entryType,
Expression.Constant(path, typeof(object)));
this.annotations.Add(result, new ExpressionAnnotation() { Segment = path[path.Count - 1] });
return result;
}
Expand Down Expand Up @@ -671,7 +683,7 @@ private Expression RebindEntityMemberInit(MemberInitExpression init)
assignment.Expression.NodeType == ExpressionType.MemberInit))
{
Expression nestedEntry = CallMaterializer(
"ProjectionGetEntry",
nameof(ODataEntityMaterializerInvoker.ProjectionGetEntry),
entryParameterAtMemberInit,
Expression.Constant(assignment.Member.Name, typeof(string)),
Expression.Constant(this.materializerContext));
Expand Down Expand Up @@ -771,7 +783,7 @@ private Expression RebindEntityMemberInit(MemberInitExpression init)
}

Expression reboundExpression = CallMaterializer(
"ProjectionInitializeEntity",
nameof(ODataEntityMaterializerInvoker.ProjectionInitializeEntity),
this.materializerExpression,
entryToInitValue,
expectedParamValue,
Expand All @@ -797,7 +809,7 @@ private Expression GetDeepestEntry(Expression[] path)
do
{
result = CallMaterializer(
"ProjectionGetEntry",
nameof(ODataEntityMaterializerInvoker.ProjectionGetEntry),
result ?? this.pathBuilder.ParameterEntryInScope,
Expression.Constant(((MemberExpression)path[pathIndex]).Member.Name, typeof(string)));
pathIndex++;
Expand Down Expand Up @@ -1051,7 +1063,7 @@ private Expression RebindMethodCallForMemberSelect(MethodCallExpression call)
// helpers, eg to preserve paging information.
Type returnElementType = call.Method.ReturnType.GetGenericArguments()[0];
result = CallMaterializer(
"ProjectionSelect",
nameof(ODataEntityMaterializerInvoker.ProjectionSelect),
this.materializerExpression,
this.pathBuilder.ParameterEntryInScope,
this.pathBuilder.ExpectedParamTypeInScope,
Expand All @@ -1060,7 +1072,7 @@ private Expression RebindMethodCallForMemberSelect(MethodCallExpression call)
selectorExpression);
this.annotations.Add(result, annotation);
result = CallMaterializerWithType(
"EnumerateAsElementType",
nameof(ODataEntityMaterializerInvoker.EnumerateAsElementType),
new Type[] { returnElementType },
result);
this.annotations.Add(result, annotation);
Expand Down Expand Up @@ -1150,7 +1162,7 @@ private Expression RebindMethodCallForNewSequence(MethodCallExpression call)
// {(mat, entry1, type1) => Convert(ProjectionValueForPath(mat, entry1, type1, p->*.FirstName))}
Type returnElementType = call.Method.ReturnType.GetGenericArguments()[0];
result = CallMaterializer(
"ProjectionSelect",
nameof(ODataEntityMaterializerInvoker.ProjectionSelect),
this.materializerExpression,
this.pathBuilder.ParameterEntryInScope,
this.pathBuilder.ExpectedParamTypeInScope,
Expand All @@ -1159,7 +1171,7 @@ private Expression RebindMethodCallForNewSequence(MethodCallExpression call)
selectorExpression);
this.annotations.Add(result, annotation);
result = CallMaterializerWithType(
"EnumerateAsElementType",
nameof(ODataEntityMaterializerInvoker.EnumerateAsElementType),
new Type[] { returnElementType },
result);
this.annotations.Add(result, annotation);
Expand Down Expand Up @@ -1209,7 +1221,7 @@ private Expression TypedEnumerableToList(Expression source, Type targetType)

// Return the annotated expression.
Expression result = CallMaterializerWithType(
"ListAsElementType",
nameof(ODataEntityMaterializer.ListAsElementType),
new Type[] { enumeratedType, listElementType },
this.materializerExpression,
source);
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OData.Client/QueryResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ private MaterializeAtom CreateMaterializer(ProjectionPlan plan, ODataPayloadKind
this.ContentType,
responseMessageWrapper,
payloadKind,
this.materializerCache);
base.MaterializerCache);
}
}
}
4 changes: 2 additions & 2 deletions src/Microsoft.OData.Client/SaveResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ protected override MaterializeAtom GetMaterializer(EntityDescriptor entityDescri
{
Debug.Assert(this.cachedResponse.Exception == null && this.cachedResponse.MaterializerEntry != null, "this.cachedResponse.Exception == null && this.cachedResponse.Entry != null");
ODataResource entry = this.cachedResponse.MaterializerEntry == null ? null : this.cachedResponse.MaterializerEntry.Entry;
return new MaterializeAtom(responseInfo, new[] { entry }, entityDescriptor.Entity.GetType(), this.cachedResponse.MaterializerEntry.Format, this.materializerCache);
return new MaterializeAtom(responseInfo, new[] { entry }, entityDescriptor.Entity.GetType(), this.cachedResponse.MaterializerEntry.Format, base.MaterializerCache);
}

/// <summary>
Expand Down Expand Up @@ -865,7 +865,7 @@ private void HandleOperationResponseData(IODataResponseMessage responseMsg, Stre
responseMsg.StatusCode,
() => responseStream);

ODataMaterializerContext materializerContext = new ODataMaterializerContext(responseInfo, this.materializerCache);
ODataMaterializerContext materializerContext = new ODataMaterializerContext(responseInfo, base.MaterializerCache);
entry = ODataReaderEntityMaterializer.ParseSingleEntityPayload(responseMessageWrapper, responseInfo, entityDescriptor.Entity.GetType(), materializerContext);
entityDescriptor.TransientEntityDescriptor = entry.EntityDescriptor;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace Microsoft.Test.OData.Tests.Client.AsynchronousTests
using System.Linq;
using System.Net;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.OData.Client;
using Microsoft.Test.OData.Services.TestServices;
using Microsoft.Test.OData.Services.TestServices.AstoriaDefaultServiceReference;
Expand Down Expand Up @@ -788,6 +789,24 @@ public void Linq_ProjectPropertiesFromEntityandExpandedEntity()
this.EnqueueTestComplete();
}

[Fact]
public async Task Linq_ProjectPropertiesFromEntityWithConditionalNullCheckOnExpandedEntity()
{
var context = this.CreateWrappedContext<DefaultContainer>().Context;
var query = context.Computer.Where(c => c.ComputerId == -10)
.Select(c => new Computer
{
ComputerId = c.ComputerId,
ComputerDetail = c.ComputerDetail == null ? null : c.ComputerDetail,
}) as DataServiceQuery<Computer>;

var result = await query.ExecuteAsync();

var computer = result.First();
Assert.Equal(-10, computer.ComputerId);
Assert.Equal(-10, computer.ComputerDetail.ComputerDetailId);
}

/// <summary>
/// LINQ query Project Name Stream Property
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void AddingCollectionToComplexCollectionShouldFail()
[Fact]
public void DataServicCollectionOfTAsCollectionTypeShouldFailForPrimitiveOrComplexCollections()
{
var testContext = new TestMaterializerContext(new MaterializerCache());
var testContext = new TestMaterializerContext();
var edmType = testContext.Model.GetOrCreateEdmType(typeof(MyInfo));
var clientTypeAnnotation = new ClientTypeAnnotation(edmType, typeof(MyInfo), "MyInfo", testContext.Model);

Expand All @@ -141,7 +141,7 @@ public void DataServicCollectionOfTAsCollectionTypeShouldFailForPrimitiveOrCompl
[Fact]
public void CreateCollectionInstanceShouldFailOnTypeWithNoParametersLessConstructors()
{
var testContext = new TestMaterializerContext(new MaterializerCache());
var testContext = new TestMaterializerContext();
var edmType = testContext.Model.GetOrCreateEdmType(typeof(ListWithNoEmptyConstructors));
var clientTypeAnnotation = new ClientTypeAnnotation(edmType, typeof(ListWithNoEmptyConstructors), "Points", testContext.Model);

Expand All @@ -153,7 +153,7 @@ public void CreateCollectionInstanceShouldFailOnTypeWithNoParametersLessConstruc
public void CreateCollectionPropertyInstanceShouldFailOnTypeWithNoParametersLessConstructors()
{
var odataProperty = new ODataProperty() { Name = "foo", Value = new ODataCollectionValue() { TypeName = "Points" } };
var testContext = new TestMaterializerContext(new MaterializerCache());
var testContext = new TestMaterializerContext();
testContext.ResolveTypeForMaterializationOverrideFunc = (Type type, string name) =>
{
var edmType = testContext.Model.GetOrCreateEdmType(typeof(ListWithNoEmptyConstructors));
Expand All @@ -167,7 +167,7 @@ public void CreateCollectionPropertyInstanceShouldFailOnTypeWithNoParametersLess
[Fact]
public void NonMissingMethodExceptionOnCreateInstanceShouldNotBeCaught()
{
var testContext = new TestMaterializerContext(new MaterializerCache());
var testContext = new TestMaterializerContext();
var edmType = testContext.Model.GetOrCreateEdmType(typeof(ListWithNoEmptyConstructors));
var clientTypeAnnotation = new ClientTypeAnnotation(edmType, typeof(ErrorThrowingList), "Points", testContext.Model);

Expand All @@ -181,7 +181,7 @@ public void NonMissingMethodExceptionOnCreateInstanceShouldNotBeCaught()

internal CollectionValueMaterializationPolicy CreateCollectionValueMaterializationPolicy()
{
return CreateCollectionValueMaterializationPolicy(new TestMaterializerContext(new MaterializerCache()));
return CreateCollectionValueMaterializationPolicy(new TestMaterializerContext());
}

internal CollectionValueMaterializationPolicy CreateCollectionValueMaterializationPolicy(IODataMaterializerContext materializerContext)
Expand Down
Loading

0 comments on commit 1eadadf

Please sign in to comment.