From a61a2b4d977eceb817d0bf369b0cb8bda5362375 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 20 Feb 2019 20:37:10 +1100 Subject: [PATCH 1/9] Removes notes (cherry picked from commit 717efb6b094536782000fa7980e710a34f3666c6) --- .../Services/Implement/PackagingService.cs | 12 ------------ src/Umbraco.Web/Mvc/PluginController.cs | 9 +-------- src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs | 6 ------ 3 files changed, 1 insertion(+), 26 deletions(-) diff --git a/src/Umbraco.Core/Services/Implement/PackagingService.cs b/src/Umbraco.Core/Services/Implement/PackagingService.cs index 5194a26eb5..b4dcc70a96 100644 --- a/src/Umbraco.Core/Services/Implement/PackagingService.cs +++ b/src/Umbraco.Core/Services/Implement/PackagingService.cs @@ -3,26 +3,14 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Net.Http; -using System.Text.RegularExpressions; using System.Threading.Tasks; -using System.Web; -using System.Xml.Linq; using Semver; -using Umbraco.Core.Collections; using Umbraco.Core.Events; using Umbraco.Core.Exceptions; using Umbraco.Core.IO; -using Umbraco.Core.Logging; using Umbraco.Core.Models; -using Umbraco.Core.Models.Entities; using Umbraco.Core.Models.Packaging; using Umbraco.Core.Packaging; -using Umbraco.Core.Persistence.Querying; -using Umbraco.Core.Persistence.Repositories; -using Umbraco.Core.PropertyEditors; -using Umbraco.Core.Scoping; -using Umbraco.Core.Strings; -using Content = Umbraco.Core.Models.Content; namespace Umbraco.Core.Services.Implement { diff --git a/src/Umbraco.Web/Mvc/PluginController.cs b/src/Umbraco.Web/Mvc/PluginController.cs index 43c8bb5479..2336bb56d1 100644 --- a/src/Umbraco.Web/Mvc/PluginController.cs +++ b/src/Umbraco.Web/Mvc/PluginController.cs @@ -22,14 +22,7 @@ namespace Umbraco.Web.Mvc // for debugging purposes internal Guid InstanceId { get; } = Guid.NewGuid(); - - // note - // properties marked as [Inject] below will be property-injected (vs constructor-injected) in - // order to keep the constructor as light as possible, so that ppl implementing eg a SurfaceController - // don't need to implement complex constructors + need to refactor them each time we change ours. - // this means that these properties have a setter. - // what can go wrong? - + /// /// Gets the Umbraco context. /// diff --git a/src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs b/src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs index 68359252d4..5ac4037fdb 100644 --- a/src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs +++ b/src/Umbraco.Web/Mvc/UmbracoViewPageOfTModel.cs @@ -26,12 +26,6 @@ namespace Umbraco.Web.Mvc private UmbracoContext _umbracoContext; private UmbracoHelper _helper; - // note - // properties marked as [Inject] below will be property-injected (vs constructor-injected) since - // we have no control over the view constructor (generated by eg the Razor engine). - // this means that these properties have a setter. - // what can go wrong? - /// /// Gets or sets the database context. /// From d9d54f79822f0c902ae7016c0460a325a964459f Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 20 Feb 2019 12:56:14 +0100 Subject: [PATCH 2/9] Merge branch 'temp8-fixes-published-content-children-method' into dev-v8 (cherry picked from commit 735d7592089838d546717f1191536412109dc895) --- src/Umbraco.Web/PublishedContentExtensions.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Web/PublishedContentExtensions.cs b/src/Umbraco.Web/PublishedContentExtensions.cs index c77594eabe..f880076c50 100644 --- a/src/Umbraco.Web/PublishedContentExtensions.cs +++ b/src/Umbraco.Web/PublishedContentExtensions.cs @@ -979,11 +979,11 @@ namespace Umbraco.Web /// /// The content. /// The specific culture to filter for. If null is used the current culture is used. (Default is null) - /// One or more content type alias. + /// The content type alias. /// The children of the content, of any of the specified types. - public static IEnumerable Children(this IPublishedContent content, string culture = null, params string[] alias) + public static IEnumerable ChildrenOfType(this IPublishedContent content, string contentTypeAlias, string culture = null) { - return content.Children(x => alias.InvariantContains(x.ContentType.Alias), culture); + return content.Children(x => contentTypeAlias.InvariantContains(x.ContentType.Alias), culture); } /// @@ -1010,9 +1010,9 @@ namespace Umbraco.Web /// /// Gets the first child of the content, of a given content type. /// - public static IPublishedContent FirstChildOfType(this IPublishedContent content, string alias, string culture = null) + public static IPublishedContent FirstChildOfType(this IPublishedContent content, string contentTypeAlias, string culture = null) { - return content.Children(culture,alias).FirstOrDefault(); + return content.ChildrenOfType(contentTypeAlias, culture).FirstOrDefault(); } public static IPublishedContent FirstChild(this IPublishedContent content, Func predicate, string culture = null) From d9d68cf6bf46198a87e4d365372bd65acbdde46c Mon Sep 17 00:00:00 2001 From: Claus Date: Wed, 20 Feb 2019 16:05:42 +0100 Subject: [PATCH 3/9] The content service is not returning the invalid properties when publishing fails (cherry picked from commit d94e383ed7778c72e11cbb303d1cce4798f780c7) --- .../Models/ContentRepositoryExtensions.cs | 10 +++- .../Services/ContentServiceExtensions.cs | 6 +-- src/Umbraco.Core/Services/IContentService.cs | 48 +++++++++---------- .../Services/Implement/ContentService.cs | 31 +++++++++--- 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs index 27678c047c..01812f8469 100644 --- a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs +++ b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs @@ -193,6 +193,13 @@ namespace Umbraco.Core.Models public static bool PublishCulture(this IContent content, string culture = "*") { + return PublishCulture(content, out _, culture); + } + + public static bool PublishCulture(this IContent content, out Property[] invalidProperties, string culture = "*") + { + invalidProperties = null; + culture = culture.NullOrWhiteSpaceAsNull(); // the variation should be supported by the content type properties @@ -202,7 +209,8 @@ namespace Umbraco.Core.Models throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{content.ContentType.Alias}\" with variation \"{content.ContentType.Variations}\"."); // the values we want to publish should be valid - if (content.ValidateProperties(culture).Any()) + invalidProperties = content.ValidateProperties(culture); + if (invalidProperties.Length > 0) return false; var alsoInvariant = false; diff --git a/src/Umbraco.Core/Services/ContentServiceExtensions.cs b/src/Umbraco.Core/Services/ContentServiceExtensions.cs index 4d673fc902..1175df81dc 100644 --- a/src/Umbraco.Core/Services/ContentServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentServiceExtensions.cs @@ -31,16 +31,16 @@ namespace Umbraco.Core.Services /// /// /// - /// + /// /// /// - public static IContent CreateContent(this IContentService contentService, string name, Udi parentId, string mediaTypeAlias, int userId = Constants.Security.SuperUserId) + public static IContent CreateContent(this IContentService contentService, string name, Udi parentId, string contentTypeAlias, int userId = Constants.Security.SuperUserId) { var guidUdi = parentId as GuidUdi; if (guidUdi == null) throw new InvalidOperationException("The UDI provided isn't of type " + typeof(GuidUdi) + " which is required by content"); var parent = contentService.GetById(guidUdi.Guid); - return contentService.Create(name, parent, mediaTypeAlias, userId); + return contentService.Create(name, parent, contentTypeAlias, userId); } /// diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index b2fce8f3e6..784d04864e 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -391,30 +391,30 @@ namespace Umbraco.Core.Services /// IEnumerable SaveAndPublishBranch(IContent content, bool force, string[] cultures, int userId = Constants.Security.SuperUserId); - /// - /// Saves and publishes a document branch. - /// - /// The root document. - /// A value indicating whether to force-publish documents that are not already published. - /// A function determining cultures to publish. - /// A function publishing cultures. - /// The identifier of the user performing the operation. - /// - /// The parameter determines which documents are published. When false, - /// only those documents that are already published, are republished. When true, all documents are - /// 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 - /// cultures may trigger an unwanted republish. - /// The parameter is a function to execute to publish cultures, on - /// each document. It can publish all, one, or a selection of cultures. It returns a boolean indicating - /// whether the cultures could be published. - /// - IEnumerable SaveAndPublishBranch(IContent content, bool force, - Func> shouldPublish, - Func, bool> publishCultures, - int userId = Constants.Security.SuperUserId); + ///// + ///// Saves and publishes a document branch. + ///// + ///// The root document. + ///// A value indicating whether to force-publish documents that are not already published. + ///// A function determining cultures to publish. + ///// A function publishing cultures. + ///// The identifier of the user performing the operation. + ///// + ///// The parameter determines which documents are published. When false, + ///// only those documents that are already published, are republished. When true, all documents are + ///// 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 + ///// cultures may trigger an unwanted republish. + ///// The parameter is a function to execute to publish cultures, on + ///// each document. It can publish all, one, or a selection of cultures. It returns a boolean indicating + ///// whether the cultures could be published. + ///// + //IEnumerable SaveAndPublishBranch(IContent content, bool force, + // Func> shouldPublish, + // Func, bool> publishCultures, + // int userId = Constants.Security.SuperUserId); /// /// Unpublishes a document. diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 02117d064c..922eb4e2db 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -869,19 +869,28 @@ namespace Umbraco.Core.Services.Implement // if culture is specific, first publish the invariant values, then publish the culture itself. // if culture is '*', then publish them all (including variants) + Property[] invalidProperties; + // explicitly SaveAndPublish a specific culture also publishes invariant values if (!culture.IsNullOrWhiteSpace() && culture != "*") { // publish the invariant values - var publishInvariant = content.PublishCulture(null); + var publishInvariant = content.PublishCulture(out invalidProperties, null); if (!publishInvariant) - return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content); + return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content) + { + InvalidProperties = invalidProperties ?? Enumerable.Empty() + }; + } // publish the culture(s) - var publishCulture = content.PublishCulture(culture); + var publishCulture = content.PublishCulture(out invalidProperties, culture); if (!publishCulture) - return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content); + return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content) + { + InvalidProperties = invalidProperties ?? Enumerable.Empty() + }; // finally, "save publishing" // what happens next depends on whether the content can be published or not @@ -1242,6 +1251,7 @@ namespace Umbraco.Core.Services.Implement .Distinct() .ToList(); + Property[] invalidProperties = null; var publishing = true; foreach (var culture in pendingCultures) { @@ -1250,14 +1260,17 @@ namespace Umbraco.Core.Services.Implement if (d.Trashed) continue; // won't publish - publishing &= d.PublishCulture(culture); //set the culture to be published + publishing &= d.PublishCulture(out invalidProperties, culture); //set the culture to be published if (!publishing) break; // no point continuing } if (d.Trashed) result = new PublishResult(PublishResultType.FailedPublishIsTrashed, evtMsgs, d); else if (!publishing) - result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d); + result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d) + { + InvalidProperties = invalidProperties ?? Enumerable.Empty() + }; else result = CommitDocumentChanges(d, d.WriterId); @@ -1436,7 +1449,7 @@ namespace Umbraco.Core.Services.Implement } /// - public IEnumerable SaveAndPublishBranch(IContent document, bool force, + internal IEnumerable SaveAndPublishBranch(IContent document, bool force, Func> shouldPublish, Func, bool> publishCultures, int userId = Constants.Security.SuperUserId) @@ -1536,7 +1549,11 @@ namespace Umbraco.Core.Services.Implement // publish & check if values are valid if (!publishCultures(document, culturesToPublish)) + { + //TODO: Based on this callback behavior there is no way to know which properties may have been invalid if this failed, see other results of FailedPublishContentInvalid return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, document); + } + var result = CommitDocumentChangesInternal(scope, document, userId, branchOne: true, branchRoot: isRoot); if (result.Success) From e923b940f9c49d7e6424094b1ecc31b2d6979142 Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 21 Feb 2019 09:35:38 +0100 Subject: [PATCH 4/9] Cleanup Url token encoding --- src/Umbraco.Core/StringExtensions.cs | 109 ++++++++---------- .../Strings/StringExtensionsTests.cs | 15 +++ 2 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index b80149a4ef..4df1105bf7 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -593,13 +593,14 @@ namespace Umbraco.Core /// public static string ToUrlBase64(this string input) { - if (input == null) throw new ArgumentNullException("input"); + if (input == null) throw new ArgumentNullException(nameof(input)); - if (String.IsNullOrEmpty(input)) return String.Empty; + if (string.IsNullOrEmpty(input)) + return string.Empty; + //return Convert.ToBase64String(bytes).Replace(".", "-").Replace("/", "_").Replace("=", ","); var bytes = Encoding.UTF8.GetBytes(input); return UrlTokenEncode(bytes); - //return Convert.ToBase64String(bytes).Replace(".", "-").Replace("/", "_").Replace("=", ","); } /// @@ -609,14 +610,14 @@ namespace Umbraco.Core /// public static string FromUrlBase64(this string input) { - if (input == null) throw new ArgumentNullException("input"); + if (input == null) throw new ArgumentNullException(nameof(input)); //if (input.IsInvalidBase64()) return null; try { //var decodedBytes = Convert.FromBase64String(input.Replace("-", ".").Replace("_", "/").Replace(",", "=")); - byte[] decodedBytes = UrlTokenDecode(input); + var decodedBytes = UrlTokenDecode(input); return decodedBytes != null ? Encoding.UTF8.GetString(decodedBytes) : null; } catch (FormatException) @@ -795,42 +796,40 @@ namespace Umbraco.Core internal static byte[] UrlTokenDecode(string input) { if (input == null) + throw new ArgumentNullException(nameof(input)); + + if (input.Length == 0) + return Array.Empty(); + + // calc array size - must be groups of 4 + var arrayLength = input.Length; + var remain = arrayLength % 4; + if (remain != 0) arrayLength += 4 - remain; + + var inArray = new char[arrayLength]; + for (var i = 0; i < input.Length; i++) { - throw new ArgumentNullException("input"); - } - int length = input.Length; - if (length < 1) - { - return new byte[0]; - } - int num2 = input[length - 1] - '0'; - if ((num2 < 0) || (num2 > 10)) - { - return null; - } - char[] inArray = new char[(length - 1) + num2]; - for (int i = 0; i < (length - 1); i++) - { - char ch = input[i]; + var ch = input[i]; switch (ch) { - case '-': + case '-': // restore '-' as '+' inArray[i] = '+'; break; - case '_': + case '_': // restore '_' as '/' inArray[i] = '/'; break; - default: + default: // keep char unchanged inArray[i] = ch; break; } } - for (int j = length - 1; j < inArray.Length; j++) - { + + // pad with '=' + for (var j = input.Length; j < inArray.Length; j++) inArray[j] = '='; - } + return Convert.FromBase64CharArray(inArray, 0, inArray.Length); } @@ -842,54 +841,40 @@ namespace Umbraco.Core internal static string UrlTokenEncode(byte[] input) { if (input == null) + throw new ArgumentNullException(nameof(input)); + + if (input.Length == 0) + return string.Empty; + + // base-64 digits are A-Z, a-z, 0-9, + and / + // the = char is used for trailing padding + + var str = Convert.ToBase64String(input); + + var pos = str.IndexOf('='); + if (pos < 0) pos = str.Length; + + // replace chars that would cause problems in urls + var chArray = new char[pos]; + for (var i = 0; i < pos; i++) { - throw new ArgumentNullException("input"); - } - if (input.Length < 1) - { - return String.Empty; - } - string str = null; - int index = 0; - char[] chArray = null; - str = Convert.ToBase64String(input); - if (str == null) - { - return null; - } - index = str.Length; - while (index > 0) - { - if (str[index - 1] != '=') - { - break; - } - index--; - } - chArray = new char[index + 1]; - chArray[index] = (char)((0x30 + str.Length) - index); - for (int i = 0; i < index; i++) - { - char ch = str[i]; + var ch = str[i]; switch (ch) { - case '+': + case '+': // replace '+' with '-' chArray[i] = '-'; break; - case '/': + case '/': // replace '/' with '_' chArray[i] = '_'; break; - case '=': - chArray[i] = ch; - break; - - default: + default: // keep char unchanged chArray[i] = ch; break; } } + return new string(chArray); } diff --git a/src/Umbraco.Tests/Strings/StringExtensionsTests.cs b/src/Umbraco.Tests/Strings/StringExtensionsTests.cs index f0db3991b8..4c365d733f 100644 --- a/src/Umbraco.Tests/Strings/StringExtensionsTests.cs +++ b/src/Umbraco.Tests/Strings/StringExtensionsTests.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Text; using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Composing; @@ -202,6 +203,20 @@ namespace Umbraco.Tests.Strings Assert.AreEqual(expected, result); } + [TestCase("hello", "aGVsbG8")] + [TestCase("tad", "dGFk")] + [TestCase("AmqGr+Fd!~ééé", "QW1xR3IrRmQhfsOpw6nDqQ")] + public void UrlTokenEncoding(string value, string expected) + { + var bytes = Encoding.UTF8.GetBytes(value); + Console.WriteLine("base64: " + Convert.ToBase64String(bytes)); + var encoded = StringExtensions.UrlTokenEncode(bytes); + Assert.AreEqual(expected, encoded); + + var backBytes = StringExtensions.UrlTokenDecode(encoded); + var backString = Encoding.UTF8.GetString(backBytes); + Assert.AreEqual(value, backString); + } // FORMAT STRINGS From c85b90d65db4a79292c1f69f6d93e0e30f7579a4 Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 21 Feb 2019 11:28:51 +0100 Subject: [PATCH 5/9] Combined Guids media path scheme --- .../CompositionExtensions/FileSystems.cs | 2 +- src/Umbraco.Core/GuidUtils.cs | 67 +++++++++++++++++++ .../CombinedGuidsMediaPathScheme.cs | 8 ++- src/Umbraco.Core/IO/ShadowWrapper.cs | 26 +++++++ .../CoreThings/GuidUtilsTests.cs | 8 +++ src/Umbraco.Tests/IO/FileSystemsTests.cs | 2 +- src/Umbraco.Tests/Testing/UmbracoTestBase.cs | 2 +- 7 files changed, 110 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/Composing/CompositionExtensions/FileSystems.cs b/src/Umbraco.Core/Composing/CompositionExtensions/FileSystems.cs index 76b363f80e..4fbbec0265 100644 --- a/src/Umbraco.Core/Composing/CompositionExtensions/FileSystems.cs +++ b/src/Umbraco.Core/Composing/CompositionExtensions/FileSystems.cs @@ -79,7 +79,7 @@ namespace Umbraco.Core.Composing.CompositionExtensions composition.RegisterUnique(factory => factory.GetInstance()); // register the scheme for media paths - composition.RegisterUnique(); + composition.RegisterUnique(); // register the IMediaFileSystem implementation composition.RegisterFileSystem(); diff --git a/src/Umbraco.Core/GuidUtils.cs b/src/Umbraco.Core/GuidUtils.cs index 3768e1385d..d878cee16b 100644 --- a/src/Umbraco.Core/GuidUtils.cs +++ b/src/Umbraco.Core/GuidUtils.cs @@ -40,5 +40,72 @@ namespace Umbraco.Core public DecomposedGuid(Guid value) : this() => this.Value = value; } + + private static readonly char[] Base32Table = + { + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', + 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '0', '1', '2', '3', '4', '5' + }; + + /// + /// Converts a Guid into a base-32 string. + /// + /// A Guid. + /// The string length. + /// A base-32 encoded string. + /// + /// A base-32 string representation of a Guid is the shortest, efficient, representation + /// that is case insensitive (base-64 is case sensitive). + /// Length must be 1-26, anything else becomes 26. + /// + public static string ToBase32String(Guid guid, int length = 26) + { + if (length <= 0 || length > 26) + length = 26; + + var bytes = guid.ToByteArray(); // a Guid is 128 bits ie 16 bytes + + // this could be optimized by making it unsafe, + // and fixing the table + bytes + chars (see Convert.ToBase64CharArray) + + // each block of 5 bytes = 5*8 = 40 bits + // becomes 40 bits = 8*5 = 8 byte-32 chars + // a Guid is 3 blocks + 8 bits + + // so it turns into a 3*8+2 = 26 chars string + var chars = new char[length]; + + var i = 0; + var j = 0; + + while (i < 15) + { + if (j == length) break; + chars[j++] = Base32Table[(bytes[i] & 0b1111_1000) >> 3]; + if (j == length) break; + chars[j++] = Base32Table[((bytes[i] & 0b0000_0111) << 2) | ((bytes[i + 1] & 0b1100_0000) >> 6)]; + if (j == length) break; + chars[j++] = Base32Table[(bytes[i + 1] & 0b0011_1110) >> 1]; + if (j == length) break; + chars[j++] = Base32Table[(bytes[i + 1] & 0b0000_0001) | ((bytes[i + 2] & 0b1111_0000) >> 4)]; + if (j == length) break; + chars[j++] = Base32Table[((bytes[i + 2] & 0b0000_1111) << 1) | ((bytes[i + 3] & 0b1000_0000) >> 7)]; + if (j == length) break; + chars[j++] = Base32Table[(bytes[i + 3] & 0b0111_1100) >> 2]; + if (j == length) break; + chars[j++] = Base32Table[((bytes[i + 3] & 0b0000_0011) << 3) | ((bytes[i + 4] & 0b1110_0000) >> 5)]; + if (j == length) break; + chars[j++] = Base32Table[bytes[i + 4] & 0b0001_1111]; + + i += 5; + } + + if (j < length) + chars[j++] = Base32Table[(bytes[i] & 0b1111_1000) >> 3]; + if (j < length) + chars[j] = Base32Table[(bytes[i] & 0b0000_0111) << 2]; + + return new string(chars); + } } } diff --git a/src/Umbraco.Core/IO/MediaPathSchemes/CombinedGuidsMediaPathScheme.cs b/src/Umbraco.Core/IO/MediaPathSchemes/CombinedGuidsMediaPathScheme.cs index 59f8213414..fa5ec12142 100644 --- a/src/Umbraco.Core/IO/MediaPathSchemes/CombinedGuidsMediaPathScheme.cs +++ b/src/Umbraco.Core/IO/MediaPathSchemes/CombinedGuidsMediaPathScheme.cs @@ -7,16 +7,20 @@ namespace Umbraco.Core.IO.MediaPathSchemes /// Implements a combined-guids media path scheme. /// /// - /// Path is "{combinedGuid}/{filename>}" where combinedGuid is a combination of itemGuid and propertyGuid. + /// Path is "{combinedGuid}/{filename}" where combinedGuid is a combination of itemGuid and propertyGuid. /// public class CombinedGuidsMediaPathScheme : IMediaPathScheme { + private const int DirectoryLength = 8; + /// public string GetFilePath(IMediaFileSystem fileSystem, Guid itemGuid, Guid propertyGuid, string filename, string previous = null) { // assumes that cuid and puid keys can be trusted - and that a single property type // for a single content cannot store two different files with the same name - var directory = HexEncoder.Encode(GuidUtils.Combine(itemGuid, propertyGuid).ToByteArray()/*'/', 2, 4*/); // could use ext to fragment path eg 12/e4/f2/... + + var combinedGuid = GuidUtils.Combine(itemGuid, propertyGuid); + var directory = GuidUtils.ToBase32String(combinedGuid, DirectoryLength); // see also HexEncoder, we may want to fragment path eg 12/e4/f3... return Path.Combine(directory, filename).Replace('\\', '/'); } diff --git a/src/Umbraco.Core/IO/ShadowWrapper.cs b/src/Umbraco.Core/IO/ShadowWrapper.cs index d71f328713..d6e54d6092 100644 --- a/src/Umbraco.Core/IO/ShadowWrapper.cs +++ b/src/Umbraco.Core/IO/ShadowWrapper.cs @@ -22,6 +22,32 @@ namespace Umbraco.Core.IO _isScoped = isScoped; } + public static string CreateShadowId() + { + const int retries = 50; // avoid infinite loop + const int idLength = 8; // 6 chars + + // shorten a Guid to idLength chars, and see whether it collides + // with an existing directory or not - if it does, try again, and + // we should end up with a unique identifier eventually - but just + // detect infinite loops (just in case) + + for (var i = 0; i < retries; i++) + { + var id = GuidUtils.ToBase32String(Guid.NewGuid(), idLength); + + var virt = ShadowFsPath + "/" + id; + var shadowDir = IOHelper.MapPath(virt); + if (Directory.Exists(shadowDir)) + continue; + + Directory.CreateDirectory(shadowDir); + return id; + } + + throw new Exception($"Could not get a shadow identifier (tried {retries} times)"); + } + internal void Shadow(Guid id) { // note: no thread-safety here, because ShadowFs is thread-safe due to the check diff --git a/src/Umbraco.Tests/CoreThings/GuidUtilsTests.cs b/src/Umbraco.Tests/CoreThings/GuidUtilsTests.cs index 5ef8cba356..639d85b4ff 100644 --- a/src/Umbraco.Tests/CoreThings/GuidUtilsTests.cs +++ b/src/Umbraco.Tests/CoreThings/GuidUtilsTests.cs @@ -15,6 +15,14 @@ namespace Umbraco.Tests.CoreThings Assert.AreEqual(GuidUtils.Combine(a, b).ToByteArray(), Combine(a, b)); } + [Test] + public void GuidThingTest() + { + var guid = new Guid("f918382f-2bba-453f-a3e2-1f594016ed3b"); + Assert.AreEqual("f22br4n0fm5fli5c", GuidUtils.ToBase32String(guid, 16)); + Assert.AreEqual("f22br4n0f", GuidUtils.ToBase32String(guid, 9)); + } + // Reference implementation taken from original code. private static byte[] Combine(Guid guid1, Guid guid2) { diff --git a/src/Umbraco.Tests/IO/FileSystemsTests.cs b/src/Umbraco.Tests/IO/FileSystemsTests.cs index 8c70de883f..1f0250c066 100644 --- a/src/Umbraco.Tests/IO/FileSystemsTests.cs +++ b/src/Umbraco.Tests/IO/FileSystemsTests.cs @@ -33,7 +33,7 @@ namespace Umbraco.Tests.IO composition.Register(_ => Mock.Of()); composition.Register(_ => Mock.Of()); composition.Register(_ => Mock.Of()); - composition.RegisterUnique(); + composition.RegisterUnique(); composition.Configs.Add(SettingsForTests.GetDefaultGlobalSettings); composition.Configs.Add(SettingsForTests.GetDefaultUmbracoSettings); diff --git a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs index f6f8f220b8..8fb8984fea 100644 --- a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs +++ b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs @@ -243,7 +243,7 @@ namespace Umbraco.Tests.Testing Composition.WithCollectionBuilder(); Composition.RegisterUnique(); - Composition.RegisterUnique(); + Composition.RegisterUnique(); // register empty content apps collection Composition.WithCollectionBuilder(); From 73c4e734b35aac3903a520a91d6882c266614bba Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 21 Feb 2019 12:20:10 +0100 Subject: [PATCH 6/9] Move to UniqueMediaPathScheme --- .../CompositionExtensions/FileSystems.cs | 2 +- .../CombinedGuidsMediaPathScheme.cs | 1 + .../MediaPathSchemes/UniqueMediaPathScheme.cs | 38 +++++++++++++++++++ src/Umbraco.Core/Umbraco.Core.csproj | 1 + src/Umbraco.Tests/IO/FileSystemsTests.cs | 2 +- src/Umbraco.Tests/Testing/UmbracoTestBase.cs | 2 +- 6 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 src/Umbraco.Core/IO/MediaPathSchemes/UniqueMediaPathScheme.cs diff --git a/src/Umbraco.Core/Composing/CompositionExtensions/FileSystems.cs b/src/Umbraco.Core/Composing/CompositionExtensions/FileSystems.cs index 4fbbec0265..078a505be9 100644 --- a/src/Umbraco.Core/Composing/CompositionExtensions/FileSystems.cs +++ b/src/Umbraco.Core/Composing/CompositionExtensions/FileSystems.cs @@ -79,7 +79,7 @@ namespace Umbraco.Core.Composing.CompositionExtensions composition.RegisterUnique(factory => factory.GetInstance()); // register the scheme for media paths - composition.RegisterUnique(); + composition.RegisterUnique(); // register the IMediaFileSystem implementation composition.RegisterFileSystem(); diff --git a/src/Umbraco.Core/IO/MediaPathSchemes/CombinedGuidsMediaPathScheme.cs b/src/Umbraco.Core/IO/MediaPathSchemes/CombinedGuidsMediaPathScheme.cs index fa5ec12142..8a4c0e5dc0 100644 --- a/src/Umbraco.Core/IO/MediaPathSchemes/CombinedGuidsMediaPathScheme.cs +++ b/src/Umbraco.Core/IO/MediaPathSchemes/CombinedGuidsMediaPathScheme.cs @@ -8,6 +8,7 @@ namespace Umbraco.Core.IO.MediaPathSchemes /// /// /// Path is "{combinedGuid}/{filename}" where combinedGuid is a combination of itemGuid and propertyGuid. + /// This scheme is dangerous, as it does not prevent potential collisions. /// public class CombinedGuidsMediaPathScheme : IMediaPathScheme { diff --git a/src/Umbraco.Core/IO/MediaPathSchemes/UniqueMediaPathScheme.cs b/src/Umbraco.Core/IO/MediaPathSchemes/UniqueMediaPathScheme.cs new file mode 100644 index 0000000000..722c92d3de --- /dev/null +++ b/src/Umbraco.Core/IO/MediaPathSchemes/UniqueMediaPathScheme.cs @@ -0,0 +1,38 @@ +using System; +using System.IO; + +namespace Umbraco.Core.IO.MediaPathSchemes +{ + /// + /// Implements a unique directory media path scheme. + /// + /// + /// This scheme provides short paths, yet handle potential collisions. + /// + public class UniqueMediaPathScheme : IMediaPathScheme + { + private const int DirectoryLength = 8; + + /// + public string GetFilePath(IMediaFileSystem fileSystem, Guid itemGuid, Guid propertyGuid, string filename, string previous = null) + { + string directory; + + // no point "combining" guids if all we want is some random guid - just get a new one + // and then, because we don't want collisions, ensure that the directory does not already exist + // (should be quite rare, but eh...) + + do + { + var combinedGuid = Guid.NewGuid(); + directory = GuidUtils.ToBase32String(combinedGuid, DirectoryLength); // see also HexEncoder, we may want to fragment path eg 12/e4/f3... + + } while (fileSystem.DirectoryExists(directory)); + + return Path.Combine(directory, filename).Replace('\\', '/'); + } + + /// + public string GetDeleteDirectory(IMediaFileSystem fileSystem, string filepath) => Path.GetDirectoryName(filepath); + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index c3fe7be10a..d07c2e54eb 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -205,6 +205,7 @@ + diff --git a/src/Umbraco.Tests/IO/FileSystemsTests.cs b/src/Umbraco.Tests/IO/FileSystemsTests.cs index 1f0250c066..d4124530ba 100644 --- a/src/Umbraco.Tests/IO/FileSystemsTests.cs +++ b/src/Umbraco.Tests/IO/FileSystemsTests.cs @@ -33,7 +33,7 @@ namespace Umbraco.Tests.IO composition.Register(_ => Mock.Of()); composition.Register(_ => Mock.Of()); composition.Register(_ => Mock.Of()); - composition.RegisterUnique(); + composition.RegisterUnique(); composition.Configs.Add(SettingsForTests.GetDefaultGlobalSettings); composition.Configs.Add(SettingsForTests.GetDefaultUmbracoSettings); diff --git a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs index 8fb8984fea..fedc94d45b 100644 --- a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs +++ b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs @@ -243,7 +243,7 @@ namespace Umbraco.Tests.Testing Composition.WithCollectionBuilder(); Composition.RegisterUnique(); - Composition.RegisterUnique(); + Composition.RegisterUnique(); // register empty content apps collection Composition.WithCollectionBuilder(); From aaa8e881f84bf997032cee06984f8d699658f66e Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 21 Feb 2019 14:45:12 +0100 Subject: [PATCH 7/9] Version number revert --- src/SolutionInfo.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SolutionInfo.cs b/src/SolutionInfo.cs index 9ed398d52f..2a5759d768 100644 --- a/src/SolutionInfo.cs +++ b/src/SolutionInfo.cs @@ -18,5 +18,5 @@ using System.Resources; [assembly: AssemblyVersion("8.0.0")] // these are FYI and changed automatically -[assembly: AssemblyFileVersion("8.1.0")] -[assembly: AssemblyInformationalVersion("8.1.0")] +[assembly: AssemblyFileVersion("8.0.0")] +[assembly: AssemblyInformationalVersion("8.0.0")] From e52ff0ea031c3a91e03cdc6b8c462430aae3ac88 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 21 Feb 2019 14:57:41 +0100 Subject: [PATCH 8/9] merge fix --- src/Umbraco.Core/IO/ShadowWrapper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/IO/ShadowWrapper.cs b/src/Umbraco.Core/IO/ShadowWrapper.cs index d6e54d6092..dacef52a92 100644 --- a/src/Umbraco.Core/IO/ShadowWrapper.cs +++ b/src/Umbraco.Core/IO/ShadowWrapper.cs @@ -48,7 +48,7 @@ namespace Umbraco.Core.IO throw new Exception($"Could not get a shadow identifier (tried {retries} times)"); } - internal void Shadow(Guid id) + internal void Shadow(string id) { // note: no thread-safety here, because ShadowFs is thread-safe due to the check // on ShadowFileSystemsScope.None - and if None is false then we should be running From a2279d03f69922e53216888db57448b5de305d9a Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 21 Feb 2019 16:13:21 +0100 Subject: [PATCH 9/9] Materializing the TypeContentSources before iterating them. --- .../PublishedCache/NuCache/PublishedSnapshotService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index a85e75f14c..c9709ddd5f 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -947,7 +947,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { scope.ReadLock(Constants.Locks.ContentTypes); var typesA = CreateContentTypes(PublishedItemType.Content, refreshedIdsA).ToArray(); - var kits = _dataSource.GetTypeContentSources(scope, refreshedIdsA); + var kits = _dataSource.GetTypeContentSources(scope, refreshedIdsA).ToArray(); _contentStore.UpdateContentTypes(removedIds, typesA, kits); _contentStore.UpdateContentTypes(CreateContentTypes(PublishedItemType.Content, otherIds.ToArray()).ToArray()); _contentStore.NewContentTypes(CreateContentTypes(PublishedItemType.Content, newIds.ToArray()).ToArray());