aws-sdk-java-v2 icon indicating copy to clipboard operation
aws-sdk-java-v2 copied to clipboard

Enhanced DynamoDB annotations are incompatible with Lombok

Open bill-phast opened this issue 4 years ago • 22 comments

Describe the Feature

Lombok is a popular system for reducing boilerplate code, especially in data-centric objects such as database entries. By annotating the class and/or member variables, it autogenerates setters and getters. This creates problems with enhanced dynamodb, because the annotations such as DynamoDbAttribute and DynamoDbPartitionKey cannot be applied to member variables, only to setter/getter functions, and the setter/getter function is not present in the source code. In DynamoDbMapper (from AWS SDK v1), annotations could be applied to functions or member variables, so Lombok worked well. It would be great if the enhanced DynamoDb library could work the same way.

Is your Feature Request related to a problem?

I'm always frustrated when I cannot use the new SDK because it is incompatible with other software that my development team depends on.

Proposed Solution

Ideally the annotations can be applied to member variables in the same manner that the SDKv1 dynamoDbMapper annotations could. As a second-best alternative, it could allow the annotations to be applied to member variables, but ignore them; with a little work, Lombok can be set to copy annotations from member variables to the setters and getters.

Describe alternatives you've considered

The alternative is to write explicit setters and getters for all dynamo-annotation values. This would be a significant pain point, however; we will probably prefer to stay with SDKv1.

Additional Context

Lombok annotations are as follows:

@Setter
@Getter
public class Customer {
  private String id;

  private String email;

  private Instant registrationDate;
}

Functions "setId(String)", "getId()", etc. are all generated automatically, so there is no way to get the annotations onto them.

  • [ ] I may be able to implement this feature request

Your Environment

  • AWS Java SDK version used: 2.13.47
  • JDK version used: Java 11
  • Operating System and version: Ubuntu (on EC2), AWS Lambda (whatever version of Linux that is)

bill-phast avatar Jun 30 '20 19:06 bill-phast

Marking as a feature request.

debora-ito avatar Jul 01 '20 23:07 debora-ito

Lombok supports injecting Annotations to the generated Setter/Getter (although it is an experimental feature) Lombok Getter/Setter Reference ctrl+f "onMethod" Usage Reference

Lombok onX Reference

ocind avatar Jul 02 '20 08:07 ocind

Lombok supports injecting Annotations to the generated Setter/Getter

Yes that could be helpful, but this feature copies annotations from the field to the setter/getter. Enhanced DynamoDB annotations cannot be put onto the field in the first place, so we still need changes before this can work.

bill-phast avatar Jul 03 '20 16:07 bill-phast

@bill-phast I'm not seeing how this is a major pain point, if I am understanding your problem statement correctly; please correct me if I have it wrong. Lombok will not overwrite an existing getter/setter if you define one, it will just ignore code generation for it.

I am currently using the enhanced DynamoDB client with this exact scenario. Example:

@DynamoDbBean
@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
public class ExampleModel {
    private String fieldOne;
    private String fieldTwo;

    @DynamoDbPartitionKey
    public String getFieldOne() {
        return fieldOne;
    }
}

With the Lombok feature mentioned by @ocind this is almost identical to applying the DDB annotation by itself:

@DynamoDbBean
@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
public class ExampleModel {
    @Getter(onMethod_={@DynamoDbPartitionKey})
    private String fieldOne;
    private String fieldTwo;
}

justinsa avatar Aug 05 '20 19:08 justinsa

@justinsa onMethod is experimental feature and according to the page: https://projectlombok.org/features/experimental/onX "Current status: uncertain - Currently we feel this feature cannot move out of experimental status." I wouldn't suggest to use it due to its uncertain status.

Lombok compliant solution would be to allow annotations on field level what for the provided example means:

@DynamoDbBean
@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
public class ExampleModel {

    @DynamoDbPartitionKey
    private String fieldOne;
    private String fieldTwo;
}

Then fieldOne would have getters and setters generated and at the same time fieldOne would be recognised as partition key by AWS SDK in V2.

pawelkaliniakit avatar Aug 05 '20 20:08 pawelkaliniakit

@justinsa - @pawelkaliniakit has it right, with proper annotation support we wouldn't need to explicitly write getters. I think maybe what makes that so painful to us was a decision to use @DynamoDBAttribute heavily (for any field that appears in query expressions) and for a lot of enum usage that means we use @DynamoDBTyped a lot. The end result is that we need a lot of getters that should (thanks to Lombok) not be there.

