shiftcode / dynamo-easy

DynamoDB client for NodeJS and browser with a fluent api to build requests. We take care of the type mapping between JS and DynamoDB, customizable trough typescript decorators.
https://shiftcode.github.io/dynamo-easy/
MIT License
206 stars 27 forks source link

Can't conditionally update custom mapped list of strings with empty value #335

Open LucasDachman opened 3 years ago

LucasDachman commented 3 years ago

Describe the bug

If I have a property with a custom mapper like

@Model()
class MyModel {
  @Property({ mapper: stringArrayOrEmptyMapper })
  myList: string[]
}

export const stringArrayOrEmptyMapper: MapperForType<
  string[],
  ListAttribute<StringAttribute>
> = {
  fromDb: attributeValue =>
    attributeValue?.L?.map((item: { S: string }) => item.S) ?? [],
  toDb: propertyValue =>
    propertyValue?.length
      ? { L: propertyValue.map(item => ({ S: item })) }
      : { L: [] },
};

and I try to do a conditional transact update like:

new TransactUpdate(MyModel, PK, SK)
  .onlyIfAttribute('myList')
  .eq([])
  .updateAttribute('myList)
  .set(['liststuff'])

I get Error: expected 1 value(s) for operator =, this is not the right amount of method parameters for this operator

This works fine if the list is populated but fails when the list is empty

Expected behavior I expect the operation to succeed if the list is empty and fail if the list is not empty. I do not expect an error

LucasDachman commented 3 years ago

Seems like this is due to line 310 in condition-expression-builder.ts. The validator throws if the conditional values length is falsey. Why is this needed?

function validateForOperator(operator: ConditionOperator, values?: any[]) {
  validateArity(operator, values)

  /*
   * validate values if operator supports values
   */
  if (!isFunctionOperator(operator) || (isFunctionOperator(operator) && !isNoParamFunctionOperator(operator))) {
    if (values && Array.isArray(values) && values.length) {
      validateValues(operator, values)
    } else {
      throw new Error(
        dynamicTemplate(ERR_ARITY_DEFAULT, { parameterArity: operatorParameterArity(operator), operator }),
      )
    }
  }
}

Also empty arrays are striped by the deepFilter in buildFilterExpression in condition-expression-builder.ts. Is this necessary?

export function buildFilterExpression(
  attributePath: string,
  operator: ConditionOperator,
  values: any[],
  existingValueNames: string[] | undefined,
  metadata: Metadata<any> | undefined,
): Expression {
  // metadata get rid of undefined values
  values = deepFilter(values, (value) => value !== undefined)

This is legal to DynamoDB

TransactItems: [
   {
      Update: {
        TableName: ...,
        Key: ...,
        ConditionExpression: 'myList = :emptyList',
        UpdateExpression: 'SET myList = :myList',
        ExpressionAttributeValues: {
          ':emptyList': { L: [] },
          ':myList': { L: [{ S: 'value' }] },
        },
      },
    }
]
danielblignaut commented 3 years ago

any progress on this issue? I'm experiencing the same problem which leads to an issue with GraphQL resolvers with nonNull lists

LucasDachman commented 3 years ago

I ended up just using the using the dynamodb client directly for this specific use case.

stvngrsh commented 2 years ago

This problem extends to all actions containing empty arrays, not just condition expressions. I can't see any reason deepFilter doesn't allow empty arrays. They are 100% valid in dynamoDB. Not sure this repo is actively maintained anymore though. Forked repo and fixed here: https://github.com/Fantom-App/dynamo-easy/pull/1

TanjaBayer commented 8 months ago

Any changes the suggested fix will be added into this repo? Would be great 😄