facebookarchive / swift

An annotation-based Java library for creating Thrift serializable types and services.
Apache License 2.0
900 stars 297 forks source link

Both params constructor and builder patterns at the same time #262

Closed vicmosin closed 9 years ago

vicmosin commented 9 years ago

Why it is forbidden two use both patterns within the same object?

@ThriftStruct(builder = Sms.SmsBuilder.class)
@NoArgsConstructor
public final class Sms extends Base {

    private String messageId;

    private String provider;

    public Sms(Long _id, String uuid, long created, String messageId, String provider) {
        super(_id, uuid, created);

        this.messageId = messageId;
        this.provider = provider;
    }

    public static class SmsBuilder {
        private String messageId;
        private String provider;

        private SmsBuilder() { }

        @ThriftField
        public SmsBuilder messageId(String messageId) {
            this.messageId = messageId;
            return this;
        }

        @ThriftField
        public SmsBuilder provider(String provider) {
            this.provider = provider;
            return this;
        }

        @ThriftConstructor
        public Sms build() {
            Sms sms = new Sms();
            sms.setMessageId(messageId);
            sms.setProvider(provider);
            return sms;
        }
    }

    public static SmsBuilder builder() {
        return new SmsBuilder();
    }

    @ThriftField(5)
    public String getMessageId() {
        return messageId;
    }

    @ThriftField
    public void setMessageId(String messageId) {
        this.messageId = messageId;
    }

    @ThriftField(5)
    public String getProvider() {
        return provider;
    }

    @ThriftField
    public void setProvider(String provider) {
        this.provider = provider;
    }
}
dain commented 9 years ago

I think the problem is messageId and provider are both tagged as being field 5

andrewcox commented 9 years ago

What happens when you try to run the program? (What error do you get?)

vicmosin commented 9 years ago

I am sorry regarding the numbers mistake, but it wasn't an issue... I talking about the following exception:

com.facebook.swift.codec.metadata.MetadataErrorException: Error: Inject method com. Sms. setMessageId is not allowed on struct class, since struct has a builder 
at com.facebook.swift.codec.metadata.MetadataErrors.addError(MetadataErrors.java:95)
at com.facebook.swift.codec.metadata.AbstractThriftMetadataBuilder.addMethod(AbstractThriftMetadataBuilder.java:430)

I solved the issue by removing the builder annotation and moving the builder itself to external class, but the solution is not so nice to me...

andrewcox commented 9 years ago

I think the idea is that when deserializing an object, swift has to choose one of the two ways to set the field, and we don't have any guarantee both have the same behavior.

If you want swift to use the builder, but you still want to have the setter, you can I think, you just have to de-annotate it.