Skip to content

Commit

Permalink
Improve return code
Browse files Browse the repository at this point in the history
  • Loading branch information
chinadragon0515 committed Jul 19, 2016
1 parent f8ddcbd commit a2173ff
Show file tree
Hide file tree
Showing 14 changed files with 309 additions and 55 deletions.
2 changes: 1 addition & 1 deletion RESTier.sln
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 14
VisualStudioVersion = 14.0.25123.0
VisualStudioVersion = 14.0.25420.1
MinimumVisualStudioVersion = 10.0.40219.1
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{D5E947EB-03CB-4D04-8937-FF2131BB1F04}"
EndProject
Expand Down
3 changes: 3 additions & 0 deletions src/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#GetEnumerableItemType(System.Type)")]
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#GetSelectExpandElementType(System.Type)")]
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#OfType(System.Linq.IQueryable,System.Type)")]
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#RemoveUnneededStatement(System.Linq.Expressions.MethodCallExpression)")]
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#RemoveSelectExpandStatement(System.Linq.Expressions.MethodCallExpression)")]
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#RemoveAppendWhereStatement(System.Linq.Expressions.Expression)")]
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#Select(System.Linq.IQueryable,System.Linq.Expressions.LambdaExpression)")]
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#SelectMany(System.Linq.IQueryable,System.Linq.Expressions.LambdaExpression,System.Type)")]
[assembly: SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Scope = "member", Target = "System.Linq.Expressions.ExpressionHelpers.#StripQueryMethod(System.Linq.Expressions.Expression,System.String)")]
Expand Down
9 changes: 9 additions & 0 deletions src/Microsoft.Restier.Core/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Microsoft.Restier.Core/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,7 @@
<data name="EdmTypeNotSupported" xml:space="preserve">
<value>{0} is not a supported EDM type.</value>
</data>
<data name="ResourceNotFound" xml:space="preserve">
<value>The request resource is not found.</value>
</data>
</root>
126 changes: 126 additions & 0 deletions src/Microsoft.Restier.Core/Query/DefaultQueryHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.OData.Edm;
using Microsoft.Restier.Core.Exceptions;
using Microsoft.Restier.Core.Properties;

namespace Microsoft.Restier.Core.Query
Expand All @@ -17,6 +19,10 @@ namespace Microsoft.Restier.Core.Query
/// </summary>
internal static class DefaultQueryHandler
{
private const string ExpressionMethodNameOfWhere = "Where";
private const string ExpressionMethodNameOfSelect = "Select";
private const string ExpressionMethodNameOfSelectMany = "SelectMany";

/// <summary>
/// Asynchronously executes the query flow.
/// </summary>
Expand Down Expand Up @@ -76,6 +82,9 @@ public static async Task<QueryResult> QueryAsync(
};
var task = method.Invoke(executor, parameters) as Task<QueryResult>;
result = await task;

await CheckSubExpressionResult(
context, cancellationToken, result.Results, visitor, executor, expression);
}
else
{
Expand All @@ -98,6 +107,123 @@ public static async Task<QueryResult> QueryAsync(
return result;
}

private static async Task CheckSubExpressionResult(
QueryContext context,
CancellationToken cancellationToken,
IEnumerable enumerableResult,
QueryExpressionVisitor visitor,
IQueryExecutor executor,
Expression expression)
{
if (enumerableResult.GetEnumerator().MoveNext())
{
// If there is some result, will not have additional processing
return;
}

var methodCallExpression = expression as MethodCallExpression;

// This will remove unneeded statement which includes $expand, $select,$top,$skip,$orderby
methodCallExpression = methodCallExpression.RemoveUnneededStatement();
if (methodCallExpression == null || methodCallExpression.Arguments.Count != 2)
{
return;
}

if (methodCallExpression.Method.Name == ExpressionMethodNameOfWhere)
{
// Throw exception if key as last where statement, or remove $filter where statement
methodCallExpression = CheckWhereCondition(methodCallExpression);
if (methodCallExpression == null || methodCallExpression.Arguments.Count != 2)
{
return;
}

// Call without $filter where statement and with Key where statement
if (methodCallExpression.Method.Name == ExpressionMethodNameOfWhere)
{
// The last where from $filter is removed and run with key where statement
await ExecuteSubExpression(context, cancellationToken, visitor, executor, methodCallExpression);
return;
}
}

if (methodCallExpression.Method.Name != ExpressionMethodNameOfSelect
&& methodCallExpression.Method.Name != ExpressionMethodNameOfSelectMany)
{
// If last statement is not select property, will no further checking
return;
}

var subExpression = methodCallExpression.Arguments[0];

// Remove appended statement like Where(Param_0 => (Param_0.Prop != null)) if there is one
subExpression = subExpression.RemoveAppendWhereStatement();

await ExecuteSubExpression(context, cancellationToken, visitor, executor, subExpression);
}

private static async Task ExecuteSubExpression(
QueryContext context,
CancellationToken cancellationToken,
QueryExpressionVisitor visitor,
IQueryExecutor executor,
Expression expression)
{
// get element type
Type elementType = null;
var queryType = expression.Type.FindGenericType(typeof(IQueryable<>));
if (queryType != null)
{
elementType = queryType.GetGenericArguments()[0];
}

var query = visitor.BaseQuery.Provider.CreateQuery(expression);
var method = typeof(IQueryExecutor)
.GetMethod("ExecuteQueryAsync")
.MakeGenericMethod(elementType);
var parameters = new object[]
{
context, query, cancellationToken
};
var task = method.Invoke(executor, parameters) as Task<QueryResult>;
var result = await task;

var any = result.Results.Cast<object>().Any();
if (!any)
{
// Which means previous expression does not have result, and should throw ResourceNotFoundException.
throw new ResourceNotFoundException(Resources.ResourceNotFound);
}
}

private static MethodCallExpression CheckWhereCondition(MethodCallExpression methodCallExpression)
{
// This means a select for expand is appended, will remove it for resource existing check
var lastWhere = methodCallExpression.Arguments[1] as UnaryExpression;
var lambdaExpression = lastWhere.Operand as LambdaExpression;
if (lambdaExpression == null)
{
return null;
}

var binaryExpression = lambdaExpression.Body as BinaryExpression;
if (binaryExpression == null)
{
return null;
}

// Key segment will have ConstantExpression but $filter will not have ConstantExpression
var rightExpression = binaryExpression.Right as ConstantExpression;
if (rightExpression != null)
{
// This means where statement is key segment but not for $filter
throw new ResourceNotFoundException(Resources.ResourceNotFound);
}

return methodCallExpression.Arguments[0] as MethodCallExpression;
}

private class QueryExpressionVisitor : ExpressionVisitor
{
private readonly QueryExpressionContext context;
Expand Down
10 changes: 1 addition & 9 deletions src/Microsoft.Restier.Publishers.OData/RestierController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,6 @@ private HttpResponseMessage CreateQueryResponse(
IQueryable query, IEdmType edmType, bool isIfNoneMatch, ETag etag)
{
IEdmTypeReference typeReference = GetTypeReference(edmType);

// TODO, GitHubIssue#328 : 404 should be returned when requesting property of non-exist entity
BaseSingleResult singleResult = null;
HttpResponseMessage response = null;

Expand Down Expand Up @@ -505,13 +503,7 @@ private HttpResponseMessage CreateQueryResponse(
var entityResult = query.SingleOrDefault();
if (entityResult == null)
{
// TODO GitHubIssue#288: 204 expected when requesting single nav property which has null value
// ~/People(nonexistkey) and ~/People(nonexistkey)/BestFriend, expected 404
// ~/People(key)/BestFriend, and BestFriend is null, expected 204
throw new HttpResponseException(
this.Request.CreateErrorResponse(
HttpStatusCode.NotFound,
Resources.ResourceNotFound));
return this.Request.CreateResponse(HttpStatusCode.NoContent);
}

// Check the ETag here
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Restier/Microsoft.Restier.nuspec
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<package >
<metadata>
<id>Microsoft.Restier</id>
<version>0.6.0-beta</version>
<version>0.6.0</version>
<title>RESTier</title>
<authors>Microsoft</authors>
<owners>Microsoft</owners>
Expand Down
109 changes: 109 additions & 0 deletions src/Shared/ExpressionHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ internal static class ExpressionHelpers
private const string MethodNameOfQueryTake = "Take";
private const string MethodNameOfQuerySelect = "Select";
private const string MethodNameOfQuerySkip = "Skip";
private const string MethodNameOfQueryWhere = "Where";
private const string MethodNameOfQueryOrderBy = "OrderBy";
private const string InterfaceNameISelectExpandWrapper = "ISelectExpandWrapper";
private const string ExpandClauseReflectedTypeName = "SelectExpandBinder";

public static IQueryable Select(IQueryable query, LambdaExpression select)
Expand Down Expand Up @@ -116,6 +119,112 @@ internal static Type GetEnumerableItemType(this Type enumerableType)
return enumerableType;
}

internal static MethodCallExpression RemoveUnneededStatement(this MethodCallExpression methodCallExpression)
{
if (methodCallExpression == null || methodCallExpression.Arguments.Count != 2)
{
return methodCallExpression;
}

if (methodCallExpression.Method.Name == MethodNameOfQuerySelect)
{
// Check where it is expand case or select, if yes, need to get rid of last select
methodCallExpression = RemoveSelectExpandStatement(methodCallExpression);
if (methodCallExpression == null || methodCallExpression.Arguments.Count != 2)
{
return methodCallExpression;
}
}

if (methodCallExpression.Method.Name == MethodNameOfQueryTake)
{
// Check where it is top query option, and if yes, remove it.
methodCallExpression = methodCallExpression.Arguments[0] as MethodCallExpression;
if (methodCallExpression == null || methodCallExpression.Arguments.Count != 2)
{
return methodCallExpression;
}
}

if (methodCallExpression.Method.Name == MethodNameOfQuerySkip)
{
// Check where it is skip query option, and if yes, remove it.
methodCallExpression = methodCallExpression.Arguments[0] as MethodCallExpression;
if (methodCallExpression == null || methodCallExpression.Arguments.Count != 2)
{
return methodCallExpression;
}
}

if (methodCallExpression.Method.Name == MethodNameOfQueryOrderBy)
{
// Check where it is orderby query option, and if yes, remove it.
methodCallExpression = methodCallExpression.Arguments[0] as MethodCallExpression;
if (methodCallExpression == null || methodCallExpression.Arguments.Count != 2)
{
return methodCallExpression;
}
}

return methodCallExpression;
}

internal static MethodCallExpression RemoveSelectExpandStatement(this MethodCallExpression methodCallExpression)
{
// This means a select for expand is appended, will remove it for resource existing check
var expandSelect = methodCallExpression.Arguments[1] as UnaryExpression;
var lambdaExpression = expandSelect.Operand as LambdaExpression;
if (lambdaExpression == null)
{
return methodCallExpression;
}

var memberInitExpression = lambdaExpression.Body as MemberInitExpression;
if (memberInitExpression == null)
{
return methodCallExpression;
}

Type returnType = lambdaExpression.ReturnType;
var wrapperInterface = returnType.GetInterface(InterfaceNameISelectExpandWrapper);
if (wrapperInterface != null)
{
methodCallExpression = methodCallExpression.Arguments[0] as MethodCallExpression;
}

return methodCallExpression;
}

internal static Expression RemoveAppendWhereStatement(this Expression expression)
{
var methodCallExpression = expression as MethodCallExpression;
if (methodCallExpression == null || methodCallExpression.Method.Name != MethodNameOfQueryWhere)
{
return expression;
}

// This means there may be an appended statement Where(Param_0 => (Param_0.Prop != null))
var appendedWhere = methodCallExpression.Arguments[1] as UnaryExpression;
var lambdaExpression = appendedWhere.Operand as LambdaExpression;
if (lambdaExpression == null)
{
return expression;
}

var binaryExpression = lambdaExpression.Body as BinaryExpression;
if (binaryExpression != null && binaryExpression.NodeType == ExpressionType.NotEqual)
{
var rightExpression = binaryExpression.Right as ConstantExpression;
if (rightExpression != null && rightExpression.Value == null)
{
// remove statement like Where(Param_0 => (Param_0.Prop != null))
expression = methodCallExpression.Arguments[0];
}
}

return expression;
}

private static Expression StripQueryMethod(Expression expression, string methodName)
{
var methodCall = expression as MethodCallExpression;
Expand Down
Loading

0 comments on commit a2173ff

Please sign in to comment.