Skip to content

Commit

Permalink
fix MapsterMapper#272 IEnumerable is enumerating twice when adapted t…
Browse files Browse the repository at this point in the history
…o array
  • Loading branch information
chaowlert committed Oct 22, 2020
1 parent 5c9f5aa commit 4b5b49e
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 22 deletions.
49 changes: 49 additions & 0 deletions src/Mapster.Tests/WhenMappingCollections.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Shouldly;
using Assert = Microsoft.VisualStudio.TestTools.UnitTesting.Assert;

namespace Mapster.Tests
Expand Down Expand Up @@ -213,5 +214,53 @@ public void ShouldNotUsingTheSameEnumerable()
Assert.IsFalse(src.Ints == dest.Ints);
Assert.IsFalse(src.IntArray == dest.IntArray);
}

[TestMethod]
public void MapToArrayEnumerateOnlyOnce()
{
var i = 0;

IEnumerable<int> GetInts()
{
i++;
yield return 1;
yield return 2;
}

GetInts().Adapt<int[]>();
i.ShouldBe(1);
}

[TestMethod]
public void MapToListEnumerateOnlyOnce()
{
var i = 0;

IEnumerable<int> GetInts()
{
i++;
yield return 1;
yield return 2;
}

GetInts().Adapt<List<int>>();
i.ShouldBe(1);
}

[TestMethod]
public void MapToMultiRanksArrayEnumerateOnlyOnce()
{
var i = 0;

IEnumerable<int> GetInts()
{
i++;
yield return 1;
yield return 2;
}

GetInts().Adapt<int[,]>();
i.ShouldBe(1);
}
}
}
27 changes: 27 additions & 0 deletions src/Mapster.Tool/Properties/launchSettings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"iisSettings": {
"windowsAuthentication": false,
"anonymousAuthentication": true,
"iisExpress": {
"applicationUrl": "http:https://localhost:49597/",
"sslPort": 44315
}
},
"profiles": {
"IIS Express": {
"commandName": "IISExpress",
"launchBrowser": true,
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development"
}
},
"Mapster.Tool": {
"commandName": "Project",
"launchBrowser": true,
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development"
},
"applicationUrl": "https://localhost:5001;http:https://localhost:5000"
}
}
}
6 changes: 3 additions & 3 deletions src/Mapster/Adapters/ArrayAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ protected override bool CanMap(PreCompileArgument arg)

protected override bool CanInline(Expression source, Expression? destination, CompileArgument arg)
{
return arg.MapType == MapType.Projection;
return arg.MapType == MapType.Projection || ExpressionEx.CreateCountExpression(source) == null;
}

protected override Expression CreateInstantiationExpression(Expression source, Expression? destination, CompileArgument arg)
{
var destinationElementType = arg.DestinationType.ExtractCollectionType();
return Expression.NewArrayBounds(
destinationElementType,
ExpressionEx.CreateCountExpression(source, true)); //new TDestinationElement[count]
ExpressionEx.CreateCountExpression(source)); //new TDestinationElement[count]
}

protected override Expression CreateBlockExpression(Expression source, Expression destination, CompileArgument arg)
Expand All @@ -39,7 +39,7 @@ protected override Expression CreateBlockExpression(Expression source, Expressio
{
//Array.Copy(src, 0, dest, 0, src.Length)
var method = typeof(Array).GetMethod("Copy", new[] { typeof(Array), typeof(int), typeof(Array), typeof(int), typeof(int) });
return Expression.Call(method, source, Expression.Constant(0), destination, Expression.Constant(0), ExpressionEx.CreateCountExpression(source, true));
return Expression.Call(method, source, Expression.Constant(0), destination, Expression.Constant(0), ExpressionEx.CreateCountExpression(source));
}

return CreateArraySet(source, destination, arg);
Expand Down
22 changes: 19 additions & 3 deletions src/Mapster/Adapters/BaseAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ protected virtual Expression CreateExpressionBody(Expression source, Expression?
}
}

protected virtual Expression TransformSource(Expression source)
{
return source;
}

