[DISCUSSION - DO NOT MERGE] Can the Permissionable hierarchy be sealed? (#34154)

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:<deps> 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.

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)"

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 {
+}

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"; }
+}

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";
+    }
+}

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`.
+     *
+     * <p>Host precedes Contentlet because Host extends it; the compiler rejects the other order with
+     * "this case label is dominated by a preceding case label".</p>
+     */
+    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";
+        };
+    }
+}

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();
+}

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";
+    }
+}

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"; }
+}

docs/experiments/sealed-permissionable/src/module-info.java

@@ -0,0 +1,10 @@
+/**
+ * The one thing that unlocks everything below.
+ *
+ * <p>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.</p>
+ */
+module dotcms.permissions {
+    exports com.dotmarketing.business;
+}