[DISCUSSION - DO NOT MERGE] Sealing Permissionable / Inode on the real sources — the cost, the blocker, and the one branch that almost works (#34154) - #36994
Conversation
…34154) 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) <noreply@anthropic.com>
…lly costs (#34154) Round 1 sealed Permissionable over four representative subtypes and declared Inode non-sealed. This models the complete hierarchy — 40 types, 25 packages — and seals it two levels deep: Permissionable -> Inode -> WebAsset, with Contentlet sealing its own eight subclasses. Four results the first round could not produce: - The permits clause everyone writes from memory (Inode permits IHTMLPage, Container, Link, Contentlet) fails with ten errors, each one a fact about the real hierarchy: Contentlet is no relation of Inode, Container and Link arrive via WebAsset, and a page is a Contentlet. Writing the clause is what disproves the mental model. - Sealing Inode buys exhaustiveness NOTHING while Inode stays instantiable: the compiler requires a case for the base type itself, and that case is a default in all but name. Adding a subclass compiles in silence until Inode is made abstract. - The eight sub-interfaces of Permissionable cost eight unreachable switch cases out of 23 — exhaustiveness is recursive, and a non-sealed interface is covered by nothing. - Sealing is not a refactor with a price tag but the removal of a published extension point: com.dotmarketing.beans and .business are exported to plugin bundles, and a plugin can never be a permitted subtype. Demonstrated at compile time and again at class load, with IncompatibleClassChangeError. Still docs-only and still DO NOT MERGE: the sources sit outside every Maven source root because five of the seven experiments are supposed to fail, and run.sh needs only a JDK. Supersedes the closed #36992. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
❌ Linked Issue Needs Team LabelThis PR is linked to issue #36992, but that issue has no How to fix this:Apply a This comment was automatically generated by the issue linking workflow |
|
Claude finished @fabrizzio-dotCMS's task in 2m 24s —— View job ReviewDocs-only experiment, The findings are all cross-reference errors in the in-source javadoc — worth fixing precisely because this PR's whole thesis is precision, but non-blocking. New Issues
The experiment design itself is solid — the scripts, patches, and recorded error outputs support the README's numbers. |
…es (#34154) The model tree answered what a sealed hierarchy would look like; it could not answer what dotCMS's own compiler says. This applies `sealed` to the real Permissionable, Inode and ContentType, runs the real Maven compile against the real classpath and the real Immutables processor, records the output, and reverts. No production file is modified by this commit — real-tree/run-on-real-tree.sh patches, measures, and restores. Four things only the real tree could produce: - Sealing Permissionable is a 22-file change before it is a module problem. One line gives 42 distinct errors: 20 "cannot extend a sealed class in a different package" and 22 "sealed, non-sealed or final modifiers expected" — every permitted subtype must be re-declared. Inode: 5 + 6. - ContentType is the one branch where packages are not the obstacle (all nine subclasses share its package) and it still fails, with two errors of a kind no permits clause can fix: `new ContentType() {}` in StructureTransformer:102 and DbContentTypeTransformer:60. Anonymous classes cannot extend a sealed class. Two refactors and that branch becomes sealable in place. - permits DOES resolve annotation-processor-generated classes: SimpleContentType seals green over ImmutableSimpleContentType — but only once the generator's package-private nested `Json` subclass is named too, which couples the declaration to Immutables internals. This closes the open question the model left. - A control run (one subtype deliberately omitted) proves javac names what is missing, so the zero "not allowed to extend" results elsewhere mean the implementor lists are complete for dotCMS/src/main/java rather than merely unreported. Also worth knowing for anyone reading a raw log: annotation processing makes javac report every diagnostic twice, so the honest counts are half the raw ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fabrizzio-dotCMS
changed the title
1 participant
Summary
Follow-up to #36982, which turned
resolvePermissionType'sthirteen chained
instanceoftests into a patternswitch. That switch still ends in adefault— thecatch-all that stays quiet when a new asset type appears. Sealing the hierarchy is what would let the
defaultgo, so this measures whether that is possible.Two experiments, because they answer different halves:
real-tree/Permissionable,InodeandContentTypesealed, in place, and runs the real Maven compile — real classpath, real Immutables processor, every dotCMS subclass in scopesrc/No production file is modified by this PR.
run-on-real-tree.shapplies a patch todotCMS/src/main/java, records what came back, and reverts; it refuses to start if that tree is dirty.Every number below is its output, committed under
real-tree/results/.Sealing
Permissionableis a 22-file change before it is a module problemOne line changed —
public interface→public sealed interface … permits <22 types>:The first 20 are the known rule (unnamed module ⇒ permitted subtypes must share the package; only
TreeableandRuleablealready do). The other 22 are the part nobody predicts: every permittedsubtype must itself be re-declared
final/sealed/non-sealed. SealingInodegives the same shapeat smaller scale — 5 + 6.
A control run with one subtype deliberately left out shows javac names what is missing, by name, even
with the rest of the clause already in error — so the zero
not allowed to extendresults elsewheremean the implementor lists are complete for
dotCMS/src/main/java, not merely unreported.ContentType: the one branch where packages are not the obstacle — and it still failsAll nine
ContentTypesubclasses live in its own package, so the module rule has nothing to bite on.Seal it anyway and exactly two errors come back, of a kind no
permitsclause can fix:com/dotcms/contenttype/transform/contenttype/StructureTransformer.java:102—new ContentType() { … }com/dotcms/contenttype/transform/contenttype/DbContentTypeTransformer.java:60— sameTwo refactors (anonymous → named type) and that branch becomes sealable in place, today. That is the
one actionable item in the whole investigation, and it is invisible from a model of the hierarchy.
permitsdoes work with generated classes — at a priceEvery concrete
ContentTypeis produced by the Immutables processor, sopermitsmust name classes thatdo not exist until the processor has run. First attempt:
Line 1980 is not
ImmutableSimpleContentType— it isstatic final class Json extends SimpleContentType,a nested helper the generator emits for Jackson. Name both and the real build goes green:
So javac resolves processor-generated classes fine, but sealing an
@Value.Immutabletype writes thegenerator's package-private internals into your own declaration — which move when its version does.
From the model: three results the real tree cannot reach
The real tree stops at the package rule, so it can never show a working sealed hierarchy.
The hierarchy is not the shape any of us remembers. Compiling
Inode permits IHTMLPage, Container, Link, Contentletverbatim gives ten errors, each a fact:Contentletis no relation ofInode(it implements
Permissionabledirectly),Container/Linkarrive viaWebAsset, a page is aContentlet, andHostextendsContentlet. Writing the clause down is what disproves the picture.Sealing
Inodebuys exhaustiveness nothing whileInodeis instantiable. Add a subclass and itcompiles in silence:
Inodeis not abstract (Inode.java:40), so the compiler demands a case for thebase type, and
case Inode _swallows every future subclass — adefaultin all but name. Make itabstract and the switch finally breaks. Also: the top-level switch covers the whole subtree with one
case Inode i, sealed or not, so the Inode-level seal only ever pays off one level down.Eight of the resolver's 23 cases exist only to satisfy the compiler, one per
non-sealedsub-interface, none reachable in practice — exhaustiveness is recursive and a non-sealed interface is
covered by nothing.
The part that is not a cost you can choose to pay
com.dotmarketing.beans,com.dotmarketing.business,…portlets.contentlet.model,…htmlpageasset.modelandcom.dotcms.contenttype.model.typeare all published to plugin bundles inosgi-extra.conf(lines 126, 237, 336, 152, 27). ImplementingPermissionablefrom a plugin issupported today. Sealed it is not — and it cannot be opted in, because permitted subtypes must live in
the same module:
Enforced at compile time and again at class load. Every existing plugin implementing
Permissionablefails on startup with no source change on their side.
The honest limit
Even fully sealed, this resolver keeps most of its shape: half its branches do not dispatch on Java
types. A Site usually arrives as a plain
Contentletwhose content type is namedHost— a row in thedatabase. Sealed types verify the variants that live in the type system; dotCMS's live in the DB. That is
why the method's
defaultis honesty rather than laziness.Testing
real-tree/run-on-real-tree.sh(5 real Maven compiles, ~4 min, reverts the tree) andrun.sh(themodel, ~10s). Expected: A
exit 1· Bexit 1· B2exit 1· Cexit 1· Dexit 0; model1:1 2:0 3:1 4:1 5a:0 5b:1 6:1 7a:1 7b:1. Baseline./mvnw compile -pl :dotcms-coreis green on thisbranch, and
git statusis clean after a run.Breaking Changes
None — no production file is modified.
Context
Devoxx Belgium 2025 Lunch and Learn groundwork. Supersedes the closed #36992, which only modelled the
first level and never touched the real sources.
This PR fixes: #34154
🤖 Generated with Claude Code