Mailing-List: contact kerby-help@directory.apache.org; run by ezmlm
Precedence: bulk
Reply-To: kerby@directory.apache.org
Message-ID: <55939AA6.4070603@gmail.com>
Date: Wed, 01 Jul 2015 09:45:42 +0200
From: =?UTF-8?B?RW1tYW51ZWwgTMOpY2hhcm55?= <elecharny@gmail.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9;
 rv:31.0) Gecko/20100101 Thunderbird/31.7.0
MIME-Version: 1.0
To: kerby@directory.apache.org
Subject: Re: [backend] AbstractIdentityBackend interface
References: 
 <CABzFU-cpd+DeC10HC0_TpSGmZ3RZLKzbMNV9+KRrRBiBawiuMg@mail.gmail.com>
	<8D5F7E3237B3ED47B84CF187BB17B66611B8AA97@SHSMSX103.ccr.corp.intel.com>
	<CABzFU-eLAAEeaJe+i32wL-RfBJ05uHE-pisy-TfbfKasezY6Tg@mail.gmail.com>
	<8D5F7E3237B3ED47B84CF187BB17B66611B8AEA4@SHSMSX103.ccr.corp.intel.com>
	<CABzFU-cERQvOjeoP2XtUtP0czPgSM-nMqmMskURr4b4TOPzkFA@mail.gmail.com>
	<8D5F7E3237B3ED47B84CF187BB17B66611B8B6BA@SHSMSX103.ccr.corp.intel.com>
	<CABzFU-cL+DPW6xoFn1k0L2A_sztNjHahVfsxqzxnBn_iL86jFg@mail.gmail.com>
	<8D5F7E3237B3ED47B84CF187BB17B66611B8B737@SHSMSX103.ccr.corp.intel.com>
	<8D5F7E3237B3ED47B84CF187BB17B66611B8B76A@SHSMSX103.ccr.corp.intel.com>
	<5593937D.2070005@gmail.com>
 <CABzFU-e2wYtDLDaQVCJCjmvEto4MKbPU6YfdrFy=cH7eaWVCcA@mail.gmail.com>
In-Reply-To: 
 <CABzFU-e2wYtDLDaQVCJCjmvEto4MKbPU6YfdrFy=cH7eaWVCcA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Le 01/07/15 09:33, Kiran Ayyagari a écrit :
> On Wed, Jul 1, 2015 at 3:15 PM, Emmanuel Lécharny <elecharny@gmail.com>
> wrote:
>
>> Hi Kiran, Kai,
>>
>> just reading this thread (for some weird reason, my mail client wasn't
>> refreshing this mailing list, so I have missed it from day one...)
>>
>> I have reviewed the code and I'd like to add my perception.
>>
>> * the doXXX method
>>
>> I don't see anything wrong on having protected doXXX methods in such a
>> code, where you have some Interfaces describing the contract, some
>> implementation classes and an abstract layer that gather some common
>> beahviour and delegates what is specific to the implementation classes
>> through doXXX methods.
>>
>> As Kai says, this is a very common pattern.
>>
>> I think that Kiran expressed his concern - which is real, but bear with
>> me, I'll come to that later - the wrong way, by pointing to those doXXX
>> in his first mail. IMHO, the name is not a problem, and I interpret
>> Kiran's first mail this way : "the doXXX methods, as part of an API, are
>> less readable than XXX". Kai, you correctly explained that those doXXX
>> methods are not part of the API
>>
>> * the Cache
>>
>> Now, let's get to the real pb : what Kiran says, and I think he is
>> correct, is that the only reason those doXXX methods exists is for the
>> abstract class to implement a cache, which will b updated after the
>> impelmentation doXXX method has been called.
>>
>> Let's do some mental exercise now : let say we don't have a cache at
>> all. In this case, we immediatly see that those doXXX methods are
>> useless. We can simply replace the doXXX methods in the implementations
>> by the XXX methods, which is part of the API, instead of having to go
>> through the abstract class.
>>
>> So far, so good. But what if we need a cache, then ? That's a very valid
>> concern. I would expect that, for performances reason, we don't pound
>> the backend every time we need to get an identity. The cache serves this
>> purpose, AFAICT. So let's say xe need a cache.
>>
>> At this point, this is the real question Kiran is raising : if we need a
>> cache, where should this cache been implemented ?
>>
>> * When and where should we implement a cache ?
>>
>> That's an important point. There are two reasons for having a cache :
>> because we want to answer fast to a client request, and because the
>> backend does not have such a cache.
>>
>> Now, that's one of Kiran's point : as he is to implement a Mavibot
>> implementation, he does not need a cache, as Mavibot already implement
>> this cache. His suggestion then is quite natural : we should have two
>> categories of abstract classes : one with a cache, one with no cache,
>> and the inheritance schema should be like :
>>
>> (IdentityBackend/IdentityService)
>>   o
>>   |
>>   +-- [[AbstractIdentityBackend]]
>>            ^             ^
>>            |             |
>>            |             +-- [[AbstractCacheableIdentityBackend]]
>>            |                                 ^
>>            |                                 |
>>            |                                 +-- [JsonIdentityBackend]
>>            |                                 |
>>            |                                 +-- [LdapIdentityBackend]
>>            |                                 |
>>            |                                 +-- [ZookeeperIdentityBackend]
>>            |                                 |
>>            |                                 +-- [InMemoryIdentityBackend]
>>            |
>>            +-- [MavibotIdentityBackend]
>>
>> This is my understanding of Kiran's proposal.
> yes, you did, but a minor difference, the idea is not to have an
> AbstractCacheableIdentityBackend
> but instead make a cache based backend that contains a cache and accepts
> another instance of
> backend and delegates the calls to the wrapped backend instance when a
> cache miss occurs
> or when an update needs to be performed.
>
> In the end all those backedns that actually persist data are free from
> cache and server
> finally instantiates a cacheable backend(which takes another persisting
> backend instance).
>
That's definitively an option.