Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
LucasDachman opened this issue Oct 17, 2020 · 5 comments

Comments

@LucasDachman
Copy link

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
Copy link
Author

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
Copy link

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

@LucasDachman
Copy link
Author

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

@stvngrsh
Copy link

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: Fantom-App#1

TanjaBayer added a commit to cubesoftorg/dynamo-easy that referenced this issue Jul 24, 2022
@TanjaBayer
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants