diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index 64099606d6..d635b9f939 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -51,23 +51,20 @@ namespace Umbraco.Core } /// - /// Method to return the cultures that have been flagged for unpublishing + /// Gets the cultures that have been flagged for unpublishing. /// - /// - /// + /// Gets cultures for which content.UnpublishCulture() has been invoked. internal static IReadOnlyList GetCulturesUnpublishing(this IContent content) { - if (!content.ContentType.VariesByCulture() && !content.IsPropertyDirty("PublishCultureInfos") && !content.Published) + // fixme/review - assuming it's || here not && ? + if (!content.Published || !content.ContentType.VariesByCulture() || !content.IsPropertyDirty("PublishCultureInfos")) return Array.Empty(); var culturesChanging = content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key); - - var result = new List(); - foreach (var culture in culturesChanging) - if (!content.IsCulturePublished(culture) && content.WasCulturePublished(culture)) - result.Add(culture); - - return result; + return culturesChanging + .Where(x => !content.IsCulturePublished(x) && // is not published anymore + content.WasCulturePublished(x)) // but was published before + .ToList(); } /// diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 5d020a8ffb..11ef3cf37f 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -20,8 +20,6 @@ namespace Umbraco.Core.Models private ContentScheduleCollection _schedule; private bool _published; private PublishedState _publishedState; - private DateTime? _releaseDate; - private DateTime? _expireDate; private ContentCultureInfosCollection _publishInfos; private ContentCultureInfosCollection _publishInfosOrig; private HashSet _editedCultures; @@ -102,7 +100,7 @@ namespace Umbraco.Core.Models _schedule = new ContentScheduleCollection(); _schedule.CollectionChanged += ScheduleCollectionChanged; } - return _schedule ?? (_schedule = new ContentScheduleCollection()); + return _schedule; } set { @@ -301,7 +299,8 @@ namespace Umbraco.Core.Models _publishInfos.Remove(culture); //we need to set the culture name to be dirty so we know it's being modified - //fixme is there a better way to do this, not as far as i know. + //fixme is there a better way to do this, not as far as i know. + // fixme why do we need this? SetCultureName(GetCultureName(culture), culture); if (_publishInfos.Count == 0) _publishInfos = null; diff --git a/src/Umbraco.Core/Models/ContentSchedule.cs b/src/Umbraco.Core/Models/ContentSchedule.cs index d2905ed54d..3eecce6321 100644 --- a/src/Umbraco.Core/Models/ContentSchedule.cs +++ b/src/Umbraco.Core/Models/ContentSchedule.cs @@ -3,14 +3,16 @@ using System.Runtime.Serialization; namespace Umbraco.Core.Models { - /// - /// Model for scheduled content + /// Represents a scheduled action for a document. /// [Serializable] [DataContract(IsReference = true)] public class ContentSchedule : IDeepCloneable { + /// + /// Initializes a new instance of the class. + /// public ContentSchedule(int id, string culture, DateTime date, ContentScheduleChange change) { Id = id; @@ -20,32 +22,46 @@ namespace Umbraco.Core.Models } /// - /// The unique Id of the schedule item + /// Gets the unique identifier of the document targeted by the scheduled action. /// [DataMember] public int Id { get; } /// - /// The culture for the schedule item + /// Gets the culture of the scheduled action. /// /// - /// string.Empty represents invariant culture + /// string.Empty represents the invariant culture. /// [DataMember] public string Culture { get; } /// - /// The date for the schedule + /// Gets the date of the scheduled action. /// [DataMember] public DateTime Date { get; } /// - /// The action to take for the schedule + /// Gets the action to take. /// [DataMember] public ContentScheduleChange Change { 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); + + public bool Equals(ContentSchedule other) + { + // 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; + } + public object DeepClone() { return new ContentSchedule(Id, Culture, Date, Change); diff --git a/src/Umbraco.Core/Models/ContentScheduleChange.cs b/src/Umbraco.Core/Models/ContentScheduleChange.cs index 9196d16b61..a89be62022 100644 --- a/src/Umbraco.Core/Models/ContentScheduleChange.cs +++ b/src/Umbraco.Core/Models/ContentScheduleChange.cs @@ -1,11 +1,21 @@ namespace Umbraco.Core.Models { + // fixme/review - should this be named DocumentScheduleAction? + // fixme/review - should values be Release and Expire not Start and Stop? + /// - /// The scheduled change types of scheduled content + /// Defines scheduled actions for documents. /// public enum ContentScheduleChange { + /// + /// Release the document. + /// Start, + + /// + /// Expire the document. + /// End } } diff --git a/src/Umbraco.Core/Models/ContentScheduleCollection.cs b/src/Umbraco.Core/Models/ContentScheduleCollection.cs index f1628f1645..c2d64817b1 100644 --- a/src/Umbraco.Core/Models/ContentScheduleCollection.cs +++ b/src/Umbraco.Core/Models/ContentScheduleCollection.cs @@ -67,7 +67,7 @@ namespace Umbraco.Core.Models { changes = new SortedList(); _schedule[culture] = changes; - } + } //TODO: Below will throw if there are duplicate dates added, should validate/return bool? // but the bool won't indicate which date was in error, maybe have 2 diff methods to schedule start/end? @@ -78,7 +78,7 @@ namespace Umbraco.Core.Models changes.Add(releaseDate.Value, entry); OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, entry)); } - + if (expireDate.HasValue) { var entry = new ContentSchedule(0, culture, expireDate.Value, ContentScheduleChange.End); @@ -102,7 +102,7 @@ namespace Umbraco.Core.Models if (s.Count == 0) _schedule.Remove(change.Culture); } - + } } @@ -135,9 +135,9 @@ namespace Umbraco.Core.Models OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, ofChange.Value)); if (s.Count == 0) _schedule.Remove(culture); - } + } } - + } } @@ -193,41 +193,27 @@ namespace Umbraco.Core.Models } public override bool Equals(object obj) - { - if (!(obj is ContentScheduleCollection c)) - return false; - return Equals(c); - } + => obj is ContentScheduleCollection other && Equals(other); public bool Equals(ContentScheduleCollection other) { - var thisSched = this._schedule; + if (other == null) return false; + + var thisSched = _schedule; var thatSched = other._schedule; - var equal = false; - if (thisSched.Count == thatSched.Count) + 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) { - equal = true; - foreach (var pair in thisSched) - { - if (thatSched.TryGetValue(pair.Key, out var val)) - { - if (val.SequenceEqual(pair.Value)) - { - equal = false; - break; - } - } - else - { - // Require key be present. - equal = false; - break; - } - } + // if culture is missing, or actions differ, false + if (!thatSched.TryGetValue(culture, out var thatList) || !thatList.SequenceEqual(thisList)) + return false; } - return equal; + + return true; } - } } diff --git a/src/Umbraco.Core/Models/ContentStatus.cs b/src/Umbraco.Core/Models/ContentStatus.cs index 4caf214399..1d35844874 100644 --- a/src/Umbraco.Core/Models/ContentStatus.cs +++ b/src/Umbraco.Core/Models/ContentStatus.cs @@ -4,20 +4,42 @@ using System.Runtime.Serialization; namespace Umbraco.Core.Models { /// - /// Enum for the various statuses a Content object can have + /// Describes the states of a document, with regard to (schedule) publishing. /// [Serializable] [DataContract] public enum ContentStatus { + // typical flow: + // Unpublished (add release date)-> AwaitingRelease (release)-> Published (expire)-> Expired + + /// + /// The document is not trashed, and not published. + /// [EnumMember] Unpublished, + + /// + /// The document is published. + /// [EnumMember] Published, + + /// + /// The document is not trashed, not published, after being unpublished by a scheduled action. + /// [EnumMember] Expired, + + /// + /// The document is trashed. + /// [EnumMember] Trashed, + + /// + /// The document is not trashed, not published, and pending publication by a scheduled action. + /// [EnumMember] AwaitingRelease } diff --git a/src/Umbraco.Core/Models/Language.cs b/src/Umbraco.Core/Models/Language.cs index 940648c4b9..7cb83c601d 100644 --- a/src/Umbraco.Core/Models/Language.cs +++ b/src/Umbraco.Core/Models/Language.cs @@ -1,7 +1,10 @@ using System; using System.Globalization; +using System.Linq; using System.Reflection; using System.Runtime.Serialization; +using System.Threading; +using Umbraco.Core.Configuration; using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Models @@ -48,7 +51,58 @@ namespace Umbraco.Core.Models [DataMember] public string CultureName { - get => _cultureName ?? CultureInfo.GetCultureInfo(IsoCode).DisplayName; + // CultureInfo.DisplayName is the name in the installed .NET language + // .NativeName is the name in culture info's language + // .EnglishName is the name in English + // + // there is no easy way to get the name in a specified culture (which would need to be installed on the server) + // this works: + // var rm = new ResourceManager("mscorlib", typeof(int).Assembly); + // var name = rm.GetString("Globalization.ci_" + culture.Name, displayCulture); + // but can we rely on it? + // + // and... DisplayName is captured and cached in culture infos returned by GetCultureInfo(), using + // the value for the current thread culture at the moment it is first retrieved - whereas creating + // a new CultureInfo() creates a new instance, which _then_ can get DisplayName again in a different + // culture + // + // I assume that, on a site, all language names should be in the SAME language, in DB, + // and that would be the umbracoDefaultUILanguage (app setting) - BUT if by accident + // ANY culture has been retrieved with another current thread culture - it's now corrupt + // + // 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? + // + // but then, some tests that compare audit strings (for culture names) would need to be fixed + + get + { + if (_cultureName != null) return _cultureName; + + // capture + var threadUiCulture = Thread.CurrentThread.CurrentUICulture; + + try + { + var globalSettings = (IGlobalSettings) Composing.Current.Container.GetInstance(typeof(IGlobalSettings)); + var defaultUiCulture = CultureInfo.GetCultureInfo(globalSettings.DefaultUILanguage); + Thread.CurrentThread.CurrentUICulture = defaultUiCulture; + + // get name - new-ing an instance to get proper display name + return new CultureInfo(IsoCode).DisplayName; + } + finally + { + // restore + Thread.CurrentThread.CurrentUICulture = threadUiCulture; + } + } + set => SetPropertyValueAndDetectChanges(value, ref _cultureName, Ps.Value.CultureNameSelector); } diff --git a/src/Umbraco.Core/Persistence/Dtos/DocumentDto.cs b/src/Umbraco.Core/Persistence/Dtos/DocumentDto.cs index 30e8f295fb..abe13a0e23 100644 --- a/src/Umbraco.Core/Persistence/Dtos/DocumentDto.cs +++ b/src/Umbraco.Core/Persistence/Dtos/DocumentDto.cs @@ -1,5 +1,4 @@ using NPoco; -using System.Collections.Generic; using Umbraco.Core.Persistence.DatabaseAnnotations; namespace Umbraco.Core.Persistence.Dtos @@ -55,6 +54,5 @@ namespace Umbraco.Core.Persistence.Dtos [ResultColumn] [Reference(ReferenceType.OneToOne)] public DocumentVersionDto PublishedVersionDto { get; set; } - } } diff --git a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs index e7f8cc0094..bb16e62e4e 100644 --- a/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/ContentBaseFactory.cs @@ -175,20 +175,15 @@ namespace Umbraco.Core.Persistence.Factories public static IEnumerable BuildScheduleDto(IContent entity, ILanguageRepository languageRepository) { - var schedule = new List(); - foreach(var s in entity.ContentSchedule.FullSchedule) - { - schedule.Add(new ContentScheduleDto + return entity.ContentSchedule.FullSchedule.Select(x => + new ContentScheduleDto { - Action = s.Change.ToString(), - Date = s.Date, + Action = x.Change.ToString(), + Date = x.Date, NodeId = entity.Id, - LanguageId = languageRepository.GetIdByIsoCode(s.Culture, false), - Id = s.Id + LanguageId = languageRepository.GetIdByIsoCode(x.Culture, false), + Id = x.Id }); - } - - return schedule; } /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 8ab5b43bae..76a5a51327 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -83,11 +83,16 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var translator = new SqlTranslator(sqlClause, query); var sql = translator.Translate(); + AddGetByQueryOrderBy(sql); + + return MapDtosToContent(Database.Fetch(sql)); + } + + private void AddGetByQueryOrderBy(Sql sql) + { sql // fixme why - this should be Path .OrderBy(x => x.Level) .OrderBy(x => x.SortOrder); - - return MapDtosToContent(Database.Fetch(sql)); } protected override Sql GetBaseQuery(QueryType queryType) @@ -321,7 +326,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var contentVersionDto = dto.DocumentVersionDto.ContentVersionDto; contentVersionDto.NodeId = nodeDto.NodeId; contentVersionDto.Current = !publishing; - contentVersionDto.Text = content.Name; //fixme is this requried? isn't this mapped in the ContentBaseFactory? Database.Insert(contentVersionDto); content.VersionId = contentVersionDto.Id; @@ -367,11 +371,11 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Insert(dto); //insert the schedule - var scheduleDto = ContentBaseFactory.BuildScheduleDto(content, LanguageRepository); - foreach (var c in scheduleDto) + var scheduleDtos = ContentBaseFactory.BuildScheduleDto(content, LanguageRepository); + foreach (var scheduleDto in scheduleDtos) { - c.NodeId = nodeDto.NodeId; - Database.Insert(c); + scheduleDto.NodeId = nodeDto.NodeId; + Database.Insert(scheduleDto); } // persist the variations @@ -502,7 +506,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement { documentVersionDto.Published = true; // now published contentVersionDto.Current = false; // no more current - contentVersionDto.Text = content.Name; } Database.Update(contentVersionDto); Database.Update(documentVersionDto); @@ -596,22 +599,22 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (content.IsPropertyDirty("ContentSchedule")) { //update the schedule, get the existing one so we know what to update - var dtoSchedule = ContentBaseFactory.BuildScheduleDto(content, LanguageRepository); - var existingSched = Database.Fetch(Sql() - .Select(x => x.Id) - .From() - .Where(x => x.NodeId == content.Id)); + var scheduleDtos = ContentBaseFactory.BuildScheduleDto(content, LanguageRepository).ToList(); + //remove any that no longer exist - var schedToRemove = existingSched.Except(dtoSchedule.Select(x => x.Id)); - foreach (var c in schedToRemove) - Database.DeleteWhere("id = @id", new { id = c }); + var ids = scheduleDtos.Where(x => x.Id > 0).Select(x => x.Id).Distinct(); + Database.Execute(Sql() + .Delete() + .Where(x => x.NodeId == content.Id) + .WhereNotIn(x => x.Id, ids)); + //add/update the rest - foreach (var c in dtoSchedule) + foreach (var scheduleDto in scheduleDtos) { - if (c.Id == 0) - Database.Insert(c); + if (scheduleDto.Id == 0) + Database.Insert(scheduleDto); else - Database.Update(c); + Database.Update(scheduleDto); } } @@ -907,47 +910,41 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// public IEnumerable GetContentForRelease(DateTime date) { - var sqlSchedule = Sql() - .Select(x => x.NodeId) - .From() - .Where(x => - x.Action == ContentScheduleChange.Start.ToString() - && x.Date <= date); + var action = ContentScheduleChange.Start.ToString(); - //fixme - If we don't cast to IEnumerable or do a ToArray, the Expression Visitor will FAIL! - // in the ExpressionVisitorBase.VisitMethodCall where the switch checks for "Contains" for some reason - // the 'special case' that redirects to `SqlIn` fails because the m.Arguments.Count is only ONE instead of TWO, - // no time to investigate right now. - // fixed here: https://github.com/umbraco/Umbraco-CMS/pull/3516/files so when that's merged we don't need the AsEnumerable() thing - var scheduledIds = Database.Fetch(sqlSchedule); - if (scheduledIds.Count == 0) return Enumerable.Empty(); - var e = scheduledIds.AsEnumerable(); + // fixme/review - code would blow if more than 2000 items + // fixme/review - isn't this simpler? + var sql = GetBaseQuery(QueryType.Many) + .WhereIn(x => x.NodeId, Sql() + .Select(x => x.NodeId) + .From() + .Where(x => x.Action == action && x.Date <= date)); - var query = Query().Where(x => x.Published == false && e.Contains(x.Id)); - return Get(query); + sql.Where(x => !x.Trashed); // fixme/review - shouldn't we exclude trashed nodes? + sql.Where(x => !x.Published); + + AddGetByQueryOrderBy(sql); + + return MapDtosToContent(Database.Fetch(sql)); } /// public IEnumerable GetContentForExpiration(DateTime date) { - var sqlSchedule = Sql() - .Select(x => x.NodeId) - .From() - .Where(x => - x.Action == ContentScheduleChange.End.ToString() - && x.Date <= date); + var action = ContentScheduleChange.End.ToString(); - //fixme - If we don't cast to IEnumerable or do a ToArray, the Expression Visitor will FAIL! - // in the ExpressionVisitorBase.VisitMethodCall where the switch checks for "Contains" for some reason - // the 'special case' that redirects to `SqlIn` fails because the m.Arguments.Count is only ONE instead of TWO, - // no time to investigate right now. - // fixed here: https://github.com/umbraco/Umbraco-CMS/pull/3516/files so when that's merged we don't need the AsEnumerable() thing - var scheduledIds = Database.Fetch(sqlSchedule); - if (scheduledIds.Count == 0) return Enumerable.Empty(); - var e = scheduledIds.AsEnumerable(); + // fixme/review - see as above + var sql = GetBaseQuery(QueryType.Many) + .WhereIn(x => x.NodeId, Sql() + .Select(x => x.NodeId) + .From() + .Where(x => x.Action == action && x.Date <= date)); - var query = Query().Where(x => x.Published && e.Contains(x.Id)); - return Get(query); + sql.Where(x => x.Published); + + AddGetByQueryOrderBy(sql); + + return MapDtosToContent(Database.Fetch(sql)); } #endregion @@ -1160,23 +1157,27 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private IDictionary GetContentSchedule(params int[] contentIds) { var result = new Dictionary(); - foreach(var ids in contentIds.InGroupsOf(2000)) - { - var sql = Sql().Select().From().Where(x => ids.Contains(x.NodeId)); - var scheduleDto = Database.Fetch(sql); - foreach (var dto in scheduleDto) - { - var schedule = new ContentScheduleCollection(); - schedule.Add(new ContentSchedule(dto.Id, - LanguageRepository.GetIsoCodeById(dto.LanguageId) ?? string.Empty, - dto.Date, - dto.Action == ContentScheduleChange.Start.ToString() - ? ContentScheduleChange.Start - : ContentScheduleChange.End)); - result[dto.NodeId] = schedule; - } + var scheduleDtos = Database.FetchByGroups(contentIds, 2000, batch => Sql() + .Select() + .From() + .WhereIn(x => x.NodeId, batch)); + + 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)); } + return result; } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs index af9029244c..09fb664ffe 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs @@ -236,7 +236,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // fast way of getting an id for an isoCode - avoiding cloning // _codeIdMap is rebuilt whenever PerformGetAll runs - public int? GetIdByIsoCode(string isoCode, bool throwOnNotFound) + public int? GetIdByIsoCode(string isoCode, bool throwOnNotFound = true) { if (isoCode == null) return null; diff --git a/src/Umbraco.Core/Publishing/ScheduledPublisher.cs b/src/Umbraco.Core/Publishing/ScheduledPublisher.cs deleted file mode 100644 index 2eb3e39ed1..0000000000 --- a/src/Umbraco.Core/Publishing/ScheduledPublisher.cs +++ /dev/null @@ -1,39 +0,0 @@ -using System; -using System.Linq; -using Umbraco.Core.Logging; -using Umbraco.Core.Models; -using Umbraco.Core.Services; -using Umbraco.Core.Services.Implement; - -namespace Umbraco.Core.Publishing -{ - /// - /// Used to perform scheduled publishing/unpublishing - /// - internal class ScheduledPublisher - { - private readonly IContentService _contentService; - private readonly ILogger _logger; - private readonly IUserService _userService; - - public ScheduledPublisher(IContentService contentService, ILogger logger, IUserService userService) - { - _contentService = contentService; - _logger = logger; - _userService = userService; - } - - /// - /// Processes scheduled operations - /// - /// - /// Returns the number of items successfully completed - /// - public int CheckPendingAndProcess() - { - var results = _contentService.PerformScheduledPublish(DateTime.Now); - return results.Count(x => x.Success); - - } - } -} diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index d0d6ce9039..2b04757450 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -132,7 +132,7 @@ namespace Umbraco.Core.Services IEnumerable GetRootContent(); /// - /// Gets documents with an expiration date greater then today. + /// Gets documents having an expiration date before (lower than, or equal to) a specified date. /// /// An Enumerable list of objects /// @@ -142,7 +142,7 @@ namespace Umbraco.Core.Services IEnumerable GetContentForExpiration(DateTime date); /// - /// Gets documents with a release date greater then today. + /// Gets documents having a release date before (lower than, or equal to) a specified date. /// /// An Enumerable list of objects /// @@ -362,23 +362,7 @@ namespace Umbraco.Core.Services /// PublishResult SavePublishing(IContent content, int userId = 0, bool raiseEvents = true); - /* - fixme - document this better + test - If the item being published is Invariant and it has Variant descendants and - we are NOT forcing publishing of anything not published - the result will be that the Variant cultures that are - already published (but may contain a draft) are published. Any cultures that don't have a published version are not published - fixme: now, if publishing '*' then all cultures - If the item being published is Invariant and it has Variant descendants and - we ARE forcing publishing of anything not published - the result will be that all Variant cultures are - published regardless of whether they don't have any current published versions - - If the item being published is Variant and it has Invariant descendants and - we are NOT forcing publishing of anything not published - the result will be that all Invariant documents are - published that already have a published versions, regardless of what cultures are selected to be published - If the item being published is Variant and it has Invariant descendants and - we ARE forcing publishing of anything not published - the result will be that all Invariant documents are - published regardless of whether they have a published version or not and regardless of what cultures are selected to be published - */ + // fixme/review - should SaveAndPublishBranch always publish the root document of the branch, even when !force? /// /// Saves and publishes a document branch. @@ -392,7 +376,7 @@ namespace Umbraco.Core.Services /// than one culture, see the other overloads of this method. /// The parameter determines which documents are published. When false, /// only those documents that are already published, are republished. When true, all documents are - /// published. + /// published. The root of the branch is always published, regardless of . /// IEnumerable SaveAndPublishBranch(IContent content, bool force, string culture = "*", int userId = 0); @@ -406,7 +390,7 @@ namespace Umbraco.Core.Services /// /// The parameter determines which documents are published. When false, /// only those documents that are already published, are republished. When true, all documents are - /// published. + /// published. The root of the branch is always published, regardless of . /// IEnumerable SaveAndPublishBranch(IContent content, bool force, string[] cultures, int userId = 0); @@ -421,7 +405,7 @@ namespace Umbraco.Core.Services /// /// The parameter determines which documents are published. When false, /// only those documents that are already published, are republished. When true, all documents are - /// published. + /// published. The root of the branch is always published, regardless of . /// The parameter is a function which determines whether a document has /// changes to publish (else there is no need to publish it). If one wants to publish only a selection of /// cultures, one may want to check that only properties for these cultures have changed. Otherwise, other diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index c1616308b3..e7668e61aa 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -2,8 +2,6 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; -using Umbraco.Core; -using Umbraco.Core.Composing; using Umbraco.Core.Events; using Umbraco.Core.Exceptions; using Umbraco.Core.IO; @@ -942,9 +940,7 @@ namespace Umbraco.Core.Services.Implement } // finally, "save publishing" - // what happens next depends on whether the content can be published or not - var saved = SavePublishing(content, userId); - return saved; + return SavePublishing(content, userId); } /// @@ -970,6 +966,7 @@ namespace Umbraco.Core.Services.Implement ((Content)content).PublishedState = PublishedState.Publishing; // state here is either Publishing or Unpublishing + // (even though, Publishing to unpublish a culture may end up unpublishing everything) var publishing = content.PublishedState == PublishedState.Publishing; var unpublishing = content.PublishedState == PublishedState.Unpublishing; @@ -1014,11 +1011,11 @@ namespace Umbraco.Core.Services.Implement 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 anways + // keep going, though, as we want to save anyways } //fixme - casting - ((Content)content).Published = content.Published; // reset published state = save unchanged + ((Content)content).Published = content.Published; // reset published state = save unchanged - fixme doh? } } @@ -1026,7 +1023,7 @@ namespace Umbraco.Core.Services.Implement { var newest = GetById(content.Id); // ensure we have the newest version - in scope if (content.VersionId != newest.VersionId) // but use the original object if it's already the newest version - content = newest; + content = newest; // fixme confusing should just die here - else we'll miss some changes if (content.Published) { @@ -1269,18 +1266,20 @@ namespace Umbraco.Core.Services.Implement // should not trigger a publish). HashSet ShouldPublish(IContent c) { + var isRoot = c.Id == content.Id; + if (c.ContentType.VariesByCulture()) { // variant content type // add culture if edited, and already published or forced - if (c.IsCultureEdited(culture) && (c.IsCulturePublished(culture) || force)) + if (c.IsCultureEdited(culture) && (c.IsCulturePublished(culture) || force || isRoot)) return new HashSet { culture.ToLowerInvariant() }; } else { // invariant content type // add "*" if edited, and already published or forced - if (c.Edited && (c.Published || force)) + if (c.Edited && (c.Published || force || isRoot)) return new HashSet { "*" }; } @@ -1314,6 +1313,7 @@ namespace Umbraco.Core.Services.Implement HashSet ShouldPublish(IContent c) { var culturesToPublish = new HashSet(); + var isRoot = c.Id == content.Id; if (c.ContentType.VariesByCulture()) { @@ -1321,7 +1321,7 @@ namespace Umbraco.Core.Services.Implement // add cultures which are edited, and already published or forced foreach (var culture in cultures) { - if (c.IsCultureEdited(culture) && (c.IsCulturePublished(culture) || force)) + if (c.IsCultureEdited(culture) && (c.IsCulturePublished(culture) || force || isRoot)) culturesToPublish.Add(culture.ToLowerInvariant()); } } @@ -1329,7 +1329,7 @@ namespace Umbraco.Core.Services.Implement { // invariant content type // add "*" if edited, and already published or forced - if (c.Edited && (c.Published || force)) + if (c.Edited && (c.Published || force || isRoot)) culturesToPublish.Add("*"); } @@ -1425,7 +1425,7 @@ namespace Umbraco.Core.Services.Implement // publishValues: a function publishing values (using the appropriate PublishCulture calls) private PublishResult SaveAndPublishBranchOne(IScope scope, IContent document, Func> shouldPublish, Func, bool> publishCultures, - bool checkPath, + bool isRoot, ICollection publishedDocuments, EventMessages evtMsgs, int userId) { @@ -1442,8 +1442,28 @@ namespace Umbraco.Core.Services.Implement if (publishCultures != null && !publishCultures(document, culturesToPublish)) return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, document); - return SavePublishingInternal(scope, document, userId); + // fixme - this is totally kinda wrong + var culturesPublishing = document.ContentType.VariesByCulture() + ? document.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList() + : null; + var result = StrategyCanPublish(scope, document, userId, /*checkPath:*/ isRoot, culturesPublishing, Array.Empty(), evtMsgs); + if (!result.Success) + return result; + result = StrategyPublish(scope, document, userId, culturesPublishing, Array.Empty(), evtMsgs); + if (!result.Success) + throw new Exception("panic"); + if (document.HasIdentity == false) + document.CreatorId = userId; + document.WriterId = userId; + _documentRepository.Save(document); + publishedDocuments.Add(document); + // fixme - but then, we have all the audit thing to run + // so... it would be better to re-run the internal thing? + return result; + + // we want _some part_ of it but not all of it + return SavePublishingInternal(scope, document, userId); } #endregion @@ -1969,7 +1989,7 @@ namespace Umbraco.Core.Services.Implement Audit(AuditType.SendToPublish, content.WriterId, content.Id); } - // fixme here, on only on success? + // fixme here, on only on success? scope.Complete(); return saveResult.Success; @@ -2312,7 +2332,7 @@ namespace Umbraco.Core.Services.Implement } var variesByCulture = content.ContentType.VariesByCulture(); - + //First check if mandatory languages fails, if this fails it will mean anything that the published flag on the document will // be changed to Unpublished and any culture currently published will not be visible. if (variesByCulture) @@ -2322,7 +2342,7 @@ namespace Umbraco.Core.Services.Implement // missing mandatory culture = cannot be published var mandatoryCultures = _languageRepository.GetMany().Where(x => x.IsMandatory).Select(x => x.IsoCode); - var mandatoryMissing = mandatoryCultures.Any(x => !content.PublishedCultures.Contains(x, StringComparer.OrdinalIgnoreCase)); + var mandatoryMissing = mandatoryCultures.Any(x => !content.PublishedCultures.Contains(x, StringComparer.OrdinalIgnoreCase)); if (mandatoryMissing) return new PublishResult(PublishResultType.FailedPublishMandatoryCultureMissing, evtMsgs, content); @@ -2454,7 +2474,7 @@ namespace Umbraco.Core.Services.Implement } /// - /// Unpublishes a document + /// Unpublishes a document /// /// /// diff --git a/src/Umbraco.Core/Services/PublishResultType.cs b/src/Umbraco.Core/Services/PublishResultType.cs index d97bf014c6..7462855754 100644 --- a/src/Umbraco.Core/Services/PublishResultType.cs +++ b/src/Umbraco.Core/Services/PublishResultType.cs @@ -1,8 +1,7 @@ namespace Umbraco.Core.Services { - /// - /// A value indicating the result of publishing a content item. + /// A value indicating the result of publishing or unpublishing a document. /// public enum PublishResultType : byte { @@ -10,15 +9,19 @@ // every failure codes as >128 - see OperationResult and OperationResultType for details. #region Success - Publish + /// - /// The publishing was successful. + /// The document was successfully published. /// SuccessPublish = 0, + /// + /// The specified document culture was successfully published. + /// SuccessPublishCulture = 1, /// - /// The item was already published. + /// The document was already published. /// SuccessPublishAlready = 2, @@ -27,22 +30,22 @@ #region Success - Unpublish /// - /// The unpublishing was successful. + /// The document was successfully unpublished. /// SuccessUnpublish = 3, /// - /// The item was already unpublished. + /// The document was already unpublished. /// SuccessUnpublishAlready = 4, /// - /// The specified variant was unpublished, the content item itself remains published. + /// The specified document culture was unpublished, the document item itself remains published. /// SuccessUnpublishCulture = 5, /// - /// The specified variant was a mandatory culture therefore it was unpublished and the content item itself is unpublished + /// The specified document culture was unpublished, and was a mandatory culture, therefore the document itself was unpublished. /// SuccessUnpublishMandatoryCulture = 6, @@ -51,13 +54,12 @@ #region Success - Mixed /// - /// A variant content item has a culture published and another culture unpublished in the same operation + /// Specified document cultures were successfully published and unpublished (in the same operation). /// SuccessMixedCulture = 7, #endregion - #region Failed - Publish /// @@ -67,36 +69,36 @@ FailedPublish = 128, /// - /// The content could not be published because it's ancestor path isn't published. + /// The document could not be published because its ancestor path is not published. /// FailedPublishPathNotPublished = FailedPublish | 1, /// - /// The content item was scheduled to be un-published and it has expired so we cannot force it to be + /// The document has expired so we cannot force it to be /// published again as part of a bulk publish operation. /// FailedPublishHasExpired = FailedPublish | 2, /// - /// The content item is scheduled to be released in the future and therefore we cannot force it to + /// The document is scheduled to be released in the future and therefore we cannot force it to /// be published during a bulk publish operation. /// FailedPublishAwaitingRelease = FailedPublish | 3, /// - /// A culture on the content item was scheduled to be un-published and it has expired so we cannot force it to be + /// A document culture has expired so we cannot force it to be /// published again as part of a bulk publish operation. /// FailedPublishCultureHasExpired = FailedPublish | 4, /// - /// A culture on the content item is scheduled to be released in the future and therefore we cannot force it to + /// A document culture is scheduled to be released in the future and therefore we cannot force it to /// be published during a bulk publish operation. /// FailedPublishCultureAwaitingRelease = FailedPublish | 5, /// - /// The content item could not be published because it is in the trash. + /// The document could not be published because it is in the trash. /// FailedPublishIsTrashed = FailedPublish | 6, @@ -106,17 +108,17 @@ FailedPublishCancelledByEvent = FailedPublish | 7, /// - /// The content item could not be published because it contains invalid data (has not passed validation requirements). + /// The document could not be published because it contains invalid data (has not passed validation requirements). /// FailedPublishContentInvalid = FailedPublish | 8, /// - /// Cannot publish a document that has no publishing flags or values + /// The document cannot be published because it has no publishing flags or values. /// FailedPublishNothingToPublish = FailedPublish | 9, // in ContentService.StrategyCanPublish - fixme weird /// - /// Some mandatory cultures are missing. + /// The document could not be published because some mandatory cultures are missing. /// FailedPublishMandatoryCultureMissing = FailedPublish | 10, // in ContentService.SavePublishing @@ -125,7 +127,7 @@ #region Failed - Unpublish /// - /// Unpublish failed + /// The document could not be unpublished. /// FailedUnpublish = FailedPublish | 11, // in ContentService.SavePublishing @@ -135,6 +137,5 @@ FailedUnpublishCancelledByEvent = FailedPublish | 12, #endregion - } } diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index b2ea58c90d..a5056ada7a 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -1268,7 +1268,6 @@ - diff --git a/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs b/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs index 4fb1703b29..d8e37cc8db 100644 --- a/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs +++ b/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs @@ -37,13 +37,13 @@ namespace Umbraco.Tests.Services // ii2 !published !edited // !force = publishes those that are actually published, and have changes - // here: nothing + // here: root (root is always published) var r = SaveAndPublishInvariantBranch(iRoot, false, method).ToArray(); AssertPublishResults(r, x => x.Content.Name, "iroot", "ii1", "ii2"); AssertPublishResults(r, x => x.Result, - PublishResultType.SuccessPublishAlready, + PublishResultType.SuccessPublish, PublishResultType.SuccessPublishAlready, PublishResultType.SuccessPublishAlready); @@ -220,8 +220,8 @@ namespace Umbraco.Tests.Services AssertPublishResults(r, x => x.Content.Name, "vroot.de", "iv1.de", "iv2.de"); AssertPublishResults(r, x => x.Result, - PublishResultType.SuccessPublish, - PublishResultType.SuccessPublish, + PublishResultType.SuccessPublishCulture, + PublishResultType.SuccessPublishCulture, PublishResultType.SuccessPublishAlready); // reload - SaveAndPublishBranch has modified other instances @@ -291,7 +291,7 @@ namespace Umbraco.Tests.Services AssertPublishResults(r, x => x.Result, PublishResultType.SuccessPublishAlready, PublishResultType.SuccessPublish, - PublishResultType.SuccessPublish); + PublishResultType.SuccessPublishCulture); // reload - SaveAndPublishBranch has modified other instances Reload(ref ii1); diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 305ef68f28..e03e7ca0d3 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -351,10 +351,11 @@ namespace Umbraco.Tests.Services content.ContentSchedule.Add(null, DateTime.Now.AddHours(2)); contentService.Save(content, Constants.Security.SuperUserId); + Assert.AreEqual(1, content.ContentSchedule.FullSchedule.Count); content = contentService.GetById(content.Id); var sched = content.ContentSchedule.FullSchedule; - Assert.AreEqual(1, sched.Count()); + Assert.AreEqual(1, sched.Count); Assert.AreEqual(1, sched.Count(x => x.Culture == string.Empty)); content.ContentSchedule.Clear(ContentScheduleChange.End); contentService.Save(content, Constants.Security.SuperUserId); @@ -363,7 +364,7 @@ namespace Umbraco.Tests.Services // Assert content = contentService.GetById(content.Id); sched = content.ContentSchedule.FullSchedule; - Assert.AreEqual(0, sched.Count()); + Assert.AreEqual(0, sched.Count); Assert.IsTrue(contentService.SaveAndPublish(content).Success); } diff --git a/src/Umbraco.Web/Editors/LanguageController.cs b/src/Umbraco.Web/Editors/LanguageController.cs index a3e613670b..8b6d8ec6ac 100644 --- a/src/Umbraco.Web/Editors/LanguageController.cs +++ b/src/Umbraco.Web/Editors/LanguageController.cs @@ -3,6 +3,7 @@ using System.Globalization; using System.Linq; using System.Net; using System.Net.Http; +using System.Threading; using System.Web.Http; using AutoMapper; using Umbraco.Core; @@ -28,10 +29,14 @@ namespace Umbraco.Web.Editors [HttpGet] public IDictionary GetAllCultures() { - return - CultureInfo.GetCultures(CultureTypes.AllCultures) - .Where(x => !x.Name.IsNullOrWhiteSpace()) - .OrderBy(x => x.DisplayName).ToDictionary(x => x.Name, x => x.DisplayName); + // get cultures - new-ing instances to get proper display name, + // in the current culture, and not the cached one + // (see notes in Language class about culture info names) + return CultureInfo.GetCultures(CultureTypes.AllCultures) + .Where(x => !x.Name.IsNullOrWhiteSpace()) + .Select(x => new CultureInfo(x.Name)) // important! + .OrderBy(x => x.DisplayName) + .ToDictionary(x => x.Name, x => x.DisplayName); } /// diff --git a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs index 471eb213c0..db9299af37 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs @@ -1,7 +1,6 @@ using System; using Umbraco.Core; using Umbraco.Core.Logging; -using Umbraco.Core.Publishing; using Umbraco.Core.Services; using Umbraco.Core.Sync; @@ -12,16 +11,14 @@ namespace Umbraco.Web.Scheduling private readonly IRuntimeState _runtime; private readonly IContentService _contentService; private readonly ILogger _logger; - private readonly IUserService _userService; public ScheduledPublishing(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, - IRuntimeState runtime, IContentService contentService, ILogger logger, IUserService userService) + IRuntimeState runtime, IContentService contentService, ILogger logger) : base(runner, delayMilliseconds, periodMilliseconds) { _runtime = runtime; _contentService = contentService; _logger = logger; - _userService = userService; } public override bool PerformRun() @@ -58,9 +55,8 @@ namespace Umbraco.Web.Scheduling // run // fixme context & events during scheduled publishing? // in v7 we create an UmbracoContext and an HttpContext, and cache instructions - // are batched, and we have to explicitely flush them, how is it going to work here? - var publisher = new ScheduledPublisher(_contentService, _logger, _userService); - var count = publisher.CheckPendingAndProcess(); + // are batched, and we have to explicitly flush them, how is it going to work here? + _contentService.PerformScheduledPublish(DateTime.Now); } catch (Exception ex) { diff --git a/src/Umbraco.Web/Scheduling/SchedulerComponent.cs b/src/Umbraco.Web/Scheduling/SchedulerComponent.cs index d9b2893c62..d14a7bb34f 100644 --- a/src/Umbraco.Web/Scheduling/SchedulerComponent.cs +++ b/src/Umbraco.Web/Scheduling/SchedulerComponent.cs @@ -26,7 +26,6 @@ namespace Umbraco.Web.Scheduling { private IRuntimeState _runtime; private IContentService _contentService; - private IUserService _userService; private IAuditService _auditService; private ILogger _logger; private ProfilingLogger _proflog; @@ -45,13 +44,12 @@ namespace Umbraco.Web.Scheduling private IBackgroundTask[] _tasks; public void Initialize(IRuntimeState runtime, - IContentService contentService, IAuditService auditService, IUserService userService, + IContentService contentService, IAuditService auditService, HealthCheckCollection healthChecks, HealthCheckNotificationMethodCollection notifications, IScopeProvider scopeProvider, ILogger logger, ProfilingLogger proflog) { _runtime = runtime; _contentService = contentService; - _userService = userService; _auditService = auditService; _scopeProvider = scopeProvider; _logger = logger; @@ -118,7 +116,7 @@ namespace Umbraco.Web.Scheduling { // scheduled publishing/unpublishing // install on all, will only run on non-replica servers - var task = new ScheduledPublishing(_publishingRunner, 60000, 60000, _runtime, _contentService, _logger, _userService); + var task = new ScheduledPublishing(_publishingRunner, 60000, 60000, _runtime, _contentService, _logger); _publishingRunner.TryAdd(task); return task; } diff --git a/src/umbraco.sln.DotSettings b/src/umbraco.sln.DotSettings index 3c08188b8a..d6826e5f90 100644 --- a/src/umbraco.sln.DotSettings +++ b/src/umbraco.sln.DotSettings @@ -1,8 +1,9 @@ - + <data><IncludeFilters /><ExcludeFilters /></data> <data /> Disposable construction HINT False - CSharp71 - \ No newline at end of file + CSharp71 + True + True