Dynamo DB Mapper

[millems]

Review the inherited state of V1 Dynamo DB mapper support and determine which changes are necessary for V2.

(Feel free to comment on this issue with desired changes).

[cjkent]

Please consider supporting immutable classes in the mapper.

DynamoDbMapper requires all mapped classes to be mutable. Immutable domain objects are increasingly the norm in Java, and using DynamoDbMapper means creating mutable duplicates of all domain objects or giving up on the benefits of immutability.

[MikeFHay]

Please remove PaginatedList, and replace uses of it with Stream.

java.util.List implementations are expected to have fast size() methods, but as far as I can tell there is no way to implement that for a DynamoDB scan or query. Currently PaginatedList will either load the entire scan result into memory on a size() call, or simply fail if configured as ITERATION_ONLY. This is a potentially surprising behaviour which can be made much more intuitive and explicit by returning a Stream and requiring users to collect(toList()) if they want a List.

[millems]

We're looking to make the automatic depagination provided by PaginatedList work for all service's paginated APIs with this story: #26.

+1 to avoiding accidentally blowing up your memory and holding up a thread by using something as simple as size(). Using Stream is a solid idea. We'll need to think about whether other pagination strategies for different APIs can have an efficient size() method. If so, we shouldn't expose it in as innocuous a form for services like Dynamo that make it expensive.

[leadVisionary]

One of the things I'd call out here is that I think there's a problem with the com.amazonaws.services.dynamodbv2.datamodeling.StandardBeanProperties method of reflecting over an object. Say I have:

@DynamoDBTable(tableName = "Polyps")
public final class Polyp {
private Set<String> endpoints = new ConcurrentSkipListSet<>();
public Set<String> getEndpoints() {
        return endpoints;
    }
    public void setEndpoints(final Collection<String> endpoints) {
        if (endpoints != null && !endpoints.isEmpty()) {
            this.endpoints = new ConcurrentSkipListSet<>(endpoints);
        }
    }
}

In theory, even if DynamoDB only respects Sets, I should be able to handle the case where a setter allows a wider Collection type.

It looks like this is because of line 140-146, the setterOf method, which does a "clobber get with set and look for the exact type" algorithm. Consequently, this method returns null, which later becomes a problem during line 111 of DynamoDBMapperFieldModel.

I think it should be checking if the return type has parent types, and if so trying to run up the tree.

[humanzz]

With the current DynamoDBMapper implementation, when performing scans, there's no easy way to make the scan operation respect the DynamoDB table's read throughputs.
Solutions combining https://aws.amazon.com/blogs/developer/rate-limited-scans-in-amazon-dynamodb/ and https://aws.amazon.com/blogs/developer/understanding-auto-paginated-scan-with-dynamodbmapper/ have to be used and are not that straightforward.

Please provide a scan operation that is able to respect a table's read throughput (whether by explicitly specifying a percentage of throughput to use or an absolute value of read units to use).

[spfink]

DynamoDBMapper feature requests from v1:

aws/aws-sdk-java#214
aws/aws-sdk-java#527
aws/aws-sdk-java#534
aws/aws-sdk-java#547
aws/aws-sdk-java#674
aws/aws-sdk-java#832
aws/aws-sdk-java#953
aws/aws-sdk-java#1170
aws/aws-sdk-java#1201
aws/aws-sdk-java#1235
aws/aws-sdk-java#1253

[ryonday]

Compatibility Between DynamoDBMapper, DynamoDB and AmazonDynamoDB

So this is pretty irritating. Within the Java DynamoDB SDK, there are essentially three different ways of working with Dynamo; all of them have their specific uses for specific situations, yet it's really difficult to convert between the different sets of classes that each one uses:

• DynamoDBMapper uses annotated domain classes.
• AmazonDynamoDB uses various forms of Map<String, AttributeValue>
• DynamoDB uses Item and so forth.

Each of these requires irritating bespoke code to convert. Sure, DynamoDBMapper has two methods (marshallIntoObject and marshallIntoObjects), but there is no corresponding unmarshallIntoMap or anything like that. It should be simple to convert between domain classes and Map<String, AttributeValue>. It should be simple to convert between domain classes and Item. It should be simple to convert between any two of these.

Primarily, since DynamoDBMapper just doesn't handle Update Expressions at all, there is no elegant method of interop between DynamoDBMapper and when you need to use them.

Unexpected and Confusing Behavior of DynamoDBMapper Type Conversion

This one is long.

At the very least, the documentation could use clarification here.

