Skip to content

Commit

Permalink
feat: do not use deep cloning in queryable projections (#1717)
Browse files Browse the repository at this point in the history
  • Loading branch information
latonz authored Feb 12, 2025
1 parent 33660f8 commit ca3080e
Show file tree
Hide file tree
Showing 16 changed files with 58 additions and 29 deletions.
4 changes: 4 additions & 0 deletions docs/docs/configuration/mapper.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ public partial class CarMapper
}
```

:::note
Deep cloning is not applied to `IQueryable` projection mappings.
:::

## Properties / fields

On each mapping method declaration, property and field mappings can be customized.
Expand Down
1 change: 1 addition & 0 deletions docs/docs/configuration/queryable-projections.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ such mappings have several limitations:
- Enum mappings do not support the `ByName` strategy
- Reference handling is not supported
- Nullable reference types are disabled
- Deep cloning is not applied
:::

## Property configurations
Expand Down
19 changes: 15 additions & 4 deletions src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,32 @@ MapperConfiguration defaultMapperConfiguration
mapper.EnumNamingStrategy
),
new MembersMappingConfiguration([], [], [], [], [], mapper.IgnoreObsoleteMembersStrategy, mapper.RequiredMappingStrategy),
[]
[],
mapper.UseDeepCloning
);
}

public MappingConfiguration MapperConfiguration { get; }

public MappingConfiguration BuildFor(MappingConfigurationReference reference, DiagnosticCollection diagnostics)
public MappingConfiguration BuildFor(
MappingConfigurationReference reference,
bool supportsDeepCloning,
DiagnosticCollection diagnostics
)
{
if (reference.Method == null)
return MapperConfiguration;
return supportsDeepCloning ? MapperConfiguration : MapperConfiguration with { UseDeepCloning = false };

var enumConfig = BuildEnumConfig(reference, diagnostics);
var membersConfig = BuildMembersConfig(reference, diagnostics);
var derivedTypesConfig = BuildDerivedTypeConfigs(reference.Method);
return new MappingConfiguration(MapperConfiguration.Mapper, enumConfig, membersConfig, derivedTypesConfig);
return new MappingConfiguration(
MapperConfiguration.Mapper,
enumConfig,
membersConfig,
derivedTypesConfig,
supportsDeepCloning && MapperConfiguration.Mapper.UseDeepCloning
);
}

private IReadOnlyCollection<DerivedTypeMappingConfiguration> BuildDerivedTypeConfigs(IMethodSymbol method)
Expand Down
3 changes: 2 additions & 1 deletion src/Riok.Mapperly/Configuration/MappingConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ public record MappingConfiguration(
MapperAttribute Mapper,
EnumMappingConfiguration Enum,
MembersMappingConfiguration Members,
IReadOnlyCollection<DerivedTypeMappingConfiguration> DerivedTypes
IReadOnlyCollection<DerivedTypeMappingConfiguration> DerivedTypes,
bool UseDeepCloning
);
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Riok.Mapperly.Descriptors;
public class InlineExpressionMappingBuilderContext : MappingBuilderContext
{
public InlineExpressionMappingBuilderContext(MappingBuilderContext ctx, IUserMapping? userMapping, TypeMappingKey mappingKey)
: base(ctx, userMapping, null, mappingKey, false) { }
: base(ctx, userMapping, null, mappingKey, ignoreDerivedTypes: false, supportsDeepCloning: false) { }

private InlineExpressionMappingBuilderContext(
InlineExpressionMappingBuilderContext ctx,
Expand All @@ -25,7 +25,7 @@ private InlineExpressionMappingBuilderContext(
TypeMappingKey mappingKey,
bool ignoreDerivedTypes
)
: base(ctx, userMapping, diagnosticLocation, mappingKey, ignoreDerivedTypes) { }
: base(ctx, userMapping, diagnosticLocation, mappingKey, ignoreDerivedTypes, supportsDeepCloning: false) { }

public override bool IsExpression => true;

Expand Down
21 changes: 17 additions & 4 deletions src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,38 @@ public MappingBuilderContext(
FormatProviderCollection formatProviders,
IUserMapping? userMapping,
TypeMappingKey mappingKey,
Location? diagnosticLocation = null
Location? diagnosticLocation = null,
bool supportsDeepCloning = true
)
: base(parentCtx, diagnosticLocation ?? userMapping?.Method.GetSyntaxLocation())
{
InstanceConstructors = instanceConstructors;
_formatProviders = formatProviders;
UserMapping = userMapping;
MappingKey = mappingKey;
Configuration = ReadConfiguration(new MappingConfigurationReference(UserSymbol, mappingKey.Source, mappingKey.Target));
Configuration = ReadConfiguration(
new MappingConfigurationReference(UserSymbol, mappingKey.Source, mappingKey.Target),
supportsDeepCloning
);
}

protected MappingBuilderContext(
MappingBuilderContext ctx,
IUserMapping? userMapping,
Location? diagnosticLocation,
TypeMappingKey mappingKey,
bool ignoreDerivedTypes
bool ignoreDerivedTypes,
bool supportsDeepCloning = true
)
: this(ctx, ctx.InstanceConstructors, ctx._formatProviders, userMapping, mappingKey, diagnosticLocation)
: this(
ctx,
ctx.InstanceConstructors,
ctx._formatProviders,
userMapping,
mappingKey,
diagnosticLocation,
supportsDeepCloning: supportsDeepCloning
)
{
if (ignoreDerivedTypes)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public static class DirectAssignmentMappingBuilder
{
return
SymbolEqualityComparer.IncludeNullability.Equals(ctx.Source, ctx.Target)
&& (!ctx.Configuration.Mapper.UseDeepCloning || ctx.Source.IsImmutable())
&& (!ctx.Configuration.UseDeepCloning || ctx.Source.IsImmutable())
? new DirectAssignmentMapping(ctx.Source)
: null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public static class EnumerableMappingBuilder
// cannot cast if the method mapping is synthetic, deep clone is enabled or target is an unknown collection
if (
!elementMapping.IsSynthetic
|| ctx.Configuration.Mapper.UseDeepCloning
|| ctx.Configuration.UseDeepCloning
|| ctx.CollectionInfos!.Target.CollectionType == CollectionType.None
)
{
Expand Down Expand Up @@ -202,7 +202,7 @@ private static INewInstanceMapping BuildArrayToArrayMapping(MappingBuilderContex
// use a for loop mapping otherwise.
if (elementMapping.IsSynthetic)
{
return ctx.Configuration.Mapper.UseDeepCloning
return ctx.Configuration.UseDeepCloning
? new ArrayCloneMapping(ctx.Source, ctx.Target)
: new CastMapping(ctx.Source, ctx.Target);
}
Expand Down Expand Up @@ -320,7 +320,7 @@ private static (bool CanMapWithLinq, string? CollectMethod) ResolveCollectMethod
// if the target is an IEnumerable<T> don't collect at all
// except deep cloning is enabled.
var targetIsIEnumerable = ctx.CollectionInfos!.Target.CollectionType == CollectionType.IEnumerable;
if (targetIsIEnumerable && !ctx.Configuration.Mapper.UseDeepCloning)
if (targetIsIEnumerable && !ctx.Configuration.UseDeepCloning)
return (true, null);

// if the target is IReadOnlyCollection<T> or IEnumerable<T>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public static class ExplicitCastMappingBuilder
if (!ctx.IsConversionEnabled(MappingConversionType.ExplicitCast))
return null;

if (ctx.Configuration.Mapper.UseDeepCloning && !ctx.Source.IsImmutable() && !ctx.Target.IsImmutable())
if (ctx.Configuration.UseDeepCloning && !ctx.Source.IsImmutable() && !ctx.Target.IsImmutable())
return null;

// ClassifyConversion does not check if tuple field member names are the same
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public static class ImplicitCastMappingBuilder
if (!ctx.IsConversionEnabled(MappingConversionType.ImplicitCast))
return null;

if (ctx.Configuration.Mapper.UseDeepCloning && !ctx.Source.IsImmutable() && !ctx.Target.IsImmutable())
if (ctx.Configuration.UseDeepCloning && !ctx.Source.IsImmutable() && !ctx.Target.IsImmutable())
return null;

// ClassifyConversion does not check if tuple field member names are the same
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public static class MemoryMappingBuilder

private static NewInstanceMapping? BuildSpanToMemoryMapping(MappingBuilderContext ctx, INewInstanceMapping elementMapping)
{
if (elementMapping.IsSynthetic && !ctx.Configuration.Mapper.UseDeepCloning)
if (elementMapping.IsSynthetic && !ctx.Configuration.UseDeepCloning)
return new SourceObjectMethodMapping(ctx.Source, ctx.Target, ToArrayMethodName);

var targetArray = ctx.Types.GetArrayType(elementMapping.TargetType);
Expand All @@ -117,23 +117,23 @@ public static class MemoryMappingBuilder

private static NewInstanceMapping? BuildMemoryToArrayMapping(MappingBuilderContext ctx, INewInstanceMapping elementMapping)
{
if (!elementMapping.IsSynthetic || ctx.Configuration.Mapper.UseDeepCloning)
if (!elementMapping.IsSynthetic || ctx.Configuration.UseDeepCloning)
return BuildSpanToArrayMethodMapping(ctx, elementMapping);

return new SourceObjectMethodMapping(ctx.Source, ctx.Target, ToArrayMethodName);
}

private static NewInstanceMapping? BuildMemoryToSpanMapping(MappingBuilderContext ctx, INewInstanceMapping elementMapping)
{
if (!elementMapping.IsSynthetic || ctx.Configuration.Mapper.UseDeepCloning)
if (!elementMapping.IsSynthetic || ctx.Configuration.UseDeepCloning)
return BuildMemoryToSpanMethod(ctx, elementMapping);

return new SourceObjectMemberMapping(ctx.Source, ctx.Target, SpanMemberName);
}

private static INewInstanceMapping BuildArrayToMemoryMapping(MappingBuilderContext ctx, INewInstanceMapping elementMapping)
{
if (!elementMapping.IsSynthetic || ctx.Configuration.Mapper.UseDeepCloning)
if (!elementMapping.IsSynthetic || ctx.Configuration.UseDeepCloning)
return new ArrayForMapping(
ctx.Source,
ctx.Types.GetArrayType(elementMapping.TargetType),
Expand All @@ -155,7 +155,7 @@ private static INewInstanceMapping BuildArrayToMemoryMapping(MappingBuilderConte

private static NewInstanceMapping? BuildMemoryToMemoryMapping(MappingBuilderContext ctx, INewInstanceMapping elementMapping)
{
if (!elementMapping.IsSynthetic || ctx.Configuration.Mapper.UseDeepCloning)
if (!elementMapping.IsSynthetic || ctx.Configuration.UseDeepCloning)
return BuildSpanToArrayMethodMapping(ctx, elementMapping);

return new CastMapping(ctx.Source, ctx.Target);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public static class SpanMappingBuilder
// if the source is Span/ReadOnlySpan or Array and target is Span/ReadOnlySpan
// and element type is the same, then direct cast
(CollectionType.Span or CollectionType.ReadOnlySpan or CollectionType.Array, CollectionType.Span or CollectionType.ReadOnlySpan)
when elementMapping.IsSynthetic && !ctx.Configuration.Mapper.UseDeepCloning => new CastMapping(ctx.Source, ctx.Target),
when elementMapping.IsSynthetic && !ctx.Configuration.UseDeepCloning => new CastMapping(ctx.Source, ctx.Target),

// otherwise map each value into an Array
_ => BuildToArrayOrMap(ctx, elementMapping),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static class ToObjectMappingBuilder
if (ctx.Target.SpecialType != SpecialType.System_Object)
return null;

if (!ctx.Configuration.Mapper.UseDeepCloning)
if (!ctx.Configuration.UseDeepCloning)
return new CastMapping(ctx.Source, ctx.Target);

if (ctx.Source.SpecialType == SpecialType.System_Object)
Expand Down
4 changes: 2 additions & 2 deletions src/Riok.Mapperly/Descriptors/SimpleMappingBuilderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,6 @@ public virtual bool IsConversionEnabled(MappingConversionType conversionType) =>
public void ReportDiagnostic(DiagnosticDescriptor descriptor, ISymbol? symbolLocation, params object[] messageArgs) =>
_diagnostics.ReportDiagnostic(descriptor, symbolLocation?.GetSyntaxLocation() ?? _diagnosticLocation, messageArgs);

protected MappingConfiguration ReadConfiguration(MappingConfigurationReference configRef) =>
_configurationReader.BuildFor(configRef, _diagnostics);
protected MappingConfiguration ReadConfiguration(MappingConfigurationReference configRef, bool supportsDeepCloning) =>
_configurationReader.BuildFor(configRef, supportsDeepCloning, _diagnostics);
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public Task ClassToClassNullableSourcePathAutoFlatten()
[Fact]
public Task NestedPropertyWithDeepCloneable()
{
// deep cloneable should be ignored.
// see https://github.com/riok/mapperly/issues/1710
var source = TestSourceBuilder.MapperWithBodyAndTypes(
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ public partial class Mapper
source,
x => new global::B()
{
Value0 = x.Nested != null && x.Nested.Value0 != null ? global::System.Linq.Enumerable.ToArray(
global::System.Linq.Enumerable.Select(x.Nested.Value0, x1 => x1 == null ? default : x1)
) : default,
Value0 = x.Nested != null && x.Nested.Value0 != null ? x.Nested.Value0 : default,
Value = x.Nested != null && x.Nested.Value != null ? x.Nested.Value : default,
}
);
Expand Down

0 comments on commit ca3080e

Please sign in to comment.