API reference needed

[bitinn]

I spent a longtime hunting for a way to close my connection (using sqlite), it seems connection is not exposed to end user, so how can I exit an one-time migration script gracefully?

(besides process.exit(0); which works, but it's ignoring the store connection)

[PhilWaldmann]

Hi @bitinn

call store.close() - I've just realized that it was missing in the docs and not implemented for sqlite3 (knex.js did not support it back then). Update to v. 2.2.2 and this will automatically exit your script:

const Store = require('openrecord/store/sqlite3')

const store = new Store({
  file: './test.sqlite3',
  migrations: [...]
})

store.ready(() => {
  console.log('DONE!')
  store.close()
})

[bitinn]

Thx Phil, do you think it's a good idea to expose knex underneath?

Also, how does it set multiple column as a single unique key? https://knexjs.org/#Schema-unique

[PhilWaldmann]

store.close() calls the knex.js close function. Similar to queries or migrations. I don't see a problem here.

Compound primary keys are not yet supported in openrecord migrations. But implementing it should be no big deal. Give me a moment

[PhilWaldmann]

With version 2.3 you could add multiple primary keys via:

this.createTable('test', {id: false}, function(){
  this.integer('foo', {primary: true})
  this.integer('bar', {primary: true})
})

[bitinn]

Thx again! My suggestion to expose knex was really to stop me from nagging you every time something in knex is missing from openrecord.

Speaking of which: is the current list of supported column attributes/modifiers up-to-date in the doc?

• You listed 4 (primary, unique, default, notnull) in https://openrecord.js.org/#/migrations
• I think notnull is a typo and should probably be not_null.
• And what about index? https://knexjs.org/#Chainable

I will keep this issue open for a bit and see if I run into other doc/api related issue.

[PhilWaldmann]

Yeah, migrations definitely need some more love! notnull is correct, but I agree with you: it should be named not_null or null.

[bitinn]

That’s weird as your code use not_null?

(From my phone)

On Oct 3, 2018, at 20:07, Philipp Waldmann notifications@github.com wrote:

Yeah, migrations definitely need some more love! notnull is correct, but I agree with you: it should be named not_null or null.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[PhilWaldmann]

it uses both

if (options.notnull || options.not_null) { ...}

but in the docs it's just notnull.

[bitinn]

Another doc issue: I don't think the default example works on node v10

https://openrecord.js.org/#/setup

It seems require('openrecord') always default to postgres even with type set to sqlite3 and file pointing to the correct file, and it throws:

> node setup.js

internal/modules/cjs/loader.js:583
    throw err;
    ^

Error: Cannot find module 'pg'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:581:15)
    at Function.Module._load (internal/modules/cjs/loader.js:507:25)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/Users/df/git/animeshot2/node_modules/openrecord/lib/stores/postgres/connection.js:2:12)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/Users/df/git/animeshot2/node_modules/openrecord/lib/stores/postgres/index.js:4:3)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)

Issue exists at least between 2.2 and 2.4 releases.

A similar config with require('openrecord/store/sqlite3') did work for me, so perhaps a bug somewhere

[bitinn]

Another thing: I know openrecord doesn't support rollback yet, but I am thinking of how to handle a database reset script (mostly development testing sake), I would like to:

a. tell openrecord to not remember a specific migration script b. or run removeTable outside of migration context

Are there any solution other than trying to deletion everything in openrecord_migrations table?

[PhilWaldmann]

Thanks, I'll take a look at the require('openrecord') and Error: Cannot find module 'pg' bug!

Regarding rollbacks: My plan is to do the opposite actions on a rollback (createTable -> dropTable). For cases where this is not possible. e.g. if you do an this.raw('some SQL...') you have to define a down method. So there will be a migration with a single function (like now) and a migration similar to this:

module.exports = {
  up(){
    this.raw('some SQL...')
  },

  down(){
    this.raw('some SQL for undo...')
  }
}

But to support that, openrecord will need a cli, similar to Rails or ecto. Which will remove the need for the store´s migrations option. You'll instead use

