feat(database): enhance updateColumnsByIdWithoutHooks with heartbeat write logic and add tests for SQL-expression rejection

Author: nawazdhandala

App/FeatureSet/Telemetry/Jobs/ProbeIngest/ProcessProbeIngest.ts

@@ -119,8 +119,14 @@ export async function processIncomingEmailFromQueue(
     onlyCheckForIncomingEmailReceivedAt: false,
   };
 
-  // Update monitor with last email received time
-  await MonitorService.updateOneById({
+  /*
+   * Update monitor with last email received time. Heartbeat write:
+   * single-statement UPDATE, no hooks and no `version` bump. These columns
+   * trigger no onUpdateSuccess work and Monitor enables no update
+   * workflow/realtime/audit, so nothing is lost. See
+   * ServiceService.updateLastSeen.
+   */
+  await MonitorService.updateColumnsByIdWithoutHooks({
     id: new ObjectID(monitor._id.toString()),
     data: {
       incomingEmailMonitorLastEmailReceivedAt: now,
@@ -130,9 +136,6 @@ export async function processIncomingEmailFromQueue(
       >,
       incomingEmailMonitorHeartbeatCheckedAt: now,
     },
-    props: {
-      isRoot: true,
-    },
   });
 
   /*

App/FeatureSet/Workers/Index.ts

@@ -193,7 +193,6 @@ import "./Jobs/EnterpriseLicense/ReportUserCount";
 
 import AnalyticsTableManagement from "./Utils/AnalyticsDatabase/TableManegement";
 import RunDatabaseMigrations from "./Utils/DataMigration";
-import { RunDatabaseMigrationsOnBoot } from "Common/Server/EnvironmentConfig";
 import JobDictionary from "./Utils/JobDictionary";
 import { PromiseVoidFunction } from "Common/Types/FunctionTypes";
 import Queue, { QueueJob, QueueName } from "Common/Server/Infrastructure/Queue";
@@ -204,6 +203,7 @@ import {
   DisableQueueWorkers,
   EnableQueueDashboard,
   QueueDashboardSecret,
+  RunDatabaseMigrationsOnBoot,
 } from "Common/Server/EnvironmentConfig";
 import { WORKER_CONCURRENCY } from "./Config";
 import MetricsAPI from "./API/Metrics";

App/Migrate.ts

@@ -7,6 +7,7 @@ import {
 import RunDatabaseMigrations from "./FeatureSet/Workers/Utils/DataMigration";
 import AnalyticsTableManagement from "./FeatureSet/Workers/Utils/AnalyticsDatabase/TableManegement";
 import logger from "Common/Server/Utils/Logger";
+import { PromiseVoidFunction } from "Common/Types/FunctionTypes";
 
 /*
  * One-shot migration runner. Runs the SAME schema + data migrations the app
@@ -22,7 +23,7 @@ import logger from "Common/Server/Utils/Logger";
 
 const APP_NAME: string = "migrate";
 
-const migrate = async (): Promise<void> => {
+const migrate: PromiseVoidFunction = async (): Promise<void> => {
   logger.debug(
     `${APP_NAME}: connecting to Postgres (applies schema migrations)`,
   );

Common/Server/Services/DatabaseService.ts

@@ -69,7 +69,6 @@ import {
 } from "typeorm";
 import { ColumnMetadata } from "typeorm/metadata/ColumnMetadata";
 import { EntityMetadata } from "typeorm/metadata/EntityMetadata";
-import { QueryDeepPartialEntity } from "typeorm/query-builder/QueryPartialEntity";
 import { ObjectLiteral } from "typeorm/common/ObjectLiteral";
 import { FindWhere } from "../../Types/BaseDatabase/Query";
 import Realtime from "../Utils/Realtime";
@@ -2005,7 +2004,7 @@ class DatabaseService<TBaseModel extends BaseModel> extends BaseService {
   @CaptureSpan()
   public async updateColumnsByIdWithoutHooks(input: {
     id: ObjectID;
-    data: QueryDeepPartialEntity<TBaseModel>;
+    data: PartialEntity<TBaseModel>;
   }): Promise<void> {
     if (!input.id) {
       throw new BadDataException("id is required");
@@ -2028,6 +2027,17 @@ class DatabaseService<TBaseModel extends BaseModel> extends BaseService {
           `updateColumnsByIdWithoutHooks: unknown column "${propertyName}" on "${metadata.tableName}"`,
         );
       }
+      /*
+       * PartialEntity permits `() => string` SQL-expression values, but this
+       * raw bind path can only parameterize literals (and the save()-based
+       * updateOneById it replaces doesn't support expressions either). Fail
+       * loudly rather than binding a function object.
+       */
+      if (typeof value === "function") {
+        throw new BadDataException(
+          `updateColumnsByIdWithoutHooks: SQL-expression values are not supported (column "${propertyName}"); pass a literal value.`,
+        );
+      }
       /*
        * Run the value through the driver's persist path so column
        * transformers (e.g. ObjectID <-> uuid), JSON stringification, and

Common/Server/Utils/Monitor/MonitorResource.ts

@@ -316,17 +316,21 @@ export default class MonitorResourceUtil {
            * the monitor to flap between Online and Offline every minute.
            */
           if (!serverMonitorResponse.onlyCheckRequestReceivedAt) {
-            await MonitorService.updateOneById({
+            /*
+             * Heartbeat write: single-statement UPDATE, no hooks and no
+             * `version` bump. Monitor enables no update workflow/realtime/
+             * audit and these heartbeat columns trigger no onUpdateSuccess
+             * work, so nothing is lost; it also skips the pre-SELECT that
+             * would reload the large serverMonitorResponse jsonb row. See
+             * ServiceService.updateLastSeen.
+             */
+            await MonitorService.updateColumnsByIdWithoutHooks({
               id: monitor.id!,
               data: {
                 serverMonitorRequestReceivedAt:
                   serverMonitorResponse.requestReceivedAt!,
                 serverMonitorResponse,
               },
-              props: {
-                isRoot: true,
-                ignoreHooks: true,
-              },
             });
 
             logger.debug(
@@ -344,7 +348,13 @@ export default class MonitorResourceUtil {
             `${dataToProcess.monitorId.toString()} - Incoming request received at ${incomingMonitorRequest.incomingRequestReceivedAt}`,
           );
 
-          await MonitorService.updateOneById({
+          /*
+           * Heartbeat write: single-statement UPDATE, no hooks and no
+           * `version` bump; skips the pre-SELECT of the large
+           * incomingMonitorRequest jsonb row. See
+           * ServiceService.updateLastSeen.
+           */
+          await MonitorService.updateColumnsByIdWithoutHooks({
             id: monitor.id!,
             data: {
               incomingRequestMonitorHeartbeatCheckedAt:
@@ -353,10 +363,6 @@ export default class MonitorResourceUtil {
                 JSON.stringify(incomingMonitorRequest),
               ) as IncomingMonitorRequest,
             } as any,
-            props: {
-              isRoot: true,
-              ignoreHooks: true,
-            },
           });
 
           logger.debug(

Common/Server/Utils/Telemetry/EntityRegistry.ts

@@ -271,11 +271,17 @@ export async function reconcileByNaturalKey<
   });
 
   if (existing) {
-    // Throttled bump of lastSeenAt (+ any caller-supplied merge fields).
-    await data.service.updateOneById({
+    /*
+     * Throttled bump of lastSeenAt (+ any caller-supplied merge fields).
+     * Heartbeat write: single-statement UPDATE, no hooks and no `version`
+     * bump (TelemetryEntity/Relationship enable no update workflow/realtime/
+     * audit). buildBump returns only plain values — lastSeenAt plus, at most,
+     * the descriptiveAttributes / labels JSON columns — which the primitive
+     * persists via the driver transformer path. See ServiceService.updateLastSeen.
+     */
+    await data.service.updateColumnsByIdWithoutHooks({
       id: existing.id!,
       data: buildBump(existing),
-      props: { isRoot: true },
     });
     return;
   }
@@ -306,10 +312,9 @@ export async function reconcileByNaturalKey<
       logger.debug(
         `EntityRegistry: create raced for ${data.describe} (concurrent insert); bumping lastSeenAt on the winning row.`,
       );
-      await data.service.updateOneById({
+      await data.service.updateColumnsByIdWithoutHooks({
         id: winner.id!,
         data: buildBump(winner),
-        props: { isRoot: true },
       });
       return;
     }

Common/Tests/Server/Services/UpdateColumnsByIdWithoutHooks.test.ts

@@ -128,6 +128,17 @@ describe("DatabaseService.updateColumnsByIdWithoutHooks", () => {
     ).rejects.toThrow(/unknown column "notAColumn"/);
   });
 
+  test("rejects SQL-expression (function) values it cannot bind", async () => {
+    const query: jest.Mock = mockRepository();
+    await expect(
+      ServiceService.updateColumnsByIdWithoutHooks({
+        id: ObjectID.generate(),
+        data: { lastSeenAt: (() => "NOW()") as never } as never,
+      }),
+    ).rejects.toThrow(/SQL-expression values are not supported/);
+    expect(query).not.toHaveBeenCalled();
+  });
+
   test("does not issue a query when there are no columns to update", async () => {
     const query: jest.Mock = mockRepository();
     await ServiceService.updateColumnsByIdWithoutHooks({