[O2B-952] Fixed a bug where the tally of replies was not accurately reflected on the log overview pages (#1279)

* [O2B-952] add custom query that counts a log's total reply count

* [O2B-952] make reply counter available to all log overviews

* [O2B-952] fix: addressing SQL injection vulnerability in the existing getTagDistributionQuery

* [O2B-952] reverted code simplification because of breaking tests

* [O2B-952] missed addChildrenCountByRootLog that has been replaced

* [O2B-952] Implemented the query as a combined query to reduce amount of queries sent

* [O2B-952] add a safety check to prevent the method's execution when there are now logs in the rows

Author: SandroBoelsz

lib/database/repositories/LogRepository.js

@@ -21,17 +21,39 @@ const { timestampToMysql } = require('../../server/utilities/timestampToMysql.js
  * @param {Period} the period in which logs should have been created
  * @return {string} the query
  */
-const getTagDistributionQuery = ({ from, to }) => `
+const getTagDistributionQuery = ({ from, to }) => sequelize.query(`
     SELECT t.text tag, COUNT(*) count
     FROM logs l
              INNER JOIN log_tags lt on l.id = lt.log_id
              INNER JOIN tags t on lt.tag_id = t.id
-    WHERE l.created_at >= '${timestampToMysql(from)}'
-      AND l.created_at < '${timestampToMysql(to)}'
+    WHERE l.created_at >= :fromTimeStamp
+      AND l.created_at < :toTimeStamp
       AND t.archived_at IS NULL
     GROUP BY t.text
     ORDER BY COUNT(*) DESC, t.text
-`;
+`, {
+    replacements: { fromTimeStamp: timestampToMysql(from), toTimeStamp: timestampToMysql(to) },
+});
+
+/**
+ * Returns the query to fetch the reply count for a given array of logIds.
+ *
+ * @param {Array<number>} logIds The log ids for which the reply count will be fetched.
+ * @returns {Promise<Array<{ originLogId: number, replyCounter: number }>>} A promise resolving to an array of objects,
+ * each containing for each originLogId the reply count.
+ */
+const getLogsWithReplyCountQuery = (logIds) => sequelize.query(`
+    WITH RECURSIVE log_tree (originLogId, id) AS (
+        SELECT id originId, id FROM logs
+        WHERE id IN (:logIds)
+        UNION ALL
+        SELECT originLogId, l.id FROM logs as l
+            JOIN log_tree AS lt ON lt.id = l.parent_log_id
+    )
+    SELECT originLogId, COUNT(*) - 1 as replyCount FROM log_tree GROUP BY originLogId;
+`, {
+    replacements: { logIds },
+});
 
 /**
  * Sequelize implementation of the LogRepository.
@@ -45,21 +67,24 @@ class LogRepository extends Repository {
     }
 
     /**
-     * Adds a count of root log by their amount of children onto every applicable log.
+     * Adds a 'replies' property to the given rows array which denotes the amount of children for each applicable log.
      *
-     * @param {Array} rows The collection of rows already fetched
-     * @param {QueryBuilder} queryBuilder The QueryBuilder to use.
-     * @returns {Promise} Promise object representing the full mock data
+     * @param {Array} rows The collection of log rows already fetched.
+     * @returns {Promise} Promise returning logs with a 'replies' property, denoting the reply count for each log.
      */
-    async addChildrenCountByRootLog(rows, queryBuilder = new QueryBuilder()) {
-        const rootLogIds = [...new Set(rows.map((row) => row.rootLogId))];
-        queryBuilder
-            .where('rootLogId').oneOf(...rootLogIds)
-            .set('group', ['rootLogId']);
+    async addChildrenCountByLog(rows) {
+        const logIds = [...new Set(rows.map((row) => row.id))];
+
+        if (logIds.length === 0) {
+            return rows;
+        }
+
+        const [results] = await getLogsWithReplyCountQuery(logIds);
+
+        results.forEach((result) => {
+            const count = result.replyCount || 0;
+            const rowToAttachTo = rows.find(({ id }) => id === result.originLogId);
 
-        const replies = await this.count(queryBuilder);
-        replies.forEach(({ rootLogId, count }) => {
-            const rowToAttachTo = rows.find(({ id }) => id === rootLogId);
             if (rowToAttachTo) {
                 rowToAttachTo.replies = count;
             }
@@ -109,7 +134,7 @@ class LogRepository extends Repository {
      * @return {Promise<{tag: string, count: number}[]>} the tag and their amount
      */
     async getTagDistribution(period) {
-        const [rows] = await sequelize.query(getTagDistributionQuery(period));
+        const [rows] = await getTagDistributionQuery(period);
         return rows;
     }
 }

lib/server/services/log/LogService.js

@@ -87,6 +87,7 @@ class LogService {
 
     /**
      * Return all the logs that correspond to a given list of ids with a default list of shallow relations
+     * and also adds the number of replies to each log.
      *
      * @param {number[]} logIds the list of ids of logs to fetch
      * @return {Promise<Log[]>} the resulting logs
@@ -98,7 +99,9 @@ class LogService {
             .include({ association: 'lhcFills', attributes: ['fillNumber'] })
             .include({ association: 'environments', attributes: ['id'] });
 
-        return (await LogRepository.findAll(logQueryBuilder)).map(logAdapter.toEntity);
+        const logs = await LogRepository.findAll(logQueryBuilder);
+        const logEntities = logs.map(logAdapter.toEntity);
+        return LogRepository.addChildrenCountByLog(logEntities);
     }
 
     /**

lib/usecases/log/GetAllLogsUseCase.js

@@ -245,7 +245,7 @@ class GetAllLogsUseCase {
             return LogRepository.findAndCountAll(queryBuilder);
         });
 
-        const rowsWithReplies = await LogRepository.addChildrenCountByRootLog(rows);
+        const rowsWithReplies = await LogRepository.addChildrenCountByLog(rows);
 
         return {
             count,

lib/usecases/run/GetLogsByRunUseCase.js

@@ -45,7 +45,7 @@ class GetLogsByRunUseCase {
         });
 
         if (logs) {
-            const logsWithReplies = await LogRepository.addChildrenCountByRootLog(logs);
+            const logsWithReplies = await LogRepository.addChildrenCountByLog(logs);
             return logsWithReplies.map(logAdapter.toEntity);
         } else {
             return null;

lib/usecases/subsystem/GetLogsBySubsystemUseCase.js

@@ -45,7 +45,7 @@ class GetLogsBySubsystemUseCase {
         });
 
         if (logs) {
-            const logsWithReplies = await LogRepository.addChildrenCountByRootLog(logs);
+            const logsWithReplies = await LogRepository.addChildrenCountByLog(logs);
             return logsWithReplies.map(logAdapter.toEntity);
         } else {
             return null;

lib/usecases/tag/GetLogsByTagUseCase.js

@@ -63,7 +63,7 @@ class GetLogsByTagUseCase {
                 },
             });
 
-            const logsWithCount = await LogRepository.addChildrenCountByRootLog(logs);
+            const logsWithCount = await LogRepository.addChildrenCountByLog(logs);
 
             return { logs: logsWithCount, count };
         });