updating comments for NestedContentValueConnector

Author: Claus

.gitignore

@@ -30,3 +30,4 @@ src/Umbraco.Deploy.Contrib.Connectors/App_Plugins/*
 src/Umbraco.Deploy.Contrib.Connectors/Config/*
 src/Umbraco.Deploy.Contrib.Connectors/app.config
 /build/UmbracoDeploy.Contrib.*.nupkg
+/build/UmbracoDeploy.Contrib.*.zip

src/Umbraco.Deploy.Contrib.Connectors/ValueConnectors/NestedContentValueConnector.cs

@@ -21,6 +21,14 @@ public class NestedContentValueConnector : IValueConnector
         private readonly Lazy<ValueConnectorCollection> _valueConnectorsLazy;
         private readonly ILogger _logger;
 
+        // Our.Umbraco.NestedContent is the original NestedContent package
+        // Umbraco.NestedContent is Core NestedContent (introduced in v7.7)
+        public virtual IEnumerable<string> PropertyEditorAliases => new[] { "Our.Umbraco.NestedContent", "Umbraco.NestedContent" };
+
+        // cannot inject ValueConnectorCollection else of course it creates a circular (recursive) dependency,
+        // so we have to inject it lazily and use the lazy value when actually needing it
+        private ValueConnectorCollection ValueConnectors => _valueConnectorsLazy.Value;
+
         public NestedContentValueConnector(IContentTypeService contentTypeService, Lazy<ValueConnectorCollection> valueConnectors, ILogger logger)
         {
             if (contentTypeService == null) throw new ArgumentNullException(nameof(contentTypeService));
@@ -31,8 +39,6 @@ public NestedContentValueConnector(IContentTypeService contentTypeService, Lazy<
             _logger = logger;
         }
 
-        // Our.Umbraco.NestedContent is the original NestedContent package
-        // Umbraco.NestedContent is Core NestedContent (introduced in v7.7)
         public string ToArtifact(object value, PropertyType propertyType, ICollection<ArtifactDependency> dependencies)
         {
             var svalue = value as string;
@@ -44,9 +50,13 @@ public string ToArtifact(object value, PropertyType propertyType, ICollection<Ar
 
             var nestedContent = new List<NestedContentValue>();
             if (svalue.Trim().StartsWith("{"))
+            {
                 nestedContent.Add(JsonConvert.DeserializeObject<NestedContentValue>(svalue));
+            }
             else
+            {
                 nestedContent.AddRange(JsonConvert.DeserializeObject<NestedContentValue[]>(svalue));
+            }
 
             if (nestedContent.All(x => x == null))
                 return null;
@@ -86,13 +96,11 @@ public string ToArtifact(object value, PropertyType propertyType, ICollection<Ar
                         continue;
                     }
 
+                    // fetch the right value connector from the collection of connectors, intended for use with this property type.
                     // throws if not found - no need for a null check
                     var propValueConnector = ValueConnectors.Get(propType);
 
-                    // this should be enough for all other value connectors to work with
-                    // as all they should need is the value, and the property type infos
-                    //var mockProperty = new Property(propType);
-                    //var mockProperty = new Property(propType, row.PropertyValues[key]);
+                    // pass the value, property type and the dependencies collection to the connector to get a "artifact" value
                     var val = row.PropertyValues[key];
                     object parsedValue = propValueConnector.ToArtifact(val, propType, dependencies);
 
@@ -145,25 +153,10 @@ public object FromArtifact(string value, PropertyType propertyType, object curre
                 throw new InvalidOperationException($"Could not resolve these content types for the Nested Content property: {string.Join(",", allContentTypes.Where(x => x.Value == null).Select(x => x.Key))}");
             }
 
-            var mocks = new Dictionary<IContentType, IContent>();
-
             foreach (var row in nestedContent)
             {
                 var contentType = allContentTypes[row.ContentTypeAlias];
 
-                // note
-                // the way we do it here, doing content.SetValue() several time on the same content, reduces
-                // allocations and should be ok because SetValue does not care about the previous value - would
-                // be different for the overloads that manage eg files for uploads (not sure how NestedContent
-                // deals with them really)
-
-                // we need a fake content instance to pass in to the value connector, since the value connector
-                // wants to SetValue on an object - then we can extract the value back from that object to set
-                // it correctly on the real instance
-                IContent mockContent;
-                if (!mocks.TryGetValue(contentType, out mockContent))
-                    mockContent = mocks[contentType] = new Content("NC_" + Guid.NewGuid(), -1, contentType);
-
                 foreach (var key in row.PropertyValues.Keys.ToArray())
                 {
                     // key is a system property that is added by NestedContent in Core v7.7
@@ -179,13 +172,15 @@ public object FromArtifact(string value, PropertyType propertyType, object curre
                         continue;
                     }
 
+                    // fetch the right value connector from the collection of connectors, intended for use with this property type.
                     // throws if not found - no need for a null check
                     var propValueConnector = ValueConnectors.Get(innerPropertyType);
 
                     var rowValue = row.PropertyValues[key];
 
                     if (rowValue != null)
                     {
+                        // pass the artifact value and property type to the connector to get a real value from the artifact
                         var convertedValue = propValueConnector.FromArtifact(rowValue.ToString(), innerPropertyType, null);
                         if (convertedValue == null)
                         {
@@ -200,7 +195,7 @@ public object FromArtifact(string value, PropertyType propertyType, object curre
                         {
                             // test if the value is a json object (thus could be a nested complex editor)
                             // if that's the case we'll need to add it as a json object instead of string to avoid it being escaped
-                            JToken jtokenValue = convertedValue.ToString().DetectIsJson() ? JToken.Parse(convertedValue.ToString()) : null;
+                            var jtokenValue = convertedValue.ToString().DetectIsJson() ? JToken.Parse(convertedValue.ToString()) : null;
                             if (jtokenValue != null)
                             {
                                 row.PropertyValues[key] = jtokenValue;
@@ -223,12 +218,6 @@ public object FromArtifact(string value, PropertyType propertyType, object curre
             return value;
         }
 
-        public virtual IEnumerable<string> PropertyEditorAliases => new[] { "Our.Umbraco.NestedContent", "Umbraco.NestedContent" };
-
-        // cannot inject ValueConnectorCollection else of course it creates a circular (recursive) dependency,
-        // so we have to inject it lazily and use the lazy value when actually needing it
-        private ValueConnectorCollection ValueConnectors => _valueConnectorsLazy.Value;
-
         /// <summary>
         /// The typed value stored for Nested Content
         /// </summary>
@@ -249,13 +238,6 @@ public class NestedContentValue
             [JsonProperty("ncContentTypeAlias")]
             public string ContentTypeAlias { get; set; }
 
-            // starting with v7.7, Core's NestedContent implement "key" as a system property
-            // but since we are supporting pre-v7.7 including the NestedContent package, we
-            // cannot do it this way - it's all managed "manually" when dealing with
-            // PropertyValues.
-            //[JsonProperty("key")]
-            //public Guid Key { get; set; }
-
             /// <summary>
             /// The remaining properties will be serialized to a dictionary
             /// </summary>