Decouple biz rules from auth items #499

Closed
qiangxue opened this Issue Jun 5, 2013 · 16 comments

Comments

Projects
None yet
7 participants
@qiangxue
Member

qiangxue commented Jun 5, 2013

Currently in RBAC, we store a biz rule directly with its associated auth item. This has several drawbacks.

  1. The same biz rule needs to be repeated several times if it can be associated with multiple auth items.
  2. Because of 1, it's not easy to maintain biz rules.
  3. It's not flexible to evaluate biz rules. In particular, each biz rule is a PHP statement that needs to be evaluated via eval().

To overcome the above drawbacks, we may consider decoupling biz rules from auth items by introducing a list of named biz rules called biz rule map. The map contains a list of biz rules, each having a unique name. If a biz rule needs to be associated with an auth item, we store the biz rule name instead of the biz rule itself in the auth item.

The biz rule map can be stored as a PHP file, or multiple PHP files (one for each biz rule). Or we may consider storing biz rule map as a DB table.

If we use PHP files to store biz rule map, we may support using anonymous functions as biz rules. If DB table, perhaps our only choice is to use PHP statements as biz rules.

Related discussion: #471

@creocoder

This comment has been minimized.

Show comment
Hide comment
@creocoder

creocoder Jun 5, 2013

Contributor

The biz rule map can be stored as a PHP file, or multiple PHP files (one for each biz rule).

Both are good. How about option?

To overcome the above drawbacks, we may consider decoupling biz rules

And illiminate old approach, right? Or decoupling is an option?

Or we may consider storing biz rule map as a DB table.

I think we can introduce Manager::STORE_BIZ_RULES_SINGLE_FILE, Manager::STORE_BIZ_RULES_MULTIPLE_FILES and DbManager::STORE_BIZ_RULES_DB. As result PhpManager will have 2 options, DbManager 3 options.

Contributor

creocoder commented Jun 5, 2013

The biz rule map can be stored as a PHP file, or multiple PHP files (one for each biz rule).

Both are good. How about option?

To overcome the above drawbacks, we may consider decoupling biz rules

And illiminate old approach, right? Or decoupling is an option?

Or we may consider storing biz rule map as a DB table.

I think we can introduce Manager::STORE_BIZ_RULES_SINGLE_FILE, Manager::STORE_BIZ_RULES_MULTIPLE_FILES and DbManager::STORE_BIZ_RULES_DB. As result PhpManager will have 2 options, DbManager 3 options.

@creocoder

This comment has been minimized.

Show comment
Hide comment
@creocoder

creocoder Jun 5, 2013

Contributor

If DB table, perhaps our only choice is to use PHP statements as biz rules.

Did not see any problems for using biz rules as Closures that stored in files for DB as option.

Contributor

creocoder commented Jun 5, 2013

If DB table, perhaps our only choice is to use PHP statements as biz rules.

Did not see any problems for using biz rules as Closures that stored in files for DB as option.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Jun 5, 2013

Member

How will you save a Closure as a string in DB?
And how will you update a single PHP file which contains multiple biz rules, each represented as a Closure?

Member

qiangxue commented Jun 5, 2013

How will you save a Closure as a string in DB?
And how will you update a single PHP file which contains multiple biz rules, each represented as a Closure?

@creocoder

This comment has been minimized.

Show comment
Hide comment
@creocoder

creocoder Jun 5, 2013

Contributor

How will you save a Closure as a string in DB?

We did not save it to DB when it (as option) will be stored in file.

And how will you update a single PHP file which contains multiple biz rules, each represented as a Closure?

Manually, this is why i say about various store options.

Contributor

creocoder commented Jun 5, 2013

How will you save a Closure as a string in DB?

We did not save it to DB when it (as option) will be stored in file.

And how will you update a single PHP file which contains multiple biz rules, each represented as a Closure?

Manually, this is why i say about various store options.

@creocoder

This comment has been minimized.

Show comment
Hide comment
@creocoder

creocoder Jun 5, 2013

Contributor

@qiangxue Ok, i'll explain more:

Manager::STORE_BIZ_RULES_SINGLE_FILE, Manager::STORE_BIZ_RULES_MULTIPLE_FILES

We can use Closures or php statement for biz rules.

DbManager::STORE_BIZ_RULES_DB

Only php statements allowed.

Contributor

creocoder commented Jun 5, 2013

@qiangxue Ok, i'll explain more:

Manager::STORE_BIZ_RULES_SINGLE_FILE, Manager::STORE_BIZ_RULES_MULTIPLE_FILES

We can use Closures or php statement for biz rules.

DbManager::STORE_BIZ_RULES_DB

Only php statements allowed.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Jun 5, 2013

Member

I think we can start simple first by only supporting one mode: use a single PHP file to keep all biz rules and each rule is represented as a Closure.

We should have a new class possibly named Rule to represent a biz rule. It should contain name, description, callback and params properties.

Manager should provide a method to save rules.

Member

qiangxue commented Jun 5, 2013

I think we can start simple first by only supporting one mode: use a single PHP file to keep all biz rules and each rule is represented as a Closure.

