Skip to content

Commit 0786272

Browse files
briancbrianc
committed
Merge pull request brianc#252 from brianc/connection-parameters
Connection parameters
2 parents b9cedb2 + 113b629 commit 0786272

11 files changed

Lines changed: 203 additions & 267 deletions

File tree

‎lib/client.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ var crypto = require('crypto');
22
var EventEmitter = require('events').EventEmitter;
33
var util = require('util');
44

5+
var ConnectionParameters = require(__dirname + '/connection-parameters');
56
var Query = require(__dirname + '/query');
67
var utils = require(__dirname + '/utils');
78
var defaults = require(__dirname + '/defaults');
@@ -10,20 +11,21 @@ var CopyFromStream = require(__dirname + '/copystream').CopyFromStream;
1011
var CopyToStream = require(__dirname + '/copystream').CopyToStream;
1112
var Client = function(config) {
1213
EventEmitter.call(this);
13-
if(typeof config === 'string') {
14-
config = utils.normalizeConnectionInfo(config)
15-
}
14+
15+
this.connectionParameters = new ConnectionParameters(config);
16+
this.user = this.connectionParameters.user;
17+
this.database = this.connectionParameters.database;
18+
this.port = this.connectionParameters.port;
19+
this.host = this.connectionParameters.host;
20+
this.password = this.connectionParameters.password;
21+
1622
config = config || {};
17-
this.user = config.user || defaults.user;
18-
this.database = config.database || defaults.database;
19-
this.port = config.port || defaults.port;
20-
this.host = config.host || defaults.host;
23+
2124
this.connection = config.connection || new Connection({
2225
stream: config.stream,
2326
ssl: config.ssl
2427
});
2528
this.queryQueue = [];
26-
this.password = config.password || defaults.password;
2729
this.binary = config.binary || defaults.binary;
2830
this.encoding = 'utf8';
2931
this.processID = null;

‎lib/connection-parameters.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
var dns = require('dns');
2+
var path = require('path');
3+
14
var defaults = require(__dirname + '/defaults');
25

36
var val = function(key, config) {
@@ -33,6 +36,44 @@ var ConnectionParameters = function(config) {
3336
this.password = val('password', config);
3437
this.binary = val('binary', config);
3538
this.ssl = config.ssl || defaults.ssl;
39+
//a domain socket begins with '/'
40+
this.isDomainSocket = (!(this.host||'').indexOf('/'));
41+
};
42+
43+
var add = function(params, config, paramName) {
44+
var value = config[paramName];
45+
if(value) {
46+
params.push(paramName+"='"+value+"'");
47+
}
48+
};
49+
50+
ConnectionParameters.prototype.getLibpqConnectionString = function(cb) {
51+
var params = []
52+
add(params, this, 'user');
53+
add(params, this, 'password');
54+
add(params, this, 'port');
55+
if(this.database) {
56+
params.push("dbname='" + this.database + "'");
57+
}
58+
if(this.isDomainSocket) {
59+
params.push("host=" + this.getDomainSocketName());
60+
return cb(null, params.join(' '));
61+
}
62+
dns.lookup(this.host, function(err, address) {
63+
if(err) return cb(err, null);
64+
params.push("hostaddr=" + address);
65+
return cb(null, params.join(' '));
66+
});
67+
};
68+
69+
ConnectionParameters.prototype.getDomainSocketName = function() {
70+
var filename = '.s.PGSQL.' + this.port;
71+
72+
//if host is full path to socket fd with port number, just return it
73+
if(this.host.indexOf(filename) > -1) return this.host;
74+
75+
//otherwise, build it from host + standard filename + port
76+
return path.join(this.host, filename);
3677
};
3778

3879
module.exports = ConnectionParameters;

‎lib/native/index.js‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
//require the c++ bindings & export to javascript
22
var EventEmitter = require('events').EventEmitter;
3+
4+
var ConnectionParameters = require(__dirname + '/../connection-parameters');
35
var utils = require(__dirname + "/../utils");
46
var CopyFromStream = require(__dirname + '/../copystream').CopyFromStream;
57
var CopyToStream = require(__dirname + '/../copystream').CopyToStream;
@@ -28,7 +30,7 @@ var nativeConnect = p.connect;
2830

2931
p.connect = function(cb) {
3032
var self = this;
31-
utils.buildLibpqConnectionString(this._config, function(err, conString) {
33+
this.connectionParameters.getLibpqConnectionString(function(err, conString) {
3234
if(err) {
3335
return cb ? cb(err) : self.emit('error', err);
3436
}
@@ -143,13 +145,13 @@ var clientBuilder = function(config) {
143145
connection._queryQueue = [];
144146
connection._namedQueries = {};
145147
connection._activeQuery = null;
146-
connection._config = utils.normalizeConnectionInfo(config);
148+
connection.connectionParameters = new ConnectionParameters(config);
147149
//attach properties to normalize interface with pure js client
148-
connection.user = connection._config.user;
149-
connection.password = connection._config.password;
150-
connection.database = connection._config.database;
151-
connection.host = connection._config.host;
152-
connection.port = connection._config.port;
150+
connection.user = connection.connectionParameters.user;
151+
connection.password = connection.connectionParameters.password;
152+
connection.database = connection.connectionParameters.database;
153+
connection.host = connection.connectionParameters.host;
154+
connection.port = connection.connectionParameters.port;
153155
connection.on('connect', function() {
154156
connection._connected = true;
155157
connection._pulseQueryQueue(true);

‎lib/utils.js‎

Lines changed: 0 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -13,88 +13,6 @@ if(typeof events.EventEmitter.prototype.once !== 'function') {
1313
};
1414
}
1515

16-
var parseConnectionString = function(str) {
17-
//unix socket
18-
if(str.charAt(0) === '/') {
19-
return { host: str };
20-
}
21-
var result = url.parse(str);
22-
var config = {};
23-
config.host = result.hostname;
24-
config.database = result.pathname ? result.pathname.slice(1) : null
25-
var auth = (result.auth || ':').split(':');
26-
config.user = auth[0];
27-
config.password = auth[1];
28-
config.port = result.port;
29-
return config;
30-
};
31-
32-
//allows passing false as property to remove it from config
33-
var norm = function(config, propName) {
34-
config[propName] = (config[propName] || (config[propName] === false ? undefined : defaults[propName]))
35-
};
36-
37-
//normalizes connection info
38-
//which can be in the form of an object
39-
//or a connection string
40-
var normalizeConnectionInfo = function(config) {
41-
switch(typeof config) {
42-
case 'object':
43-
norm(config, 'user');
44-
norm(config, 'password');
45-
norm(config, 'host');
46-
norm(config, 'port');
47-
norm(config, 'database');
48-
return config;
49-
case 'string':
50-
return normalizeConnectionInfo(parseConnectionString(config));
51-
default:
52-
throw new Error("Unrecognized connection config parameter: " + config);
53-
}
54-
};
55-
56-
57-
var add = function(params, config, paramName) {
58-
var value = config[paramName];
59-
if(value) {
60-
params.push(paramName+"='"+value+"'");
61-
}
62-
}
63-
64-
//builds libpq specific connection string
65-
//from a supplied config object
66-
//the config object conforms to the interface of the config object
67-
//accepted by the pure javascript client
68-
var getLibpgConString = function(config, callback) {
69-
if(typeof config == 'object') {
70-
var params = []
71-
add(params, config, 'user');
72-
add(params, config, 'password');
73-
add(params, config, 'port');
74-
if(config.database) {
75-
params.push("dbname='" + config.database + "'");
76-
}
77-
if(config.host) {
78-
if (!config.host.indexOf("/")) {
79-
params.push("host=" + config.host);
80-
} else {
81-
if(config.host != 'localhost' && config.host != '127.0.0.1') {
82-
//do dns lookup
83-
return require('dns').lookup(config.host, function(err, address) {
84-
if(err) return callback(err, null);
85-
params.push("hostaddr="+address)
86-
callback(null, params.join(" "))
87-
})
88-
}
89-
params.push("hostaddr=127.0.0.1 ");
90-
}
91-
}
92-
callback(null, params.join(" "));
93-
} else {
94-
throw new Error("Unrecognized config type for connection");
95-
}
96-
}
97-
9816
//converts values from javascript types
9917
//to their 'raw' counterparts for use as a postgres parameter
10018
//note: you can override this function to provide your own conversion mechanism
@@ -126,12 +44,6 @@ function normalizeQueryConfig (config, values, callback) {
12644
}
12745

12846
module.exports = {
129-
normalizeConnectionInfo: normalizeConnectionInfo,
130-
//only exported here to make testing of this method possible
131-
//since it contains quite a bit of logic and testing for
132-
//each connection scenario in an integration test is impractical
133-
buildLibpqConnectionString: getLibpgConString,
134-
parseConnectionString: parseConnectionString,
13547
prepareValue: prepareValue,
13648
normalizeQueryConfig: normalizeQueryConfig
13749
}

‎src/binding.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ class Connection : public ObjectWrap {
494494
}
495495
if (copied == 0) {
496496
//wait for next read ready
497-
//result was not handled copmpletely
497+
//result was not handled completely
498498
return false;
499499
} else if (copied == -1) {
500500
this->copyOutMode_ = false;
@@ -503,6 +503,7 @@ class Connection : public ObjectWrap {
503503
this->copyOutMode_ = false;
504504
return true;
505505
}
506+
return false;
506507
}
507508
bool HandleResult(PGresult* result)
508509
{
@@ -532,7 +533,7 @@ class Connection : public ObjectWrap {
532533
{
533534
this->copyInMode_ = true;
534535
Emit("copyInResponse");
535-
return false;
536+
return false;
536537
}
537538
break;
538539
case PGRES_COPY_OUT:
@@ -546,6 +547,7 @@ class Connection : public ObjectWrap {
546547
printf("YOU SHOULD NEVER SEE THIS! PLEASE OPEN AN ISSUE ON GITHUB! Unrecogized query status: %s\n", PQresStatus(status));
547548
break;
548549
}
550+
return true;
549551
}
550552

551553
void EmitCommandMetaData(PGresult* result)

‎test/cli.js‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,5 @@
1-
var config = {};
2-
if(process.argv[2]) {
3-
config = require(__dirname + '/../lib/utils').parseConnectionString(process.argv[2]);
4-
}
5-
//TODO use these environment variables in lib/ code
6-
//http://www.postgresql.org/docs/8.4/static/libpq-envars.html
7-
config.host = config.host || process.env['PGHOST'] || process.env['PGHOSTADDR'];
8-
config.port = config.port || process.env['PGPORT'];
9-
config.database = config.database || process.env['PGDATABASE'];
10-
config.user = config.user || process.env['PGUSER'];
11-
config.password = config.password || process.env['PGPASSWORD'];
1+
var ConnectionParameters = require(__dirname + '/../lib/connection-parameters');
2+
var config = new ConnectionParameters(process.argv[2]);
123

134
for(var i = 0; i < process.argv.length; i++) {
145
switch(process.argv[i].toLowerCase()) {

‎test/integration/client/configuration-tests.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
var helper = require(__dirname + '/test-helper');
22
var pg = helper.pg;
33

4+
//clear process.env
5+
var realEnv = {};
6+
for(var key in process.env) {
7+
realEnv[key] = process.env[key];
8+
if(!key.indexOf('PG')) delete process.env[key];
9+
}
10+
411
test('default values', function() {
512
assert.same(pg.defaults,{
613
user: process.env.USER,
@@ -44,3 +51,8 @@ if(!helper.args.native) {
4451
})
4552
}
4653

54+
55+
//restore process.env
56+
for(var key in realEnv) {
57+
process.env[key] = realEnv[key];
58+
}

‎test/integration/client/copy-tests.js‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ test('COPY TO', function () {
5858
});
5959
});
6060
});
61+
6162
test('COPY TO, queue queries', function () {
62-
pg.connect(helper.config, function (error, client) {
63+
if(helper.config.native) return false;
64+
pg.connect(helper.config, assert.calls(function (error, client) {
6365
assert.equal(error, null, "Failed to connect: " + helper.sys.inspect(error));
6466
prepareTable(client, function () {
6567
var query1Done = false,
@@ -73,7 +75,7 @@ test('COPY TO, queue queries', function () {
7375
//imitate long query, to make impossible,
7476
//that copy query end callback runs after
7577
//second query callback
76-
client.query("SELECT pg_sleep(5)", function () {
78+
client.query("SELECT pg_sleep(1)", function () {
7779
query2Done = true;
7880
assert.ok(copyQueryDone && query2Done, "second query has to be executed after others");
7981
});
@@ -93,15 +95,17 @@ test('COPY TO, queue queries', function () {
9395
pg.end(helper.config);
9496
}, "COPY IN stream should emit end event after all rows");
9597
});
96-
});
98+
}));
9799
});
100+
98101
test("COPY TO incorrect usage with large data", function () {
102+
if(helper.config.native) return false;
99103
//when many data is loaded from database (and it takes a lot of time)
100104
//there are chance, that query will be canceled before it ends
101105
//but if there are not so much data, cancel message may be
102106
//send after copy query ends
103107
//so we need to test both situations
104-
pg.connect(helper.config, function (error, client) {
108+
pg.connect(helper.config, assert.calls(function (error, client) {
105109
assert.equal(error, null, "Failed to connect: " + helper.sys.inspect(error));
106110
//intentionally incorrect usage of copy.
107111
//this has to report error in standart way, instead of just throwing exception
@@ -116,10 +120,12 @@ test("COPY TO incorrect usage with large data", function () {
116120
}));
117121
})
118122
);
119-
});
123+
}));
120124
});
125+
121126
test("COPY TO incorrect usage with small data", function () {
122-
pg.connect(helper.config, function (error, client) {
127+
if(helper.config.native) return false;
128+
pg.connect(helper.config, assert.calls(function (error, client) {
123129
assert.equal(error, null, "Failed to connect: " + helper.sys.inspect(error));
124130
//intentionally incorrect usage of copy.
125131
//this has to report error in standart way, instead of just throwing exception
@@ -134,7 +140,7 @@ test("COPY TO incorrect usage with small data", function () {
134140
}));
135141
})
136142
);
137-
});
143+
}));
138144
});
139145

140146
test("COPY FROM incorrect usage", function () {

‎test/unit/client/configuration-tests.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ test('client settings', function() {
44

55
test('defaults', function() {
66
var client = new Client();
7-
assert.equal(client.user, process.env.USER);
8-
assert.equal(client.database, process.env.USER);
7+
assert.equal(client.user, process.env['PGUSER'] || process.env.USER);
8+
assert.equal(client.database, process.env['PGDATABASE'] || process.env.USER);
99
assert.equal(client.port, 5432);
1010
});
1111

@@ -41,11 +41,11 @@ test('initializing from a config string', function() {
4141

4242
test('when not including all values the defaults are used', function() {
4343
var client = new Client("pg://host1")
44-
assert.equal(client.user, process.env.USER)
45-
assert.equal(client.password, null)
44+
assert.equal(client.user, process.env['PGUSER'] || process.env.USER)
45+
assert.equal(client.password, process.env['PGPASSWORD'] || null)
4646
assert.equal(client.host, "host1")
47-
assert.equal(client.port, 5432)
48-
assert.equal(client.database, process.env.USER)
47+
assert.equal(client.port, process.env['PGPORT'] || 5432)
48+
assert.equal(client.database, process.env['PGDATABASE'] || process.env.USER)
4949
})
5050

5151

0 commit comments

Comments
 (0)