Mapster icon indicating copy to clipboard operation
Mapster copied to clipboard

Support for mapping protobuf RepeatedField

Open jetersen opened this issue 4 years ago • 14 comments

I can understand from #254 that mapping list with AddRange and such was previously supported. Currently protobuf generates repeated without a setter, your expected to either use the constructor, Add or AddRange Since protobuf has a RepeatedField type perhaps this use case could be easily be supported? See the full signature of RepeatedField over at https://github.com/protocolbuffers/protobuf/blob/master/csharp/src/Google.Protobuf/Collections/RepeatedField.cs

Here is a snippet of generated getter code, I have included entire signature for Destination at the bottom

    private readonly pbc::RepeatedField<string> files_ = new pbc::RepeatedField<string>();
    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public pbc::RepeatedField<string> Files {
      get { return files_; }
    }
message Destination {
    repeated string files = 0;
}
public class Source {
    public List<string> files { get; set; } = new()
}

Our current solution is to either use AfterMapping or ConstructorUsing which feels hacky especially for setting up config for a ServiceMapper. Of course this issue is doubled when receiving end has to go back from Destination back to Source as they also need to have configure. The problem also adds up if you have multiple repeated items on your protobuf message.

config.NewConfig<Source, Destination>()
    .ConstructUsing(source => new Destination
    {
        Files =
        {
            source.Files.Adapt<IEnumerable<string>>(),
        },
    });
config.NewConfig<Destination, Source>()
    .ConstructUsing(destination => new Source
    {
        Files =
        {
            destination.Files.Adapt<IEnumerable<string>>(),
        },
    });

The usual signature we use when mapping from a POCO to protobuf is _mapper.Map<Destination>(source); which means the Destination object does not exist yet and as such Mapster could use the constructor or AddRange logic for the mapping without concern 🤔

var source = new Source
{
    Files =
    {
        "Hello",
        "World",
    },
};
var destination = _mapper.Map<Destination>(source);

Protobuf signature for Destination message

  public sealed partial class Destination : pb::IMessage<Destination>
  #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE
      , pb::IBufferMessage
  #endif
  {
    private static readonly pb::MessageParser<Destination> _parser = new pb::MessageParser<Destination>(() => new Destination());
    private pb::UnknownFieldSet _unknownFields;
    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public static pb::MessageParser<Destination> Parser { get { return _parser; } }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public static pbr::MessageDescriptor Descriptor {
      get { return global::SpectrumFileService.Grpc.SpectrumFileServiceReflection.Descriptor.MessageTypes[2]; }
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    pbr::MessageDescriptor pb::IMessage.Descriptor {
      get { return Descriptor; }
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public Destination() {
      OnConstruction();
    }

    partial void OnConstruction();

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public Destination(Destination other) : this() {
      files_ = other.files_.Clone();
      _unknownFields = pb::UnknownFieldSet.Clone(other._unknownFields);
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public Destination Clone() {
      return new Destination(this);
    }

    /// <summary>Field number for the "files" field.</summary>
    public const int FilesFieldNumber = 1;
    private static readonly pb::FieldCodec<string> _repeated_files_codec
        = pb::FieldCodec.ForString(10);
    private readonly pbc::RepeatedField<string> files_ = new pbc::RepeatedField<string>();
    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public pbc::RepeatedField<string> Files {
      get { return files_; }
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public override bool Equals(object other) {
      return Equals(other as Destination);
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public bool Equals(Destination other) {
      if (ReferenceEquals(other, null)) {
        return false;
      }
      if (ReferenceEquals(other, this)) {
        return true;
      }
      if(!files_.Equals(other.files_)) return false;
      return Equals(_unknownFields, other._unknownFields);
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public override int GetHashCode() {
      int hash = 1;
      hash ^= files_.GetHashCode();
      if (_unknownFields != null) {
        hash ^= _unknownFields.GetHashCode();
      }
      return hash;
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public override string ToString() {
      return pb::JsonFormatter.ToDiagnosticString(this);
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public void WriteTo(pb::CodedOutputStream output) {
    #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE
      output.WriteRawMessage(this);
    #else
      files_.WriteTo(output, _repeated_files_codec);
      if (_unknownFields != null) {
        _unknownFields.WriteTo(output);
      }
    #endif
    }

    #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE
    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    void pb::IBufferMessage.InternalWriteTo(ref pb::WriteContext output) {
      files_.WriteTo(ref output, _repeated_files_codec);
      if (_unknownFields != null) {
        _unknownFields.WriteTo(ref output);
      }
    }
    #endif

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public int CalculateSize() {
      int size = 0;
      size += files_.CalculateSize(_repeated_files_codec);
      if (_unknownFields != null) {
        size += _unknownFields.CalculateSize();
      }
      return size;
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public void MergeFrom(Destination other) {
      if (other == null) {
        return;
      }
      files_.Add(other.files_);
      _unknownFields = pb::UnknownFieldSet.MergeFrom(_unknownFields, other._unknownFields);
    }

    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    public void MergeFrom(pb::CodedInputStream input) {
    #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE
      input.ReadRawMessage(this);
    #else
      uint tag;
      while ((tag = input.ReadTag()) != 0) {
        switch(tag) {
          default:
            _unknownFields = pb::UnknownFieldSet.MergeFieldFrom(_unknownFields, input);
            break;
          case 10: {
            files_.AddEntriesFrom(input, _repeated_files_codec);
            break;
          }
        }
      }
    #endif
    }

    #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE
    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    void pb::IBufferMessage.InternalMergeFrom(ref pb::ParseContext input) {
      uint tag;
      while ((tag = input.ReadTag()) != 0) {
        switch(tag) {
          default:
            _unknownFields = pb::UnknownFieldSet.MergeFieldFrom(_unknownFields, ref input);
            break;
          case 10: {
            files_.AddEntriesFrom(ref input, _repeated_files_codec);
            break;
          }
        }
      }
    }
    #endif

  }

jetersen avatar Mar 06 '21 03:03 jetersen

Mapster doesn't map to getter only property by default. You can add following config to map to getter only property.

TypeAdapterConfig.GlobalSettings.Default
    .UseDestinationValue(member => member.SetterModifier == AccessModifier.None &&
                                   member.Type.IsGenericType &&
                                   member.Type.GetGenericTypeDefinition() == typeof(RepeatedField<>));

chaowlert avatar Apr 09 '21 15:04 chaowlert

@chaowlert Thanks that is a really good solution :)

jetersen avatar Apr 09 '21 16:04 jetersen

Does this solution support codegeneration with mapster tool? I got Cannot convert immutable type, please consider using 'MapWith' method to create mapping. But i cant use 'MapWith' wit generic types. And also I cant use next configuration

TypeAdapterConfig.GlobalSettings            .ForType<IEnumerable<SourceType>,RepeatedField<DestinationType>>()            
            .MapWith(src =>
            {
                var repeated = new RepeatedField<DestinationType>();
                repeated.AddRange(src?.Select(x => x.Adapt<DestinationType>()) ?? Array.Empty<DestinationType>());
                
                return repeated;
            });

because get Program.cs(58, 22): [CS0834] A lambda expression with a statement body cannot be converted to an expression tree. from compiler

sergey-gindin avatar Mar 21 '25 14:03 sergey-gindin

Yes, I am using Mapster.Tool with gRPC and repeated fields. For example:

repeated EmploymentLevelMessage employment_levels = 8;

and this is the .proto for EmploymentLevelMessage

message EmploymentLevelMessage
{
	EmploymentLevelIdMessage id = 1;
	string employment_level_id = 2;
}

and this is the .proto for EmploymentLevelIdMessage

message EmploymentLevelIdMessage
{
	string id = 1;
}

I have the TypeAdapterConfig.GlobalSettings.Default.UseDestinationValue... in my config and it may seem strange but I also needed the following in my config to get the mapping to work

config.ForType<EmploymentLevelId, EmploymentLevelId>().MapWith(d => d);

EmploymentLevelId is a value object that is mapped to EmploymentLevelIdMessage.

No further mapping was required. I am however using List of T and not IEnumerable of T so I switched one property and it compiled successfully.

One caveat though. I am using .Net 9 and the pre-release versions that added support for .Net 9 were not working so I forked the master branch and updated the versions to .Net 8/9. This forked version is at https://github.com/stagep/Mapster. It was 2 months ago so I cannot remember if the exception I was getting is the same exception you are getting.

Also, you might be over-configuring as I do not have anything like your global MapWith. Remove that from your configuration and if you still get an exception, you may need to clone my fork and build the binaries. I can give you instructions on the steps required to use a local build of Mapster and Mapster.Tool.

stagep avatar Mar 21 '25 16:03 stagep

@sergey-gindin @stagep Here you want to convert a collection of elements of one type to a collection of another type?

List<SourceType> source ;

result = source.Adapt<RepeatField<Destination>>()

If only this, then such a setting is really not needed.

 // I mean this setting
var repeated = new RepeatedField<DestinationType>();
                repeated.AddRange(src?.Select(x => x.Adapt<DestinationType>()) ?? Array.Empty<DestinationType>());
                
                return repeated;

And then maybe Mapster does not define RepeatField<T> as a collection?

update This should work without any settings, RepeatedField implements IList and should be recognized as a collection


[TestMethod]
public void RepeatedFieldWork()
{
    var source = new List<int> { 1, 2, 3 };

    var result = source.Adapt<RepeatedField<string>>();

    result[0].ShouldBe("1");
    result[1].ShouldBe("2");
    result[2].ShouldBe("3");

}

DocSvartz avatar Mar 21 '25 18:03 DocSvartz

I now recall that I was getting the exception "Cannot convert immutable type, please consider using 'MapWith' method to create mapping" when I used the latest pre-release version that supported .Net 9. In my source control I can see that I changed from Mapster.Tool 8.4.1-pre01 (the version that worked with .Net 8) to 8.5.0 which is the version I updated Mapster.Tool to for my local build. What version of .Net are you using? I believe that you are experiencing the identical issue that I was having. I can upload the Nuget packages for you so you do not have to build and the instructions on how to install Mapster.Tool from a local source. Let me know if you want to try this. It will take around 5 to 10 minutes to do this and if this doesn't resolve the issue then another 5 minutes to revert back to your current packages.

stagep avatar Mar 21 '25 18:03 stagep

@stagep I get this Behavior for this example

[TestMethod]
public void RepeatedFieldTest()
{
    var source = new Source316() { Value = new List<int> { 1,2,3 } };

    TypeAdapterConfig.GlobalSettings.Default
    .UseDestinationValue(member => member.SetterModifier == AccessModifier.None &&
                           member.Type.IsGenericType &&
                           member.Type.GetGenericTypeDefinition() == typeof(RepeatedField<>));

    var result = source.Adapt<Destination316>();
}

public class Source316
{
    public IEnumerable<int> Value;
}


public class Destination316
{
    private readonly RepeatedField<string> Value;
} 

If this is really what the author encountered. Then it's like to Poco detection bug and something else. Because the elements are not mapped to the RepeatedField Value collection.

update: this should work

 [TestMethod]
 public void RepeatedFieldTest()
 {
     var source = new Source316() { Value = new List<int> { 1,2,3 } };

     TypeAdapterConfig<Source316, Destination316>
         .NewConfig()
         .EnableNonPublicMembers(true);

     TypeAdapterConfig.GlobalSettings.Default
     .UseDestinationValue(member => member.SetterModifier == AccessModifier.None &&
                            member.Type.IsGenericType &&
                            member.Type.GetGenericTypeDefinition() == typeof(RepeatedField<>));


     var result = source.Adapt<Destination316>();
 }

 public class Destination316
 {
     private readonly RepeatedField<string> Value = new RepeatedField<string>();

     public bool IsPoco { get; set; } // need at least one property with public getter
 } 

DocSvartz avatar Mar 24 '25 04:03 DocSvartz

@stagep @DocSvartz I suggest Mappster detect class with only RepeatedField as immutable because RepeatedField property hasn't public setter.

public sealed partial class GetCountryTariffsGrpcResponse : pb::IMessage<GetCountryTariffsGrpcResponse>
{
...
    /// <summary>Field number for the "CountryTariffs" field.</summary>
    public const int CountryTariffsFieldNumber = 1;
    private static readonly pb::FieldCodec<global::Unitrade.Grpc.Sellers.CustomerReturnsMicroservice.GetCountryTariffsGrpcResponse.Types.CountryTariffDto> _repeated_countryTariffs_codec
        = pb::FieldCodec.ForMessage(10, global::Unitrade.Grpc.Sellers.CustomerReturnsMicroservice.GetCountryTariffsGrpcResponse.Types.CountryTariffDto.Parser);
    private readonly pbc::RepeatedField<global::Unitrade.Grpc.Sellers.CustomerReturnsMicroservice.GetCountryTariffsGrpcResponse.Types.CountryTariffDto> countryTariffs_ = new pbc::RepeatedField<global::Unitrade.Grpc.Sellers.CustomerReturnsMicroservice.GetCountryTariffsGrpcResponse.Types.CountryTariffDto>();
    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
    [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
    public pbc::RepeatedField<global::Unitrade.Grpc.Sellers.CustomerReturnsMicroservice.GetCountryTariffsGrpcResponse.Types.CountryTariffDto> CountryTariffs {
      get { return countryTariffs_; }
    }
...
}

When I Added public bool 'Test' property in protoboof I got follow generated code

public partial class Mapper : IMapper
{
...
        public GetCountryTariffsGrpcResponse Map(GetCountryTariffsQueryResponse p3)
        {
            return p3 == null ? null : new GetCountryTariffsGrpcResponse() {Test = p3.Test};
        }
...
}

there is no code to fill the RepeatedField property 'CountryTariffs'

sergey-gindin avatar Mar 27 '25 11:03 sergey-gindin

@sergey-gindin Try Get this,


 TypeAdapterConfig<Source316, Destination316>
         .NewConfig()
         .EnableNonPublicMembers(true);

or 

TypeAdapterConfig.GlobalSettings.Default.EnableNonPublicMembers(true);

Have there been any changes? And what version of Marster and mapster tool are you using?

Have you set this setting?

 TypeAdapterConfig.GlobalSettings.Default
     .UseDestinationValue(member => member.SetterModifier == AccessModifier.None &&
               member.Type.IsGenericType &&
               member.Type.GetGenericTypeDefinition() == typeof(RepeatedField<>));

DocSvartz avatar Mar 27 '25 11:03 DocSvartz

@DocSvartz I use Mapster 7.4.1-pre01 and Mapster.Tool 8.4.1-pre01 also I created MyRegister class

public class MyRegister : ICodeGenerationRegister
{
public void Register(CodeGenerationConfig config)
    {
          TypeAdapterConfig.GlobalSettings.Default
                      .UseDestinationValue(member => member.SetterModifier == AccessModifier.None &&
                                                     member.Type.IsGenericType &&
                                                     member.Type.GetGenericTypeDefinition() == typeof(RepeatedField<>));
          
          TypeAdapterConfig<GetCountryTariffsQueryResponse, GetCountryTariffsQueryResponse>
                      .NewConfig().MapWith(d => d);

    TypeAdapterConfig<GetCountryTariffsQueryResponse, GetCountryTariffsGrpcResponse>
            .NewConfig()
            .EnableNonPublicMembers(true);

    }
}

sergey-gindin avatar Mar 27 '25 13:03 sergey-gindin

remove this

 TypeAdapterConfig<GetCountryTariffsQueryResponse, GetCountryTariffsQueryResponse>
                      .NewConfig().MapWith(d => d);

    TypeAdapterConfig<GetCountryTariffsQueryResponse, GetCountryTariffsGrpcResponse>
            .NewConfig()
            .EnableNonPublicMembers(true);

add public bool IsPoco { get; set; }

in GetCountryTariffsQueryResponse

Strange, it should work. Maybe GetCountryTariffsQueryResponse contains some other types?

DocSvartz avatar Mar 27 '25 13:03 DocSvartz

This works for me with bypassing Poco problems.

The only thing is that I probably won't be able to determine which build the mapster is from)

 [Mapper]
 public interface IInterfaceMapper
 {
     Destination316 MapTo(Source316 cA);
 }

 public class InterfaceMappingRegister : IRegister
 {
     public void Register(TypeAdapterConfig config)
     {
         TypeAdapterConfig.GlobalSettings.Default
             .UseDestinationValue(member => member.SetterModifier == AccessModifier.None &&
                       member.Type.IsGenericType &&
                       member.Type.GetGenericTypeDefinition() == typeof(RepeatedField<>));
     }



     public class Source316
     {
         public IEnumerable<int> Value;
     }

     public sealed class Destination316
     {
         private readonly RepeatedField<string> Value_ = new RepeatedField<string>();

         public RepeatedField<string> Value { get => Value_; }

         public bool IsPoco { get; set; } // need at least one property with public getter
     }
 }

result


 public partial class InterfaceMapper : TEstConsole.clases.IInterfaceMapper
 {
     public TEstConsole.clases.InterfaceMappingRegister.Destination316 MapTo(TEstConsole.clases.InterfaceMappingRegister.Source316 p1)
     {
         if (p1 == null)
         {
             return null;
         }
         TEstConsole.clases.InterfaceMappingRegister.Destination316 result = new TEstConsole.clases.InterfaceMappingRegister.Destination316();
         
         funcMain1(p1.Value, result.Value);
         return result;
         
     }
     
     private Google.Protobuf.Collections.RepeatedField<string> funcMain1(System.Collections.Generic.IEnumerable<int> p2, Google.Protobuf.Collections.RepeatedField<string> p3)
     {
         if (p2 == null)
         {
             return null;
         }
         Google.Protobuf.Collections.RepeatedField<string> result = p3 ?? new Google.Protobuf.Collections.RepeatedField<string>();
         
         result.Clear();
         
         System.Collections.Generic.IEnumerator<int> enumerator = p2.GetEnumerator();
         
         while (enumerator.MoveNext())
         {
             int item = enumerator.Current;
             result.Add(item.ToString());
         }
         return result;
         
     }
 }

DocSvartz avatar Mar 27 '25 13:03 DocSvartz

@DocSvartz Thanks! It's work! It was my bad. I'm newbie in Mapster. I used ICodeGenerationRegister instead of IRegister. Any ideas to except public bool IsPoco walkaround ?

sergey-gindin avatar Mar 27 '25 19:03 sergey-gindin

@sergey-gindin regarding Poco Detection problem. I hope this PR #770 will solve it. In current version it is required to have at least one property or field with a public setter

(Since working on this problem, I have already forgotten some of the features.
Setter has been must be . Access level does not matter. You can set private or protected from setter).

Perhaps there is another solution to bypass the current conditions. But I don't know it.

DocSvartz avatar Mar 28 '25 01:03 DocSvartz