Skip to content

Commit

Permalink
Disallows private-scoped migrations with default names. (#328)
Browse files Browse the repository at this point in the history
* Add critical & fatal error to migration process if a private context is discovered. Add an error but allow continuing if a different unexpected name is discovered.

* code doc for new method.

* fix private fluent benchmark migration.
  • Loading branch information
mattpolzin authored Jul 6, 2020
1 parent 931fa1f commit c9d8cf2
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
2 changes: 1 addition & 1 deletion Sources/FluentBenchmark/Tests/MigratorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ extension FluentBenchmarker {
}
}

private struct ErrorMigration: Migration {
internal struct ErrorMigration: Migration {
init() { }

struct Error: Swift.Error { }
Expand Down
4 changes: 4 additions & 0 deletions Sources/FluentKit/Migration/Migration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ public protocol Migration {

extension Migration {
public var name: String {
return defaultName
}

internal var defaultName: String {
return String(reflecting: Self.self)
}
}
38 changes: 28 additions & 10 deletions Sources/FluentKit/Migration/Migrator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,29 @@ private final class DatabaseMigrator {
// MARK: Setup

func setupIfNeeded() -> EventLoopFuture<Void> {
return MigrationLog.migration.prepare(on: self.database).flatMap {
self.fixPrereleaseMigrationNames()
return MigrationLog.migration.prepare(on: self.database)
.flatMap(self.preventUnstableNames)
.flatMap(self.fixPrereleaseMigrationNames)
}

/// An unstable name is a name that is not the same every time migrations
/// are run.
///
/// For example, the default name for `Migrations` in private contexts
/// will include an identifier that can change from one execution to the next.
private func preventUnstableNames() -> EventLoopFuture<Void> {
for migration in self.migrations {
let migrationName = migration.name
guard migration.name == migration.defaultName else { continue }
guard migrationName.contains("$") else { continue }

if migrationName.contains("unknown context at") {
self.database.logger.critical("The migration at \(migrationName) is in a private context. Either explicitly give it a name by adding the `var name: String` property or make the migration `internal` or `public` instead of `private`.")
fatalError("Private migrations not allowed")
}
self.database.logger.error("The migration at \(migrationName) has an unexpected default name. Consider giving it an explicit name by adding a `var name: String` property before applying these migrations.")
}
return self.database.eventLoop.makeSucceededFuture(())
}

// This migration just exists to smooth the gap between
Expand All @@ -153,14 +173,12 @@ private final class DatabaseMigrator {
var nameOverrides = Set<String>()

for migration in self.migrations {
let releaseCandidateDefaultName = "\(type(of: migration))"
let v4DefaultName = String(reflecting:type(of: migration))

// if the migration does not override the default name.
// NOTE we compare to the _current_ style of default, not
// the release candidate style of default name.
if migration.name == v4DefaultName {
migrationNameMap[releaseCandidateDefaultName] = v4DefaultName
// if the migration does not override the default name
// then it is a candidate for a name change.
if migration.name == migration.defaultName {
let releaseCandidateDefaultName = "\(type(of: migration))"

migrationNameMap[releaseCandidateDefaultName] = migration.defaultName
} else {
nameOverrides.insert(migration.name)
}
Expand Down

0 comments on commit c9d8cf2

Please sign in to comment.