diff --git a/.gitignore b/.gitignore index 286a2c1b94..ca6b947232 100644 --- a/.gitignore +++ b/.gitignore @@ -95,3 +95,4 @@ /.idea /.claude /.playwright-mcp +/tools/__pycache__ diff --git a/.opencode/skills/update-changelog/SKILL.md b/.opencode/skills/update-changelog/SKILL.md index 29c3d295b4..a65e90a607 100644 --- a/.opencode/skills/update-changelog/SKILL.md +++ b/.opencode/skills/update-changelog/SKILL.md @@ -45,10 +45,17 @@ python3 tools/gh.py issues "2.16.0" --state all python3 tools/gh.py issues "2.16.0" --exclude "release blocker,no changelog" ``` -**Exclusion rules:** +**Exclusion rules (issue-level):** - `no changelog` label — Chore/refactor work that doesn't need a changelog entry +- `release blocker` label — Blocked issues not yet ready for changelog - `Task` issue type — Internal chores are not user-facing; filter these out after fetching +**Exclusion rules (PR-level):** +In addition to issue-level exclusions, PRs with these labels should be +excluded regardless of their linked issue's labels: +- `release blocker` — PR is part of a pending release blocker batch +- `no issue required` — Trivial fix not tracked as an issue + The script outputs JSON with each entry containing `number`, `title`, `state`, `issue_type`, `labels`, and `closing_prs` (the PRs that fix each issue). @@ -63,6 +70,10 @@ python3 tools/gh.py issues "2.16.0" --exclude "release blocker,no changelog" --c This returns a filtered JSON array with only the missing issues. +> **Note:** The `--compare` flag checks **issues** only (via issue number +> references in the changelog). To find merged **PRs** not yet referenced, +> use the milestone PR cross-reference described in step 10 below. + ### 4. Fetch additional PR details when needed When you need more context for specific PRs (e.g. to find the PR author for @@ -80,9 +91,20 @@ python3 tools/gh.py prs --file prs.txt cat prs.txt | python3 tools/gh.py prs --stdin ``` +The `prs` command also supports listing all PRs in a milestone in one call: + +```bash +# All merged PRs in a milestone (default) +python3 tools/gh.py prs --milestone "2.16.0" + +# All states (merged, open, closed) +python3 tools/gh.py prs --milestone "2.16.0" --state all +``` + The `prs` command returns JSON with `number`, `title`, `body`, `state`, `merged_at`, `author`, `labels`, and `closing_issues`. PRs are fetched in -batches of 50 via GraphQL to stay within API limits. +batches of 50 via GraphQL to stay within API limits (milestone mode uses +paginated GraphQL on the milestone's `pullRequests` connection). ### 5. Categorize entries — strictly by issue type, never by labels or emoji @@ -134,6 +156,37 @@ tracked in the milestone. | PR exists with no linked issue | If a corresponding closed issue exists in the same milestone, link the issue. Otherwise, skip the entry (the issue must be the changelog unit). | | Closed issue with no fix PR in milestone | Link the issue directly, without a PR reference. | +> **False-positive associations:** A PR may incorrectly claim to close an issue +> from a different context (e.g., a very old PR referencing a modern issue, or a +> cross-project reference). If the PR title and issue title are clearly unrelated, +> or the PR was created years before the issue, treat it as a data glitch and +> skip it. PR [#3](https://github.com/penpot/penpot/pull/3) (ancient License PR +> claiming to close a plugin API issue) is a known example. + +### 5a. ⚠️ Verify PR merge status before writing + +A closed issue may list closing PRs that were **closed without merging** +(e.g., a community PR that was superseded by another). The changelog must +only reference **merged** PRs. Verify before writing: + +```bash +# Collect all PR numbers from the candidate entries and check them +python3 tools/gh.py prs | python3 -c " +import json, sys +for pr in json.load(sys.stdin): + if pr['state'] != 'MERGED': + print(f'WARNING: #{pr[\"number\"]} is {pr[\"state\"]} (not merged)') +" +``` + +If a closing PR is closed-unmerged, find the actual merged PR that +superseded it: +1. Check the issue's closing PRs list for other PRs (there may be multiple) +2. Look for other PRs with similar titles or descriptions referencing the same issue +3. Inspect the closed PR's conversation timeline for a pointer to the replacement + +Replace the reference in the changelog entry with the correct merged PR number. + ### 6. Read the current CHANGES.md Read the top of `CHANGES.md` to understand the existing format and find the @@ -198,6 +251,72 @@ Read the top of `CHANGES.md` and confirm: - The section ordering is correct (newest first) - Formatting matches the surrounding entries +### 10. Cross-reference milestone PRs against the changelog + +Issues can be fixed by PRs that aren't in the milestone, and merged PRs in +the milestone may not close any tracked issue. After writing, run a full +cross-reference to catch gaps: + +```bash +# List all merged PRs in the milestone +python3 tools/gh.py prs --milestone "" --state merged > /tmp/milestone-prs.json + +# Extract PR numbers from the changelog section +python3 -c " +import json, re + +with open('CHANGES.md') as f: + content = f.read() + +# Extract the version section (adjust regex to match the actual version) +match = re.search(r'## \(Unreleased\)\n(.*?)(?:\n## |\Z)', content, re.DOTALL) +section = match.group(1) + +# Collect all PR numbers referenced +changelog_prs = set() +for m in re.findall(r'\[#(\d+)\]\(https://github\.com/penpot/penpot/pull/\d+\)', section): + changelog_prs.add(int(m)) + +# Collect all milestone PRs (filtered) +with open('/tmp/milestone-prs.json') as f: + milestone_prs = json.load(f) + +milestone_merged = {pr['number'] for pr in milestone_prs} + +# PRs in milestone but not in changelog +missing = sorted(milestone_merged - changelog_prs) +print(f'Milestone merged PRs: {len(milestone_merged)}') +print(f'Changelog referenced PRs: {len(changelog_prs)}') +print(f'PRs in milestone but NOT in changelog: {len(missing)}') +for num in missing: + pr = next(p for p in milestone_prs if p['number'] == num) + print(f' #{num} {pr[\"title\"][:80]}') +" +``` + +For each missing PR found, decide whether it should be added to the +changelog or is legitimately excluded (check its labels). + +Also verify that no closed-unmerged PRs remain in the changelog: + +```bash +python3 tools/gh.py prs --milestone "" --state all | python3 -c " +import json, sys +data = json.load(sys.stdin) +closed = [p for p in data if p['state'] == 'CLOSED'] +if closed: + print('WARNING: CLOSED (unmerged) PRs in milestone:') + for p in closed: + print(f' #{p[\"number\"]} {p[\"title\"][:80]}') +" +``` + +**Post-edit audit checklist:** +- ✅ All referenced PRs are merged (no closed-unmerged artifacts) +- ✅ Every merged milestone PR is either in the changelog or excluded by label +- ✅ PR and issue counts are internally consistent +- ✅ No false-positive PR-to-issue associations + ## Version section template ```markdown @@ -244,3 +363,17 @@ Read the top of `CHANGES.md` and confirm: - **Use `tools/gh.py`.** Prefer the helper script over raw `gh api` calls for milestone issue listing and PR detail fetching. It handles GraphQL pagination, batching, and label filtering automatically. +- **Verify PR merge status.** Not all closing PRs are merged — community PRs + can be superseded and closed without merging. Always check that every PR + referenced in the changelog has `state: MERGED`. +- **PR-level exclusions apply.** A PR can carry its own exclusion labels + (`release blocker`, `no issue required`) independent of its linked issue's + labels. Check both. +- **Cross-reference milestone PRs, not just issues.** The `--compare` flag on + the `issues` command only compares issue numbers. Merged PRs not linked to + any milestone issue can be missed. Use `python3 tools/gh.py prs --milestone` + for a full PR cross-reference. +- **False-positive PR-to-issue associations.** A PR may claim to close an + issue from a different project or context. If the PR title and issue title + are clearly unrelated, or the PR predates the issue by years, treat it as a + data glitch and skip it. diff --git a/CHANGES.md b/CHANGES.md index 24aa714417..f98aa1c8c5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -82,11 +82,11 @@ - Add clear artboard guides to frame context menu (by @eureka0928) [#6987](https://github.com/penpot/penpot/issues/6987) (PR: [#8936](https://github.com/penpot/penpot/pull/8936)) - Add loader feedback while importing and exporting files (by @moorsecopers99) [#9020](https://github.com/penpot/penpot/issues/9020) (PR: [#9024](https://github.com/penpot/penpot/pull/9024)) - Allow duplicating color and typography styles (by @MkDev11) [#2912](https://github.com/penpot/penpot/issues/2912) (PR: [#8449](https://github.com/penpot/penpot/pull/8449)) -- Add woff2 support on user uploaded fonts (by @Nivl) [#3521](https://github.com/penpot/penpot/issues/3521) (PR: [#8248](https://github.com/penpot/penpot/pull/8248)) -- Import Tokens from linked library (by @dfelinto) [#9635](https://github.com/penpot/penpot/issues/9635) (PR: [#8391](https://github.com/penpot/penpot/pull/8391)) -- Option to download custom fonts (by @dfelinto) [#9672](https://github.com/penpot/penpot/issues/9672) (PR: [#8320](https://github.com/penpot/penpot/pull/8320)) -- Add copy as image to workspace context menu (by @dfelinto) [#9607](https://github.com/penpot/penpot/issues/9607) (PR: [#8313](https://github.com/penpot/penpot/pull/8313)) -- Add Tab/Shift+Tab navigation to rename layers sequentially (by @bittoby) [#2569](https://github.com/penpot/penpot/issues/2569) (PR: [#8474](https://github.com/penpot/penpot/pull/8474)) +- Add woff2 support on user uploaded fonts (by @Nivl) [#3521](https://github.com/penpot/penpot/issues/3521) (PR: [#8367](https://github.com/penpot/penpot/pull/8367)) +- Import Tokens from linked library (by @dfelinto) [#9635](https://github.com/penpot/penpot/issues/9635) (PR: [#8439](https://github.com/penpot/penpot/pull/8439)) +- Option to download custom fonts (by @dfelinto) [#9672](https://github.com/penpot/penpot/issues/9672) (PR: [#8335](https://github.com/penpot/penpot/pull/8335)) +- Add copy as image to workspace context menu (by @dfelinto) [#9607](https://github.com/penpot/penpot/issues/9607) (PR: [#8364](https://github.com/penpot/penpot/pull/8364), [#9586](https://github.com/penpot/penpot/pull/9586)) +- Add Tab/Shift+Tab navigation to rename layers sequentially (by @bittoby) [#2569](https://github.com/penpot/penpot/issues/2569) (PR: [#8506](https://github.com/penpot/penpot/pull/8506)) - Copy and paste entire rows in existing table (by @bittoby) [#5969](https://github.com/penpot/penpot/issues/5969) (PR: [#8498](https://github.com/penpot/penpot/pull/8498)) - Rename token group [#9637](https://github.com/penpot/penpot/issues/9637) (PR: [#8275](https://github.com/penpot/penpot/pull/8275)) - Duplicate token group [#9638](https://github.com/penpot/penpot/issues/9638) (PR: [#8886](https://github.com/penpot/penpot/pull/8886)) @@ -94,7 +94,7 @@ - Add drag-to-change for numeric inputs in workspace sidebar (by @RenzoMXD) [#2466](https://github.com/penpot/penpot/issues/2466) (PR: [#8536](https://github.com/penpot/penpot/pull/8536)) - Add CSS linter [#9636](https://github.com/penpot/penpot/issues/9636) (PR: [#8592](https://github.com/penpot/penpot/pull/8592)) - Add per-group add button for typographies (by @eureka0928) [#5275](https://github.com/penpot/penpot/issues/5275) (PR: [#8895](https://github.com/penpot/penpot/pull/8895)) -- Add Find & Replace for text content and layer names (by @statxc) [#7108](https://github.com/penpot/penpot/issues/7108) (PR: [#8899](https://github.com/penpot/penpot/pull/8899)) +- Add Find & Replace for text content and layer names (by @statxc) [#7108](https://github.com/penpot/penpot/issues/7108) (PR: [#8899](https://github.com/penpot/penpot/pull/8899), [#9687](https://github.com/penpot/penpot/pull/9687)) - Use page name for multi-export ZIP/PDF downloads (by @Dexterity104) [#8773](https://github.com/penpot/penpot/issues/8773) (PR: [#8874](https://github.com/penpot/penpot/pull/8874)) - Make links in comments clickable (by @eureka0928) [#1602](https://github.com/penpot/penpot/issues/1602) (PR: [#8894](https://github.com/penpot/penpot/pull/8894)) - Add visibility toggle for strokes (by @eureka0928) [#7438](https://github.com/penpot/penpot/issues/7438) (PR: [#8913](https://github.com/penpot/penpot/pull/8913)) @@ -115,7 +115,7 @@ - Add HEX/HSB/HSL support to color picker with persistent model switcher (by @edwin-rivera-dev) [#9133](https://github.com/penpot/penpot/issues/9133) (PR: [#9134](https://github.com/penpot/penpot/pull/9134)) - Show specific invitation-link error messages (by @niwinz) [#9220](https://github.com/penpot/penpot/issues/9220) (PR: [#9223](https://github.com/penpot/penpot/pull/9223)) - Show detailed file import error messages (by @jsdevninja) [#8212](https://github.com/penpot/penpot/issues/8212) (PR: [#9004](https://github.com/penpot/penpot/pull/9004)) -- Add read-only preview mode for saved versions (by @wdeveloper16) [#7622](https://github.com/penpot/penpot/issues/7622) (PR: [#8976](https://github.com/penpot/penpot/pull/8976)) +- Add read-only preview mode for saved versions (by @wdeveloper16) [#7622](https://github.com/penpot/penpot/issues/7622) (PR: [#8976](https://github.com/penpot/penpot/pull/8976), [#9514](https://github.com/penpot/penpot/pull/9514)) - Add clipboard read/write permissions to the plugin system (by @wdeveloper16) [#6980](https://github.com/penpot/penpot/issues/6980) (PR: [#9053](https://github.com/penpot/penpot/pull/9053)) - Update auth hero illustration on login screen [#9532](https://github.com/penpot/penpot/issues/9532) (PR: [#9552](https://github.com/penpot/penpot/pull/9552)) - Update Open Graph link preview metadata [#9555](https://github.com/penpot/penpot/issues/9555) (PR: [#9557](https://github.com/penpot/penpot/pull/9557)) @@ -123,6 +123,9 @@ - Preserve Inkscape labels when pasting SVGs (by @jeffrey701) [#7869](https://github.com/penpot/penpot/issues/7869) (PR: [#9252](https://github.com/penpot/penpot/pull/9252)) - Add Alt+click to expand layer subtree (by @MilosM348) [#7736](https://github.com/penpot/penpot/issues/7736) (PR: [#9179](https://github.com/penpot/penpot/pull/9179)) - Allow deleting the profile avatar after uploading (by @moorsecopers99) [#9067](https://github.com/penpot/penpot/issues/9067) (PR: [#9068](https://github.com/penpot/penpot/pull/9068)) +- Clarify self-hosted OIDC configuration for containerized (by @sancfc) [#9764](https://github.com/penpot/penpot/issues/9764) (PR: [#9758](https://github.com/penpot/penpot/pull/9758)) +- Update User Guide with 2.16 features (by @myfunnyandy) [#9767](https://github.com/penpot/penpot/issues/9767) (PR: [#9768](https://github.com/penpot/penpot/pull/9768)) +- Improve file validation performance and fix orphan shape detection [#9790](https://github.com/penpot/penpot/issues/9790) (PR: [#9789](https://github.com/penpot/penpot/pull/9789)) ### :bug: Bugs fixed @@ -132,7 +135,7 @@ - Add natural sorting on token names [#8635](https://github.com/penpot/penpot/issues/8635) (PR: [#8672](https://github.com/penpot/penpot/pull/8672)) - Fix warnings for unsupported token $type (by @Dexterity104) [#8790](https://github.com/penpot/penpot/issues/8790) (PR: [#8873](https://github.com/penpot/penpot/pull/8873)) - Apply styles to selection (by @AzazelN28) [#9661](https://github.com/penpot/penpot/issues/9661) (PR: [#8625](https://github.com/penpot/penpot/pull/8625)) -- Fix Alt/Option to draw shapes from center point (by @offreal) [#8360](https://github.com/penpot/penpot/issues/8360) (PR: [#8361](https://github.com/penpot/penpot/pull/8361)) +- Fix Alt/Option to draw shapes from center point (by @offreal) [#8360](https://github.com/penpot/penpot/issues/8360) (PR: [#8381](https://github.com/penpot/penpot/pull/8381)) - Fix library update button freezing [#9330](https://github.com/penpot/penpot/issues/9330) (PR: [#9513](https://github.com/penpot/penpot/pull/9513)) - Fix typo in subscription settings success key (by @jack-stormentswe) [#9203](https://github.com/penpot/penpot/issues/9203) (PR: [#9204](https://github.com/penpot/penpot/pull/9204)) - Add token name on broken token pill on sidebar [#9534](https://github.com/penpot/penpot/issues/9534) (PR: [#8527](https://github.com/penpot/penpot/pull/8527)) @@ -200,8 +203,16 @@ - Fix resize cursor appearing on login and register buttons [#9505](https://github.com/penpot/penpot/issues/9505) (PR: [#9590](https://github.com/penpot/penpot/pull/9590)) - Fix version restore restoring first previewed version instead of selected one [#9588](https://github.com/penpot/penpot/issues/9588) (PR: [#9626](https://github.com/penpot/penpot/pull/9626)) - Fix incorrect error message when applying tokens while editing text [#9620](https://github.com/penpot/penpot/issues/9620) (PR: [#9708](https://github.com/penpot/penpot/pull/9708)) +- Fix standalone tokens ordering separated from token groups [#9733](https://github.com/penpot/penpot/issues/9733) (PR: [#9736](https://github.com/penpot/penpot/pull/9736)) +- Fix delete invitation modal readability in light theme [#9737](https://github.com/penpot/penpot/issues/9737) (PR: [#9747](https://github.com/penpot/penpot/pull/9747)) +- Fix team invitation not automatically accepted after account validation [#9776](https://github.com/penpot/penpot/issues/9776) (PR: [#9782](https://github.com/penpot/penpot/pull/9782)) -## 2.15.4 (Unreleased) + +## 2.15.4 + +### :sparkles: New features & Enhancements + +- Add rate limiting and concurrency safety for file snapshot operations [#9723](https://github.com/penpot/penpot/issues/9723) (PR: [#9722](https://github.com/penpot/penpot/pull/9722)) ### :bug: Bugs fixed @@ -210,6 +221,7 @@ - Fix API doc endpoint returning HTML as text/plain [#9680](https://github.com/penpot/penpot/issues/9680) (PR: [#9681](https://github.com/penpot/penpot/pull/9681)) - Fix unexpected error when opening the export dialog [#9721](https://github.com/penpot/penpot/issues/9721) (PR: [#9704](https://github.com/penpot/penpot/pull/9704)) + ## 2.15.3 ### :bug: Bugs fixed diff --git a/backend/resources/climit.edn b/backend/resources/climit.edn index 34d2184153..7d8234499b 100644 --- a/backend/resources/climit.edn +++ b/backend/resources/climit.edn @@ -19,7 +19,7 @@ {:permits 40} :root/by-profile - {:permits 10} + {:permits 10 :queue 30 :timeout 30000} :file-thumbnail-ops/global {:permits 20} @@ -27,4 +27,16 @@ {:permits 2} :submit-audit-events/by-profile - {:permits 1 :queue 3}} + {:permits 1 :queue 3} + + :restore-file-snapshot/global + {:permits 3} + + :restore-file-snapshot/by-profile + {:permits 1 :queue 2 :timeout 60000} + + :create-file-snapshot/global + {:permits 3} + + :create-file-snapshot/by-profile + {:permits 1 :queue 2 :timeout 60000}} diff --git a/backend/src/app/db.clj b/backend/src/app/db.clj index c23ea07524..de80175040 100644 --- a/backend/src/app/db.clj +++ b/backend/src/app/db.clj @@ -27,7 +27,9 @@ [next.jdbc.transaction]) (:import com.zaxxer.hikari.HikariConfig + com.zaxxer.hikari.HikariConfigMXBean com.zaxxer.hikari.HikariDataSource + com.zaxxer.hikari.HikariPoolMXBean com.zaxxer.hikari.metrics.prometheus.PrometheusMetricsTrackerFactory io.whitfin.siphash.SipHasher io.whitfin.siphash.SipHasherContainer @@ -67,9 +69,8 @@ (def defaults {::name :main - ::min-size 0 ::max-size 60 - ::connection-timeout 10000 + ::connection-timeout 30000 ::validation-timeout 10000 ::idle-timeout 120000 ; 2min ::max-lifetime 1800000 ; 30m @@ -82,7 +83,7 @@ (defmethod ig/init-key ::pool [_ cfg] (let [{:keys [::uri ::read-only] :as cfg} - (merge defaults cfg)] + (merge defaults (d/without-nils cfg))] (when uri (l/info :hint "initialize connection pool" :name (d/name (::name cfg)) @@ -90,7 +91,8 @@ :read-only read-only :credentials (and (contains? cfg ::username) (contains? cfg ::password)) - :min-size (::min-size cfg) + :min-size (or (::min-size cfg) + (::max-size cfg)) :max-size (::max-size cfg)) (create-pool cfg)))) @@ -111,7 +113,9 @@ [{:keys [::uri] :as cfg}] ;; (app.common.pprint/pprint cfg) - (let [config (HikariConfig.)] + (let [config (HikariConfig.) + max-size (::max-size cfg) + min-size (or (::min-size cfg) max-size)] (doto config (.setJdbcUrl (str "jdbc:" uri)) (.setPoolName (d/name (::name cfg))) @@ -121,8 +125,8 @@ (.setValidationTimeout (::validation-timeout cfg)) (.setIdleTimeout (::idle-timeout cfg)) (.setMaxLifetime (::max-lifetime cfg)) - (.setMinimumIdle (::min-size cfg)) - (.setMaximumPoolSize (::max-size cfg)) + (.setMinimumIdle min-size) + (.setMaximumPoolSize max-size) (.setConnectionInitSql initsql) (.setInitializationFailTimeout -1)) @@ -180,6 +184,20 @@ :code :invalid-connection :hint "invalid connection provided"))) +(defn pool-stats + "Given a HikariDataSource instance, returns a map with current pool + statistics: active/idle connections, threads awaiting connection, + total connections, maximum pool size, and minimum idle connections." + [^HikariDataSource pool] + (let [^HikariPoolMXBean pool-mxbean (.getHikariPoolMXBean pool) + ^HikariConfigMXBean cfg-mxbean (.getHikariConfigMXBean pool)] + {:active-connections (.getActiveConnections pool-mxbean) + :idle-connections (.getIdleConnections pool-mxbean) + :threads-awaiting-connection (.getThreadsAwaitingConnection pool-mxbean) + :total-connections (.getTotalConnections pool-mxbean) + :maximum-pool-size (.getMaximumPoolSize cfg-mxbean) + :minimum-idle (.getMinimumIdle cfg-mxbean)})) + (defn create-pool [cfg] (let [dsc (create-datasource-config cfg)] diff --git a/backend/src/app/features/file_snapshots.clj b/backend/src/app/features/file_snapshots.clj index e013b90d00..b347367003 100644 --- a/backend/src/app/features/file_snapshots.clj +++ b/backend/src/app/features/file_snapshots.clj @@ -66,11 +66,6 @@ LEFT JOIN file_data AS fd ON (fd.file_id = f.id AND fd.id = f.id) WHERE f.id = ?") -(defn- get-minimal-file - [cfg id & {:as opts}] - (-> (db/get-with-sql cfg [sql:get-minimal-file id] opts) - (d/update-when :metadata fdata/decode-metadata))) - (def ^:private sql:get-snapshot-without-data (str "WITH snapshots AS (" sql:snapshots ")" "SELECT c.id, @@ -112,7 +107,7 @@ THEN (c.deleted_at IS NULL OR c.deleted_at >= ?::timestamptz) END")) -(defn get-snapshot-data +(defn get-snapshot "Get a fully decoded snapshot for read-only preview or restoration. Returns the snapshot map with decoded :data field." [cfg file-id snapshot-id] @@ -320,79 +315,87 @@ (defn restore! [{:keys [::db/conn] :as cfg} file-id snapshot-id] - (let [file (get-minimal-file conn file-id {::db/for-update true}) - vern (rand-int Integer/MAX_VALUE) + (let [lock-sql (str sql:get-minimal-file " FOR UPDATE OF f SKIP LOCKED") + row (db/exec-one! conn [lock-sql file-id])] - storage - (sto/resolve cfg {::db/reuse-conn true}) + (when-not row + (ex/raise :type :conflict + :code :file-locked + :hint "the file is currently locked by another operation, retry later")) - snapshot - (get-snapshot-data cfg file-id snapshot-id)] + (let [file (d/update-when row :metadata fdata/decode-metadata) + vern (rand-int Integer/MAX_VALUE) - (when-not snapshot - (ex/raise :type :not-found - :code :snapshot-not-found - :hint "unable to find snapshot with the provided label" - :snapshot-id snapshot-id - :file-id file-id)) + storage + (sto/resolve cfg {::db/reuse-conn true}) - (when-not (:data snapshot) - (ex/raise :type :internal - :code :snapshot-without-data - :hint "snapshot has no data" - :label (:label snapshot) - :file-id file-id)) + snapshot + (get-snapshot cfg file-id snapshot-id)] - (let [;; If the snapshot has applied migrations stored, we reuse - ;; them, if not, we take a safest set of migrations as - ;; starting point. This is because, at the time of - ;; implementing snapshots, migrations were not taken into - ;; account so we need to make this backward compatible in - ;; some way. - migrations - (or (:migrations snapshot) - (fmg/generate-migrations-from-version 67)) + (when-not snapshot + (ex/raise :type :not-found + :code :snapshot-not-found + :hint "unable to find snapshot with the provided label" + :snapshot-id snapshot-id + :file-id file-id)) - file - (-> file - (update :revn inc) - (assoc :migrations migrations) - (assoc :data (:data snapshot)) - (assoc :vern vern) - (assoc :version (:version snapshot)) - (assoc :has-media-trimmed false) - (assoc :modified-at (:modified-at snapshot)) - (assoc :features (:features snapshot)))] + (when-not (:data snapshot) + (ex/raise :type :internal + :code :snapshot-without-data + :hint "snapshot has no data" + :label (:label snapshot) + :file-id file-id)) - (l/dbg :hint "restoring snapshot" - :file-id (str file-id) - :label (:label snapshot) - :snapshot-id (str (:id snapshot))) + (let [;; If the snapshot has applied migrations stored, we reuse + ;; them, if not, we take a safest set of migrations as + ;; starting point. This is because, at the time of + ;; implementing snapshots, migrations were not taken into + ;; account so we need to make this backward compatible in + ;; some way. + migrations + (or (:migrations snapshot) + (fmg/generate-migrations-from-version 67)) - ;; In the same way, on reseting the file data, we need to restore - ;; the applied migrations on the moment of taking the snapshot - (bfc/update-file! cfg file ::bfc/reset-migrations? true) + file + (-> file + (update :revn inc) + (assoc :migrations migrations) + (assoc :data (:data snapshot)) + (assoc :vern vern) + (assoc :version (:version snapshot)) + (assoc :has-media-trimmed false) + (assoc :modified-at (:modified-at snapshot)) + (assoc :features (:features snapshot)))] - ;; FIXME: this should be separated functions, we should not have - ;; inline sql here. + (l/dbg :hint "restoring snapshot" + :file-id (str file-id) + :label (:label snapshot) + :snapshot-id (str (:id snapshot))) - ;; clean object thumbnails - (let [sql (str "update file_tagged_object_thumbnail " - " set deleted_at = now() " - " where file_id=? returning media_id") - res (db/exec! conn [sql file-id])] - (doseq [media-id (into #{} (keep :media-id) res)] - (sto/touch-object! storage media-id))) + ;; In the same way, on reseting the file data, we need to restore + ;; the applied migrations on the moment of taking the snapshot + (bfc/update-file! cfg file ::bfc/reset-migrations? true) - ;; clean file thumbnails - (let [sql (str "update file_thumbnail " - " set deleted_at = now() " - " where file_id=? returning media_id") - res (db/exec! conn [sql file-id])] - (doseq [media-id (into #{} (keep :media-id) res)] - (sto/touch-object! storage media-id))) + ;; FIXME: this should be separated functions, we should not have + ;; inline sql here. - vern))) + ;; clean object thumbnails + (let [sql (str "update file_tagged_object_thumbnail " + " set deleted_at = now() " + " where file_id=? returning media_id") + res (db/exec! conn [sql file-id])] + (doseq [media-id (into #{} (keep :media-id) res)] + (sto/touch-object! storage media-id))) + + ;; clean file thumbnails + (let [sql (str "update file_thumbnail " + " set deleted_at = now() " + " where file_id=? returning media_id") + res (db/exec! conn [sql file-id])] + (doseq [media-id (into #{} (keep :media-id) res)] + (sto/touch-object! storage media-id))) + + vern)))) (defn delete! [cfg & {:keys [id file-id deleted-at]}] diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 940775bdf0..2c154ea95c 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -154,8 +154,8 @@ ::db/username (cf/get :database-username) ::db/password (cf/get :database-password) ::db/read-only (cf/get :database-readonly false) - ::db/min-size (cf/get :database-min-pool-size 0) - ::db/max-size (cf/get :database-max-pool-size 60) + ::db/min-size (cf/get :database-min-pool-size) + ::db/max-size (cf/get :database-max-pool-size) ::mtx/metrics (ig/ref ::mtx/metrics)} ;; Default netty IO pool (shared between several services) diff --git a/backend/src/app/rpc/commands/files_snapshot.clj b/backend/src/app/rpc/commands/files_snapshot.clj index 7736b66cd9..7ac6d51cf4 100644 --- a/backend/src/app/rpc/commands/files_snapshot.clj +++ b/backend/src/app/rpc/commands/files_snapshot.clj @@ -18,6 +18,7 @@ [app.main :as-alias main] [app.msgbus :as mbus] [app.rpc :as-alias rpc] + [app.rpc.climit :as-alias climit] [app.rpc.commands.files :as files] [app.rpc.commands.teams :as teams] [app.rpc.doc :as-alias doc] @@ -54,7 +55,7 @@ [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id file-id id] :as params}] (let [perms (bfc/get-file-permissions conn profile-id file-id)] (files/check-read-permissions! perms) - (let [snapshot (fsnap/get-snapshot-data cfg file-id id)] + (let [snapshot (fsnap/get-snapshot cfg file-id id)] (when-not snapshot (ex/raise :type :not-found :code :snapshot-not-found @@ -81,9 +82,10 @@ (sv/defmethod ::create-file-snapshot {::doc/added "1.20" ::sm/params schema:create-file-snapshot - ::db/transaction true} - [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id file-id label]}] - (files/check-edition-permissions! conn profile-id file-id) + ::climit/id [[:create-file-snapshot/by-profile ::rpc/profile-id] + [:create-file-snapshot/global]]} + [cfg {:keys [::rpc/profile-id file-id label]}] + (files/check-edition-permissions! cfg profile-id file-id) (let [file (bfc/get-file cfg file-id :realize? true) project (db/get-by-id cfg :project (:project-id file))] @@ -95,10 +97,10 @@ (quotes/check! {::quotes/id ::quotes/snapshots-per-file} {::quotes/id ::quotes/snapshots-per-team})) - (fsnap/create! cfg file - {:label label - :profile-id profile-id - :created-by "user"}))) + (db/tx-run! cfg fsnap/create! file + {:label label + :profile-id profile-id + :created-by "user"}))) (def ^:private schema:restore-file-snapshot [:map {:title "restore-file-snapshot"} @@ -108,29 +110,43 @@ (sv/defmethod ::restore-file-snapshot {::doc/added "1.20" ::sm/params schema:restore-file-snapshot - ::db/transaction true} - [{:keys [::db/conn ::mbus/msgbus] :as cfg} {:keys [::rpc/profile-id ::rpc/session-id file-id id] :as params}] - (files/check-edition-permissions! conn profile-id file-id) + ::climit/id [[:restore-file-snapshot/by-profile ::rpc/profile-id] + [:restore-file-snapshot/global]]} + [{:keys [::db/pool ::mbus/msgbus] :as cfg} {:keys [::rpc/profile-id ::rpc/session-id file-id id] :as params}] + + ;; Check permissions and read current file state (short-lived, outside restore transaction) + (files/check-edition-permissions! pool profile-id file-id) (let [file (bfc/get-file cfg file-id) - team (teams/get-team conn + team (teams/get-team pool :profile-id profile-id :file-id file-id) - delay (ldel/get-deletion-delay team)] + delay (ldel/get-deletion-delay team) + file-revn (:revn file)] + ;; Create backup snapshot of the current state (committed immediately + ;; independently of the restore outcome) (fsnap/create! cfg file {:profile-id profile-id :deleted-at (ct/in-future delay) :created-by "system"}) - (let [vern (fsnap/restore! cfg file-id id)] - ;; Send to the clients a notification to reload the file - (mbus/pub! msgbus - :topic (:id file) - :message {:type :file-restored - :session-id session-id - :file-id (:id file) - :vern vern}) - nil))) + ;; Restore snapshot inside its own transaction; the revn check + ;; ensures no data is lost if the file was edited concurrently + (db/tx-run! cfg + (fn [{:keys [::db/conn] :as cfg}] + (let [current (bfc/get-minimal-file conn file-id {::db/for-update true})] + (when (not= (:revn current) file-revn) + (ex/raise :type :conflict + :code :file-modified + :hint "the file was modified during the restore process, please retry"))) + (let [vern (fsnap/restore! cfg file-id id)] + (mbus/pub! msgbus + :topic (:id file) + :message {:type :file-restored + :session-id session-id + :file-id (:id file) + :vern vern}) + nil))))) (def ^:private schema:update-file-snapshot [:map {:title "update-file-snapshot"} diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 94d1f90096..e70ed93f41 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -101,6 +101,17 @@ :mod-typography :del-typography}) +(def ^:private token-change-types + #{:set-tokens-lib + :set-token + :set-token-set + :set-token-theme + :set-active-token-themes + :rename-token-set-group + :move-token-set + :move-token-set-group + :set-base-font-size}) + (def ^:private file-change-types #{:add-obj :mod-obj @@ -111,6 +122,7 @@ (defn- library-change? [{:keys [type] :as change}] (or (contains? library-change-types type) + (contains? token-change-types type) (contains? file-change-types type))) ;; If features are specified from params and the final feature @@ -367,19 +379,16 @@ (l/error :hint "file schema validation error" :cause cause)))) (defn- soft-validate-file! - [file libs] + [file libs changes] (try - (val/validate-file! file libs) + (val/validate-file-affected! file libs changes) (catch Throwable cause (l/error :hint "file validation error" :cause cause)))) - (defn- process-changes-and-validate [cfg file changes skip-validate] - (let [;; WARNING: this ruins performance; maybe we need to find - ;; some other way to do general validation - libs + (let [libs (when (and (or (contains? cf/flags :file-validation) (contains? cf/flags :soft-file-validation)) (not skip-validate)) @@ -407,14 +416,14 @@ (binding [pmap/*tracked* nil] (when (contains? cf/flags :soft-file-validation) - (soft-validate-file! file libs)) + (soft-validate-file! file libs changes)) (when (contains? cf/flags :soft-file-schema-validation) (soft-validate-file-schema! file)) (when (and (contains? cf/flags :file-validation) (not skip-validate)) - (val/validate-file! file libs)) + (val/validate-file-affected! file libs changes)) (when (and (contains? cf/flags :file-schema-validation) (not skip-validate)) diff --git a/backend/test/backend_tests/db_test.clj b/backend/test/backend_tests/db_test.clj new file mode 100644 index 0000000000..b61ce6c920 --- /dev/null +++ b/backend/test/backend_tests/db_test.clj @@ -0,0 +1,43 @@ +;; 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 INC + +(ns backend-tests.db-test + (:require + [app.db :as db] + [backend-tests.helpers :as th] + [clojure.test :as t]) + (:import + com.zaxxer.hikari.HikariConfig + com.zaxxer.hikari.HikariDataSource + java.sql.Connection)) + +(t/use-fixtures :once th/state-init) + +(t/deftest pool-stats-returns-expected-keys + (let [stats (db/pool-stats th/*pool*)] + (t/testing "all expected keys are present" + (t/is (contains? stats :active-connections)) + (t/is (contains? stats :idle-connections)) + (t/is (contains? stats :threads-awaiting-connection)) + (t/is (contains? stats :total-connections)) + (t/is (contains? stats :maximum-pool-size)) + (t/is (contains? stats :minimum-idle))) + + (t/testing "values are non-negative integers" + (t/is (>= (:active-connections stats) 0)) + (t/is (>= (:idle-connections stats) 0)) + (t/is (>= (:threads-awaiting-connection stats) 0)) + (t/is (>= (:total-connections stats) 0)) + (t/is (>= (:maximum-pool-size stats) 0)) + (t/is (>= (:minimum-idle stats) 0))) + + (t/testing "total connections equals active + idle" + (t/is (= (:total-connections stats) + (+ (:active-connections stats) + (:idle-connections stats))))) + + (t/testing "maximum pool size is reasonable" + (t/is (pos? (:maximum-pool-size stats)))))) diff --git a/common/src/app/common/files/comp_processors.cljc b/common/src/app/common/files/comp_processors.cljc index 80e782e7cc..f88b7f5636 100644 --- a/common/src/app/common/files/comp_processors.cljc +++ b/common/src/app/common/files/comp_processors.cljc @@ -38,31 +38,61 @@ (dissoc component :objects)) component))))) +(defn normalize-component-root + "Some old files have shapes with an explicit :component-root false. This is semantically + equivalent to the attribute being absent (instance-root? only treats true as root), but + breaks the subcopy-head? predicate, which expects nil. Remove the explicit false so the + downstream fixers can recognize these shapes as nested copy heads." + [file-data] + (try + (ctf/update-all-shapes + file-data + (fn [shape] + (if (false? (:component-root shape)) + (do + (log/warn :msg "Normalizing :component-root false on shape" + :shape-id (:id shape) + :shape-name (:name shape) + :file-id (:id file-data)) + {:result :update :updated-shape (dissoc shape :component-root)}) + {:result :keep}))) + (catch #?(:clj Throwable :cljs :default) e + (log/error :msg "Failed to normalize :component-root on shapes" + :file-id (:id file-data) + :cause e) + file-data))) + (defn fix-missing-swap-slots "Locate shapes that have been swapped (i.e. their shape-ref does not point to the near match) but they don't have a swap slot. In this case, add one pointing to the near match." [file-data libraries] - (ctf/update-all-shapes - file-data - (fn [shape] - (if (ctk/subcopy-head? shape) - (let [container (:container (meta shape)) - file {:id (:id file-data) :data file-data} - near-match (ctf/find-near-match file container libraries shape :include-deleted? true :with-context? false)] - (if (and (some? near-match) - (not= (:shape-ref shape) (:id near-match)) - (nil? (ctk/get-swap-slot shape))) - (let [updated-shape (ctk/set-swap-slot shape (:id near-match))] - (log/warn :msg "Adding missing swap slot to shape" - :shape-id (:id shape) - :shape-name (:name shape) - :swap-slot (:id near-match) - :file-id (:id file) - :container-id (:id container) - :container-type (:type container)) - {:result :update :updated-shape updated-shape}) - {:result :keep})) - {:result :keep})))) + (try + (ctf/update-all-shapes + file-data + (fn [shape] + (if (ctk/subcopy-head? shape) + (let [container (:container (meta shape)) + file {:id (:id file-data) :data file-data} + near-match (ctf/find-near-match file container libraries shape :include-deleted? true :with-context? false)] + (if (and (some? near-match) + (not= (:shape-ref shape) (:id near-match)) + (nil? (ctk/get-swap-slot shape))) + (let [updated-shape (ctk/set-swap-slot shape (:id near-match))] + (log/warn :msg "Adding missing swap slot to shape" + :shape-id (:id shape) + :shape-name (:name shape) + :swap-slot (:id near-match) + :file-id (:id file) + :container-id (:id container) + :container-type (:type container)) + {:result :update :updated-shape updated-shape}) + {:result :keep})) + {:result :keep}))) + (catch #?(:clj Throwable :cljs :default) e + (log/error :msg "Failed to fix missing swap slots on shapes" + :file-id (:id file-data) + :cause e) + file-data))) (defn sync-component-id-with-ref-shape "Ensure that all copies heads have the same component id and file as the referenced shape. @@ -70,40 +100,46 @@ [file-data libraries] (letfn [(sync-one-iteration [file-data libraries] - (ctf/update-all-shapes - file-data - (fn [shape] - (if (and (ctk/subcopy-head? shape) (nil? (ctk/get-swap-slot shape))) - (let [container (:container (meta shape)) - file {:id (:id file-data) :data file-data} - ref-shape (ctf/find-ref-shape file container libraries shape {:include-deleted? true :with-context? true})] - (if (and (some? ref-shape) - (or (not= (:component-id shape) (:component-id ref-shape)) - (not= (:component-file shape) (:component-file ref-shape)))) - (let [shape' (cond-> shape - (some? (:component-id ref-shape)) - (assoc :component-id (:component-id ref-shape)) + (try + (ctf/update-all-shapes + file-data + (fn [shape] + (if (and (ctk/subcopy-head? shape) (nil? (ctk/get-swap-slot shape))) + (let [container (:container (meta shape)) + file {:id (:id file-data) :data file-data} + ref-shape (ctf/find-ref-shape file container libraries shape {:include-deleted? true :with-context? true})] + (if (and (some? ref-shape) + (or (not= (:component-id shape) (:component-id ref-shape)) + (not= (:component-file shape) (:component-file ref-shape)))) + (let [shape' (cond-> shape + (some? (:component-id ref-shape)) + (assoc :component-id (:component-id ref-shape)) - (nil? (:component-id ref-shape)) - (dissoc :component-id) + (nil? (:component-id ref-shape)) + (dissoc :component-id) - (some? (:component-file ref-shape)) - (assoc :component-file (:component-file ref-shape)) + (some? (:component-file ref-shape)) + (assoc :component-file (:component-file ref-shape)) - (nil? (:component-file ref-shape)) - (dissoc :component-file))] - (log/warn :msg "Syncing component id and file with ref shape" - :shape-id (:id shape) - :shape-name (:name shape) - :component-id (:component-id shape') - :component-file (:component-file shape') - :ref-shape-id (:id ref-shape) - :file-id (:id file) - :container-id (:id container) - :container-type (:type container)) - {:result :update :updated-shape shape'}) - {:result :keep})) - {:result :keep}))))] + (nil? (:component-file ref-shape)) + (dissoc :component-file))] + (log/warn :msg "Syncing component id and file with ref shape" + :shape-id (:id shape) + :shape-name (:name shape) + :component-id (:component-id shape') + :component-file (:component-file shape') + :ref-shape-id (:id ref-shape) + :file-id (:id file) + :container-id (:id container) + :container-type (:type container)) + {:result :update :updated-shape shape'}) + {:result :keep})) + {:result :keep}))) + (catch #?(:clj Throwable :cljs :default) e + (log/error :msg "Failed to sync component id and file with ref shape" + :file-id (:id file-data) + :cause e) + file-data)))] ;; If a copy inside a main is updated, we need to repeat the process for the change to be ;; propagated to all copies. (loop [current-data file-data diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 9ca673ce72..dc40e69456 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -1812,6 +1812,34 @@ (ctob/fix-conflicting-token-names) (ctob/fix-missing-sets-in-themes)))) +(defmethod migrate-data "0021-fix-shape-svg-attrs" + [data _] + (some-> cfeat/*new* (swap! conj "fdata/shape-data-type")) + (letfn [(update-object [object] + (-> object + (d/update-when :svg-attrs csvg/attrs->props) + (d/update-when :svg-viewbox grc/make-rect))) + + (update-container [container] + (d/update-when container :objects d/update-vals update-object))] + + (-> data + (update :pages-index d/update-vals update-container) + (d/update-when :components d/update-vals update-container)))) + +;; Re-run the 0019 and 0020 fixers after normalizing :component-root. +;; Migrations 0019 and 0020 missed shapes with an explicit :component-root +;; false because subcopy-head? expects nil. Normalize first, then re-run. +(defmethod migrate-data "0022-normalize-component-root-and-resync" + [data _] + (let [libraries (if (:libs data) + (deref (:libs data)) + {})] + (-> data + (cfcp/normalize-component-root) + (cfcp/fix-missing-swap-slots libraries) + (cfcp/sync-component-id-with-ref-shape libraries)))) + (def available-migrations (into (d/ordered-set) ["legacy-2" @@ -1890,4 +1918,6 @@ "0018-remove-unneeded-objects-from-components" "0019-fix-missing-swap-slots" "0020-sync-component-id-with-near-main" - "0021-repair-bad-tokens"])) + "0021-repair-bad-tokens" + "0021-fix-shape-svg-attrs" + "0022-normalize-component-root-and-resync"])) diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index 29e6d4fdf5..6a637eada7 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -644,9 +644,10 @@ (fn [shape] ;; Set the desired swap slot (let [slot (:swap-slot args)] - (when (some? slot) - (log/debug :hint (str " -> set swap-slot to " slot)) - (ctk/set-swap-slot shape slot))))] + (if (some? slot) + (do (log/debug :hint (str " -> set swap-slot to " slot)) + (ctk/set-swap-slot shape slot)) + shape)))] (log/dbg :hint "repairing shape :missing-slot" :id (:id shape) :name (:name shape) :page-id page-id) (-> (pcb/empty-changes nil page-id) diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc index 1c16c4dcbc..d1979dd725 100644 --- a/common/src/app/common/files/validate.cljc +++ b/common/src/app/common/files/validate.cljc @@ -17,7 +17,6 @@ [app.common.types.components-list :as ctkl] [app.common.types.container :as ctn] [app.common.types.file :as ctf] - [app.common.types.pages-list :as ctpl] [app.common.types.shape-tree :as ctst] [app.common.types.variant :as ctv] [app.common.uuid :as uuid] @@ -94,6 +93,52 @@ (def ^:dynamic ^:private *errors* nil) +;; Per-page volatile map used to memoize `ctf/find-ref-shape` calls during a +;; single validation pass. Keys are shape-ids; values are the returned ref-shape +;; (or `nil` when the shape has no ref). The cache is reset once per page so +;; that stale results never cross page boundaries. +(def ^:dynamic ^:private *ref-shape-cache* nil) + +;; Per-page pre-computed map from parent-id to the set of its children ids. +;; Enables O(1) containment checks in `check-parent-children` instead of the +;; default O(k) linear `(some #(= shape-id %) (:shapes parent))` scan. +;; Bound as a plain immutable map (not a volatile) since it is read-only during +;; a page's validation pass. +(def ^:dynamic ^:private *children-sets* nil) + +(defn- build-children-sets + "Return a {parent-id → #{child-ids}} map built from `objects` in a single + `reduce-kv` pass. Only shapes that have at least one child get an entry." + [objects] + (reduce-kv (fn [m _ shape] + (if-let [kids (not-empty (:shapes shape))] + (assoc m (:id shape) (set kids)) + m)) + {} + objects)) + +(defn- find-ref-shape* + "Cached wrapper around `ctf/find-ref-shape` with `:include-deleted? true`. + When `*ref-shape-cache*` is bound, each shape-id is resolved at most once + per validation pass regardless of how many check functions request it. + + Cache miss detection uses `contains?` rather than a sentinel default + (e.g. `(get cache id ::miss)` + `identical?`). The reason: in + ClojureScript, `identical?` on namespace-qualified keywords is not + reliable across compilation units because keyword interning is not + guaranteed — a sentinel retrieved from `get` may not be `===` to the + same literal written in code, causing every lookup to appear as a miss + and breaking the whole optimisation." + [file page libraries shape] + (if-let [cache *ref-shape-cache*] + (let [id (:id shape)] + (if (contains? @cache id) + (get @cache id) + (let [result (ctf/find-ref-shape file page libraries shape :include-deleted? true)] + (vswap! cache assoc id result) + result))) + (ctf/find-ref-shape file page libraries shape :include-deleted? true))) + (defn- library-exists? [file libraries shape] (or (= (:component-file shape) (:id file)) @@ -149,15 +194,32 @@ shape file page) (do (when-not (cfh/root? shape) - (when-not (some #(= shape-id %) (:shapes parent)) + ;; Fast path: O(1) set lookup when `*children-sets*` is pre-computed + ;; for the current page. Falls back to an O(k) linear scan via + ;; `some` when the var is unbound (e.g. in isolated single-shape + ;; validation calls outside a full page pass). + (when-not (if-let [cs *children-sets*] + (contains? (get cs (:id parent)) shape-id) + (some #(= shape-id %) (:shapes parent))) (report-error :child-not-in-parent (str/ffmt "Shape % not in parent's children list" shape-id) shape file page))) - (when-not (= (count shapes) (count (distinct shapes))) - (report-error :duplicated-children - (str/ffmt "Shape % has duplicated children" shape-id) - shape file page)) + ;; Single-pass duplicate detection: walk the children list once, + ;; accumulating seen IDs into a set and short-circuiting on the + ;; first duplicate. This replaces the previous two-pass + ;; `(not= (count shapes) (count (distinct shapes)))` approach + ;; which allocated an intermediate sequence and scanned twice. + (let [dup? (reduce (fn [seen id] + (if (contains? seen id) + (reduced true) + (conj seen id))) + #{} + shapes)] + (when (true? dup?) + (report-error :duplicated-children + (str/ffmt "Shape % has duplicated children" shape-id) + shape file page))) (doseq [child-id shapes] (let [child (ctst/get-shape page child-id)] @@ -292,7 +354,7 @@ [shape file page libraries] (let [library-exists (library-exists? file libraries shape) ref-shape (when library-exists - (ctf/find-ref-shape file page libraries shape :include-deleted? true))] + (find-ref-shape* file page libraries shape))] (when (and library-exists (nil? ref-shape)) (report-error :ref-shape-not-found (str/ffmt "Referenced shape % not found in near component" (:shape-ref shape)) @@ -309,7 +371,7 @@ (defn- check-ref-is-not-head "Validate that the referenced shape is not a nested copy root." [shape file page libraries] - (let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)] + (let [ref-shape (find-ref-shape* file page libraries shape)] (when (and (some? ref-shape) (ctk/instance-head? ref-shape)) (report-error :ref-shape-is-head @@ -319,7 +381,7 @@ (defn- check-ref-is-head "Validate that the referenced shape is a nested copy root." [shape file page libraries] - (let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)] + (let [ref-shape (find-ref-shape* file page libraries shape)] (when (and (some? ref-shape) (not (ctk/instance-head? ref-shape))) (report-error :ref-shape-is-not-head @@ -333,7 +395,7 @@ the same as in the referenced shape in the near main." [shape file page libraries] (when (nil? (ctk/get-swap-slot shape)) - (when-let [ref-shape (ctf/find-ref-shape file page libraries shape :include-deleted? true)] + (when-let [ref-shape (find-ref-shape* file page libraries shape)] (when (or (not= (:component-id shape) (:component-id ref-shape)) (not= (:component-file shape) (:component-file ref-shape))) (report-error :component-id-mismatch @@ -351,12 +413,21 @@ shape file page))) (defn- has-duplicate-swap-slot? + "Returns true if any two children of `shape` share the same swap slot. + Uses a single pass with early exit on the first duplicate, avoiding + the intermediate `frequencies` map allocation." [shape container] - (let [shapes (map #(get (:objects container) %) (:shapes shape)) - slots (->> (map #(ctk/get-swap-slot %) shapes) - (remove nil?)) - counts (frequencies slots)] - (some (fn [[_ count]] (> count 1)) counts))) + (let [objects (:objects container) + result (reduce (fn [seen child-id] + (let [slot (ctk/get-swap-slot (get objects child-id))] + (if (nil? slot) + seen + (if (contains? seen slot) + (reduced true) + (conj seen slot))))) + #{} + (:shapes shape))] + (true? result))) (defn- check-duplicate-swap-slot "Validate that the children of this shape does not have duplicated slots." @@ -370,14 +441,16 @@ "Validate that the shape has swap-slot if it's a subinstance head and the ref shape is not the matching shape by position in the near main." [shape file page libraries] - (let [near-match (ctf/find-near-match file page libraries shape :include-deleted? true :with-context? false)] - (when (and (some? near-match) - (not= (:shape-ref shape) (:id near-match)) - (nil? (ctk/get-swap-slot shape))) - (report-error :missing-slot - "Shape has been swapped, should have swap slot" - shape file page - :swap-slot (or (ctk/get-swap-slot near-match) (:id near-match)))))) + ;; Guard first: if the shape already has a swap slot the invariant is satisfied + ;; and we can avoid the expensive `find-near-match` call entirely. + (when (nil? (ctk/get-swap-slot shape)) + (let [near-match (ctf/find-near-match file page libraries shape :include-deleted? true :with-context? false)] + (when (and (some? near-match) + (not= (:shape-ref shape) (:id near-match))) + (report-error :missing-slot + "Shape has been swapped, should have swap slot" + shape file page + :swap-slot (or (ctk/get-swap-slot near-match) (:id near-match))))))) (defn- check-valid-touched "Validate that the text touched flags are coherent." @@ -491,26 +564,28 @@ -all its children should be variants with variant-id equals to the shape-id -all the components should have the same properties" [shape file page] - (let [shape-id (:id shape) - shapes (:shapes shape) - children (map #(ctst/get-shape page %) shapes) - prop-names (cfv/extract-properties-names (first children) (:data file))] - (doseq [child children] - (when child - (if (not (ctk/is-variant? child)) - (report-error :not-a-variant - (str/ffmt "Shape % should be a variant" (:id child)) - child file page) - (do - (when (not= (:variant-id child) shape-id) - (report-error :invalid-variant-id - (str/ffmt "Variant % has invalid variant-id %" (:id child) (:variant-id child)) - child file page)) - (when (not= prop-names (cfv/extract-properties-names child (:data file))) - (report-error :invalid-variant-properties - (str/ffmt "Variant % has invalid properties %" (:id child) (vec prop-names)) - child file page)))))))) - + (let [shape-id (:id shape) + shapes (:shapes shape) + objects (:objects page) + file-data (:data file) + first-child (get objects (first shapes)) + prop-names (cfv/extract-properties-names first-child file-data)] + (run! (fn [child-id] + (when-let [child (get objects child-id)] + (if (not (ctk/is-variant? child)) + (report-error :not-a-variant + (str/ffmt "Shape % should be a variant" (:id child)) + child file page) + (do + (when (not= (:variant-id child) shape-id) + (report-error :invalid-variant-id + (str/ffmt "Variant % has invalid variant-id %" (:id child) (:variant-id child)) + child file page)) + (when (not= prop-names (cfv/extract-properties-names child file-data)) + (report-error :invalid-variant-properties + (str/ffmt "Variant % has invalid properties %" (:id child) (vec prop-names)) + child file page)))))) + shapes))) (defn- check-variant "Shape is a variant, so -it should be a main component @@ -615,18 +690,26 @@ shape file page) (check-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) - (check-shape-main-not-root shape file page libraries)) + ;; Short-circuit `inside-component-main?` when the propagated + ;; `context` already classifies this sub-tree as belonging to a + ;; main component. `inside-component-main?` performs an O(depth) + ;; upward ancestor walk; skipping it for every non-head shape that + ;; sits inside a known-main context avoids redundant tree traversals + ;; that would otherwise dominate validation time on deep hierarchies. + (let [in-main? (or (#{:main-top :main-nested :main-any} context) + (ctn/inside-component-main? (:objects page) shape))] + (if in-main? + (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) + (check-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) - (check-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) + (check-shape-not-component shape file page libraries))))))))) (defn check-component-duplicate-swap-slot [component file] @@ -638,11 +721,13 @@ (defn check-ref-cycles [component file] - (let [cycles-ids (->> component - :objects - vals - (filter #(= (:id %) (:shape-ref %))) - (map :id))] + (let [cycles-ids (-> (reduce-kv (fn [acc id shape] + (if (= id (:shape-ref shape)) + (conj! acc id) + acc)) + (transient []) + (:objects component)) + (persistent!))] (when (seq cycles-ids) (report-error :shape-ref-cycle @@ -703,10 +788,25 @@ (check-variant-component component file))) (defn- get-orphan-shapes - [{:keys [objects] :as page}] - (let [xf (comp (map #(contains? objects (:parent-id %))) - (map :id))] - (into [] xf (vals objects)))) + "Return the ids of shapes whose parent does not exist in the objects + map (i.e. shapes unreachable from the root traversal). The root + shape itself is excluded since it is always validated separately. + + Implemented with `reduce-kv` rather than a `map`/`filter` pipeline. + The previous implementation mapped over `objects` and returned a + lazy seq that could contain `nil` entries (for shapes that were + *not* orphans), meaning `check-shape` was called with `nil` IDs and + orphaned shapes were silently skipped. The `reduce-kv` approach + builds a plain vector of IDs and never yields `nil` entries." + [{:keys [objects] :as _page}] + (persistent! + (reduce-kv (fn [result id shape] + (if (and (not (cfh/root? shape)) + (not (contains? objects (:parent-id shape)))) + (conj! result id) + result)) + (transient []) + objects))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; PUBLIC API: VALIDATION FUNCTIONS @@ -720,20 +820,39 @@ (when (contains? features "components/v2") (binding [*errors* (volatile! [])] - (doseq [page (filter :id (ctpl/pages-seq data))] - (check-shape uuid/zero file page libraries) - (->> (get-orphan-shapes page) - (run! #(check-shape % file page libraries)))) + ;; `reduce-kv` is used throughout this function (and `validate-file-affected`) + ;; instead of `(run! f (vals m))` or `(doseq [[k v] m])` because persistent + ;; maps implement `IKVReduce`, which drives iteration internally without + ;; first materialising an intermediate sequence of key-value pairs. + (reduce-kv + (fn [_ _ page] + (when (some? page) + ;; Both performance caches are scoped to a single page: ref-shape + ;; lookups and parent→children sets are page-local and must never + ;; carry over to the next page. A fresh volatile is created for + ;; each page so stale entries cannot corrupt subsequent pages. + (binding [*ref-shape-cache* (volatile! {}) + *children-sets* (build-children-sets (:objects page))] + (check-shape uuid/zero file page libraries) + (run! #(check-shape % file page libraries) + (get-orphan-shapes page))))) + nil + (:pages-index data)) - (->> (vals (:components data)) - (run! #(check-component % file))) + (reduce-kv + (fn [_ _ component] + (check-component component file)) + nil + (:components data)) (-> *errors* deref not-empty)))) (defn validate-shape "Validate a shape and all its children. Returns a list of errors." [shape-id file page libraries] - (binding [*errors* (volatile! [])] + (binding [*errors* (volatile! []) + *ref-shape-cache* (volatile! {}) + *children-sets* (build-children-sets (:objects page))] (check-shape shape-id file page libraries) (deref *errors*))) @@ -744,6 +863,107 @@ (check-component component file) (deref *errors*))) +(defn- extract-affected-ids + "Single reduce pass over a changes batch. + Returns {:page-ids #{uuid …} :component-ids #{uuid …}}. + + Only entities that need re-validation after applying `changes` are + included. Deleted entities and pure file-level changes (colors, + tokens, typography…) produce no entries because there is nothing left + to check." + [changes] + + (loop [changes (seq changes) + page-ids #{} + component-ids #{}] + (if-let [change (first changes)] + (let [{:keys [type page-id component-id id]} change + page-ids + (case type + ;; Shape-level ops are scoped to either a page or a component + (:add-obj :mod-obj :del-obj :fix-obj :mov-objects :reorder-children :reg-objects) + (cond-> page-ids + page-id (conj page-id)) + + ;; A new or modified page needs a full page sweep + (:add-page :mod-page) + (conj page-ids id) + + ;; restore-component resurrects a deleted component (touches the + ;; component definition) and places its main instance on a page + ;; (touches that page's shape tree) + :restore-component + (conj page-ids page-id) + + ;; Otherwise don't change the ids + page-ids) + + component-ids + (case type + ;; Shape-level ops are scoped to either a page or a component + (:add-obj :mod-obj :del-obj :fix-obj :mov-objects :reorder-children :reg-objects) + (cond-> component-ids + component-id (conj component-id)) + + ;; A new, modified, restored component needs component-level checking + (:add-component :mod-component :restore-component) + (conj component-ids id) + + ;; Otherwise don't change the ids + component-ids)] + (recur (rest changes) page-ids component-ids)) + + ;; Return result of accumulated ids + {:page-ids page-ids :component-ids component-ids}))) + +(defn validate-file-affected + "Validate only the pages and components touched by `changes`. + + Semantics are identical to `validate-file` but the work is bounded + to the entities that were actually mutated, making it safe and cheap + to call on every incremental file update. + + Returns a list of errors or `nil`." + [{:keys [data features] :as file} libraries changes] + (when (contains? features "components/v2") + (let [{:keys [page-ids component-ids]} (extract-affected-ids changes)] + (binding [*errors* (volatile! [])] + + (reduce-kv + (fn [_ page-id page] + (when (and (some? page) (contains? page-ids page-id)) + ;; Both performance caches are scoped to a single page: ref-shape + ;; lookups and parent→children sets are page-local and must never + ;; carry over to the next page. A fresh volatile is created for + ;; each page so stale entries cannot corrupt subsequent pages. + (binding [*ref-shape-cache* (volatile! {}) + *children-sets* (build-children-sets (:objects page))] + (check-shape uuid/zero file page libraries) + (run! #(check-shape % file page libraries) + (get-orphan-shapes page))))) + nil + (:pages-index data)) + + (reduce-kv + (fn [_ id component] + (when (contains? component-ids id) + (check-component component file))) + nil + (:components data)) + + (-> *errors* deref not-empty))))) + +(defn validate-file-affected! + "Like `validate-file-affected` but raises on the first non-empty + error list instead of returning it." + [file libraries changes] + (when-let [errors (validate-file-affected file libraries changes)] + (ex/raise :type :validation + :code :referential-integrity + :hint "error on validating file referential integrity" + :file-id (:id file) + :details errors))) + (defn validate-file-schema! "Validates the file itself, without external dependencies, it performs the schema checking and some semantical validation of the diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index 62b422b0b4..97988028d6 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -2098,6 +2098,8 @@ For :selrect we compare width/height only; for :points we normalise each vector so the first point is the origin before comparing. + For :content on path shapes we compare the bounding-box width/height + of the path segments, again ignoring absolute position. Comparisons use `mth/close?` (and `gpt/close?` for points) rather than exact `=` because `previous-shape` here may carry sub-pixel drift from @@ -2118,8 +2120,17 @@ a (normalize-pts (get shape :points)) b (normalize-pts (get origin-shape :points))] (and (= (count a) (count b)) - (every? identity (map gpt/close? a b))))))) - + (every? identity (map gpt/close? a b))))) + (and (= attr :content) + (cfh/path-shape? shape) + (let [ca (:content shape) + cb (:content origin-shape)] + (and (some? ca) (some? cb) + (let [selrect-a (segment/content->selrect ca) + selrect-b (segment/content->selrect cb)] + (and (some? selrect-a) (some? selrect-b) + (mth/close? (:width selrect-a) (:width selrect-b)) + (mth/close? (:height selrect-a) (:height selrect-b))))))))) (defn update-attrs-on-switch "Copy attributes that have changed in the shape previous to the switch diff --git a/common/src/app/common/types/component.cljc b/common/src/app/common/types/component.cljc index ecc6e30c65..1b5648804a 100644 --- a/common/src/app/common/types/component.cljc +++ b/common/src/app/common/types/component.cljc @@ -269,10 +269,19 @@ [group] (str/starts-with? (name group) "swap-slot-")) +(def ^:private xf:normal-touched + "Transducer that removes swap-slot touched groups." + (remove swap-slot?)) + (defn normal-touched-groups - "Gets all touched groups that are not swap slots." + "Gets all touched groups that are not swap slots. + Returns an empty set immediately when `:touched` is nil or empty, + avoiding an unnecessary `into #{}` allocation for the common case." [shape] - (into #{} (remove swap-slot? (:touched shape)))) + (let [touched (:touched shape)] + (if (empty? touched) + #{} + (into #{} xf:normal-touched touched)))) (defn group->swap-slot [group] diff --git a/common/test/common_tests/data_test.cljc b/common/test/common_tests/data_test.cljc index 56b43078dc..70f13e5119 100644 --- a/common/test/common_tests/data_test.cljc +++ b/common/test/common_tests/data_test.cljc @@ -888,7 +888,7 @@ (t/is (not (d/editable-collection? "hello"))) (t/is (not (d/editable-collection? 42)))) -(t/deftest num-predicate +(t/deftest num-predicate-2 (t/is (d/num? 1)) (t/is (d/num? 0)) (t/is (d/num? -3.14)) diff --git a/common/test/common_tests/files/comp_processors_test.cljc b/common/test/common_tests/files/comp_processors_test.cljc index c1cabbbb72..412986ede4 100644 --- a/common/test/common_tests/files/comp_processors_test.cljc +++ b/common/test/common_tests/files/comp_processors_test.cljc @@ -114,6 +114,84 @@ (t/is (= expected-diff diff))))) +(t/deftest test-normalize-component-root + + (t/testing "nil file should return nil" + (let [file nil + file' (ctf/update-file-data file cfcp/normalize-component-root)] + (t/is (nil? file')))) + + (t/testing "empty file should not need any action" + (let [file (thf/sample-file :file1) + file' (ctf/update-file-data file cfcp/normalize-component-root)] + (t/is (empty? (d/map-diff file file'))))) + + (t/testing "shape with :component-root true should not be modified" + (let [file + (-> (thf/sample-file :file1) + (tho/add-simple-component :component1 :main1-root :main1-child) + (thc/instantiate-component :component1 :copy1-root)) + + file' (ctf/update-file-data file cfcp/normalize-component-root)] + + (t/is (empty? (d/map-diff file file'))))) + + (t/testing "shape with :component-root false should have it removed" + (let [file + (-> (thf/sample-file :file1) + (tho/add-nested-component-with-copy :component1 :main1-root :main1-child + :component2 :main2-root :nested-head + :copy2-root) + (ths/update-shape :nested-head :component-root false)) + + file' (ctf/update-file-data file cfcp/normalize-component-root) + + shape' (ths/get-shape file' :nested-head)] + + (t/is (not (contains? shape' :component-root)))))) + +(t/deftest test-migration-0022-fixes-component-root-false-mismatch + + (t/testing "a nested copy with :component-root false and a stale :component-id is fixed by the full migration sequence" + (let [file + ;; Reproduce the legacy corruption: a nested copy head has + ;; :component-root false (instead of absent) AND a stale + ;; component-id. Migrations 0019/0020 alone don't catch it + ;; because subcopy-head? expects nil. After normalize-component-root, + ;; sync-component-id-with-ref-shape can repair it. + (-> (thf/sample-file :file1) + (tho/add-nested-component :component1 :main1-root :main1-child + :component2 :main2-root :nested-head) + (thc/instantiate-component :component2 :copy2-root :children-labels [:copy2-nested-head]) + (ths/update-shape :copy2-nested-head :component-root false) + (ths/update-shape :copy2-nested-head :component-id (thi/new-id! :wrong-id))) + + ;; Run the old sequence (0019/0020 without normalization): the bad shape is skipped. + old-sequence + (-> file + (ctf/update-file-data #(cfcp/fix-missing-swap-slots % {})) + (ctf/update-file-data #(cfcp/sync-component-id-with-ref-shape % {}))) + + old-shape (ths/get-shape old-sequence :copy2-nested-head) + + ;; Run the new sequence (0022): normalize first, then 0019/0020 logic. + new-sequence + (-> file + (ctf/update-file-data cfcp/normalize-component-root) + (ctf/update-file-data #(cfcp/fix-missing-swap-slots % {})) + (ctf/update-file-data #(cfcp/sync-component-id-with-ref-shape % {}))) + + new-shape (ths/get-shape new-sequence :copy2-nested-head)] + + ;; Confirm the old sequence fails to repair (this guards against a future + ;; subcopy-head? change accidentally fixing it and masking the regression). + (t/is (= false (:component-root old-shape))) + (t/is (= (thi/id :wrong-id) (:component-id old-shape))) + + ;; Confirm the new sequence repairs both the explicit false and the bad ref. + (t/is (not (contains? new-shape :component-root))) + (t/is (= (thi/id :component1) (:component-id new-shape)))))) + (t/deftest test-fix-missing-swap-slots (t/testing "nil file should return nil" diff --git a/common/test/common_tests/files/validate_test.cljc b/common/test/common_tests/files/validate_test.cljc new file mode 100644 index 0000000000..271cfa611e --- /dev/null +++ b/common/test/common_tests/files/validate_test.cljc @@ -0,0 +1,449 @@ +;; 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 INC + +(ns common-tests.files.validate-test + "Exhaustive tests for the change-scoped partial validation functions in + app.common.files.validate: + + - validate-file-affected – returns nil or list of errors + - validate-file-affected! – same but raises on non-empty errors + + The tests verify the scoping logic implemented by extract-affected-ids + (tested indirectly) by injecting controlled broken states into specific + pages / components and confirming that only the expected entities are + validated." + (:require + [app.common.files.validate :as cfv] + [app.common.test-helpers.files :as thf] + [app.common.test-helpers.ids-map :as thi] + [app.common.types.file :as ctf] + [app.common.types.pages-list :as ctpl] + [app.common.types.shape-tree :as ctst] + [app.common.uuid :as uuid] + [clojure.test :as t])) + +(t/use-fixtures :each thi/test-fixture) + +;; ---------------------------------------------------------------- +;; Test-local helpers +;; ---------------------------------------------------------------- + +(defn- inject-broken-child + "Add a reference to a non-existent shape ID to the root shape's + children list on the page identified by `page-label`. + + This causes `check-parent-children` to report a :child-not-found + error whenever that page is validated." + [file page-label] + (let [page-id (thi/id page-label) + missing-id (uuid/next)] + (ctf/update-file-data + file + (fn [file-data] + (ctpl/update-page + file-data + page-id + (fn [page] + (let [root (ctst/get-shape page uuid/zero)] + (ctst/set-shape page (update root :shapes conj missing-id))))))))) + +(defn- inject-broken-component + "Add a deleted component with `:objects nil` to the file. + + This causes `check-component` to report a + :component-nil-objects-not-allowed error whenever that component is + validated. The component id is registered under `comp-label` in the + ids-map so callers can look it up with `(thi/id comp-label)`." + [file comp-label] + (let [comp-id (thi/new-id! comp-label)] + (ctf/update-file-data + file + (fn [file-data] + (assoc-in file-data [:components comp-id] + {:id comp-id + :name "broken-component" + :objects nil + :deleted true + :main-instance-id (uuid/next) + :main-instance-page (uuid/next)}))))) + +;; ---------------------------------------------------------------- +;; 1. Feature gate +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-no-feature + (t/testing "returns nil when file does not have the components/v2 feature" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (assoc :features #{})) + page-id (thi/id :page1) + changes [{:type :add-obj :page-id page-id :id (uuid/next)}]] + (t/is (nil? (cfv/validate-file-affected file {} changes)))))) + +;; ---------------------------------------------------------------- +;; 2. Empty changes – nothing to validate +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-empty-changes + (t/testing "returns nil when the changes list is empty" + (let [file (thf/sample-file :file1 :page-label :page1)] + (t/is (nil? (cfv/validate-file-affected file {} [])))))) + +;; ---------------------------------------------------------------- +;; 3. Page-level scoping – add-obj / mod-obj / fix-obj key: :page-id +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-page-scoping-misses-untouched-page + (t/testing "add-obj on page1 does not validate broken page2" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page2) + page1-id (thi/id :page1) + ;; The change only touches page1 + changes [{:type :add-obj :page-id page1-id :id (uuid/next)}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes))))) + + (t/testing "mod-obj on page1 does not validate broken page2" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page2) + page1-id (thi/id :page1) + changes [{:type :mod-obj :page-id page1-id :id uuid/zero + :operations [{:type :set :attr :name :val "root"}]}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes))))) + + (t/testing "fix-obj on page1 does not validate broken page2" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page2) + page1-id (thi/id :page1) + changes [{:type :fix-obj :page-id page1-id :id (uuid/next) + :operations []}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes)))))) + +(t/deftest validate-file-affected-page-scoping-catches-error-on-touched-page + (t/testing "add-obj on broken page1 surfaces the error" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :add-obj :page-id page1-id :id (uuid/next)}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors))))) + + (t/testing "mod-obj on broken page1 surfaces the error" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :mod-obj :page-id page1-id :id uuid/zero + :operations [{:type :set :attr :name :val "root"}]}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors))))) + + (t/testing "reg-objects on broken page1 surfaces the error" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :reg-objects :page-id page1-id :shapes []}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors))))) + + (t/testing "mov-objects on broken page1 surfaces the error" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :mov-objects :page-id page1-id :parent-id uuid/zero :shapes []}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors)))))) + +;; ---------------------------------------------------------------- +;; 4. add-page / mod-page – scoped by :id (not :page-id) +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-add-page-scoped-by-id + (t/testing "add-page with :id=page2 validates page2 (broken → errors)" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page2) + page2-id (thi/id :page2) + changes [{:type :add-page :id page2-id}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors))))) + + (t/testing "add-page with :id=page1 does not validate broken page2" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page2) + page1-id (thi/id :page1) + changes [{:type :add-page :id page1-id}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes))))) + + (t/testing "mod-page with :id=page2 validates page2 (broken → errors)" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page2) + page2-id (thi/id :page2) + changes [{:type :mod-page :id page2-id}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors)))))) + +;; ---------------------------------------------------------------- +;; 5. del-obj – shape-level op scoped by :page-id +;; del-page / del-component / mov-page / purge-component – no-ops +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-del-obj-scopes-page + (t/testing "del-obj scopes its :page-id just like add-obj / mod-obj" + ;; del-obj is in the same shape-level ops bucket and scopes by :page-id. + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :del-obj :page-id page1-id :id (uuid/next)}]] + ;; page1 has an error and del-obj touches page1 → error surfaced + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors))))) + + (t/testing "del-obj on page1 does not validate broken page2" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page2) + page1-id (thi/id :page1) + changes [{:type :del-obj :page-id page1-id :id (uuid/next)}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes)))))) + +(t/deftest validate-file-affected-noop-change-types + (t/testing "del-page produces no affected entries" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :del-page :id page1-id}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes))))) + + (t/testing "del-component produces no affected entries" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (inject-broken-component :comp1)) + comp-id (thi/id :comp1) + changes [{:type :del-component :id comp-id}]] + (t/is (nil? (cfv/validate-file-affected file {} changes))))) + + (t/testing "mov-page produces no affected entries" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :mov-page :id page1-id}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes))))) + + (t/testing "purge-component produces no affected entries" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (inject-broken-component :comp1)) + comp-id (thi/id :comp1) + changes [{:type :purge-component :id comp-id}]] + (t/is (nil? (cfv/validate-file-affected file {} changes))))) + + (t/testing "add-color (library change) produces no affected entries" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + changes [{:type :add-color :id (uuid/next) :color {}}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes))))) + + (t/testing "mod-color (library change) produces no affected entries" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + changes [{:type :mod-color :id (uuid/next) :color {}}]] + (t/is (nil? (cfv/validate-file-affected file' {} changes)))))) + +;; ---------------------------------------------------------------- +;; 6. add-component / mod-component – scoped by :id (component-id) +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-add-component-scoped-by-id + (t/testing "add-component :id=comp1 validates comp1 (broken → errors)" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (inject-broken-component :comp1)) + comp-id (thi/id :comp1) + changes [{:type :add-component :id comp-id}]] + (let [errors (cfv/validate-file-affected file {} changes)] + (t/is (seq errors)) + (t/is (some #(= :component-nil-objects-not-allowed (:code %)) errors))))) + + (t/testing "add-component :id=other-id does not validate broken comp1" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (inject-broken-component :comp1)) + _ (thi/id :comp1) + changes [{:type :add-component :id (uuid/next)}]] + (t/is (nil? (cfv/validate-file-affected file {} changes))))) + + (t/testing "mod-component :id=comp1 validates comp1 (broken → errors)" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (inject-broken-component :comp1)) + comp-id (thi/id :comp1) + changes [{:type :mod-component :id comp-id}]] + (let [errors (cfv/validate-file-affected file {} changes)] + (t/is (seq errors)) + (t/is (some #(= :component-nil-objects-not-allowed (:code %)) errors)))))) + +;; ---------------------------------------------------------------- +;; 7. restore-component – scopes BOTH :id (component) and :page-id +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-restore-component-scopes-page + (t/testing "restore-component touches its :page-id (broken page → errors)" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :restore-component :id (uuid/next) :page-id page1-id}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors))))) + + (t/testing "restore-component does not validate a page it does not reference" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + file' (inject-broken-child file :page2) + page1-id (thi/id :page1) + changes [{:type :restore-component :id (uuid/next) :page-id page1-id}]] + ;; page2 has an error but the change only touches page1 + (t/is (nil? (cfv/validate-file-affected file' {} changes)))))) + +(t/deftest validate-file-affected-restore-component-scopes-component + (t/testing "restore-component touches its component :id (broken component → errors)" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (inject-broken-component :comp1)) + comp-id (thi/id :comp1) + page1-id (thi/id :page1) + changes [{:type :restore-component :id comp-id :page-id page1-id}]] + (let [errors (cfv/validate-file-affected file {} changes)] + (t/is (seq errors)) + (t/is (some #(= :component-nil-objects-not-allowed (:code %)) errors)))))) + +;; ---------------------------------------------------------------- +;; 8. Mixed changes – union of affected entities +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-mixed-changes-union + (t/testing "two changes on different pages: both pages are validated" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + ;; page2 is broken; page1 is clean + file' (inject-broken-child file :page2) + page1-id (thi/id :page1) + page2-id (thi/id :page2) + ;; Both pages are touched + changes [{:type :add-obj :page-id page1-id :id (uuid/next)} + {:type :mod-obj :page-id page2-id :id uuid/zero + :operations []}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + ;; page2 is validated → error surfaced + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors))))) + + (t/testing "del-page (true no-op) mixed with add-obj on page1: only page1 is validated" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (thf/add-sample-page :page2)) + ;; page2 is broken; page1 is clean + file' (inject-broken-child file :page2) + page1-id (thi/id :page1) + page2-id (thi/id :page2) + ;; del-page on page2 is a no-op; add-obj on page1 scopes page1 only + changes [{:type :del-page :id page2-id} + {:type :add-obj :page-id page1-id :id (uuid/next)}]] + ;; page2's error is NOT surfaced because del-page produces no scope + (t/is (nil? (cfv/validate-file-affected file' {} changes))))) + + (t/testing "duplicate page-ids in changes are deduplicated (page validated once)" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + ;; Three changes all touching the same page + changes [{:type :add-obj :page-id page1-id :id (uuid/next)} + {:type :mod-obj :page-id page1-id :id uuid/zero :operations []} + {:type :del-obj :page-id page1-id :id (uuid/next)}]] + ;; del-obj excluded, add-obj + mod-obj both scope page1; error surfaced once + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + ;; There should be exactly one :child-not-found (not duplicated) + (t/is (= 1 (count (filter #(= :child-not-found (:code %)) errors)))))))) + +;; ---------------------------------------------------------------- +;; 9. reorder-children and component-context changes +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected-reorder-children + (t/testing "reorder-children on broken page surfaces the error" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :reorder-children :page-id page1-id :id uuid/zero :shapes []}]] + (let [errors (cfv/validate-file-affected file' {} changes)] + (t/is (seq errors)) + (t/is (some #(= :child-not-found (:code %)) errors)))))) + +(t/deftest validate-file-affected-obj-in-component + (t/testing "add-obj with :component-id (not :page-id) scopes a component" + ;; When a shape change carries :component-id instead of :page-id it + ;; means the change happened inside a deleted component's object tree. + ;; extract-affected-ids routes it to :component-ids. + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (inject-broken-component :comp1)) + comp-id (thi/id :comp1) + changes [{:type :add-obj :component-id comp-id :id (uuid/next)}]] + (let [errors (cfv/validate-file-affected file {} changes)] + (t/is (seq errors)) + (t/is (some #(= :component-nil-objects-not-allowed (:code %)) errors))))) + + (t/testing "add-obj with :component-id does not scope an unrelated component" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (inject-broken-component :comp1)) + _ (thi/id :comp1) + changes [{:type :add-obj :component-id (uuid/next) :id (uuid/next)}]] + (t/is (nil? (cfv/validate-file-affected file {} changes)))))) + +;; ---------------------------------------------------------------- +;; 10. validate-file-affected! – raises vs returns nil +;; ---------------------------------------------------------------- + +(t/deftest validate-file-affected!-returns-nil-on-clean-file + (t/testing "returns nil when the file has no errors in the touched page" + (let [file (thf/sample-file :file1 :page-label :page1) + page1-id (thi/id :page1) + changes [{:type :add-obj :page-id page1-id :id (uuid/next)}]] + (t/is (nil? (cfv/validate-file-affected! file {} changes)))))) + +(t/deftest validate-file-affected!-returns-nil-empty-changes + (t/testing "returns nil when there are no changes (no pages validated)" + (let [file (thf/sample-file :file1 :page-label :page1)] + (t/is (nil? (cfv/validate-file-affected! file {} [])))))) + +(t/deftest validate-file-affected!-raises-on-error + (t/testing "raises an exception when the touched page has validation errors" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :mod-obj :page-id page1-id :id uuid/zero + :operations []}]] + (t/is (thrown? #?(:clj Exception :cljs js/Error) + (cfv/validate-file-affected! file' {} changes))))) + + (t/testing "raised exception is of :validation type with :referential-integrity code" + (let [file (thf/sample-file :file1 :page-label :page1) + file' (inject-broken-child file :page1) + page1-id (thi/id :page1) + changes [{:type :mod-obj :page-id page1-id :id uuid/zero + :operations []}]] + (try + (cfv/validate-file-affected! file' {} changes) + (t/is false "expected exception to be thrown") + (catch #?(:clj clojure.lang.ExceptionInfo :cljs cljs.core.ExceptionInfo) e + (let [data (ex-data e)] + (t/is (= :validation (:type data))) + (t/is (= :referential-integrity (:code data))) + (t/is (seq (:details data))))))))) diff --git a/common/test/common_tests/logic/variants_switch_test.cljc b/common/test/common_tests/logic/variants_switch_test.cljc index abb53fef70..01bebe2f91 100644 --- a/common/test/common_tests/logic/variants_switch_test.cljc +++ b/common/test/common_tests/logic/variants_switch_test.cljc @@ -2970,3 +2970,68 @@ ;; its :selrect.y is internally inconsistent and renders incorrectly. (t/is (= post-btn-rel-y post-btn-selrect-rel-y) ":y and :selrect.y must agree after switch"))) + +(t/deftest test-switch-does-not-override-path-content-when-only-repositioned + ;; Regression: when a path shape inside a variant has :geometry-group touched + ;; (e.g. because auto-layout repositioned it after the copy's parent was + ;; resized), switching variants must NOT copy the old variant's path position + ;; to the new variant. The path should stay at the new variant's default position. + ;; + ;; Root cause: equal-geometry? did not handle the :content attr for path shapes, + ;; so switch-path-change-value was always invoked and placed the new path at the + ;; pre-switch absolute position instead of the target master's default position. + (let [;; A small closed triangle path whose bounding box is 24x14 px, + ;; anchored at absolute position (x0, y0). + triangle (fn [x0 y0] + [{:command :move-to :params {:x x0 :y y0}} + {:command :line-to :params {:x (+ x0 24) :y y0}} + {:command :line-to :params {:x (+ x0 12) :y (+ y0 14)}} + {:command :close-path}]) + + ;; V1 has the path at y=10; V2 has the same-shape path at y=30. + file (-> (thf/sample-file :file1) + (thv/add-variant :v01 :c01 :m01 :c02 :m02 + {:variant1-params {:width 100 :height 100} + :variant2-params {:width 100 :height 100}}) + (ths/add-sample-shape :path1 :type :path + :parent-label :m01 + :content (triangle 0 10)) + (ths/add-sample-shape :path2 :type :path + :parent-label :m02 + :content (triangle 0 30)) + (thc/instantiate-component :c01 :copy01)) + + ;; Simulate auto-layout repositioning the path inside the copy by + ;; moving it to y=50. This touches :geometry-group on the copy's path. + page (thf/current-page file) + copy01 (ths/get-shape file :copy01) + copy-path (->> (cfh/get-children-with-self (:objects page) (:id copy01)) + (filter #(= :path (:type %))) + first) + changes (cls/generate-update-shapes + (pcb/empty-changes nil (:id page)) + #{(:id copy-path)} + #(gsh/absolute-move % (gpt/point (:x %) 50)) + (:objects page) {}) + file (thf/apply-changes file changes) + + ;; Switch copy01 from V1 (c01) to V2 (c02). + file' (tho/swap-component-in-shape file :copy01 :c02 {:keep-touched? true}) + + page' (thf/current-page file') + copy01' (ths/get-shape file' :copy01) + copy-path' (->> (cfh/get-children-with-self (:objects page') (:id copy01')) + (filter #(= :path (:type %))) + first) + + ;; Expected: V2's path sits at y=30 (its master default), not y=50 + ;; (the pre-switch repositioned position). + m02 (ths/get-shape file :m02) + path2 (ths/get-shape file :path2) + target-rel-y (- (-> path2 :selrect :y) (-> m02 :selrect :y)) + actual-rel-y (- (-> copy-path' :selrect :y) (-> copy01' :selrect :y))] + + (t/is (some? copy-path') "path should exist in switched copy") + (t/is (= target-rel-y actual-rel-y) + (str "path :selrect.y should match target master layout (expected " + target-rel-y " got " actual-rel-y ")")))) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 9245d062f7..5451c5a034 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -20,6 +20,7 @@ [common-tests.files-changes-test] [common-tests.files-migrations-test] [common-tests.files.shapes-builder-test] + [common-tests.files.validate-test] [common-tests.geom-align-test] [common-tests.geom-bounds-map-test] [common-tests.geom-flex-layout-test] @@ -56,6 +57,7 @@ [common-tests.logic.swap-and-reset-test] [common-tests.logic.swap-as-override-test] [common-tests.logic.token-test] + [common-tests.logic.variants-switch-test] [common-tests.media-test] [common-tests.path-names-test] [common-tests.record-test] @@ -91,6 +93,7 @@ 'common-tests.files-changes-test 'common-tests.files-builder-test 'common-tests.files-migrations-test + 'common-tests.files.validate-test 'common-tests.geom-align-test 'common-tests.geom-bounds-map-test 'common-tests.geom-flex-layout-test @@ -127,6 +130,7 @@ 'common-tests.logic.swap-and-reset-test 'common-tests.logic.swap-as-override-test 'common-tests.logic.token-test + 'common-tests.logic.variants-switch-test 'common-tests.media-test 'common-tests.path-names-test 'common-tests.record-test diff --git a/docker/devenv/files/postgresql.conf b/docker/devenv/files/postgresql.conf index f7f19c5793..1f65ad317d 100644 --- a/docker/devenv/files/postgresql.conf +++ b/docker/devenv/files/postgresql.conf @@ -1,5 +1,5 @@ listen_addresses = '*' -max_connections = 50 +max_connections = 160 shared_buffers = 256MB temp_buffers = 18MB work_mem = 18MB diff --git a/docker/images/docker-compose.yaml b/docker/images/docker-compose.yaml index 25d419a354..cd5c9bf113 100644 --- a/docker/images/docker-compose.yaml +++ b/docker/images/docker-compose.yaml @@ -89,6 +89,7 @@ services: depends_on: - penpot-backend - penpot-exporter + - penpot-mcp networks: - penpot @@ -106,7 +107,8 @@ services: environment: << : [*penpot-flags, *penpot-http-body-size, *penpot-public-uri] - + # Set to "true" on hosts where IPv6 is disabled at kernel boot level. + # PENPOT_DISABLE_IPV6_LISTEN: "true" penpot-backend: image: "penpotapp/backend:${PENPOT_VERSION:-2.15}" restart: always diff --git a/docker/images/files/nginx-entrypoint.sh b/docker/images/files/nginx-entrypoint.sh index 571c9f6782..0948d38c61 100644 --- a/docker/images/files/nginx-entrypoint.sh +++ b/docker/images/files/nginx-entrypoint.sh @@ -46,7 +46,11 @@ export PENPOT_NITRATE_URI=${PENPOT_NITRATE_URI:-http://penpot-nitrate:3000} export PENPOT_MCP_URI=${PENPOT_MCP_URI:-http://penpot-mcp:4401} export PENPOT_MCP_URI_WS=${PENPOT_MCP_URI_WS:-http://penpot-mcp:4402} export PENPOT_HTTP_SERVER_MAX_BODY_SIZE=${PENPOT_HTTP_SERVER_MAX_BODY_SIZE:-367001600} # Default to 350MiB -envsubst "\$PENPOT_BACKEND_URI,\$PENPOT_EXPORTER_URI,\$PENPOT_NITRATE_URI,\$PENPOT_MCP_URI,\$PENPOT_MCP_URI_WS,\$PENPOT_HTTP_SERVER_MAX_BODY_SIZE" \ +export PENPOT_IPV6_LISTEN_DIRECTIVE=${PENPOT_IPV6_LISTEN_DIRECTIVE:-"listen [::]:8080 default_server;"} +if [ "${PENPOT_DISABLE_IPV6_LISTEN}" = "true" ]; then + export PENPOT_IPV6_LISTEN_DIRECTIVE="" +fi +envsubst "\$PENPOT_BACKEND_URI,\$PENPOT_EXPORTER_URI,\$PENPOT_NITRATE_URI,\$PENPOT_MCP_URI,\$PENPOT_MCP_URI_WS,\$PENPOT_HTTP_SERVER_MAX_BODY_SIZE,\$PENPOT_IPV6_LISTEN_DIRECTIVE" \ < /tmp/nginx.conf.template > /etc/nginx/nginx.conf PENPOT_DEFAULT_INTERNAL_RESOLVER="$(awk 'BEGIN{ORS=" "} $1=="nameserver" { sub(/%.*$/,"",$2); print ($2 ~ ":")? "["$2"]": $2}' /etc/resolv.conf)" diff --git a/docker/images/files/nginx.conf.template b/docker/images/files/nginx.conf.template index 3be94f0a5d..6211c7a332 100644 --- a/docker/images/files/nginx.conf.template +++ b/docker/images/files/nginx.conf.template @@ -73,7 +73,7 @@ http { server { listen 8080 default_server; - listen [::]:8080 default_server; + ${PENPOT_IPV6_LISTEN_DIRECTIVE} server_name _; client_max_body_size $PENPOT_HTTP_SERVER_MAX_BODY_SIZE; diff --git a/docs/img/customfonts-download.webp b/docs/img/customfonts-download.webp new file mode 100644 index 0000000000..cfe9ea98a9 Binary files /dev/null and b/docs/img/customfonts-download.webp differ diff --git a/docs/img/design-tokens/41-design-tab-token-dropdown.webp b/docs/img/design-tokens/41-design-tab-token-dropdown.webp new file mode 100644 index 0000000000..c642dc8824 Binary files /dev/null and b/docs/img/design-tokens/41-design-tab-token-dropdown.webp differ diff --git a/docs/img/layers/pages-separator.webp b/docs/img/layers/pages-separator.webp new file mode 100644 index 0000000000..2db5516c46 Binary files /dev/null and b/docs/img/layers/pages-separator.webp differ diff --git a/docs/img/libraries/libraries-import-tokens-button.webp b/docs/img/libraries/libraries-import-tokens-button.webp new file mode 100644 index 0000000000..6cfe1b2156 Binary files /dev/null and b/docs/img/libraries/libraries-import-tokens-button.webp differ diff --git a/docs/img/libraries/libraries-import-tokens-confirm.webp b/docs/img/libraries/libraries-import-tokens-confirm.webp new file mode 100644 index 0000000000..cc923ac079 Binary files /dev/null and b/docs/img/libraries/libraries-import-tokens-confirm.webp differ diff --git a/docs/img/objects/board-copy-as-image.webp b/docs/img/objects/board-copy-as-image.webp new file mode 100644 index 0000000000..b9e62408b2 Binary files /dev/null and b/docs/img/objects/board-copy-as-image.webp differ diff --git a/docs/img/workspace-basics/history-preview-banner.webp b/docs/img/workspace-basics/history-preview-banner.webp new file mode 100644 index 0000000000..69a7b2d323 Binary files /dev/null and b/docs/img/workspace-basics/history-preview-banner.webp differ diff --git a/docs/img/workspace-basics/history-preview-menu.webp b/docs/img/workspace-basics/history-preview-menu.webp new file mode 100644 index 0000000000..11d05a2c10 Binary files /dev/null and b/docs/img/workspace-basics/history-preview-menu.webp differ diff --git a/docs/img/workspace-basics/pixel-grid-color.webp b/docs/img/workspace-basics/pixel-grid-color.webp new file mode 100644 index 0000000000..e68da95e80 Binary files /dev/null and b/docs/img/workspace-basics/pixel-grid-color.webp differ diff --git a/docs/img/workspace-basics/ruler-guide-pill-edit.webp b/docs/img/workspace-basics/ruler-guide-pill-edit.webp new file mode 100644 index 0000000000..987b8108b2 Binary files /dev/null and b/docs/img/workspace-basics/ruler-guide-pill-edit.webp differ diff --git a/docs/technical-guide/configuration.md b/docs/technical-guide/configuration.md index ddbe801fd7..87ee6182b8 100644 --- a/docs/technical-guide/configuration.md +++ b/docs/technical-guide/configuration.md @@ -171,13 +171,14 @@ PENPOT_OIDC_CLIENT_ID: # Mainly used for auto discovery the openid endpoints PENPOT_OIDC_BASE_URI: -PENPOT_OIDC_CLIENT_SECRET: +PENPOT_OIDC_CLIENT_SECRET: # Optional backend variables, used mainly if you want override; they are # autodiscovered using the standard openid-connect mechanism. PENPOT_OIDC_AUTH_URI: PENPOT_OIDC_TOKEN_URI: PENPOT_OIDC_USER_URI: +PENPOT_OIDC_JWKS_URI: # Optional list of roles that users are required to have. If no role # is provided, roles checking disabled. @@ -187,6 +188,27 @@ PENPOT_OIDC_ROLES: "role1 role2" # not provided, the roles checking will be disabled. PENPOT_OIDC_ROLES_ATTR: ``` + +

+For self-hosted and containerized deployments, the autodiscovered OIDC endpoints are +not always enough. Some providers expose browser-facing endpoints through a public +hostname while the Penpot backend must reach the same provider through an +internal/container-resolvable hostname. In that case, explicitly set the OIDC endpoint +overrides above so the browser can use the public authorization endpoint while the +backend uses reachable token, userinfo, and JWKS endpoints. +

+ +If the backend needs to contact the OIDC provider through a hostname not already allowed +by SSRF protection, add it to: + +```bash +# Backend +# Space separated list of allowed hosts +PENPOT_SSRF_ALLOWED_HOSTS: " " +``` + +This is commonly required when the provider is reachable from the browser via a public +URL but from the backend via a different internal hostname.
__Since version 1.6.0__ diff --git a/docs/user-guide/account-teams/comments.njk b/docs/user-guide/account-teams/comments.njk index cff5f9ad9e..bec75f4858 100644 --- a/docs/user-guide/account-teams/comments.njk +++ b/docs/user-guide/account-teams/comments.njk @@ -9,6 +9,7 @@ desc: Learn how to import and export files in Penpot, the free, open-source desi

At the workspace

At the workspace, activate the comment tool by clicking the comment icon in the navbar or pressing the C key. More about comments at the Workspace

+

URLs in comments are detected automatically and open in a new browser tab when clicked.

At the View mode

You can activate comments at the View mode by pressing the comments icon at the top navbar. More about comments at the View mode.

diff --git a/docs/user-guide/design-systems/assets.njk b/docs/user-guide/design-systems/assets.njk index b0e0d91b2f..53c2818345 100644 --- a/docs/user-guide/design-systems/assets.njk +++ b/docs/user-guide/design-systems/assets.njk @@ -36,8 +36,10 @@ desc: Use Penpot's asset libraries for reusable design elements! Learn to create +

Semi-transparent colors in the library show their opacity percentage in the asset name.

Typographies

All typography styles created from the text properties (at the right sidebar) are automatically stored at the library. You can also click the “+” to create a new typography style from scratch.

+

Each typography group has its own add button to create a style directly in that group. When editing a typography style, use the delete and duplicate buttons in the typography dialog.