Merge v17/main into v18/main (2026-07-11)

dist/build-package.ps1

@@ -117,7 +117,13 @@ $sln_name = "..\uSync.slnx";
 ## restore up front with --force-evaluate so the build's implicit restore doesn't run in
 ## locked mode and fail on NU1403 package content-hash mismatches. --force-evaluate
 ## re-evaluates against the current cache (updating the lock file) instead of erroring.
-dotnet restore $sln_name --force-evaluate
+##
+## -p:Configuration=$env matters: some projects (uSync.Extend) are excluded from the
+## Debug solution configuration via <Build Solution="Debug|*" Project="false" /> in
+## uSync.slnx. A restore with no configuration defaults to Debug and silently skips
+## those projects, leaving their lock file stale for the --no-restore build/pack below
+## (which run in $env/Release) - surfacing later as a NU1903 audit failure on pack.
+dotnet restore $sln_name --force-evaluate -p:Configuration=$env
 
 ""; "##### Building project"; "--------------------------------"; ""
 ## --no-restore: we've already restored above, don't let the build kick off a second

docs/perf/batch-save-investigation.md

@@ -0,0 +1,180 @@
+# Investigation: batched content/media save on import (perf item #1)
+
+**Branch:** `v17/investigate/batch-save`
+**Question:** would routing content/media imports through Umbraco's
+`Save(IEnumerable<…>)` (the batch overload) meaningfully reduce database work,
+given that (a) Umbraco's own batching is limited and (b) notifications may not be
+batched?
+
+**Short answer:** No — not as a safe, general win. In the default configuration
+the only saving would come at the cost of the per-item failure isolation that the
+current default is deliberately designed to provide. In the opt-in "suppressed"
+configuration the transaction and notifications are *already* batched by uSync's
+ambient scope, so the batch overload adds almost nothing. Recommendation: **do not
+wire up bulk `Save` for content/media.** Details and evidence below.
+
+---
+
+## 1. What the batch overload actually does
+
+Decompiled from `Umbraco.Cms.Core.Services.ContentService` (Umbraco 17.3.0,
+`Umbraco.Core.dll`). Media (`MediaService`) is equivalent.
+
+`Save(IContent)` — the per-item path uSync uses today:
+
+```csharp
+using (ICoreScope scope = ScopeProvider.CreateCoreScope())
+{
+    scope.WriteLock(Constants.Locks.ContentTree);
+    if (scope.Notifications.PublishCancelable(new ContentSavingNotification(content, …)))
+        { scope.Complete(); return Cancel; }
+    _documentRepository.Save(content);                 // 1 row write
+    scope.Notifications.Publish(new ContentSavedNotification(content, …));
+    scope.Notifications.Publish(new ContentTreeChangeNotification(content, RefreshNode, …));
+    Audit(...);
+    scope.Complete();                                   // 1 transaction commit
+}
+```
+
+`Save(IEnumerable<IContent>)` — the batch overload:
+
+```csharp
+IContent[] array = contents.ToArray();
+using (ICoreScope scope = ScopeProvider.CreateCoreScope())
+{
+    scope.WriteLock(Constants.Locks.ContentTree);
+    if (scope.Notifications.PublishCancelable(new ContentSavingNotification(array, …)))  // ONE, batched
+        { scope.Complete(); return Cancel; }
+    foreach (IContent content in array)
+        _documentRepository.Save(content);              // still 1 row write PER item
+    scope.Notifications.Publish(new ContentSavedNotification(array, …));                 // ONE, batched
+    scope.Notifications.Publish(new ContentTreeChangeNotification(array, RefreshNode, …));// ONE, batched
+    Audit(...);
+    scope.Complete();                                   // ONE transaction commit
+}
+```
+
+Key observations:
+
+- **The actual row writes are identical** — `_documentRepository.Save(content)` runs
+  once per item in both. The batch overload does **not** issue a single set-based
+  SQL statement; it loops. So there is **no reduction in the number of INSERT/UPDATE
+  round-trips**.
+- The batch overload's savings are purely **structural**: 1 scope/transaction/commit
+  and 1 write-lock instead of N, and **notifications *are* batched** — `ContentSaving`,
+  `ContentSaved` and `ContentTreeChange` each fire **once with an array** rather than
+  N times. (This corrects the assumption that "notifications aren't batched" — at the
+  `ContentService` level they are, when you use the batch overload.)
+- The batch overload also **skips** the per-item validation that `Save(IContent)`
+  performs: the `PublishedState` guard and the 255-char name-length check. It also
+  doesn't accept a `ContentSchedule`.
+
+So the theoretical benefit of switching is: **N transactions → 1, and N notification
+dispatches → 1.** No change to the number of row writes.
+
+## 2. Does uSync actually pay "N transactions" today? It depends on config.
+
+uSync wraps an import handler run in
+`ICoreScopeProvider.CreateNotificationScope(...)`
+([`ScopeExtensions.cs`](../../uSync.BackOffice/Extensions/ScopeExtensions.cs)):
+
+```csharp
+if (syncConfigService.Settings.DisableNotificationSuppression)
+    return null;                                    // <-- default path
+return scopeProvider.CreateCoreScope(
+    scopedNotificationPublisher: notificationPublisher,   // SyncScopedNotificationPublisher
+    autoComplete: true);
+```
+
+`DisableNotificationSuppression` **defaults to `true`** on v16+
+([`uSyncSettings.cs:224`](../../uSync.BackOffice/Configuration/uSyncSettings.cs)),
+so there are two very different runtime shapes:
+
+### Default config — `DisableNotificationSuppression = true`
+
+`CreateNotificationScope` returns **null**. There is **no uSync ambient scope**
+around the import (`SyncService_Handlers.cs`: `scope?.Complete()` is a no-op). Each
+per-item `contentService.Save(item)` therefore opens its **own** root scope →
+its **own** transaction/commit, and fires its notifications **immediately**.
+
+- Here, N items really do mean **N transactions + N notification sets**.
+- The batch overload *would* collapse these to 1 + 1.
+- **BUT** this per-item, non-batched behaviour is a *deliberate design decision*.
+  From the setting's own XML docs:
+
+  > on v16 the default is true, because some of the notifications appear to be
+  > closely coupled to the save/publish process, and if something goes wrong in one
+  > item's import it can cause a cascade of failures across everything that might have
+  > been imported along with it. If the notifications are not suppressed, then if an
+  > item fails to import it doesn't stop other items from being imported.
+
+  Batch-saving reintroduces exactly the failure mode this default exists to avoid:
+  a single bad item (DB constraint, a throwing `Saving`/`Saved` handler, an
+  over-long name that the batch overload no longer validates) rolls back or aborts
+  the **whole batch**, and uSync loses its per-item error attribution (which item
+  failed, with what message). This matches the "batching causes issues" experience.
+
+### Opt-in config — `DisableNotificationSuppression = false`
+
+`CreateNotificationScope` returns a real ambient scope using
+`SyncScopedNotificationPublisher`
+([`SyncScopedNotificationPublisher.cs`](../../uSync.BackOffice/Notifications/SyncScopedNotificationPublisher.cs)).
+
+- **Transaction is already batched.** Umbraco scopes nest; the per-item
+  `contentService.Save` calls become child scopes that share the single ambient
+  transaction, which commits once when uSync completes the outer scope. So the
+  "N transactions" cost is **already gone** without the batch overload.
+- **Notifications are already deferred and grouped.** The scoped publisher collects
+  every notification raised during the import and, at completion, dispatches them
+  grouped by type in one `IEventAggregator.Publish(items)` call per type — or, if
+  `BackgroundNotifications = true`, hands them to the background task queue.
+
+  In this mode, switching to the batch overload only changes "N single-entity
+  `ContentSavedNotification`s, then group-published" into "1 array
+  `ContentSavedNotification`". That is a marginal allocation/dispatch difference,
+  **not** a database saving.
+
+## 3. Why the schema types already use the bulk path — and content doesn't
+
+This asymmetry is intentional. `ContentTypeSerializer`, `MediaTypeSerializer`,
+`MemberTypeSerializer` and `DataTypeSerializer` already override `SaveAsync` and
+honour `SerializerFlags.DoNotSave`, because saving a **doctype/datatype** triggers
+expensive schema changes and full cache/nucache rebuilds — there, batching many into
+one operation is a genuine, large win and the items are few. For **content/media**
+items the per-save cost is dominated by the unavoidable per-row write, and the items
+are many, so the batch overload buys far less while adding the atomicity risk above.
+
+The handler-level bulk hook does exist —
+[`SyncHandlerRoot.ImportAllAsync`](../../uSync.BackOffice/SyncHandlers/SyncHandlerRoot.cs)
+calls `serializer.SaveAsync(updates…)` — but only under
+`if (options.Flags.HasFlag(SerializerFlags.DoNotSave))`, and `DoNotSave` is never set
+for the content/media import path. So the plumbing is present but deliberately dormant
+for these types.
+
+## 4. Conclusion & recommendation
+
+| Config | Transactions today | Notifications today | Batch overload benefit | Cost/risk |
+|---|---|---|---|---|
+| **Default** (`DisableNotificationSuppression = true`) | N (one per item) | N, fired immediately | N→1 txn, N→1 notifications | **Loses per-item failure isolation** (the reason this is the default); no reduction in row writes |
+| **Suppressed** (`= false`) | 1 (ambient scope) | Deferred + grouped (opt. background) | ~none (already batched) | Marginal |
+
+**Recommendation: do not wire up bulk `Save(IEnumerable)` for content/media.**
+It delivers no reduction in the dominant cost (per-row writes), its transaction/
+notification batching is either already provided by the suppressed-scope path or
+directly conflicts with the intentional per-item isolation of the default path, and
+it removes per-item error reporting plus two validation checks.
+
+If import throughput on large content sets is the goal, the existing, lower-risk
+levers are configuration, not code:
+
+- **`DisableNotificationSuppression = false`** — collapses the import to a single
+  transaction and defers/groups notifications (this is the "batch" people actually
+  want, done at the scope level rather than the save level).
+- **`BackgroundNotifications = true`** — moves notification processing off the import
+  thread entirely.
+
+If a save-level optimisation is ever revisited, the only shape worth prototyping is a
+**bounded** batch (e.g. save in chunks of N with a per-chunk try/catch that falls back
+to per-item on failure) so the failure blast radius stays small — and it should be
+measured against a real content tree before adoption, since the row-write cost (which
+batching does not change) is expected to dominate.

uSync.BackOffice/Configuration/uSyncHandlerSettings.cs

@@ -4,6 +4,8 @@
 
 using Umbraco.Extensions;
 
+using uSync.Core.Extensions;
+
 namespace uSync.BackOffice.Configuration;
 
 /// <summary>
@@ -89,10 +91,10 @@ public static class HandlerSettingsExtensions
     /// <returns></returns>
     public static TResult GetSetting<TResult>(this HandlerSettings settings, string key, TResult defaultValue)
     {
-        if (settings.Settings != null && settings.Settings.TryGetValue(key, out var value))
+        if (settings.Settings != null && settings.Settings.TryGetValue(key, out var value) && value is not null)
         {
-            var attempt = value.TryConvertTo<TResult>();
-            if (attempt) return attempt.Result ?? defaultValue;
+            if (value.TryGetValueAs<TResult>(out var result) && result is not null)
+                return result;
         }
 
         return defaultValue;

uSync.BackOffice/Services/ISyncFileService.cs

@@ -136,6 +136,16 @@ public interface ISyncFileService
     /// <returns></returns>
     Task<XElement> LoadXElementAsync(string file);
 
+    /// <summary>
+    ///  load just the item key (the Key attribute on the root element) from a file.
+    /// </summary>
+    /// <remarks>
+    ///  This streams the file and stops at the root element, so we don't pay the cost
+    ///  of parsing the whole document when all we need is the key (e.g. when working out
+    ///  which items live in a folder for a 'clean' operation).
+    /// </remarks>
+    Task<Guid> LoadKeyFromFileAsync(string file);
+
     /// <summary>
     ///  merge all the files in the given folders into a single xml node, that can be bulk imported
     /// </summary>

uSync.BackOffice/Services/SyncFileService.cs

@@ -211,6 +211,51 @@ public async Task<XElement> LoadXElementAsync(string file)
         }
     }
 
+    private static readonly XmlReaderSettings _keyReaderSettings = new()
+    {
+        CheckCharacters = false,
+        Async = true,
+        IgnoreWhitespace = true,
+        IgnoreComments = true,
+        IgnoreProcessingInstructions = true,
+        DtdProcessing = DtdProcessing.Prohibit,
+    };
+
+    /// <inheritdoc/>
+    public async Task<Guid> LoadKeyFromFileAsync(string file)
+    {
+        EnsureFileExists(file);
+
+        try
+        {
+            using (var stream = OpenRead(file))
+            {
+                if (stream is null)
+                    throw new FileNotFoundException($"Cannot create stream for {file}");
+
+                using (var reader = XmlReader.Create(stream, _keyReaderSettings.Clone()))
+                {
+                    // move to the first (root) element and read its Key attribute,
+                    // we don't need to read any further into the document.
+                    while (await reader.ReadAsync())
+                    {
+                        if (reader.NodeType != XmlNodeType.Element) continue;
+
+                        var key = reader.GetAttribute(global::uSync.Core.uSyncConstants.Xml.Key);
+                        return Guid.TryParse(key, out var guid) ? guid : Guid.Empty;
+                    }
+                }
+            }
+        }
+        catch (Exception ex)
+        {
+            _logger.LogWarning("Error while reading key from {file} {message}", file, ex.Message);
+            throw new Exception($"Error while reading key from {file}", ex);
+        }
+
+        return Guid.Empty;
+    }
+
     /// <inheritdoc/>
     public async Task SaveFileAsync(string filename, Stream stream)
     {

uSync.BackOffice/SyncHandlers/SyncHandlerBase.cs

@@ -99,9 +99,11 @@ protected override async Task<IEnumerable<uSyncAction>> CleanFolderAsync(string
 
     private async Task<Guid?> GetCleanParentKeyAsync(string cleanFile)
     {
-        var node = await syncFileService.LoadXElementAsync(cleanFile);
-        if (node.GetKey() == Guid.Empty) return Guid.Empty;
-        return (await GetCleanParentAsync(cleanFile))?.Key;
+        // stream the key rather than parsing the whole file, and reuse it for the
+        // parent lookup so we don't read the clean file a second time.
+        var key = await syncFileService.LoadKeyFromFileAsync(cleanFile);
+        if (key == Guid.Empty) return Guid.Empty;
+        return (await GetFromServiceAsync(key))?.Key;
     }
 
     /// <summary>