search

choutianxius / lucia-adapter-dynamodb

A DynamoDB adapter for lucia-auth
MIT License
3 stars 2 forks source link

Feature/remove extra calls #3

Closed erikrj closed 5 months ago

erikrj commented 5 months ago

The getSessionAndUser methods on DynamoDBAdapter1 and DynamoDBAdapter2 are making two calls to DynamoDB and only need to make one. I think it's reasonable to require ProjectionType set to all for the Gsi and you already have this in your README example. Removing this call cuts the potential costs associated with DynamoDB when using this adapter and it will improve performance.

choutianxius commented 5 months ago

Hi @erikrj , user attributes are not necessarily contained in session records, as there can be custom user attributes, e.g., roles, bios, addresses, ...

You may want to replicate all user data in sessions, but this is also problematic because

  1. These user data can only be passed at your application level by lucia.createSession(userId, sessionAttributes). The adapter should only be responsible for persisting the data as is that are passed to it in setSession(databaseSession), not creating or mutating them;
  2. If some user data are mutable, you have to implement some logic to cascadingly update all relevant session records to ensure consistency when you update your user record, which again is not the concern of the adapter.

Again, please run the tests to check whether the update works with docker compose run --rm app npm run test.

erikrj commented 5 months ago

Thanks. I didn't understand how lucia was handling users. I'll make an alternative modification.