dynogels icon indicating copy to clipboard operation
dynogels copied to clipboard

Table name defaults to lower case of name

Open moraneva opened this issue 8 years ago • 5 comments

Hi there,

I was just curious if there is a good reason for the table name of a model to default to all lower case? I was dealing with a table that had upper case characters and it was a pain trying to figure out what the issue was. Are there some sort of standards somewhere around dynamo table naming? I missed that part in the documentation and it was confusing to me why the table name was being overridden by the library.

Thanks!

moraneva avatar Dec 20 '17 16:12 moraneva

Hi @moraneva,

As far as I know there isn't a "standard" when it comes to naming dynamodb tables. You can modify the table name using a helper:

const eventTable = dynogels.define('event', {
  hashKey: 'id',
  timestamps: true,
  schema,
  indexes,
  tableName: name => name.toUpperCase(),
});

are you suggesting we use the first argument of define as the table name? I'd be happy to see a PR if you're up for it?

Thanks

Clarkie

clarkie avatar Dec 21 '17 10:12 clarkie

Cool, didn't now that you could modify that in the config. Specifically I'm talking about https://github.com/clarkie/dynogels/blob/master/lib/index.js#L67

  const tableName = name.toLowerCase();

  const table = new Table(tableName, schema, serializer, internals.loadDocClient(), log);

So if I didn't set the table name in the schema, it uses the first argument of define which is cast to lower case here. Is there a reason for doing that casting in the define? Because the table name definitely uses this if there is nothing defined in the schema:

Table.prototype.tableName = function () {
  if (this.schema.tableName) {
    if (_.isFunction(this.schema.tableName)) {
      return this.schema.tableName.call(this);
    } else {
      return this.schema.tableName;
    }
  } else {
    return this.config.name;
  }
};

moraneva avatar Dec 21 '17 12:12 moraneva

Personally, I explicitly state the name of the table as a string in the config.

const FooTable = dynogels.define('FooTable', {
    tableName: 'FooTable',
    // ...
});

I found that dynogels tries (tried?) to pluralize the table, for example, and that the best way to ensure that I know exactly what the table will be named is to explicitly name it. This is my current recommendation as a best practice.

IMO, tableName should default to exactly the name of the model, but this is a backwards-incompatible change.

cdhowie avatar Dec 21 '17 19:12 cdhowie

@cdhowie yeah good point, didn't even think about the compatibility of it. Maybe it could be a feature for the next major release?

moraneva avatar Dec 21 '17 19:12 moraneva

I'm fully on board with that. I think that having dynogels munge the table name in any way violates the principle of least surprise. (To be fair, this functionality was inherited from vogels.)

cdhowie avatar Dec 21 '17 19:12 cdhowie