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

ParameterPreconditions aren't working as of 1.0.0 #732

Closed
jmazouri opened this issue Jun 30, 2017 · 1 comment
Closed

ParameterPreconditions aren't working as of 1.0.0 #732

jmazouri opened this issue Jun 30, 2017 · 1 comment

Comments

@jmazouri
Copy link
Contributor

I noticed this while upgrading my project from 1.0.0-rc2 (Nuget Prerelease) to 1.0.0 "Final" (also Nuget). It seems parameter preconditions simply never run.

Here is a simple repro, which I used in conjunction with the Command Service example in the documentation.

public class TestModule : ModuleBase
{
    [Command("test")]
    public async Task DoTest([TestPrecon]string input)
    {
        //this should never happen
        await ReplyAsync("Test Succeeded");
    }
}

public class TestPrecon : ParameterPreconditionAttribute
{
    public override async Task<PreconditionResult> CheckPermissions(ICommandContext context, ParameterInfo parameter, object value, IServiceProvider services)
    {
        //do some async work
        var ownerId = (await services.GetService<DiscordSocketClient>().GetApplicationInfoAsync()).Owner.Id;

        //always return an error
        return PreconditionResult.FromError("Failure.");
    }
}

With this code, "Test Succeeded" is always sent, regardless of the PreconditionResult being an error result. None of the code within CheckPermissions is executed.

@jmazouri
Copy link
Contributor Author

jmazouri commented Jul 1, 2017

After looking into it, it appears to be a regression caused by this large PR: #689
032aba9#diff-c2289b02a37b82dc75e06f172eddff2dL204

You can see at this line, during the refactoring that switched it to modern C# 7 pattern matching, ParameterPreconditionAttribute was mistakenly removed. In a local test build, simply adding the following to the top of the switch block fixed the issue:

case ParameterPreconditionAttribute precon:
    builder.AddPrecondition(precon);
    break;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants