Studio 3.1.2: a function and a procedure are objects, not labels

Docs/KnownIssues.md

@@ -1620,6 +1620,12 @@ Studio’s Plan panel does - draws the wrong tree, faithfully. **The renderer is
 
 ## 27. Studio: a function or a procedure can only be refreshed
 
+> **FIXED in Studio 3.1.2.** A routine is offered *View definition* and *Drop*, the definition is
+> the whole `CREATE` assembled from `ROUTINES` and `PARAMETERS`, and - found beside it -
+> **`Dump Database…` had been writing no routines at all**, so a dumped database restored without
+> its functions and said nothing. `ARoutineIsAnObjectLikeAnyOtherTests` drops each routine and runs
+> the definition Studio wrote back into the database.
+>
 > Studio 3.1.1. A gap rather than a wrong answer.
 
 The tree’s context menu offers a routine exactly one item, `Refresh`. The engine has
@@ -1628,7 +1634,7 @@ inspector on the right shows it - so both *View definition* and *Drop* are possi
 offered. Every other kind of object in the tree was given its own menu in 3.1.0; routines were not
 included.
 
-Until they are: drop a routine by running the statement in a query tab.
+Until 3.1.2 the way to drop a routine was to run the statement in a query tab.
 
 ---
 ## Verifying a fix

Tools/OutWit.Database.Studio.Tests/ViewModels/ARoutineIsAnObjectLikeAnyOtherTests.cs

