michaellavelle / spring-data-dynamodb

Simplifies the development of creating an Amazon DynamoDB-based Java data access layer
https://derjust.github.io/spring-data-dynamodb/
Apache License 2.0
169 stars 284 forks source link

Dates formatted by DefaultDynamoDBDateMarshaller are local time with 'Z' suffix #19

Closed rshan2 closed 10 years ago

rshan2 commented 10 years ago

Dates are marshalled and stored correctly in DynamoDB (local times converted to UTC with Z suffix), however, dates that are provided as parameters for queries are converted to local time strings with Z suffix. Consequently, methods like findByUserIdAndTImestampBetween("joe", startDate, endDate) don't work as expected.

Changing the class

public class DefaultDynamoDBDateMarshaller extends AbstractDynamoDBDateMarshaller {

    public DefaultDynamoDBDateMarshaller() {
        super(new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"));
    }
}

to the following resolves the issue

public class DefaultDynamoDBDateMarshaller extends AbstractDynamoDBDateMarshaller {

public DefaultDynamoDBDateMarshaller() {
    super(new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'") {
        private static final long serialVersionUID = 1L;
        {
            setTimeZone(TimeZone.getTimeZone("GMT"));
        }

    });
}
}

The DefaultDynamoDBDateMarshaller is used by AbstractDynamoDBQueryCriteria.addAttributeValue

else if (ClassUtils.isAssignable(Date.class, propertyType)) {
        List<Date> attributeValueAsList = getAttributeValueAsList(attributeValue);
        if (expandCollectionValues && attributeValueAsList != null) {
            List<String> attributeValueAsStringList = getDateListAsStringList(attributeValueAsList);
            attributeValueObject.withNS(attributeValueAsStringList);
        } else {
            Date date = (Date) attributeValue;
            String marshalledDate = new DefaultDynamoDBDateMarshaller().marshall(date);
            attributeValueObject.withS(marshalledDate);
        }
    } 
michaellavelle commented 10 years ago

Great spot Bob - many thanks. The fix worked perfectly :-)

FYI, I also spotted an existing typo in the AbstractDynamoDBQueryCriteria.addAttributeValue method which affected scenario if date collections were registered as criteria ( eg. methods such as findByUserIdAndTImestampIn(...) ). I created and fixed as issue #20 - should be working now.

rshan2 commented 10 years ago

Thank you Michael.

BTW, I noticed that your UTC test cases are fetching the timezone for "UCT". Interestingly, the TimeZone class returns the UTC timezone if it doesn't recognize the ID. Fetching timezone "BLA" yields the same result as "UTC" and "UCT".

@Test
public void addAttributeValueTest_WhenValueIsSingleDate() throws ParseException
{
    // Setup date formats for EST and UCT
    DateFormat estDateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'");
    estDateFormat.setTimeZone(TimeZone.getTimeZone("EST"));
    DateFormat utcDateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'");
    utcDateFormat.setTimeZone(TimeZone.getTimeZone("UCT"));

...
}
michaellavelle commented 10 years ago

Thanks Bob - another good spot. That's interesting about TimeZone defaulting to GMT for unknown ids - just saw that in the docs but hadn't realised that before...Cheers, Michael