Implementing Consumer<Builder> fluent setters on model builders. #278

kiiadi · 2017-11-09T23:10:26Z

Description

Allows providing a Consumer<Builder> to complex fluent models setters to avoid having to create the appropriate builder yourself. As an example SES send message goes from:

client.sendEmail(SendEmailRequest.builder()
                                 .destination(Destination.builder()
                                                         .toAddresses("[email protected]")
                                                         .build())
                                 .message(Message.builder().body(Body.builder()
                                                                     .text(Content.builder().data("Crud").charset("UTF-8").build())
                                                                     .build())
                                                 .subject(Content.builder().data("Hello").build())
                                                 .build())
                                 .build());

to a:

client.sendEmail(SendEmailRequest.builder()
                                 .destination(d -> d.toAddresses("[email protected]"))
                                 .message(m -> m.body(b -> b.text(t -> t.data("Crud").charset("UTF-8")))
                                                .subject(s -> s.data("Hello")))
                                 .build());

Motivation and Context

Allows customers who dislike the verbose ComplexObject.builder().property("..").build() pattern to create object hierarchies with a shorter syntax.

Testing

New poet tests added.

millems · 2017-11-09T23:25:42Z

codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java

+        return new StringBuilder()
+            .append(StringUtils.isNotBlank(documentation) ? documentation : DEFAULT_SETTER.replace("%s", name) + "\n")
+            .append(LF)
+            .append("This is a convenience which creates an instance of the {@link ")


Can we just use string appending instead of the builder?

could do - but this is how the other doc builders work...

spfink · 2017-11-09T23:51:36Z

codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java

@@ -385,6 +385,30 @@ public String getFluentSetterDocumentation() {
        return docBuilder.toString();
    }

+    public String getDefaultConsumerFluentSetterDocumentation() {


Should we define this in Poet or here?

Seems like all the other documentation definitions are here...

I'm fine with this way, but I kinda like what Varun did in his pagination PR having a PaginationDocs.java file with the docs related to pagination

spfink

Just the one minor comment on docs as far as the code goes.

If we have examples for this style, definitely need to figure out some formatting pattern for nested objects.

spfink · 2017-11-10T21:21:39Z

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesrequest.java

@@ -1064,6 +1065,21 @@ public String toString() {
        Builder structWithNestedTimestampMember(StructWithTimestamp structWithNestedTimestampMember);

        /**
+         * Sets the value of the StructWithNestedTimestampMember property for this object.
+         *
+         * This is a convenience which creates an instance of the {@link StructWithTimestamp.Builder} avoiding the need


Include something on how we will call build automatically?

varunnvs92 · 2017-11-10T23:45:03Z

codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java

-                .append(getEnumDoc());
-        return docBuilder.toString();
+        return getSetterDocumentation()
+               + LF


what does LF abbr stand for (saw the member declaration)?

millems reviewed Nov 9, 2017

View reviewed changes

kiiadi force-pushed the kiiadi/consumer-builders-on-models branch from 88e365b to 0ef37a2 Compare November 9, 2017 23:45

kiiadi mentioned this pull request Nov 9, 2017

sdk2 comment #188

Closed

spfink reviewed Nov 9, 2017

View reviewed changes

kiiadi force-pushed the kiiadi/consumer-builders-on-models branch from 0ef37a2 to 0725586 Compare November 10, 2017 01:38

kiiadi mentioned this pull request Nov 10, 2017

Implementing Consumer<Builder> client operations #280

Merged

spfink approved these changes Nov 10, 2017

View reviewed changes

varunnvs92 reviewed Nov 10, 2017

View reviewed changes

varunnvs92 approved these changes Nov 10, 2017

View reviewed changes

kiiadi force-pushed the kiiadi/consumer-builders-on-models branch from 0725586 to f7f0a91 Compare November 13, 2017 20:06

Implementing Consumer<Builder> fluent setters on model builders.

710b3af

kiiadi force-pushed the kiiadi/consumer-builders-on-models branch from 73351c3 to 710b3af Compare November 13, 2017 22:42

kiiadi merged commit c56d647 into master Nov 13, 2017

zoewangg deleted the kiiadi/consumer-builders-on-models branch December 6, 2017 19:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing Consumer<Builder> fluent setters on model builders. #278

Implementing Consumer<Builder> fluent setters on model builders. #278

kiiadi commented Nov 9, 2017

millems Nov 9, 2017

kiiadi Nov 10, 2017

kiiadi Nov 10, 2017

spfink Nov 9, 2017

kiiadi Nov 10, 2017

spfink Nov 10, 2017

spfink left a comment

spfink Nov 10, 2017

kiiadi Nov 13, 2017

varunnvs92 Nov 10, 2017

kiiadi Nov 13, 2017

Implementing Consumer<Builder> fluent setters on model builders. #278

Implementing Consumer<Builder> fluent setters on model builders. #278

Conversation

kiiadi commented Nov 9, 2017

Description

Motivation and Context

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spfink left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment