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

Adding possibility to return context keys for dynamically populate context. #13

Closed
wants to merge 8 commits into from

Conversation

ncosmin2001
Copy link

In my case, I want to be able to populate my context dynamically so i've added this possibility.
The code will look something like this:

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

    $rule  = 'group in ("customer", "guest") and points > 30';

    $rule =  \Hoa\Ruler\Ruler::interprete($rule);
    $contextKeys = $ruler->getContextKeys();

    //dynamically populate context based on context keys
    $context = getContext (new \Hoa\Ruler\Context(), $contextKeys);

    var_dump($ruler->assert($rule, $context));

Adding possibility to return context keys for dynamically populate context.
Adding possibility to return context keys for dynamically populate context.
@Hywan
Copy link
Member

Hywan commented Mar 24, 2014

Hello :-),

I don't understand your usecase. Can you explain?

@Hywan Hywan added the question label Mar 24, 2014
@Hywan Hywan self-assigned this Mar 24, 2014
@ncosmin2001
Copy link
Author

Hello again,

Sure I can explain.
I have a config of different rules and i want to be able for each one to know exactly which is my context.
let's see the following example:

  1. i have this rule -> $rule = 'group in ("customer", "guest") and points > 30';
    I want to know that I need to set $context['group'] and $context['points']
  2. $contextKeys = $ruler->getContextKeys();
    this call will return an array with the values ('group', 'points')
    and i will be able to do something like this
  foreach($contextKeys as $item ) {
        $context[$item] = $myArray[$item];
                 or
        $method = 'get'.$item;
        $context[$item] = $this->$method();
}
  1. i need to provide $myArray or to implement $method()

Sounds legit? :D

@Hywan
Copy link
Member

Hywan commented Mar 24, 2014

Why not writing:

foreach($myArray as $item => $value)
$context[$item] = $value;

which does the same thing :-/ ?

I don't know why do it is needed to collect variables in a rule.

@ncosmin2001
Copy link
Author

Because myArray will have much more info that is actually needed and because option 2 suites better my needs because I have to take info from many places
(
option 2
$method = 'get'.$item;
$context[$item] = $this->$method();
)

@CircleCode
Copy link
Member

foreach($myArray as $item => $value)
    $context[$item] = $value;

does the same thing as

foreach($contextKeys as $item ) {
    $context[$item] = $myArray[$item];
}

but is different from

foreach($contextKeys as $item ) {
    $method = 'get'.$item;
    $context[$item] = $this->$method();
}

In some cases, calling $this->$method() can be long, and then we would want to compute it only if used in the rule

In some other cases, we might be unable to know exhaustively all the keys that can be used.

@Hywan
Copy link
Member

Hywan commented Mar 24, 2014

Ok so the goal here is to get a list of variables in a rule, right?

@ncosmin2001
Copy link
Author

Exactly, you got it :)
I think it will be a good feature 👍

@stephpy
Copy link
Member

stephpy commented Mar 25, 2014

Hi, why do not use \Closure ? If you did this PR because you don't want to populate context with data and loose perf, closure will never been executed except if they are called via a rule.

Your context could be

<?php
$context = array(
   'group' => function() use ($self) {
      return $self->getGroup();
   },
   'points' => function() use ($self) {
       return $self->getPoints();
   }
);

@ncosmin2001
Copy link
Author

Hi @stephpy,

I don't think you understand exactly what my needs are.
I want to have a list of content variables present in a rule. For example:

  1. $rule = 'group in ("customer", "guest") and points > 30';
    i need to know that i will have to set $context['group'] and $context['points']
  2. $rule = 'group in ("customer", "guest") and points > 30 and age < 60';
    i need to know that i will have to set $context['group'], $context['points'] and $context['age ']

... and all this without knowing how my rule will look.

All this is because if my rule changes i can provide additional context without adding new code as i will have getters for all my context variables.

@Hywan
Copy link
Member

Hywan commented Mar 25, 2014