@@ -0,0 +1,299 @@
+using OutWit.Database.Studio.Models;
+using OutWit.Database.Studio.Tests.Helpers;
+
+namespace OutWit.Database.Studio.Tests.ViewModels;
+
+/// <summary>
+/// A function and a procedure are objects in the tree, not labels in it.
+/// </summary>
+/// <remarks>
+/// <para>
+/// <b>Known issue 27, found by clicking through 3.1.1.</b> The sixth folder arrived with WS-21 and
+/// nothing else did: a routine's context menu offered one item, <i>Refresh</i>, while the engine has
+/// had <c>DROP FUNCTION</c> and <c>DROP PROCEDURE</c> since phase 9d and the catalogue already
+/// carried the body - the inspector on the right was showing it.
+/// </para>
+/// <para>
+/// <b>And the dump had the same hole, which is worse than a missing menu item</b>: it wrote views,
+/// indexes and triggers, so a database with routines dumped to a script that restored without them
+/// and said nothing about it.
+/// </para>
+/// <para>
+/// The assertion that matters here is not "a definition is produced" but that the definition RUNS
+/// BACK: the routine is dropped and the text Studio wrote is executed, and the routine is there
+/// again and still answers.
+/// </para>
+/// </remarks>
+[TestFixture]
+public class ARoutineIsAnObjectLikeAnyOtherTests
+{
+    #region Constants
+
+    private const string FUNCTION = "CREATE FUNCTION DiscountedTotal(Amount DECIMAL(18,2), Percent INT) "
+        + "RETURNS DECIMAL(18,2) AS BEGIN RETURN (Amount - ((Amount * Percent) / 100)); END";
+
+    private const string PROCEDURE = "CREATE PROCEDURE ArchiveOld() AS BEGIN "
+        + "UPDATE Orders SET Status = 'archived' WHERE Status = 'new'; END";
+
+    #endregion
+
+    #region Fields
+
+    private StudioFixture m_studio = null!;
+
+    #endregion
+
+    #region Setup
+
+    [SetUp]
+    public async Task SetUp()
+    {
+        m_studio = await StudioFixture.CreateAsync();
+
+        await m_studio.Database.ExecuteNonQueryAsync(FUNCTION);
+        await m_studio.Database.ExecuteNonQueryAsync(PROCEDURE);
+
+        await m_studio.Explorer.RefreshAsync();
+    }
+
+    [TearDown]
+    public async Task TearDown()
+    {
+        await m_studio.DisposeAsync();
+    }
+
+    #endregion
+
+    #region What the tree offers
+
+    [Test]
+    public void ARoutineIsOfferedItsDefinitionAndItsRemovalTest()
+    {
+        Select("DiscountedTotal");
+
+        var explorer = m_studio.Explorer;
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(explorer.ShowsViewDefinition, Is.True, "the catalogue holds its body");
+            Assert.That(explorer.ShowsDrop, Is.True, "and the engine has DROP FUNCTION");
+
+            Assert.That(explorer.CanViewDefinition, Is.True);
+            Assert.That(explorer.CanDropObject, Is.True);
+
+            // What a routine still does NOT have, so that this is not a licence to offer everything.
+            Assert.That(explorer.ShowsRename, Is.False, "there is no ALTER FUNCTION in this language");
+            Assert.That(explorer.ShowsEditData, Is.False);
+            Assert.That(explorer.ShowsBrowseData, Is.False);
+        });
+    }
+
+    [Test]
+    public void TheTreeKnowsWhichOfTheTwoEachOneIsTest()
+    {
+        Assert.Multiple(() =>
+        {
+            Assert.That(Node("DiscountedTotal").IsFunction, Is.True);
+            Assert.That(Node("ArchiveOld").IsFunction, Is.False);
+
+            // A fact rather than a rendering: the label is what the fact is drawn as, not the other
+            // way round.
+            Assert.That(Node("DiscountedTotal").Detail, Does.StartWith("function"));
+            Assert.That(Node("ArchiveOld").Detail, Is.EqualTo("procedure"));
+        });
+    }
+
+    #endregion
+
+    #region The definition runs back
+
+    [Test]
+    public async Task TheDefinitionOfAFunctionRunsBackIntoTheDatabaseTest()
+    {
+        await ItRunsBackAsync("DiscountedTotal", isFunction: true);
+    }
+
+    [Test]
+    public async Task TheDefinitionOfAProcedureRunsBackIntoTheDatabaseTest()
+    {
+        await ItRunsBackAsync("ArchiveOld", isFunction: false);
+    }
+
+    /// <summary>
+    /// A function's parameters and return type are part of it. A definition without them parses and
+    /// creates a DIFFERENT routine, which is the failure this case exists to catch.
+    /// </summary>
+    [Test]
+    public async Task TheDefinitionCarriesTheParametersAndTheReturnTypeTest()
+    {
+        var definition = await m_studio.Database.GetRoutineDefinitionAsync("DiscountedTotal");
+
+        Assert.That(definition, Is.Not.Null);
+
+        // The SIGNATURE, not the whole text: the parameter names also appear in the body, so
+        // «the definition contains Amount» passes on a definition with no parameters at all -
+        // measured, by removing them.
+        var signature = definition![..definition.IndexOf("RETURNS", StringComparison.Ordinal)];
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(signature, Does.Contain("Amount"));
+            Assert.That(signature, Does.Contain("Percent"));
+            Assert.That(signature, Does.Contain("DECIMAL(18,2)"),
+                "a parameter carries its type WITH its precision - DECIMAL alone restores a "
+                + "routine that rounds differently from the one in the database");
+
+            Assert.That(definition, Does.Contain("RETURNS DECIMAL"), "and the function its return type");
+        });
+    }
+
+    #endregion
+
+    #region Dropping one
+
+    [Test]
+    public async Task DroppingAFunctionTakesItOutOfTheDatabaseAndTheTreeTest()
+    {
+        Select("DiscountedTotal");
+
+        m_studio.Confirmations.AllowDestructive = true;
+
+        await StudioFixture.PressAsync(m_studio.Explorer.DropObjectCommand);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(Routines(), Does.Not.Contain("DiscountedTotal"));
+            Assert.That(Walk().Any(node => node.Name == "DiscountedTotal"), Is.False,
+                "and the tree was refreshed");
+        });
+    }
+
+    [Test]
+    public async Task DroppingAProcedureTakesItOutTooTest()
+    {
+        Select("ArchiveOld");
+
+        m_studio.Confirmations.AllowDestructive = true;
+
+        await StudioFixture.PressAsync(m_studio.Explorer.DropObjectCommand);
+
+        Assert.That(Routines(), Does.Not.Contain("ArchiveOld"));
+    }
+
+    /// <summary>
+    /// The other direction: a refused question leaves the routine alone. DROP PROCEDURE and DROP
+    /// FUNCTION are different statements, and a menu that asks and drops anyway would be worse than
+    /// one that never offered.
+    /// </summary>
+    [Test]
+    public async Task ARefusedQuestionLeavesTheRoutineAloneTest()
+    {
+        Select("DiscountedTotal");
+
+        m_studio.Confirmations.AllowDestructive = false;
+
+        await StudioFixture.PressAsync(m_studio.Explorer.DropObjectCommand);
+
+        Assert.That(Routines(), Does.Contain("DiscountedTotal"));
+    }
+
+    /// <summary>
+    /// The line under the tree counts what the tree draws. It named five folders while the tree
+    /// drew six, so a database whose only objects are routines was summarised as empty.
+    /// </summary>
+    [Test]
+    public async Task TheSummaryCountsTheSixthFolderTest()
+    {
+        await m_studio.Explorer.RefreshAsync();
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(m_studio.MainWindow.StatusText, Does.Contain("2 routines"));
+
+            // CONTROL: the same line, still naming what it named before.
+            Assert.That(m_studio.MainWindow.StatusText, Does.Contain("1 trigger"));
+        });
+    }
+
+    #endregion
+
+    #region The dump
+
+    [Test]
+    public async Task ADumpCarriesTheRoutinesTest()
+    {
+        var script = await Studio.Services.DatabaseDump.WriteAsync(
+            m_studio.Database, new Studio.Services.DumpOptions());
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(script, Does.Contain("CREATE FUNCTION"), "a dump without them restores without them");
+            Assert.That(script, Does.Contain("DiscountedTotal"));
+
+            Assert.That(script, Does.Contain("CREATE PROCEDURE"));
+            Assert.That(script, Does.Contain("ArchiveOld"));
+
+            // CONTROL: the dump is the real one, with the objects that were already written.
+            Assert.That(script, Does.Contain("CREATE TABLE"));
+            Assert.That(script, Does.Contain("CREATE TRIGGER"));
+        });
+    }
+
+    #endregion
+
+    #region Tools
+
+    private async Task ItRunsBackAsync(string name, bool isFunction)
+    {
+        var definition = await m_studio.Database.GetRoutineDefinitionAsync(name);
+
+        Assert.That(definition, Is.Not.Null, $"{name} has a definition");
+
+        await m_studio.Database.ExecuteNonQueryAsync(
+            $"DROP {(isFunction ? "FUNCTION" : "PROCEDURE")} {name}");
+
+        Assume.That(Routines(), Does.Not.Contain(name), "it is gone before the definition is run");
+
+        await m_studio.Database.ExecuteNonQueryAsync(definition!);
+
+        Assert.That(Routines(), Does.Contain(name),
+            "the definition Studio wrote does not restore the routine:" + Environment.NewLine + definition);
+    }
+
+    private IReadOnlyList<string> Routines()
+    {
+        return m_studio.Database.GetRoutinesAsync().GetAwaiter().GetResult()
+            .Select(routine => routine.Name)
+            .ToList();
+    }
+
+    private DatabaseNode Node(string name)
+    {
+        var node = Walk().FirstOrDefault(candidate =>
+            candidate.NodeType == DatabaseNodeType.Routine && candidate.Name == name);
+
+        Assert.That(node, Is.Not.Null, $"the tree has a routine called {name}");
+
+        return node!;
+    }
+
+    private void Select(string name)
+    {
+        m_studio.Explorer.SelectedNode = Node(name);
+    }
+
+    private IEnumerable<DatabaseNode> Walk()
+    {
+        return m_studio.Explorer.Nodes.SelectMany(Flatten);
+    }
+
+    private static IEnumerable<DatabaseNode> Flatten(DatabaseNode node)
+    {
+        yield return node;
+
+        foreach (var child in node.Children.SelectMany(Flatten))
+            yield return child;
+    }
+
+    #endregion
+}

