Skip to content

Default expansion of dictionaries supported #240

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

Merged
merged 5 commits into from
May 9, 2025

Conversation

alexmo16
Copy link
Contributor

@alexmo16 alexmo16 commented May 5, 2025

Hi @BlaiseD,

Related to #239.

I think I added what was missing to support default expansion of dictionaries.

I was wondering if I would need to do do some code to select child properties of the dictionary values when TValue is an object with properties. i.e. `Dictionary<string, MyObj> and MyObj is a complex type.

@alexmo16 alexmo16 changed the title Dictionaries supported within DefaultExpansionsBuilder Default expansion of dictionaries supported May 5, 2025
@alexmo16
Copy link
Contributor Author

alexmo16 commented May 5, 2025

I did some testing, and it seems to also work with a complex type as TValue.

@BlaiseD
Copy link
Member

BlaiseD commented May 7, 2025

Don't we want to just support dictionaries as lists? For example just replace the call to GetUnderlyingElementType with one from this library.

        public static Type GetUnderlyingElementType(this Type type)
        {
            TypeInfo tInfo = type.GetTypeInfo();
            if (tInfo.IsArray)
                return tInfo.GetElementType();

            if (!type.IsGenericType)
                throw new ArgumentException(nameof(type));

            Type[] genericArguments = type.GetGenericArguments();
            Type genericTypeDefinition = type.GetGenericTypeDefinition();

            if (genericTypeDefinition == typeof(IGrouping<,>))
                return genericArguments[1];
            else if (typeof(IDictionary<,>).IsAssignableFrom(genericTypeDefinition))
                return typeof(KeyValuePair<,>).MakeGenericType(genericArguments[0], genericArguments[1]);
            else if (genericArguments.Length == 1)
                return genericArguments[0];
            else
                throw new ArgumentException(nameof(type));
        }

@BlaiseD
Copy link
Member

BlaiseD commented May 7, 2025

Probably best to include a test with a key or value complex type if that's possible - agree?

Add an implementation of GetUnderlyingElementType inside AutoMapper.AspNet.OData instead of using the one from LogicBuilder.Expressions.Utils.
@alexmo16
Copy link
Contributor Author

alexmo16 commented May 7, 2025

Don't we want to just support dictionaries as lists? For example just replace the call to GetUnderlyingElementType with one from this library.

Yeah I agree. I added GetUnderlyingElementType method in TypeExtensions.

@alexmo16
Copy link
Contributor Author

alexmo16 commented May 7, 2025

Probably best to include a test with a key or value complex type if that's possible - agree?

I will add a test.

Side note: It made me realize that it would be useful to have an easier way of running Web.Tests. We would be able to test if the serialization of dictionaries is working. Maybe in another PR.

@alexmo16
Copy link
Contributor Author

alexmo16 commented May 7, 2025

ready for review.

….TypeExtensions.GetUnderlyingElementType in LinqExtensions.
@BlaiseD BlaiseD merged commit 3a314ae into AutoMapper:master May 9, 2025
3 checks passed
@BlaiseD
Copy link
Member

BlaiseD commented May 9, 2025

Thanks.

@BlaiseD
Copy link
Member

BlaiseD commented May 10, 2025

Probably best to include a test with a key or value complex type if that's possible - agree?

I will add a test.

Side note: It made me realize that it would be useful to have an easier way of running Web.Tests. We would be able to test if the serialization of dictionaries is working. Maybe in another PR.

On the build server? Nice if you can make it work. I've always used IIS hosts so didn't consider it honestly.

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

Successfully merging this pull request may close these issues.

2 participants