Bladeren bron

feat: retention reaches files too, and moves an expiry without rewriting the document

I said both of these were impossible. Both had been possible since 2.8; I
had read an older header and not checked again.

`patchWithTtl` moves a document's expiry and nothing else. The restamp
used to read each document whole and write it back, because before 2.8
only insert and upsert carried a TTL - so changing one date rewrote every
field and bumped the version of every execution in the collection. It
passes an empty patch now, which changes no field at all.

`setFileTtl` changes a stored file's expiry after the fact. That was the
half of the problem I reported as a dead end; the other half was finding
a workflow's files, since the store can only be searched by type, name,
checksum or related_id. So the owning workflow is written to related_id
when a file is stored - nothing queries that field, which is what makes
it free to carry the owner - and whatever the node put there is kept in
the metadata rather than thrown away.

So a retention change now covers a workflow's executions, its documents
and its files, which is what it always claimed to. The preview no longer
reports filesReachable: false.

Verified: a workflow stored a file, the file was found by its owner, and
a 20-second retention applied afterwards took the execution, the document
and the file with it - "1 executions, 1 documents, 1 files" in the log,
then nothing left. 70 passed, 0 failed, 0 skipped.
fszontagh 1 maand geleden
bovenliggende
commit
802d30c269

+ 40 - 0
lib/storage/storage_client.cpp

@@ -141,6 +141,17 @@ Result<int64_t> StorageClient::update(const std::string& collection,
     return -1;
 }
 
+Result<int64_t> StorageClient::patchWithTtl(const std::string& collection,
+                                            const std::string& id,
+                                            const nlohmann::json& fields,
+                                            int64_t ttl_ms) {
+    uint64_t version = impl_->client_->patchWithTtl(collection, id, fields, msToSec(ttl_ms));
+    if (version == 0) {
+        return Error(ErrorCode::DatabaseError, "Patch with TTL failed: " + collection + "/" + id);
+    }
+    return static_cast<int64_t>(version);
+}
+
 Result<void> StorageClient::remove(const std::string& collection,
                                    const std::string& id,
                                    int64_t expected_version) {
@@ -507,6 +518,35 @@ Result<FileInfo> StorageClient::getFileInfo(const std::string& id) {
     return out;
 }
 
+Result<std::vector<FileInfo>> StorageClient::listFilesForWorkflow(const std::string& workflow_id,
+                                                                  uint32_t limit, uint32_t offset) {
+    if (workflow_id.empty()) return std::vector<FileInfo>{};
+    auto listed = impl_->client_->listFiles("", workflow_id, limit, offset);
+    std::vector<FileInfo> out;
+    out.reserve(listed.files.size());
+    for (const auto& f : listed.files) {
+        FileInfo info;
+        info.id = f.id;
+        info.name = f.name;
+        info.mime_type = f.mime_type;
+        info.size = static_cast<int64_t>(f.size);
+        info.file_type = f.file_type;
+        info.related_id = f.related_id;
+        info.checksum = f.checksum;
+        info.is_public = f.is_public;
+        out.push_back(info);
+    }
+    return out;
+}
+
+Result<void> StorageClient::setFileTtl(const std::string& id, int64_t ttl_ms) {
+    if (!impl_->client_->setFileTtl(id, msToSec(ttl_ms))) {
+        return Error(ErrorCode::DocumentNotFound,
+                     "No such file in this project, or it has already expired: " + id);
+    }
+    return {};
+}
+
 Result<void> StorageClient::deleteFile(const std::string& id) {
     if (!impl_->client_->deleteFile(id)) {
         return Error(ErrorCode::DatabaseError, "Failed to delete file: " + id);

+ 27 - 0
lib/storage/storage_client.hpp

@@ -168,6 +168,17 @@ public:
                                    int64_t expected_version = 0,
                                    bool partial = false);
 
+    // Merge fields and set the expiry in one server-side step. ttl_ms of 0
+    // clears the expiry; pass an empty object to change only the expiry.
+    //
+    // Before this existed, changing a document's TTL meant reading it whole and
+    // writing it back, which rewrites every field and bumps the version to move
+    // one date.
+    common::Result<int64_t> patchWithTtl(const std::string& collection,
+                                         const std::string& id,
+                                         const nlohmann::json& fields,
+                                         int64_t ttl_ms);
+
     common::Result<void> remove(const std::string& collection,
                                 const std::string& id,
                                 int64_t expected_version = 0);
@@ -252,6 +263,22 @@ public:
 
     common::Result<void> deleteFile(const std::string& id);
 
+    // Change the expiry of a file that is already stored. ttl_ms of 0 clears it
+    // and the file is kept indefinitely.
+    //
+    // Until 2.8 a file's lifetime was fixed at upload, which is why retention
+    // changes could not reach files at all. A file belonging to another project
+    // reads as absent, so this cannot be used to shorten somebody else's, and an
+    // already-expired file cannot be brought back by extending it.
+    common::Result<void> setFileTtl(const std::string& id, int64_t ttl_ms);
+
+    // Files stored by one workflow. The owning workflow is written to
+    // related_id when a file is stored, because it is the only field the file
+    // store can be searched by.
+    common::Result<std::vector<FileInfo>> listFilesForWorkflow(const std::string& workflow_id,
+                                                               uint32_t limit = 500,
+                                                               uint32_t offset = 0);
+
     bool isConnected() const;
 
 private:

+ 20 - 2
src/runner/workflow_engine.cpp

@@ -1373,6 +1373,7 @@ NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
     // Nothing declared leaves documents on the lifetime they have always had -
     // for ever - so an installation that has not chosen a retention keeps
     // behaving exactly as it did.
+    const std::string own_workflow_id = workflow.id;
     const auto declared_retention = retentionFor(workflow);
     const int64_t retention_ttl_ms = declared_retention ? declared_retention->ttlMs() : 0;
     const std::string own_collection = storage::workflowCollectionName(workflow.id);
@@ -1606,7 +1607,8 @@ NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
     // Files sit outside the collection namespace, so they are gated on a reserved
     // "files" permission entry. Deny-by-default, consistent with collections: a
     // workflow must declare settings.storagePermissions.collections.files.
-    ctx.storage_upload_file = [this, canWriteCollection, retention_ttl_ms](const std::vector<uint8_t>& data,
+    ctx.storage_upload_file = [this, canWriteCollection, retention_ttl_ms, own_workflow_id](
+                                    const std::vector<uint8_t>& data,
                                     const engine::ScriptFileMeta& meta)
         -> common::Result<engine::ScriptFileInfo> {
         if (!canWriteCollection("files")) {
@@ -1618,10 +1620,26 @@ NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
         upstream.name = meta.name;
         upstream.mime_type = meta.mime_type;
         upstream.file_type = meta.file_type;
-        upstream.related_id = meta.related_id;
         upstream.is_public = meta.is_public;
         upstream.metadata = meta.metadata;
 
+        // related_id carries which workflow stored the file, because it is the
+        // only field the file store can be searched by - and without it a
+        // workflow's files cannot be found again, so a retention change or a
+        // deletion could never reach them.
+        //
+        // Whatever the node put there is kept, under its own key, rather than
+        // thrown away: http-request used it for the checksum and
+        // sdcpp-fetch-output for the source path. Nothing queries it, which is
+        // what makes it free to reuse.
+        upstream.related_id = own_workflow_id.empty() ? meta.related_id : own_workflow_id;
+        if (!meta.related_id.empty() && !own_workflow_id.empty()) {
+            upstream.metadata["nodeRelatedId"] = meta.related_id;
+        }
+        if (!own_workflow_id.empty()) {
+            upstream.metadata["workflowId"] = own_workflow_id;
+        }
+
         // The file expires with the document that points at it. A stored file
         // whose record has aged out is unreachable weight, and a record whose
         // file has gone is a broken link - they have to share one lifetime.

+ 29 - 9
src/webserver/retention/retention_service.cpp

@@ -67,8 +67,27 @@ void RetentionService::worker() {
                     job.from_creation);
         const int64_t documents = restamp(own, kNoWorkflowField, job.workflow_id, job.ttl_seconds, job.from_creation);
 
-        LOG_INFO("Retention: workflow {} restamped to {}s - {} executions, {} documents",
-                 job.workflow_id, job.ttl_seconds, executions, documents);
+        // Files too, now that a stored file's expiry can be changed after the
+        // fact and the owning workflow is recorded on it. Both were true only
+        // from 2.8: before that a file's lifetime was fixed at upload and the
+        // store could not be asked whose it was, which is why this used to
+        // report files as out of reach.
+        //
+        // Measured from now rather than from creation. The file store answers
+        // when a file was made only in its own listing, and being a little
+        // generous with a file whose document has already gone is the harmless
+        // direction - the sweeper takes it either way.
+        int64_t files = 0;
+        auto owned = storage_.listFilesForWorkflow(job.workflow_id);
+        if (owned.ok()) {
+            for (const auto& file : owned.value()) {
+                if (stopping_) break;
+                if (storage_.setFileTtl(file.id, job.ttl_seconds * 1000).ok()) files++;
+            }
+        }
+
+        LOG_INFO("Retention: workflow {} restamped to {}s - {} executions, {} documents, {} files",
+                 job.workflow_id, job.ttl_seconds, executions, documents, files);
 
         if (job.drop_collection_after) {
             // The documents are not gone yet - they expire on their own shortly.
@@ -157,6 +176,7 @@ std::optional<storage::Retention> RetentionService::effectiveFor(const nlohmann:
 RetentionService::Preview RetentionService::preview(const std::string& workflow_id,
                                                     int64_t ttl_seconds) {
     Preview preview;
+    preview.files_reachable = true;
     preview.executions =
         measure(kExecutions, kExecutionWorkflowField, workflow_id, ttl_seconds);
     preview.documents = measure(storage::workflowCollectionName(workflow_id), kNoWorkflowField,
@@ -237,13 +257,13 @@ int64_t RetentionService::restamp(const std::string& collection,
                     }
                 }
 
-                // upsert rather than update: only insert and upsert carry a TTL,
-                // and upsert swaps the expiry entry in one locked server-side
-                // step. It preserves _created_at, so a restamped record still
-                // says when it was made - verified against the live database,
-                // because a retention change that quietly re-dated everything
-                // would be worse than the growth it was meant to fix.
-                auto written = storage_.upsert(collection, doc, id, ttl_ms);
+                // patchWithTtl moves the expiry and nothing else. The whole
+                // document used to be read and written back, because before 2.8
+                // only insert and upsert carried a TTL - so changing one date
+                // rewrote every field and bumped the version of every execution
+                // in the collection. An empty patch changes no field at all.
+                auto written = storage_.patchWithTtl(collection, id, nlohmann::json::object(),
+                                                     ttl_ms);
                 if (written.failed()) {
                     LOG_WARN("Retention: could not restamp {}/{} ({})", collection, id,
                              written.error().message());

+ 7 - 7
src/webserver/retention/retention_service.hpp

@@ -29,12 +29,12 @@ namespace smartbotic::webserver::retention {
 // That is slow enough to belong on a worker rather than inside the request that
 // asked for it - nine and a half thousand executions is a real number here.
 //
-// **Files are out of reach.** A file's TTL can only be set when it is uploaded,
-// and the file store can be searched by type, name, checksum or related id but
-// not by workflow. A workflow's files therefore keep whatever retention was in
-// force when they were written, and a later change does not reach them. The
-// workflow id is recorded on every file as it is stored so that this becomes
-// possible the moment the file store can be asked for it.
+// Files are covered too. That needed two things which only arrived in 2.8:
+// setFileTtl, so a stored file's expiry can be changed after the fact, and the
+// owning workflow being written to the file's related_id when it is stored,
+// since that is the only field the file store can be searched by. Before those,
+// a file kept whatever retention was in force when it was written and nothing
+// could find it again.
 
 class RetentionService {
 public:
@@ -53,7 +53,7 @@ public:
     struct Preview {
         Impact executions;
         Impact documents;   // the workflow's own collection
-        bool files_reachable = false;  // always false today - see the note above
+        bool files_reachable = true;   // true since 2.8 - see the note above
     };
 
     // The retention actually in force for a workflow, resolving what it