Fixes

backend/src/Squidex.Domain.Apps.Core.Operations/ConvertContent/ContentConverter.cs

@@ -149,22 +149,27 @@ public sealed class ContentConverter(ResolvedComponents components, Schema schem
             return (true, default);
         }
 
-        for (int i = 0; i < array.Count; i++)
+        // Compact in place instead of removing items, because every remove moves all the items
+        // after it and dropping many items from a large array would be quadratic.
+        var target = 0;
+
+        for (var i = 0; i < array.Count; i++)
         {
             var oldValue = array[i];
 
             var (removed, newValue) = ConvertArrayItem(field, oldValue);
             if (removed)
             {
-                array.RemoveAt(i);
-                i--;
-            }
-            else if (!ReferenceEquals(newValue.Value, oldValue.Value))
-            {
-                array[i] = newValue;
+                continue;
             }
+
+            // Faster to check for reference equality than for deep equals.
+            array[target] = ReferenceEquals(newValue.Value, oldValue.Value) ? oldValue : newValue;
+            target++;
         }
 
+        array.RemoveRange(target, array.Count - target);
+
         return (false, array);
     }
 
@@ -175,23 +180,27 @@ public sealed class ContentConverter(ResolvedComponents components, Schema schem
             return (true, default);
         }
 
-        for (int i = 0; i < array.Count; i++)
+        // Compact in place instead of removing items, because every remove moves all the items
+        // after it and dropping many items from a large array would be quadratic.
+        var target = 0;
+
+        for (var i = 0; i < array.Count; i++)
         {
             var oldValue = array[i];
 
             var (removed, newValue) = ConvertComponent(oldValue, parent);
             if (removed)
             {
-                array.RemoveAt(i);
-                i--;
-            }
-            else if (!ReferenceEquals(newValue.Value, oldValue.Value))
-            {
-                // Faster to check for reference equality than for deep equals.
-                array[i] = newValue;
+                continue;
             }
+
+            // Faster to check for reference equality than for deep equals.
+            array[target] = ReferenceEquals(newValue.Value, oldValue.Value) ? oldValue : newValue;
+            target++;
         }
 
+        array.RemoveRange(target, array.Count - target);
+
         return (false, array);
     }
 

backend/src/Squidex.Domain.Apps.Entities/Assets/DefaultAssetFileStore.cs

@@ -6,6 +6,7 @@
 // ==========================================================================
 
 using System.Text;
+using Microsoft.Extensions.Caching.Memory;
 using Microsoft.Extensions.Options;
 using Squidex.Assets;
 using Squidex.Domain.Apps.Core.Apps;
