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
204 stars 27 forks source link

BatchWriteRequest should not require modelClazz parameter in put method #342

Open LouDou opened 3 years ago

LouDou commented 3 years ago

Is your feature request related to a problem? Please describe.

I don't really understand why, in particular, the BatchWriteRequest.put() method requires passing in the modelClazz argument before the array of models.

I would like to be able to pass in an array of mixed Model types, and have the class correctly determine where to write each one.

Describe the solution you'd like

It is quite straightforward to find the constructor from a JS object, using obj.constructor. Therefore, internally, the BatchWriteRequest class can determine the constructor object upon which to read metadata about the Model.

I've tested this alternative interface, and it works as such:

import * as DE from '@shiftcoders/dynamo-easy';

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
class AutoBatchWrite extends DE.BatchWriteRequest {
  protected itemCount = 0;

  putMixed(models: unknown[]) {
    if (this.itemCount + models.length > DE.BATCH_WRITE_MAX_REQUEST_ITEM_COUNT) {
      throw new Error(`batch write takes at max ${DE.BATCH_WRITE_MAX_REQUEST_ITEM_COUNT} items`);
    }

    for (const model of models) {
      const meta = Reflect.getMetadata('sc-reflect:model', model.constructor);
      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
      // @ts-ignore
      const item = { PutRequest: { Item: DE.toDb(model, model.constructor) } };
      const tableName = meta.tableName;
      this.params.RequestItems[tableName] = this.params.RequestItems[tableName] || [];
      this.params.RequestItems[tableName].push(item);
    }
    this.itemCount += models.length;

    return this;
  }
}

Note that the ts/eslint comments are required here for this proof because: a) the properties required to make this work are private inside BatchWriteRequest - this would be moot if this implementation was in BatchWriteRequest to start with b) model.constructor for T = unknown type doesn't match ModelConstructor<T> - I'm not completely sure about the solution for this one. Particularly because there's no such thing as a base model type, and the type of object in the models array can be mixed.

Additional context

That's about it... I love this library, by the way. Even if this improvement is not considered, I can still make good use of it :)

Regards, Doug.

LouDou commented 3 years ago

Here's the full working PoC: https://github.com/LouDou/de-mm-poc

de-mm-poc$ yarn
de-mm-poc$ yarn start
$ curl -XPOST -H'Content-Type: application/json' -d'{}' http://localhost:3000/local/hello