But, the best case scenario is that the various methods of type conversion work together harmoniously so that the representation of data in Dynamo when using DynamoDBMapper can best match the user's intent and use case. Right now things are kind of close, which for me is more irritating than were it nowhere near at all.

I have an entire project full of nothing but code examples and JUnit tests to document and remind myself of the oddities of and tips and tricks for this system. I am more than happy to share them at any time.

Anyway, Let's start from the top.

At Least Three Methods of Type Conversion (and one kind of)?

I count three methods of explicit type conversion in the DynamoDBMapper system, and one outlier.

1. ConversionSchema
2. DynamoDBTypeConverterFactory
3. The DynamoDBTyped and DynamoDBTypeConverted family of annotations
4. AttributeTransformer (Yes, the use case for this is somewhat tangental but there are some improvements to be made here too.)

Throughout this section, I'll use the java.time.Instant class as an example for two reasons:

1. In the 1.11.x SDK, it has no native support.

2. Crucially, it can have a number of valid representations depending on the context.

   • A String/S, for UTC timestamps.
   • A DynamoDB M for separating out different aspects of a point on the timeline.
   • A Long/Integer/N for Epoch milli
   • A Long/Integer/N for Epoch second (hello, DynamoDB TTL!)

   Each of these representations has a valid use case. Each of these could even conceivably be used within the same project or even DynamoDBMapper domain class.

ConversionSchema

What I've gathered from looking through the code, the ConversionSchema is intended to define the base "primitive", direct type mappings between Java types and DynamoDB types by doing a class-to-AttributeValue mapping. For instance byte, integer and family map to AttributeValue.withN(thingie.toString()) and so forth.

So the neat thing is that you can add new primitive types to your mapper using a DynamoDBMapperConfig. Let's do it and default to a UTC timestamp!

public class InstantArgumentPal implements ArgumentMarshaller, ArgumentUnmarshaller {
    @Override
    public AttributeValue marshall(Object obj) { return new AttributeValue(obj.toString()); }
    
    @Override
    public void typeCheck(AttributeValue value, Method setter) { // Why? When?  }

    @Override
    public Object unmarshall(AttributeValue value) throws ParseException { return Instant.parse(value.getS()); }
}

I cannot for the life of me figure out when typeCheck is useful. Using it, you cannot tell the mapper "this Unmarshaller is not suitable for this AtributeValue for this Method" except by throwing an exception, which just kills your entire operation. Can this return a boolean?

Regardless, let's use our Pal!

// Omit getters, setters, equals, hashcode, etc etc
@DynamoDBTable(tableName = "instant_conversion_example")
public class InstantConversionExample {
    @DynamoDBHashKey(attributeName = "hash_key")
    private Instant hashKey;
    @DynamoDBAttribute(attributeName = "whatever")
    private String whatever;
 }

Later, let's set up a DynamoDBMapperConfig.Builder.

InstantArgumentPal pal = new InstantArgumentPal();
.withConversionSchema(ConversionSchemas.v2Builder("v2WithInstant")
                .addFirstType(Instant.class, pal, pal)
                .build());

Let's give it a whirl!

 @Test
    public void go_go_gadget_instant() throws Exception {
        InstantConversionExample example = new InstantConversionExample()
            .setHashKey(Instant.now())
            .setWhatever("whatevs");
        mapper.save(example);
        InstantConversionExample example1 = mapper.load(InstantConversionExample.class, example.getHashKey());
        assertThat(example1).isEqualTo(example);
	//
    	// Caused by: com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMappingException: InstantConversionExample[hash_key]; only scalar (B, N, or S) type allowed for key
    	//
    	// (┛◉Д◉)┛彡┻━┻
    }

sigh

@DynamoDBTable(tableName = "instant_conversion_example")
public class InstantConversionExample {
    @DynamoDBHashKey(attributeName = "hash_key")
    private String hashKey;
    @DynamoDBAttribute(attributeName = "whatever")
    private Instant whatever;
    public String getHashKey() {
        return hashKey;
    }

