From ea7479e8ad662e675b8dea3efb68ab6fb2f585d0 Mon Sep 17 00:00:00 2001
From: Chen Yangjian <252317+cyjake@users.noreply.github.com>
Date: Thu, 15 Jul 2021 19:58:56 +0800
Subject: [PATCH] fix: formatting select join with subqueries should not tamper
 the subquery itself

---
 index.js                                     |  3 +-
 package.json                                 |  1 +
 src/drivers/abstract/spellbook.js            |  2 +-
 src/expr.js                                  |  4 +-
 src/utils/string.js                          |  5 +-
 test/unit/drivers/abstract/spellbook.test.js | 54 ++++++++++++++++++++
 test/unit/spell.test.js                      |  1 -
 7 files changed, 63 insertions(+), 7 deletions(-)
 create mode 100644 test/unit/drivers/abstract/spellbook.test.js

diff --git a/index.js b/index.js
index e9e9d19a..9e780723 100644
--- a/index.js
+++ b/index.js
@@ -11,7 +11,7 @@ const DataTypes = require('./src/data_types');
 const { findDriver } = require('./src/drivers');
 const migrations = require('./src/migrations');
 const sequelize = require('./src/adapters/sequelize');
-const { camelCase } = require('./src/utils/string');
+const { camelCase, heresql } = require('./src/utils/string');
 const Hint = require('./src/hint');
 
 /**
@@ -253,6 +253,7 @@ Object.assign(Realm, {
   Logger,
   Spell,
   sequelize,
+  heresql,
   ...Hint,
 });
 
diff --git a/package.json b/package.json
index 08a46394..2b5733a1 100644
--- a/package.json
+++ b/package.json
@@ -45,6 +45,7 @@
   },
   "dependencies": {
     "debug": "^3.1.0",
+    "heredoc": "^1.3.1",
     "pluralize": "^7.0.0",
     "sqlstring": "^2.3.0",
     "strftime": "^0.10.0",
diff --git a/src/drivers/abstract/spellbook.js b/src/drivers/abstract/spellbook.js
index d81f0dd2..71a908f7 100644
--- a/src/drivers/abstract/spellbook.js
+++ b/src/drivers/abstract/spellbook.js
@@ -399,7 +399,7 @@ function createSubspell(spell) {
       }
     });
     if (internal) {
-      const token = copyExpr(JSON.parse(JSON.stringify(condition)), ({ type, value }) => {
+      const token = copyExpr(condition, ({ type, value }) => {
         if (type === 'id') return { type, value };
       });
       subspell.whereConditions.unshift(token);
diff --git a/src/expr.js b/src/expr.js
index ee980fb9..944c6290 100644
--- a/src/expr.js
+++ b/src/expr.js
@@ -436,9 +436,7 @@ function walkExpr(ast, fn) {
 function copyExpr(ast, fn) {
   ast = fn(ast) || ast;
   if (ast.args) {
-    for (let i = 0; i < ast.args.length; i++) {
-      ast.args[i] = copyExpr(ast.args[i], fn);
-    }
+    ast.args = ast.args.map(arg => copyExpr(arg, fn));
   }
   return ast;
 }
diff --git a/src/utils/string.js b/src/utils/string.js
index e2c91637..3bd1f28a 100644
--- a/src/utils/string.js
+++ b/src/utils/string.js
@@ -1,5 +1,7 @@
 'use strict';
 
+const heredoc = require('heredoc');
+
 /**
  * Convert the first charactor of the string from lowercase to uppercase.
  * @param {string} str
@@ -43,10 +45,11 @@ function snakeCase(str) {
 
 /**
  * Convert multiline SQL into single line for better logging
- * @param {string} text
+ * @param {string|Function} text
  * @returns {string}
  */
 function heresql(text) {
+  if (typeof text === 'function') text = heredoc(text);
   return text.trim().split('\n').map(line => line.trim()).join(' ');
 }
 
diff --git a/test/unit/drivers/abstract/spellbook.test.js b/test/unit/drivers/abstract/spellbook.test.js
new file mode 100644
index 00000000..b82f1e0b
--- /dev/null
+++ b/test/unit/drivers/abstract/spellbook.test.js
@@ -0,0 +1,54 @@
+'use strict';
+
+const assert = require('assert').strict;
+const { connect, heresql, Bone } = require('../../../..');
+
+describe('=> Spellbook', function() {
+  class User extends Bone {}
+  class Post extends Bone {
+    static table = 'articles'
+    static initialize() {
+      this.belongsTo('author', { className: 'User' });
+    }
+  }
+
+  before(async function() {
+    await connect({
+      models: [ User, Post ],
+      database: 'leoric',
+      user: 'root',
+      port: process.env.MYSQL_PORT,
+    });
+  });
+
+  describe('formatSelect', function() {
+    it('should not tamper subquery when formatting subspell', async function() {
+      const query = Post.where({
+        isPrivate: true,
+        authorId: User.where({ stauts: 1 }),
+      }).with('author');
+
+      assert.equal(query.limit(10).toString(), heresql(function() {/*
+        SELECT `posts`.*, `author`.*
+          FROM (SELECT * FROM `articles`
+                  WHERE `is_private` = true
+                    AND `author_id` IN (SELECT * FROM `users` WHERE `stauts` = 1)
+                    AND `gmt_deleted` IS NULL LIMIT 10) AS `posts`
+      LEFT JOIN `users` AS `author`
+            ON `posts`.`userId` = `author`.`id`
+      */}));
+
+      assert.doesNotThrow(function() {
+        assert.equal(query.count().toString(), heresql(function() {/*
+          SELECT `posts`.*, `author`.*, COUNT(*) AS `count`
+            FROM `articles` AS `posts`
+       LEFT JOIN `users` AS `author`
+              ON `posts`.`userId` = `author`.`id`
+           WHERE `posts`.`is_private` = true
+             AND `posts`.`author_id` IN (SELECT * FROM `users` WHERE `stauts` = 1)
+             AND `posts`.`gmt_deleted` IS NULL
+        */}));
+      });
+    });
+  });
+});
diff --git a/test/unit/spell.test.js b/test/unit/spell.test.js
index c3f500a4..19d51a0d 100644
--- a/test/unit/spell.test.js
+++ b/test/unit/spell.test.js
@@ -537,7 +537,6 @@ describe('=> raw sql', () => {
     );
   });
 
-
   it('select sub raw query', function() {
     assert.equal(
       Post.where({