[Pki-devel] [PATCH] pki-ftweedal-0015-Monitor-database-for-changes-to-LDAP-profiles.patch

[Endi Sukma Dewata]

On 4/8/2015 2:40 AM, Fraser Tweedale wrote:
> On Mon, Apr 06, 2015 at 08:27:05PM -0500, Endi Sukma Dewata wrote:
>> On 3/30/2015 2:27 AM, Fraser Tweedale wrote:
>> Let's say a profile is modified then deleted. These operations are
>> replicated to a clone which is monitoring the changes using persistent
>> search. When the clone receives the modify operation it will call
>> readProfile(), which will call createProfile(), which will create an
>> LDAPConfigStore for that profile. When LDAPConfigStore is created, it will
>> read the profile from the database using a separate query. The problem is,
>> at that point it's unclear whether the profile still exists.
>>
>> The current code might ignore the error if the entry doesn't exist, but in
>> general I don't think the code should make a separate query to retrieve the
>> profile info because it could lead to inconsistencies. It should have used
>> the LDAP entry returned by the persistent search that corresponds to the
>> operation being processed.
>>
> Okey doke, new patch attached.  The LDAPConfigStore no longer reads
> the entry on construction, and the LDAPProfileSubsystem reads the
> config from the incoming persistent search results and loads it into
> the LDAPConfigStore.

ACK.

There are some minor issues, they can be addressed separately:

1. The try-catch blocks inside the while loop in run() probably can be 
combined. If an error occurs and it's still (!stopped) it will sleep for 
a while before continuing.

2. The profileId should be obtained from the cn in the DN instead of 
from the cn in the entry since the cn in the entry could have multiple 
values:

   String profileId = (String)
       ldapProfile.getAttribute("cn").getStringValues().nextElement();

3. In LDAPProfileSubsystem.createProfile() the mProfileIds is guarded 
against duplicates:

   if (!mProfiles.containsKey(id))
       mProfileIds.addElement(id);

but in ProfileSubsystem.createProfile() it's not:

   mProfileIds.addElement(id);

I think they should be consistent:

4. Related to #3, I think the mProfileIds can be replaced with 
mProfiles.keys() which contains unique profile IDs.

-- 
Endi S. Dewata