$ openrecord migrate # run missing migrations
$ openrecord rollback # with options --steps <n> arg
$ openrecord reset # rollback everything and migrate again

[bitinn]

@PhilWaldmann about migration, I am not looking for a rollback mechanism, if I am I would be using knex directly.

I am more interested in a simple option to prevent a migration script from being recorded in openrecord_migrations table. So that I can run them multiply times (like database seeding, for example).

What I currently does is to remove the entry from openrecord_migrations after migration is ran, it's an ok workaround.

[bitinn]

Another question: from reading the doc it's not clear to me whether relations are setup automatically. For example, if I have POSTS.user_id which has a reference/foreign key on USERS.id column, what relation is being created when autoLoad: true?

The relation will be initialized after the target model is ready - to automatically get the primary and foreign key. The default for the foreign key is _ - all lower case! You could manually set the from and to key, if you need.

In short, I don't quite understand this line.

[bitinn]

Perhaps more importantly, can I inspect the relations of an autoloaded model??

[bitinn]

Yet another question: I always use db.Model('users') (aka the actual table name) but it seems I should use db.Model('User'), the doc doesn't appear to explain the drawbacks of using table name clearly (in terms of relation definition).

[bitinn]

Also, there should be a mention of toJson in the doc: when working with relations and templates, we couldn't just access post.author.username as it's a Promise. It's often much easier to use toJson and convert the result into a plain object.

