0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-02-13 02:28:18 -05:00

Improve schema validation handling

And properly honor the file-validation flag
This commit is contained in:
Andrey Antukh 2023-11-20 15:38:07 +01:00 committed by Andrés Moya
parent 3eb987897a
commit ec8b68721b
6 changed files with 54 additions and 42 deletions

View file

@ -630,10 +630,12 @@
(pu/with-open [input (zstd-input-stream input) (pu/with-open [input (zstd-input-stream input)
input (io/data-input-stream input)] input (io/data-input-stream input)]
(binding [*state* (volatile! {:media [] :index {}})] (binding [*state* (volatile! {:media [] :index {}})]
(let [team (teams/get-team options (let [team (teams/get-team options
:profile-id profile-id :profile-id profile-id
:project-id project-id) :project-id project-id)
features (cfeat/get-team-enabled-features cf/flags team)]
validate? (contains? cf/flags :file-validation)
features (cfeat/get-team-enabled-features cf/flags team)]
;; Process all sections ;; Process all sections
(run! (fn [section] (run! (fn [section]
@ -651,13 +653,13 @@
(doseq [[feature file-id] (-> *state* deref :pending-to-migrate)] (doseq [[feature file-id] (-> *state* deref :pending-to-migrate)]
(case feature (case feature
"components/v2" "components/v2"
(features.components-v2/migrate-file! options file-id) (features.components-v2/migrate-file! options file-id
:validate? validate?
:throw-on-validate? true)
"fdata/shape-data-type" "fdata/shape-data-type"
nil nil
;; "fdata/shape-data-type"
;; (features.fdata/enable-objects-map
(ex/raise :type :internal (ex/raise :type :internal
:code :no-migration-defined :code :no-migration-defined
:hint (str/ffmt "no migation for feature '%' on file importation" feature) :hint (str/ffmt "no migation for feature '%' on file importation" feature)
@ -774,17 +776,16 @@
;; environments messed up with the version ;; environments messed up with the version
;; numbers When this problem is fixed delete ;; numbers When this problem is fixed delete
;; the following line ;; the following line
(assoc :version 0) (assoc :version 22)
(update :pages-index relink-shapes) (update :pages-index relink-shapes)
(update :components relink-shapes) (update :components relink-shapes)
(update :media relink-media) (update :media relink-media)
(pmg/migrate-data) (pmg/migrate-data)
(d/without-nils)))) (d/without-nils))))
;; Without providing all libs, here we just (cond-> (contains? cf/flags :file-validation)
;; peform a structural file data validation, (fval/validate-file-schema!))
;; full referential check is omited.
(fval/validate-file!)
(postprocess-file) (postprocess-file)
(update :features #(db/create-array conn "text" %)) (update :features #(db/create-array conn "text" %))
(update :data blob/encode))] (update :data blob/encode))]

View file

@ -266,6 +266,7 @@
;; Retrieve and return lagged data ;; Retrieve and return lagged data
(get-lagged-changes conn params)))) (get-lagged-changes conn params))))
(defn- update-file-data (defn- update-file-data
[conn file changes skip-validate] [conn file changes skip-validate]
(let [file (update file :data (fn [data] (let [file (update file :data (fn [data]
@ -278,7 +279,6 @@
;; some other way to do general validation ;; some other way to do general validation
libs (when (and (contains? cf/flags :file-validation) libs (when (and (contains? cf/flags :file-validation)
(not skip-validate)) (not skip-validate))
;; FIXME: we need properly handle pointer-map here ????
(->> (files/get-file-libraries conn (:id file)) (->> (files/get-file-libraries conn (:id file))
(into [file] (map (fn [{:keys [id]}] (into [file] (map (fn [{:keys [id]}]
(binding [pmap/*load-fn* (partial files/load-pointer conn id)] (binding [pmap/*load-fn* (partial files/load-pointer conn id)]
@ -293,7 +293,10 @@
(update :data cpc/process-changes changes) (update :data cpc/process-changes changes)
;; If `libs` is defined, then full validation is performed ;; If `libs` is defined, then full validation is performed
(val/validate-file! libs) (cond-> (and (contains? cf/flags :file-validation)
(not skip-validate))
(-> (val/validate-file! libs)
(val/validate-file-schema!)))
(cond-> (and (contains? cfeat/*current* "fdata/objects-map") (cond-> (and (contains? cfeat/*current* "fdata/objects-map")
(not (contains? cfeat/*previous* "fdata/objects-map"))) (not (contains? cfeat/*previous* "fdata/objects-map")))

View file

@ -238,7 +238,7 @@
(migrate-team [team-id] (migrate-team [team-id]
(try (try
(-> (assoc system ::db/rollback rollback?) (-> (assoc system ::db/rollback rollback?)
(feat/migrate-team! team-id :validate? validate?)) (feat/migrate-team! team-id :validate? validate? :throw-on-validate? (not skip-on-error)))
(catch Throwable cause (catch Throwable cause
(l/err :hint "unexpected error on processing team" :team-id (dm/str team-id) :cause cause)))) (l/err :hint "unexpected error on processing team" :team-id (dm/str team-id) :cause cause))))

View file

@ -6,4 +6,4 @@
(ns app.common.files.defaults) (ns app.common.files.defaults)
(def version 35) (def version 36)

View file

@ -651,3 +651,15 @@
(migrate) (migrate)
(assoc :version 35))) (assoc :version 35)))
(defmethod migrate 36
[data]
(letfn [(update-container [container]
(d/update-when container :objects (fn [objects]
(if (contains? objects nil)
(dissoc objects nil)
objects))))]
(-> data
(update :pages-index update-vals update-container)
(update :components update-vals update-container))))

View file

@ -442,44 +442,40 @@
"Get schema explain data for file data" "Get schema explain data for file data"
(sm/lazy-explainer ::ctf/data)) (sm/lazy-explainer ::ctf/data))
(defn validate-file! (defn validate-file-schema!
"Validate file data structure. [{:keys [id data] :as file}]
(when-not (valid-fdata? data)
(if (some? *errors*)
(vswap! *errors* conj
{:code :invalid-file-data-structure
:hint (str/ffmt "invalid file data structure found on file '%'" id)
:file-id id})
(ex/raise :type :validation
:code :data-validation
:hint (str/ffmt "invalid file data structure found on file '%'" id)
:file-id id
::sm/explain (get-fdata-explain data))))
file)
If libraries are provided, then a full referential integrity and (defn validate-file!
semantic coherence check will be performed on all content of the "Validate full referential integrity and semantic coherence on file data.
file.
Raises a validation exception on first error found." Raises a validation exception on first error found."
[{:keys [data] :as file} libraries]
([file] (validate-file! file nil)) (doseq [page (filter :id (ctpl/pages-seq data))]
([{:keys [data] :as file} libraries] (validate-shape! uuid/zero file page libraries))
;; (when-not (valid-fdata? data)
;; (if (some? *errors*)
;; (vswap! *errors* conj
;; {:code :invalid-file-data-structure
;; :hint (str/ffmt "invalid file data structure found on file '%'" id)
;; :file-id id})
;; (ex/raise :type :validation
;; :code :data-validation
;; :hint (str/ffmt "invalid file data found on file '%'" id)
;; :file-id id
;; ::sm/explain (get-fdata-explain data))))
;; If `libraries` is provided, this means the full file (doseq [component (vals (:components data))]
;; validation is activated so we proceed to execute the (validate-component! component file))
;; validation
(when (some? libraries)
(doseq [page (filter :id (ctpl/pages-seq data))]
(validate-shape! uuid/zero file page libraries))
(doseq [component (vals (:components data))]
(validate-component! component file)))
file)) file)
(defn validate-file (defn validate-file
"Validate referencial integrity and semantic coherence of "Validate structure, referencial integrity and semantic coherence of
all contents of a file. Returns a list of errors." all contents of a file. Returns a list of errors."
[file libraries] [file libraries]
(binding [*errors* (volatile! [])] (binding [*errors* (volatile! [])]
(validate-file-schema! file)
(validate-file! file libraries) (validate-file! file libraries)
(deref *errors*))) (deref *errors*)))