feroult / yawp

Kotlin/Java API framework for Google Appengine
http://yawp.io
MIT License
132 stars 20 forks source link

Let Shields change status code #63

Closed feroult closed 8 years ago

luanpotter commented 8 years ago

Currently one could simply throw a HttpException; I often do the following:

@Override
public void create(List<Entity> entities) {
    for (Entity e : entities) {
        e.validate();
    }
    allow();
}

Where the validate method throws a 422 with proper errors when the validation fails.

I think one other possibility would be to have a disallowWithError(int, String) method in the ShieldBase (analogous to HttpException's constructor). But I'm not sure that is really necessary. Also, it would be confusing if it were to be called twice with different status code. Or with 2xx codes. Though I guess you can throw a 200 HttpException anyway.

What do you have in mind, @feroult?

feroult commented 8 years ago

@luanpotter, I think yours is nice approach and should be used where needed.

Following your code, I've thought something like:

@Override
public void create(List<Entity> entities) {
    for (Entity e : entities) {
        e.shield(this);
        // or
        e.allow(this);
    }
    denyWith(403);
}

But I don't know... Your approach seems better at a first glance.

feroult commented 8 years ago

You're going to stay with HttpException ;)