 @Test
    public void go_go_gadget_instant() throws Exception {
        InstantConversionExample example = new InstantConversionExample()
            .setHashKey("hash")
            .setWhatever(Instant.now());
        mapper.save(example);
        InstantConversionExample example1 = mapper.load(InstantConversionExample.class, "hash");
        assertThat(example1).isEqualTo(example);
	// IT WORKS NOW!
    }

OK cool, so far so good. Basically neat but:

• Why aren't ArgumentMarshaller and ArgumentUnmarshaller generic?
• What is typeCheck good for?
• The addFirst is basically addOnly, from what I've been able to tell.
• Technically, I have defined Instant as a scalar in my first example, yet it does not work for hash keys.

The last point tells me that my own additions to the ConversionSchema are not first class citizens. They should be.

DynamoDBTypeConverterFactory

Let's play around with the DynamoDBTypeConverterFactory!

public class InstantStringConverter extends DynamoDBTypeConverter.AbstractConverter<String, Instant> {
    @Override
    public String convert(Instant instant) {
        return instant.toString();
    }
    @Override
    public Instant unconvert(String string) {
        return Instant.parse(string);
    }
}

OK so this seems a little cleaner. It's generic! Let's skip the set-up and tests this time and go directly to the spoilers:

• Still cannot use Instant as a hash key
• Basically works exactly the same as the ConversionSchema thingie but it's generic.

But what's the big problem here?

DynamoDBTypeConverterFactory Is Only Meaningful for String

What happens if I make say, DynamoDBTypeConverter.AbstractConverter<Long, Instant>? Nothing. No conversion happens.

This is especially important if I register, say two converters:

 .withTypeConverterFactory(DynamoDBTypeConverterFactory.standard()
                .override()
                .with(Long.class, Instant.class, new InstantLongConverter())
                .with(String.class, Instant.class, new InstantStringConverter())
                .build());

And annotate the Instant a certain way:

  @DynamoDBTyped(DynamoDBMapperFieldModel.DynamoDBAttributeType.N)
  private Instant instant;

Since I've registered a type converter for Instant for a Java type that directly corresponds to a DynamoDB N type, it stands to reason that the type converter factory would look for a converter for the domain class type that converts to a Java numeric type, right?

No. What happens is this:

1. The TypeConverterFactory explicitly searches for a String converter for the domain class type
2. The TypeConverterFactory attempts to coerce the output of the String converter to a number type.

That is to say, for @DynamoDBTyped(DynamoDBMapperFieldModel.DynamoDBAttributeType.N) to work, There must be a String converter registered for the type and the output of the converter must be a Number formatted as a String

this is extremely counter-intuitive. It severely limits the utility of DynamoDBTypeConverterFactory.

But it gets worse. It turns out that ConversionSchema and DynamoDBTypeConverterFactory do not play well at all:

If a ConversionSchema Marshaller/Unmarshaller pair is registered for a type, the DynamoDBTypeConverterFactory converters for that type are completely ignored, as are any @DynamoDBTyped or @DynamoDBTypeConverted annotations on domain class members for that type.

Also, you cannot register a converter for a non-built-in scalar type (map, etc).

My final comment about this is that it appears that scalar types for this system are hardcoded along with their marshalers and unmarshallers into an Enum. This makes it impossible to extend the DynamoDB type system in a nuanced, elegant way.

The DynamoDBTyped and DynamoDBTypeConverted Family

These work OK, they are just really verbose, and honestly I wish that I could just have more nuanced usage of DynamoDBTypeConverterFactory.

AttributeTransformer

Only comment here is that you should be able to register these on a per-class basis, similar to the ConversionSchema and DynamoDBTypeConverterFactory marshalers and unmarshallers.

Summing Up

• Documentation around the available type conversion options is limited and confusing. There is little or no guidance around which one to use in which situation.
• The interoperation between the type conversion options is confusing and counter-intuitive.
• The behavior of DynamoDBTypeConverterFactory is confusing, counter-intuitive and limiting.
• Because of the above, DynamoDBTyped is of seriously limited utility for non-built-in scalar types.
• DynamoDBTypeConverted with explicit converters works, but the verbosity is irritating
• There should be a way to define your own primitives/scalar types and map them to DynamoDB types.

AttributeValue Is Irritating

new AttributeValue.withN(someNumberType.toString()) // WHYYYYYYY

Please, please, please add static methods that do null checks.

• AttributeValue.S(String s)
• AttributeValue.N(Number n)
• AttributeValue.N(String s)
• AttributeValue.mk(Object) // Introspect argument in a documented way and "do the right thing"

[millems]

Issue reported external to github: @DynamoDBGeneratedUuid does not work for nested list objects.

@DynamoDBTable(tableName="XYZ")
class A {
    @DynamoDBAttribute
    List<B> listOfB;
}

@DynamoDBDocument
class B {
    @DynamoDBGeneratedUuid(DynamoDBAutoGenerateStrategy.CREATE)
    UUID id;
} 

[millems]

Issue reported external to github: It would be nice to be able to annotate either the get method OR the type for type-specific annotations like @DynamoDBTypeConverted. Currently, the method has to be annotated.

[millems]

Issue reported external to github: We should support automatic conversion of non-string key values in maps.