bill-phast avatar Aug 10 '20 20:08 bill-phast

The 'onMethod' is the current approved workaround for this (I had even put an example in the REAME when coding immutables) :

    @Value
    @Builder
    @DynamoDbImmutable(builder = Customer.CustomerBuilder.class)
    public static class Customer {
        @Getter(onMethod = @__({@DynamoDbPartitionKey}))
        private String accountId;

        @Getter(onMethod = @__({@DynamoDbSortKey}))
        private int subId;  
      
        @Getter(onMethod = @__({@DynamoDbSecondaryPartitionKey(indexNames = "customers_by_name")}))
        private String name;

        @Getter(onMethod = @__({@DynamoDbSecondarySortKey(indexNames = {"customers_by_date", "customers_by_name"})}))
        private Instant createdDate;
    }

Get your point about it being experimental (plus it's ugly), so we'll definitely keep an eye on this. The tricky part is there will undoubtedly also be customers that depend on the current behavior and have properties without getters and setters they would not want to suddenly start having show up in their TableSchema, and I don't want to head down the slippery slope of adding lots of opt-in feature flags for the sake of backwards compatibility which really just leaves us with the option of creating a new introspector entirely... maybe we can figure out one that works both for Kotlin and more elegantly with Lombok at the same time.

bmaizels avatar Sep 05 '20 02:09 bmaizels

The @Builder(toBuilder = true) Lombok builder feature is also incompatible with enhanced dynamodb annotations. This feature generates a method for creating a builder pre-populated with all the fields of the current object. This feature is extremely useful in the context of DynamoDb mapped immutable objects. For example, code for updating an attribute is very easy to read and more resilient to schema changes:

    Customer customer = table.getItem(customerKey);
    Customer updated = customer.toBulder().name(updatedName).build();
    table.putItem(updated);

However, the enhanced DynamoDb validation fails because it detects a 'toBuilder' getter method without an associated setter:

A method was found on the immutable class that does not appear to have a matching setter on the builder class. Use the @DynamoDbIgnore annotation on the method if you do not want it to be included in the TableSchema introspection.

Unfortunately, it is not possible to add an annotation to the 'toBuilder()' method generated by Lombok. The only work-around is to hand-code the copy builder method.

If compatibility with Lombok (a other field-based object frameworks) is being pursued, this issue should be considered as well.

nadernader99 avatar Oct 04 '20 21:10 nadernader99

I'd like to add one more suggestion that is closely related to this. When working with Lombok and @DynamoDbImmutable, I keep having to do this for derived fields:

@DynamoDbImmutable(builder = Customer.CustomerBuilder.class)
@Value
@Builder
public class Customer {

    @DynamoDbAttribute("Type")
    public String getType() {
        return "Customer";
    }

    // "Type" is a derived field (see hand-written getter above). There is no point in exposing it 
    // on the Builder. However, it is required to be on the Builder by the AWS SDK v2 Enhanced 
    // Client, so we must add this here so that Lombok adds it to the Builder
    @Getter(AccessLevel.NONE)
    String type;

    // Here's a normal, non-derived field. Notice that we use Lombok's @NonNull to enforce that 
    // this is set at object creation time, whereas we don't use @NonNull on the "type" field above
    @Getter(onMethod = @__({@DynamoDbAttribute("CustomerId")}))
    @NonNull
    String customerId;

This follows from the docs which state this as a requirement under Working with immutable data classes.

Every getter in the immutable class must have a corresponding setter on the builder class that has a case-sensitive matching name.

So ideally the Enhanced Client would support something like below, where the @DynamoDbDerivedAttribute would tell the SDK not to look for this field on the Builder:

    @DynamoDbAttribute("Type")
    @DynamoDbDerivedAttribute
    public String getType() {
        return "Customer";
    }

helloworldless avatar Nov 24 '20 19:11 helloworldless

I also stumbled onto the issue with toBuilder support. Would be great if you could address it, as it does make a lot of sense when working with immutable classes. 🙏

bigunyak avatar Feb 03 '21 11:02 bigunyak

Same for me, need to get over toBuilder

Bolshem avatar Feb 19 '21 22:02 Bolshem

Another upvote for this feature. It will help keep my project code clean and not verbose.

reply2srij avatar Jul 20 '21 19:07 reply2srij

This feature has even been released?

ecaglar avatar Jul 29 '21 20:07 ecaglar

Also waiting for this feature

ervinpm avatar Sep 23 '21 14:09 ervinpm

@helloworldless

So ideally the Enhanced Client would support something like below, where the @DynamoDbDerivedAttribute would tell the SDK not to look for this field on the Builder:

    @DynamoDbAttribute("Type")
    @DynamoDbDerivedAttribute
    public String getType() {
        return "Customer";
    }

Actually, I have a question about this. Is it not the case that you would either not want a derived field written into the database, or that you would also want it to be read out if it was part of the schema. In the former case @DynamoDbIgnore should suffice, and in the latter case you would need to add a method to the builder that decomposed the derived field so that it could be read from the DB. Maybe I'm misunderstanding your use-case?

bmaizels avatar Sep 24 '21 00:09 bmaizels

The 'Type' attribute (from single table design best practices) is a good example of a static field that should be written to the database but it shouldn't be exposed on the class API. Another one is if you have a partition key named PK, and a sort key named SK. An item might have PK=CUSTOMER#123, SK=A, where PK is derived like PK=CUSTOMER#{CustomerID}. So again the PK and SK are kind of implementation details that shouldn't be exposed on the class API, like on the builder for instance.

helloworldless avatar Sep 24 '21 00:09 helloworldless

Still giving this deep thought. I recently implemented a compound attribute like the second example you describe, and in that case I actually added a method to the builder to read it and decompose it (it also validated it if the attributes it decomposed into were also read/specified as there were different cases depending on projected attributes whether those would be available or not). I guess what we're really talking about is an attribute that gets written to the database but not read like @DynamoDbWriteOnly or something (trying to think of something that is more immediately intuitive than 'derivedAttribute').

bmaizels avatar Sep 24 '21 02:09 bmaizels

@nadernader99 @bigunyak @Bolshem @reply2srij @ecaglar @ervinpm you now have support for toBuilder (see https://github.com/aws/aws-sdk-java-v2/pull/2735). I know this only addresses part of the issue with Lombok, for the other stuff you will still have to use the onMethod annotation and I'll keep this issue open until we've delivered a better solution for that or Lombok stops calling it experimental.

bmaizels avatar Sep 24 '21 02:09 bmaizels

While upgrading from Java 16 to Java 17 I discovered a new issue related to Lombok and Enhanced DynamoDB. The following code reproduces the issue.

@Builder
@DynamoDbImmutable(builder = Example.ExampleBuilder.class)
public class Example {

    @Getter(onMethod = @__(@DynamoDbPartitionKey))
    private String field1;
    @Getter(onMethod = @__(@DynamoDbSecondaryPartitionKey(indexNames = { "field2-index" })))
    private String field2;

}

This code fails to compile while using these version: Java 17.0.1, Lombok 1.18.22, AWS SDK 2.17.56. The compiler error is:

[ERROR] Lombok annotation handler class lombok.javac.handlers.HandleGetter failed on ...\src\main\java\com\temp\Example.java: java.lang.NullPointerException: Cannot assign field "type" because "tree" is null [ERROR] .../src/main/java/com/temp/Example.java:[15,25] cannot find symbol symbol: class __ location: class com.temp.Example

From the observed failures it appears they are caused by the combination of the DynamoDbSecondaryPartitionKey and Getter annotations. I was able to work around the issue by removing the annotation on field2 and replacing it with a full getter implementation as follows:

@Builder
@DynamoDbImmutable(builder = Example.ExampleBuilder.class)
public class Example {

    @Getter(onMethod = @__(@DynamoDbPartitionKey))
    private String field1;
    private String field2;

    @DynamoDbSecondaryPartitionKey(indexNames = { "field2-index" })
    public String getField2() {

        return field2;
    }
}

The previous work done by @bmaizels is very appreciated. I am adding this compile failure since it is related to the overall enhancement request of general DynamoDB/Lombok compatibility.

nadernader99 avatar Oct 19 '21 23:10 nadernader99

Any updates on this issue ?

driverpt avatar Dec 31 '21 15:12 driverpt

While upgrading from Java 16 to Java 17 I discovered a new issue related to Lombok and Enhanced DynamoDB. The following code reproduces the issue. ....

@nadernader99 We've run into this same issue with java 11 and lombok 1.18.22 and aws sdk 2.17.86

texlnghorn avatar Jan 07 '22 18:01 texlnghorn

As of SDK v2.17.121 (and still working in v2.17.226), the following test case works as expected:

import org.junit.jupiter.api.Test;

import lombok.Builder;
import lombok.Data;
import lombok.Getter;
import lombok.ToString;
import lombok.extern.jackson.Jacksonized;
import software.amazon.awssdk.enhanced.dynamodb.TableSchema;
import software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbBean;
import software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbImmutable;

class DynamoEnhancedSchemaTest {

	@Jacksonized
	@Builder(toBuilder = true)
	@DynamoDbImmutable(builder = Foo.FooBuilder.class)
	@Getter
	@ToString
	public static class Foo {
		@Getter(onMethod_ = @DynamoDbPartitionKey)
		private final String s;
		@Getter(onMethod_ = @DynamoDbSortKey)
		private final int i;
		private final boolean b;
	}

	@Jacksonized
	@Builder(toBuilder = true)
	@DynamoDbBean
	@Data
	@ToString
	public static class Bar {
		@Getter(onMethod_ = @DynamoDbPartitionKey)
		private String s;
		@Getter(onMethod_ = @DynamoDbSortKey)
		private int i;
		private boolean b;

		public Bar() {
		}

		public Bar(String s, int i, boolean b) {
			this.s = s;
			this.i = i;
			this.b = b;
		}
	}

	@Test
	void testFoo() {
		TableSchema<Foo> schema = TableSchema.fromClass(Foo.class);
		System.out.println(schema.attributeNames());

		Foo foo = Foo.builder().s("s").i(0).b(true).build().toBuilder().s("ess").build();
		System.out.println(foo);
	}

	@Test
	void testBar() {
		TableSchema<Bar> schema = TableSchema.fromClass(Bar.class);
		System.out.println(schema.attributeNames());

		Bar bar = Bar.builder().s("s").i(0).b(true).build().toBuilder().s("ess").build();
		System.out.println(bar);
	}
}

Am I missing anything?

mbenson avatar Jul 07 '22 17:07 mbenson

I wanted to get the Builder support going as well, especially with some of the DynamoDB Single Table best practices mentioned by @helloworldless What I learned: you can partly customize the Builder generated by Lombok: https://projectlombok.org/features/Builder#:~:text=You%20can%20customize%20parts%20of%20your%20builder

@nadernader99 @bigunyak @Bolshem @reply2srij @ecaglar @ervinpm @bmaizels I hope the following sample if useful for you as well:

Personally I really like that the code to work around the fact you need a setter for a static attribute like Type or a derived attribute like PK is now pushed down to the Builder, which make the core of your Domain Object Class more readable I think.

@Builder
@DynamoDbImmutable(builder = User.UserBuilder.class)
@EqualsAndHashCode
@ToString
public class User {
    public static final String TYPE = "USER";

    @Getter(onMethod = @__({@DynamoDbAttribute("user_id")}))
    private String userId;

    @Getter(onMethod = @__({@DynamoDbAttribute("email")}))
    private String email;

    @DynamoDbPartitionKey
    @DynamoDbAttribute("PK")
    public String getPartitionKey() {
        return "USER#" + userId;
    }

    @DynamoDbSortKey
    @DynamoDbAttribute("SK")
    public String getSortKey() {
        return "PROFILE";
    }

    @DynamoDbAttribute("Type")
    public String getType() {
        return TYPE;
    }

    public static class UserBuilder {
        public void setPartitionKey(String partitionKey) {
            // Do nothing, this is a derived attribute
        }

        public void setSortKey(String sortKey) {
            // Do nothing, this fixed value attribute
        }

        public void setType(String type) {
            // Do nothing, this fixed value attribute
        }
    }
}

arjanschaaf avatar Nov 14 '22 11:11 arjanschaaf

Wish this would be resolved in 2023 as it introduced uneeded boilerplate

skal111 avatar Mar 10 '23 20:03 skal111

A method was found on the immutable class that does not appear to have a matching setter on the builder class.
Use the @DynamoDbIgnore annotation on the method if you do not want it to be included in the TableSchema introspection.

In my case, I faced the above error in Java 17 (with @DynamoDbImmutable and Java record instead of class) and SDK enhanced client was not working with Lombok. The code here is expecting the builder-generated-setters to be named exactly as the propertyName itself, failing which I got this error. I removed the setterPrefix = "with" from the @Builder annotation and it fixed my issue. This situation is not ideal but was a small compromise to make it work, and it avoids boilerplate too.

ritam777 avatar Jul 12 '23 23:07 ritam777