Nodejs ; bug fix

.github/workflows/nodejs-knex-tests.yaml

@@ -5,10 +5,10 @@ on:
     branches:
       - master
     paths:
-      - nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex
+      - nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/**
   pull_request:
     paths:
-      - nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex
+      - nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/**
 jobs:
   unittests:
     runs-on: ubuntu-latest

.github/workflows/nodejs-sequelize-tests.yaml

@@ -5,10 +5,10 @@ on:
     branches:
       - master
     paths:
-      - nodejs/sqlcommenter-nodejs/packages/sqlcommenter-sequelize
+      - nodejs/sqlcommenter-nodejs/packages/sqlcommenter-sequelize/**
   pull_request:
     paths:
-      - nodejs/sqlcommenter-nodejs/packages/sqlcommenter-sequelize
+      - nodejs/sqlcommenter-nodejs/packages/sqlcommenter-sequelize/**
 jobs:
   unittests:
     runs-on: ubuntu-latest

nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/index.js

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-const {hasComment} = require('./util');
+const { hasComment } = require('./util');
 const provider = require('./provider');
 const hook = require('./hooks');
 
@@ -33,7 +33,7 @@ const defaultFields = {
  *  TraceProvider: Should be either 'OpenCensus' or 'OpenTelemetry', indicating which library to collect trace data from.
  * @return {void}
  */
