Fix: Cannot add multiple attribute decorators for primary key

[blazejkustra]

Summary:

Code sample:

Model

import attribute from '../dist/decorators';
import Entity from '../dist/entity';
import TableManager from '../dist/table';

export class TableUser extends Entity {
  @attribute.gsi.partitionKey.string({ indexName: 'index-1' })
  @attribute.partitionKey.string()
  domain: string;

  @attribute.sortKey.string()
  @attribute.gsi.sortKey.string({ indexName: 'index-1' })
  email: string;

  constructor(data: { domain: string; email: string }) {
    super(data);
    this.domain = data.domain;
    this.email = data.email;
  }
}

export const UserdataTableManager = new TableManager(TableUser, {
  tableName: 'USERS_TABLE_NAME',
  partitionKey: 'domain',
  sortKey: 'email',
  indexes: {
    'index-1': {
      partitionKey: 'domain',
      sortKey: 'email',
    },
  },
});

const EntityManager = UserdataTableManager.entityManager();

General

async function test() {
  const entity = await EntityManager.put(
    new TableUser({
      domain: 'test',
      email: 'not_empty',
    }),
  );
  console.log(entity);
}

async function createTable() {
  const table = await UserdataTableManager.createTable();
  console.log(table);
}

createTable();
test();

GitHub linked issue:

#30

Type of change:

• Bug fix
• Feature implementation
• Documentation improvement
• Testing improvement
• Something not listed here

Is this a breaking change?

• YES 🚨
• No

Other:

• I have searched through the GitHub pull requests to ensure this PR has not already been submitted
• I have updated the Dynamode documentation (if required)
• I have added/updated the Dynamode test cases (if required)
• I agree that all changes made in this pull request may be distributed and are made available in accordance with the Dynamode License.

[blazejkustra]

In case you have any questions fell free to leave them here @afronski 👍

I'll add some e2e tests next week.

[afronski]

One remark: as in the proposed example, the original case from #30 looks good.

However, one of the allowed scenarios does not work:

export class TableUser extends Entity {
  @attribute.partitionKey.string()
  id: string;

  // --> This throws a "role mismatch" - and it's perfectly allowed as a reprojection for GSI. <--
  // "Attribute "type" is decorated with a wrong role in "TableUser" Entity."
  @attribute.sortKey.string()
  @attribute.gsi.partitionKey.string({ indexName: 'index-1' })
  type: string;

  @attribute.gsi.sortKey.string({ indexName: 'index-1' })
  email: string;
  
  // ...
}

[blazejkustra]

One remark: as in the proposed example, the original case from #30 looks good.

However, one of the allowed scenarios does not work:

export class TableUser extends Entity {
  @attribute.partitionKey.string()
  id: string;

  // --> This throws a "role mismatch" - and it's perfectly allowed as a reprojection for GSI. <--
  // "Attribute "type" is decorated with a wrong role in "TableUser" Entity."
  @attribute.sortKey.string()
  @attribute.gsi.partitionKey.string({ indexName: 'index-1' })
  type: string;

  @attribute.gsi.sortKey.string({ indexName: 'index-1' })
  email: string;
  
  // ...
}

Should be fixed with 3edd954, great find! (I haven't used DynamoDB for a while so I forgot about this use case which is 100% valid)

[afronski]

@blazejkustra Thanks for the update - I rechecked the case presented above, and it works as expected now.

LGTM. 🙇

[blazejkustra]

I'll add some e2e tests and merge this week. Thank you!

[blazejkustra]

Update: I found one issue connected to entity metadata caused by this change 😢 I have to rethink how getEntityAttributes is implemented but I have little time this week.

[blazejkustra]

Turned out it is an edge case that I can fix in a different PR. Added remaining tests and merging now. Thanks @afronski for the review!