Repository navigation
Add publishing transitions, versions and reference integrity - #2756
Merged
Merged
Conversation
3 tasks
3 tasks
This was referenced Nov 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2695
Adds publish, unpublish, discard, version reads, version restore and diff to
IContentServiceandIContentService<T>, onEntryStore, with reference integrity at publish and version pruning. Each transition is a write likeSaveAsync: the permission, the entry and its row rule, then in one transaction the field limits, hooks, validation, the write and the outbox rows, with the sink and the response after the commit. No system migration: nothing in the system model changed.Dependencies: none.
Acceptance
Under Q19, everything here is SQLite only until #2711.
PublishingTransitionTestscovers save draft, publish, unpublish, discard and restore version, andPublishingEventTestscovers their events. Trash and restore from trash came with Add the content service with optimistic concurrency #2747, and here they are checked against published rows. Purge is Trash with retention and purge job #2696. Schedule is Scheduled publish and unpublish per locale #2699.ReferenceIntegrityTests.VersionDiffTests:sections[1].heading,body.content[1].content[0].text.Verification
On Windows:
dotnet build -c Release -warnaserror,dotnet test -c Release(2362 tests, all passed) anddotnet pack -c Release(exit 0).markdownlint-cli2on the spec found nothing. The review's probes were rerun against the fixes and are not in the suite; what they check is.I also disabled 60 guards one at a time and ran the publishing tests, in three passes. 57 made a test fail. One survivor showed a duplicated check, which I removed. The other two each needed one more case, which I added: a locale guard that a later check masks unless a target has rows in both a covered and an uncovered locale, and trash counting as existing, which only matters to a principal that cannot read the entry. Both new tests were confirmed to catch their mutation. The first pass was not rerun after the duplicate check was removed. The event mapping was not mutated: without it, the transition throws when the outbox row is staged. The script is not in the repository.
Low-privilege behaviour
Tests are in
PublishingAccessTests,ReferenceIntegrityTestsandNestedTargetTests, from the low-privilege side.draftorpreviewtoken is refused: a preview key shows the draft as it is, not everything it has been. An unknown type and an unreadable one give the engine's one error, so type names cannot be tried.update, notrestore, because it writes a draft. Non-system principals get only what they could send themselves: a field that is unwritable, unreadable, hidden or read-only keeps the value the draft holds, at any depth, and so do relation targets they cannot read. Targets they cannot read do not come back from the snapshot. The system principal restores everything. The row rule is checked again after the write, and a restore that leaves it rolls back.update. A principal that may write every field gets the published values back whole. One with a field or locale limit changes only what it may write, keeps hidden and read-only values at every depth, and the entry staysModified.ContentAccessDeniedExceptionand name no field.AuditPrincipal.Id, authorization fromPrincipal.Reference integrity is not an oracle
A write takes any id without looking, so anything publish says about a target would let a principal ask about any entry. For a principal other than the system principal:
FCMS2403with the idFCMS2403on the fieldFCMS2404, no id, all identicalFCMS2404on the fieldThe system principal gets the id on
FCMS2404. A target counts as published if it has a published row in the entry's locale or its fallback chain. A self-reference is exempt. A cycle stays blocked: the way out is to publish one side without the reference and set it afterwards, which #2703 should note.The decision to review: under
Block, a principal that cannot read the target's type can never publish that reference, even when the target is fine. A scheduled publish runs as the system principal and is not affected. The alternative, deciding on the target's real state, tells the principal whether any id it names exists.Partial writes and events
AHookThatThrowsLeavesNothingBehindAndTellsNoOneruns each before and after hook that can fail on publish and unpublish, andAfterUpdateon discard and restore. It compares every row of the entry (document, versions, both states, references) and checks the sink heard nothing.EntryEvents.FrommapsPublished,Unpublished,DraftDiscardedandVersionRestored. All four add to the same pending list asSaveAsyncinsideInTransactionAsync, so they stage outbox rows in the transaction and dispatch after the commit.PublishingEventTestshas one test per transition for each of: committed (handler told, row staged, same event id), rolled back (neither), and in a transaction the service did not begin (row goes with it, no in-process handler told).A
BeforeValidateorBeforePublishhook that saves the draft through the service, in any locale, is refused withFCMS2405and rolls back. Otherwise the shared row it moved would go out unvalidated.The decision on #2740's question
Spec 4.5 records it, and the query engine does it. On every read, a relation target the principal may not read by type or row rule, in another site, of another type than the field's, or with rows only in locales its grants do not cover, is null or left out of a list. A rule is asked only of the locale being read, so where that locale has no rule, only rule-free grants count as covering a locale. A principal's own entry that exists only in a rule-limited locale is therefore left out when read through another locale, though a direct read in that locale shows it; this fails closed. A published read also leaves out a target that is trashed or has no published row in any locale. A target published only in another locale stays
{ id, type }. A trashed target of a readable type stays a stub on a draft read, as existing draft behaviour.These rules hold for top-level relations, relations inside blocks and components, rich text entry links, and relations in the
dataof an inline block. A link keeps its text and loses its target, and a link to an unknown type counts as one to an unreadable type. The extractor now reports inline blocks, andEntryStoreresolves their fields from the schema, socms_referenceand integrity see those relations too. An inline block of an unregistered type is left alone.AnUnpublishedOrTrashedTargetStaysAReferencefrom #2740 becameAnUnpublishedOrTrashedTargetIsNotReturnedByAPublishedRead.A consequence the review did not list: once a read drops nested targets, a low-privilege save of what it read would have wiped them. So
EntryComposer.CarryProtectedcarries them, as it does hidden values, skipping any already in the list, and they do not count when blocks are paired. Top-level relations already behaved this way.Keeping is not an existence test. This reverses #2747 for this case, on the orchestrator's decision. A non-system principal keeps every target its read left out, with no test of whether it exists, so a later warning says the same whatever the id is. The exception is a principal whose read grant on that type has no row rule and no locale limit, because a read tells it anyway: it loses a target that does not exist in that type and site, trashed counting as existing, which is how a ghost gets cleared. The system principal keeps nothing it did not send. This covers top-level relations, blocks and rich text.
Beyond the specification
Decided here, and worth a reviewer's eye:
IContentServicerather thanIPublishingService. Spec 4.4 and Add the content service with optimistic concurrency #2747 put these on the content service, so I removedIPublishingServicefrom the spec's other two mentions.expectedVersionis optional on publish, discard and restore, so a caller can publish only what it reviewed. A stale one is aVersionConflictException. Added to 4.4.GetVersionAsyncis new, because the spec's list returns no values.PublishResult<T>,PublishWarning,VersionChangeandPublishPolicyAttributeare new SDK types;ITypeBuilder.PublishPolicyandSchemaType.UnpublishedReferences/MaxVersionsare new members.FluentCMS.Testing's scoped services forward the new methods.status_afteron its row, which avoids a migration. It is the one column of a version that changes. A discard or restore row recordsModifiedeven when the entry's status isPublished, so it cannot pass the cap.published_versionnames what a locale last published; its shared fields can have moved since through another locale's publish, so what it serves can differ from that snapshot. Spec 4.5 says so.BeforeValidate,BeforePublish,AfterPublish. Unpublish runs its pair; what they change is ignored. Discard and restore are saves, so they runBeforeValidate,BeforeUpdate,AfterUpdateunder draft rules, not the restore hooks, which belong to the trash.Published.Modifiedstatus right.'', so a second entry with an empty slug is refused on publish withFCMS2111instead of failing at the index.FieldValue.IsEmptytreats a rich text document with no visible content as empty:RichText.Empty, no blocks, several empty paragraphs, whitespace only. An image or a rule is content. This also applies toCondition.IsEmpty.Known gaps
entry:{id}covers it.Blockthe entry stays refused until a principal with an open read of that type clears it.dataof an inline block has no field rules. It is not validated against its block on write, and a field marked hidden there is not removed on read. Only relations in it are filtered, hidden or not. This predates this PR and needs its own issue.notNull. Spec 4.5 records this.ValidationExceptionandContentAccessDeniedException), as Add the content service with optimistic concurrency #2747 does. Only version reads were changed.publishedAtand raises the event again.MarkPublishedAsync,MarkUnpublishedAsyncand the version swap are compare and swap, but uniqueness between check and commit still relies onBEGIN IMMEDIATE, as Add the content service with optimistic concurrency #2747 notes. A second empty value would also behave differently under SQL Server's unique rules.What later issues must do
Actor, which already records the actor.Blockneeds one side set after publishing.warnings[], and mapFCMS2402toFCMS2405as 422.