(alternatively, we can use post._author.username but it's kinda weird to expose this detail in the template).

Accessing a relation via it's name (e.g. threads[0].posts) will always return a promise. If you use include() this promise will resolve instantly! If you need, you can access the preloaded results directly by adding a underscore: threads[0]._posts

Also this line can be misleading as include() here refer to the original query for threads.

[bitinn]

I am sure you also notice the custom inspect function is now causing warning like this in node v10:

(node:2389) [DEP0079] DeprecationWarning: Custom inspection function on Objects via .inspect() is deprecated

To be honest I don't think the custom inspect function is very useful here: they hide object details when you console.log(posts[0]), but object details might be exactly what a user is trying to look at.

To be fair, one can disable this behavior globally with this:

const util = require('util');
util.inspect.defaultOptions.customInspect = false;

But I argue perhaps the custom inspect shouldn't be used by default in the first place?

[PhilWaldmann]

I am more interested in a simple option to prevent a migration script from being recorded in openrecord_migrations table. So that I can run them multiply times (like database seeding, for example).

What I currently does is to remove the entry from openrecord_migrations after migration is ran, it's an ok workaround.

Sorry for the delay! There is currently no mechanism to run a migration multiple times. Could you give me an example, why you would run your migrations/seedings multiple times...

[PhilWaldmann]

Another question: from reading the doc it's not clear to me whether relations are setup automatically. For example, if I have POSTS.user_id which has a reference/foreign key on USERS.id column, what relation is being created when autoLoad: true?

The relation will be initialized after the target model is ready - to automatically get the primary and foreign key. The default for the foreign key is _ - all lower case! You could manually set the from and to key, if you need.

In short, I don't quite understand this line.

Relation setup is never automatic (unlike model loading via autoLoad store config). Openrecord currently does not read db field references (on my todo list). If you define a relation e.g. this.belongsTo('user') the model definition will check if a model User and the field user_id exists. If that's the case, it will connect these two models via a relation.

If you want to give your relation another name. e.g. author the you need to provide additional options: this.belongsTo('author', {model: 'User', from: 'creator_id', to: 'id'})

[PhilWaldmann]

Yet another question: I always use db.Model('users') (aka the actual table name) but it seems I should use db.Model('User'), the doc doesn't appear to explain the drawbacks of using table name clearly (in terms of relation definition).

Both is fine. It will convert the string internally to a normalized form

[PhilWaldmann]

Also, there should be a mention of toJson in the doc: when working with relations and templates, we couldn't just access post.author.username as it's a Promise. It's often much easier to use toJson and convert the result into a plain object.

(alternatively, we can use post._author.username but it's kinda weird to expose this detail in the template).

Accessing a relation via it's name (e.g. threads[0].posts) will always return a promise. If you use include() this promise will resolve instantly! If you need, you can access the preloaded results directly by adding a underscore: threads[0]._posts

Also this line can be misleading as include() here refer to the original query for threads.

Thanks! I've updated the docs to make it more clear.

[PhilWaldmann]

I am sure you also notice the custom inspect function is now causing warning like this in node v10:

(node:2389) [DEP0079] DeprecationWarning: Custom inspection function on Objects via .inspect() is deprecated

To be honest I don't think the custom inspect function is very useful here: they hide object details when you console.log(posts[0]), but object details might be exactly what a user is trying to look at.

To be fair, one can disable this behavior globally with this:

const util = require('util');
util.inspect.defaultOptions.customInspect = false;

But I argue perhaps the custom inspect shouldn't be used by default in the first place?

I've implemented the custom inspect function to hide all the internal attributes and focus on the important parts of your records. I'm not happy with it, but at the time of writing it was the only solution.

[bitinn]

Thx for following up!

I just ran into another question:

Say I have a bookmarks table with individual entries, and a posts table with a column bookmark_count. What's the best approach to insert a bookmark entry and increment the bookmark_count?

Is it do-able with relation?

If not, is there a shorthand for incrementing a field?

(EDIT: actually, since we are using models, do you recommend setting the model and then .save() instead?)

[PhilWaldmann]

I don't recommend to increment the bookmark_count field outside of your database. The best way is to use a stored procedure triggered by an AFTER INSERT. However, this option is not available for sqlite (no support for stored procedures).

Another way would be to calculate the count on every select (via custom select or a view)

If you need to do it in nodejs, the easiest way would be to use the afterCreate() hook inside your Bookmark model to update the corresponding Post record.

[bitinn]

@PhilWaldmann another question: is there an API that returns the actual query string? I would like to run the query separately to see what indexes are being used for debugging purpose.

[PhilWaldmann]

there is an internal toSQL method. But it's used only for testing thus far

[bitinn]

@PhilWaldmann I think this is a bug:

const shots = await shotModel.where({ text_romanized_like: text }).order('created', true).limit(limit, offset).include('user');

I am seeing this error:

Error: Can't find a operator 'romanized_like' for attribute 'text'

Which I think means you are making an incorrect assumption about no _ in attributes?

[bitinn]

Apologize for another question: Is it possible to express the has_bookmarked with relations?

Imagine this scenario: given a post and a user, there is a chance a bookmark exists that point to both user.id and post.id.

I have setup relation between post, user, bookmark correctly, and can load any bookmark along with post using include()

Problem is, I don't actually need these bookmark data, all I need is whether user has bookmarked said post. Basically, count bookmark where user_id = user.id and post_id = post.id

Is the best way simply to count the include('bookmark').where({ bookmark: { user_id: id } }) result in code? Or can I somehow express count with include/where?

[PhilWaldmann]

@PhilWaldmann I think this is a bug:

const shots = await shotModel.where({ text_romanized_like: text }).order('created', true).limit(limit, offset).include('user');

I am seeing this error:

Error: Can't find a operator 'romanized_like' for attribute 'text'

Which I think means you are making an incorrect assumption about no _ in attributes?

Hey,

do you have a column named text in your model? I'll need more code to understand your problem.

[bitinn]

do you have a column named text in your model? I'll need more code to understand your problem.

Ah yes, I forgot about that :)

I had both text_romanized and text column.

[PhilWaldmann]

I've enhanced custom relations and added some docs. With that you should be able to add a has_bookmarked relation which returns true or false and works with include('has_bookmarked').
Update to version 2.5

I had both text_romanized and text column.

Ahh, okay. I'll add some tests! ;)

[PhilWaldmann]

Error: Can't find a operator 'romanized_like' for attribute 'text'

I've found the problem.
Fixed with version 2.5.1

Thanks

[bitinn]

Thx! I am closing this as my side project is done, no more issues from me for now :)