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

No mapping for attribute by name #139

Closed
michielboekhoff opened this issue Feb 14, 2018 · 6 comments
Closed

No mapping for attribute by name #139

michielboekhoff opened this issue Feb 14, 2018 · 6 comments
Labels
Milestone

Comments

@michielboekhoff
Copy link

michielboekhoff commented Feb 14, 2018

Expected Behavior

We've got a Spring repository test that looks like this:

@Test
public void testDeleteByClientId() throws Exception {
    repo.save(ClientCredentials.newInstance(CLIENT_ID));
    assertTrue(repo.findByClientId(CLIENT_ID).isPresent());
    repo.delete(CLIENT_ID);
    assertFalse(repo.findByClientId(CLIENT_ID).isPresent());
}

and a setup that looks like

@Before
public void setUp() throws Exception {
    final DynamoDBMapper mapper = new DynamoDBMapper(db);
    final CreateTableRequest tableRequest = mapper.generateCreateTableRequest(ClientCredentials.class);
    tableRequest.setProvisionedThroughput(new ProvisionedThroughput(1L, 1L));
    db.createTable(tableRequest);
}

and a model like so

@Getter
@Setter
@DynamoDBTable(tableName = "ClientCredentials")
public final class ClientCredentials extends BaseModel {

    @DynamoDBHashKey(attributeName = "ClientId")
    private String clientId;
    @DynamoDBAttribute
    private String hashedSecret;
    @DynamoDBAttribute
    private String description;
    @DynamoDBAttribute
    private String scope;

    public static ClientCredentials newInstance(final String clientId) {
        final ClientCredentials instance = new ClientCredentials();
        instance.clientId = clientId;
        return instance;
    }
}

This test should pass, and did on version 4.4.1.

Actual Behavior

The test currently fails, with a DynamoDBMapperException with the message "ClientCredentials[clientId]: No mapping for attribute by name".

We suspect it's because of a change to getPropertyAttributeValue in AbstractDynamoDBQueryCriteria, where the following block has been added in between 4.4.1 and 4.5.4:

if (tableModel != null) {  // purely here for testing as DynamoDBMapperTableModel cannot be mocked using Mockito
        DynamoDBMapperFieldModel<T,Object> fieldModel = tableModel.field(propertyName);
        if (fieldModel != null) {
            return fieldModel.convert(value);
        }
}

tableModel.field throws the exception, as the DynamoDBMapper has a record only for ClientId, whereas propertyName is clientId. This triggers a DynamoDBMapperException because it cannot find the field.

propertyName is the hashKeyPropertyName, which is clientId. One of the constructor parameters is a DynamoDBEntityInformation, which contains the method getOverriddenAttributeName(String propertyName) that could be used to resolve the name, maybe?

Steps to Reproduce the Problem

  1. Create a Spring repository test
  2. Witness failure

Specifications

  • Spring Data DynamoDB Version: 4.5.4
  • Spring Data Version: 1.13.10.RELEASE
  • AWS SDK Version: 1.11.275
  • Java Version: 8
  • Platform Details: Windows 7
@derjust
Copy link
Owner

derjust commented Feb 14, 2018

Thanks for reporting. From your analytics it looks like it is a 'duplicate' of #118 on the v4.5.x branch.
Will have a look

@derjust derjust added the bug label Feb 14, 2018
@michielboekhoff
Copy link
Author

michielboekhoff commented Feb 14, 2018 via email

@derjust
Copy link
Owner

derjust commented Feb 14, 2018

The comment re: testing was introduced for testing the Auditing support (#26).

There is already a method to get the (overriden) attribute name for a given property - no need to store more if I remember correctly. Will try to have a look later.

@XutaoZhang
Copy link

Hello,

I seen that this bug is fixed in a branch (4.5.x) with Spring 4 dependencies, but we can't find any release with that fixes for Spring 4.
We wondering if you can make a new release for Spring 4, merging the 4.5.x branch. We are using Maven for dependencies managment.

Thank you very much.
Kind regards.
Tao

@derjust
Copy link
Owner

derjust commented Mar 7, 2018

Version v4.5.5 released under the old coordinates com.github.derjust:spring-data-dynamodb as also the new coordinates com.github.spring-data-dynamodb:spring-data-dynamodb

Please allow up to 2h to sync Maven Central

@derjust derjust closed this as completed Mar 7, 2018
@derjust derjust added this to the v4.5.5 milestone Mar 7, 2018
@XutaoZhang
Copy link

Thank you for so quick response.

PD: You are doing a great job.

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

No branches or pull requests

3 participants