protected Expression CreateBlockExpressionBody(Expression source, Expression? destination, CompileArgument arg)
{
if (arg.MapType == MapType.Projection)
Expand Down Expand Up @@ -181,7 +186,15 @@ protected Expression CreateBlockExpressionBody(Expression source, Expression? de
}

//new TDest();
var set = CreateInstantiationExpression(source, destination, arg);
Expression transformedSource = source;
var transform = TransformSource(source);
if (transform != source)
{
var src = Expression.Variable(transform.Type);
vars.Add(src);
transformedSource = src;
}
var set = CreateInstantiationExpression(transformedSource, destination, arg);
if (destination != null && this.UseTargetValue && arg.GetConstructUsing()?.Parameters.Count != 2 && destination.CanBeNull())
{
//dest ?? new TDest();
Expand Down Expand Up @@ -209,14 +222,17 @@ protected Expression CreateBlockExpressionBody(Expression source, Expression? de
//var result = new TDest();
var result = Expression.Variable(arg.DestinationType, "result");
var assign = Expression.Assign(result, set);
var assignActions = new List<Expression> {assign};
var assignActions = new List<Expression>();
if (transform != source)
assignActions.Add(Expression.Assign(transformedSource, transform));
assignActions.Add(assign);

//before(source, result);
var beforeMappings = arg.Settings.BeforeMappingFactories.Select(it => InvokeMapping(it, source, result, arg, true));
assignActions.AddRange(beforeMappings);

//result.prop = adapt(source.prop);
var mapping = CreateBlockExpression(source, result, arg);
var mapping = CreateBlockExpression(transformedSource, result, arg);
var settingActions = new List<Expression> {mapping};

//after(source, result);
Expand Down
2 changes: 1 addition & 1 deletion src/Mapster/Adapters/CollectionAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected override Expression CreateInstantiationExpression(Expression source, E
listType = typeof(HashSet<>).MakeGenericType(destinationElementType);
}
}
var count = ExpressionEx.CreateCountExpression(source, false);
var count = ExpressionEx.CreateCountExpression(source);
if (count == null)
return Expression.New(listType); //new List<T>()
var ctor = (from c in listType.GetConstructors()
Expand Down
23 changes: 21 additions & 2 deletions src/Mapster/Adapters/MultiDimensionalArrayAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,25 @@ protected override bool CanInline(Expression source, Expression? destination, Co
return false;
}

protected override Expression TransformSource(Expression source)
{
if (ExpressionEx.CreateCountExpression(source) != null)
return source;
var transformed = source;
var elemType = source.Type.ExtractCollectionType();
var type = typeof(IEnumerable<>).MakeGenericType(elemType);
if (!type.IsAssignableFrom(source.Type))
{
var cast = typeof(Enumerable).GetMethod(nameof(Enumerable.Cast))!
.MakeGenericMethod(elemType);
transformed = Expression.Call(cast, transformed);
}

var toList = typeof(Enumerable).GetMethod(nameof(Enumerable.ToList))!
.MakeGenericMethod(elemType);
return Expression.Call(toList, transformed);
}

protected override Expression CreateInstantiationExpression(Expression source, Expression? destination, CompileArgument arg)
{
return Expression.NewArrayBounds(arg.DestinationType.GetElementType(), GetArrayBounds(source, arg.DestinationType));
Expand All @@ -38,7 +57,7 @@ private static IEnumerable<Expression> GetArrayBounds(Expression source, Type de
{
yield return Expression.Constant(1);
}
yield return ExpressionEx.CreateCountExpression(source, true)!;
yield return ExpressionEx.CreateCountExpression(source)!;
}
else
{
Expand All @@ -64,7 +83,7 @@ protected override Expression CreateBlockExpression(Expression source, Expressio
{
//Array.Copy(src, 0, dest, 0, src.Length)
var method = typeof(Array).GetMethod("Copy", new[] { typeof(Array), typeof(int), typeof(Array), typeof(int), typeof(int) });
return Expression.Call(method, source, Expression.Constant(0), destination, Expression.Constant(0), ExpressionEx.CreateCountExpression(source, true));
return Expression.Call(method, source, Expression.Constant(0), destination, Expression.Constant(0), ExpressionEx.CreateCountExpression(source));
}

return CreateArraySet(source, destination, arg);
Expand Down
20 changes: 7 additions & 13 deletions src/Mapster/Utils/ExpressionEx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,26 +131,20 @@ public static Delegate Compile(this LambdaExpression exp, CompileArgument arg)
}
}

public static Expression? CreateCountExpression(Expression source, bool allowCountAll)
public static Expression? CreateCountExpression(Expression source)
{
if (source.Type.IsArray)
{
if (source.Type.GetArrayRank() == 1)
return Expression.ArrayLength(source); //array.Length
else
return Expression.Property(source, "Length");
return source.Type.GetArrayRank() == 1
? (Expression?) Expression.ArrayLength(source)
: Expression.Property(source, "Length");
}
else
{
var countProperty = GetCount(source.Type);
if (countProperty != null)
return Expression.Property(source, countProperty); //list.Count
if (!allowCountAll)
return null;
var countMethod = typeof(Enumerable).GetMethods()
.First(m => m.Name == nameof(Enumerable.Count) && m.GetParameters().Length == 1)
.MakeGenericMethod(source.Type.ExtractCollectionType());
return Expression.Call(countMethod, source); //list.Count()
return countProperty != null
? Expression.Property(source, countProperty)
: null;
}
}

Expand Down

0 comments on commit 4b5b49e

Please sign in to comment.