-exports.wrapMainKnex = (Knex, include={}, options={}) => {
+exports.wrapMainKnex = (Knex, include = {}, options = {}) => {
 
     /* c8 ignore next 2 */
     if (Knex.___alreadySQLCommenterWrapped___)
@@ -46,7 +46,7 @@ exports.wrapMainKnex = (Knex, include={}, options={}) => {
 
     // Please don't change this prototype from an explicit function
     // to use arrow functions lest we'll get bugs with not resolving "this".
-    Knex.Client.prototype.query = function(conn, obj) {
+    Knex.Client.prototype.query = function (conn, obj) {
 
         // If Knex.VERSION() is available, that means they are using a version of knex.js < 0.16.1
         // because as per https://github.com/tgriesser/knex/blob/master/CHANGELOG.md#0161---28-nov-2018
@@ -74,7 +74,7 @@ exports.wrapMainKnex = (Knex, include={}, options={}) => {
         // Add trace context to comments, depending on the current provider.
         provider.attachComments(options.TraceProvider, comments);
 
-        const filtering = typeof include === 'object' && include && Object.keys(include).length > 0; 
+        const filtering = typeof include === 'object' && include && Object.keys(include).length > 0;
         // Filter out keys whose values are undefined or aren't to be included by default.
         const keys = Object.keys(comments).filter((key) => {
             /* c8 ignore next 6 */
@@ -94,11 +94,19 @@ exports.wrapMainKnex = (Knex, include={}, options={}) => {
             const uri_encoded_value = encodeURIComponent(comments[key]);
             return `${uri_encoded_key}='${uri_encoded_value}'`;
         }).join(',');
-
+
+        var trimmedSqlStmt = sqlStmt.trim();
+        if (sqlStmt.slice(-1) === ';') {
+            commentedSQLStatement = `${trimmedSqlStmt.slice(0, -1)} /*${commentStr}*/;`
+        }
+        else {
+            commentedSQLStatement = `${trimmedSqlStmt} /*${commentStr}*/`
+        }
+
         if (typeof obj === 'string') {
-            obj = {sql: `${sqlStmt} /*${commentStr}*/`};
+            obj = { sql: commentedSQLStatement };
         } else {
-            obj.sql = `${sqlStmt} /*${commentStr}*/`;
+            obj.sql = commentedSQLStatement;
         }
 
         return query.apply(this, [conn, obj]);
@@ -141,7 +149,7 @@ const getKnexVersion = (Knex) => {
  *  TraceProvider: Should be either 'OpenCensus' or 'OpenTelemetry', indicating which library to collect trace data from.
  * @return {Function} A middleware that is compatible with the express framework.
  */
-exports.wrapMainKnexAsMiddleware = (Knex, include=null, options) => {
+exports.wrapMainKnexAsMiddleware = (Knex, include = null, options) => {
 
     exports.wrapMainKnex(Knex, include, options);
 

nodejs/sqlcommenter-nodejs/packages/sqlcommenter-knex/test/comment.test.js

@@ -94,6 +94,15 @@ describe("Comments for Knex", () => {
             done();
         });
 
+        it("should add ; after the comment ", (done) => {
+            const want = "SELECT * from foo /*db_driver='knex%3Afake%3A0.0.1'*/;";
+            fakeKnex.Client.prototype.query(null, { sql: 'SELECT * from foo;' }).then(({ sql }) => {
+                expect(sql).equals(want);
+            });
+            done();
+        });
+
+
         it("chaining and repeated calls should NOT indefinitely chain SQL", (done) => {
 
             const want = "SELECT * from foo /*db_driver='knex%3Afake%3A0.0.1'*/";
@@ -154,10 +163,10 @@ describe("With OpenTelemetry tracing", () => {
     it('Starting an OpenTelemetry trace should produce `traceparent`', (done) => {
         const rootSpan = tracer.startSpan('rootSpan');
         context.with(trace.setSpan(context.active(), rootSpan), async () => {
-            const obj = { sql: 'SELECT * FROM foo' };
+            const obj = { sql: 'SELECT * FROM foo;' };
             fakeKnex.Client.prototype.query(null, obj).then((got) => {
                 const augmentedSQL = got.sql;
-                const wantSQL = `SELECT * FROM foo /*db_driver='knex%3Afake%3A0.0.1',traceparent='00-${rootSpan.spanContext().traceId}-${rootSpan.spanContext().spanId}-01'*/`;
+                const wantSQL = `SELECT * FROM foo /*db_driver='knex%3Afake%3A0.0.1',traceparent='00-${rootSpan.spanContext().traceId}-${rootSpan.spanContext().spanId}-01'*/;`;
                 console.log(augmentedSQL);
                 expect(augmentedSQL).equals(wantSQL);
                 rootSpan.end();

nodejs/sqlcommenter-nodejs/packages/sqlcommenter-sequelize/index.js

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-const {hasComment} = require('./util');
+const { hasComment } = require('./util');
 const provider = require('./provider');
 const hook = require('./hooks');
 
@@ -33,7 +33,7 @@ const defaultFields = {
  *  TraceProvider: Should be either 'OpenCensus' or 'OpenTelemetry', indicating which library to collect trace data from.
  * @return {void}
  */
-exports.wrapSequelize = (sequelize, include={}, options={}) => {
+exports.wrapSequelize = (sequelize, include = {}, options = {}) => {
 
     /* c8 ignore next 2 */
     if (sequelize.___alreadySQLCommenterWrapped___)
@@ -43,7 +43,7 @@ exports.wrapSequelize = (sequelize, include={}, options={}) => {
 
     // Please don't change this prototype from an explicit function
     // to use arrow functions lest we'll get bugs with not resolving "this".
-    sequelize.dialect.Query.prototype.run = function(sql, sql_options) {
+    sequelize.dialect.Query.prototype.run = function (sql, sql_options) {
 
         // If a comment already exists, do not insert a new one.
         // See internal issue #20.
@@ -54,7 +54,7 @@ exports.wrapSequelize = (sequelize, include={}, options={}) => {
             client_timezone: this.sequelize.options.timezone,
             db_driver: `sequelize:${sequelizeVersion}`
         };
-        
+
         if (sequelize.__middleware__) {
             const context = hook.getContext();
             if (context && context.req) {
@@ -64,9 +64,9 @@ exports.wrapSequelize = (sequelize, include={}, options={}) => {
 
         // Add trace context to comments, depending on the provider.
         provider.attachComments(options.TraceProvider, comments);
-    
+
         // Filter out keys whose values are undefined or aren't to be included by default.
-        const filtering = typeof include === 'object' && include && Object.keys(include).length > 0; 
+        const filtering = typeof include === 'object' && include && Object.keys(include).length > 0;
         const keys = Object.keys(comments).filter((key) => {
             /* c8 ignore next 6 */
             if (!filtering)
@@ -85,10 +85,20 @@ exports.wrapSequelize = (sequelize, include={}, options={}) => {
             const uri_encoded_value = encodeURIComponent(comments[key]);
             return `${uri_encoded_key}='${uri_encoded_value}'`;
         }).join(',');
-
-        if (commentStr && commentStr.length > 0)
-            sql = `${sql} /*${commentStr}*/`;
-
+
+        sql = sql.trim()
+        if (commentStr && commentStr.length > 0) {
+            if (sql.slice(-1) === ';') {
+                var trimmedSql = sql.slice(0, -1);
+                sql = `${trimmedSql} /*${commentStr}*/;`;
+
+            }
+            else {
+                sql = `${sql} /*${commentStr}*/`;
+            }
+
+        }
+
         return run.apply(this, [sql, sql_options]);
     }
 
@@ -119,7 +129,7 @@ const sequelizeVersion = resolveSequelizeVersion();
  *  TraceProvider: Should be either 'OpenCensus' or 'OpenTelemetry', indicating which library to collect trace data from.
  * @return {Function} A middleware that is compatible with the express framework. 
  */
-exports.wrapSequelizeAsMiddleware = (sequelize, include=null, options) => {
+exports.wrapSequelizeAsMiddleware = (sequelize, include = null, options) => {
 
     exports.wrapSequelize(sequelize, include, options);
 

nodejs/sqlcommenter-nodejs/packages/sqlcommenter-sequelize/test/comment.test.js

@@ -77,6 +77,18 @@ describe("Comments for Sequelize", () => {
             done();
         });
 
+        it("should add ; after the comment ", (done) => {
+
+            const want = `SELECT * FROM foo /*client_timezone='%2B00%3A00',db_driver='sequelize%3A${seq_version}'*/;`;
+            const query = 'SELECT * FROM foo; ';
+
+            fakeSequelize.dialect.Query.prototype.run(query).then((sql) => {
+                expect(sql).equals(want);
+            });
+
+            done();
+        });
+
         it("should NOT affix comments to statements with existing comments", (done) => {
 
             const q = [