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.

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

Fix up Fluent provider for Sendable-correct FluentKit #774

Merged
merged 3 commits into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ on:

jobs:
unit-tests:
uses: vapor/ci/.github/workflows/run-unit-tests.yml@main
uses: vapor/ci/.github/workflows/run-unit-tests.yml@main
secrets: inherit
43 changes: 31 additions & 12 deletions Package.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version:5.7
// swift-tools-version:5.8
import PackageDescription

let package = Package(
Expand All @@ -13,18 +13,37 @@ let package = Package(
.library(name: "Fluent", targets: ["Fluent"]),
],
dependencies: [
.package(url: "https://github.com/vapor/fluent-kit.git", from: "1.45.0"),
.package(url: "https://github.com/vapor/vapor.git", from: "4.91.1"),
.package(url: "https://github.com/vapor/fluent-kit.git", from: "1.48.0"),
.package(url: "https://github.com/vapor/vapor.git", from: "4.94.1"),
],
targets: [
.target(name: "Fluent", dependencies: [
.product(name: "FluentKit", package: "fluent-kit"),
.product(name: "Vapor", package: "vapor"),
]),
.testTarget(name: "FluentTests", dependencies: [
.target(name: "Fluent"),
.product(name: "XCTFluent", package: "fluent-kit"),
.product(name: "XCTVapor", package: "vapor"),
]),
.target(
name: "Fluent",
dependencies: [
.product(name: "FluentKit", package: "fluent-kit"),
.product(name: "Vapor", package: "vapor"),
],
swiftSettings: swiftSettings
),
.testTarget(
name: "FluentTests",
dependencies: [
.target(name: "Fluent"),
.product(name: "XCTFluent", package: "fluent-kit"),
.product(name: "XCTVapor", package: "vapor"),
],
swiftSettings: swiftSettings
),
]
)