@@ -17,9 +18,11 @@ namespace Squidex.Domain.Apps.Entities.Assets;
 public sealed class DefaultAssetFileStore(
     IAssetStore assetStore,
     IAssetRepository assetRepository,
+    IMemoryCache cache,
     IOptions<AssetOptions> options)
     : IAssetFileStore, IDeleter
 {
+    private static readonly TimeSpan CacheDuration = TimeSpan.FromHours(1);
     private readonly AssetOptions options = options.Value;
 
     async Task IDeleter.DeleteAppAsync(App app,
@@ -48,37 +51,73 @@ public sealed class DefaultAssetFileStore(
     public async Task<long> GetFileSizeAsync(DomainId appId, DomainId id, long fileVersion, string? suffix,
         CancellationToken ct = default)
     {
-        try
+        if (options.FolderPerApp)
         {
-            var fileNameNew = GetFileName(appId, id, fileVersion, suffix);
+            return await assetStore.GetSizeAsync(GetFileName(appId, id, fileVersion, suffix), ct);
+        }
 
-            return await assetStore.GetSizeAsync(fileNameNew, ct);
+        var (first, second, isOldFirst) = FileNames(appId, id, fileVersion, suffix);
+        try
+        {
+            return await assetStore.GetSizeAsync(first, ct);
         }
-        catch (AssetNotFoundException) when (!options.FolderPerApp)
+        catch (AssetNotFoundException)
         {
-            var fileNameOld = GetFileName(id, fileVersion, suffix);
+            RememberFileName(appId, id, !isOldFirst);
 
-            return await assetStore.GetSizeAsync(fileNameOld, ct);
+            return await assetStore.GetSizeAsync(second, ct);
         }
     }
 
     public async Task DownloadAsync(DomainId appId, DomainId id, long fileVersion, string? suffix, Stream stream, BytesRange range = default,
         CancellationToken ct = default)
     {
-        try
+        if (options.FolderPerApp)
         {
-            var fileNameNew = GetFileName(appId, id, fileVersion, suffix);
+            await assetStore.DownloadAsync(GetFileName(appId, id, fileVersion, suffix), stream, range, ct);
+            return;
+        }
 
-            await assetStore.DownloadAsync(fileNameNew, stream, range, ct);
+        var (first, second, isOldFirst) = FileNames(appId, id, fileVersion, suffix);
+        try
+        {
+            await assetStore.DownloadAsync(first, stream, range, ct);
         }
-        catch (AssetNotFoundException) when (!options.FolderPerApp)
+        catch (AssetNotFoundException)
         {
-            var fileNameOld = GetFileName(id, fileVersion, suffix);
+            RememberFileName(appId, id, !isOldFirst);
 
-            await assetStore.DownloadAsync(fileNameOld, stream, range, ct);
+            await assetStore.DownloadAsync(second, stream, range, ct);
         }
     }
 
+    // Assets from older versions are stored under a file name without the app ID. Which name an
+    // asset uses can only be found out by trying, and a failed try is a full roundtrip to the asset
+    // store. Therefore the outcome is remembered as a hint. Both names are still tried, so that a
+    // stale hint only costs the roundtrip it was there to save.
+    private (string First, string Second, bool IsOldFirst) FileNames(DomainId appId, DomainId id, long fileVersion, string? suffix)
+    {
+        var fileNameNew = GetFileName(appId, id, fileVersion, suffix);
+        var fileNameOld = GetFileName(id, fileVersion, suffix);
+
+        if (cache.TryGetValue<bool>(CacheKey(appId, id), out var useOld) && useOld)
+        {
+            return (fileNameOld, fileNameNew, true);
+        }
+
+        return (fileNameNew, fileNameOld, false);
+    }
+
+    private void RememberFileName(DomainId appId, DomainId id, bool useOld)
+    {
+        cache.Set(CacheKey(appId, id), useOld, CacheDuration);
+    }
+
+    private static object CacheKey(DomainId appId, DomainId id)
+    {
+        return (typeof(DefaultAssetFileStore), appId, id);
+    }
+
     public Task DownloadAsync(string tempFile, Stream stream,
         CancellationToken ct = default)
     {

backend/src/Squidex.Domain.Apps.Entities/Assets/Transformations.cs

@@ -76,7 +76,9 @@ public static class Transformations
         {
             await DownloadAsync(asset, assetFileStore, stream, ct);
 
-            var bytes = stream.ToArray();
+            // Use the buffer of the pooled stream. ToArray would allocate another copy of the whole
+            // file, which for a file of up to the maximum size would go to the large object heap.
+            var bytes = new ReadOnlySpan<byte>(stream.GetBuffer(), 0, (int)stream.Length);
 
             switch (encoding?.ToLowerInvariant())
             {

backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/ConvertContent/ContentConversionRemovalTests.cs

@@ -0,0 +1,113 @@
+// ==========================================================================
+//  Squidex Headless CMS
+// ==========================================================================
+//  Copyright (c) Squidex UG (haftungsbeschraenkt)
+//  All rights reserved. Licensed under the MIT license.
+// ==========================================================================
+
+using Squidex.Domain.Apps.Core.Contents;
+using Squidex.Domain.Apps.Core.ConvertContent;
+using Squidex.Domain.Apps.Core.Schemas;
+using Squidex.Infrastructure;
+using Squidex.Infrastructure.Json.Objects;
+
+namespace Squidex.Domain.Apps.Core.Operations.ConvertContent;
+
+public class ContentConversionRemovalTests
+{
+    private static readonly DomainId ComponentId = DomainId.NewGuid();
+    private readonly ResolvedComponents components;
+    private readonly Schema schema;
+
+    public ContentConversionRemovalTests()
+    {
+        schema =
+            new Schema { Name = "my-schema" }
+                .AddComponents(1, "components", Partitioning.Invariant)
+                .AddArray(2, "array", Partitioning.Invariant, a => a
+                    .AddString(21, "value"));
+
+        components = new ResolvedComponents(new Dictionary<DomainId, Schema>
+        {
+            [ComponentId] =
+                new Schema { Name = "my-component" }
+                    .AddString(1, "value", Partitioning.Invariant),
+        });
+    }
+
+    [Theory]
+    [InlineData("abc", "abc")]
+    [InlineData("-bc", "bc")]
+    [InlineData("a-c", "ac")]
+    [InlineData("ab-", "ab")]
+    [InlineData("--c", "c")]
+    [InlineData("-b-", "b")]
+    [InlineData("a--", "a")]
+    [InlineData("---", "")]
+    public void Should_remove_array_items_that_are_not_objects(string source, string expected)
+    {
+        var items = source.Select(x => x == '-' ? JsonValue.Create(0) : Item(x));
+
+        Assert.Equal(expected, Convert("array", items));
+    }
+
+    [Theory]
+    [InlineData("abc", "abc")]
+    [InlineData("-bc", "bc")]
+    [InlineData("a-c", "ac")]
+    [InlineData("ab-", "ab")]
+    [InlineData("--c", "c")]
+    [InlineData("-b-", "b")]
+    [InlineData("a--", "a")]
+    [InlineData("---", "")]
+    public void Should_remove_components_of_unknown_schema(string source, string expected)
+    {
+        var items = source.Select(x => x == '-' ? ComponentOf(x, DomainId.NewGuid()) : ComponentOf(x, ComponentId));
+
+        Assert.Equal(expected, Convert("components", items));
+    }
+
+    [Theory]
+    [InlineData("abc", "abc")]
+    [InlineData("-bc", "bc")]
+    [InlineData("a-c", "ac")]
+    [InlineData("ab-", "ab")]
+    [InlineData("--c", "c")]
+    [InlineData("-b-", "b")]
+    [InlineData("a--", "a")]
+    [InlineData("---", "")]
+    public void Should_remove_components_without_discriminator(string source, string expected)
+    {
+        var items = source.Select(x => x == '-' ? Item(x) : ComponentOf(x, ComponentId));
+
+        Assert.Equal(expected, Convert("components", items));
+    }
+
+    private string Convert(string field, IEnumerable<JsonValue> items)
+    {
+        var source =
+            new ContentData()
+                .AddField(field,
+                    new ContentFieldData()
+                        .AddInvariant(JsonValue.Array(items.ToArray())));
+
+        var converted = new ContentConverter(components, schema).Convert(source);
+
+        if (!converted.TryGetValue(field, out var data) || data?["iv"].Value is not JsonArray array)
+        {
+            return string.Empty;
+        }
+
+        return string.Concat(array.Select(x => ((JsonObject)x.Value!)["value"].ToString()));
+    }
+
+    private static JsonValue Item(char value)
+    {
+        return JsonValue.Object().Add("value", value.ToString());
+    }
+
+    private static JsonValue ComponentOf(char value, DomainId schemaId)
+    {
+        return JsonValue.Object().Add("value", value.ToString()).Add(Component.Discriminator, schemaId);
+    }
+}

backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/DefaultAssetFileStoreTests.cs

@@ -6,6 +6,7 @@
 // ==========================================================================
 
 using System.Globalization;
+using Microsoft.Extensions.Caching.Memory;
 using Microsoft.Extensions.Options;
 using Squidex.Assets;
 using Squidex.Domain.Apps.Entities.Assets.Repositories;
@@ -21,6 +22,7 @@ public class DefaultAssetFileStoreTests : GivenContext
     private readonly DomainId assetId = DomainId.NewGuid();
     private readonly long assetFileVersion = 21;
     private readonly AssetOptions options = new AssetOptions();
+    private readonly IMemoryCache cache = new MemoryCache(Options.Create(new MemoryCacheOptions()));
     private readonly DefaultAssetFileStore sut;
 
     public static readonly TheoryData<bool, string, string> PathCases = new TheoryData<bool, string, string>
@@ -39,7 +41,7 @@ public class DefaultAssetFileStoreTests : GivenContext
 
     public DefaultAssetFileStoreTests()
     {
-        sut = new DefaultAssetFileStore(assetStore, assetRepository, Options.Create(options));
+        sut = new DefaultAssetFileStore(assetStore, assetRepository, cache, Options.Create(options));
     }
 
     [Theory]

resolved.md

@@ -108,6 +108,10 @@ across threads; a cold race can build the same pattern twice, which only wastes
 work and never returns anything incorrect. The critical section is just the dictionary
 and linked-list updates.
 
+`MemoryCache` would remove the lock, but `PatternValidator` is constructed without
+dependency injection, so it would have to create and hold its own cache instance. The lock
+is the smaller change and it is already covered by the concurrency harness below.
+
 **Verified:** `dotnet build` clean (0 warnings); a harness mirroring `GetRegex` ran 1.6M
 operations over 8 threads against 3000 distinct patterns in a 1000-entry cache
 (continuous eviction) with 0 exceptions, 0 wrong matches and the cache correctly bounded
@@ -922,3 +926,110 @@ They should be validated against a container run before release.
 
 Items 25, 26 and 27 are pure hoists with no behavioural change and are covered by the
 enrichment tests that did run.
+
+---
+
+### 28. Asset downloads used an exception as the legacy-path fallback — **FIXED**
+`backend/src/Squidex.Domain.Apps.Entities/Assets/DefaultAssetFileStore.cs`
+
+**Was:** `GetFileSizeAsync` and `DownloadAsync` tried the current file name, caught
+`AssetNotFoundException`, and retried with the legacy name (no app ID). On an instance that
+still holds assets under the old scheme, *every* access to those assets threw and caught
+first — and against a cloud store the failed attempt is a full network round trip, so the
+fallback roughly doubled the latency of every legacy asset served.
+
+**Now:** the outcome is remembered per asset in the injected `IMemoryCache`, keyed by
+`(typeof(DefaultAssetFileStore), appId, id)` with a one hour sliding lifetime, so the wrong
+name is only tried once. `IMemoryCache` rather than `Squidex.Caching.LRUCache` because it is
+thread safe on its own — see item 7 for what `LRUCache` does under concurrent access.
+
+**The memo is a hint, not a decision.** `FileNames(...)` returns both names ordered by what
+was last seen to work, and the other one is still tried on failure. That matters because an
+asset can move between schemes — a migration, or an eviction followed by a re-probe — and a
+cache that *decided* rather than *hinted* would turn a stale entry into a hard failure. The
+cost of a wrong hint is one extra round trip, exactly what the code did before.
+
+Two things fell out of it: the `options.FolderPerApp` case now short-circuits to a single
+name with no try/catch at all, and the partial-write hazard flagged in the finding (a retry
+appending to a stream the first attempt already wrote to) is now hit far less often, since a
+warm asset takes the right branch first. It is not *fixed* — that would need the asset store
+to guarantee it writes nothing before failing.
+
+---
+
+### 29. Removing items while iterating a `JsonArray` was quadratic — **FIXED**
+`backend/src/Squidex.Domain.Apps.Core.Operations/ConvertContent/ContentConverter.cs:145,175`
+
+**Was:** `ConvertArray` and `ConvertComponents` both removed in place with
+`array.RemoveAt(i); i--;`. `JsonArray` derives from `List<JsonValue>`, so each removal shifts
+every following element — dropping *k* of *n* items costs O(n·k), and the case where many
+items are dropped (entries referencing deleted component schemas) is exactly the case where
+the array is large.
+
+**Now:** a single compaction pass with a write index, then one `RemoveRange` for the tail.
+
+```csharp
+var target = 0;
+
+for (var i = 0; i < array.Count; i++)
+{
+    var oldValue = array[i];
+
+    var (removed, newValue) = ConvertArrayItem(field, oldValue);
+    if (removed)
+    {
+        continue;
+    }
+
+    array[target] = ReferenceEquals(newValue.Value, oldValue.Value) ? oldValue : newValue;
+    target++;
+}
+
+array.RemoveRange(target, array.Count - target);
+```
+
+The write index is always `<= i`, so a slot is only ever overwritten after it has been read —
+no read-after-write hazard, and the surviving order is preserved.
+
+**Verified with new tests** — `ContentConversionRemovalTests`, 27 cases covering nine removal
+patterns (none, first, middle, last, adjacent pairs, alternating, all) across three ways an
+item gets dropped: a non-object in an array, a component of an unknown schema, and a
+component with no discriminator.
+
+Two checks on the tests themselves, because this is a behaviour-preserving rewrite rather
+than a bug fix:
+
+- They pass against **both** the original `RemoveAt` implementation and the new one, which is
+  the property that actually matters here — they pin the contract rather than the code.
+- Mutation check: deleting the `RemoveRange` line fails 24 of the 27, so they are not
+  vacuous.
+
+A first attempt at these tests drove removal through a custom `IContentItemConverter` that
+stripped the discriminator; that never removed anything, because `ConvertComponent` checks
+the discriminator *before* calling `ConvertNested`. The tests now use inherently invalid
+items, which is both simpler and closer to the real cause.
+
+---
+
+### 30. `stream.ToArray()` copied straight back out of the pooled buffer — **FIXED**
+`backend/src/Squidex.Domain.Apps.Entities/Assets/Transformations.cs:79`
+
+**Was:** `GetTextAsync` downloaded into a `DefaultPools.MemoryStream`
+(`RecyclableMemoryStreamManager`) and then called `ToArray()`, allocating a fresh array of
+the whole file and copying the pooled buffer into it — for a file at the 4 MB limit, straight
+onto the large object heap on every call.
+
+**Now:**
+
+```csharp
+var bytes = new ReadOnlySpan<byte>(stream.GetBuffer(), 0, (int)stream.Length);
+```
+
+`Convert.ToBase64String` and `Encoding.GetString` all have `ReadOnlySpan<byte>` overloads, so
+nothing downstream changed.
+
+Worth being precise about why `GetBuffer` is better rather than just "avoids a copy":
+`RecyclableMemoryStream.GetBuffer()` still consolidates into a single contiguous buffer when
+the stream spans several blocks. The difference is that the buffer it returns comes from the
+pool and goes back on dispose, whereas `ToArray` allocates a new GC array every time.
+`RecyclableMemoryStream` documents `ToArray` as the call to avoid for exactly this reason.

todo.md

@@ -9,9 +9,9 @@ overhead / allocation churn), **S4** low (worth fixing while nearby).
 
 Item numbers are stable and never reused. Completed items move to
 [resolved.md](resolved.md) keeping their number, so gaps in the sequence here are
-expected — items **4**–**22**, **24**–**27** are closed and live there.
+expected — items **4**–**22** and **24**–**30** are closed and live there.
 
-**Status: 7 open of 30 — items 1, 2, 3, 23, 28, 29, 30. The other 23 are in [resolved.md](resolved.md).**
+**Status: 4 open of 30 — items 1, 2, 3 (one root cause) and 23. The other 26 are in [resolved.md](resolved.md).**
 
 ---
 
@@ -92,81 +92,6 @@ than a fixed 1000-id prefix that the outer query re-sorts.
 
 ---
 
-## S3 — Moderate
-
-### 28. Asset downloads use an exception as the legacy-path fallback
-`backend/src/Squidex.Domain.Apps.Entities/Assets/DefaultAssetFileStore.cs:57,74`
-
-```csharp
-try
-{
-    await assetStore.DownloadAsync(fileNameNew, stream, range, ct);
-}
-catch (AssetNotFoundException) when (!options.FolderPerApp)
-{
-    await assetStore.DownloadAsync(fileNameOld, stream, range, ct);
-}
-```
-
-On an instance that still holds assets under the old naming scheme, **every** download of those
-assets throws and catches first. A .NET exception costs on the order of tens of microseconds,
-and against a cloud store (S3, Azure Blob) the failed attempt is also a full network round trip
-before the retry — so the fallback doubles the latency of every legacy asset served.
-
-`GetFileSizeAsync` (line 57) has the same shape.
-
-Worth checking while fixing: whether `assetStore.DownloadAsync` can write bytes into the target
-stream before discovering the file is missing. If it can, the retry appends to a partially
-written response body rather than replacing it.
-
-**Fix:** probe once per asset and remember which naming scheme it uses, or migrate the names so
-the fallback can be deleted.
-
----
-
-## S4 — Low
-
-### 29. Removing items while iterating a `JsonArray` is quadratic
-`backend/src/Squidex.Domain.Apps.Core.Operations/ConvertContent/ContentConverter.cs:159,185`
-
-```csharp
-for (int i = 0; i < array.Count; i++)
-{
-    ...
-    if (removed)
-    {
-        array.RemoveAt(i);
-        i--;
-    }
-}
-```
-
-`JsonArray` derives from `List<JsonValue>`, so each `RemoveAt` shifts every following element.
-Dropping *k* items from an *n*-element array costs O(n·k). It only bites when many items are
-removed at once — an array or components field whose entries reference deleted schemas or
-components — but that is exactly the case where the array is large.
-
-**Fix:** compact in a single pass (write index) rather than removing in place.
-
----
-
-### 30. `stream.ToArray()` copies straight back out of the pooled buffer
-`backend/src/Squidex.Domain.Apps.Entities/Assets/Transformations.cs:79`
-
-`GetTextAsync` downloads into a `DefaultPools.MemoryStream` (a
-`RecyclableMemoryStreamManager`) and then calls `stream.ToArray()`, which allocates a fresh
-array of the full file and copies the pooled buffer into it — defeating the point of the pool.
-`RecyclableMemoryStream` documents `ToArray` as the thing not to call for this reason.
-
-Bounded at 4 MB by `MaxSize`, and this runs from scripts rather than the main content path,
-which is why it ranks last — but 4 MB straight into the large object heap per call is still
-worth avoiding.
-
-**Fix:** `GetBuffer()` with the stream length, or the `Encoding.GetString(ReadOnlySpan<byte>)`
-overload over the stream's sequence.
-
----
-
 ## Suggested order of attack
 
 1. **Item 23** — full-text paging. Really a correctness fix that happens to also be faster:
@@ -176,7 +101,9 @@ overload over the stream's sequence.
    engine must not carry state from one script into the next. **Profile before writing it**:
    the estimate that engine construction dominates a scripted content list is read off the
    loops, not taken from a trace.
-3. **Items 28, 29, 30** — narrow, conditional or off the main path.
+
+Nothing else is outstanding. Everything cheap, every correctness-shaped finding and
+everything in the stability category is closed.
 
 ---