Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/Umbraco.Core/Models/ContentRepositoryExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,24 @@ public static bool UnpublishCulture(this IContent content, string? culture = "*"
/// Clears all publish culture information from the content item.
/// </summary>
/// <param name="content">The content item to clear publish information from.</param>
public static void ClearPublishInfos(this IContent content) => content.PublishCultureInfos = null;
public static void ClearPublishInfos(this IContent content)
{
if (content.PublishCultureInfos is null)
{
return;
}

// Pass each published culture through ClearPublishInfo([culture]) to ensure correct change tracking.
var cultures = content.PublishCultureInfos.Values.Select(c => c.Culture).ToArray();
foreach (var culture in cultures)
{
content.ClearPublishInfo(culture);
}

// Following #22799 the explicit calls to `ClearPublishInfo` for each culture cause the unpublish in all cultures.
// `PublishCultureInfos` is set to null purely to retain previous behaviour at a property level.
content.PublishCultureInfos = null;
Comment thread
kjac marked this conversation as resolved.
}

/// <summary>
/// Returns false if the culture is already unpublished
Expand Down
22 changes: 16 additions & 6 deletions src/Umbraco.Core/Services/ContentService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Immutable;

Check notice on line 1 in src/Umbraco.Core/Services/ContentService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

✅ Getting better: Overall Code Complexity

The mean cyclomatic complexity decreases from 4.06 to 4.05, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.

Check notice on line 1 in src/Umbraco.Core/Services/ContentService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 46.92% to 46.78%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -1670,147 +1670,152 @@
{
// Determine cultures publishing/unpublishing which will be based on previous calls to content.PublishCulture and ClearPublishInfo
culturesUnpublishing = content.GetCulturesUnpublishing();
culturesPublishing = variesByCulture
? content.PublishCultureInfos?.Values.Where(x => x.IsDirty()).Select(x => x.Culture).ToList()
: null;
culturesPublishing = GetCulturesPublishing(content);

// ensure that the document can be published, and publish handling events, business rules, etc
publishResult = StrategyCanPublish(
scope,
content, /*checkPath:*/
!branchOne || branchRoot,
culturesPublishing,
culturesUnpublishing,
eventMessages,
allLangs,
notificationState);

if (publishResult.Success)
{
// raise Publishing notification
if (scope.Notifications.PublishCancelable(
new ContentPublishingNotification(content, eventMessages).WithState(notificationState)))
{
_logger.LogInformation("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "publishing was cancelled");
return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, eventMessages, content);
}

// note: StrategyPublish flips the PublishedState to Publishing!
publishResult = StrategyPublish(content, culturesPublishing, culturesUnpublishing, eventMessages);

// Check if a culture has been unpublished and if there are no cultures left, and then unpublish document as a whole
if (publishResult.Result == PublishResultType.SuccessUnpublishCulture &&
content.PublishCultureInfos?.Count == 0)
{
// This is a special case! We are unpublishing the last culture and to persist that we need to re-publish without any cultures
// so the state needs to remain Publishing to do that. However, we then also need to unpublish the document and to do that
// the state needs to be Unpublishing and it cannot be both. This state is used within the documentRepository to know how to
// persist certain things. So before proceeding below, we need to save the Publishing state to publish no cultures, then we can
// mark the document for Unpublishing.
SaveDocument(content);

// Set the flag to unpublish and continue
unpublishing = content.Published; // if not published yet, nothing to do
}
}
else
{
// in a branch, just give up
if (branchOne && !branchRoot)
{
return publishResult;
}

// Check for mandatory culture missing, and then unpublish document as a whole
if (publishResult.Result == PublishResultType.FailedPublishMandatoryCultureMissing)
{
publishing = false;
unpublishing = content.Published; // if not published yet, nothing to do

// we may end up in a state where we won't publish nor unpublish
// keep going, though, as we want to save anyways
}

// reset published state from temp values (publishing, unpublishing) to original value
// (published, unpublished) in order to save the document, unchanged - yes, this is odd,
// but: (a) it means we don't reproduce the PublishState logic here and (b) setting the
// PublishState to anything other than Publishing or Unpublishing - which is precisely
// what we want to do here - throws
content.Published = content.Published;
}
}

// won't happen in a branch
if (unpublishing)
{
if (culturesUnpublishing is null)
{
culturesUnpublishing = content.GetCulturesUnpublishing();
culturesPublishing = GetCulturesPublishing(content);
}

IContent? newest = GetById(content.Id); // ensure we have the newest version - in scope
if (content.VersionId != newest?.VersionId)
{
return new PublishResult(PublishResultType.FailedPublishConcurrencyViolation, eventMessages, content);
}

if (content.Published)
{
// ensure that the document can be unpublished, and unpublish
// handling events, business rules, etc
// note: StrategyUnpublish flips the PublishedState to Unpublishing!
// note: This unpublishes the entire document (not different variants)
unpublishResult = StrategyCanUnpublish(scope, content, eventMessages, notificationState);
if (unpublishResult.Success)
{
unpublishResult = StrategyUnpublish(content, eventMessages);
}
else
{
// reset published state from temp values (publishing, unpublishing) to original value
// (published, unpublished) in order to save the document, unchanged - yes, this is odd,
// but: (a) it means we don't reproduce the PublishState logic here and (b) setting the
// PublishState to anything other than Publishing or Unpublishing - which is precisely
// what we want to do here - throws
content.Published = content.Published;
return unpublishResult;
}
}
else
{
// already unpublished - optimistic concurrency collision, really,
// and I am not sure at all what we should do, better die fast, else
// we may end up corrupting the db
throw new InvalidOperationException("Concurrency collision.");
}
}

// Persist the document
SaveDocument(content);

// we have tried to unpublish - won't happen in a branch
if (unpublishing)
{
// and succeeded, trigger events
if (unpublishResult?.Success ?? false)
{
// events and audit
scope.Notifications.Publish(
new ContentUnpublishedNotification(content, eventMessages).WithState(notificationState));
scope.Notifications.Publish(new ContentTreeChangeNotification(
content,
TreeChangeTypes.RefreshBranch,
variesByCulture ? culturesPublishing.IsCollectionEmpty() ? null : culturesPublishing : null,
variesByCulture ? culturesUnpublishing.IsCollectionEmpty() ? null : culturesUnpublishing : ["*"],
eventMessages));

if (culturesUnpublishing != null)
{
// This will mean that that we unpublished a mandatory culture or we unpublished the last culture.
var langs = GetLanguageDetailsForAuditEntry(allLangs, culturesUnpublishing);
Audit(AuditType.UnpublishVariant, userId, content.Id, $"Unpublished languages: {langs}", langs);

if (publishResult == null)
PublishResultType? publishResultType = publishResult?.Result ?? unpublishResult?.Result;
if (publishResultType == null)
{
throw new PanicException("publishResult == null - should not happen");
throw new PanicException("publishResultType == null - should not happen");
}

switch (publishResult.Result)
switch (publishResultType)

Check warning on line 1818 in src/Umbraco.Core/Services/ContentService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

❌ Getting worse: Complex Method

CommitDocumentChangesInternal increases in cyclomatic complexity from 57 to 58, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
{
case PublishResultType.FailedPublishMandatoryCultureMissing:
// Occurs when a mandatory culture was unpublished (which means we tried publishing the document without a mandatory culture)
Expand Down Expand Up @@ -2428,6 +2433,11 @@
return result;
}

private IReadOnlyList<string>? GetCulturesPublishing(IContent content)
=> content.ContentType.VariesByCulture()
? content.PublishCultureInfos?.Values.Where(x => x.IsDirty()).Select(x => x.Culture).ToList()
: null;

#endregion

#region Delete
Expand Down
19 changes: 17 additions & 2 deletions src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,14 @@ public async Task AddOrUpdateStatusAsync(Guid documentKey, CancellationToken can
{
using ICoreScope scope = _coreScopeProvider.CreateCoreScope();
ISet<string> publishedCultures = await _publishStatusRepository.GetPublishStatusAsync(documentKey, cancellationToken);
_publishedCultures[documentKey] = publishedCultures;
UpdatePublishedCultures(documentKey, publishedCultures);
scope.Complete();
}

/// <inheritdoc/>
public Task RemoveAsync(Guid documentKey, CancellationToken cancellationToken)
{
_publishedCultures.TryRemove(documentKey, out _);
RemovePublishedCultures(documentKey);
return Task.CompletedTask;
}

Expand All @@ -166,8 +166,23 @@ public async Task AddOrUpdateStatusWithDescendantsAsync(Guid rootDocumentKey, Ca
}

foreach ((Guid documentKey, ISet<string> publishedCultures) in publishStatus)
{
UpdatePublishedCultures(documentKey, publishedCultures);
}
}

private void UpdatePublishedCultures(Guid documentKey, ISet<string> publishedCultures)
{
if (publishedCultures.Count > 0)
{
_publishedCultures[documentKey] = publishedCultures;
}
else
{
RemovePublishedCultures(documentKey);
}
}

private void RemovePublishedCultures(Guid documentKey)
=> _publishedCultures.TryRemove(documentKey, out _);
}
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,78 @@ await GetRequiredService<ILanguageService>()
Assert.IsFalse(PublishStatusQueryService.HasPublishedAncestorPath(grandchild.Key, cultureToUnpublish));
Assert.IsTrue(PublishStatusQueryService.HasPublishedAncestorPath(grandchild.Key, Constants.System.InvariantCulture));
}

[TestCase(true)]
[TestCase(false)]
public async Task Fully_Unpublished_Culture_Variant_Document_Tracks_Unpublished_State_For_All_Cultures(bool unpublishAllCulturesAtOnce)
{
await GetRequiredService<ILanguageService>()
.CreateAsync(new Language("da-DK", "Danish"), Constants.Security.SuperUserKey);

var contentTypeKey = Guid.NewGuid();
var contentType = new ContentTypeBuilder()
.WithKey(contentTypeKey)
.WithAlias("variant")
.WithContentVariation(ContentVariation.Culture)
.WithAllowAsRoot(true)
.Build();
await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey);
contentType.AllowedContentTypes = [new ContentTypeSort(contentTypeKey, 1, "variant")];
await ContentTypeService.UpdateAsync(contentType, Constants.Security.SuperUserKey);

IContent root = new ContentBuilder()
.WithContentType(contentType)
.WithCultureName("en-US", "Root EN")
.WithCultureName("da-DK", "Root DA")
.Build();
ContentService.Save(root);

IContent child = new ContentBuilder()
.WithContentType(contentType)
.WithCultureName("en-US", "Child EN")
.WithCultureName("da-DK", "Child DA")
.WithParent(root)
.Build();
ContentService.Save(child);

IContent grandchild = new ContentBuilder()
.WithContentType(contentType)
.WithCultureName("en-US", "Grandchild EN")
.WithCultureName("da-DK", "Grandchild DA")
.WithParent(child)
.Build();
ContentService.Save(grandchild);

ContentService.PublishBranch(root, PublishBranchFilter.IncludeUnpublished, ["en-US", "da-DK"]);

// must refresh the child instance before unpublishing it, to reflect the state changes from the branch publish above
child = ContentService.GetById(child.Key)!;

if (unpublishAllCulturesAtOnce)
{
ContentService.Unpublish(child);
}
else
{
ContentService.Unpublish(child, "en-US");

// refresh again before unpublishing the last culture
child = ContentService.GetById(child.Key)!;
ContentService.Unpublish(child, "da-DK");
}

// refresh to get the latest state
child = ContentService.GetById(child.Key)!;
Assert.IsFalse(child.Published);
Assert.IsEmpty(child.PublishedCultures);
Assert.IsEmpty(child.PublishCultureInfos!);

Assert.IsFalse(PublishStatusQueryService.IsDocumentPublished(child.Key, "en-US"));
Assert.IsFalse(PublishStatusQueryService.IsDocumentPublished(child.Key, "da-DK"));
Assert.IsFalse(PublishStatusQueryService.IsDocumentPublished(child.Key, Constants.System.InvariantCulture));

Assert.IsFalse(PublishStatusQueryService.HasPublishedAncestorPath(grandchild.Key, "da-DK"));
Assert.IsFalse(PublishStatusQueryService.HasPublishedAncestorPath(grandchild.Key, "en-US"));
Assert.IsFalse(PublishStatusQueryService.HasPublishedAncestorPath(grandchild.Key, Constants.System.InvariantCulture));
}
}
Loading