vapor/console-kit

AsyncCommand integration with Vapor.Application

edwinveger opened this issue · 2 comments

Is your feature request related to a problem? Please describe.
I'm writing a Command for migrating between two database. The majority (if not all) of database interaction with Fluent is async.
So my command needs to conform to ConsoleKit.AsyncCommand. However, application.commands (of type ConsoleKit.Commands) only accepts synchronous commands.

Describe the solution you'd like
An extension on Commands to accept any AsyncCommand, which wraps it into a makeshift "sync" command.

import ConsoleKit

extension Commands {

    public mutating func use(_ command: any AsyncCommand, as name: String, isDefault: Bool = false) {
        self.use(command.wrapped(), as: name, isDefault: isDefault)
    }
}

extension AsyncCommand {

    private func wrapped() -> AnyCommand {
        AsyncCommandWrapper(asyncCommand: self)
    }
}

/// Inspired by others.
private struct AsyncCommandWrapper<Wrapped: AsyncCommand>: Command {

    let asyncCommand: Wrapped

    var help: String { asyncCommand.help }

    func run(using context: CommandContext, signature: Wrapped.Signature) throws {
        let promise = context.application.eventLoopGroup.next()
            .makePromise(of: Void.self)

        promise.completeWithTask {
            try await asyncCommand.run(using: context, signature: signature)
        }

        try promise.futureResult.wait()
    }

    func outputAutoComplete(using context: inout CommandContext) throws {
        try asyncCommand.outputAutoComplete(using: &context)
    }

    func outputHelp(using context: inout CommandContext) throws {
        try asyncCommand.outputHelp(using: &context)
    }

    func renderCompletionFunctions(using context: CommandContext, shell: Shell) -> String {
        return asyncCommand.renderCompletionFunctions(using: context, shell: shell)
    }
}

Describe alternatives you've considered
Adding an async commands container to Vapor.Application. I think this might be less desirable since it broadens the interface.

 public var asyncCommands: AsyncCommands {
        get { self.core.storage.asyncCommands }
        set { self.core.storage.asyncCommands = newValue }
    }

Additional context
The suggested solution does conflate some of the responsibilities of Commands and AsyncCommands. I would say it is useful from a vapor standpoint, but not necessarily from a ConsoleKit standpoint.
I've no strong preference. I was confused that there is this nice async API inside vapor/console-kit which does not fit into Vapor.Application. I'm sure someone with a better understanding of these packages and their relationship can bring insight.

I just noticed I probably filed this issue in the wrong repository - I'll wait for any input/discussion before moving it though.

t-ae commented

I'm using simpler solution inspired by #175.

protocol MyAsyncCommand: Command {
    func run(using context: CommandContext, signature: Signature) async throws
}

extension MyAsyncCommand {
    func run(using context: CommandContext, signature: Signature) throws {
        let promise = context.application.eventLoopGroup.next().makePromise(of: Void.self)
        promise.completeWithTask {
            try await run(using: context, signature: signature)
        }
        try promise.futureResult.wait()
    }
}

The problem in bringing it into console-kit is that we don't have CommandContext.application in this repository.
(It's defined in vapor repository.)

There are various way to wait async execution other than using EventLoopPromise. But I'm not sure if it's good idea.
https://stackoverflow.com/questions/70962534/swift-await-async-how-to-wait-synchronously-for-an-async-task-to-complete

In my opinion Command.run should be declared as async in the first place.
It'll be breaking change but doesn't break existing user-defined commands (sync definition fulfills the conformance).