tywalch / electrodb

A DynamoDB library to ease the use of modeling complex hierarchical relationships and implementing a Single Table Design while keeping your query code readable.
MIT License
997 stars 63 forks source link

ExpressionAttributeNames/Values can include hyphens #321

Closed metuuu closed 11 months ago

metuuu commented 11 months ago

Describe the bug I have a map that include keys that contain hyphens. DynamoDB throws error when I try to update the map values. \"ExpressionAttributeNames contains invalid key: Syntax error; key: \"#Cwxpw5TMn06X1-kscvMxS\"

It seems that electrodb uses the map keys as the ExpressionAttribute keys without removing special characters or hashing the keys.

For example updating

attribute: {
  type: "map",
  properties: {
    'example-key XXX _ 1 2 3 4': { type: "string" },
  }
}

Outputs

"UpdateExpression": "SET #map.#example-key XXX _ 1 2 3 4 = :example-key XXX _ 1 2 3 4_u0, ...

ElectroDB Version 2.10.5

ElectroDB Playground Link Use the ElectroDB Playground

tywalch commented 11 months ago

Hi @metuuu 👋

First of all, how dare you.

Second of all, this is a good find.

In most cases the library strives limit the surface area of the validation and constraints it adds. This is typically because DynamoDB changes their own limits and constraints all the time, so it wouldn't be right for the library to try match/duplicate those.

I'll note this change see about improving the formatting of these expression attributes/names 👍

Does DynamoDB even allow for spaces in their field names? I've never tried.

metuuu commented 11 months ago

My example was a bit over the top I agree 😄 I'm sure you don't have to worry about spaces.

Thank you!

metuuu commented 11 months ago

Just to note that in my actual case I was storing images to an object and the image IDs were generated with nanoid which can include hyphens and underscore. UUIDs would also have the same issue.

images: {
  // { <image-id>: <s3-key> }
  type: CustomAttributeType<{ [imageId: string]: string }>>('any'),
}
tywalch commented 11 months ago

I just published the fix with 2.10.6. You can use the same playground link you created to see the change in action

We can close this ticket when you've verified your use case 👍

metuuu commented 11 months ago

All seems good! I agree with your solution, that if someone wants to shoot themselves in the leg by trying to store two items like test-item & testitem to same map its completely their own fault.

Thank you 🙏🏻