Fix default related table id name when using schema's

[7kasper]

• Related Issues: Handling schemas #458
• Previous PRs: Add support for schema's when using withRelated. #1983

Introduction

This is a fix for the break in the proper naming-convention that happens when using database schema's in bookshelf.

Motivation

Models use a certain table name. Name convention has it (which bookshelf uses as default) that the primary id of this table is formatted to be "tablename_id". This is good. When describing a database (for instance with Postgres) that uses tables across multiple database schema's however the current implementation breaks. Why? Well to use Schema's effectively in bookshelf we can put it in front of the tablename when defining our model (as also discussed in #458). When we do this we can reference tables properly and everything works well, but the default naming convention for the ids needed to construct the queries breaks. Our previously used "tablename_id" now becomes the non standard "schema.tablename_id" because bookshelf thinks schema. is part of the tablename. We should not have to make the name of the id field be "schema.tablename_id" or specifically enter the id (see alternative solution) for bookshelf to work. Instead bookshelf should in this instance (getting the tables primary id by the default name) filter out the schema if those are used.

How I found the issue

Click to reveal

1. Make a database (in postgres)

• Make the table organisations and the table locations in the public schema.
• Make sure they both have a id field (primary key) according to the name convention: 'tablename_id'

2. Make the sample models:

// Model - Definition of Organisation.
const shelf = require('..').shelf;
require('./locations');
module.exports = shelf.model('organisations', {
    tableName: 'organisations',
    locations: function() {
        return this.hasMany('locations');
    }
});
// Model - Definition of Location.
const shelf = require('..').shelf;
require('./organisations');
module.exports = shelf.model('locations', {
    tableName: 'locations',
    organisation: function () {
        return this.belongsTo('organisations');
    }
});

3. Test the solution. It should work as expected.
4. Now we move the tables to their own database schema. (organisation, location).
5. The models don't work anymore, tables can't be found.
6. Of course we need to reference this in the models as well:

// Model - Definition of Organisation.
const shelf = require('..').shelf;
require('./locations');
module.exports = shelf.model('organisations', {
    tableName: 'organisation.organisations', // <- Changed
    locations: function() {
        return this.hasMany('locations');
    }
});
// Model - Definition of Location.
const shelf = require('..').shelf;
require('./organisations');
module.exports = shelf.model('locations', {
    tableName: 'location.locations', // <- Changed
    organisation: function () {
        return this.belongsTo('organisations');
    }
});

7. While fetching now the individual tables work without issue.
8. When trying to fetch the relations everything breaks. The SQL errors because "location.locations" does not contain the field "location.location_id". It only contains "location_id".

Proposed solution

The current solution filters out any "schema." when generating a default name for referencing the ID field of a model.

Current PR Issues

No. On the previous PR some issues were already solved and it was stated that some tests were required. Apart from my personal tests with postgres and mysql which have not revealed any issues (this is a quite simple fix) perhaps it is good to have some unit tests. Although I am not sure (how) this is required.

Alternatives considered

The only way to make schema's work with the default ID naming convention currently is to use the full method (and thus specifically force the ID):
return this.belongsTo('organisations'); becomes return this.belongsTo('organisations', organisation_id');

[7kasper]

bump 🤔