Fully work around MySQL 5.7's lack of the DROP CONSTRAINT statement (#…

…575)

* The work done in #522 added the ability to drop foreign keys properly when using a MySQL 5.7 database, enabling Fluent to generate the necessary DROP FOREIGN KEY syntax to make up for the old version's lack of the more standard and more flexible DROP CONSTRAINT statement. However, this still required users to specify the complete and current foreign key constraint definition at the time of deletion so Fluent could work out the correct "normalized" constraint identifier. These changes make it possible to also drop a foreign key constraint by name, requiring only the knowledge that it is, in fact, a foreign key.

.github/workflows/api-docs.yml

@@ -11,4 +11,4 @@ jobs:
      with:
        package_name: fluent-kit
        modules: FluentKit
-       pathsToInvalidate: /fluentkit
+       pathsToInvalidate: /fluentkit/*

.github/workflows/test.yml

@@ -85,10 +85,12 @@ jobs:
           repository: vapor/${{ matrix.dependent }}
           path: ${{ matrix.dependent }}
           ref: ${{ matrix.ref }}
-      - name: Use local package
-        run: swift package --package-path ${{ matrix.dependent }} edit fluent-kit --path fluent-kit
-      - name: Run tests
-        run: swift test --package-path ${{ matrix.dependent }}
+      - name: Use local package and run tests
+        env:
+          DEPENDENT: ${{ matrix.dependent }}
+        run: |
+          swift package --package-path ${DEPENDENT} edit fluent-kit --path fluent-kit
+          swift test --package-path ${DEPENDENT}
 
   # also serves as code coverage baseline update
   unit-tests:

Sources/FluentBenchmark/Tests/SchemaTests.swift

@@ -153,16 +153,22 @@ extension FluentBenchmarker {
 
                 .field("catid", .uuid)
                 .foreignKey(["catid"], references: Category.schema, [.id], onDelete: .noAction, onUpdate: .noAction)
+                .foreignKey(["catid"], references: Category.schema, [.id], onDelete: .noAction, onUpdate: .noAction, name: "second_fkey")
                 .unique(on: "catid")
+                .unique(on: "id", name: "second_ukey")
 
                 .create().wait()
 
             if (self.database as! SQLDatabase).dialect.alterTableSyntax.allowsBatch {
                 try self.database.schema("normal_constraints")
                     // Test `DROP FOREIGN KEY` (MySQL) or `DROP CONSTRAINT` (Postgres)
                     .deleteConstraint(.constraint(.foreignKey([.key("catid")], Category.schema, [.key(.id)], onDelete: .noAction, onUpdate: .noAction)))
+                    // Test name-based `DROP FOREIGN KEY` (MySQL)
+                    .deleteForeignKey(name: "second_fkey")
                     // Test `DROP KEY` (MySQL) or `DROP CONSTRAINT` (Postgres)
                     .deleteUnique(on: "catid")
+                    // Test name-based `DROP KEY` (MySQL) or `DROP CONSTRAINT` (Postgres)
+                    .deleteConstraint(name: "second_ukey")
 
                     .update().wait()
             }

Sources/FluentKit/Schema/DatabaseSchema.swift

@@ -143,6 +143,35 @@ public struct DatabaseSchema {
         case constraint(ConstraintAlgorithm)
         case name(String)
         case custom(Any)
+
+        /// Deletion specifier for an explicitly-named constraint known to be a referential constraint.
+        /// 
+        /// Certain old versions of certain databases (I'm looking at you, MySQL 5.7...) do not support dropping
+        /// a `FOREIGN KEY` constraint by name without knowing ahead of time that it is a foreign key. When an
+        /// unfortunate user runs into this, the options are:
+        ///
+        /// - Trap the resulting error and retry. This is exceptionally awkward to handle automatically, and can
+        ///   lead to retry loops if multiple deletions are specified in a single operation.
+        /// - Force the user to issue a raw SQL query instead. This is obviously undesirable.
+        /// - Force an upgrade of the underlying database. No one should be using MySQL 5.7 anymore, but Fluent
+        ///   recognizes that this isn't always under the user's control.
+        /// - Require the user to specify the deletion with ``constraint(_:)``, providing the complete, accurate,
+        ///   and current definition of the foreign key. This is information the user may not even know, and
+        ///   certainly should not be forced to repeat here.
+        /// - Provide a means for the user to specify that a given constraint to be deleted by name is known to be
+        ///   a foreign key. For databases which _don't_ suffer from this particular syntactical issue (so, almost
+        ///   everything), this is exactly the same as specifying ``name(_:)``.
+        ///
+        /// In short, this is the marginal best choice from a list of really bad choices - an ugly, backhanded
+        /// workaround for MySQL 5.7 users.
+        ///
+        /// > Note: A static method is provided rather than a new `enum` case because adding new cases to a public
+        /// > enum without library evolution enabled (which only the stdlib can do) is a source compatibility break
+        /// > and requires a `semver-major` version bump. This rule is often ignored, but ignoring it doesn't make
+        /// > the problem moot.
+        public static func namedForeignKey(_ name: String) -> ConstraintDelete {
+            self.custom(_ForeignKeyByNameExtension(name: name))
+        }
     }
 
     public var action: Action
@@ -167,3 +196,15 @@ public struct DatabaseSchema {
         self.exclusiveCreate = true
     }
 }
+
+extension DatabaseSchema.ConstraintDelete {
+    /// For internal use only.
+    ///
+    /// Do not use this type directly; it's only public because FluentSQL needs to be able to get at it.
+    /// The use of `@_spi` will be replaced with the `package` modifier once a suitable minimum version
+    /// of Swift becomes required.
+    @_spi(FluentSQLSPI)
+    public/*package*/ struct _ForeignKeyByNameExtension {
+        public/*package*/ let name: String
+    }
+}

