Protobuf3-friendly mappers

[jvmlet]

Hello I'm trying to map POJO to protobuf 3 generated classes. As you may know, protobuf uses Builder pattern to instantiate structures and all inner fields as well as iterable internal fields. It also doesn't allow to set null values to any field. Having said this, I would like to ask you to support the following features :

• Factory methods (except factory of root object if any) might be passed parent (field container) target object the same way you are passing it to @After/Before Mapping methods (using @MappingTarget).
• Adding objects to iterable target field might be overridden in some way (there is no setSomeList in protobuf message, only addSome(Object,[index]))
• null-evaluated source fields might be skipped for mapping to target object.

Regards, Alexander

[gunnarmorling]

Hi @jvmlet, thanks for opening this one, I think it's a very interesting feature request.

I am no expert on Protocol Buffers, so could you maybe share a protobuf class as it is generated by the tool so we can discuss where it deviates from the JavaBeans convention and what would have to be done in order to support this properly?

Factory methods (except factory of root object if any) might be passed parent...

I am not sure I am following. Could you provide an example for what you have in mind here?

Adding objects to iterable target field might be overridden in some way

There is support for addXy() methods already, admittedly without the index, though. An example would help to better understand what else is needed here.

null-evaluated source fields might be skipped for mapping to target object.

Again an example would be great :) We exclude nulls in many places already and it will be further improved with pending PR #720 (which addresses issue #669 that also is based on a protobuf requirement).

Bottom line is, I'd love to see better support for protobuf in MapStruct, but we'll need your help to make it happen :)

[gunnarmorling]

Ah, and what are the significant differences between protobuf2 and protobuf3 types? Is there anything we'd have to consider specifically?

[jvmlet]

HI @gunnarmorling Sorry it took time to respond. I'll provide the simplest sample of proto message :

message B{

}
message A{
    B b =1;
}

The protoc compiler will generate inner builder class for each message :

public class A{
  public static Builder   extends com.google.protobuf.GeneratedMessage.Builder<Builder>{
    setB(B){}
    setB(B.Builder){}
    .....
    public B.Builder  getBBuilder(){ }
  }
}

In addition to strongly typed API, each protoc-generated class extends from basic AbstractMessage.Builder (via GeneratedMessage.Builder) that provides low-level API to query fields description and factory methods based on FieldDescription

Factory methods (except factory of root object if any) might be passed parent (field container) target object the same way you are passing it to @After/Before Mapping methods (using @MappingTarget).

I prefer to use getBBuilder() instead of set(B/B.Builder) to populate B fields. It would be possible to create B.Builder via getBBuilder() if you support factory methods, that accept parent object instance and field name:

public B.Builder bBulderFactory(AbstractMessage.Builder parent, String fieldName){
 return  (B.Builder) parent.getFieldBuilder(parent.getDescriptorForType().findFieldByName(fieldName));
}

I know that this might looks to you as antipattern for accessing fields by reflections, but this is not, this API is also generated by protoc.

Adding objects to iterable target field might be overridden in some way

I'll check this option

null-evaluated source fields might be skipped for mapping to target object.

Looked at the conversation of this one. looking forward for next release.

what are the significant differences between protobuf2 and protobuf3 types?

I'm not sure, but I think the generated API remains the same, it's only about a different proto syntax.

[eiswind]

I struggle hard with collections. for a "list property" permissions the getter is getPermissionsList but it is initialized with am immutable empty list. there is an adder addPermissions and addAllPermissions but when I map to permissionsList those are not recognized. is there any way to specify the adder?

[eiswind]

Just to be exact: https://developers.google.com/protocol-buffers/docs/reference/java-generated#repeated-fields

[filiphr]

So the property that you write in the @Mapping is permissionsList, right? You can use the AccessorNamingStrategy, even better extend the DefaultAccessorNamingStrategy and override the getElementName(ExecutableElement adderMethod) so MapStruct will be able to match the correct adder method. Currently the property that MapStruct sees for your adders is permissions and allPermissions.

If you are interested you can create a PR with an AccessorNamingStrategy that will work for protobufs. If only repeated fields are in question, maybe protobuf would be interested to have the protobuf strategy in their own repo, that way they can keep it compatible with their latest versions.

[eiswind]

Its so small! Had to find your example to see how to include it in the classpath. Works like a charm. But I don't know if it's worth a pr: https://github.com/eiswind/mapstruct-proto3/blob/master/src/main/java/de/eiswind/bmfx/mapstruct/proto3/Proto3AccessorNamingStrategy.java

[filiphr]

Yes it looks really small. I don't know much about protobuf, but are all builders extending or implementing some java class or interface? If that is the case, then I would say that the Proto3AccessorNamingStrategy should also check that, because if you are not in a protobuf builder the added should behave differently.

You can get the enclosed type from the executable method and do the checks. I am not sure if it is interesting for MapStruct to provide support for protobuf (maybe in it's own module), what do you think @gunnarmorling, @agudian, @sjaakd?

In any case an example with protobuf and MapStruct in our mapstruct-examples would be interesting, if you are interested in making one :).

[eiswind]

yes that could make sense. if I could check for the type (protobuf messages extend a baseclass) I could also turn around the naming convention, so that the property mapped will be "permissions" instead of "permissionsList", that would free me from specifying the additional mapping. if I find time this week i could put together an example.

[sjaakd]

What about adding a this strategy as ProtobufAccessorNamingStrategy to our org.mapstruct.ap.spi and adding an example to our examples repository on how to use it.. There's no hard dependency on protobuf or its api's right?

It serves 2 purposes: explaining our spi and enriching our MapStruct with out-of -the-box functionality.

[eiswind]

