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

Feature: Adds createdAt, updatedAt and siteId columns in postgres #45

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions postgres/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const { POSTGRES_USER, POSTGRES_PASSWORD, POSTGRES_HOST, POSTGRES_PORT, POSTGRES
{ parseOrNot, wrapInObject, decode } = require('../services/utils'),
{ findSchemaAndTable, wrapJSONStringInObject } = require('../services/utils'),
knexLib = require('knex'),
{ isList, isUri } = require('clayutils'),
{ isList, isUri, isPage } = require('clayutils'),
TransformStream = require('../services/list-transform-stream'),
META_PUT_PATCH_FN = patch('meta');
var knex, log = require('../services/log').setup({ file: __filename });
Expand Down Expand Up @@ -130,10 +130,17 @@ function columnToValueMap(column, value, obj = {}) {
*/
function put(key, value) {
const { schema, table } = findSchemaAndTable(key),
map = columnToValueMap('id', key); // create the value map
map = columnToValueMap('id', key), // create the value map
isPublicEntity = isPage(key) || isList(key) || isUri(key),
parsedValue = parseOrNot(value);

if (isPublicEntity && parsedValue.siteSlug) {
// add site id column to map
columnToValueMap('site_id', parsedValue.siteSlug, map);
}

// add data to the map
columnToValueMap('data', wrapInObject(key, parseOrNot(value)), map);
columnToValueMap('data', wrapInObject(key, parsedValue.data || parsedValue), map);

let url;

Expand All @@ -144,7 +151,6 @@ function put(key, value) {
columnToValueMap('url', url, map);
}


return onConflictPut(map, schema, table)
.then(() => map.data);
}
Expand Down Expand Up @@ -207,15 +213,23 @@ function del(key) {
* @param {[type]} ops [description]
* @return {[type]} [description]
*/
/* eslint complexity: 0 */
function batch(ops) {
var commands = [], url;

for (let i = 0; i < ops.length; i++) {
for (let i = 0, opsLength = ops.length; i < opsLength; i++) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using opsLength here and not ops.length?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done for loop optimization, we assign the value of length to a property to avoid having to look it up at every iteration. You can read more about it here: https://jaysoo.ca/2009/12/23/javascript-optimizing-loops/

let { key, value } = ops[i],
{ table, schema } = findSchemaAndTable(key),
map = columnToValueMap('id', key);
map = columnToValueMap('id', key),
isPublicEntity = isPage(key) || isList(key) || isUri(key),
parsedValue = isPublicEntity ? parseOrNot(value) : { data: value };

if (isPublicEntity && parsedValue.siteSlug) {
// add site id column to map
columnToValueMap('site_id', parsedValue.siteSlug, map);
}

columnToValueMap('data', wrapJSONStringInObject(key, value), map);
columnToValueMap('data', wrapJSONStringInObject(key, parsedValue.data), map);

// add url column to map if putting a uri
if (isUri(key)) {
Expand Down
27 changes: 24 additions & 3 deletions postgres/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,27 @@ describe('postgres/client', () => {
expect(data).toEqual(data);
});
});

test('inserts a row with a column for site id', () => {
const key = 'nymag.com/_pages/sample-article',
tableName = 'pages',
putData = { data, siteSlug: 'nymag' };

client.setClient(knex);

return client.put(key, putData).then((data) => {
expect(table.mock.calls.length).toBe(1);
expect(table.mock.calls[0][0]).toBe(tableName);
expect(insert.mock.calls.length).toBe(1);
expect(insert.mock.calls[0][0]).toEqual({ id: key, data, site_id: 'nymag' });
expect(queryBuilder.mock.calls.length).toBe(1);
expect(update.mock.calls.length).toBe(1);
expect(raw.mock.calls.length).toBe(1);
expect(raw.mock.calls[0][0]).toBe('? ON CONFLICT (id) DO ? returning *');
expect(raw.mock.calls[0][1]).toEqual(['insert sql', 'update sql']);
expect(putData.data).toEqual(data);
});
});
});

describe('putMeta', () => {
Expand Down Expand Up @@ -674,11 +695,11 @@ describe('postgres/client', () => {
ops = [
{
key: 'nymag.com/_uris/someinstance',
value: 'nymag.com/_pages/someinstance'
value: { data: 'nymag.com/_pages/someinstance', siteSlug: 'nymag' }
},
{
key: 'nymag.com/_uris/someotherinstance',
value: 'nymag.com/_pages/someotherinstance'
value: { data: 'nymag.com/_pages/someotherinstance', siteSlug: 'nymag' }
}
];

Expand All @@ -694,7 +715,7 @@ describe('postgres/client', () => {

for (let index = 0; index < results.length; index++) {
expect(table.mock.calls[index][0]).toBe('uris');
expect(insert.mock.calls[index][0]).toEqual({ id: ops[index].key, data: results[index], url: decode(ops[index].key.split('/_uris/').pop()) });
expect(insert.mock.calls[index][0]).toEqual({ id: ops[index].key, data: results[index], url: decode(ops[index].key.split('/_uris/').pop()), site_id: 'nymag' });
expect(raw.mock.calls[index][0]).toBe('? ON CONFLICT (id) DO ? returning *');
expect(raw.mock.calls[index][1]).toEqual(['insert sql', 'update sql']);
}
Expand Down
33 changes: 16 additions & 17 deletions postgres/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ function createTables() {
return bluebird.all(getComponents().map(component => client.createTable(`components.${component}`)))
.then(() => bluebird.all(getLayouts().map(layout => client.createTableWithMeta(`layouts.${layout}`))))
.then(() => client.createTableWithMeta('pages'))
.then(() => client.raw('CREATE TABLE IF NOT EXISTS ?? ( id TEXT PRIMARY KEY NOT NULL, data TEXT NOT NULL, url TEXT );', ['uris']))
.then(() => createRemainingTables());
}

Expand All @@ -59,24 +60,22 @@ function setup(testPostgresHost) {
}

return client.connect()
.then(() => {
return migrate(
{
database: POSTGRES_DB,
user: POSTGRES_USER,
password: POSTGRES_PASSWORD,
host: postgresHost,
port: POSTGRES_PORT
},
path.join(__dirname, '../services/migrations')
);
})
.then(() => {
log('info', 'Migrations Complete');
})
.then(() => createTables())
.then(() => client.createSchema('components'))
.then(() => client.createSchema('layouts'))
.then(createTables)
.then(() => migrate(
{
database: POSTGRES_DB,
user: POSTGRES_USER,
password: POSTGRES_PASSWORD,
host: postgresHost,
port: POSTGRES_PORT
},
path.join(__dirname, '../services/migrations')
))
.then(() => log('info', 'Migrations Complete'))
.then(() => ({ server: `${postgresHost}:${POSTGRES_PORT}` }))
.catch(logGenericError);
.catch(logGenericError(__filename));
}

module.exports.setup = setup;
17 changes: 13 additions & 4 deletions redis/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ const bluebird = require('bluebird'),
Redis = require('ioredis'),
{ REDIS_URL, REDIS_HASH } = require('../services/constants'),
{ isPublished, isUri, isUser } = require('clayutils'),
{ notFoundError, logGenericError } = require('../services/errors');
{ notFoundError, logGenericError } = require('../services/errors'),
{ parseOrNot } = require('../services/utils');
var log = require('../services/log').setup({ file: __filename });

/**
Expand Down Expand Up @@ -50,7 +51,11 @@ function shouldProcess(key) {
function put(key, value) {
if (!shouldProcess(key)) return bluebird.resolve();

return module.exports.client.hsetAsync(REDIS_HASH, key, value);
const data = isUri(key)
? parseOrNot(value).data || value
: value;

return module.exports.client.hsetAsync(REDIS_HASH, key, data);
}

/**
Expand All @@ -69,7 +74,7 @@ function get(key) {
}

/**
* [batch description]
* Makes a batch operation to redis
* @param {[type]} ops
* @return {[type]}
*/
Expand All @@ -84,8 +89,12 @@ function batch(ops) {
let { key, value } = ops[i];

if (shouldProcess(key)) {
const data = isUri(key)
? parseOrNot(value).data || value
: value;

batch.push(key);
batch.push(value);
batch.push(data);
}
}

Expand Down
1 change: 0 additions & 1 deletion services/migrations/001_create_components_schema.sql

This file was deleted.

7 changes: 7 additions & 0 deletions services/migrations/001_create_timestamp_trigger.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE OR REPLACE FUNCTION trigger_set_timestamp()
RETURNS TRIGGER AS $$
BEGIN
NEW.updated_at = NOW();
RETURN NEW;
END;
$$ LANGUAGE plpgsql;
8 changes: 8 additions & 0 deletions services/migrations/002_add_created_at_uris_table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
ALTER TABLE IF EXISTS uris
ADD COLUMN IF NOT EXISTS created_at TIMESTAMPTZ DEFAULT NOW(),
ADD COLUMN IF NOT EXISTS updated_at TIMESTAMPTZ DEFAULT NOW();

CREATE TRIGGER set_timestamp
BEFORE UPDATE ON uris
FOR EACH ROW
EXECUTE PROCEDURE trigger_set_timestamp();
1 change: 0 additions & 1 deletion services/migrations/002_create_layouts_schema.sql

This file was deleted.

16 changes: 16 additions & 0 deletions services/migrations/003_add_created_at_lists_table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
CREATE OR REPLACE FUNCTION trigger_set_timestamp()
RETURNS TRIGGER AS $$
BEGIN
NEW.updated_at = NOW();
RETURN NEW;
END;
$$ LANGUAGE plpgsql;

ALTER TABLE IF EXISTS lists
ADD COLUMN IF NOT EXISTS created_at TIMESTAMPTZ DEFAULT NOW(),
ADD COLUMN IF NOT EXISTS updated_at TIMESTAMPTZ DEFAULT NOW();

CREATE TRIGGER set_timestamp
BEFORE UPDATE ON lists
FOR EACH ROW
EXECUTE PROCEDURE trigger_set_timestamp();
1 change: 0 additions & 1 deletion services/migrations/003_create_pages_table.sql

This file was deleted.

8 changes: 8 additions & 0 deletions services/migrations/004_add_created_at_pages_table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
ALTER TABLE IF EXISTS pages
ADD COLUMN IF NOT EXISTS created_at TIMESTAMPTZ DEFAULT NOW(),
ADD COLUMN IF NOT EXISTS updated_at TIMESTAMPTZ DEFAULT NOW();

CREATE TRIGGER set_timestamp
BEFORE UPDATE ON pages
FOR EACH ROW
EXECUTE PROCEDURE trigger_set_timestamp();
1 change: 0 additions & 1 deletion services/migrations/004_create_uris_table.sql

This file was deleted.

8 changes: 8 additions & 0 deletions services/migrations/005_add_created_at_users_table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
ALTER TABLE IF EXISTS users
ADD COLUMN IF NOT EXISTS created_at TIMESTAMPTZ DEFAULT NOW(),
ADD COLUMN IF NOT EXISTS updated_at TIMESTAMPTZ DEFAULT NOW();

CREATE TRIGGER set_timestamp
BEFORE UPDATE ON users
FOR EACH ROW
EXECUTE PROCEDURE trigger_set_timestamp();
2 changes: 2 additions & 0 deletions services/migrations/006_add_site_id_column_pages.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE IF EXISTS pages
ADD COLUMN IF NOT EXISTS site_id VARCHAR(255);
2 changes: 2 additions & 0 deletions services/migrations/007_add_site_id_column_lists.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE IF EXISTS lists
ADD COLUMN IF NOT EXISTS site_id VARCHAR(255);
2 changes: 2 additions & 0 deletions services/migrations/008_add_site_id_column_uris.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE IF EXISTS uris
ADD COLUMN IF NOT EXISTS site_id VARCHAR(255);