Sources/FluentKit/Schema/SchemaBuilder.swift

@@ -60,16 +60,22 @@ public final class SchemaBuilder {
 
     @discardableResult
     public func deleteUnique(on fields: FieldKey...) -> Self {
-        self.schema.deleteConstraints.append(.constraint(
-            .unique(fields: fields.map { .key($0) })
-        ))
-        return self
+        self.deleteConstraint(.constraint(.unique(fields: fields.map { .key($0) })))
+    }
+
+    /// Delete a FOREIGN KEY constraint with the given name.
+    ///
+    /// This method allows correctly handling referential constraints with custom names when using MySQL 5.7
+    /// without being forced to also know the full definition of the constraint at the time of deletion. See
+    /// ``DatabaseSchema/ConstraintDelete/namedForeignKey(_:)`` for a more complete discussion.
+    @discardableResult
+    public func deleteForeignKey(name: String) -> Self {
+        self.deleteConstraint(.namedForeignKey(name))
     }
 
     @discardableResult
     public func deleteConstraint(name: String) -> Self {
-        self.schema.deleteConstraints.append(.name(name))
-        return self
+        self.deleteConstraint(.name(name))
     }
 
     @discardableResult
@@ -145,7 +151,7 @@ public final class SchemaBuilder {
 
     @discardableResult
     public func deleteField(_ name: FieldKey) -> Self {
-        return self.deleteField(.key(name))
+        self.deleteField(.key(name))
     }
 
     @discardableResult

Sources/FluentSQL/SQLSchemaConverter.swift

@@ -1,5 +1,5 @@
 import SQLKit
-import FluentKit
+@_spi(FluentSQLSPI) import FluentKit
 
 public protocol SQLConverterDelegate {
     func customDataType(_ dataType: DatabaseSchema.DataType) -> SQLExpression?
@@ -112,8 +112,11 @@ public struct SQLSchemaConverter {
             let name = self.constraintIdentifier(algorithm, table: table)
             return SQLDropTypedConstraint(name: SQLIdentifier(name), algorithm: algorithm)
         case .name(let name):
-            return SQLDropTypedConstraint(name: SQLIdentifier(name), algorithm: .sql(raw: ""))
+            return SQLDropTypedConstraint(name: SQLIdentifier(name), algorithm: .custom(""))
         case .custom(let any):
+            if let fkeyExt = any as? DatabaseSchema.ConstraintDelete._ForeignKeyByNameExtension {
+                return SQLDropTypedConstraint(name: SQLIdentifier(fkeyExt.name), algorithm: .foreignKey([], "", [], onDelete: .noAction, onUpdate: .noAction))
+            }
             return custom(any)
         }
     }