Retire the old mapper as the last subscription leaves it

SW.Bitween.Api/Resources/Adapters/RetiringMapper.cs

@@ -0,0 +1,69 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.EntityFrameworkCore;
+using SW.Bitween.Domain;
+using SW.Bitween.NativeAdapters;
+
+namespace SW.Bitween.Resources.Adapters;
+
+/// <summary>
+/// The Scriban mapper the native mapper replaced, offered only while something still runs on it.
+/// </summary>
+/// <remarks>
+/// <para>
+/// This is how the old mapper retires. A subscription already using it is untouched — it keeps
+/// running, and it keeps its own editor — but the mapper stops being offered as a choice once the
+/// last subscription has moved across. The list of mappers then shrinks by itself as the migration
+/// finishes, rather than when someone remembers to delete the adapter.
+/// </para>
+/// <para>
+/// One-way on purpose: past that point there is no picking it again. Which is why the question of
+/// what counts as "in use" is answered generously — an inactive subscription still holds its
+/// template and can be switched back on, so it counts. Withholding a mapper a subscription is
+/// still saved with would leave that subscription's own mapper missing from the list its picker
+/// draws from, which reads as no mapper chosen at all.
+/// </para>
+/// <para>
+/// The same idea already exists a level down: <see cref="NativeAdapterDiscoveryService"/> withholds
+/// the Rebex-backed adapters while no license key is set, so that a picker never offers something
+/// that could only fail. This withholds one that would only lengthen a migration.
+/// </para>
+/// </remarks>
+internal static class RetiringMapper
+{
+    /// <summary>The adapter id, which is the class name the discovery service reports.</summary>
+    public const string Id = nameof(NativeJSONMapper);
+
+    private static readonly string Lowered = Id.ToLowerInvariant();
+
+    /// <summary>
+    /// <paramref name="adapters"/> without the retiring mapper, unless a subscription still names it.
+    /// </summary>
+    /// <remarks>
+    /// The database is only asked when the retiring mapper is in the list to begin with, so the
+    /// other adapter kinds — receivers, handlers, validators — cost nothing.
+    /// </remarks>
+    public static async Task<List<string>> ExceptRetiring(
+        this IEnumerable<string> adapters, BitweenDbContext dbContext)
+    {
+        var listed = adapters.ToList();
+
+        if (!listed.Any(Matches)) return listed;
+
+        // Compared lowered rather than as stored: the lookup that resolves a mapper at run time is
+        // itself case-insensitive, so a row whose casing drifted is still a subscription running on
+        // this mapper, and answering "no" to that would take the mapper away from it.
+        var stillInUse = await dbContext.Set<Subscription>()
+            .AnyAsync(s => s.MapperId != null && s.MapperId.ToLower() == Lowered);
+
+        if (stillInUse) return listed;
+
+        listed.RemoveAll(Matches);
+        return listed;
+    }
+
+    private static bool Matches(string adapter) =>
+        adapter.Equals(Id, StringComparison.OrdinalIgnoreCase);
+}

SW.Bitween.Api/Resources/Adapters/Search.cs

@@ -31,7 +31,8 @@ public async Task<object> Handle(AdapterSearchRequest request)
             await _requestContext.EnsurePermission(_dbContext, Model.Permissions.Subscriptions.View);
 
             // Get native adapters first
-            var nativeAdapters = _nativeAdapterDiscovery.GetNativeAdapters(request.Prefix).ToList();
+            var nativeAdapters = await _nativeAdapterDiscovery.GetNativeAdapters(request.Prefix)
+                .ExceptRetiring(_dbContext);
 
             // Get external adapters from storage
             var cloudFilesList =

SW.Bitween.Api/Resources/Adapters/SearchVersioned.cs

@@ -35,7 +35,8 @@ public async Task<object> Handle(AdapterSearchRequest request)
             var index = _serverlessOptions.AdapterRemotePath.Length + 1;
 
             // Get native adapters first (they don't have versions)
