Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MapDynamic with array property always results in a single-member array #57

Open
FilipFilipov opened this issue Jul 5, 2017 · 7 comments

Comments

@FilipFilipov
Copy link

You can reproduce this easily in your MapDynamicTests if you change the Customer's Order type from ICollection<Order> to Order[] and then try to map a customer with more than one Order. The resulting array will always contain only the final Order.

Looking through the code I think this is caused by the MapCollection method, which for arrays does

var arrayList = new ArrayList { instanceToAddToCollectionInstance };

or in the case where isNewlyCreatedInstance is false

var arrayList = new ArrayList((ICollection)instance);

In both cases shouldn't that instead be

var arrayList = new ArrayList((ICollection)instance) { instanceToAddToCollectionInstance };
@Flaniga3
Copy link

@FilipFilipov Not sure if you're still struggling with this, but I ran into this same issue and no one anywhere could help. Finally I came up with this somewhat brutal solution in this stack overflow question: https://stackoverflow.com/questions/47239739/slapper-only-maps-one-object-when-selecting-multiple-objects-from-database. Check my first edit.

@FilipFilipov
Copy link
Author

Oh, I just merged Slapper into my codebase and did the fix I suggested above myself. I'd make a pull request to upstream it, but this project is obviously not maintained anymore.

@Flaniga3
Copy link

@FilipFilipov I might consider taking that route myself. Thanks for posting your solution.

@odelvalle
Copy link
Member

Hi @FilipFilipov I changed ICollection to array in Order, and this test pass without change anything in MapCollection:

[Test]
public void Can_Handle_Mapping_A_Single_Dynamic_Object_With_Multiple_Childs()
{
	// Arrange
	dynamic dynamicCustomer = new ExpandoObject();
	dynamicCustomer.Id = 1;
	dynamicCustomer.FirstName = "Bob";
	dynamicCustomer.LastName = "Smith";
	dynamicCustomer.Orders = new[]
	{
		new Order {Id = 1, OrderTotal = 50.50m},
		new Order {Id = 2, OrderTotal = 60.50m},
	};

	// Act
	var customer = Slapper.AutoMapper.MapDynamic<Customer>(dynamicCustomer) as Customer;

	// Assert
	Assert.NotNull(customer);
	Assert.That(customer.Orders.Count() == 2);
}

Can you send me any test that fail?
Thanks

@Flaniga3
Copy link

@odelvalle The StackOverflow link I posted outlines a very specific example of the mapper returning a single member IEnumerable when there should clearly be multiple items. It also features a terrible solution I sledgehammered into my code to deal with this for now.

@odelvalle
Copy link
Member

Thanks @Flaniga3 but I can´t test this issue using external SQL Query. Please, can you post some test that help me to reproduce this "error" using only slapper.automapper?

@FilipFilipov
Copy link
Author

I've found a test that better exemplifies the issue in ReadMeTests.

First change the models' Orders and OrderDetails properties from IList to Array, then try running I_Can_Map_Nested_Types_And_Resolve_Duplicate_Entries_Properly_Using_Dynamics. You will actually hit a different bug first, when trying to invoke the array's Contains method in MapCollection, because that's actually an extension method for arrays. The proper invocation in that case is something like:

MethodInfo containsMethod = typeof(Enumerable).GetMethods()
  .Single(m => m.Name == "Contains" && m.GetParameters().Length == 2)
  .MakeGenericMethod(type);
bool alreadyContainsInstance = (bool)containsMethod.Invoke(null,
  new[] { instance, instanceToAddToCollectionInstance });

Once you've done that, you should run into my original bug, as your mapped Customer will now have only one OrderDetails instead of two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants