diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index d635b9f939..b15e371e87 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -36,11 +36,11 @@ namespace Umbraco.Core else if (culture.IsNullOrWhiteSpace()) throw new ArgumentNullException($"{nameof(culture)} cannot be null or empty"); - var expires = content.ContentSchedule.GetSchedule(culture, ContentScheduleChange.End); + var expires = content.ContentSchedule.GetSchedule(culture, ContentScheduleAction.Expire); if (expires != null && expires.Any(x => x.Date > DateTime.MinValue && DateTime.Now > x.Date)) return ContentStatus.Expired; - var release = content.ContentSchedule.GetSchedule(culture, ContentScheduleChange.Start); + var release = content.ContentSchedule.GetSchedule(culture, ContentScheduleAction.Release); if (release != null && release.Any(x => x.Date > DateTime.MinValue && x.Date > DateTime.Now)) return ContentStatus.AwaitingRelease; @@ -56,7 +56,6 @@ namespace Umbraco.Core /// Gets cultures for which content.UnpublishCulture() has been invoked. internal static IReadOnlyList GetCulturesUnpublishing(this IContent content) { - // fixme/review - assuming it's || here not && ? if (!content.Published || !content.ContentType.VariesByCulture() || !content.IsPropertyDirty("PublishCultureInfos")) return Array.Empty(); diff --git a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TablesForScheduledPublishing.cs b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TablesForScheduledPublishing.cs index 1ebd988a2d..cd4de179bd 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TablesForScheduledPublishing.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/TablesForScheduledPublishing.cs @@ -30,11 +30,11 @@ namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0 foreach(var s in schedules) { var date = s.Value.releaseDate; - var action = ContentScheduleChange.Start.ToString(); + var action = ContentScheduleAction.Release.ToString(); if (!date.HasValue) { date = s.Value.expireDate; - action = ContentScheduleChange.End.ToString(); + action = ContentScheduleAction.Expire.ToString(); } Insert.IntoTable(ContentScheduleDto.TableName) diff --git a/src/Umbraco.Core/Models/ContentSchedule.cs b/src/Umbraco.Core/Models/ContentSchedule.cs index 3eecce6321..0466a1e23a 100644 --- a/src/Umbraco.Core/Models/ContentSchedule.cs +++ b/src/Umbraco.Core/Models/ContentSchedule.cs @@ -13,12 +13,12 @@ namespace Umbraco.Core.Models /// /// Initializes a new instance of the class. /// - public ContentSchedule(int id, string culture, DateTime date, ContentScheduleChange change) + public ContentSchedule(int id, string culture, DateTime date, ContentScheduleAction action) { Id = id; Culture = culture; Date = date; - Change = change; + Action = action; } /// @@ -46,12 +46,8 @@ namespace Umbraco.Core.Models /// Gets the action to take. /// [DataMember] - public ContentScheduleChange Change { get; } + public ContentScheduleAction Action { get; } - // fixme/review - must implement Equals? - // fixing ContentScheduleCollection.Equals which was broken, breaks content Can_Deep_Clone test - // because SequenceEqual on the inner sorted lists fails, because it ends up doing reference-equal - // on each content schedule - so we *have* to implement Equals for us too? public override bool Equals(object obj) => obj is ContentSchedule other && Equals(other); @@ -59,12 +55,12 @@ namespace Umbraco.Core.Models { // don't compare Ids, two ContentSchedule are equal if they are for the same change // for the same culture, on the same date - and the collection deals w/duplicates - return Culture.InvariantEquals(other.Culture) && Date == other.Date && Change == other.Change; + return Culture.InvariantEquals(other.Culture) && Date == other.Date && Action == other.Action; } public object DeepClone() { - return new ContentSchedule(Id, Culture, Date, Change); + return new ContentSchedule(Id, Culture, Date, Action); } } } diff --git a/src/Umbraco.Core/Models/ContentScheduleChange.cs b/src/Umbraco.Core/Models/ContentScheduleAction.cs similarity index 58% rename from src/Umbraco.Core/Models/ContentScheduleChange.cs rename to src/Umbraco.Core/Models/ContentScheduleAction.cs index a89be62022..0816f17731 100644 --- a/src/Umbraco.Core/Models/ContentScheduleChange.cs +++ b/src/Umbraco.Core/Models/ContentScheduleAction.cs @@ -1,21 +1,18 @@ namespace Umbraco.Core.Models { - // fixme/review - should this be named DocumentScheduleAction? - // fixme/review - should values be Release and Expire not Start and Stop? - /// /// Defines scheduled actions for documents. /// - public enum ContentScheduleChange + public enum ContentScheduleAction { /// /// Release the document. /// - Start, + Release, /// /// Expire the document. /// - End + Expire } } diff --git a/src/Umbraco.Core/Models/ContentScheduleCollection.cs b/src/Umbraco.Core/Models/ContentScheduleCollection.cs index f80561baf2..2d7206ff06 100644 --- a/src/Umbraco.Core/Models/ContentScheduleCollection.cs +++ b/src/Umbraco.Core/Models/ContentScheduleCollection.cs @@ -75,14 +75,14 @@ namespace Umbraco.Core.Models if (releaseDate.HasValue) { - var entry = new ContentSchedule(0, culture, releaseDate.Value, ContentScheduleChange.Start); + var entry = new ContentSchedule(0, culture, releaseDate.Value, ContentScheduleAction.Release); changes.Add(releaseDate.Value, entry); OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, entry)); } if (expireDate.HasValue) { - var entry = new ContentSchedule(0, culture, expireDate.Value, ContentScheduleChange.End); + var entry = new ContentSchedule(0, culture, expireDate.Value, ContentScheduleAction.Expire); changes.Add(expireDate.Value, entry); OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, entry)); } @@ -112,45 +112,46 @@ namespace Umbraco.Core.Models /// /// Clear all of the scheduled change type for invariant content /// - /// + /// /// If specified, will clear all entries with dates less than or equal to the value - public void Clear(ContentScheduleChange changeType, DateTime? changeDate = null) + public void Clear(ContentScheduleAction action, DateTime? changeDate = null) { - Clear(string.Empty, changeType, changeDate); + Clear(string.Empty, action, changeDate); } /// /// Clear all of the scheduled change type for the culture /// /// - /// - /// If specified, will clear all entries with dates less than or equal to the value - public void Clear(string culture, ContentScheduleChange changeType, DateTime? changeDate = null) + /// + /// If specified, will clear all entries with dates less than or equal to the value + public void Clear(string culture, ContentScheduleAction action, DateTime? date = null) { - if (_schedule.TryGetValue(culture, out var s)) - { - foreach (var ofChange in s.Where(x => x.Value.Change == changeType - && (changeDate.HasValue ? x.Value.Date <= changeDate.Value : true)).ToList()) - { - var removed = s.Remove(ofChange.Value.Date); - if (removed) - { - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, ofChange.Value)); - if (s.Count == 0) - _schedule.Remove(culture); - } - } + if (!_schedule.TryGetValue(culture, out var schedules)) + return; + var removes = schedules.Where(x => x.Value.Action == action && (!date.HasValue || x.Value.Date <= date.Value)).ToList(); + + foreach (var remove in removes) + { + var removed = schedules.Remove(remove.Value.Date); + if (!removed) + continue; + + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, remove.Value)); } + + if (schedules.Count == 0) + _schedule.Remove(culture); } /// /// Returns all pending schedules based on the date and type provided /// - /// + /// /// /// - public IReadOnlyList GetPending(ContentScheduleChange changeType, DateTime date) + public IReadOnlyList GetPending(ContentScheduleAction action, DateTime date) { return _schedule.Values.SelectMany(x => x.Values).Where(x => x.Date <= date).ToList(); } @@ -159,9 +160,9 @@ namespace Umbraco.Core.Models /// Gets the schedule for invariant content /// /// - public IEnumerable GetSchedule(ContentScheduleChange? changeType = null) + public IEnumerable GetSchedule(ContentScheduleAction? action = null) { - return GetSchedule(string.Empty, changeType); + return GetSchedule(string.Empty, action); } /// @@ -169,10 +170,10 @@ namespace Umbraco.Core.Models /// /// /// - public IEnumerable GetSchedule(string culture, ContentScheduleChange? changeType = null) + public IEnumerable GetSchedule(string culture, ContentScheduleAction? action = null) { if (_schedule.TryGetValue(culture, out var changes)) - return changeType == null ? changes.Values : changes.Values.Where(x => x.Change == changeType.Value); + return action == null ? changes.Values : changes.Values.Where(x => x.Action == action.Value); return Enumerable.Empty(); } @@ -208,7 +209,6 @@ namespace Umbraco.Core.Models if (thisSched.Count != thatSched.Count) return false; - // fixme/review - code was returning false *if* thatList.SequenceEqual(thisList) and not the opposite? foreach (var (culture, thisList) in thisSched) { // if culture is missing, or actions differ, false diff --git a/src/Umbraco.Core/Models/Language.cs b/src/Umbraco.Core/Models/Language.cs index 7cb83c601d..e190c8ad3b 100644 --- a/src/Umbraco.Core/Models/Language.cs +++ b/src/Umbraco.Core/Models/Language.cs @@ -73,10 +73,9 @@ namespace Umbraco.Core.Models // so, the logic below ensures that the name always end up being the correct name // see also LanguageController.GetAllCultures which is doing the same // - // fixme/review - stop saving language names in database? // all this, including the ugly settings injection, because se store language names in db, // otherwise it would be ok to simply return new CultureInfo(IsoCode).DisplayName to get the name - // in whatever culture is current - could we ... not do it? + // in whatever culture is current - we should not do it, see task #3623 // // but then, some tests that compare audit strings (for culture names) would need to be fixed diff --git a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs index bb16e62e4e..c44244bd1b 100644 --- a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs @@ -178,7 +178,7 @@ namespace Umbraco.Core.Persistence.Factories return entity.ContentSchedule.FullSchedule.Select(x => new ContentScheduleDto { - Action = x.Change.ToString(), + Action = x.Action.ToString(), Date = x.Date, NodeId = entity.Id, LanguageId = languageRepository.GetIdByIsoCode(x.Culture, false), diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 02df4172d6..88adbc346a 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -917,14 +917,14 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// public IEnumerable GetContentForRelease(DateTime date) { - var action = ContentScheduleChange.Start.ToString(); + var action = ContentScheduleAction.Release.ToString(); var sql = GetBaseQuery(QueryType.Many) .WhereIn(x => x.NodeId, Sql() .Select(x => x.NodeId) .From() .Where(x => x.Action == action && x.Date <= date)); - + AddGetByQueryOrderBy(sql); return MapDtosToContent(Database.Fetch(sql)); @@ -933,9 +933,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// public IEnumerable GetContentForExpiration(DateTime date) { - var action = ContentScheduleChange.End.ToString(); + var action = ContentScheduleAction.Expire.ToString(); - // fixme/review - see as above var sql = GetBaseQuery(QueryType.Many) .WhereIn(x => x.NodeId, Sql() .Select(x => x.NodeId) @@ -1165,17 +1164,15 @@ namespace Umbraco.Core.Persistence.Repositories.Implement foreach (var scheduleDto in scheduleDtos) { - // fixme/review - code was adding a new collection on each dto? - if (!result.TryGetValue(scheduleDto.NodeId, out var col)) col = result[scheduleDto.NodeId] = new ContentScheduleCollection(); col.Add(new ContentSchedule(scheduleDto.Id, LanguageRepository.GetIsoCodeById(scheduleDto.LanguageId) ?? string.Empty, scheduleDto.Date, - scheduleDto.Action == ContentScheduleChange.Start.ToString() - ? ContentScheduleChange.Start - : ContentScheduleChange.End)); + scheduleDto.Action == ContentScheduleAction.Release.ToString() + ? ContentScheduleAction.Release + : ContentScheduleAction.Expire)); } return result; diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 2b04757450..6c08e3aad9 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -362,8 +362,6 @@ namespace Umbraco.Core.Services /// PublishResult SavePublishing(IContent content, int userId = 0, bool raiseEvents = true); - // fixme/review - should SaveAndPublishBranch always publish the root document of the branch, even when !force? - /// /// Saves and publishes a document branch. /// diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 69ada21b35..613737bf06 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1197,7 +1197,7 @@ namespace Umbraco.Core.Services.Implement if (d.ContentType.VariesByCulture()) { //find which cultures have pending schedules - var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleChange.Start, date) + var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleAction.Release, date) .Select(x => x.Culture) .Distinct() .ToList(); @@ -1206,7 +1206,7 @@ namespace Umbraco.Core.Services.Implement foreach (var culture in pendingCultures) { //Clear this schedule for this culture - d.ContentSchedule.Clear(culture, ContentScheduleChange.Start, date); + d.ContentSchedule.Clear(culture, ContentScheduleAction.Release, date); if (d.Trashed) continue; // won't publish @@ -1229,7 +1229,7 @@ namespace Umbraco.Core.Services.Implement else { //Clear this schedule - d.ContentSchedule.Clear(ContentScheduleChange.Start, date); + d.ContentSchedule.Clear(ContentScheduleAction.Release, date); result = d.Trashed ? new PublishResult(PublishResultType.FailedPublishIsTrashed, evtMsgs, d) @@ -1248,7 +1248,7 @@ namespace Umbraco.Core.Services.Implement if (d.ContentType.VariesByCulture()) { //find which cultures have pending schedules - var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleChange.End, date) + var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleAction.Expire, date) .Select(x => x.Culture) .Distinct() .ToList(); @@ -1256,7 +1256,7 @@ namespace Umbraco.Core.Services.Implement foreach (var c in pendingCultures) { //Clear this schedule for this culture - d.ContentSchedule.Clear(c, ContentScheduleChange.End, date); + d.ContentSchedule.Clear(c, ContentScheduleAction.Expire, date); //set the culture to be published d.UnpublishCulture(c); } @@ -1272,7 +1272,7 @@ namespace Umbraco.Core.Services.Implement else { //Clear this schedule - d.ContentSchedule.Clear(ContentScheduleChange.End, date); + d.ContentSchedule.Clear(ContentScheduleAction.Expire, date); result = Unpublish(d, userId: d.WriterId); if (result.Success == false) Logger.Error(null, "Failed to unpublish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); @@ -2505,7 +2505,7 @@ namespace Umbraco.Core.Services.Implement // they should be removed so they don't interrupt an unpublish // otherwise it would remain released == published - var pastReleases = content.ContentSchedule.GetPending(ContentScheduleChange.End, DateTime.Now); + var pastReleases = content.ContentSchedule.GetPending(ContentScheduleAction.Expire, DateTime.Now); foreach (var p in pastReleases) content.ContentSchedule.Remove(p); if (pastReleases.Count > 0) diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index a5056ada7a..579e96b122 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -378,7 +378,7 @@ - + diff --git a/src/Umbraco.Tests/Models/ContentScheduleTests.cs b/src/Umbraco.Tests/Models/ContentScheduleTests.cs index 30be9d032c..5d03da5bae 100644 --- a/src/Umbraco.Tests/Models/ContentScheduleTests.cs +++ b/src/Umbraco.Tests/Models/ContentScheduleTests.cs @@ -71,10 +71,10 @@ namespace Umbraco.Tests.Models var schedule = new ContentScheduleCollection(); schedule.Add(now, now.AddDays(1)); - schedule.Clear(ContentScheduleChange.Start); + schedule.Clear(ContentScheduleAction.Release); - Assert.AreEqual(0, schedule.GetSchedule(ContentScheduleChange.Start).Count()); - Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleChange.End).Count()); + Assert.AreEqual(0, schedule.GetSchedule(ContentScheduleAction.Release).Count()); + Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleAction.Expire).Count()); Assert.AreEqual(1, schedule.FullSchedule.Count()); } @@ -86,20 +86,20 @@ namespace Umbraco.Tests.Models schedule.Add(now, now.AddDays(1)); schedule.Add("en-US", now, now.AddDays(1)); - schedule.Clear(ContentScheduleChange.End); + schedule.Clear(ContentScheduleAction.Expire); - Assert.AreEqual(0, schedule.GetSchedule(ContentScheduleChange.End).Count()); - Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleChange.Start).Count()); - Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleChange.End).Count()); - Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleChange.Start).Count()); + Assert.AreEqual(0, schedule.GetSchedule(ContentScheduleAction.Expire).Count()); + Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleAction.Release).Count()); + Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleAction.Expire).Count()); + Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleAction.Release).Count()); Assert.AreEqual(3, schedule.FullSchedule.Count()); - schedule.Clear("en-US", ContentScheduleChange.End); + schedule.Clear("en-US", ContentScheduleAction.Expire); - Assert.AreEqual(0, schedule.GetSchedule(ContentScheduleChange.End).Count()); - Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleChange.Start).Count()); - Assert.AreEqual(0, schedule.GetSchedule("en-US", ContentScheduleChange.End).Count()); - Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleChange.Start).Count()); + Assert.AreEqual(0, schedule.GetSchedule(ContentScheduleAction.Expire).Count()); + Assert.AreEqual(1, schedule.GetSchedule(ContentScheduleAction.Release).Count()); + Assert.AreEqual(0, schedule.GetSchedule("en-US", ContentScheduleAction.Expire).Count()); + Assert.AreEqual(1, schedule.GetSchedule("en-US", ContentScheduleAction.Release).Count()); Assert.AreEqual(2, schedule.FullSchedule.Count()); } diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index e03e7ca0d3..2c90ce90c5 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -357,7 +357,7 @@ namespace Umbraco.Tests.Services var sched = content.ContentSchedule.FullSchedule; Assert.AreEqual(1, sched.Count); Assert.AreEqual(1, sched.Count(x => x.Culture == string.Empty)); - content.ContentSchedule.Clear(ContentScheduleChange.End); + content.ContentSchedule.Clear(ContentScheduleAction.Expire); contentService.Save(content, Constants.Security.SuperUserId);