Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 0.16.14

Bug #1131 unnecessary unique key in rights table
Submitted: 2004-04-04 21:17 UTC Modified: 2006-12-24 15:34 UTC
From: simon dot hamilton at ntlworld dot com Assigned: lsmith
Status: Closed Package: LiveUser
PHP Version: Irrelevant OS: n/a
Roadmaps: (Not assigned)    
Subscription  


 [2004-04-04 21:17 UTC] simon dot hamilton at ntlworld dot com
Description: ------------ In perm_db.sql there is: UNIQUE KEY `right_define_name` (`right_define_name`) in the `liveuser_rights` table. As far as I've been using it without this being unique and having several overlaps I don't see that its necessary to have this in as default. Since defines in an app are mostly composed with AREA_ or maybe even APP_AREA_ then there is no need for individual rights definenames to be unique. I have for instance: AREA1_READ AREA1_WRITE AREA2_READ AREA2_WRITE What do you think? -- Simon H

Comments

 [2004-04-04 21:35 UTC] dufuz
First of all this should have been a post on the mailinglist. Second I added this after a short talk with Lukas. And yes of course the right name has to be unique, what would happen if there are 2x NEWS_WRITE with it own ID, this could mess everything up if they are conflicting. I'll leave this open for 1-2 days and see what the others say.
 [2004-04-04 21:39 UTC] dufuz
Here's maybe a better example for you: (this is just a dummy example) This define was added as soon as the app was installed define('NEWS_WRITE', 1); This define was accedently added few weeks later define('NEWS_WRITE', 2); user A has news write access and thus right id 1 assigned to him. user B was created AFTER the second define was accedently added and thus got right id 2 assigned to him. since if this would be the row of the defines only user B can write. the later define is used (well duh :)) if (checkRight(NEWS_WRITE)) { echo 'write'; }
 [2004-04-05 04:10 UTC] simon dot hamilton at ntlworld dot com
I'd have to say that you should not be relying on unique keys to catch your development errors accidental or not. Also, I wrote it here rather than the list because I have lost discussions on topics like this (that are important) on the list before. When a bug report is submitted at least it gets a decent reponse. If we have the ability to create Defines with APP_AREA_RIGHT then we would have: APP_NEWS_NEWS_WRITE That could get ugly.
 [2004-04-05 12:39 UTC] lsmith
Well Simon does have a point. Since we dont really need the define_names to be unique inside the database, but after the export it may make more sense to handle the "uniqueness" elsewhere. Either during the import into the database or during the export, with the first probably making the most sense. However generally it might make sense to handle the settings for the export (prefixing with app/area or not) inside the global conf array (however this option being admin specific) and thereby enabling us to know if we need to worry about non unique define_names in different apps/areas or not.
 [2004-04-22 09:01 UTC] smith at backendmedia dot com
Ok I have deleted the unique setting. I am not in favor of handling the uniqueness in the export method. Can someone handle this?
 [2004-04-22 11:19 UTC] mscifo
I agree with Simon that one should not rely on the database to catch development errors. However, I think we shouldn't completely get rid of unique keys just because of that. Personally I think this issue has had too much thought put into it. We want to ensure that a right_define_name is unique, but only in its own area. Example: APP1_AREA1_READ //ok APP1_AREA2_READ //ok APP1_AREA1_READ //already exixts! To ensure this uniqueness we can simply use multi column unique constraints. In 'liveuser_rights' table: UNIQUE KEY `right_define_name` (`area_id`, `right_define_name`) In 'liveuser_areas' table: UNIQUE KEY `area_define_name` (`application_id`, `area_define_name`) In 'liveuser_applications' table: UNIQUE KEY `application_define_name` (`application_define_name`) Is this too simple of a solution or am I missing something?
 [2004-04-22 11:35 UTC] smith at backendmedia dot com
You are probably right .. this does seem like to way to go .. I had my mind twisted into all sorts of unusual ways.
 [2004-05-13 05:34 UTC] smith at backendmedia dot com
This bug has been fixed in CVS. In case this was a documentation problem, the fix will show up at the end of next Sunday (CET) on pear.php.net. In case this was a pear.php.net website problem, the change will show up on the website in short time. Thank you for the report, and for helping us make PEAR better.