Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the specified the index name above the default name #60

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Use the specified the index name above the default name #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4faa3c9
Use the specified the index name above the default name
Joannis Jun 11, 2023
fcee321
Add a test that this Fluent Driver supports multiple unique indexes
Joannis Jun 11, 2023
76578ce
Check that the `name` of an index isn't an empty string
Joannis Jun 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions Sources/FluentMongoDriver/MongoDB+Schema.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,18 @@ extension FluentMongoDatabase {
var futures = [EventLoopFuture<Void>]()

nextConstraint: for constraint in schema.createConstraints {
guard case .constraint(let algorithm, _) = constraint else {
guard case .constraint(let algorithm, let name) = constraint else {
continue nextConstraint
}

let indexName: String
if let name = name, !name.isEmpty {
indexName = name
} else {
// this could be `composite` for the `.compositeIdentifier` case. But keeping it as `unique` keeps backwards compatibility
indexName = "unique"
}

switch algorithm {
case .unique(let fields), .compositeIdentifier(let fields):
let indexKeys = try fields.map { field -> String in
Expand All @@ -38,7 +47,7 @@ extension FluentMongoDatabase {
}

var index = CreateIndexes.Index(
named: "unique",
named: indexName,
keys: keys
)

Expand Down
21 changes: 21 additions & 0 deletions Tests/FluentMongoDriverTests/FluentMongoDriverTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ final class FluentMongoDriverTests: XCTestCase {
XCTFail("\(error)")
}
}

func testDuplicateIndexNames() throws {
XCTAssertNoThrow(try DuplicateIndexMigration().prepare(on: self.db).wait())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this won't actually test anything; the original reporter said in Discord that only one index gets created, not that an error is thrown. Is there a way to verify the indexes were actually added? Or alternatively, can we either 1) check for duplicates and throw an error in the schema logic, or 2) ask MongoDB to reject duplicates somehow?

}

func testDate() throws {
let range = DateRange(from: Date(), to: Date())
Expand Down Expand Up @@ -278,6 +282,23 @@ final class FluentMongoDriverTests: XCTestCase {
}
}

struct DuplicateIndexMigration: Migration {
func prepare(on database: Database) -> EventLoopFuture<Void> {
database
.schema("index-tests")
Joannis marked this conversation as resolved.
Show resolved Hide resolved
.id()
.field("name", .string, .required)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.field("name", .string, .required)
.compositeIdentifier(over: "name", "email")
.field("name", .string, .required)

In light of my remarks above (and since it'll just translate to an additional unique index for Mongo)

.field("email", .string, .required)
.unique(on: "name", name: "name")
.unique(on: "email", name: "email")
.create()
}

func revert(on database: FluentKit.Database) -> NIOCore.EventLoopFuture<Void> {
database.schema("index-tests").delete()
}
}

func env(_ name: String) -> String? {
return ProcessInfo.processInfo.environment[name]
}
Expand Down