We should have a new class possibly named Rule to represent a biz rule. It should contain name, description, callback and params properties.

Manager should provide a method to save rules.

@bwoester

This comment has been minimized.

Show comment
Hide comment
@bwoester

bwoester Jun 5, 2013

Contributor

Does it need to be one data source for all bizRules? I really don't want to over-complicate things, but I could imagine a solution where part of the bizRules comes from a single PHP file (static bizRules, defined as callables) and another part of the bizRules comes from some other data source, maybe but not necessarily a DB table (used for bizRules created by admins).

Manager would have a property $_ruleMap = array( 'ruleId' => $ruleInstance ) which probably should be lazy-populated. RuleSources would be responsible for loading and storing bizRules, configured in property Manager::$_ruleSources:

$_ruleSources = array(
  0 => array(
    'class' => 'Rule\Source\Static'
    'file' => '@data/rules.php'
  ),
  1 => array(
    'class' => 'Rule\Source\Db'
    'table' => 'yii_biz_rules'
  ),
);

The order of RuleSources would also determine if one source can override bizRules from another source (we're using the rule from the first source it is found in - or the other way round, just needs to be defined).

Contributor

bwoester commented Jun 5, 2013

Does it need to be one data source for all bizRules? I really don't want to over-complicate things, but I could imagine a solution where part of the bizRules comes from a single PHP file (static bizRules, defined as callables) and another part of the bizRules comes from some other data source, maybe but not necessarily a DB table (used for bizRules created by admins).

Manager would have a property $_ruleMap = array( 'ruleId' => $ruleInstance ) which probably should be lazy-populated. RuleSources would be responsible for loading and storing bizRules, configured in property Manager::$_ruleSources:

$_ruleSources = array(
  0 => array(
    'class' => 'Rule\Source\Static'
    'file' => '@data/rules.php'
  ),
  1 => array(
    'class' => 'Rule\Source\Db'
    'table' => 'yii_biz_rules'
  ),
);

The order of RuleSources would also determine if one source can override bizRules from another source (we're using the rule from the first source it is found in - or the other way round, just needs to be defined).

@creocoder

This comment has been minimized.

Show comment
Hide comment
@creocoder

creocoder Jun 6, 2013

Contributor

@bwoester

but I could imagine a solution where part of the bizRules comes from a single PHP file (static bizRules, defined as callables) and another part of the bizRules comes from some other data source

I think about this too yesterday. Something like one rules from file, other rules from db, but after some time i'm realize that this is really overcoding since there is no practice useage to support this. Moreover this is something what lead away us from goal.

Contributor

creocoder commented Jun 6, 2013

@bwoester

but I could imagine a solution where part of the bizRules comes from a single PHP file (static bizRules, defined as callables) and another part of the bizRules comes from some other data source

I think about this too yesterday. Something like one rules from file, other rules from db, but after some time i'm realize that this is really overcoding since there is no practice useage to support this. Moreover this is something what lead away us from goal.

@alpharder

This comment has been minimized.

Show comment
Hide comment
@alpharder

alpharder Oct 16, 2013

In my opinion the bizRules may have a form of classes with common interface BizRule. One class = one rule, namespaced classnames can be stored at persistent storage like DB

In my opinion the bizRules may have a form of classes with common interface BizRule. One class = one rule, namespaced classnames can be stored at persistent storage like DB

@samdark

This comment has been minimized.

Show comment
Hide comment
@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Nov 26, 2013

Member

SuperClosure is a great thing, but it uses eval() itself to unserialize the closure. No real improvement in this situation.

Member

cebe commented Nov 26, 2013

SuperClosure is a great thing, but it uses eval() itself to unserialize the closure. No real improvement in this situation.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Nov 26, 2013

Member

Well, yeah :) There's no way to store code in DB w/o sort of eval be it PHP or our own dialect.

Member

samdark commented Nov 26, 2013

Well, yeah :) There's no way to store code in DB w/o sort of eval be it PHP or our own dialect.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Nov 26, 2013

Member

yeah, right :)

Member

cebe commented Nov 26, 2013

yeah, right :)

@caleblloyd

This comment has been minimized.

Show comment
Hide comment
@caleblloyd

caleblloyd Mar 8, 2014

I agree with @alphard-code. Storing BizRules as classes would allow for more flexible custom RBAC implementation. I'd be interested in writing a custom DbBizRule that would take an ActiveQuery along with parameters that could run in the Database instead of PHP.

Namespaced Classnames of the BizRule could be stored in the DB, as could serialized parameters to initialize the class.

I agree with @alphard-code. Storing BizRules as classes would allow for more flexible custom RBAC implementation. I'd be interested in writing a custom DbBizRule that would take an ActiveQuery along with parameters that could run in the Database instead of PHP.

Namespaced Classnames of the BizRule could be stored in the DB, as could serialized parameters to initialize the class.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Mar 31, 2014

Member

@qiangxue are we going to do something about this one for beta?

Member

samdark commented Mar 31, 2014

@qiangxue are we going to do something about this one for beta?

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Apr 2, 2014

Member

I will handle it.

Member

samdark commented Apr 2, 2014

I will handle it.

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