Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v5.0.1 Regression] Query Creation Uses Attribute Name instead of Column Name #118

Closed
pluttrell opened this issue Jan 23, 2018 · 12 comments
Closed
Labels
Milestone

Comments

@pluttrell
Copy link

pluttrell commented Jan 23, 2018

I'm migrating a Spring Boot 1.5.9 project to Spring Boot 2.0 and thus have upgraded to com.github.derjust:spring-data-dynamodb:5.0.1. This project contains several tables that use contrived keys and we map them like this:

@Id
private CustomerDocumentId customerDocumentId;

@DynamoDBHashKey(attributeName = "customerId|documentType")
public String getCustomerDocumentKey() {
    if (customerDocumentId == null) {
      return null;
    }
    return customerDocumentId.getCustomerDocumentKey();
}

public void setCustomerDocumentKey(String customerDocumentId) {
    if (customerDocumentId == null) {
      this.customerDocumentId = new CustomerDocumentId();
    }
    customerDocumentId.setCustomerDocumentKey(customerDocumentId);
}

In case you're wondering why we have contrived keys like this, it has to do with DynamoDB's limit to two keys per table.. And we have a separate column for the Range key. I just omitted it for brevity.

In our CrudRepository we have a query method such as this (among others):

@EnableScan
public interface CustomerDocumentRepository extends CrudRepository<CustomerDocument, CustomerDocumentId> {
    List<CustomerDocument> findByCustomerDocumentKey(String customerDocumentKey);
}

Notice how we've defined the entity property customerDocumentKey to map the column name customerId|documentType.

The store operation works perfectly, but when we lookup records using findByCustomerDocumentKey we get this error:

com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMappingException: CustomerDocument[customerDocumentKey]; no mapping for attribute by name
	at com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel.field(DynamoDBMapperTableModel.java:94) ~[aws-java-sdk-dynamodb-1.11.268.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQueryCriteria.getPropertyAttributeValue(AbstractDynamoDBQueryCriteria.java:501) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQueryCriteria.withHashKeyEquals(AbstractDynamoDBQueryCriteria.java:395) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.DynamoDBEntityWithHashAndRangeKeyCriteria.withPropertyEquals(DynamoDBEntityWithHashAndRangeKeyCriteria.java:369) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQueryCreator.addCriteria(AbstractDynamoDBQueryCreator.java:129) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQueryCreator.create(AbstractDynamoDBQueryCreator.java:66) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQueryCreator.create(AbstractDynamoDBQueryCreator.java:40) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.springframework.data.repository.query.parser.AbstractQueryCreator.createCriteria(AbstractQueryCreator.java:115) ~[spring-data-commons-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.data.repository.query.parser.AbstractQueryCreator.createQuery(AbstractQueryCreator.java:94) ~[spring-data-commons-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.data.repository.query.parser.AbstractQueryCreator.createQuery(AbstractQueryCreator.java:80) ~[spring-data-commons-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.socialsignin.spring.data.dynamodb.repository.query.PartTreeDynamoDBQuery.doCreateQuery(PartTreeDynamoDBQuery.java:63) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQuery.doCreateQueryWithPermissions(AbstractDynamoDBQuery.java:76) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQuery$CollectionExecution.execute(AbstractDynamoDBQuery.java:98) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQuery.execute(AbstractDynamoDBQuery.java:294) ~[spring-data-dynamodb-5.0.1.jar:na]
	at org.springframework.data.repository.core.support.RepositoryFactorySupport$QueryExecutorMethodInterceptor.doInvoke(RepositoryFactorySupport.java:597) ~[spring-data-commons-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.data.repository.core.support.RepositoryFactorySupport$QueryExecutorMethodInterceptor.invoke(RepositoryFactorySupport.java:580) ~[spring-data-commons-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.2.RELEASE.jar:5.0.2.RELEASE]
	at org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.invoke(DefaultMethodInvokingMethodInterceptor.java:59) ~[spring-data-commons-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.2.RELEASE.jar:5.0.2.RELEASE]
	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92) ~[spring-aop-5.0.2.RELEASE.jar:5.0.2.RELEASE]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.2.RELEASE.jar:5.0.2.RELEASE]
	at org.springframework.data.repository.core.support.SurroundingTransactionDetectorMethodInterceptor.invoke(SurroundingTransactionDetectorMethodInterceptor.java:61) ~[spring-data-commons-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185) ~[spring-aop-5.0.2.RELEASE.jar:5.0.2.RELEASE]
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:212) ~[spring-aop-5.0.2.RELEASE.jar:5.0.2.RELEASE]
	at com.sun.proxy.$Proxy110.findByCustomerDocumentKey(Unknown Source) ~[na:na]
	at com.mycompany.CustomerDocumentRepository.findByCustomerDocumentKey(CustomerDocumentRepository.java:52) ~[classes/:na]

In looking at this in the debugger, this line of code org.socialsignin.spring.data.dynamodb.repository.query.AbstractDynamoDBQueryCriteria.getPropertyAttributeValue(..) on line 501 is calling the AWS SDK's DynamoDBMapperTableModel::field but sending in the attribute name, which in this case is customerDocumentKey and the AWS SDK only knows about the actual column names, which in this case is customerId|documentType. I've confirmed that the actual column name exists in the fields Map within DynamoDBMapperTableModel.

@derjust
Copy link
Owner

derjust commented Jan 23, 2018

Interesting approach. Just to be clear here: You say it was working with spring-data-dynamodb in the 4.5.x brach but with 5.0.x (and the updated Spring) it doesn't work anymore?

Your analysis makes a lot of sense - it's just that I do not recall to change anything with that regard from 4.5 -> 5.0. Nevertheless want to fix it.

@pluttrell pluttrell changed the title [v5.0.1] Query Creation Uses Attribute Name instead of Column Name [v5.0.1 Regression] Query Creation Uses Attribute Name instead of Column Name Jan 23, 2018
@pluttrell
Copy link
Author

pluttrell commented Jan 23, 2018

Correct, this is a regression between 4.5.x and 5.0.x. Here's a stripped down (and somewhat contrived) example app: pluttrell/spring-data-dynamodb-issue-118. It contains a branch that works on 1.5.9 and 2.0.0, which does not work.

@pluttrell
Copy link
Author

pluttrell commented Jan 23, 2018

Also the bug doesn't appear to be related to our rather odd column names with pipe characters, but rather whenever the column name differs from the field name. This will also trigger the bug:

@DynamoDBHashKey(attributeName = "theHashKey")
public String getCustomerDocumentKey() {
    ...
}

@derjust
Copy link
Owner

derjust commented Jan 23, 2018 via email

@derjust derjust added this to the v5.0.2 milestone Jan 23, 2018
derjust added a commit that referenced this issue Jan 26, 2018
@derjust
Copy link
Owner

derjust commented Jan 26, 2018

Transfered the testcase into its own PR: #123
Can reproduce the behavior with Spring 5 - but also with Spring 4

@pluttrell May you provide a mvn dependency:tree from which spring or AWS fies a loaded (|grep om.amazonaws)

@pluttrell
Copy link
Author

@derjust I'm not sure what's different about how we're running it, but here's what I see when I run both branches of my example project: https://gist.github.com/pluttrell/4f263ace8053eb3038fd96e4e436aa67.

@pluttrell
Copy link
Author

@derjust And here's the dependency tree for both branches: https://gist.github.com/pluttrell/0117793ed15c698b58077f90b3d97517. Not sure what specific versions your looking for, but hope this helps.

@pluttrell
Copy link
Author

I also just tested with the version of the aws sdk that shipped yesterday with both branches and they both exhibit the same problem.

compile 'com.amazonaws:aws-java-sdk-core:1.11.271'
compile 'com.amazonaws:aws-java-sdk-dynamodb:1.11.271'

@derjust derjust added this to To Do in 5.0.2 Bugfix release Jan 31, 2018
derjust added a commit that referenced this issue Feb 2, 2018
derjust added a commit that referenced this issue Feb 2, 2018
Issue #118: Query Creation Uses Attribute Name instead of Column Name
@derjust
Copy link
Owner

derjust commented Feb 2, 2018

Still unsure why it worked in the first place - classic I guess 🤣
Will be part of 5.0.2

@derjust derjust closed this as completed Feb 2, 2018
5.0.2 Bugfix release automation moved this from To Do to Done Feb 2, 2018
@pluttrell
Copy link
Author

Many thanks for the fix. Wondering if you have a target timeframe for 5.0.2? I'm not trying to push or anything, just curious.

@derjust
Copy link
Owner

derjust commented Feb 2, 2018

I'd love to get all the issues marked as 'bug' solved with 5.0.2 - as the list of bugs is quite stable (luckily).
If you really need to get an 'official upstream' build to please some gods and make your life easier we can talk. Otherwise I try to not swamp MavenCentral with releases :)

@pluttrell
Copy link
Author

Sure thing. I really wasn’t trying to push, just to see if there was a timeframe so we can plan accordingly. And in looking at the bugs, it doesn’t look like it’ll be too long, so I think we can wait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

2 participants