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

Adds assertEquals() and guard() functions #63

Closed
wants to merge 4 commits into from

Conversation

twof
Copy link
Contributor

@twof twof commented Feb 3, 2018

Let me know if you want them moved into their own extension files. Example usage:

User.query(on: req)
    .filter(\.email == registerRequest.email)
    .count()
    .assertEquals(0)
    .guard(elseThrow: userExistsError)
    .saveUserWith(
        name: registerRequest.name,
        email: registerRequest.email,
        password: registerRequest.password,
        on: req
    )

Marking it as `throws` made `Abort` be mishandled
@bensyverson
Copy link

bensyverson commented Feb 4, 2018

I'm still seeing weird behavior where an exception within guard is not bubbling up. Check out this example project which uses the proposed change. You'll need to reconfigure psql, then register a new User (call /api/v1/register or use the included Paw file).

Registering multiple users with the same email address should be prevented by guard, but it passes through. The console will read I should throw!—which is printed from the same block that throws the Error—but the Error (an Abort) is silently caught somewhere, allowing the chain to continue with a successful 200

@twof
Copy link
Contributor Author

twof commented Feb 8, 2018

In reference to @bensyverson's issue, there seems to be a problem where Aborts thrown earlier in the chain will be overridden buy errors thrown later in the chain.

@twof
Copy link
Contributor Author

twof commented Feb 8, 2018

Added a variadic version of assertEquals and made guard's result discardable so you can now do things like validate query results. Example:

extension QueryBuilder where Model == Reservation {
    func validNumberOfReservationsInDatabase() -> Self {
        let invalidNumberOfReservationsError = Abort(.badRequest, reason: "There are an unusual number of reservations matching that query in your DB", identifier: "Whoops")
        
        self
            .count()
            .assertEquals(0, 1)
            .guard(elseThrow: invalidNumberOfReservationsError)
        
        return self
    }
}

In this example I'm checking to see if the number of Reservations returned is either 0 or 1 and Aborting if that's not the case.

@tanner0101 tanner0101 added this to the 1.0 milestone Feb 9, 2018
@tanner0101 tanner0101 self-assigned this Feb 9, 2018
@tanner0101
Copy link
Contributor

I like these methods, but not convinced (yet) they are critical enough to require in the core. These strike me as suitable for addition in a third party "helpers" package.

@Joannis
Copy link
Contributor

Joannis commented Feb 9, 2018

I concur with @tanner0101

@jdmcd
Copy link

jdmcd commented Feb 9, 2018

@twof Maybe you can start a helpers package in vapor-community?

@twof
Copy link
Contributor Author

twof commented Feb 9, 2018

@mcdappdev I'm down. Do I need to be added to the organization or how does that work?

@jdmcd
Copy link

jdmcd commented Feb 9, 2018

I believe so but I'm not positive

@twof
Copy link
Contributor Author

twof commented Feb 18, 2018

Hey I'm still down to maintain (or comaintain) a repo in vapor-community if someone wants to open an AsyncHelpers repo for me

@tanner0101 @Joannis

CC @MaximBazarov

@twof twof mentioned this pull request Feb 20, 2018
@gperdomor
Copy link
Member

@twof @tanner0101 @Joannis @mcdappdev @bensyverson I made a repo with some extensions, please make PR for new extensions here --> https://github.com/vapor-community/vapor-extensions

@twof
Copy link
Contributor Author

twof commented Feb 21, 2018

CC @MaximBazarov

@gperdomor
Copy link
Member

I think we can close this in favor of https://github.com/vapor-community/async-extensions

@twof
Copy link
Contributor Author

twof commented Feb 26, 2018

Closing this PR now that there's a community extensions package

@twof twof closed this Feb 26, 2018
@tanner0101 tanner0101 removed this from Todo in Vapor 3 Mar 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

6 participants