✨ Add auto-repair for integrity error files (#12524)

* ✨ Add auto-repair for integrity error files

* ✨ MR changes
This commit is contained in:
Pablo Alba 2026-10-26 11:04:06 +01:00 committed by GitHub
parent 7cf914088a
commit 3cd073fc6a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 976 additions and 77 deletions

View File

@ -21,6 +21,7 @@
- Full referential/semantic validation currently runs only when file features contain `"components/v2"`.
- Validation starts at root plus orphan shapes, then validates component records. `validate-file!` raises `:validation :referential-integrity` with collected details.
- `repair-file` does not mutate data directly; it reduces validation errors into redo changes using `changes-builder`. Callers must apply or persist those changes.
- `repair-file-until-valid` stops on: no errors; unchanged `:data` after apply (backend page/`PointerMap` `=` is pointer identity); stable error fingerprint `(frequencies (map #(dissoc % :shape) errors))` vs previous pass (check fingerprint before `repair-file`/`process-changes`); or `max-iterations`. Each `repair-error` builds from the original `file-data`, so N `:set`-based errors on one shape take N passes.
- `:missing-slot` fires only for a REAL swap: a copy sub-head whose `shape-ref` is no longer a child of the near main parent. A pure positional mismatch (ref still a sibling elsewhere) is a reorder — valid, realigned by the async component sync; do not "repair" it by assigning swap slots (a slot freezes the child out of normal sync). `fix-missing-swap-slots` (migration 0019) follows the same membership rule.
## Migrations

View File

@ -75,6 +75,12 @@
::mdef/buckets mtx/default-request-histogram-buckets
::mdef/type :histogram}
:update-file-repair
{::mdef/name "penpot_rpc_update_file_repair_total"
::mdef/help "update-file auto-repair attempts by outcome (repaired or unrepaired)."
::mdef/labels ["outcome"]
::mdef/type :counter}
:rpc-main-timing
{::mdef/name "penpot_rpc_main_timing"
::mdef/help "RPC command method call timing for main"

View File

@ -12,6 +12,7 @@
[app.common.features :as cfeat]
[app.common.files.changes :as cpc]
[app.common.files.migrations :as fmg]
[app.common.files.repair :as cfr]
[app.common.files.validate :as val]
[app.common.logging :as l]
[app.common.schema :as sm]
@ -41,6 +42,7 @@
(declare ^:private get-lagged-changes)
(declare ^:private send-notifications!)
(declare ^:private send-repair-notifications)
(declare ^:private update-file)
(declare ^:private update-file*)
(declare ^:private process-changes-and-validate)
@ -124,6 +126,27 @@
(contains? token-change-types type)
(contains? file-change-types type)))
;; Not a real editor session: file-change messages with this id are
;; delivered to every connected session, including the saver.
(def repair-session-id
#uuid "a11c0de5-0000-4000-8000-000000000001")
(defn- referential-integrity-error?
[cause]
(= :referential-integrity (:code (ex-data cause))))
(defn- record-repair-outcome
[cfg outcome]
(when-let [metrics (::mtx/metrics cfg)]
(when (mtx/metrics? metrics)
(mtx/run! metrics {:id :update-file-repair
:labels [(name outcome)]
:inc 1}))))
(defn- error-codes
[errors]
(into [] (keep :code) errors))
;; If features are specified from params and the final feature
;; set is different than the persisted one, update it on the
;; database.
@ -225,7 +248,9 @@
:soft-validate-affected-ms (get timings :soft-validate-affected-ms)
:soft-validate-schema-ms (get timings :soft-validate-schema-ms)
:validate-affected-ms (get timings :validate-affected-ms)
:validate-schema-ms (get timings :validate-schema-ms)))))
:validate-schema-ms (get timings :validate-schema-ms)
:repair-ms (get timings :repair-ms)
:reprocess-changes-ms (get timings :reprocess-changes-ms)))))
(catch Throwable cause
;; NOTE: `with-defer` hooks only run on the success path
;; (`handle-response`), so timings collected before a hard
@ -241,7 +266,9 @@
:soft-validate-affected-ms (get timings :soft-validate-affected-ms)
:soft-validate-schema-ms (get timings :soft-validate-schema-ms)
:validate-affected-ms (get timings :validate-affected-ms)
:validate-schema-ms (get timings :validate-schema-ms)))
:validate-schema-ms (get timings :validate-schema-ms)
:repair-ms (get timings :repair-ms)
:reprocess-changes-ms (get timings :reprocess-changes-ms)))
(throw cause))))))
(defn- update-file*
@ -275,27 +302,65 @@
changes skip-validate))
deleted-at
(ct/plus timestamp (ct/duration {:hours 1}))]
(ct/plus timestamp (ct/duration {:hours 1}))
(when-let [file (::snapshot file)]
(let [deleted-at (ct/plus timestamp (ldel/get-deletion-delay team))
label (str "internal/snapshot/" revn)]
repair
(::repair file)
(fsnap/create! cfg file
{:label label
snapshot-deleted-at
(ct/plus timestamp (ldel/get-deletion-delay team))
change-created-at
(cond-> timestamp
repair
(ct/plus (ct/duration {:millis 1})))]
(if repair
(let [snapshot-file (or (::snapshot file)
(fdata/realize cfg (:original repair)))]
(fsnap/create! cfg snapshot-file
{:label (str "internal/repair/" revn)
:created-by "system"
:deleted-at deleted-at
:deleted-at snapshot-deleted-at
:profile-id profile-id
:session-id repair-session-id}))
(when-let [file (::snapshot file)]
(fsnap/create! cfg file
{:label (str "internal/snapshot/" revn)
:created-by "system"
:deleted-at snapshot-deleted-at
:profile-id profile-id
:session-id session-id})))
(when repair
(db/insert! conn :file-change
{:id (uuid/next)
:session-id repair-session-id
:profile-id profile-id
:created-at timestamp
:updated-at timestamp
:deleted-at deleted-at
:file-id (:id file)
:revn (:revn repair)
:version (:version file)
:features (into-array (:features file))
:changes (blob/encode (:changes repair))}
{::db/return-keys false})
(l/wrn :hint "file auto-repaired"
:file-id (str (:id file))
:error-codes (error-codes (:errors repair))
:error-count (count (:errors repair))
:change-count (count (:changes repair)))
(record-repair-outcome cfg :repaired))
;; Insert change (xlog) with deleted_at in a future data for
;; make them automatically eleggible for GC once they expires
(db/insert! conn :file-change
{:id (uuid/next)
:session-id session-id
:profile-id profile-id
:created-at timestamp
:updated-at timestamp
:created-at change-created-at
:updated-at change-created-at
:deleted-at deleted-at
:file-id (:id file)
:revn (:revn file)
@ -304,12 +369,14 @@
:changes (blob/encode changes)}
{::db/return-keys false})
(persist-file! cfg file)
(persist-file! cfg (dissoc file ::repair))
(when (contains? cf/flags :redis-cache)
(invalidate-caches! cfg file))
;; Send asynchronous notifications
(when repair
(send-repair-notifications cfg params file repair))
(send-notifications! cfg params file)
(with-meta {:revn revn :lagged (get-lagged-changes conn params)}
@ -434,9 +501,105 @@
(l/error :hint "file validation error"
:cause cause))))
(defn- apply-changes
[cfg file changes]
(let [state (atom {})
file (binding [cpc/*state* state]
(-> file
(update :data cpc/process-changes changes)
(update :data d/without-nils)))]
(if-let [media-refs (-> @state :media-refs not-empty)]
(bfc/update-media-references! cfg file media-refs)
file)))
(defn- fail-repair
[cfg file errors cause]
(l/wrn :hint "file auto-repair did not recover the file"
:file-id (str (:id file))
:error-codes (error-codes errors)
:error-count (count errors))
(record-repair-outcome cfg :unrepaired)
(throw cause))
(defn- repair-preexisting-damage
"Repair damage that was already in `original` on the pages/components
the client's `changes` touch, then re-apply those changes. Throws
`cause` when the client's changes are at fault or the repair cannot
fix the file."
[cfg original libs changes tracked tracked-0 cause]
(let [libs (or libs {})]
(try
(let [pre-errors
(binding [pmap/*tracked* nil]
(val/validate-file-affected original libs changes))]
(when (empty? pre-errors)
(throw cause))
(when tracked
(reset! tracked tracked-0))
(let [trepair (ct/tpoint)
result
(try
(binding [pmap/*tracked* tracked]
(cfr/repair-file-until-valid
original libs
(fn [file applied]
(binding [pmap/*tracked* nil]
(val/validate-file-affected file libs (into applied changes))))
:initial-errors pre-errors))
(finally
(record-validation-timing cfg :repair-ms "repair" (inst-ms (trepair)))))
repair-changes (:changes result)]
(when (seq (:errors result))
(fail-repair cfg original (:errors result) cause))
(let [treprocess (ct/tpoint)
repaired (try
(binding [pmap/*tracked* tracked]
(apply-changes cfg (:file result) changes))
(finally
(record-validation-timing cfg :reprocess-changes-ms "reprocess-changes"
(inst-ms (treprocess)))))
scope (into repair-changes changes)]
(binding [pmap/*tracked* nil]
(val/validate-file-affected! repaired libs scope)
(when (contains? cf/flags :file-schema-validation)
(let [t (ct/tpoint)]
(try
(val/validate-file-schema! repaired)
(finally
(record-validation-timing cfg :validate-schema-ms "validate-schema" (inst-ms (t))))))))
(-> repaired
(assoc :revn (+ (:revn original) 2))
(assoc ::repair {:changes repair-changes
:errors pre-errors
:revn (inc (:revn original))
:original (dissoc original ::snapshot)})))))
(catch Throwable inner
(if (identical? inner cause)
(throw cause)
(do
(l/wrn :hint "file auto-repair failed"
:file-id (str (:id original))
:cause inner)
(record-repair-outcome cfg :unrepaired)
(throw cause)))))))
(defn- process-changes-and-validate
[cfg file changes skip-validate]
(let [should-resolve-libs?
(let [original file
tracked pmap/*tracked*
tracked-0 (some-> tracked deref)
should-resolve-libs?
(and (or (contains? cf/flags :file-validation)
(contains? cf/flags :soft-file-validation))
(not skip-validate))
@ -452,33 +615,20 @@
(when should-resolve-libs?
(record-validation-timing cfg :resolve-libs-ms "resolve-libs" (inst-ms (tlibs))))))
;; The main purpose of this atom is provide a contextual state
;; for the changes subsystem where optionally some hints can
;; be provided for the changes processing. Right now we are
;; using it for notify about the existence of media refs when
;; a new shape is added.
state
(atom {})
tproc
(ct/tpoint)
file
(try
(let [file
(binding [cpc/*state* state]
(-> (files/check-version! file)
(update :revn inc)
(update :data cpc/process-changes changes)
(update :data d/without-nils)))
file
(if-let [media-refs (-> @state :media-refs not-empty)]
(bfc/update-media-references! cfg file media-refs)
file)]
file)
(apply-changes cfg
(-> (files/check-version! file)
(update :revn inc))
changes)
(finally
(record-validation-timing cfg :process-changes-ms "process-changes" (inst-ms (tproc)))))]
(record-validation-timing cfg :process-changes-ms "process-changes" (inst-ms (tproc)))))
pending-repair
(atom nil)]
(binding [pmap/*tracked* nil]
(when (contains? cf/flags :soft-file-validation)
@ -500,18 +650,25 @@
(let [t (ct/tpoint)]
(try
(val/validate-file-affected! file libs changes)
(catch Throwable cause
(reset! pending-repair cause)
(when-not (referential-integrity-error? cause)
(throw cause)))
(finally
(record-validation-timing cfg :validate-affected-ms "validate-affected" (inst-ms (t)))))))
(when (and (contains? cf/flags :file-schema-validation)
(not skip-validate))
(not skip-validate)
(nil? @pending-repair))
(let [t (ct/tpoint)]
(try
(val/validate-file-schema! file)
(finally
(record-validation-timing cfg :validate-schema-ms "validate-schema" (inst-ms (t))))))))
file))
(if-let [cause @pending-repair]
(repair-preexisting-damage cfg original libs changes tracked tracked-0 cause)
file)))
(defn- take-snapshot?
"Defines the rule when file `data` snapshot should be saved."
@ -541,6 +698,34 @@
(mapv (fn [row]
(update row :changes blob/decode)))))
(defn- send-repair-notifications
[cfg {:keys [team session-id] :as params} file repair]
(let [changes (:changes repair)
lchanges (filter library-change? changes)
msgbus (::mbus/msgbus cfg)]
(mbus/pub! msgbus
:topic (:id file)
:message {:type :file-change
:profile-id (:profile-id params)
:file-id (:id file)
:session-id repair-session-id
:revn (:revn repair)
:vern (:vern file)
:changes changes
:repaired true})
(when (and (:is-shared file) (seq lchanges))
(mbus/pub! msgbus
:topic (:id team)
:message {:type :library-change
:profile-id (:profile-id params)
:file-id (:id file)
:session-id session-id
:revn (:revn repair)
:modified-at (ct/now)
:changes lchanges}))))
(defn- send-notifications!
[cfg {:keys [team changes session-id] :as params} file]
(let [lchanges (filter library-change? changes)

View File

@ -6,7 +6,6 @@
(ns app.srepl.procs.file-repair
(:require
[app.common.files.changes :as cfc]
[app.common.files.repair :as cfr]
[app.common.files.validate :as cfv]
[app.common.logging :as l]))
@ -17,44 +16,28 @@
(defn repair-file
"Internal helper for validate and repair the file. The operation is
applied multiple times untile file is fixed or max iteration counter
applied multiple times until file is fixed or max iteration counter
is reached (default 10).
This function should not be used directly, it is used throught the
This function should not be used directly, it is used through the
app.srepl.main/repair-file! helper. In practical terms this function
is private and implementation detail."
[file libs & {:keys [max-iterations] :or {max-iterations 10}}]
(let [validate-and-repair
(fn [file libs iteration]
(when-let [errors (not-empty (cfv/validate-file file libs))]
(l/trc :hint "repairing file"
:file-id (str (:id file))
:iteration iteration
:errors (count errors))
(let [changes (cfr/repair-file file libs errors)]
(-> file
(update :revn inc)
(update :data cfc/process-changes changes)))))
(let [orig-revn (:revn file)
result (cfr/repair-file-until-valid
file libs
(fn [file _]
(cfv/validate-file file libs))
:max-iterations max-iterations)
file' (update (:file result) :revn (fnil + 0) (:iterations result))]
process-file
(fn [file libs]
(loop [file file
iteration 0]
(if (< iteration max-iterations)
(if-let [file (validate-and-repair file libs iteration)]
(recur file (inc iteration))
file)
(do
(l/wrn :hint "max retry num reached on repairing file"
:file-id (str (:id file))
:iteration iteration)
file))))
(when (>= (:iterations result) max-iterations)
(l/wrn :hint "max retry num reached on repairing file"
:file-id (str (:id file))
:iteration (:iterations result)))
file'
(process-file file libs)]
(when (not= (:revn file) (:revn file'))
(when (not= orig-revn (:revn file'))
(l/trc :hint "file repaired" :file-id (str (:id file))))
file'))

View File

@ -0,0 +1,433 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns backend-tests.rpc-file-update-repair-test
(:require
[app.binfile.common :as bfc]
[app.common.features :as cfeat]
[app.common.files.repair :as cfr]
[app.common.files.validate :as cfv]
[app.common.test-helpers.compositions :as tho]
[app.common.test-helpers.files :as thf]
[app.common.test-helpers.ids-map :as thi]
[app.common.test-helpers.shapes :as ths]
[app.common.types.shape :as cts]
[app.common.uuid :as uuid]
[app.config :as cf]
[app.db :as db]
[app.features.file-snapshots :as fsnap]
[app.metrics :as mtx]
[app.msgbus :as mbus]
[app.rpc :as-alias rpc]
[app.rpc.commands.files-update :as files-update]
[app.srepl.procs.file-repair :as file-repair]
[app.util.pointer-map :as pmap]
[backend-tests.helpers :as th]
[clojure.test :as t])
(:import
io.prometheus.client.CollectorRegistry))
(t/use-fixtures :once th/state-init)
(t/use-fixtures :each th/database-reset)
(defn- with-repair-flags
[f]
(with-redefs [cf/flags (conj cf/flags :file-validation)]
(f)))
(defn- with-full-repair-flags
[f]
(with-redefs [cf/flags (conj cf/flags :file-validation :file-schema-validation)]
(f)))
(defn- repair-count
[outcome]
(or (.getSampleValue ^CollectorRegistry
(mtx/get-registry (::mtx/metrics th/*system*))
"penpot_rpc_update_file_repair_total"
(into-array String ["outcome"])
(into-array String [(name outcome)]))
0.0))
(defn- add-rect
[page-id shape-id]
{:type :add-obj
:page-id page-id
:id shape-id
:parent-id uuid/zero
:frame-id uuid/zero
:components-v2 true
:obj (cts/setup-shape
{:id shape-id
:name "rect"
:frame-id uuid/zero
:parent-id uuid/zero
:type :rect})})
(defn- set-attr
[page-id shape-id attr val]
{:type :mod-obj
:id shape-id
:page-id page-id
:operations [{:type :set :attr attr :val val}]})
(defn- update-file
[profile-id file-id revn changes & {:keys [skip-validate session-id]
:or {session-id (uuid/random)}}]
(th/command! (cond-> {::th/type :update-file
::rpc/profile-id profile-id
:id file-id
:session-id session-id
:revn revn
:vern 0
:features cfeat/supported-features
:changes changes}
skip-validate (assoc :skip-validate true))))
(defn- stored-file
[file-id]
(bfc/get-file th/*system* file-id :realize? true))
(defn- change-rows
[file-id]
(->> (th/db-query :file-change {:file-id file-id} {::db/remove-deleted false})
(filter (fn [row]
(and (some? (:changes row))
(nil? (:label row)))))
(sort-by :created-at)
(vec)))
(defn- label-rows
[file-id]
(->> (th/db-query :file-change {:file-id file-id} {::db/remove-deleted false})
(filter :label)
(vec)))
(defn- pointer-map-in?
[x]
(cond
(pmap/pointer-map? x) true
(map? x) (boolean (some pointer-map-in? (vals x)))
(sequential? x) (boolean (some pointer-map-in? x))
:else false))
(defn- setup-file
[& {:keys [is-shared] :or {is-shared false}}]
(let [profile (th/create-profile* 1 {:is-active true})
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared is-shared})
page-id (first (get-in file [:data :pages]))
shape-a (uuid/next)
shape-b (uuid/next)
out (update-file (:id profile) (:id file) 0
[(add-rect page-id shape-a)
(add-rect page-id shape-b)])]
(t/is (th/success? out))
{:profile profile
:file-id (:id file)
:page-id page-id
:shape-a shape-a
:shape-b shape-b
:revn (:revn (stored-file (:id file)))}))
(defn- break-frame
[{:keys [profile file-id page-id shape-a revn]}]
(let [out (update-file (:id profile) file-id revn
[(set-attr page-id shape-a :frame-id (uuid/next))]
:skip-validate true)]
(t/is (th/success? out))
(stored-file file-id)))
(t/deftest repairs-preexisting-frame-not-found
(with-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-a shape-b] :as ctx} (setup-file)
broken (break-frame ctx)
before (repair-count :repaired)
out (update-file (:id profile) file-id (:revn broken)
[(set-attr page-id shape-b :name "renamed")])
stored (stored-file file-id)
shape (get-in stored [:data :pages-index page-id :objects shape-a])
frame (get-in stored [:data :pages-index page-id :objects (:frame-id shape)])]
(t/is (th/success? out))
(t/is (nil? (cfv/validate-file stored {})))
(t/is (some? frame))
(t/is (= "renamed"
(get-in stored [:data :pages-index page-id :objects shape-b :name])))
(t/is (= 1.0 (- (repair-count :repaired) before)))))))
(t/deftest repairs-preexisting-frame-not-found-with-schema-validation
(with-full-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-a shape-b] :as ctx} (setup-file)
broken (break-frame ctx)
n (:revn broken)
before-repaired (repair-count :repaired)
before-unrepaired (repair-count :unrepaired)
out (update-file (:id profile) file-id n
[(set-attr page-id shape-b :name "renamed")])
stored (stored-file file-id)
shape (get-in stored [:data :pages-index page-id :objects shape-a])
frame (get-in stored [:data :pages-index page-id :objects (:frame-id shape)])
labels (label-rows file-id)
repair (filter #(= (str "internal/repair/" n) (:label %)) labels)
snap (filter #(= (str "internal/snapshot/" n) (:label %)) labels)]
(t/is (th/success? out))
(t/is (nil? (cfv/validate-file stored {})))
(t/is (some? frame))
(t/is (= "renamed"
(get-in stored [:data :pages-index page-id :objects shape-b :name])))
(t/is (= (+ n 2) (:revn stored)))
(t/is (= 1 (count repair)))
(t/is (zero? (count snap)))
(t/is (= 1.0 (- (repair-count :repaired) before-repaired)))
(t/is (= 0.0 (- (repair-count :unrepaired) before-unrepaired)))))))
(t/deftest rejects-client-caused-break
(with-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-a revn]} (setup-file)
before (stored-file file-id)
out (update-file (:id profile) file-id revn
[(set-attr page-id shape-a :frame-id (uuid/next))])
after (stored-file file-id)]
(t/is (not (th/success? out)))
(t/is (= :referential-integrity (:code (ex-data (:error out)))))
(t/is (= (:revn before) (:revn after)))))))
(t/deftest skip-validate-does-not-repair
(with-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-a shape-b] :as ctx} (setup-file)
broken (break-frame ctx)
out (update-file (:id profile) file-id (:revn broken)
[(set-attr page-id shape-b :name "renamed")]
:skip-validate true)
stored (stored-file file-id)
shape (get-in stored [:data :pages-index page-id :objects shape-a])]
(t/is (th/success? out))
(t/is (nil? (get-in stored [:data :pages-index page-id :objects (:frame-id shape)])))))))
(t/deftest damage-on-untouched-page-is-left
(with-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-a revn]} (setup-file)
page-2 (uuid/next)
shape-c (uuid/next)
_ (t/is (th/success?
(update-file (:id profile) file-id revn
[{:type :add-page :id page-2 :name "page-2"}
(add-rect page-2 shape-c)])))
file (stored-file file-id)
_ (t/is (th/success?
(update-file (:id profile) file-id (:revn file)
[(set-attr page-2 shape-c :frame-id (uuid/next))]
:skip-validate true)))
file (stored-file file-id)
out (update-file (:id profile) file-id (:revn file)
[(set-attr page-id shape-a :name "on-page-1")])
stored (stored-file file-id)
shape (get-in stored [:data :pages-index page-2 :objects shape-c])]
(t/is (th/success? out))
(t/is (nil? (get-in stored [:data :pages-index page-2 :objects (:frame-id shape)])))))))
(t/deftest growing-scope-repairs-component-touched-by-repair
(thi/reset-idmap!)
(with-repair-flags
(fn []
(let [file (-> (thf/sample-file :file1 :page-label :page1)
(assoc :revn 0)
(tho/add-simple-component :component1 :root1 :child1)
(ths/add-sample-shape :other)
(update-in [:data :components (thi/id :component1)]
assoc :main-instance-id (uuid/next)
:objects {(uuid/next) {:id (uuid/next)}}))
page-id (thi/id :page1)
changes [(set-attr page-id (thi/id :other) :name "renamed")]
cfg {::files-update/validation-timings (atom {})}
result (with-redefs [bfc/get-resolved-file-libraries (fn [_ _] {})]
(@#'files-update/process-changes-and-validate cfg file changes nil))
stored result]
(t/is (some? (::files-update/repair stored)))
(t/is (nil? (cfv/validate-file stored {})))
(t/is (nil? (get-in stored [:data :components (thi/id :component1) :objects])))))))
(t/deftest repair-records-timings-with-schema-validation
(thi/reset-idmap!)
(with-full-repair-flags
(fn []
(let [file (-> (thf/sample-file :file1 :page-label :page1)
(assoc :revn 0)
(ths/add-sample-shape :shape1)
(ths/update-shape :shape1 :frame-id (uuid/next)))
changes [(set-attr (thi/id :page1) (thi/id :shape1) :name "renamed")]
timings (atom {})
cfg {::files-update/validation-timings timings}
result (with-redefs [bfc/get-resolved-file-libraries (fn [_ _] {})]
(@#'files-update/process-changes-and-validate cfg file changes nil))]
(t/is (some? (::files-update/repair result)))
(t/is (number? (:repair-ms @timings)))
(t/is (number? (:reprocess-changes-ms @timings)))
(t/is (number? (:validate-schema-ms @timings)))))))
(t/deftest unrepairable-rejects-and-calls-repair-once
(with-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-b] :as ctx} (setup-file)
broken (break-frame ctx)
calls (atom 0)
before (repair-count :unrepaired)
out (with-redefs [cfr/repair-file (fn [_ _ _]
(swap! calls inc)
[])]
(update-file (:id profile) file-id (:revn broken)
[(set-attr page-id shape-b :name "renamed")]))]
(t/is (not (th/success? out)))
(t/is (= :referential-integrity (:code (ex-data (:error out)))))
(t/is (= 1 @calls))
(t/is (= 1.0 (- (repair-count :unrepaired) before)))))))
(t/deftest repair-writes-change-rows-and-reuses-snapshot
(with-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-b] :as ctx} (setup-file)
broken (break-frame ctx)
before (count (change-rows file-id))
session (uuid/next)
calls (atom [])
n (:revn broken)
out (with-redefs [mbus/pub! (fn [_cfg & {:keys [topic message]}]
(swap! calls conj {:topic topic :message message}))]
(update-file (:id profile) file-id n
[(set-attr page-id shape-b :name "renamed")]
:session-id session))
rows (change-rows file-id)
new (subvec rows before)
labels (label-rows file-id)
repair (filter #(= (str "internal/repair/" n) (:label %)) labels)
snap (filter #(= (str "internal/snapshot/" n) (:label %)) labels)
msgs (filter #(= :file-change (:type (:message %))) @calls)
stored (stored-file file-id)]
(t/is (th/success? out))
(t/is (= "renamed"
(get-in stored [:data :pages-index page-id :objects shape-b :name])))
(t/is (= 2 (count new)))
(t/is (= (inc n) (:revn (first new))))
(t/is (= files-update/repair-session-id (:session-id (first new))))
(t/is (some? (:deleted-at (first new))))
(t/is (= (:created-at (first new)) (:updated-at (first new))))
(t/is (= (+ n 2) (:revn (second new))))
(t/is (= session (:session-id (second new))))
(t/is (some? (:deleted-at (second new))))
(t/is (= (:created-at (second new)) (:updated-at (second new))))
(t/is (not (pos? (compare (:created-at (first new))
(:created-at (second new))))))
(t/is (= 1 (count repair)))
(t/is (zero? (count snap)))
(t/is (>= (count msgs) 2))
(t/is (true? (get-in (first msgs) [:message :repaired])))
(t/is (= files-update/repair-session-id
(get-in (first msgs) [:message :session-id])))
(t/is (= session (get-in (second msgs) [:message :session-id])))
(t/is (not (true? (get-in (second msgs) [:message :repaired]))))
(t/is (= (+ n 2) (get-in (second msgs) [:message :revn])))))))
(t/deftest repair-library-change-keeps-saver-session
(with-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-b] :as ctx} (setup-file :is-shared true)
broken (break-frame ctx)
session (uuid/next)
calls (atom [])
out (with-redefs [mbus/pub! (fn [_cfg & {:keys [topic message]}]
(swap! calls conj {:topic topic :message message}))]
(update-file (:id profile) file-id (:revn broken)
[(set-attr page-id shape-b :name "renamed")]
:session-id session))
lib-msgs (filter #(= :library-change (:type (:message %))) @calls)]
(t/is (th/success? out))
(t/is (seq lib-msgs))
(doseq [msg lib-msgs]
(t/is (= session (get-in msg [:message :session-id])))
(t/is (not= files-update/repair-session-id
(get-in msg [:message :session-id]))))))))
(t/deftest repair-snapshot-without-auto-snapshot
(with-redefs [cf/flags (-> cf/flags
(conj :file-validation)
(disj :auto-file-snapshot))]
(let [{:keys [profile file-id page-id shape-b] :as ctx} (setup-file)
broken (break-frame ctx)
n (:revn broken)
out (update-file (:id profile) file-id n
[(set-attr page-id shape-b :name "renamed")])
labels (label-rows file-id)
repair (filter #(= (str "internal/repair/" n) (:label %)) labels)
snaps (filter #(re-find #"^internal/snapshot/" (str (:label %))) labels)
row (first repair)
snap (fsnap/get-snapshot th/*system* file-id (:id row))
sfile {:id file-id
:features (:features (stored-file file-id))
:data (:data snap)}]
(t/is (th/success? out))
(t/is (= 1 (count repair)))
(t/is (zero? (count snaps)))
(t/is (false? (pointer-map-in? (:data snap))))
(t/is (some? (cfv/validate-file sfile {}))))))
(t/deftest srepl-repair-grows-revn-by-iterations
(thi/reset-idmap!)
(let [file (-> (thf/sample-file :file1 :page-label :page1)
(assoc :revn 3)
(ths/add-sample-shape :shape1)
(ths/update-shape :shape1 :frame-id (uuid/next)))
loop (cfr/repair-file-until-valid
file {}
(fn [file _] (cfv/validate-file file {})))
wrapped (file-repair/repair-file file {})]
(t/is (pos? (:iterations loop)))
(t/is (= (+ (:revn file) (:iterations loop)) (:revn wrapped)))
(t/is (nil? (cfv/validate-file wrapped {})))))
(t/deftest healthy-save-writes-one-change-row
(with-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-b revn]} (setup-file)
before (count (change-rows file-id))
out (update-file (:id profile) file-id revn
[(set-attr page-id shape-b :name "ok")])
labels (label-rows file-id)]
(t/is (th/success? out))
(t/is (= 1 (- (count (change-rows file-id)) before)))
(t/is (empty? (filter #(re-find #"^internal/repair/" (str (:label %))) labels)))))))
(t/deftest repairs-several-missing-children-on-root
(with-full-repair-flags
(fn []
(let [{:keys [profile file-id page-id shape-a shape-b revn]} (setup-file)
r1 (uuid/next)
r2 (uuid/next)
r3 (uuid/next)
_ (t/is (th/success?
(update-file (:id profile) file-id revn
[(set-attr page-id uuid/zero :shapes
[shape-a shape-b r1 r2 r3])]
:skip-validate true)))
broken (stored-file file-id)
errors (cfv/validate-file broken {})
out (update-file (:id profile) file-id (:revn broken)
[(set-attr page-id shape-b :name "renamed")])
stored (stored-file file-id)
root (get-in stored [:data :pages-index page-id :objects uuid/zero])]
(t/is (= [:child-not-found :child-not-found :child-not-found]
(mapv :code errors)))
(t/is (= #{uuid/zero} (into #{} (map :shape-id) errors)))
(t/is (th/success? out))
(t/is (= [shape-a shape-b] (:shapes root)))
(t/is (nil? (cfv/validate-file stored {})))
(t/is (= "renamed"
(get-in stored [:data :pages-index page-id :objects shape-b :name])))))))

View File

@ -8,6 +8,7 @@
(:require
[app.common.data :as d]
[app.common.data.macros :as dm]
[app.common.files.changes :as cfc]
[app.common.files.changes-builder :as pcb]
[app.common.files.helpers :as cfh]
[app.common.files.tokens :as cfo]
@ -22,8 +23,6 @@
[app.common.types.variant :as ctv]
[app.common.uuid :as uuid]))
(log/set-level! :debug)
(defmulti repair-error
(fn [code _error _file-data _libraries] code))
@ -867,3 +866,58 @@
(pcb/empty-changes nil)
errors)]
redo-changes))
(defn- error-fingerprint
[errors]
(frequencies (map #(dissoc % :shape) errors)))
(defn repair-file-until-valid
"Repeatedly validate and repair `file` until `errors-fn` returns nil,
a repair pass leaves `:data` unchanged, or the error fingerprint
(errors without `:shape`) is unchanged from the previous pass.
`errors-fn` is `(fn [file applied-changes] errors-or-nil)`. Never
changes `:revn`.
Returns `{:file file' :changes [...] :iterations n :errors remaining}`."
[file libraries errors-fn & {:keys [max-iterations initial-errors]
:or {max-iterations 10}}]
(loop [file file
applied []
iteration 0
prev-fp nil
seed-errors initial-errors]
(if (>= iteration max-iterations)
{:file file
:changes applied
:iterations iteration
:errors (not-empty (errors-fn file applied))}
(let [errors (if (and (zero? iteration) (some? seed-errors))
seed-errors
(errors-fn file applied))]
(if (empty? errors)
{:file file
:changes applied
:iterations iteration
:errors nil}
(let [fp (error-fingerprint errors)]
(cond
(= fp prev-fp)
{:file file
:changes applied
:iterations (inc iteration)
:errors (not-empty errors)}
:else
(let [batch (repair-file file libraries errors)
data' (cfc/process-changes (:data file) batch)]
(if (= data' (:data file))
{:file file
:changes applied
:iterations (inc iteration)
:errors (not-empty errors)}
(recur (assoc file :data data')
(into applied batch)
(inc iteration)
fp
nil))))))))))

View File

@ -378,3 +378,171 @@
(t/is (= uuid/zero (:parent-id shape1')))
(t/is (= uuid/zero (:parent-id shape2')))
(t/is (= #{(thi/id :shape1) (thi/id :shape2)} (set (:shapes root')))))))
(defn- broken-frame-file
[]
(-> (thf/sample-file :file1 :page-label :page1)
(assoc :revn 3)
(ths/add-sample-shape :shape1)
(ths/update-shape :shape1 :frame-id (uuid/next))))
(t/deftest repair-file-until-valid-fixes-frame-not-found
(t/testing "repairs a :frame-not-found shape and leaves :revn untouched"
(let [file (broken-frame-file)
errors (cfv/validate-file file {})
result (cfr/repair-file-until-valid
file {}
(fn [file _] (cfv/validate-file file {})))
file' (:file result)
errors' (cfv/validate-file file' {})
shape' (ths/get-shape file' :shape1 :page-label :page1)]
(t/is (= #{:frame-not-found} (into #{} (map :code) errors)))
(t/is (pos? (count (:changes result))))
(t/is (nil? (:errors result)))
(t/is (nil? errors'))
(t/is (= 3 (:revn file')))
(t/is (some? (get-in file' [:data :pages-index (thi/id :page1) :objects (:frame-id shape')]))))))
(t/deftest repair-file-until-valid-healthy-file
(t/testing "a valid file is returned unchanged"
(let [file (-> (thf/sample-file :file1 :page-label :page1)
(assoc :revn 2)
(ths/add-sample-shape :shape1))
result (cfr/repair-file-until-valid
file {}
(fn [file _] (cfv/validate-file file {})))]
(t/is (identical? file (:file result)))
(t/is (= [] (:changes result)))
(t/is (zero? (:iterations result)))
(t/is (nil? (:errors result)))
(t/is (= 2 (:revn (:file result)))))))
(t/deftest repair-file-until-valid-passes-applied-changes
(t/testing "errors-fn receives the changes applied so far"
(let [calls (atom [])
file (broken-frame-file)
result (cfr/repair-file-until-valid
file {}
(fn [file applied]
(swap! calls conj applied)
(cfv/validate-file file {})))]
(t/is (nil? (:errors result)))
(t/is (>= (count @calls) 2))
(t/is (= [] (first @calls)))
(t/is (pos? (count (last @calls)))))))
(t/deftest repair-file-until-valid-stops-on-noop-repair
(t/testing "an empty repair batch stops after one iteration"
(let [file (broken-frame-file)
errors (cfv/validate-file file {})]
(with-redefs [cfr/repair-file (fn [_ _ _] [])]
(let [result (cfr/repair-file-until-valid
file {}
(fn [file _] (cfv/validate-file file {})))]
(t/is (= 1 (:iterations result)))
(t/is (= [] (:changes result)))
(t/is (= (mapv :code errors) (mapv :code (:errors result)))))))))
(t/deftest repair-file-until-valid-repairs-several-errors-on-one-shape
(t/testing "three missing children of one frame are all removed although
every pass reports the same [code shape-id]"
(let [file (-> (thf/sample-file :file1 :page-label :page1)
(ths/add-sample-shape :frame1 :type :frame)
(ths/update-shape :frame1 :shapes [(uuid/next) (uuid/next) (uuid/next)]))
errors (cfv/validate-file file {})
result (cfr/repair-file-until-valid
file {}
(fn [file _] (cfv/validate-file file {})))
frame1' (ths/get-shape (:file result) :frame1 :page-label :page1)]
(t/is (= [:child-not-found :child-not-found :child-not-found] (mapv :code errors)))
(t/is (= #{(thi/id :frame1)} (into #{} (map :shape-id) errors)))
(t/is (nil? (:errors result)))
(t/is (nil? (cfv/validate-file (:file result) {})))
(t/is (= [] (:shapes frame1'))))))
(t/deftest repair-file-until-valid-stops-when-repair-changes-nothing
(t/testing "a non-empty repair that leaves data equal stops after one iteration"
(let [file (broken-frame-file)
shape-id (thi/id :shape1)
page-id (thi/id :page1)
cur-name (:name (ths/get-shape file :shape1 :page-label :page1))]
(with-redefs [cfr/repair-file
(fn [_ _ _]
[{:type :mod-obj
:id shape-id
:page-id page-id
:operations [{:type :set :attr :name :val cur-name}]}])]
(let [result (cfr/repair-file-until-valid
file {}
(fn [_ _] [{:code :frame-not-found :shape-id shape-id}])
:max-iterations 10)]
(t/is (= 1 (:iterations result)))
(t/is (= [] (:changes result)))
(t/is (= [:frame-not-found] (mapv :code (:errors result)))))))))
(t/deftest repair-file-until-valid-stops-when-error-fingerprint-stable
(t/testing "a mutating repair with an unchanged error report stops after two iterations"
(let [file (broken-frame-file)
shape-id (thi/id :shape1)
page-id (thi/id :page1)
pass (atom 0)
sticky {:code :frame-not-found
:shape-id shape-id
:hint "stuck"
:args {:x 1}}]
(with-redefs [cfr/repair-file
(fn [_ _ _]
[{:type :mod-obj
:id shape-id
:page-id page-id
:operations [{:type :set
:attr :name
:val (str "pass-" (swap! pass inc))}]}])]
(let [result (cfr/repair-file-until-valid
file {}
(fn [_ _] [sticky])
:max-iterations 10)]
(t/is (= 2 (:iterations result)))
(t/is (= 1 (count (:changes result))))
(t/is (= [:frame-not-found] (mapv :code (:errors result)))))))))
(t/deftest repair-file-until-valid-stops-at-max-iterations
(t/testing "a repair that never empties errors stops at max-iterations"
(let [file (broken-frame-file)
pass (atom 0)
shape-id (thi/id :shape1)
page-id (thi/id :page1)]
(with-redefs [cfr/repair-file
(fn [_ _ _]
[{:type :mod-obj
:id shape-id
:page-id page-id
:operations [{:type :set
:attr :name
:val (str "pass-" (swap! pass inc))}]}])]
(let [result (cfr/repair-file-until-valid
file {}
(fn [_ _]
[{:code :frame-not-found
:shape-id (uuid/next)}])
:max-iterations 3)]
(t/is (= 3 (:iterations result)))
(t/is (= 3 (count (:changes result))))
(t/is (some? (:errors result))))))))
(t/deftest repair-file-until-valid-uses-initial-errors
(t/testing "errors-fn is not called on the first iteration when :initial-errors is given"
(let [calls (atom 0)
file (broken-frame-file)
seed (cfv/validate-file file {})
result (cfr/repair-file-until-valid
file {}
(fn [file applied]
(swap! calls inc)
(cfv/validate-file file {}))
:initial-errors seed)]
(t/is (nil? (:errors result)))
;; First iteration used the seed; errors-fn runs only after the repair.
(t/is (= 1 @calls)))))

View File

@ -16,6 +16,7 @@
[app.main.data.common :as dc]
[app.main.data.helpers :as dsh]
[app.main.data.modal :as modal]
[app.main.data.notifications :as ntf]
[app.main.data.plugins :as dpl]
[app.main.data.websocket :as dws]
[app.main.data.workspace :as-alias dw]
@ -26,6 +27,7 @@
[app.main.data.workspace.texts :as dwt]
[app.main.router :as rt]
[app.util.globals :refer [global]]
[app.util.i18n :refer [tr]]
[app.util.mouse :as mse]
[app.util.object :as obj]
[app.util.rxops :as rxs]
@ -245,7 +247,8 @@
[:session-id ::sm/uuid]
[:revn :int]
[:vern :int]
[:changes cpc/schema:changes]])
[:changes cpc/schema:changes]
[:repaired {:optional true} :boolean]])
(def ^:private check-file-change-params!
(sm/check-fn schema:handle-file-change))
@ -267,13 +270,17 @@
;; and update the persistence internal state with the updated
;; file-revn
(rx/of (dch/commit {:file-id file-id
:file-revn revn
:file-vern vern
:save-undo? false
:source :remote
:redo-changes (vec changes)
:undo-changes []})))))
(rx/concat
(rx/of (dch/commit {:file-id file-id
:file-revn revn
:file-vern vern
:save-undo? false
:source :remote
:redo-changes (vec changes)
:undo-changes []}))
(if (:repaired msg)
(rx/of (ntf/info (tr "workspace.notifications.file-repaired")))
(rx/empty))))))
(defn handle-file-deleted
[{:keys [file-id] :as msg}]

View File

@ -0,0 +1,50 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns frontend-tests.data.workspace-notifications-test
(:require
[app.common.data :as d]
[app.common.uuid :as uuid]
[app.main.data.changes :as dch]
[app.main.data.notifications :as ntf]
[app.main.data.workspace.notifications :as dwntf]
[app.util.i18n :as i18n]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.async :as async]
[potok.v2.core :as ptk]))
(defn- file-change-msg
[extra]
(merge {:type :file-change
:profile-id (uuid/next)
:file-id (uuid/next)
:session-id (uuid/next)
:revn 1
:vern 0
:changes []}
extra))
(defn- ^:async watch-file-change
[msg]
(let [events (atom [])]
(await (async/observe (ptk/watch (dwntf/handle-file-change msg) {} (rx/empty))
:on-next #(swap! events conj %)))
@events))
(t/deftest ^:async repaired-file-change-applies-and-notifies
(let [events (await (watch-file-change (file-change-msg {:repaired true})))
types (mapv ptk/type events)
show (d/seek #(= ::ntf/show (ptk/type %)) events)
state (ptk/update show {})]
(t/is (= [::dch/commit ::ntf/show] types))
(t/is (= :info (get-in state [:notification :level])))
(t/is (= (i18n/tr "workspace.notifications.file-repaired")
(get-in state [:notification :content])))))
(t/deftest ^:async ordinary-file-change-does-not-notify
(let [events (await (watch-file-change (file-change-msg {})))]
(t/is (= [::dch/commit] (mapv ptk/type events)))))

View File

@ -6977,6 +6977,12 @@ msgstr "UPDATES"
msgid "workspace.notification-pill.detail"
msgstr "Details"
#: src/app/main/data/workspace/notifications.cljs:282
msgid "workspace.notifications.file-repaired"
msgstr ""
"We fixed some issues in this file. You can find a copy of the original version in "
"the version history."
#: src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs:784
msgid "workspace.options.add-interaction"
msgstr "Click the + button to add interactions."

View File

@ -7035,6 +7035,12 @@ msgstr "ACTUALIZACIONES"
msgid "workspace.notification-pill.detail"
msgstr "Detalles"
#: src/app/main/data/workspace/notifications.cljs:282
msgid "workspace.notifications.file-repaired"
msgstr ""
"Hemos corregido algunos problemas de este archivo. Hay una copia de la "
"versión original en el historial de versiones."
#: src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs:784
msgid "workspace.options.add-interaction"
msgstr "Pulsa el botón + para añadir interacciones."