Personally I would leave as is. Having multiple roles/authorities per action, is kind of useful if you want to extend roller. The overhead is also minimal compared with what struts does internally.
On 14 August 2014 23:35, Glen Mazza <[email protected]> wrote: > Does anyone else on the team have a view? #5 below I'm no longer sure on > so I'm withdrawing that proposal but feedback on #1-#4 below is most > welcome. > > Glen > > On 08/13/2014 04:27 PM, Dave wrote: > >> I don't see the need for this change and I would leave those permissions >> in >> place. They existed to support and may still be used to support real uses >> cases like private blogging, where only registered users can see blogs and >> only those with special permissions can comment. >> >> Even if they do not work fully now, they give people a way to hook their >> own rules into their own custom versions of Roller, perhaps by adding new >> code, ServletFilters, etc. And they are a starting point for people who >> want private blogging to be fully supported in Roller. >> >> - Dave >> >> >> >> >> On Wed, Aug 13, 2014 at 3:51 PM, Glen Mazza <[email protected]> wrote: >> >> OK, checking Global Permission, we have these three levels: >>> >>> /** Allowed to login and edit profile */ >>> public static final String LOGIN = "login"; >>> >>> /** Allowed to login and do weblogging */ >>> public static final String WEBLOG = "weblog"; >>> >>> /** Allowed to login and do everything, including site-wide admin */ >>> public static final String ADMIN = "admin"; >>> >>> We don't use "weblog" though, we save it as "editor" in the userrole >>> table. We also don't use "login" for anything other than to make it the >>> minimum required setting on pages that don't require an ADMIN setting. >>> All >>> newly registered users are given "editor" as a minimum, meaning we could >>> raise minimum from "login" to "editor" and do away with the login role >>> without any difference in application behavior. >>> >>> On top of this, we allow the roles additional subroles per the >>> roller.properties file: >>> >>> # Role name to global permission action mappings >>> role.names=editor,admin >>> role.action.editor=login,comment,weblog >>> role.action.admin=login,comment,weblog,admin >>> >>> "comment" is also never used in the application, further, in the above >>> list we're inconsistently assigning admin to admin but weblog to editor. >>> Since the permissions are all Russian doll (login < comment < >>> weblog/editor < admin), it's sufficient to just store the highest role, >>> as >>> the lower ones are all implied, i.e., we don't need these properties. >>> >>> My proposal is to: >>> >>> 1.) Replace the above LOGIN/WEBLOG/ADMIN strings with a two-value >>> enumeration, EDITOR and ADMIN. Later, if we have user demand for LOGIN >>> and >>> COMMENT, and somebody actually coding in logic that uses those values, we >>> can easily add in the enumerations for them. (I don't like LOGIN much, >>> however, if we don't trust them not to blog they shouldn't be lurking >>> around the UI.) >>> >>> 2.) The "userrole" database table will be dropped, replaced with a new >>> varchar column ROLE in the Roller_User table. I'll update the migration >>> script to copy the user's highest role into that column. >>> >>> 3.) The three properties "role.name, role.action.editor, and >>> role.action.admin" will be removed. >>> >>> 4.) List<String> requiredGlobalPermissionActions() will return a single >>> enumeration constant instead (EDITOR or ADMIN), stating the minimum >>> accepted value. >>> >>> 5.) WeblogPermissions looks fine, except I'll just switch the string >>> array >>> of EDIT_DRAFT, ADMIN, POST, to an enumeration constant with the same >>> values >>> and have requiredWeblogPermissionActions() return an enumeration >>> constant >>> instead. >>> >>> How does this sound? I have other things to work on so I'll wait 72 >>> hours >>> before proceeding to give time for others to evaluate this change. >>> >>> Regards, >>> Glen >>> >>> >>> >>> On 08/13/2014 08:33 AM, Glen Mazza wrote: >>> >>> If the methods return just a single permission instead of a collection >>>> of >>>> permissions, at least for GlobalPermissionActions, that means we can >>>> move >>>> to "Russian doll" type role levels, where each permission level includes >>>> all the permission levels below it (de facto the way Roller runs now). >>>> If >>>> we can officially be on that, that means we can toss out the userrole >>>> table >>>> and just place a single column "rolename" (indicating the highest role a >>>> person has) in the roller_user table, a very sleek change. (I'm not >>>> talking about Roller_Permission, i.e., permissions a user has on each >>>> blog >>>> -- that table is still needed, but the userrole table indicating whether >>>> one's a global admin or not.) >>>> >>>> Glen >>>> >>>> On 08/13/2014 07:54 AM, Dave wrote: >>>> >>>> I don't have a strong opinion, but this seems like change just for the >>>>> sake >>>>> of change. I doubt that impacts performance in any significant way, >>>>> especially when compared to all the database calls that are made during >>>>> JSP >>>>> or page template processing. >>>>> >>>>> - Dave >>>>> >>>>> >>>>> On Tue, Aug 12, 2014 at 9:15 PM, Glen Mazza <[email protected]> >>>>> wrote: >>>>> >>>>> Hi team, one or both of these methods are heavily called within the >>>>> >>>>>> application, indeed for almost every action run: >>>>>> >>>>>> public List<String> requiredWeblogPermissionActions() { >>>>>> return Collections.singletonList(WeblogPermission.xxxxx); >>>>>> } >>>>>> >>>>>> public List<String> requiredGlobalPermissionActions() { >>>>>> return Collections.singletonList(GlobalPermission.xxxxxx); >>>>>> } >>>>>> >>>>>> I've checked every implementation of both methods within the >>>>>> application >>>>>> -- about 20-25 in all -- every one returns just a single permission >>>>>> requirement, not a list of items. >>>>>> >>>>>> I think it would be good to optimize these methods by having them >>>>>> return >>>>>> just a string or a fast and lightweight enumeration constant. The only >>>>>> thing lost I can see would be the ability to require multiple >>>>>> permissions, >>>>>> but again within the app today and through 12-14 years of Roller it >>>>>> just >>>>>> hasn't been needed. WDYT? >>>>>> >>>>>> Regards, >>>>>> Glen >>>>>> >>>>>> >>>>>> >>>>>> >
