blazejkustra / dynamode

Dynamode is a modeling tool for Amazon's DynamoDB
MIT License
61 stars 2 forks source link

Fix: Cannot add multiple attribute decorators for primary key #31

Closed blazejkustra closed 2 months ago

blazejkustra commented 3 months ago

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:

Is this a breaking change?

Other:

blazejkustra commented 3 months ago

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

I'll add some e2e tests next week.

blazejkustra commented 3 months ago

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 https://github.com/blazejkustra/dynamode/pull/31/commits/3edd9548bf2be21d1be998793c3d2ba874e80c69, great find! (I haven't used DynamoDB for a while so I forgot about this use case which is 100% valid)

afronski commented 3 months ago

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

LGTM. 🙇

blazejkustra commented 3 months ago

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

blazejkustra commented 2 months ago

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 commented 2 months ago

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!