Skip to content

Commit

Permalink
[CONJS-257] ensuring importSql stability when using callback that per…
Browse files Browse the repository at this point in the history 
…mits using commands during execution
  • Loading branch information
@rusher
rusher committed May 29, 2023
1 parent 652b8cf commit fa7d42e
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 103 deletions.
6 changes: 2 additions & 4 deletions callback.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ module.exports.importFile = function importFile(opts, callback) {
conn
.connect()
.then(() => {
return new Promise(function (res, rej) {
return conn.importFile(Object.assign({ skipDbCheck: true }, opts), res, rej);
});
return new Promise(conn.importFile.bind(conn, Object.assign({ skipDbCheck: true }, opts)));
})
.then(() => cb())
.catch((err) => cb(err))
.finally(() => {
new Promise(conn.end.bind(conn, new CommandParameter())).catch(() => {});
new Promise(conn.end.bind(conn, new CommandParameter())).catch(console.log);
});
} catch (err) {
cb(err);
Expand Down
2 changes: 1 addition & 1 deletion lib/connection-callback.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ class ConnectionCallback {
* @param cb callback
*/
importFile(opts, cb) {
if (!opts) {
if (!opts || !opts.file) {
if (cb)
cb(
Errors.createError(
Expand Down
2 changes: 1 addition & 1 deletion lib/connection-promise.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class ConnectionPromise {
* @param opts JSON array with 2 possible fields: file and database
*/
importFile(opts) {
if (!opts) {
if (!opts || !opts.file) {
return Promise.reject(
Errors.createError(
'SQL file parameter is mandatory',
Expand Down
65 changes: 38 additions & 27 deletions lib/connection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,7 @@ class Connection extends EventEmitter {

importFile(cmdParam, resolve, reject) {
const conn = this;
if (!cmdParam.file) {
if (!cmdParam || !cmdParam.file) {
return reject(
Errors.createError(
'SQL file parameter is mandatory',
Expand All @@ -1325,11 +1325,26 @@ class Connection extends EventEmitter {
);
}

const prevAddCommand = this.addCommand.bind(conn);

this.waitingAuthenticationQueue = new Queue();
this.addCommand = this.addCommandQueue;
const tmpQuery = function (sql, resolve, reject) {
const cmd = new Query(
resolve,
(err) => {
if (conn.opts.logger.error) conn.opts.logger.error(err);
reject(err);
},
conn.opts,
new CommandParameter(sql, null, {})
);
prevAddCommand(cmd);
};

let prevDatabase = null;
return (
cmdParam.skipDbCheck
? Promise.resolve()
: new Promise(conn.query.bind(conn, new CommandParameter('SELECT DATABASE() as db', null, {})))
cmdParam.skipDbCheck ? Promise.resolve() : new Promise(tmpQuery.bind(conn, 'SELECT DATABASE() as db'))
).then((res) => {
prevDatabase = res ? res[0].db : null;
if (
Expand All @@ -1349,11 +1364,22 @@ class Connection extends EventEmitter {
);
}
const searchDbPromise = cmdParam.database
? new Promise(
conn.query.bind(conn, new CommandParameter(`USE \`${cmdParam.database.replace(/`/gi, '``')}\``, null, {}))
)
? new Promise(tmpQuery.bind(conn, `USE \`${cmdParam.database.replace(/`/gi, '``')}\``))
: Promise.resolve();
return searchDbPromise.then(() => {
const endingFunction = () => {
if (conn.status < Status.CLOSING) {
conn.addCommand = conn.addCommandEnable.bind(conn);
if (conn.status < Status.CLOSING && conn.opts.pipelining) {
conn.addCommand = conn.addCommandEnablePipeline.bind(conn);
}
const commands = conn.waitingAuthenticationQueue.toArray();
commands.forEach((cmd) => {
conn.addCommand(cmd);
});
conn.waitingAuthenticationQueue = null;
}
};
return fsPromises
.open(cmdParam.file, 'r')
.then(async (fd) => {
Expand All @@ -1362,15 +1388,6 @@ class Connection extends EventEmitter {
offset: 0,
end: 0
};
let endingFunction = () => {};
if (conn.status < Status.CLOSING) {
conn.addCommand = conn.addCommandEnable.bind(conn);
if (conn.status < Status.CLOSING && conn.opts.pipelining) {
endingFunction = () => {
conn.addCommand = conn.addCommandEnablePipeline;
};
}
}

const queryPromises = [];
let cmdError = null;
Expand All @@ -1388,12 +1405,7 @@ class Connection extends EventEmitter {
Promise.allSettled(queryPromises)
.then(() => {
if (!cmdParam.skipDbCheck && cmdParam.database && cmdParam.database != prevDatabase) {
return new Promise(
conn.query.bind(
conn,
new CommandParameter(`USE \`${prevDatabase.replace(/`/gi, '``')}\``, null, {})
)
);
return new Promise(tmpQuery.bind(conn, `USE \`${prevDatabase.replace(/`/gi, '``')}\``));
}
return Promise.resolve();
})
Expand All @@ -1410,11 +1422,9 @@ class Connection extends EventEmitter {
buf.end += res.bytesRead;
const queries = Parse.parseQueries(buf);
const queryIntermediatePromise = queries.flatMap((element) => {
return new Promise(conn.query.bind(conn, new CommandParameter(element, null, null))).catch(
(err) => {
cmdError = err;
}
);
return new Promise(tmpQuery.bind(conn, element)).catch((err) => {
cmdError = err;
});
});

queryPromises.push(...queryIntermediatePromise);
Expand Down Expand Up @@ -1462,6 +1472,7 @@ class Connection extends EventEmitter {
}
})
.catch((err) => {
endingFunction();
if (err.code === 'ENOENT') {
return reject(
Errors.createError(
Expand Down
6 changes: 2 additions & 4 deletions promise.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,10 @@ module.exports.importFile = function importFile(opts) {
return conn
.connect()
.then(() => {
return new Promise(function (res, rej) {
return conn.importFile(Object.assign({ skipDbCheck: true }, opts), res, rej);
});
return new Promise(conn.importFile.bind(conn, Object.assign({ skipDbCheck: true }, opts)));
})
.finally(() => {
new Promise(conn.end.bind(conn, new CommandParameter()));
new Promise(conn.end.bind(conn, new CommandParameter())).catch(console.log);
});
} catch (err) {
return Promise.reject(err);
Expand Down
66 changes: 0 additions & 66 deletions test/tools/data-dump2.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -940,72 +940,6 @@ INSERT INTO `gis_multi_point` VALUES ('\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0
/*!40000 ALTER TABLE `gis_multi_point` ENABLE KEYS */;
UNLOCK TABLES;

--
-- Table structure for table `gis_multi_point_batch`
--

DROP TABLE IF EXISTS `gis_multi_point_batch`;
/*!40101 SET @saved_cs_client = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `gis_multi_point_batch` (
`g` multipoint DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
/*!40101 SET character_set_client = @saved_cs_client */;

--
-- Dumping data for table `gis_multi_point_batch`
--

LOCK TABLES `gis_multi_point_batch` WRITE;
/*!40000 ALTER TABLE `gis_multi_point_batch` DISABLE KEYS */;
INSERT INTO `gis_multi_point_batch` VALUES ('\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0>@\0\0\0\0\0\0>@\0\0\0\0\0\0\0\0\0$@\0\0\0\0\0\0$@\0\0\0\0\0\0\0\0\0$@\0\0\0\0\0\04@\0\0\0\0\0\0\0\0\04@\0\0\0\0\0\04@'),('\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0$@\0\0\0\0\0\0\0\0'),('\0\0\0\0\0\0\0\0\0\0\0'),('\0\0\0\0\0\0\0\0\0\0\0');
/*!40000 ALTER TABLE `gis_multi_point_batch` ENABLE KEYS */;
UNLOCK TABLES;

--
-- Table structure for table `gis_multi_point_insert`
--

DROP TABLE IF EXISTS `gis_multi_point_insert`;
/*!40101 SET @saved_cs_client = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `gis_multi_point_insert` (
`g` multipoint DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
/*!40101 SET character_set_client = @saved_cs_client */;

--
-- Dumping data for table `gis_multi_point_insert`
--

LOCK TABLES `gis_multi_point_insert` WRITE;
/*!40000 ALTER TABLE `gis_multi_point_insert` DISABLE KEYS */;
INSERT INTO `gis_multi_point_insert` VALUES ('\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0$@\0\0\0\0\0\0$@\0\0\0\0\0\0\0\0\0$@\0\0\0\0\0\04@\0\0\0\0\0\0\0\0\04@\0\0\0\0\0\04@'),('\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0$@\0\0\0\0\0\0\0\0'),(NULL),(NULL);
/*!40000 ALTER TABLE `gis_multi_point_insert` ENABLE KEYS */;
UNLOCK TABLES;

--
-- Table structure for table `gis_multi_polygon`
--

DROP TABLE IF EXISTS `gis_multi_polygon`;
/*!40101 SET @saved_cs_client = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `gis_multi_polygon` (
`g` multipolygon DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
/*!40101 SET character_set_client = @saved_cs_client */;

--
-- Dumping data for table `gis_multi_polygon`
--

LOCK TABLES `gis_multi_polygon` WRITE;
/*!40000 ALTER TABLE `gis_multi_polygon` DISABLE KEYS */;
INSERT INTO `gis_multi_polygon` VALUES ('\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0<@\0\0\0\0\0\0:@\0\0\0\0\0\0<@\0\0\0\0\0\0\0\0\0\0\0\0\0\0U@\0\0\0\0\0\0\0\0\0\0\0\0\0\0U@\0\0\0\0\0\0E@\0\0\0\0\0\0<@\0\0\0\0\0\0:@\0\0\0\0\0\0\0\0\0J@\0\0\0\0\0\02@\0\0\0\0\0�P@\0\0\0\0\0\07@\0\0\0\0\0@R@\0\0\0\0\0\0\"@\0\0\0\0\0\0H@\0\0\0\0\0\0@\0\0\0\0\0\0J@\0\0\0\0\0\02@\0\0\0\0\0\0\0\0\0\0\0\0\0\0�M@\0\0\0\0\0\02@\0\0\0\0\0�P@\0\0\0\0\0\02@\0\0\0\0\0�P@\0\0\0\0\0\0*@\0\0\0\0\0�M@\0\0\0\0\0\0*@\0\0\0\0\0�M@\0\0\0\0\0\02@'),('\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0<@\0\0\0\0\0\0:@\0\0\0\0\0\0<@\0\0\0\0\0\0\0\0\0\0\0\0\0\0U@\0\0\0\0\0\0\0\0\0\0\0\0\0\0U@\0\0\0\0\0\0E@\0\0\0\0\0\0<@\0\0\0\0\0\0:@\0\0\0\0\0\0\0\0\0J@\0\0\0\0\0\02@\0\0\0\0\0�P@\0\0\0\0\0\07@\0\0\0\0\0@R@\0\0\0\0\0\0\"@\0\0\0\0\0\0H@\0\0\0\0\0\0@\0\0\0\0\0\0J@\0\0\0\0\0\02@\0\0\0\0\0\0\0\0\0\0\0\0\0\0�M@\0\0\0\0\0\02@\0\0\0\0\0�P@\0\0\0\0\0\02@\0\0\0\0\0�P@\0\0\0\0\0\0*@\0\0\0\0\0�M@\0\0\0\0\0\0*@\0\0\0\0\0�M@\0\0\0\0\0\02@'),('\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0@\0\0\0\0\0\0@\0\0\0\0\0\0@\0\0\0\0\0\0@\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0@');
/*!40000 ALTER TABLE `gis_multi_polygon` ENABLE KEYS */;
UNLOCK TABLES;

--
-- Table structure for table `gis_multi_polygon_batch`
--
Expand Down

0 comments on commit fa7d42e

Please sign in to comment.