var swiftSettings: [SwiftSetting] { [
.enableUpcomingFeature("ConciseMagicFile"),
.enableUpcomingFeature("ForwardTrailingClosures"),
.enableUpcomingFeature("ImportObjcForwardDeclarations"),
.enableUpcomingFeature("DisableOutwardActorInference"),
.enableUpcomingFeature("IsolatedDefaultValues"),
.enableUpcomingFeature("GlobalConcurrency"),
.enableUpcomingFeature("StrictConcurrency"),
.enableExperimentalFeature("StrictConcurrency=complete"),
Copy link

Choose a reason for hiding this comment

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

You did it the other way around. You should remove strict concurrency from here and add existential any, since this is for Swift 5.8.
Also you don't need to use the upcoming StrictConcurrency flag. They did upgrade the flag to an upcoming feature for Swift 5.10, but it's backward compatible.

Copy link
Member Author

Choose a reason for hiding this comment

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

No. The only reason the Swift 5.9 manifest exists is because it's unsafe to use ExistentialAny with 5.8 (SwiftPM had a bug in 5.8 which caused the setting to virally propagate to anything using the package). StrictConcurrency, on the other hand, is harmless in 5.8 - yes, it's also ineffectual, but I leave it in to avoid having more of a disconnect than necessary between the two manifests (simplifies dealing with it next time there's a version bump). And yes, I'm aware of the dual flag being unnecessary, but I've used it elsewhere so I'm using it here, if you don't like it go open PRs against the other five repos I've copy-pasted this to 😛

] }
26 changes: 16 additions & 10 deletions [email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ let package = Package(
.library(name: "Fluent", targets: ["Fluent"]),
],
dependencies: [
.package(url: "https://github.com/vapor/fluent-kit.git", from: "1.45.0"),
.package(url: "https://github.com/vapor/vapor.git", from: "4.91.1"),
.package(url: "https://github.com/vapor/fluent-kit.git", from: "1.48.0"),
.package(url: "https://github.com/vapor/vapor.git", from: "4.94.1"),
],
targets: [
.target(
Expand All @@ -23,10 +23,7 @@ let package = Package(
.product(name: "FluentKit", package: "fluent-kit"),
.product(name: "Vapor", package: "vapor"),
],
swiftSettings: [
.enableUpcomingFeature("ExistentialAny"),
.enableExperimentalFeature("StrictConcurrency=complete"),
]
swiftSettings: swiftSettings
),
.testTarget(
name: "FluentTests",
Expand All @@ -35,10 +32,19 @@ let package = Package(
.product(name: "XCTFluent", package: "fluent-kit"),
.product(name: "XCTVapor", package: "vapor"),
],
swiftSettings: [
.enableUpcomingFeature("ExistentialAny"),
.enableExperimentalFeature("StrictConcurrency=complete"),
]
swiftSettings: swiftSettings
),
]
)

var swiftSettings: [SwiftSetting] { [
.enableUpcomingFeature("ExistentialAny"),
.enableUpcomingFeature("ConciseMagicFile"),
.enableUpcomingFeature("ForwardTrailingClosures"),
.enableUpcomingFeature("ImportObjcForwardDeclarations"),
.enableUpcomingFeature("DisableOutwardActorInference"),
.enableUpcomingFeature("IsolatedDefaultValues"),
.enableUpcomingFeature("GlobalConcurrency"),
.enableUpcomingFeature("StrictConcurrency"),
.enableExperimentalFeature("StrictConcurrency=complete"),
] }
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
<a href="https://docs.vapor.codes/4.0/"><img src="https://design.vapor.codes/images/readthedocs.svg" alt="Documentation"></a>
<a href="https://discord.gg/vapor"><img src="https://design.vapor.codes/images/discordchat.svg" alt="Team Chat"></a>
<a href="LICENSE"><img src="https://design.vapor.codes/images/mitlicense.svg" alt="MIT License"></a>
<a href="https://github.com/vapor/fluent/actions/workflows/test.yml"><img src="https://img.shields.io/github/actions/workflow/status/vapor/fluent/test.yml?event=push&style=plastic&logo=github&label=test&logoColor=%23ccc" alt="Continuous Integration"></a>
<a href="https://codecov.io/github/vapor/fluent"><img src="https://img.shields.io/codecov/c/github/vapor/fluent?style=plastic&logo=codecov&label=Codecov&token=yDzzHja8lt"></a>
<a href="https://swift.org"><img src="https://design.vapor.codes/images/swift57up.svg" alt="Swift 5.7+"></a>
<a href="https://github.com/vapor/fluent/actions/workflows/test.yml"><img src="https://img.shields.io/github/actions/workflow/status/vapor/fluent/test.yml?event=push&style=plastic&logo=github&label=tests&logoColor=%23ccc" alt="Continuous Integration"></a>
<a href="https://codecov.io/github/vapor/fluent"><img src="https://img.shields.io/codecov/c/github/vapor/fluent?style=plastic&logo=codecov&label=codecov&token=yDzzHja8lt"></a>
<a href="https://swift.org"><img src="https://design.vapor.codes/images/swift58up.svg" alt="Swift 5.8+"></a>
</p>

<br>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

import Vapor
import FluentKit

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import NIOCore
import Vapor
@preconcurrency import FluentKit
import FluentKit

extension ModelCredentialsAuthenticatable {
public static func asyncCredentialsAuthenticator(
Expand Down
2 changes: 1 addition & 1 deletion Sources/Fluent/Concurrency/Sessions+Concurrency.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import NIOCore
import Vapor
@preconcurrency import FluentKit
import FluentKit

extension Model where Self: SessionAuthenticatable, Self.SessionID == Self.IDValue {
public static func asyncSessionAuthenticator(
Expand Down
2 changes: 1 addition & 1 deletion Sources/Fluent/Docs.docc/theme-settings.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"theme": {
"aside": { "border-radius": "6px", "border-style": "double", "border-width": "3px" },
"aside": { "border-radius": "16px", "border-style": "double", "border-width": "3px" },
"border-radius": "0",
"button": { "border-radius": "16px", "border-width": "1px", "border-style": "solid" },
"code": { "border-radius": "16px", "border-width": "1px", "border-style": "solid" },
Expand Down
8 changes: 0 additions & 8 deletions Sources/Fluent/Exports.swift
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
#if swift(>=5.8)

@_documentation(visibility: internal) @_exported import FluentKit

#else

@_exported import FluentKit

#endif

infix operator ~~
infix operator =~
infix operator !~
Expand Down
9 changes: 3 additions & 6 deletions Sources/Fluent/Fluent+Cache.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import NIOCore
import Foundation
import Vapor
@preconcurrency import FluentKit
import FluentKit

extension Application.Caches {
public var fluent: any Cache {
Expand Down Expand Up @@ -49,10 +49,7 @@ private struct FluentCache: Cache {
}
}

func set<T>(_ key: String, to value: T?) -> EventLoopFuture<Void>
where T: Encodable

{
func set(_ key: String, to value: (some Encodable)?) -> EventLoopFuture<Void> {
if let value = value {
do {
let data = try JSONEncoder().encode(value)
Expand All @@ -74,7 +71,7 @@ private struct FluentCache: Cache {
}
}

public final class CacheEntry: Model {
public final class CacheEntry: Model, @unchecked Sendable {
public static let schema: String = "_fluent_cache"

struct Create: Migration {
Expand Down
10 changes: 5 additions & 5 deletions Sources/Fluent/Fluent+History.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Vapor
@preconcurrency import FluentKit
import FluentKit

struct RequestQueryHistory: StorageKey {
typealias Value = QueryHistory
Expand Down Expand Up @@ -42,12 +42,12 @@ extension Application.Fluent.History {
}

var history: QueryHistory? {
return storage[RequestQueryHistory.self]
storage[RequestQueryHistory.self]
}

/// The queries stored in this lifecycle history
public var queries: [DatabaseQuery] {
return history?.queries ?? []
history?.queries ?? []
}

/// Start recording the query history
Expand Down Expand Up @@ -82,12 +82,12 @@ extension Request.Fluent.History {
}

var history: QueryHistory? {
return storage[RequestQueryHistory.self]
storage[RequestQueryHistory.self]
}

/// The queries stored in this lifecycle history
public var queries: [DatabaseQuery] {
return history?.queries ?? []
history?.queries ?? []
}

/// Start recording the query history
Expand Down
6 changes: 3 additions & 3 deletions Sources/Fluent/Fluent+Sessions.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Foundation
import NIOCore
import Vapor
@preconcurrency import FluentKit
import FluentKit

extension Application.Fluent {
public var sessions: Sessions {
Expand Down Expand Up @@ -42,7 +42,7 @@ extension Application.Fluent.Sessions {

extension Application.Sessions.Provider {
public static var fluent: Self {
return .fluent(nil)
.fluent(nil)
}

public static func fluent(_ databaseID: DatabaseID?) -> Self {
Expand Down Expand Up @@ -110,7 +110,7 @@ private struct DatabaseSessionAuthenticator<User>: SessionAuthenticator
}
}

public final class SessionRecord: Model {
public final class SessionRecord: Model, @unchecked Sendable {
public static let schema = "_fluent_sessions"

struct Create: Migration {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Fluent/FluentProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import NIOPosix
import NIOConcurrencyHelpers
import Logging
import Vapor
@preconcurrency import FluentKit
import FluentKit

extension Request {
public var db: any Database {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Fluent/ModelAuthenticatable.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Vapor
import NIOCore
@preconcurrency import FluentKit
import FluentKit

public protocol ModelAuthenticatable: Model, Authenticatable {
static var usernameKey: KeyPath<Self, Field<String>> { get }
Expand Down
2 changes: 1 addition & 1 deletion Sources/Fluent/ModelCredentialsAuthenticatable.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Vapor
import NIOCore
@preconcurrency import FluentKit
import FluentKit

public protocol ModelCredentialsAuthenticatable: Model, Authenticatable {
static var usernameKey: KeyPath<Self, Field<String>> { get }
Expand Down
2 changes: 1 addition & 1 deletion Sources/Fluent/ModelTokenAuthenticatable.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Vapor
import NIOCore
@preconcurrency import FluentKit
import FluentKit

public protocol ModelTokenAuthenticatable: Model, Authenticatable {
associatedtype User: Model & Authenticatable
Expand Down
2 changes: 1 addition & 1 deletion Tests/FluentTests/CredentialTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ final class CredentialTests: XCTestCase {
}
}

final class CredentialsUser: Model {
final class CredentialsUser: Model, @unchecked Sendable {
static let schema = "users"

@ID(key: .id)
Expand Down
8 changes: 4 additions & 4 deletions Tests/FluentTests/OperatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ final class OperatorTests: XCTestCase {
.filter(\.$name !~ ["Earth", "Mars"])
}
}
private final class Planet: Model {
private final class Planet: Model, @unchecked Sendable {
static let schema = "planets"

@ID(custom: .id)
Expand All @@ -49,7 +49,7 @@ private struct DummyDatabase: Database {
fatalError()
}

func execute(query: DatabaseQuery, onOutput: @escaping (any DatabaseOutput) -> ()) -> EventLoopFuture<Void> {
func execute(query: DatabaseQuery, onOutput: @escaping @Sendable (any DatabaseOutput) -> ()) -> EventLoopFuture<Void> {
fatalError()
}

Expand All @@ -61,11 +61,11 @@ private struct DummyDatabase: Database {
fatalError()
}

func withConnection<T>(_ closure: @escaping (any Database) -> EventLoopFuture<T>) -> EventLoopFuture<T> {
func withConnection<T>(_ closure: @escaping @Sendable (any Database) -> EventLoopFuture<T>) -> EventLoopFuture<T> {
fatalError()
}

func transaction<T>(_ closure: @escaping (any Database) -> EventLoopFuture<T>) -> EventLoopFuture<T> {
func transaction<T>(_ closure: @escaping @Sendable (any Database) -> EventLoopFuture<T>) -> EventLoopFuture<T> {
fatalError()
}
}
23 changes: 13 additions & 10 deletions Tests/FluentTests/PaginationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,34 @@ import Vapor
import XCTVapor
import XCTFluent
import FluentKit
import NIOConcurrencyHelpers

final class PaginationTests: XCTestCase {
func testPagination() throws {
let app = Application(.testing)
defer { app.shutdown() }

var rows: [TestOutput] = []
let rows: NIOLockedValueBox<[TestOutput]> = .init([])
for i in 1...1_000 {
rows.append(TestOutput([
rows.withLockedValue { $0.append(TestOutput([
"id": i,
"title": "Todo #\(i)"
]))
])) }
}
let test = CallbackTestDatabase { query in
XCTAssertEqual(query.schema, "todos")
let result: [TestOutput]
if let limit = query.limits.first?.value, let offset = query.offsets.first?.value {
result = [TestOutput](rows[min(offset, rows.count - 1)..<min(offset + limit, rows.count)])
} else {
result = rows
var result: [TestOutput] = []
rows.withLockedValue {
if let limit = query.limits.first?.value, let offset = query.offsets.first?.value {
result = [TestOutput]($0[min(offset, $0.count - 1)..<min(offset + limit, $0.count)])
} else {
result = $0
}
}

switch query.action {
case .aggregate(_):
return [TestOutput([.aggregate: rows.count])]
return [TestOutput([.aggregate: rows.withLockedValue { $0.count }])]
default:
return result
}
Expand Down Expand Up @@ -161,7 +164,7 @@ private extension DatabaseQuery.Offset {
}
}

private final class Todo: Model, Content {
private final class Todo: Model, Content, @unchecked Sendable {
static let schema = "todos"

@ID(custom: .id)
Expand Down
2 changes: 1 addition & 1 deletion Tests/FluentTests/QueryHistoryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ final class QueryHistoryTests: XCTestCase {
}
}

private final class Post: Model, Content, Equatable {
private final class Post: Model, Content, Equatable, @unchecked Sendable {
static func == (lhs: Post, rhs: Post) -> Bool {
lhs.id == rhs.id && lhs.content == rhs.content
}
Expand Down
Loading