From dc9e53edc88fdb6d75386a40525690afcb121b25 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Mon, 25 Apr 2022 22:31:23 +0200 Subject: [PATCH] (core) Update the current time in formulas automatically every hour Summary: Adds a special user action `UpdateCurrentTime` which invalidates an internal engine dependency node that doesn't belong to any table but is 'used' by the `NOW()` function. Applies the action automatically every hour. Test Plan: Added a Python test for the user action. Tested the interval periodically applying the action manually: {F43312} Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D3389 --- app/server/lib/ActiveDoc.ts | 25 ++++++++++---- app/server/lib/GranularAccess.ts | 2 +- app/server/lib/Sharing.ts | 4 +-- sandbox/grist/engine.py | 18 +++++++++- sandbox/grist/functions/date.py | 3 +- sandbox/grist/table.py | 2 -- sandbox/grist/test_useractions.py | 56 +++++++++++++++++++++++++++++++ sandbox/grist/useractions.py | 8 +++++ 8 files changed, 105 insertions(+), 13 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 39b2007e..19e64df2 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -119,6 +119,9 @@ const MEMORY_MEASUREMENT_INTERVAL_MS = 60 * 1000; // Cleanup expired attachments every hour (also happens when shutting down) const REMOVE_UNUSED_ATTACHMENTS_INTERVAL_MS = 60 * 60 * 1000; +// Apply the UpdateCurrentTime user action every hour +const UPDATE_CURRENT_TIME_INTERVAL_MS = 60 * 60 * 1000; + // A hook for dependency injection. export const Deps = {ACTIVEDOC_TIMEOUT}; @@ -180,11 +183,18 @@ export class ActiveDoc extends EventEmitter { private _recoveryMode: boolean = false; private _shuttingDown: boolean = false; - // Cleanup expired attachments every hour (also happens when shutting down) - private _removeUnusedAttachmentsInterval = setInterval( - () => this.removeUnusedAttachments(true), - REMOVE_UNUSED_ATTACHMENTS_INTERVAL_MS, - ); + // Intervals to clear on shutdown + private _intervals = [ + // Cleanup expired attachments every hour (also happens when shutting down) + setInterval( + () => this.removeUnusedAttachments(true), + REMOVE_UNUSED_ATTACHMENTS_INTERVAL_MS, + ), + setInterval( + () => this._applyUserActions(makeExceptionalDocSession('system'), [["UpdateCurrentTime"]]), + UPDATE_CURRENT_TIME_INTERVAL_MS, + ), + ]; constructor(docManager: DocManager, docName: string, private _options?: ICreateActiveDocOptions) { super(); @@ -406,7 +416,10 @@ export class ActiveDoc extends EventEmitter { // Clear the MapWithTTL to remove all timers from the event loop. this._fetchCache.clear(); - clearInterval(this._removeUnusedAttachmentsInterval); + for (const interval of this._intervals) { + clearInterval(interval); + } + try { // Remove expired attachments, i.e. attachments that were soft deleted a while ago. // This needs to happen periodically, and doing it here means we can guarantee that it happens even if diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts index 1a687792..58782972 100644 --- a/app/server/lib/GranularAccess.ts +++ b/app/server/lib/GranularAccess.ts @@ -99,7 +99,7 @@ const SURPRISING_ACTIONS = new Set([ ]); // Actions we'll allow unconditionally for now. -const OK_ACTIONS = new Set(['Calculate']); +const OK_ACTIONS = new Set(['Calculate', 'UpdateCurrentTime']); /** * Granular access for a single bundle, in different phases. diff --git a/app/server/lib/Sharing.ts b/app/server/lib/Sharing.ts index 5bedb7f0..17626cd6 100644 --- a/app/server/lib/Sharing.ts +++ b/app/server/lib/Sharing.ts @@ -216,9 +216,9 @@ export class Sharing { try { const isCalculate = (userActions.length === 1 && - userActions[0][0] === 'Calculate'); + (userActions[0][0] === 'Calculate' || userActions[0][0] === 'UpdateCurrentTime')); // `internal` is true if users shouldn't be able to undo the actions. Applies to: - // - Calculate because it's not considered as performed by a particular client. + // - Calculate/UpdateCurrentTime because it's not considered as performed by a particular client. // - Adding attachment metadata when uploading attachments, // because then the attachment file may get hard-deleted and redo won't work properly. const internal = isCalculate || userActions.every(a => a[0] === "AddRecord" && a[1] === "_grist_Attachments"); diff --git a/sandbox/grist/engine.py b/sandbox/grist/engine.py index 33f8f120..1ba0ff5f 100644 --- a/sandbox/grist/engine.py +++ b/sandbox/grist/engine.py @@ -3,7 +3,6 @@ The data engine ties the code generated from the schema with the document data, and with dependency tracking. """ -import contextlib import itertools import re import rlcompleter @@ -1141,6 +1140,23 @@ class Engine(object): self.dep_graph.invalidate_deps(table._new_columns_node, depend.ALL_ROWS, self.recompute_map, include_self=False) + def update_current_time(self): + self.dep_graph.invalidate_deps(self._current_time_node, depend.ALL_ROWS, self.recompute_map, + include_self=False) + + def use_current_time(self): + """ + Add a dependency on the current time to the current evaluating node, + so that calling update_current_time() will invalidate the node and cause its reevaluation. + """ + if not self._current_node: + return + table_id = self._current_node[0] + table = self.tables[table_id] + self._use_node(self._current_time_node, table._identity_relation) + + _current_time_node = ("#now", None) + def mark_lookupmap_for_cleanup(self, lookup_map_column): """ Once a LookupMapColumn seems no longer used, it's added here. We'll check after recomputing diff --git a/sandbox/grist/functions/date.py b/sandbox/grist/functions/date.py index 10f953c1..c38651d7 100644 --- a/sandbox/grist/functions/date.py +++ b/sandbox/grist/functions/date.py @@ -447,10 +447,11 @@ def NOW(tz=None): """ Returns the `datetime` object for the current time. """ + engine = docmodel.global_docmodel._engine + engine.use_current_time() return datetime.datetime.now(_get_tzinfo(tz)) - def SECOND(time): """ Returns the seconds of `datetime`, as an integer from 0 to 59. diff --git a/sandbox/grist/table.py b/sandbox/grist/table.py index 6e64b4cc..3cfca9ef 100644 --- a/sandbox/grist/table.py +++ b/sandbox/grist/table.py @@ -60,8 +60,6 @@ class UserTable(object): def __init__(self, model_class): docmodel.enhance_model(model_class) self.Model = model_class - column_ids = {col for col in model_class.__dict__ if not col.startswith("_")} - column_ids.add('id') self.table = None def _set_table_impl(self, table_impl): diff --git a/sandbox/grist/test_useractions.py b/sandbox/grist/test_useractions.py index a08511c0..1b17bddb 100644 --- a/sandbox/grist/test_useractions.py +++ b/sandbox/grist/test_useractions.py @@ -1336,3 +1336,59 @@ class TestUserActions(test_engine.EngineTestCase): [5, 2], [6, 2], ]) + + def test_update_current_time(self): + self.load_sample(self.sample) + self.apply_user_action(["AddEmptyTable"]) + self.add_column('Table1', 'now', isFormula=True, formula='NOW()', type='Any') + + # No records with NOW() in a formula yet, so this action should have no effect at all. + out_actions = self.apply_user_action(["UpdateCurrentTime"]) + self.assertOutActions(out_actions, {}) + + class FakeDatetime(object): + counter = 0 + + @classmethod + def now(cls, *_): + cls.counter += 1 + return cls.counter + + import datetime + original = datetime.datetime + # This monkeypatch depends on NOW() using `import datetime` + # as opposed to `from datetime import datetime` + datetime.datetime = FakeDatetime + + def check(expected_now): + self.assertEqual(expected_now, FakeDatetime.counter) + self.assertTableData('Table1', cols="subset", data=[ + ["id", "now"], + [1, expected_now], + ]) + + try: + # The counter starts at 0. Adding an initial record calls FakeDatetime.now() for the 1st time. + # The call increments the counter to 1 before returning. + self.add_record('Table1') + check(1) + + # Testing that unrelated actions don't change the time + self.apply_user_action(["AddEmptyTable"]) + self.add_record("Table2") + self.apply_user_action(["Calculate"]) # only recalculates for fresh docs + check(1) + + # Actually testing that the time is updated as requested + self.apply_user_action(["UpdateCurrentTime"]) + check(2) + out_actions = self.apply_user_action(["UpdateCurrentTime"]) + check(3) + self.assertOutActions(out_actions, { + "direct": [False], + "stored": [["UpdateRecord", "Table1", 1, {"now": 3}]], + "undo": [["UpdateRecord", "Table1", 1, {"now": 2}]], + }) + finally: + # Revert the monkeypatch + datetime.datetime = original diff --git a/sandbox/grist/useractions.py b/sandbox/grist/useractions.py index 78796902..061d17e0 100644 --- a/sandbox/grist/useractions.py +++ b/sandbox/grist/useractions.py @@ -319,6 +319,14 @@ class UserActions(object): """ pass + @useraction + def UpdateCurrentTime(self): + """ + Somewhat similar to Calculate, trigger calculation + of any cells that depend on the current time. + """ + self._engine.update_current_time() + #---------------------------------------- # User actions on records. #----------------------------------------