-            var nativeAdapters = _nativeAdapterDiscovery.GetNativeAdapters(request.Prefix)
+            var nativeAdapters = (await _nativeAdapterDiscovery.GetNativeAdapters(request.Prefix)
+                    .ExceptRetiring(_dbContext))
                 .Select(key => new
                 {
                     Key = key,

SW.Bitween.IntegrationTests/Tests/RetiringMapperTests.cs

@@ -0,0 +1,173 @@
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.DependencyInjection;
+using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
+using SW.Bitween.Domain;
+using SW.Bitween.IntegrationTests.Fixtures;
+using SW.Bitween.Model;
+using Xunit;
+
+namespace SW.Bitween.IntegrationTests.Tests;
+
+/// <summary>
+/// The old mapper is offered only while a subscription still uses it.
+/// </summary>
+/// <remarks>
+/// Both directions are asserted in one test on purpose. The answer is a fact about the whole
+/// database, so a test that only asserted one direction would be passing or failing on whatever
+/// the other tests in this collection happened to leave behind.
+/// </remarks>
+[Collection("Bitween")]
+public class RetiringMapperTests
+{
+    private const string OldMapper = "NativeJSONMapper";
+    private const string NewMapper = "NativeMapper";
+
+    private readonly BitweenFixture _fixture;
+
+    public RetiringMapperTests(BitweenFixture fixture)
+    {
+        _fixture = fixture;
+    }
+
+    private static int _seq;
+    private static string Unique(string prefix) => $"{prefix}-{Interlocked.Increment(ref _seq)}";
+
+    /// <summary>The mapper ids the adapter catalog offers, as the picker would list them.</summary>
+    private async Task<List<string>> ListedMappers()
+    {
+        await using var scope = _fixture.CreateScope();
+        scope.Superuser();
+        var handler = ActivatorUtilities
+            .CreateInstance<Resources.Adapters.SearchVersioned>(scope.ServiceProvider);
+
+        var result = await handler.Handle(new AdapterSearchRequest { Prefix = "mappers" });
+
+        // The handler returns anonymous types; going through JSON reads them the way the browser
+        // does rather than through reflection.
+        return JArray.Parse(JsonConvert.SerializeObject(result))
+            .Select(row => row["Key"]!.ToString())
+            .ToList();
+    }
+
+    /// <summary>A subscription saved with <paramref name="mapperId"/>.</summary>
+    private async Task<int> SubscriptionUsing(string mapperId)
+    {
+        await using var scope = _fixture.CreateScope();
+        var db = scope.ServiceProvider.GetRequiredService<BitweenDbContext>();
+
+        var document = new Document(null, Unique("Retiring doc"), DocumentFormat.Json);
+        db.Set<Document>().Add(document);
+        var partner = new Partner(Unique("Retiring partner"));
+        db.Set<Partner>().Add(partner);
+        await db.SaveChangesAsync();
+
+        var subscription = new Subscription(
+            Unique("Retiring sub"), document.Id, SubscriptionType.Internal, partner.Id)
+        {
+            MapperId = mapperId,
+        };
+        db.Set<Subscription>().Add(subscription);
+        await db.SaveChangesAsync();
+
+        return subscription.Id;
+    }
+
+    private async Task SetMapper(int subscriptionId, string? mapperId)
+    {
+        await using var scope = _fixture.CreateScope();
+        var db = scope.ServiceProvider.GetRequiredService<BitweenDbContext>();
+
+        var subscription = await db.Set<Subscription>().SingleAsync(s => s.Id == subscriptionId);
+        subscription.MapperId = mapperId;
+        await db.SaveChangesAsync();
+    }
+
+    /// <summary>
+    /// Every subscription this database already holds on the old mapper, moved off it.
+    /// </summary>
+    /// <remarks>
+    /// Safe to do to a shared database: the only other tests that put a subscription on the old
+    /// mapper build a fresh one inside each test, so none of them reads a row left by an earlier
+    /// one. A collection's tests also run one at a time, so nothing is arranging while this runs.
+    /// </remarks>
+    private async Task<List<int>> ParkExistingUsers()
+    {
+        await using var scope = _fixture.CreateScope();
+        var db = scope.ServiceProvider.GetRequiredService<BitweenDbContext>();
+
+        var existing = await db.Set<Subscription>()
+            .Where(s => s.MapperId != null && s.MapperId.ToLower() == "nativejsonmapper")
+            .ToListAsync();
+
+        foreach (var subscription in existing) subscription.MapperId = null;
+        await db.SaveChangesAsync();
+
+        return existing.Select(s => s.Id).ToList();
+    }
+
+    [Fact]
+    public async Task The_old_mapper_is_listed_only_while_a_subscription_still_uses_it()
+    {
+        await ParkExistingUsers();
+
+        // Nothing on it: it is gone from the picker, and the new mapper is still there — a filter
+        // that took out more than it should would pass an assertion about the old one alone.
+        var withoutUsers = await ListedMappers();
+        Assert.DoesNotContain(OldMapper, withoutUsers);
+        Assert.Contains(NewMapper, withoutUsers);
+
+        // One subscription on it anywhere in the database is enough to bring it back everywhere.
+        var subscriptionId = await SubscriptionUsing(OldMapper);
+        Assert.Contains(OldMapper, await ListedMappers());
+
+        // And moving that last one across takes it away again, which is the whole point: the
+        // migration finishing is what retires the mapper.
+        await SetMapper(subscriptionId, NewMapper);
+        Assert.DoesNotContain(OldMapper, await ListedMappers());
+    }
+
+    [Fact]
+    public async Task An_inactive_subscription_still_counts_as_using_it()
+    {
+        await ParkExistingUsers();
+
+        await using (var scope = _fixture.CreateScope())
+        {
+            var db = scope.ServiceProvider.GetRequiredService<BitweenDbContext>();
+            var document = new Document(null, Unique("Retiring inactive doc"), DocumentFormat.Json);
+            db.Set<Document>().Add(document);
+            var partner = new Partner(Unique("Retiring inactive partner"));
+            db.Set<Partner>().Add(partner);
+            await db.SaveChangesAsync();
+
+            var subscription = new Subscription(
+                Unique("Retiring inactive sub"), document.Id, SubscriptionType.Internal, partner.Id)
+            {
+                MapperId = OldMapper,
+                Inactive = true,
+            };
+            db.Set<Subscription>().Add(subscription);
+            await db.SaveChangesAsync();
+        }
+
+        // It holds a template and can be switched back on, so it keeps the mapper listed. Withheld
+        // instead, this subscription's own picker would show nothing selected.
+        Assert.Contains(OldMapper, await ListedMappers());
+    }
+
+    [Fact]
+    public async Task Casing_that_drifted_still_counts()
+    {
+        await ParkExistingUsers();
+        await SubscriptionUsing("nativejsonmapper");
+
+        // The run-time lookup that resolves a mapper matches case-insensitively, so a row stored
+        // this way is a subscription running on the old mapper and has to keep it listed.
+        Assert.Contains(OldMapper, await ListedMappers());
+    }
+}

SW.Bitween.Web/ClientApp/e2e/native-mapper.spec.ts

@@ -164,6 +164,14 @@ test("stored rules survive a switch to a list-shaped output and back", async ({
 test("choosing the new mapper offers its editor, and the old mapper keeps its own", async ({
   page,
 }) => {
+  // The old mapper is listed only while a subscription somewhere still uses it, so this
+  // test has to put one on it before it can pick it. A separate subscription rather than
+  // the one under test: pinning that one would give it a mapper already, and the first
+  // thing asserted below is that it has none.
+  await writeMapperProperties(await createSubscription(page), "NativeJSONMapper", {
+    ScribanTemplate: "{}",
+  });
+
   const subscriptionId = await createSubscription(page);
 
   await page.goto(`subscriptions/${subscriptionId}`);