Merge branch 'v8/dev' into v8/contrib

src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs

@@ -74,9 +74,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
             if (ids.Any())
                 sql.WhereIn<NodeDto>(x => x.NodeId, ids);
 
-            return MapDtosToContent(Database.Fetch<DocumentDto>(sql), false,
-                // load everything
-                true, true, true, true);
+            return MapDtosToContent(Database.Fetch<DocumentDto>(sql));
         }
 
         protected override IEnumerable<IContent> PerformGetByQuery(IQuery<IContent> query)
@@ -88,9 +86,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
 
             AddGetByQueryOrderBy(sql);
 
-            return MapDtosToContent(Database.Fetch<DocumentDto>(sql), false,
-                // load everything
-                true, true, true, true);
+            return MapDtosToContent(Database.Fetch<DocumentDto>(sql));
         }
 
         private void AddGetByQueryOrderBy(Sql<ISqlContext> sql)
@@ -229,9 +225,24 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
                 .OrderByDescending<ContentVersionDto>(x => x.Current)
                 .AndByDescending<ContentVersionDto>(x => x.VersionDate);
 
-            return MapDtosToContent(Database.Fetch<DocumentDto>(sql), true, true, true, true, true);
+            return MapDtosToContent(Database.Fetch<DocumentDto>(sql), true);
         }
 
+        // TODO: This method needs to return a readonly version of IContent! The content returned
+        // from this method does not contain all of the data required to re-persist it and if that
+        // is attempted some odd things will occur.
+        // Either we create an IContentReadOnly (which ultimately we should for vNext so we can
+        // differentiate between methods that return entities that can be re-persisted or not), or
+        // in the meantime to not break API compatibility, we can add a property to IContentBase
+        // (or go further and have it on IUmbracoEntity): "IsReadOnly" and if that is true we throw
+        // an exception if that entity is passed to a Save method.
+        // Ideally we return "Slim" versions of content for all sorts of methods here and in ContentService.
+        // Perhaps another non-breaking alternative is to have new services like IContentServiceReadOnly
+        // which can return IContentReadOnly.
+        // We have the ability with `MapDtosToContent` to reduce the amount of data looked up for a
+        // content item. Ideally for paged data that populates list views, these would be ultra slim
+        // content items, there's no reason to populate those with really anything apart from property data,
+        // but until we do something like the above, we can't do that since it would be breaking and unclear.
         public override IEnumerable<IContent> GetAllVersionsSlim(int nodeId, int skip, int take)
         {
             var sql = GetBaseQuery(QueryType.Many, false)
@@ -240,8 +251,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
                 .AndByDescending<ContentVersionDto>(x => x.VersionDate);
 
             return MapDtosToContent(Database.Fetch<DocumentDto>(sql), true,
-                // load bare minimum
-                false, false, false, false).Skip(skip).Take(take);
+                // load bare minimum, need variants though since this is used to rollback with variants
+                false, false, false, true).Skip(skip).Take(take);
         }
 
         public override IContent GetVersion(int versionId)
@@ -837,9 +848,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
             }
 
             return GetPage<DocumentDto>(query, pageIndex, pageSize, out totalRecords,
-                x => MapDtosToContent(x, false,
-                    // load properties but nothing else
-                    true, false, false, true),
+                x => MapDtosToContent(x),
                 filterSql,
                 ordering);
         }
@@ -926,9 +935,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
                 if (ids.Length > 0)
                     sql.WhereIn<NodeDto>(x => x.UniqueId, ids);
 
-                return _outerRepo.MapDtosToContent(Database.Fetch<DocumentDto>(sql), false,
-                    // load everything
-                    true, true, true, true);
+                return _outerRepo.MapDtosToContent(Database.Fetch<DocumentDto>(sql));
             }
 
             protected override IEnumerable<IContent> PerformGetByQuery(IQuery<IContent> query)
@@ -986,9 +993,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
 
             AddGetByQueryOrderBy(sql);
 
-            return MapDtosToContent(Database.Fetch<DocumentDto>(sql),
-                // load the bare minimum
-                false, false, false, false, false);
+            return MapDtosToContent(Database.Fetch<DocumentDto>(sql));
         }
 
         /// <inheritdoc />
@@ -1004,9 +1009,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
 
             AddGetByQueryOrderBy(sql);
 
-            return MapDtosToContent(Database.Fetch<DocumentDto>(sql),
-                // load the bare minimum
-                false, false, false, false, false);
+            return MapDtosToContent(Database.Fetch<DocumentDto>(sql));
         }
 
         #endregion
@@ -1070,11 +1073,11 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
         }
 
         private IEnumerable<IContent> MapDtosToContent(List<DocumentDto> dtos,
-            bool withCache,
-            bool loadProperties,
-            bool loadTemplates,
-            bool loadSchedule,
-            bool loadVariants)
+            bool withCache = false,
+            bool loadProperties = true,
+            bool loadTemplates = true,
+            bool loadSchedule = true,
+            bool loadVariants = true)
         {
             var temps = new List<TempContent<Content>>();
             var contentTypes = new Dictionary<int, IContentType>();