@stephpy: Yes, @ncosmin2001 would like to list all the variables declared in a rule. It would be useful to you for a graphical interface for example.

@stephpy
Copy link
Member

stephpy commented Mar 25, 2014

I understood, it could be useful to have this information. But it doesn't change what i said about population of context.

Even if you have group in ("customer", "guest") and points > 30, you can populate all context keys with closures (not executed if not asked).

But you can create a dynamic context too, sure. :)

@ncosmin2001
Copy link
Author

Yes you are right about that, it was just an example. But i thought you didn't understand my problem :)

@Hywan
Copy link
Member

Hywan commented Mar 26, 2014

@stephpy: +1 for closures but this does not solve the current issue (but thanks for the reminder!).

@ncosmin2001: Your patches collect the list of variables in the interpreter, but a rule can serialized and saved in, let's say, a database. Actually, the serialized rule is representing by a model. If we collect variables during the interpretation, we loose this information when we manipulate the model, or, to fix this, we should save the model along with the list of declared variables, which seems complicated. Here is my proposal: what about implementing this feature in the model directly? There is two solutions:

  1. each time we call the Hoa\Ruler\Model::variable method, we store the new created variable in the model… but a newly created variable is not necessarily used in the model,
  2. visit the model and collect all the variables, this can be done once and cached, also it can be done when needed (this is not a heavy task).

Thoughts?

@ncosmin2001
Copy link
Author

Hi,

I am now revisiting my fix and a new thought came to me: It would be also nice if we had a list of new added operators if is the case

For example for the rule :
$rule = 'isConnection(user1,user2) and group in ("customer", "guest") and points > 30';
will return something like
array(
'operator' => isConnection,
'context' => array ('user1', 'user2')
)

also
for a rule like $rule = 'group in ("customer")' an exception is thrown
i don't know if bug or intended.

I don't have a solution yet for any of the problems mentioned above but i'm working on it :)

@Hywan
Copy link
Member

Hywan commented Mar 27, 2014

@stephpy Could you take a look at the bug mentionned by @ncosmin2001 with group in ("customer") please? Also @ncosmin2001, can you create two specific issues for these two ones? Thanks!

@ncosmin2001
Copy link
Author

what about having

protected $_contextVariables       = array();
public static function addContextVariable($variable) {
      array_push($this->_contextVariables, $variable )
}

in Ruler class and

public function variable ( $id ) {
        \Ruler\Ruler::addContextVariable($id);
        return new Bag\Context($id);
}

in Model class

@Hywan
Copy link
Member

Hywan commented Mar 27, 2014

@ncosmin2001 Nop because a created variable is not necessarily added on the model. See my comment #13 (comment).

@ncosmin2001
Copy link
Author

Then I think the 1st option from your comment is more appropriate.

@Hywan
Copy link
Member

Hywan commented Mar 27, 2014

@ncosmin2001 You meant, the 2nd? :-p

@ncosmin2001
Copy link
Author

No No :) i don't wanna visit the model :) 1st one seems ok :) ca we have it? :p

@Hywan
Copy link
Member

Hywan commented Mar 27, 2014

The first proposal does not work. This is what you did in #13 (comment).

@ncosmin2001
Copy link
Author

Visiting the model twice is a little bit redundant in my opinion ...

@Hywan
Copy link
Member

Hywan commented Mar 27, 2014

@ncosmin2001: It is a low CPU-cost operation and this is the cleaner way to do :-).

@Hywan
Copy link
Member

Hywan commented Apr 6, 2014

ping?

@Hywan
Copy link
Member

Hywan commented May 14, 2014

ping^2 ?

@Hywan
Copy link
Member

Hywan commented Jul 9, 2014

last ping (after that, we will do this by our own)

@Hywan
Copy link
Member

Hywan commented Sep 28, 2014

last last ping (we will do this by our own else :-)).

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

Successfully merging this pull request may close these issues.

4 participants