While setting up the example (https://github.com/eiswind/mapstruct-examples) I found that I have trouble with null values. Is there a way to not call the setter if the source value is null?

[sjaakd]

You might want to checkout the null value mapping strategy.

[filiphr]

@eiswind awesome. Thanks for the example, replied you back in the PR for the examples.

[bhdrk]

Mapstruct works very well for me. I'm just adding a mapper method for builder. E.g.

messages.proto

message InputFile {
    string fileId = 1;
    string fileName = 2;
    string fileExt = 3;
}

InputFileDTO.java

public class InputFileDTO {
    private String fileId;
    private String fileName;
    private String fileExt;

    // getters and setters ...
}

InputFileMapper.java

@Mapper(uses=ObjectFactory.class)
public interface InputFileMapper {
    DataSet.InputFile.Builder mapForBuilder(InputFileDTO dto);

    default DataSet.InputFile map(InputFileDTO dto) {
        return mapForBuilder(dto).build();
    }
}

ObjectFactory.java

public class ObjectFactory {
    public InputFile.Builder inputFileBuilder() {
        return InputFile.newBuilder();
    }
}

[filiphr]

@eiswind has written an example for how to use protobuf with MapStruct. Have a look in the mapstruct-protobuf3 example

[filiphr]

Maybe we should consider bringing the current AccessorNamingStrategy from the example into MapStruct, or maybe even a new module that will contain it.

In any case I think that the AccessorNamingStrategy could be a bit changed as written in my https://github.com/mapstruct/mapstruct-examples/issues/14#issuecomment-300223989

[francisip]

Thank you so much for adding the Proto example. The example works.

But I had two findings:

1. The custom AccessorNamingStrategy appears to be unnecessary.

Even if I commented out the getElementName() method of the custom AccessorNamingStrategy, the example still works. This class does not not seem to be necessary. Why?

2. Failed to mapList<String>

I added a repeated string allowed_drivers field to the proto, and a List<String> permittedDrivers field to the POJO. I failed to create mappings between these two collection fields. The problems are as follows:

2.1. First, the Java class generated from the target protocol buffer has a method of Builder#getAllowedDriversList(). But its return type is an interface ProtocolStringList, instead of List<String> like theList<PermissionDTO> (although ProtocolStringList does extendList<String>). As a result the generated codes tried to do a "new ProtocolStringList()", which of course failed.

To resolve it, I add a BuidlerFactory method that returns a new, empty, and mutable impl object of ProtocolStringList. This seems to overcome the issue.

2.2. Second, the generated codes still tries to do:

builderr.getAllowedDriversList().addAll(sourcePojo.getPermittedDrivers());

Instead of doing the builder.add() operation like the permissions.

This piece of generated codes failed in run time because targetProtoBuilder.getAllowedDriversList() returns an immutable ProtocolStringList, which throws UnsupportedException on the addAll() method.

Can someone give some pointers on how to make 'repeated string' field works?

Thanks a lot.

[pascalwhoop]

2.2. Second, the generated codes still tries to do: builderr.getAllowedDriversList().addAll(sourcePojo.getPermittedDrivers()); Instead of doing the builder.add() operation like the permissions. This piece of generated codes failed in run time because targetProtoBuilder.getAllowedDriversList() returns an immutable ProtocolStringList, which throws UnsupportedException on the addAll() method.

Running into the same Issue. With a source having these two properties:

List<Rate> rates;
List<long> supersedes;

And mapping both with

@Mappings({
            @Mapping(source = "rates", target = "ratesList"),
            @Mapping(source = "supersedes", target = "supersedesList")
    })
    @Override
    public abstract PBTariffSpecification.Builder map(TariffSpecification ptacObject);

generates

        if ( ptacObject.getRates() != null ) {
            for ( Rate ratesList : ptacObject.getRates() ) {
                builder.addRates( rateMapper.map( rateMapper.map( ratesList ) ) );
            }
        }
        if ( builder.getSupersedesList() != null ) {
            List<Long> list = ptacObject.getSupersedes();
            if ( list != null ) {
                builder.getSupersedesList().addAll( list );
            }
        }

I am circumventing with ignoring the list and mapping it manually. But I believe the fact that there is a List of primitives causes something to make MapStruct think it can go ahead and addAll the things.

[nhoughto]

Looking at using gRPC and mapstruct, are there any gotchas to be aware of?

[chris922]

Try using 1.3.0.Beta1 because it supports protobuf builders

[filiphr]

I will also look at the mapstruct-protobuf3 example in our examples repository.

[vordimous]

Cross posted: https://github.com/mapstruct/mapstruct-examples/issues/14#issuecomment-433528487

But I am trying to use mapstruct with protobuf3 and Gradle. any help would be appreciated

[alex-lzl]

I will also look at the mapstruct-protobuf3 example in our examples repository.

The getElementName() method on the example doesn't get called at all (proto3). I had to make an ugly hack to just trim of "List" from the property name (overriding getPropertyName method).

Another thing would be nice is to skip these proto3 generated properties, or provide a SPI to allow people to ignore by name (currently MappingExclusionProvider only allow filter by type). Then we can safely make unmappedTargetPolicy = ReportingPolicy.ERROR to catch mis-mapping properties).

"clearField, clearOneof, mergeFrom, idBytes, emailBytes, unknownFields, mergeUnknownFields, allFields, permissionsValueList".

Thank you!

[haojia-rogers]

any update on ignore proto3 generated properties?

[TehBakker]

Wondering if there is any way to tell mapstruct to look for list attribut without having to tell him about the "list" suffix that protobuf adds to repeated fields ?

[sjaakd]

Wondering if there is any way to tell mapstruct to look for list attribut without having to tell him about the "list" suffix that protobuf adds to repeated fields ?

https://github.com/mapstruct/mapstruct-examples/tree/master/mapstruct-protobuf3

[TehBakker]

Wondering if there is any way to tell mapstruct to look for list attribut without having to tell him about the "list" suffix that protobuf adds to repeated fields ?

https://github.com/mapstruct/mapstruct-examples/tree/master/mapstruct-protobuf3

Saw that, but they explicitly giving the list suffix in mapper @Mapping(source = "mainDepartments", target = "mainDepartmentsList")

[filiphr]

@TehBakker I think that you can adapt the AccessorNamingStrategy and remove the List when getting the property name.

[donbeave]

@TehBakker @jvmlet please check this project: https://github.com/entur/mapstruct-spi-protobuf, it provides full support for protobuf3.