Penpot/penpot
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-02-22 14:57:01 -05:00
Code Issues Activity

🎉 Improve validation and repair

Browse source
This commit is contained in:
Alejandro Alonso 2023-12-11 07:01:24 +01:00 • committed by Andrés Moya
parent 20d4c67bf3
commit ca06263018
2 changed files with 85 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
common/src/app/common/files/repair.cljc
View file

@ -68,7 +68,19 @@
(defmethod repair-error :child-not-found (defmethod repair-error :child-not-found
[_ {:keys [shape page-id args] :as error} file-data _] [_ {:keys [shape page-id args] :as error} file-data _]
(log/info :hint "Repairing shape :child-not-found" :id (:id shape) :name (:name shape) :page-id page-id) (let [repair-shape
(fn [parent-shape]
(log/debug :hint " -> Remove child" :child-id (:child-id args))
(update parent-shape :shapes (fn [shapes]
(d/removev #(= (:child-id args) %) shapes))))]
(log/info :hint "Repairing shape :child-not-found" :id (:id shape) :name (:name shape) :page-id page-id)
(-> (pcb/empty-changes nil page-id)
(pcb/with-file-data file-data)
(pcb/update-shapes [(:id shape)] repair-shape))))
(defmethod repair-error :invalid-parent
[_ {:keys [shape page-id args] :as error} file-data _]
(log/info :hint "Repairing shape :invalid-parent" :id (:id shape) :name (:name shape) :page-id page-id)
(-> (pcb/empty-changes nil page-id) (-> (pcb/empty-changes nil page-id)
(pcb/with-file-data file-data) (pcb/with-file-data file-data)
(pcb/change-parent (:parent-id args) [shape] nil {:component-swap true}))) (pcb/change-parent (:parent-id args) [shape] nil {:component-swap true})))

136
common/src/app/common/files/validate.cljc
View file

@ -6,7 +6,6 @@
(ns app.common.files.validate (ns app.common.files.validate
(:require (:require
[app.common.data.macros :as dm]
[app.common.exceptions :as ex] [app.common.exceptions :as ex]
[app.common.files.helpers :as cfh] [app.common.files.helpers :as cfh]
[app.common.schema :as sm] [app.common.schema :as sm]
@ -35,6 +34,7 @@
:invalid-main-instance-id :invalid-main-instance-id
:invalid-main-instance-page :invalid-main-instance-page
:invalid-main-instance :invalid-main-instance
:invalid-parent
:component-main :component-main
:should-be-component-root :should-be-component-root
:should-not-be-component-root :should-not-be-component-root
@ -123,11 +123,17 @@
(doseq [child-id (:shapes shape)] (doseq [child-id (:shapes shape)]
(let [child (ctst/get-shape page child-id)] (let [child (ctst/get-shape page child-id)]
(when (or (nil? child) (not= (:parent-id child) (:id shape))) (if (nil? child)
(report-error! :child-not-found (report-error! :child-not-found
(str/ffmt "Child % not found" child-id) (str/ffmt "Child % not found in parent %" child-id (:id shape))
child file page shape file page
:parent-id (:id shape))))))))) :parent-id (:id shape)
:child-id child-id)
(when (not= (:parent-id child) (:id shape))
(report-error! :invalid-parent
(str/ffmt "Child % has invalid parent %" child-id (:id shape))
child file page
:parent-id (:id shape))))))))))
(defn validate-frame! (defn validate-frame!
"Validate that the frame-id shape exists and is indeed a frame. Also "Validate that the frame-id shape exists and is indeed a frame. Also
@ -178,9 +184,17 @@
(str/ffmt "Main instance id of component % is not valid" (:component-id shape)) (str/ffmt "Main instance id of component % is not valid" (:component-id shape))
shape file page)) shape file page))
(when-not (= (:main-instance-page component) (:id page)) (when-not (= (:main-instance-page component) (:id page))
(report-error! :invalid-main-instance-page (let [component-page (ctf/get-component-page (:data file) component)
(str/ffmt "Main instance page of component % is not valid" (:component-id shape)) main-component (ctst/get-shape component-page (:main-instance-id component))]
shape file page)))))) ;; We must check if the same component has main instances in different pages.
;; In that case one of those instances shouldn't be main
(if (:main-instance main-component)
(report-error! :component-main
"Shape not expected to be main instance"
shape file page)
(report-error! :invalid-main-instance-page
(str/ffmt "Main instance page of component % is not valid" (:component-id shape))
shape file page))))))))
(defn validate-component-not-main-head! (defn validate-component-not-main-head!
"Validate shape is a not-main instance head, component "Validate shape is a not-main instance head, component
@ -350,69 +364,64 @@
" "
[shape-id file page libraries & {:keys [context] :or {context :not-component}}] [shape-id file page libraries & {:keys [context] :or {context :not-component}}]
(let [shape (ctst/get-shape page shape-id)] (let [shape (ctst/get-shape page shape-id)]
(when (some? shape)
(do
(validate-geometry! shape file page)
(validate-parent-children! shape file page)
(validate-frame! shape file page)
;; If this happens it's a bug in this validate functions (if (ctk/instance-head? shape)
(dm/verify! (if (not= :frame (:type shape))
["Shape % not found" shape-id] (report-error! :instance-head-not-frame
(some? shape)) "Instance head should be a frame"
shape file page)
(validate-geometry! shape file page) (if (ctk/instance-root? shape)
(validate-parent-children! shape file page) (if (ctk/main-instance? shape)
(validate-frame! shape file page) (if (not= context :not-component)
(report-error! :root-main-not-allowed
"Root main component not allowed inside other component"
shape file page)
(validate-shape-main-root-top! shape file page libraries))
(if (ctk/instance-head? shape) (if (not= context :not-component)
(if (not= :frame (:type shape)) (report-error! :root-copy-not-allowed
(report-error! :instance-head-not-frame "Root copy component not allowed inside other component"
"Instance head should be a frame" shape file page)
shape file page) (validate-shape-copy-root-top! shape file page libraries)))
(if (ctk/instance-root? shape) (if (ctk/main-instance? shape)
(if (ctk/main-instance? shape) (if (= context :not-component)
(if (not= context :not-component) (report-error! :nested-main-not-allowed
(report-error! :root-main-not-allowed "Nested main component only allowed inside other component"
"Root main component not allowed inside other component" shape file page)
shape file page) (validate-shape-main-root-nested! shape file page libraries))
(validate-shape-main-root-top! shape file page libraries))
(if (not= context :not-component) (if (= context :not-component)
(report-error! :root-copy-not-allowed (report-error! :nested-copy-not-allowed
"Root copy component not allowed inside other component" "Nested copy component only allowed inside other component"
shape file page) shape file page)
(validate-shape-copy-root-top! shape file page libraries))) (validate-shape-copy-root-nested! shape file page libraries)))))
(if (ctk/main-instance? shape) (if (ctk/in-component-copy? shape)
(if (= context :not-component) (if-not (#{:copy-top :copy-nested :copy-any} context)
(report-error! :nested-main-not-allowed (report-error! :not-head-copy-not-allowed
"Nested main component only allowed inside other component" "Non-root copy only allowed inside a copy"
shape file page) shape file page)
(validate-shape-main-root-nested! shape file page libraries)) (validate-shape-copy-not-root! shape file page libraries))
(if (= context :not-component) (if (ctn/inside-component-main? (:objects page) shape)
(report-error! :nested-copy-not-allowed (if-not (#{:main-top :main-nested :main-any} context)
"Nested copy component only allowed inside other component" (report-error! :not-head-main-not-allowed
shape file page) "Non-root main only allowed inside a main component"
(validate-shape-copy-root-nested! shape file page libraries))))) shape file page)
(validate-shape-main-not-root! shape file page libraries))
(if (ctk/in-component-copy? shape)
(if-not (#{:copy-top :copy-nested :copy-any} context)
(report-error! :not-head-copy-not-allowed
"Non-root copy only allowed inside a copy"
shape file page)
(validate-shape-copy-not-root! shape file page libraries))
(if (ctn/inside-component-main? (:objects page) shape)
(if-not (#{:main-top :main-nested :main-any} context)
(report-error! :not-head-main-not-allowed
"Non-root main only allowed inside a main component"
shape file page)
(validate-shape-main-not-root! shape file page libraries))
(if (#{:main-top :main-nested :main-any} context)
(report-error! :not-component-not-allowed
"Not compoments are not allowed inside a main"
shape file page)
(validate-shape-not-component! shape file page libraries)))))))
(if (#{:main-top :main-nested :main-any} context)
(report-error! :not-component-not-allowed
"Not compoments are not allowed inside a main"
shape file page)
(validate-shape-not-component! shape file page libraries)))))))))
(defn validate-shape (defn validate-shape
"Validate a shape and all its children. Returns a list of errors." "Validate a shape and all its children. Returns a list of errors."
[shape-id file page libraries] [shape-id file page libraries]
@ -464,7 +473,6 @@
Raises a validation exception on first error found." Raises a validation exception on first error found."
[{:keys [data features] :as file} libraries] [{:keys [data features] :as file} libraries]
(when (contains? features "components/v2") (when (contains? features "components/v2")
(doseq [page (filter :id (ctpl/pages-seq data))] (doseq [page (filter :id (ctpl/pages-seq data))]
(validate-shape! uuid/zero file page libraries)) (validate-shape! uuid/zero file page libraries))