On 27 Nov 2010, at 15:27, Jan Lehnardt wrote:
> Hi Chris,
>
> Good change, comments inline.
>
> On 27 Nov 2010, at 07:17, Chris Anderson wrote:
>
>> On Fri, Nov 26, 2010 at 8:41 PM, <[email protected]> wrote:
>>> Author: jchris
>>> Date: Sat Nov 27 04:41:20 2010
>>> New Revision: 1039619
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1039619&view=rev
>>> Log:
>>> rename "readers" to "members" in _security object, keep backwards
>>> compatibility with old security objects"
>>
>>
>> I've updated the _security object API to use "members" instead of
>> "readers" because readers is misleading. Please review this commit.
>> The only tricky part is the backwards compatibility so that existing
>> databases and security objects are still functional. We can remove the
>> backwards compatibility code for 2.0 if we want.
>>
>> Chris
>>> […]
>>> Modified: couchdb/trunk/src/couchdb/couch_db.erl
>>> URL:
>>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db.erl?rev=1039619&r1=1039618&r2=1039619&view=diff
>>> ==============================================================================
>>> --- couchdb/trunk/src/couchdb/couch_db.erl (original)
>>> +++ couchdb/trunk/src/couchdb/couch_db.erl Sat Nov 27 04:41:20 2010
>>> @@ -26,7 +26,7 @@
>>> -export([set_security/2,get_security/1]).
>>> -export([init/1,terminate/2,handle_call/3,handle_cast/2,code_change/3,handle_info/2]).
>>> -export([changes_since/5,changes_since/6,read_doc/2,new_revid/1]).
>>> --export([check_is_admin/1, check_is_reader/1]).
>>> +-export([check_is_admin/1, check_is_member/1]).
>>> -export([reopen/1]).
>>>
>>> -include("couch_db.hrl").
>>> @@ -77,7 +77,7 @@ open(DbName, Options) ->
>>> case couch_server:open(DbName, Options) of
>>> {ok, Db} ->
>>> try
>>> - check_is_reader(Db),
>>> + check_is_member(Db),
>>> {ok, Db}
>>> catch
>>> throw:Error ->
>>> @@ -297,14 +297,14 @@ check_is_admin(#db{user_ctx=#user_ctx{na
>>> ok
>>> end.
>>>
>>> -check_is_reader(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db)
>>> ->
>>> +check_is_member(#db{user_ctx=#user_ctx{name=Name,roles=Roles}=UserCtx}=Db)
>>> ->
>>> case (catch check_is_admin(Db)) of
>>> ok -> ok;
>>> _ ->
>>> - {Readers} = get_readers(Db),
>>> - ReaderRoles = couch_util:get_value(<<"roles">>, Readers,[]),
>>> + {Members} = get_members(Db),
>>> + ReaderRoles = couch_util:get_value(<<"roles">>, Members,[]),
>>> WithAdminRoles = [<<"_admin">> | ReaderRoles],
>>> - ReaderNames = couch_util:get_value(<<"names">>, Readers,[]),
>>> + ReaderNames = couch_util:get_value(<<"names">>, Members,[]),
>>> case ReaderRoles ++ ReaderNames of
>
> I'd s/ReaderRoles/MemberRoles for naming consistency.
>
>
>>> [] -> ok; % no readers == public access
>>> _Else ->
>>> @@ -326,8 +326,10 @@ check_is_reader(#db{user_ctx=#user_ctx{n
>>> get_admins(#db{security=SecProps}) ->
>>> couch_util:get_value(<<"admins">>, SecProps, {[]}).
>>>
>>> -get_readers(#db{security=SecProps}) ->
>>> - couch_util:get_value(<<"readers">>, SecProps, {[]}).
>>> +get_members(#db{security=SecProps}) ->
>>> + % we fallback to readers here for backwards compatibility
>>> + couch_util:get_value(<<"members">>, SecProps,
>>> + couch_util:get_value(<<"readers">>, SecProps, {[]})).
>>>
>>> get_security(#db{security=SecProps}) ->
>>> {SecProps}.
>>> @@ -343,9 +345,11 @@ set_security(_, _) ->
>>>
>>> validate_security_object(SecProps) ->
>>> Admins = couch_util:get_value(<<"admins">>, SecProps, {[]}),
>>> - Readers = couch_util:get_value(<<"readers">>, SecProps, {[]}),
>>> + % we fallback to readers here for backwards compatibility
>>> + Members = couch_util:get_value(<<"members">>, SecProps,
>>> + couch_util:get_value(<<"readers">>, SecProps, {[]})),
>
> How about making this
>
> Members = get_members(SecProps);
>
> by doing:
>
> get_members(#db{security=SecProps}) ->
> get_members_int(SecProps);
> get_members(SecProps) ->
> get_members_int(SecProps),
get_members_int(SecProps).
>
> get_members_int(SecProps) ->
> % we fallback to readers here for backwards compatibility
> couch_util:get_value(<<"members">>, SecProps,
> couch_util:get_value(<<"readers">>, SecProps, {[]})).
>
>
>>> ok = validate_names_and_roles(Admins),
>>> - ok = validate_names_and_roles(Readers),
>>> + ok = validate_names_and_roles(Members),
>>> ok.
>>>
>>> % validate user input
>>>
>>>
>>>
>
> Cheers
> Jan
> --
>
