From 20a66494f9689132f6dddd0146fb4a51e6da8c71 Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Mon, 10 Aug 2026 17:31:35 -0600 Subject: [PATCH] experiment(permissions): can the Permissionable hierarchy be sealed? (#34154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NOT FOR MERGE. Groundwork for the Java 25 talk. Three compilations that answer a question the codebase keeps raising: resolvePermissionType dispatches over Permissionable and ends in a default, so the day someone adds an asset type the code keeps compiling and the new type quietly takes the default path. Sealing is what would remove that default, and with it the silence. The sources live under docs/ and outside every Maven source root on purpose: two of the three experiments are supposed to fail to compile, and the failures are the result. run.sh reproduces all three with nothing but a JDK 22 — no Maven, no dotCMS classpath, no network. Sealing it where it lives fails with one error per permitted subtype: a sealed type in the unnamed module requires every permitted subtype in the same package, and dotCMS has no module-info. Against the real type that is fourteen errors, one per implementor, spread over nine packages. Moving them into one package is not an import refactor either — the canonical names are persisted data, hardcoded even in this class's SQL. The same sources inside a named module compile, and the resolver drops its default. Adding a fifteenth permitted type then breaks that resolver without anyone touching it. So the measurement: sealing this hierarchy is blocked by neither design nor size. It costs a module-info.java, and in exchange the compiler names every place that needs updating when an asset type is added. The README closes on the limit, because it matters more than the win: half this resolver's branches dispatch on a content type stored in the database rather than on a Java type. Sealed verifies the variants that live in the type system; dotCMS keeps its variants in a table. Co-Authored-By: Claude Opus 5 (1M context) --- .../sealed-permissionable/README.md | 153 ++++++++++++++++++ docs/experiments/sealed-permissionable/run.sh | 55 +++++++ .../src/com/dotmarketing/beans/Host.java | 7 + .../com/dotmarketing/beans/Identifier.java | 7 + .../src/com/dotmarketing/beans/Inode.java | 15 ++ .../business/PermissionResolver.java | 38 +++++ .../dotmarketing/business/Permissionable.java | 15 ++ .../portlets/contentlet/model/Contentlet.java | 12 ++ .../portlets/folders/model/Folder.java | 7 + .../src/module-info.java | 10 ++ 10 files changed, 319 insertions(+) create mode 100644 docs/experiments/sealed-permissionable/README.md create mode 100755 docs/experiments/sealed-permissionable/run.sh create mode 100644 docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Host.java create mode 100644 docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Identifier.java create mode 100644 docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Inode.java create mode 100644 docs/experiments/sealed-permissionable/src/com/dotmarketing/business/PermissionResolver.java create mode 100644 docs/experiments/sealed-permissionable/src/com/dotmarketing/business/Permissionable.java create mode 100644 docs/experiments/sealed-permissionable/src/com/dotmarketing/portlets/contentlet/model/Contentlet.java create mode 100644 docs/experiments/sealed-permissionable/src/com/dotmarketing/portlets/folders/model/Folder.java create mode 100644 docs/experiments/sealed-permissionable/src/module-info.java diff --git a/docs/experiments/sealed-permissionable/README.md b/docs/experiments/sealed-permissionable/README.md new file mode 100644 index 00000000000..e4e4d32fe0b --- /dev/null +++ b/docs/experiments/sealed-permissionable/README.md @@ -0,0 +1,153 @@ +# Can the `Permissionable` hierarchy be sealed? + +Groundwork for the Java 25 talk (#34154). **Nothing here is production code and nothing here is +built** — these sources live outside every Maven source root on purpose, because two of the three +experiments are supposed to fail to compile. The failures are the result. + +```bash +./run.sh # needs only a JDK 22+ — no Maven, no dotCMS classpath, no network +``` + +Everything quoted below is that script's output. + +## Why the question comes up + +`PermissionBitFactoryImpl.resolvePermissionType` dispatches over `Permissionable` with a pattern +`switch` that ends in a `default`. A `default` is a catch-all: the day someone adds an asset type, +the code keeps compiling and the new type quietly takes the default path. + +Sealing the hierarchy is what would let that `default` go away, and with it the silence — a sealed +type tells the compiler the complete list of subtypes, so it can check that a switch covers them all. + +The hierarchy is not large: **14 direct implementors of `Permissionable`, 7 direct subclasses of +`Inode`.** Perfectly listable. So the question is a fair one. + +## The setup + +`src/` mirrors the real package layout with a handful of the real types, because the package layout +is the whole point: + +| Type | Package | Role | +|---|---|---| +| `Permissionable` | `com.dotmarketing.business` | the sealed root | +| `Contentlet` | `com.dotmarketing.portlets.contentlet.model` | `sealed`, seals further down | +| `Host` | `com.dotmarketing.beans` | `final` — a leaf | +| `Folder` | `com.dotmarketing.portlets.folders.model` | `final` | +| `Identifier` | `com.dotmarketing.beans` | `final` | +| `Inode` | `com.dotmarketing.beans` | `non-sealed` — gives up and reopens | +| `PermissionResolver` | `com.dotmarketing.business` | the switch, with no `default` | + +`Inode` being `non-sealed` does **not** break exhaustiveness downstream: every subclass of `Inode` is +still an `Inode`, so one `case Inode` covers all of them. Sealing reasons about permitted subtypes, +not about leaves. + +## Experiment 1 — seal it where it lives + +Compile those sources **without** `module-info.java`, which is the situation dotCMS is in today: + +``` +src/com/dotmarketing/portlets/contentlet/model/Contentlet.java:7: error: class Contentlet in unnamed module cannot extend a sealed class in a different package +public sealed class Contentlet implements Permissionable permits Host { + ^ +src/com/dotmarketing/business/Permissionable.java:12: error: class Permissionable in unnamed module cannot extend a sealed class in a different package +public sealed interface Permissionable permits Contentlet, Folder, Identifier, Inode { + ^ +... 5 errors +``` + +One error per permitted subtype. The rule: **a sealed type in the unnamed module requires every +permitted subtype to live in the same package.** dotCMS has no `module-info.java`, so everything is +in the unnamed module. + +Running the same attempt against the *real* `Permissionable`, with all 14 implementors in the +`permits` clause, gives **14 errors — one per type, no exceptions**: none of the fourteen lives in +`com.dotmarketing.business`; they are spread across nine packages. + +```bash +# for the record, against the real type (needs the dotcms-core classpath) +javac --release 25 -cp dotCMS/target/classes: Permissionable.java +``` + +### Why "just move them into one package" is not the answer + +Their canonical names are **persisted data**. `permission_reference.permission_type` stores strings +like `com.dotmarketing.portlets.folders.model.Folder`, and those literals appear hardcoded in +`PermissionBitFactoryImpl`'s own SQL (lines 303 and 462). Moving `Folder` to another package is a +data migration on the permissions table, not an import refactor. + +## Experiment 2 — the same sources, inside a named module + +Add six lines and change nothing else: + +```java +module dotcms.permissions { + exports com.dotmarketing.business; +} +``` + +``` +exit: 0 +``` + +It compiles. Sealed across packages, `Contentlet` sealing down to `Host`, `Inode` reopening its +branch with `non-sealed` — and the resolver carries no `default`: + +```java +return switch (permissionable) { + case Host _ -> "Host"; + case Contentlet _ -> "Contentlet"; + case Folder _ -> "Folder"; + case Identifier _ -> "Identifier"; + case Inode _ -> "Inode"; +}; +``` + +(`Host` precedes `Contentlet` because it extends it — the other order is rejected with *"this case +label is dominated by a preceding case label"*.) + +## Experiment 3 — add an asset type, touch nothing else + +A fifteenth permitted type joins the `permits` clause. The resolver is left exactly as it was: + +``` +src/com/dotmarketing/business/PermissionResolver.java:25: error: the switch expression does not cover all possible input values + return switch (permissionable) { + ^ +1 error +``` + +That is the entire payoff, and it only exists because there is no `default`. **Put a `default` back +and this compiles in silence** — sealing does not give you the check; removing `default` gives you +the check, and sealing is what makes removing it possible. Any total pattern (`case Object o`, +`case Permissionable p`) silences it just the same. + +## What this measures + +> Sealing this hierarchy is blocked by neither its design nor its size. **It costs a +> `module-info.java`** — and in exchange the compiler names every place that needs updating when an +> asset type is added. + +Which reframes the modularisation discussion as a trade with a price tag instead of an abstract +preference. The price is real: `module-info` on a WAR carrying OSGi, Hibernate, reflection and split +packages is a project of its own, and JPMS and OSGi are two module systems competing for the same +job. + +## The honest limit, worth knowing before anyone gets excited + +Even fully sealed, this particular resolver would keep most of its shape, because **half its +branches do not dispatch on Java types at all.** From the test suite on #36982: + +> `test_contentletOfHostContentType_resolvesAsHost` — *resolves as a Host, even though the object is +> not a `Host` instance* + +A contentlet "of type Host" is usually a plain `Contentlet` whose content type — a row in the +database — is named `Host`. Hence the two branches for one concept: + +```java +case Host _ -> HOST; // variant is a type +case Contentlet c when isOfContentType(c, HOST_VELOCITY_VAR) -> HOST; // variant is data +``` + +Sealed types verify the variants that live in the type system. dotCMS's variants live in the +database. The `when` guard exists precisely because that variability escaped the type system — and it +is why that method's `default` is honesty rather than laziness. diff --git a/docs/experiments/sealed-permissionable/run.sh b/docs/experiments/sealed-permissionable/run.sh new file mode 100755 index 00000000000..e6f8e07e54c --- /dev/null +++ b/docs/experiments/sealed-permissionable/run.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# +# Three experiments on sealing the Permissionable hierarchy. Needs nothing but a JDK 22+ — no Maven, +# no dotCMS classpath, no network. Every claim in README.md is this script's output. +# +# 1. Sealed across packages, unnamed module -> fails, and the error is the whole point +# 2. The same sources inside a named module -> compiles, resolver has no `default` +# 3. Experiment 2 plus one more asset type -> the resolver stops compiling on its own +# +# The only delta between 1 and 2 is whether module-info.java is handed to javac. + +set -u + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SRC="${HERE}/src" +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +rule() { printf '\n\033[1m%s\033[0m\n%s\n' "$1" "$(printf '─%.0s' {1..78})"; } + +# ── 1. Unnamed module: every permitted subtype must be in the same package ─────────────────────── +rule "1. Sealed across packages, WITHOUT module-info.java (the unnamed module)" +javac --release 22 -d "${WORK}/out1" \ + $(find "${SRC}" -name '*.java' ! -name 'module-info.java') 2>&1 | sed 's|^.*/src/|src/|' +echo "exit: ${PIPESTATUS[0]} (non-zero expected — this is the wall)" + +# ── 2. Named module: permits may cross packages ────────────────────────────────────────────────── +rule "2. The very same sources, WITH module-info.java" +javac --release 22 -d "${WORK}/out2" \ + $(find "${SRC}" -name '*.java') 2>&1 | sed 's|^.*/src/|src/|' +echo "exit: ${PIPESTATUS[0]} (0 expected — sealed across packages, and the resolver has no default)" + +# ── 3. Add an asset type; nobody touches the resolver ──────────────────────────────────────────── +rule "3. One more permitted type — WorkflowAction — and the resolver is left untouched" +cp -r "${SRC}" "${WORK}/src3" +mkdir -p "${WORK}/src3/com/dotmarketing/portlets/workflows/model" +cat > "${WORK}/src3/com/dotmarketing/portlets/workflows/model/WorkflowAction.java" <<'JAVA' +package com.dotmarketing.portlets.workflows.model; + +import com.dotmarketing.business.Permissionable; + +public final class WorkflowAction implements Permissionable { + + public String getPermissionType() { + return "WorkflowAction"; + } +} +JAVA +# add it to the permits clause, and nothing else +perl -0pi -e 's/permits Contentlet, Folder, Identifier, Inode \{/permits Contentlet, Folder, Identifier, Inode,\n com.dotmarketing.portlets.workflows.model.WorkflowAction {/' \ + "${WORK}/src3/com/dotmarketing/business/Permissionable.java" + +javac --release 22 -d "${WORK}/out3" \ + $(find "${WORK}/src3" -name '*.java') 2>&1 | sed "s|^${WORK}/src3/|src/|" +echo "exit: ${PIPESTATUS[0]} (non-zero expected — the compiler hands you the work list)" diff --git a/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Host.java b/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Host.java new file mode 100644 index 00000000000..878073455c1 --- /dev/null +++ b/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Host.java @@ -0,0 +1,7 @@ +package com.dotmarketing.beans; + +import com.dotmarketing.portlets.contentlet.model.Contentlet; + +/** A leaf: the hierarchy ends here. */ +public final class Host extends Contentlet { +} diff --git a/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Identifier.java b/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Identifier.java new file mode 100644 index 00000000000..55f0757daf2 --- /dev/null +++ b/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Identifier.java @@ -0,0 +1,7 @@ +package com.dotmarketing.beans; + +import com.dotmarketing.business.Permissionable; + +public final class Identifier implements Permissionable { + public String getPermissionType() { return "Identifier"; } +} diff --git a/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Inode.java b/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Inode.java new file mode 100644 index 00000000000..e2dc883ba44 --- /dev/null +++ b/docs/experiments/sealed-permissionable/src/com/dotmarketing/beans/Inode.java @@ -0,0 +1,15 @@ +package com.dotmarketing.beans; + +import com.dotmarketing.business.Permissionable; + +/** + * Gives up and reopens the branch — Category, Field, WebAsset and UserComment all extend the real + * one. Note this does NOT break exhaustiveness downstream: every subclass of Inode is still an + * Inode, so a single `case Inode` covers all of them. + */ +public non-sealed class Inode implements Permissionable { + + public String getPermissionType() { + return "Inode"; + } +} diff --git a/docs/experiments/sealed-permissionable/src/com/dotmarketing/business/PermissionResolver.java b/docs/experiments/sealed-permissionable/src/com/dotmarketing/business/PermissionResolver.java new file mode 100644 index 00000000000..c35c1406d30 --- /dev/null +++ b/docs/experiments/sealed-permissionable/src/com/dotmarketing/business/PermissionResolver.java @@ -0,0 +1,38 @@ +package com.dotmarketing.business; + +import com.dotmarketing.beans.Host; +import com.dotmarketing.beans.Identifier; +import com.dotmarketing.beans.Inode; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.folders.model.Folder; + +/** + * The shape PermissionBitFactoryImpl#resolvePermissionType could take if the hierarchy were sealed. + */ +public final class PermissionResolver { + + private PermissionResolver() { + } + + /** + * The same switch as the real resolver — with no `default`. + * + *

Host precedes Contentlet because Host extends it; the compiler rejects the other order with + * "this case label is dominated by a preceding case label".

+ */ + public static String resolve(final Permissionable permissionable) { + + return switch (permissionable) { + + case Host _ -> "Host"; + + case Contentlet _ -> "Contentlet"; + + case Folder _ -> "Folder"; + + case Identifier _ -> "Identifier"; + + case Inode _ -> "Inode"; + }; + } +} diff --git a/docs/experiments/sealed-permissionable/src/com/dotmarketing/business/Permissionable.java b/docs/experiments/sealed-permissionable/src/com/dotmarketing/business/Permissionable.java new file mode 100644 index 00000000000..bb09a844296 --- /dev/null +++ b/docs/experiments/sealed-permissionable/src/com/dotmarketing/business/Permissionable.java @@ -0,0 +1,15 @@ +package com.dotmarketing.business; + +import com.dotmarketing.beans.Identifier; +import com.dotmarketing.beans.Inode; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.folders.model.Folder; + +/** + * Sealed, with its permitted subtypes in OTHER packages — mirroring the real dotCMS layout, where + * fourteen implementors are spread across nine packages. + */ +public sealed interface Permissionable permits Contentlet, Folder, Identifier, Inode { + + String getPermissionType(); +} diff --git a/docs/experiments/sealed-permissionable/src/com/dotmarketing/portlets/contentlet/model/Contentlet.java b/docs/experiments/sealed-permissionable/src/com/dotmarketing/portlets/contentlet/model/Contentlet.java new file mode 100644 index 00000000000..51512ffcffb --- /dev/null +++ b/docs/experiments/sealed-permissionable/src/com/dotmarketing/portlets/contentlet/model/Contentlet.java @@ -0,0 +1,12 @@ +package com.dotmarketing.portlets.contentlet.model; + +import com.dotmarketing.beans.Host; +import com.dotmarketing.business.Permissionable; + +/** Seals further down: the branch stays closed. */ +public sealed class Contentlet implements Permissionable permits Host { + + public String getPermissionType() { + return "Contentlet"; + } +} diff --git a/docs/experiments/sealed-permissionable/src/com/dotmarketing/portlets/folders/model/Folder.java b/docs/experiments/sealed-permissionable/src/com/dotmarketing/portlets/folders/model/Folder.java new file mode 100644 index 00000000000..39ed9a005fd --- /dev/null +++ b/docs/experiments/sealed-permissionable/src/com/dotmarketing/portlets/folders/model/Folder.java @@ -0,0 +1,7 @@ +package com.dotmarketing.portlets.folders.model; + +import com.dotmarketing.business.Permissionable; + +public final class Folder implements Permissionable { + public String getPermissionType() { return "Folder"; } +} diff --git a/docs/experiments/sealed-permissionable/src/module-info.java b/docs/experiments/sealed-permissionable/src/module-info.java new file mode 100644 index 00000000000..49a3215f0ad --- /dev/null +++ b/docs/experiments/sealed-permissionable/src/module-info.java @@ -0,0 +1,10 @@ +/** + * The one thing that unlocks everything below. + * + *

Delete this file and the very same sources stop compiling: a sealed type in the unnamed module + * requires every permitted subtype to live in the same package. Inside a named module, `permits` may + * cross packages. That is the whole difference between experiment 1 and experiment 2.

+ */ +module dotcms.permissions { + exports com.dotmarketing.business; +}