Tools/OutWit.Database.Studio/CHANGELOG.md

@@ -3,6 +3,44 @@
 Studio is versioned separately from the WitDatabase engine and released under its own `studio-v*` tag.
 The engine's changelog is `/CHANGELOG.md`.
 
+## 3.1.2
+
+**Engine: 14.0.1**, and that is the first reason for this release. Two planner defects were found
+by using Studio and fixed in the engine the same day: a join condition written `ON right.x =
+left.y` was refused outright, and `EXPLAIN` gave the right input’s child the wrong parent - so the
+Plan panel drew the wrong tree faithfully. Studio needed no change for either; it needed the
+engine.
+
+### A function and a procedure are objects, not labels
+
+The sixth folder arrived with WS-21 and nothing else did: a routine’s context menu offered one
+item, *Refresh*. It now offers **View definition** and **Drop**, like every other kind of object
+in the tree - the engine has had `DROP FUNCTION` and `DROP PROCEDURE` since phase 9d, and the
+catalogue already carried the body the inspector was showing.
+
+The definition is the whole `CREATE`, assembled from `INFORMATION_SCHEMA.ROUTINES` and
+`PARAMETERS`: a body without its parameters and return type parses and creates a DIFFERENT
+routine. A routine the catalogue cannot render is NAMED rather than half-written, which is the
+rule a view and a trigger already followed.
+
+### And the dump was losing them
+
+Worse than the missing menu item, and found beside it: **`Dump Database…` wrote views, indexes and
+triggers, and no routines at all.** A database with functions dumped to a script that restored
+without them and said nothing. The dump now carries them, and names one it cannot render.
+
+The test that matters is not that a definition appears but that it **runs back**: each routine is
+dropped and the text Studio wrote is executed, and the routine is there again.
+
+### And the line under the tree counts six folders
+
+It named five - tables, views, indexes, triggers, sequences - while the tree has drawn six since
+WS-21, so a database whose only objects are routines was summarised as having nothing in it.
+
+### Known issues
+
+Issue 27 of [Docs/KnownIssues.md](../../Docs/KnownIssues.md) is this, and is marked fixed. Issues
+25 and 26 went with engine 14.0.1.
 ## 3.1.1
 
 **Four corrections, three of them to things 3.1.0 itself broke.** They were found the day 3.1.0

Tools/OutWit.Database.Studio/Models/DatabaseNode.cs

@@ -67,6 +67,19 @@ public override DatabaseNode Clone()
     [Notify]
     public bool IsExpanded { get; set; }
 
+    /// <summary>
+    /// For a <see cref="DatabaseNodeType.Routine"/>: whether it is a function rather than a
+    /// procedure. Null for every other kind of node.
+    /// </summary>
+    /// <remarks>
+    /// One node type covers both because the tree draws them in one folder, and two DDL keywords sit
+    /// behind it - <c>DROP FUNCTION</c> and <c>DROP PROCEDURE</c> are different statements, and
+    /// "function deleted" and "procedure deleted" are different sentences in a language with cases.
+    /// The alternative, reading it back out of the label, is a rendering answering a question about
+    /// the schema.
+    /// </remarks>
+    public bool? IsFunction { get; set; }
+
     /// <summary>
     /// A stand-in child, there so that the node draws an expander.
     /// </summary>