Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

Add error message when assert is false #41

Open
vonglasow opened this issue Jan 7, 2015 · 9 comments
Open

Add error message when assert is false #41

vonglasow opened this issue Jan 7, 2015 · 9 comments

Comments

@vonglasow
Copy link
Member

Take an example

// The User object.
class User {

    const DISCONNECTED = 0;
    const CONNECTED    = 1;

    public $group      = 'customer';
    public $points     = 42;
    protected $_status = 0;

    public function getStatus ( ) {

        return $this->_status;
    }
}

$ruler = new Hoa\Ruler\Ruler();

// New rule.
$rule  = 'logged(user) and group in ["customer", "guest"] and points > 30';

// New context.
$context         = new Hoa\Ruler\Context();
$context['user'] = function ( ) use ( $context ) {

    $user              = new User();
    $context['group']  = $user->group;
    $context['points'] = $user->points;

    return $user;
};

// We add the logged() operator.
$ruler->getDefaultAsserter()->setOperator('logged', function ( User $user ) {

    return $user::CONNECTED === $user->getStatus();
});

// Finally, we assert the rule.
var_dump(
    $ruler->assert($rule, $context)
);

/**
 * Will output:
 *     bool(false)
 */

Ok so we have here a rule not respected, but why this rule doesn't passed is it because we have the user logout or because them points is <= 30
It could be useful to identify the reason of breaking rules. But all rules depends on operator so I think the operator could provide a property with error message when the result of validation is false and aggregate in ruler to got it.

To continue the example above

var_dump(
    $ruler->getErrorMessages();
);
/**
 * Will output:
 *     array(
 *         'User is not logged'
 *     )
 */

What do you think ?

@Hywan
Copy link
Member

Hywan commented Jan 7, 2015

Huge 👍 on this one!

@simkimsia
Copy link
Contributor

👍

1 similar comment
@gael-wogenstahl
Copy link

+1

vonglasow added a commit to vonglasow/Ruler that referenced this issue Apr 6, 2015
When operators are evaluated, we can found some asserter false. This
patch add a management of errors in the asserter. Operators name and
value is added in errors stack to identify which operator fail.

Two methods are available to identify if we have errors and to get there
errors:
* hasErrors return a boolean which identify if some errors have been
raised.
* getErrors return an array with operator name as key and arguments as
values.
vonglasow added a commit to vonglasow/Ruler that referenced this issue Apr 8, 2015
When operators are evaluated, we can found some asserter false. This
patch add a management of errors in the asserter. Operators name and
value is added in errors stack to identify which operator fail.

Two methods are available to identify if we have errors and to get there
errors:
* hasErrors return a boolean which identify if some errors have been
raised.
* getErrors return an array with operator name as key and arguments as
values.
* resetErrors clean all errors stored.
@1e1
Copy link

1e1 commented Jul 30, 2015

It seems compex to explain what is wrong if the function has more parameter.
What's happens with:
$rule = 'logged(user, channel) and group in ["customer", "guest"] and points > 30';

var_dump(
    $ruler->getErrorMessages();
);
/**
 * Will output:
 *     array(
 *         'User "Object" is not logged on channel 2'
 *     )
 */

?

Perhaps, something like SQL explain should be more generic, even the message wil be more technical.

I see a response like:

var_dump(
    $ruler->explain();
);
/**
 * Will output:
 * logged(user, channel) and group in ["customer", "guest"] and points > 30' = false
 * |  logged(user, channel) = false
 * |    user = { ..json_encode.. }
 * |    channel = 2 // is json_encoded
 * |  BREAK
 */

@1e1
Copy link

1e1 commented Jul 30, 2015

And a complete explaination:

var_dump(
    $ruler->explain();
);
/**
 * Will output:
 * logged(user, channel) and group in ["customer", "guest"] and points > 30' = true
 * | logged(user, channel) and group in ["customer", "guest"] = true
 * |  |  logged(user, channel) = true
 * |  |  |  user = { ..json_encode.. }
 * |  |  |  channel = 2
 * |  |  group in ["customer", "guest"] = true
 * |  |  |  group = "customer"
 * | points > 30'  = true
 * |  |  points = 42
 */

@1e1
Copy link

1e1 commented Jul 30, 2015

@vonglasow
I wonder how var_dump($context) and closure_dump() could solve this issue.
As you use a closure to define User context if you change:
$rule = 'logged(user) and group in ["customer", "guest"] and points > 30';
to
$rule = 'group in ["customer", "guest"] and points > 30 and logged(user)';
I will catch an exception because group and points are setted when you get user.

Perhaps using a Closure to define the context is not a good practice?

@Hywan Hywan self-assigned this Jul 31, 2015
@Hywan
Copy link
Member

Hywan commented Aug 3, 2015

@1e1 I like your proposal in #41 (comment), having the tree/AST view of the rule with associated error is a good __toString, if we go in this direction.

@vonglasow
Copy link
Member Author

@1e1
For now we have this kind of output

array(1) {
  ["f"]=>
  array(1) {
    [0]=>
    object(Hoa\Ruler\Model\Bag\Scalar)#251 (1) {
      ["_value":protected]=>
      bool(false)
    }
  }
}

But we can improve it. I will try to make a test with your exemple to show you what is done yet. And if you have any idea to make this AST let me know we can work on it together.
@Hywan I am OK also for an AST, we talk together about an API to manage errors. What do you see for there next step ?

@Hywan
Copy link
Member

Hywan commented Aug 6, 2015

@vonglasow Not sure. I don't know what the user needs exactly here. It is not defined. In what case are we likely to use the errors API? What do we expect? What information do we need?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

5 participants